Skip to content
sdlc

T-7AR0-orchestrate-change-detection-watch-command

Status: closed/done · Impact: high · Complexity: large

Goal

Section titled “Goal”

/sdlc:orchestrate runs hands-off, wrapped in /loop, re-firing on a wall-clock ScheduleWakeup cadence (~270 s busy / ~1800 s idle per the skill’s Step 7 delay table; the intro’s “~20 min” is the stale side of an internal inconsistency this task also corrects). Every tick is an LLM transcript, and Step 2 fans out one /sdlc:pr-check sub-agent per open PR — tens of thousands of tokens each — even when nothing changed.

This task adds a deterministic sdlc orchestrate watch op that decides, in code with no LLM in the loop, whether actionable work is pending, so a Monitor poll wrapper wakes the orchestrator only when there is. Drivers, ranked:

  1. Determinism (P-0001-prefer-deterministic-over-llm, S-0004-sdlc-cli-llm-head-deterministic-tail): the “should the orchestrator run?” decision moves from LLM judgment into a deterministic op with pinned semantics for every branch — first poll, errors, empty state. Any branch left undefined is a place LLM judgment leaks back into the loop, so the contract below pins all of them.
  2. LLM token cost: idle ticks burn an orchestrator transcript plus per-PR sub-agents to conclude “nothing to do” (~48 idle ticks/day at the 1800 s cadence). Gating wakes on pending work zeroes the idle cost; emitting the identities of pending work (PR numbers, task basenames) lets a woken tick scope Step 2 to the affected PRs instead of fanning out across all of them — the dominant remaining token cost once idle ticks are gone.
  3. API economy (secondary, still real): the poll must not cost more than the loop it gates. REST conditional requests (ETag/If-None-Match — 304 responses are rate-limit-free) and ?since= deltas keep idle polls ~free. GraphQL exhaustion was observed once and is not the driver, but the fetch path must not recreate it — note gh pr list/gh pr view are GraphQL-backed, so the op uses REST via gh api.

Design stance: level-triggered, not edge-triggered. The op reports work pending now — predicates over observable state — rather than “state changed since the last poll.” Pending-ness clears when the substrate state the tick already mutates reflects handling (a cursor advances, a task leaves open/ready, close-out removes branch+worktree): the system’s own writes are the acknowledgment channel. Consequences: a wake whose tick crashes re-fires while the work remains (redelivery is free), there is no prior-snapshot baseline to lose or corrupt, and the first poll needs no special case — it simply evaluates the predicates.

Today

Section titled “Today”
LocationRole today
plugin/skills/orchestrate/SKILL.mdThe tick skill. Step 2 dispatches one /sdlc:pr-check per open PR (enumerated with --limit 50); Step 3 categorizes in-flight via sdlc task inflight; Step 4 dispatches against the dispatcher filter — sdlc task sort --status open/ready --exclude-autonomy human-only, minus Step 3’s in-flight list; the loop re-fires on ScheduleWakeup regardless of state. Internal cadence inconsistency: intro says ~20 min idle, Step 7’s operative table says ~1800 s idle / ~270 s busy. No event gate exists.
plugin/lib/services/pr/ops/classify.tsThe per-PR classifier (sdlc pr classify). Fetches via gh pr view --json … (GraphQL), returns MERGED/CLOSED/CONFLICTS/CI-FAILED/NEEDS-RESPONSE/CLEAN/ERROR, writes the cursor unconditionally. newEntries(state, cursorSeenAt, authorMode, prAuthorLogin, selfPostedAt) is the comment/review filter. Its bot skip is the literal author === "github-actions[bot]" at two sites (comments loop ~:263, reviews loop ~:279) — and live GraphQL data carries suffix-less bot logins (github-actions), so the literal never matches in production: github-actions and cloudflare deploy-preview noise both pass today. REST payloads carry the suffixed shape (github-actions[bot]); any fix must be shape-normalized.
plugin/lib/services/pr/ops/classify.ts#readCursorPer-PR cursor at .sdlc/pr-cursors/<pr>.json: last_seen_comment_at, last_invoked_at, self_posted_at[]. It advances only when classify runs — which under level-triggering is exactly right: the cursor is the ack that a tick has surfaced the comments, not a liveness hazard.
plugin/lib/model/entities/task/ops/inflight.tssdlc task inflight — enumerates in-flight tasks. Its PR probe is one gh pr list --search head:<branch> per task (_probe_core.ts#openPrForBranch) — fine once per tick, too hot for a poll loop; the watch op reuses the single open-PR list instead.
plugin/lib/model/entities/task/ops/sort.tssdlc task sort — deterministic pickup order; repeatable --status, --exclude-autonomy. The dispatchable set is sort’s output under the dispatcher filter, minus in-flight basenames.
plugin/skills/pr-check/post_self_comment.shAppends reply timestamps to self_posted_at so the orchestrator’s own comments aren’t mistaken for feedback. The watch op honors the same array via the shared filter.
plugin/lib/services/orchestrator/Already owns the orchestrate CLI noun: ops/log-tick.ts registers path: ["orchestrate", "log-tick"] and owns .sdlc/orchestrator-log.md. The watch op joins this service dir (no new services/orchestrate/ twin). Op tests live in orchestrator/tests/.
plugin/lib/registry.ts + plugin/cli/registry_adapter.tsOp discovery walk (defaultOpsRoots: model/entities/*/ops, services/*/ops) — a new op file registers with no CLI edits. cli.render may shape exit codes; OP_ERROR_EXIT_CODES reserves 1–9 (2 = CAS_FAILED) — no ad-hoc codes. Commander pitfall: --no-… flags parse into the un-prefixed attribute and are unobservable through the adapter readback (a noWrite key can never reach the handler); the framework’s global --dry-run → ctx.dryRun is the compute-without-persisting channel.
Harness Monitor toolEach stdout line is an event; exit ends the watch; default 5-min timeout unless persistent: true; monitors that emit too many events are auto-stopped. So the op must be wrapped in a never-exiting loop, stdout is the only per-poll channel (exit codes are not), and emission must be ≤1 line per poll.
.gitignoreThe .sdlc/* blanket (line 43) already ignores any new state file. No change needed.

Proposed

Section titled “Proposed”

A new deterministic op sdlc orchestrate watch at plugin/lib/services/orchestrator/ops/watch.ts (joining the existing orchestrate noun beside log-tick): a normal defineOp with --output text|json|jsonl and no LLM in the loop. It evaluates three pending-work predicates, each defined over observable state and each naming the state change that clears it (its ack):

  1. pr-gone — an in-flight task’s PR is merged or closed but close-out has not landed (task not closed/*, branch/worktree still present). Ack: /sdlc:task-close-out completing. Merged-vs-closed is NOT distinguished in the signal — the woken tick re-classifies and dispatches close-out either way, so the distinction would buy nothing; the one-off per-departed-PR state read this predicate needs is rare and cheap.
  2. pr-attention — an open PR has ≥1 comment/review that survives the shared filter (non-bot per the normalized matcher below, not in self_posted_at, authorMode honored from sdlc.yaml pr_check.author_comments, prAuthorLogin from the list payload) and is newer than the cursor’s last_seen_comment_at. Ack: a tick’s classify advancing the cursor. Inherited limits, stated plainly rather than implied: top-level comments and review submissions only (review-thread replies are not fetched today), createdAt/submittedAt freshness only (comment edits do not signal).
  3. task-ready — the dispatchable set is nonempty, computed with the dispatcher’s exact filter (--status open/ready --exclude-autonomy human-only, minus in-flight basenames) so a human-only or already in-flight task never wakes the loop. Ack: dispatch flips the task to in-progress. Local filesystem only; zero API cost.

Known blind spots (explicit): CI-FAILED and CONFLICTS transitions, lease expiry, and the dead-stop notification have no comment/PR-set footprint and are NOT detected by these predicates. The fixed-cadence /loop tick is therefore retained at a long reconciliation interval (suggested 2–6 h) as the normative safety net — not an optional extra. (A pr-red predicate via ETag’d REST check-runs is a possible follow-up, out of scope here.)

Emit policy (--output text): at most ONE stdout line per poll:

PENDING reasons=<comma-list> prs=<sorted numbers> tasks=<sorted basenames>

emitted when (a) the canonical pending-set differs from the last emitted set, or (b) the set is nonempty and --re-emit-after seconds (default 1200) have passed since the last emit — the bounded retry that redelivers after a crashed or dropped tick. Empty pending set: never emits. Carrying identities is what lets the woken tick scope Step 2 to the listed PRs instead of all open PRs.

Exit codes: 0 on success whether pending or empty — stdout presence is the single signal (the Monitor wrapper cannot consume per-poll exit codes, and a set -e wrapper must not die on the common no-work path). Usage errors use the framework’s existing codes. On an upstream gh failure: print nothing to stdout, diagnostics to stderr, exit 0 — degrade silently rather than fabricate state or kill the wrapper loop. No new exit codes are minted (OP_ERROR_EXIT_CODES reserves 2–9, e.g. 2 = CAS_FAILED; anything semantic would have to be registered there, not invented per-op).

--output json / jsonl: always emit { pending: boolean, reasons: string[], prs: number[], tasks: string[], emitted: boolean } and exit 0 (framework default).

State file: .sdlc/orchestrator-watch.json (the orchestrator-* family, beside orchestrator-log.md), holding { last_emitted, last_emit_at, etags }. It is emission bookkeeping plus HTTP cache, NOT a change baseline: deleting it costs at most one duplicate wake (level-triggering recomputes pending-ness from live state). Resolved against ctx.projectRoot (matching classify’s cursor-dir handling); --state-path overrides it for tests; the global --dry-run (ctx.dryRun) computes and prints without writing. Already gitignored by the .sdlc/* blanket.

Fetch path (API economy, secondary): REST via gh api with conditional requests — GET repos/{o}/{r}/pulls?state=open&per_page=100 with If-None-Match (304 = unchanged at zero rate-limit cost; reuse the cached projection), the list’s updated_at as pre-filter so only PRs that moved get comment reads, then GET …/issues/{n}/comments?since=<cursor> and GET …/pulls/{n}/reviews for the survivors. REST logins carry the [bot] suffix — handled by the normalized matcher. Canonical serialization of the pending-set reuses plugin/lib/services/lease/canonical_json.ts (which documents the integer-like-key ordering trap a naive stringify would hit).

Bot-filter fix (lands with this task; fixes the live incident): replace both literal sites in newEntries with a shared isIgnoredAuthor(login): normalize by stripping a trailing [bot], then match against {github-actions, cloudflare-workers-and-pages} ∪ the new optional sdlc.yaml list pr_check.ignored_authors (so the next project’s vercel/netlify/ dependabot needs config, not a plugin release). Works identically on GraphQL (suffix-less) and REST (suffixed) shapes; fixtures capture REAL payloads in both shapes rather than hand-authored ones; pr-check/SKILL.md’s “github-actions[bot] are always excluded” prose is corrected.

Approach

Section titled “Approach”
  1. Bot filter in plugin/lib/services/pr/ops/classify.ts: shared normalized isIgnoredAuthor + optional pr_check.ignored_authors read from sdlc.yaml (precedent: readAuthorCommentMode); apply at both sites; export for the watch op. Extend plugin/skills/pr-check/tests/classify_pr.test.ts with both-shape cases; add real-payload fixtures; correct pr-check/SKILL.md filter prose; extend plugin/schemas/sdlc-yaml.schema.json (pr_check is additionalProperties: false, so the key must be declared — and its description still cites the retired classify_pr.py; fix that while there).
  2. Extract the pure entry filter from newEntries — operating on (entries, cursorSeenAt, authorMode, prAuthorLogin, selfPostedAt) with normalized author handling — so classify and watch share one filter without watch importing classify’s fetch. Classify behavior unchanged (existing goldens stay green).
  3. watch.ts at plugin/lib/services/orchestrator/ops/watch.ts: REST fetch with ETag/since through the OpCtx gh seam (note: classify’s goldens stub via --mock-state CLI fixtures, which is a different mechanism — watch tests use the actual DI seam: in-process createCtx with a fake gh runner); the three predicates; canonical pending-set; emit policy + state file; ctx.dryRun.
  4. Step-2 scoping + event-gated wake in plugin/skills/orchestrate/SKILL.md: a new “Event-gated wake (Monitor)” section giving the exact wrapper — Monitor(command: "while true; do <sdlc> orchestrate watch; sleep 60; done", persistent: true), poll interval ≥30 s — documenting that stdout presence is the only signal, that a wake SHOULD scope Step 2 to prs= when present, and that the fixed-cadence /loop is demoted to the long-interval reconciliation tick with an explicit list of what only it covers (CI-FAILED, CONFLICTS, lease expiry, dead-stop notify). Fix the intro-vs-Step-7 cadence inconsistency. Preserve ALL of invariants.yaml’s required phrases, sections, and tool refs and avoid its forbidden phrases — the gate pins ~24 phrases, two H2s, and a tool ref, not just two phrases.
  5. Tests at plugin/lib/services/orchestrator/tests/watch.test.ts (in-process, fake gh, --state-path to a temp file): empty set → no output, exit 0; comment newer than cursor → PENDING … pr-attention prs=<n>; signal persists across polls and survives state-file deletion (level-triggered redelivery); clears when the cursor advances; bot comments in BOTH shapes → no signal; self_posted_at → no signal; authorMode: self-notes honored; task-ready honors the dispatcher filter (human-only and in-flight excluded) and clears on dispatch; in-flight PR merged → pr-gone until close-out state observed; re-emit only after --re-emit-after; 304 path reuses the cached projection; gh failure → silent stdout + exit 0; --dry-run writes nothing; json schema match.

Files to touch

Section titled “Files to touch”
LocationKindChange
plugin/lib/services/orchestrator/ops/watch.tsnewThe watch op: predicates, emit policy, state file, REST+ETag fetch.
plugin/lib/services/orchestrator/tests/watch.test.tsnewIn-process tests per Approach 5.
plugin/lib/services/pr/ops/classify.tsmodifyShared normalized isIgnoredAuthor (+ sdlc.yaml list) at both sites; extract the pure entry filter; export both.
plugin/skills/pr-check/tests/classify_pr.test.ts + plugin/skills/pr-check/fixtures/modifyBoth-shape bot cases; real-payload fixtures.
plugin/skills/pr-check/SKILL.mdmodifyCorrect the bot-exclusion prose to the normalized matcher + config list.
plugin/skills/orchestrate/SKILL.mdmodify”Event-gated wake (Monitor)” section, Step-2 scoping, reconciliation-cadence framing, cadence-figure fix; keep invariants.yaml green.
plugin/schemas/sdlc-yaml.schema.jsonmodifyAdd pr_check.ignored_authors; fix the stale classify_pr.py description.

(No .gitignore change — the .sdlc/* blanket already covers the state file.)

Acceptance criteria

Section titled “Acceptance criteria”
  • AC-1: With an empty pending set, sdlc orchestrate watch prints nothing and exits 0; two consecutive polls over identical state emit at most one line total (dedup against last-emitted), and their --output json blocks are byte-identical (canonical serialization).
  • AC-2: An in-flight task’s PR merging (or closing) yields PENDING with reason pr-gone and the PR number; the signal persists across polls until close-out is observable, then clears.
  • AC-3: A non-bot, non-self comment newer than the PR’s cursor yields pr-attention with the PR number; persists until the cursor advances; deleting the state file does not lose the signal (level-triggered redelivery).
  • AC-4: Comments authored github-actions, github-actions[bot], cloudflare-workers-and-pages, and cloudflare-workers-and-pages[bot] all produce no signal; unit tests cover the normalized matcher and the pr_check.ignored_authors config path; existing classify goldens stay green.
  • AC-5: task-ready uses the dispatcher filter: a human-only task entering open/ready, or a ready task already in flight, produces NO signal; a dispatchable one does, and it clears on dispatch.
  • AC-6: Output discipline: ≤1 stdout line per poll; the line carries prs=/tasks= identities; re-emit fires only after --re-emit-after seconds while the set is nonempty.
  • AC-7: The op registers via the discovery walk beside log-tick (sdlc orchestrate watch --help works with no plugin/cli/sdlc.ts edits).
  • AC-8: Exit code is 0 for pending, empty, and upstream-gh-failure states (failure prints nothing to stdout, diagnostics to stderr); no new exit codes outside OP_ERROR_EXIT_CODES.
  • AC-9: bun test passes including the new suites; bunx tsc --noEmit is clean; sdlc gate skill-prose (orchestrate + pr-check invariants) passes.

Out of scope

Section titled “Out of scope”
  • The Monitor wiring itself — the SKILL.md section provides the exact wrapper text, but this task does not execute it.
  • Removing the fixed-cadence /loop: it is retained and required as the long-interval reconciliation tick (this replaces the earlier “additive” framing with a specified composition).
  • A pr-red (CI-FAILED/CONFLICTS) predicate — covered by the reconciliation tick; possible follow-up via ETag’d check-runs polling.
  • Review-thread replies and comment-edit detection (inherited pr-check limits, now stated in the contract rather than silently implied).
  • Webhooks / push delivery; non-GitHub forges.
  • Changing /sdlc:pr-check’s verdict enum or the per-PR cursor schema.

Dependencies

Section titled “Dependencies”
  • Existing substrate, all on main: classify’s cursor + comment filter (plugin/lib/services/pr/ops/classify.ts), sdlc task sort (dispatcher filter) and sdlc task inflight (in-flight linkage concepts), services/orchestrator/ (noun + tests home), lease/canonical_json.ts (stable serialization), the registry discovery walk, and the global --dry-run → ctx.dryRun channel. No blocking task dependency.

Discovery context

Section titled “Discovery context”

Observed in a consuming project running /loop /sdlc:orchestrate: many consecutive idle ticks each spawned per-PR pr-check sub-agents (tens of thousands of tokens) only to re-confirm “no change”; the cloudflare-workers-and-pages deploy-preview bot generated continuous false-positive “new comment” wakeups (root cause now understood: the existing bot filter compares a [bot]-suffixed literal against suffix-less GraphQL logins and never matches — github-actions noise passed too); GraphQL rate-limit exhaustion also occurred (a secondary driver — addressed by the REST/ETag fetch path).

Revised after the adversarial review on PR #422 (2026-06-11): motivation re-ranked per author (determinism > LLM token cost > API quota); design moved from an edge-triggered digest-diff to level-triggered pending-work predicates (review findings addressed: cursor-coupled digest deadlock/echo-wakes, at-most-once delivery, undefined first poll, Monitor contract mismatch, --no-write unobservable through Commander, orchestrate noun collision, bot-filter login-shape mismatch, dispatcher-filter mismatch, CI/CONFLICTS blind spots made explicit with a normative reconciliation cadence).

← Back to Tasks