From c0f872aa67843105e21ed3e7535b39792b59cffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Fran=C3=A7a?= Date: Mon, 3 Aug 2026 15:02:07 -0300 Subject: [PATCH] feat(orchestrate): delegate the slice and map the failure class Closes #362 --- CONTEXT.md | 2 +- .../orchestrate/orchestrate-mcp/dist/index.js | 6 +- .../orchestrate-mcp/src/tools/routing.ts | 4 +- .../src/tools/validate-envelope.ts | 7 +- .../orchestrate/skills/orchestrate/SKILL.md | 355 ++++++++++-------- .../references/failure-handling.md | 62 ++- .../orchestrate/references/preflight-mode.md | 2 +- .../orchestrate/references/run-state.md | 20 +- .../orchestrate/references/slice-pipeline.md | 299 +++------------ .../orchestrate/references/wave-loop.md | 5 +- 10 files changed, 328 insertions(+), 434 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index a188bfb3..f66ea6df 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -244,7 +244,7 @@ The standing constraint — expressed as a `## Scope-boundary guard` section in _Avoid_: scope check, brief filter (the guard is a positive constraint on what the brief may contain, enforced at two points — inside the investigator definition and at the orchestrator boundary) **Orchestrator judgment spine**: -The irreducible body of the orchestrate `SKILL.md` that remains after **MCP-first decomposition** — the roles & safety boundary, the two-axis complexity-tier assessment, wave-concurrency policy, failure-cause narration, and checkpoint/resume semantics. It is the residue that cannot be extracted to an `orchestrate-mcp` tool or a subagent because it is non-mechanizable orchestration judgment. After the #275 procedural-prose relocation the spine lands near ~425 lines — refined down from the ~750 the decomposition first projected — which is now *within* the project's 500-line `SKILL.md` body cap; the documented over-cap exception for this spine remains on record (ADR-0013) so it is never flagged as bloat should its judgment grow back over the cap, and is orchestrate-specific (not generalized to other skills). Deterministic procedure is extracted to MCP tools (no execution-permission prompt); judgment-bearing procedure is relocated to on-demand `references/` (loaded only when its phase runs, outside the smart zone); only judgment stays in the always-loaded spine. See ADR-0013. +The irreducible body of the orchestrate `SKILL.md` that remains after **MCP-first decomposition** — the roles & safety boundary, the two-axis complexity-tier assessment, wave-concurrency policy, the slice-executor briefing contract, failure-cause narration, the read boundary on slice-internal artifacts, and checkpoint/resume semantics. It is the residue that cannot be extracted to an `orchestrate-mcp` tool or a subagent because it is non-mechanizable orchestration judgment. The #275 procedural-prose relocation refined it to ~425 lines — down from the ~750 the decomposition first projected — but the ADR-0017 delegation then moved the intra-slice procedure out to the slice executor while adding the briefing contract, the structured envelope-recovery path, and the read boundary, leaving the spine at **542 lines of body**. It therefore *exercises* the documented over-cap exception recorded in ADR-0013, explicitly and at that measured figure, rather than merely holding it in reserve; the exception is orchestrate-specific (not generalized to other skills). Deterministic procedure is extracted to MCP tools (no execution-permission prompt); judgment-bearing procedure is relocated to on-demand `references/` (loaded only when its phase runs, outside the smart zone); only judgment stays in the always-loaded spine. See ADR-0013. _Avoid_: orchestrator core, skill body (the spine is specifically what remains after extraction, not the whole file or its runtime) **Routing variant**: diff --git a/plugins/orchestrate/orchestrate-mcp/dist/index.js b/plugins/orchestrate/orchestrate-mcp/dist/index.js index 5c738ff8..d1e77952 100755 --- a/plugins/orchestrate/orchestrate-mcp/dist/index.js +++ b/plugins/orchestrate/orchestrate-mcp/dist/index.js @@ -21980,7 +21980,7 @@ var routingConfigSchema = external_exports.object({ "Run-wide policy: how to process the independent slices within one wave. 'parallel' (default) spawns all processable slices at once and integrates them sequentially. 'sequential' processes slices one at a time in issue-id ascending order, refreshing the umbrella base between each so slice N branches from base+slice1..N-1 \u2014 guaranteed conflict-free, at the cost of serializing the wave. Optional; the three tier blocks remain required." ), continuationBudget: external_exports.number().int().min(0).default(2).describe( - "How many times the orchestrator may re-spawn the implementer in the same worktree after an 'incomplete' envelope (re-spawns BEYOND the initial run). 0 disables continuation (incomplete FAILs immediately, the legacy behavior). Defaults to 2." + "How many times the slice executor may re-spawn the implementer in the same worktree after an 'incomplete' envelope (re-spawns BEYOND the initial run). 0 disables continuation (incomplete FAILs immediately, the legacy behavior). Defaults to 2." ) }); var routingConfigSchemaV1 = routingConfigSchema; @@ -22048,7 +22048,7 @@ var runConfigSchema = external_exports.object({ "Run-wide policy: how to process the independent slices within one wave. 'parallel' (default) or 'sequential'. Lifted from the v1 top-level key." ), continuationBudget: external_exports.number().int().min(0).optional().default(2).describe( - "How many times the orchestrator may re-spawn the implementer in the same worktree after an 'incomplete' envelope. 0 disables continuation. Defaults to 2. Lifted from the v1 top-level key." + "How many times the slice executor may re-spawn the implementer in the same worktree after an 'incomplete' envelope. 0 disables continuation. Defaults to 2. Lifted from the v1 top-level key." ) }); var routingConfigSchemaV2 = external_exports.object({ @@ -23573,7 +23573,7 @@ var sliceExecutorEnvelopeSchema = external_exports.object({ "Prose description of what happened, in the executor's own words. Complements `failureClass` (the closed-set machine label) with the specific detail a human or the next executor needs. Absent for a 'completed' envelope." ), reportPath: external_exports.string().describe( - "Path, relative to the worktree root, of the slice's report \u2014 the human-readable artifact the executor wrote describing its own run." + "Path, relative to the run directory (`.orchestrate/runs//`), of the slice's report \u2014 the human-readable artifact the executor wrote describing its own run. It is written beside the executor's progress record, NEVER into the worktree, where the Changeset scope check would see it as an undeclared change." ), nextTaskBriefing: external_exports.string().describe( "Advice carried forward to whoever picks up the next slice. This is advice only, never a selection of WHICH slice runs next \u2014 wave ordering and loop termination stay computed by `plan_waves` and wave exhaustion, not declared here (see the module-level note above)." diff --git a/plugins/orchestrate/orchestrate-mcp/src/tools/routing.ts b/plugins/orchestrate/orchestrate-mcp/src/tools/routing.ts index fd4466ef..e8275f3c 100644 --- a/plugins/orchestrate/orchestrate-mcp/src/tools/routing.ts +++ b/plugins/orchestrate/orchestrate-mcp/src/tools/routing.ts @@ -123,7 +123,7 @@ export const routingConfigSchema = z.object({ .min(0) .default(2) .describe( - "How many times the orchestrator may re-spawn the implementer in the " + + "How many times the slice executor may re-spawn the implementer in the " + "same worktree after an 'incomplete' envelope (re-spawns BEYOND the " + "initial run). 0 disables continuation (incomplete FAILs immediately, " + "the legacy behavior). Defaults to 2." @@ -384,7 +384,7 @@ export const runConfigSchema = z.object({ .optional() .default(2) .describe( - "How many times the orchestrator may re-spawn the implementer in the " + + "How many times the slice executor may re-spawn the implementer in the " + "same worktree after an 'incomplete' envelope. 0 disables continuation. " + "Defaults to 2. Lifted from the v1 top-level key." ), diff --git a/plugins/orchestrate/orchestrate-mcp/src/tools/validate-envelope.ts b/plugins/orchestrate/orchestrate-mcp/src/tools/validate-envelope.ts index 92662c9f..92889cf0 100644 --- a/plugins/orchestrate/orchestrate-mcp/src/tools/validate-envelope.ts +++ b/plugins/orchestrate/orchestrate-mcp/src/tools/validate-envelope.ts @@ -344,8 +344,11 @@ export const sliceExecutorEnvelopeSchema = z.object({ reportPath: z .string() .describe( - "Path, relative to the worktree root, of the slice's report — the " + - "human-readable artifact the executor wrote describing its own run." + "Path, relative to the run directory (`.orchestrate/runs//`), of " + + "the slice's report — the human-readable artifact the executor wrote " + + "describing its own run. It is written beside the executor's progress " + + "record, NEVER into the worktree, where the Changeset scope check " + + "would see it as an undeclared change." ), nextTaskBriefing: z .string() diff --git a/plugins/orchestrate/skills/orchestrate/SKILL.md b/plugins/orchestrate/skills/orchestrate/SKILL.md index e25f5310..2953d81b 100644 --- a/plugins/orchestrate/skills/orchestrate/SKILL.md +++ b/plugins/orchestrate/skills/orchestrate/SKILL.md @@ -1,6 +1,6 @@ --- name: orchestrate -description: Implement a backlog of ready-for-agent GitHub issues end to end — order them into dependency waves, run implementer and reviewer subagents in isolated worktrees, merge slice pull requests into an umbrella branch, and checkpoint progress so an interrupted run resumes. Use when the user wants to autonomously orchestrate agent-driven implementation of tracked issues, or invokes /orchestrate in one of its three modes — a normal run (/orchestrate or /orchestrate ); /orchestrate clean (including --force, or --failed ) to remove the footprint of concluded or crashed runs; or /orchestrate preflight , the pre-flight pass that stages and inspects a run's setup before the wave loop. +description: Implement a backlog of ready-for-agent GitHub issues end to end — order them into dependency waves, delegate each slice to a slice-executor subagent in an isolated worktree, merge slice pull requests into an umbrella branch, and checkpoint progress so an interrupted run resumes. Use when the user wants to autonomously orchestrate agent-driven implementation of tracked issues, or invokes /orchestrate in one of its three modes — a normal run (/orchestrate or /orchestrate ); /orchestrate clean (including --force, or --failed ) to remove the footprint of concluded or crashed runs; or /orchestrate preflight , the pre-flight pass that stages and inspects a run's setup before the wave loop. --- # Orchestrate @@ -15,12 +15,20 @@ orchestration judgment that cannot be extracted to an `orchestrate-mcp` tool or subagent. Deterministic procedure lives in the MCP tools; the step-by-step operational mechanics of each phase live in on-demand `references/*.md`, loaded only when that phase runs. Read the spine top-to-bottom for the decision -narrative; follow each pointer into its reference for the mechanics. (The spine -is the irreducible judgment residue; after the #275 procedural-prose relocation -it lands near ~425 lines, **within** the project's 500-line `SKILL.md` cap. A -documented over-cap exception for this spine remains on record — see `CONTEXT.md` -and ADR-0013 — so the spine is never flagged as bloat should its judgment grow -back over the cap; do **not** generalize that exception to any other skill.) +narrative; follow each pointer into its reference for the mechanics. + +**On this file's length.** The spine is the irreducible judgment residue, and it +is **over** the project's 500-line `SKILL.md` cap: **542 lines of body** (546 +total, less 4 lines of frontmatter — the cap is on the body, so that is the +number being reported). It therefore **invokes the documented over-cap exception +recorded in ADR-0013 and `CONTEXT.md`**, explicitly and at that measured figure. +The ADR-0017 delegation is what put it here: handing the intra-slice procedure to +the slice executor removed the mechanics from section 3, but the spine acquired +three things it never carried before — the executor's briefing contract, the +structured recovery path for an unusable envelope, and the read boundary that +keeps slice-internal artifacts closed to the orchestrator. Those are judgment, +not procedure, so they belong here rather than in a reference. Do **not** +generalize this exception to any other skill. ## Roles and the safety boundary @@ -28,37 +36,44 @@ back over the cap; do **not** generalize that exception to any other skill.) shell operation: branches, worktrees, commits, pushes, pull requests, merges, and the `run-state.json` checkpoint. You also assess each issue's complexity tier and route each role accordingly. -- **Investigator** — the `investigator` subagent. For higher-complexity issues - only, it explores the codebase read-only and returns a research brief the - implementer builds on. -- **Implementer** — the `implementer` subagent. It edits code in an isolated - worktree and verifies it through the orchestrate capability tools. -- **Reviewer** — the `reviewer` subagent. It reviews the implemented slice in - the same worktree, fixes issues inline, re-runs the capability tools, and - gates the auto-merge. -- **Conflict-resolver** — the `conflict-resolver` subagent. When a slice - conflicts with the umbrella branch, it edits the conflicted files to a - correct merged state. It is spawned once per conflicting slice. +- **Slice executor** — the `slice-executor` subagent, and the only one you spawn + per slice. It owns one issue from investigation through to a verified + changeset, inside the worktree you created for it. Its operating procedure is + the `slice-pipeline` skill preloaded by its own definition, so none of that + procedure lives here. +- **Investigator**, **Implementer**, **Reviewer** — the workers the *executor* + spawns, not you. The investigator explores read-only and returns a research + brief; the implementer edits code in the worktree; the reviewer reviews it + there, fixes issues inline, and re-runs the capability tools. +- **Conflict-resolver** — the `conflict-resolver` subagent, spawned by **you**, + once per conflicting slice. When a slice conflicts with the umbrella branch it + edits the conflicted files to a correct merged state. It stays yours because a + conflict is between two branches, and branches are git. Every role except the orchestrator exists in two variants — `-standard` and `-deep`. The `resolve_routing` tool picks the variant and model per role from the issue's complexity tier **and its routing labels** (section 3, step 2). Each role is spawned by -its **namespaced** subagent type — `orchestrate:investigator-`, -`orchestrate:implementer-`, `orchestrate:reviewer-`, and +its **namespaced** subagent type — `orchestrate:slice-executor-`, +`orchestrate:investigator-`, `orchestrate:implementer-`, +`orchestrate:reviewer-`, and `orchestrate:conflict-resolver-`, where `` is `standard` or `deep`. The `orchestrate:` prefix is required: the plugin registers its bundled subagents under that namespace, so a bare, un-namespaced name fails to resolve. -All four subagents have **no Bash and no git access** — they are sandboxed to +All five subagents have **no Bash and no git access** — they are sandboxed to one worktree (the investigator is read-only). Only the orchestrator touches -branches, remotes, and the tracker. +branches, remotes, and the tracker; the executor's own instructions bind it to +the same boundary, and it never writes this run's checkpoint. **Routing labels — read once, frozen, suggested never applied.** A slice issue may carry a `route:*` label that patches its routing (e.g. `route:fable`, the label-gated implementer-only premium lane). `resolve_routing` reads those labels **exactly once**, at slice creation (section 3, step 2): the orchestrator passes the issue's labels to the tool and **freezes** the returned `{model, variant, -optional fallback}` into the slice's `resolvedRouting` checkpoint field. A +optional fallback}` into the slice's `resolvedRouting` checkpoint field. That +frozen block is exactly what the executor's briefing carries (section 3, step 3): +the executor never resolves routing itself, so freezing here is what keeps a +slice's routing from drifting mid-run. A resumed run routes from that frozen checkpoint, **never** from live GitHub labels — relabelling an issue mid-run changes nothing. The orchestrator may **suggest** a `route:*` label for a slice in its report but **never applies one @@ -176,52 +191,53 @@ time. Do **not** re-fetch the backlog, re-call `filter_to_one_parent_prd`, or re-call `partition_backlog`: a resumed run never widens or re-derives its own scope. Every slice in a terminal state (`passed`, `failed`, `skipped`) is left untouched — completed work is never redone. Every slice still `in-progress` was -interrupted before finishing; resume it **from its recorded `subState`** -(section 3 writes this at every per-slice transition) rather than re-processing -from scratch: **preserve** its `worktreePath` and `sliceBranch`, reconstruct its -changed-file set with `recover_changed_files`, **re-validate** the resume point -per the matrix below, then resume section 3 at the next uncompleted step -**without** re-spawning the subagents whose work the recorded `subState` already -captures. An in-progress slice with **no `subState`** (a legacy checkpoint) uses -the old discard path instead — discard its worktree+branch and coerce it back to -`pending`. The full discard mechanics and the run-discovery scan live in -`references/run-lifecycle.md`. Checkpoint the refreshed `run-state.json`, then -skip to section 2. - -**Resume re-validate matrix** — per the recorded `subState`; the worktree is -always preserved and the changed-file set always reconstructed via -`recover_changed_files` (accurate under #236's `-uall` recovery): - -| Recorded `subState` | Skip these subagents | Re-validate (cheap) | Resume at | -|---|---|---|---| -| (absent / legacy) | — | — | discard worktree+branch, coerce to `pending`, reprocess | -| `implemented` | investigator, implementer | run the capability gate (may have crashed mid-run) | step 5 (reviewer) after the `verified` gate | -| `verified` | investigator, implementer | re-run the capability gate (confirms worktree intact) | step 5 (reviewer) | -| `reviewed` | investigator, implementer, reviewer | re-run the capability gate | step 6 (commit + push) | -| `pushed` | implementer, reviewer | `git ls-remote --heads origin orchestrate/slice-` confirms the branch | step 7 (open PR) | -| `pr-open` | implementer, reviewer | `gh pr view` confirms the PR; `git ls-remote` confirms the branch | step 8 (merge) | -| `merged` | all subagents | — (merge already landed in umbrella) | step 9 only (label transition + `remove_worktree`) | +interrupted before finishing; resume it per the matrix below rather than +re-processing from scratch: **preserve** its `worktreePath` and `sliceBranch`, +and resume section 3 at the next uncompleted step. The full discard mechanics +and the run-discovery scan live in `references/run-lifecycle.md`. Checkpoint the +refreshed `run-state.json`, then skip to section 2. + +**Resume re-anchors on the slice's progress record, not on a fine-grained +`subState`.** The three intra-slice subStates you used to write — +`implemented`, `verified`, `reviewed` — mark stages you can no longer observe, so +a slice interrupted mid-execution carries **no `subState` at all** and its resume +point lives in the executor's own **slice progress record**. Ask for it through +`recover_slice_progress` (`runId` + issue number; it derives the path, and you +never open the file). The three that remain are the integration tail's, and they +resume exactly as they always did. + +| Recorded `subState` | Resume by | +|---|---| +| (absent) | Call `recover_slice_progress`. On `status: "ok"` a record exists, so the worktree holds real work — **preserve it** and re-spawn the slice executor with the same frozen `resolvedRouting`, the same progress-record path, and `executorContinuationIndex` incremented. The executor reads its own record and resumes from its `lastCompletedStage`; you do not tell it where to restart, and you do not re-run its capability gate. On `PROGRESS_NOT_FOUND` — no stage ever completed, or a legacy pre-delegation checkpoint — take the old discard path: discard worktree+branch, coerce to `pending`, reprocess. On `PROGRESS_INVALID` the record cannot be trusted: the slice has **FAILED**, with the tool's `errorMessage` in the `failureReason`. | +| `pushed` | `git ls-remote --heads origin orchestrate/slice-` confirms the branch → step 7 (open PR) | +| `pr-open` | `gh pr view` confirms the PR; `git ls-remote` confirms the branch → step 8 (merge) | +| `merged` | — (merge already landed in umbrella) → step 9 only (label transition + `remove_worktree`) | + +A **legacy intra-slice `subState`** — `implemented`, `verified`, `reviewed`, from +a checkpoint written before the delegation layer — still validates, and is +handled exactly as `(absent)`: those stages are no longer yours to resume into. **Resume routing is frozen, not re-derived (the fallback-aware dimension).** Orthogonal to the `subState` row above: when a resumed in-progress slice -**re-spawns** any subagent (the rows that do not skip the implementer/reviewer), -it routes **only** from the slice's frozen `resolvedRouting` checkpoint — its +**re-spawns** the executor, it routes **only** from the slice's frozen +`resolvedRouting` checkpoint — its recorded `model`, `variant`, and `fallback` — never from live GitHub labels and never by re-calling `resolve_routing`. A slice whose `resolvedRouting.fallbackTaken` -is `true` (a premium-spawned implementer that already failed over to the fallback -model in the prior session, section 3) resumes on that **frozen fallback model** +is `true` (a premium lane that already failed over to the fallback +model in a prior session) resumes on that **frozen fallback model** — the premium lane is **not** re-applied and the one-time fallback is **not** re-armed. This keeps routing deterministic across a handoff: the label was read once at slice creation, and the checkpoint — not the issue's current labels — is -the source of truth for every re-spawn. +the source of truth for every re-spawn. The executor's own record carries a +second `fallbackTaken` copy, because an executor cannot write your checkpoint; +the duplication is deliberate and the two are not to be unified. -On resume the implementer's *declared* `filesChanged` is gone, so the -reconstructed `recover_changed_files` set feeds the reviewer prompt and the -step-6 commit staging exactly as the live path uses the declared set. Do -**not** route reconstruction through `verify_changeset` (it needs a -`declaredFiles` argument that no longer exists on resume). For pre-push -subStates the capability gate is the re-validate; for `pushed`/`pr-open` it is -`git ls-remote` / `gh pr view`. +On a slice that failed without a usable envelope, the executor's declared +`filesChanged` is unavailable, so reconstruct the changed-file set with +`recover_changed_files` (accurate under #236's `-uall` recovery) — it is what +the failure record reports and, where the slice still integrates, what step 6 +stages. Do **not** route reconstruction through `verify_changeset`: that tool is +the executor's, and it needs a `declaredFiles` argument you do not have. ## 2. The wave loop @@ -276,36 +292,81 @@ starting the next slice. ## 3. Processing one slice -These are the per-slice steps the wave loop invokes. Update the slice's entry in -`run-state.json` and write the file at every state change. The step-by-step -procedure — create worktree, resolve routing, the investigator/implementer/ -reviewer spawn mechanics, the changeset scope check, the pre-merge capability -gate, commit+push via `finalize_slice`, the slice PR, merge, conflict resolution -via `resolve_merge_conflict`, and finishing via `finalize_slice` `post-merge` — -is in `references/slice-pipeline.md`. - -**The result-envelope trust chain.** Every subagent ends its turn with a **result -envelope** — a fenced ` ```orchestrate-envelope ` JSON block conforming to a -defined schema. The orchestrator determines a subagent's status and -changed-file set **only** from this validated envelope; it never reads the -subagent's prose. After each subagent (investigator, implementer, reviewer, -conflict-resolver) returns, call the `validate_envelope` MCP tool with the -subagent's verbatim returned text and its `role`: - -- `status: "valid"` — use the parsed `envelope` as the single source of the - subagent's outcome and `filesChanged`. +A slice is **five steps**, because you no longer run one. You prepare the ground, +delegate the whole slice to one executor, and act on the single envelope it +returns. Update the slice's entry in `run-state.json` and write the file at every +state change; the mechanics of steps 1, 2 and the integration tail are in +`references/slice-pipeline.md`. + +1. **Create the worktree** — `create_worktree`, at step 1 of the reference. + An error, including a failed `install`, is a FAILED slice. +2. **Resolve routing and freeze it** — `resolve_routing`, at step 2. Freeze the + per-role `{model, variant}` blocks (the `slice-executor` role among them), + the implementer's `fallback`, and `fallbackTaken: false` into + `resolvedRouting`, and keep the returned `continuationBudget`. +3. **Spawn the slice executor** — subagent type + `orchestrate:slice-executor-`, with the Agent `model` override taken + from `resolvedRouting["slice-executor"]`. Its briefing is below. +4. **Validate the returned envelope** — `validate_envelope` with the executor's + **verbatim** returned text and role `slice-executor`. +5. **Act on the result** — integrate it, or classify and label the failure. + +**Nothing happens between steps 3 and 4.** The executor returns exactly one +envelope, and you have no visibility into the stages that produced it, so you +write **no** `subState` while it runs — its own progress record holds that +granularity now. Resist the urge to narrate its stages; you do not know them. +(The one path that is not a return at all is a **spawn refusal**, which never +produces an envelope: `run_wave` `classify-spawn-outcome` decides +backpressure-versus-error, and that belongs to the wave loop.) + +**The briefing (step 3).** The executor starts with an empty context and is never +invoked by a person, so everything it needs must arrive here: the **issue** +number, title and body; the **acceptance criteria**, explicitly named as the hard +scope boundary, which it forwards to every worker it spawns; the **worktree +path**; the **run id** and issue number, which make its record self-identifying; +the frozen **`resolvedRouting`** with its `fallback` (a null `investigator` entry +means skip investigation); the **`continuationBudget`** from step 2; the **run +directory** to write its report into; the **progress-record path**; and its +**`executorContinuationIndex`** — 1-based, `1` on your first spawn for this +slice, incremented on each re-spawn. + +Two carry a reason worth stating, because getting them wrong is silent. +**Deriving the record's path is not reading it** — you compose +`.orchestrate/runs//slice--progress.json` and pass the string. +And the **index exists because neither party holds both factors of the +continuation bound**: two loops nest — yours re-spawning the executor, its own +re-spawning the implementer — and the bound is on their product. You know the +outer index and cannot know the inner count; a freshly spawned executor knows its +count and cannot know it is your second. Passing the index makes it the only +party that can evaluate the bound, which is why the formula lives there and not +here. + +**The result-envelope trust chain (step 4).** Every subagent ends its turn with a +**result envelope** — a fenced ` ```orchestrate-envelope ` JSON block conforming +to a defined schema. You determine its status and changed-file set **only** from +that validated envelope; you never read its prose. + +- `status: "valid"` — the parsed `envelope` is the single source of the slice's + outcome and `filesChanged`. - `status: "invalid"` (truncated, malformed, or off-schema) or - `status: "missing"` (no envelope emitted) — the subagent's result cannot be - trusted. The slice has **FAILED** (see *Failure handling*). A truncated - envelope is never silently accepted. - -This validated-envelope chain extends through the whole pipeline: the implementer -envelope is cross-checked against the worktree by `verify_changeset` (step 4a), -and after the reviewer returns `passed` the orchestrator runs its **own** -deterministic pre-merge capability gate (step 5a) rather than trusting the -reviewer's self-reported `verification` — the last link in the -`implementer → reviewer → orchestrator` trust chain. Each link's mechanics are -in `references/slice-pipeline.md`. + `status: "missing"` — the result cannot be trusted, but this is **not** an + immediate discard: the worktree may hold a finished investigation and review. + Recover through `recover_slice_progress` and branch exactly as the resume + matrix's `(absent)` row does. A truncated envelope is never accepted as + success. + +The chain is now two links, not four — the executor gates its own workers and +reports one settled outcome; validating that report is yours. Its +`nextTaskBriefing` is **advice only**, never a selection of what runs next. + +**You never open a slice-internal artifact.** Not the executor's report, not its +progress record. Use the envelope's fields for the outcome, pass `reportPath` +forward without opening it, and go through `recover_slice_progress` when the +envelope fails you — it derives the path from `(runId, issue)` and returns +validated structured data. A `PreToolUse` read guard enforces this, but it is +**defence in depth, not the rule itself**: enterprise policy can disable plugin +hooks, so this paragraph stays load-bearing and must not be deleted on the +grounds that the hook covers it. ## 4. Context handoff @@ -363,68 +424,45 @@ To hand off: ## Failure handling -A slice **FAILS** when `create_worktree` errors, a subagent's result envelope -is invalid or missing (`validate_envelope` returns `invalid` or `missing`), a -validated implementer envelope has `status: "blocked"` — or `status: "incomplete"` -**after** the continue-in-place loop exhausts the continuation budget or trips -the no-progress guard (a single `incomplete` no longer FAILs immediately; see -§3 step 4) — a validated reviewer envelope has `status: "failed"`, -`verify_changeset` reports -the implementer's declared file set does not match the worktree -(`empty-but-declared` or `suspiciously-empty`, or a `status: "error"`), the -staged changeset is empty, or a merge conflict the `conflict-resolver` cannot -fix. The orchestrator decides FAILURE **only** from the validated envelope and -tool results — never from a subagent's prose. An invalid or missing envelope is -always a FAILED slice; it is never treated as success. - -**Model fallback — one premium-spawn interception before FAILED.** A -**premium-spawned** implementer (one whose `resolvedRouting` carried a `fallback` -because a `route:*` label patched it — e.g. `route:fable`) gets **one** rescue -before the slice is declared FAILED. When such an implementer fails in a way the -fallback covers — a model **refusal**, a retention/safety **400**, or an -**invalid/missing envelope** — and the slice's `resolvedRouting.fallbackTaken` is -not yet set, the orchestrator **re-spawns it exactly once on the fallback model** -(`resolvedRouting.fallback.model`, e.g. `opus`) in the **same** worktree, sets -`resolvedRouting.fallbackTaken: true`, and narrates the swap in the final report -("fable declined → served by opus"). This swap is a **model exchange**, distinct -from the same-model continue-in-place loop: it does **not** consume or increment -`continuationBudget`, and `fallbackTaken` is a **persisted slice-level** once-only -guard (set in the checkpoint, surviving a handoff) — so the fallback fires at most -once across the initial spawn and every continuation. A fallback that is absent -(no premium label), already spent (`fallbackTaken` already `true`), or that does -not apply (a *valid* `blocked` envelope is a genuine obstacle the fallback model -would not fix) leaves the ordinary FAILED taxonomy above unchanged. The mechanics -— where the re-spawn runs and how the checkpoint is written — are in -`references/slice-pipeline.md` (the model-fallback step adjacent to step 4). - -The implementer envelope's `incomplete` status is the implementer's graceful -turn-budget self-report — partial, resumable work, carrying a `remainingWork` -handoff — as opposed to `blocked` (an unrecoverable obstacle) or an `invalid` -envelope (a hard turn-limit cutoff that truncated the envelope). Unlike `blocked` -and `invalid`, a single `incomplete` does **not** FAIL the slice: it drives the -bounded continue-in-place loop (§3 step 4), where the orchestrator re-spawns the -implementer in the same preserved worktree with the `remainingWork` until it -returns `completed` or the loop terminates. An `incomplete` slice FAILs **only** -when one of two terminal causes is reached: - -- **Budget exhausted** (resumable) — `continuationsUsed === continuationBudget` - and the last envelope is still `incomplete`. The `failureReason` names the - budget exhaustion ("implementer reported `incomplete` after exhausting the - continuation budget of N; partial work preserved in the worktree for - resumption"); label `needs-info`. -- **No progress** — a continuation returned `incomplete` whose worktree - content-fingerprint equals the prior one (the re-spawn changed nothing). The - `failureReason` names the no-progress stall; label `needs-triage`. - -In both terminal cases the `failureReason` must name the cause precisely so a -developer can tell a resumable budget exhaustion apart from a genuine stall. An -`incomplete` slice's worktree holds usable partial work — preserve it (as every -FAILED slice's worktree is preserved) so the slice can be resumed. - -Once the taxonomy above has classified a slice as FAILED, the orchestrator's -mechanical actions on it — surfacing the envelope's `rootCause`, setting `state` -to `failed` with a `failureReason` and the right `needs-info`/`needs-triage` -label, preserving the worktree, recovering the changed-file set via +**Classification descends; policy stays.** You no longer diagnose *why* a slice +failed — the executor does, because that is where the evidence was, and it +reports the diagnosis as a **failure class** drawn from a closed set it owns. You +map that class to a tracker label, because you are the single writer of tracker +state. Neither half is duplicated: the class set is defined once in +`validate_envelope`'s schema, and the class-to-label mapping once in +`references/failure-handling.md`, beside the `gh` commands that apply it. Do not +restate either here. + +So a slice **FAILS** in exactly three ways now: + +- **Before the executor** — `create_worktree` errors, or `resolve_routing` + returns `LABEL_CONFLICT` / `CONFIG_INVALID`. +- **From the executor** — a validated envelope whose `status` is not + `completed`. Read its `failureClass` and apply the mapping. A validated + `incomplete` is already terminal by the time it reaches you: the executor + exhausted its own bounded continuation before reporting, so there is nothing + for you to re-spawn on its behalf. +- **After the executor** — an envelope that will not validate and that + `recover_slice_progress` cannot rescue (the step-4 branch above), a staged + changeset that is empty, or a merge conflict the `conflict-resolver` cannot + fix. + +You decide FAILURE **only** from the validated envelope and tool results, never +from a subagent's prose. Two cases fall outside a plain class-to-label lookup — +a failure with no surviving `failureClass`, and the one class-plus-reason +combination that is an environment fault rather than a slice fault (the executor +reporting its own operating procedure was never preloaded). Both are rows of the +same `references/failure-handling.md` mapping; do not re-derive either here. + +**Model fallback surfaces, it does not run here.** The one-time premium swap now +happens **inside** the executor, which reports it as `fallbackTaken`. Narrate the +swap in the final report ("fable declined → served by opus") and carry the flag +into `resolvedRouting.fallbackTaken` so a resumed slice does not re-arm a rescue +already spent. + +Once a slice is classified FAILED, the mechanical actions on it — surfacing the +envelope's `rootCause`, setting `state` to `failed` with a `failureReason` and +the mapped label, preserving the worktree, recovering the changed-file set via `recover_changed_files` when the envelope was the failure cause, posting the triage comment, and continuing the wave — are in `references/failure-handling.md`. @@ -494,12 +532,15 @@ the `gh`-op half — together they close #230. Write the run's `run-state.json` — at `.orchestrate/runs//run-state.json` — after every slice state change and after every wave. In addition, write a -slice's `subState` at **every** section-3 per-slice transition -(`implemented` → `verified` → `reviewed` → `pushed` → `pr-open` → `merged`); -that fine-grained checkpoint is the **resume anchor** an interrupted in-progress -slice continues from (section 1), alongside the coarse-`state` and wave -checkpoints. Every write refreshes the top-level `updatedAt`, and a slice's own +slice's `subState` at every **integration-tail** transition +(`pushed` → `pr-open` → `merged`) — and at no other point, because the stages +between spawning the executor and validating its envelope are not yours to +observe. The schema still accepts the three retired intra-slice values +(`implemented`, `verified`, `reviewed`) so a checkpoint written before the +delegation layer still loads; a delegating orchestrator never writes one. +Every write refreshes the top-level `updatedAt`, and a slice's own `updatedAt` whenever its entry changes, so an artifact rendered from the file has accurate timestamps. The checkpoint is what makes a run resumable: an interrupted run, re-invoked, skips every terminal-state slice and resumes every -in-progress slice from its recorded `subState`. +in-progress slice per the resume matrix — from its recorded `subState` in the +integration tail, and from its progress record before that. diff --git a/plugins/orchestrate/skills/orchestrate/references/failure-handling.md b/plugins/orchestrate/skills/orchestrate/references/failure-handling.md index 68358bed..4baa2afe 100644 --- a/plugins/orchestrate/skills/orchestrate/references/failure-handling.md +++ b/plugins/orchestrate/skills/orchestrate/references/failure-handling.md @@ -1,28 +1,52 @@ # Failure handling — the FAILED-slice mechanical actions -The spine retains the failure-cause taxonomy and narration — the FAILED -definition, the `incomplete`-vs-`blocked`-vs-`invalid` distinction, the two -terminal `incomplete` causes (budget-exhausted / no-progress), and the SKIPPED -and other-stop-condition narration. This file holds the mechanical actions the -orchestrator performs **on a FAILED slice**, once the spine's taxonomy has -classified it. The continue-in-place loop mechanics that decide when an -`incomplete` slice FAILs live in `references/slice-pipeline.md` (section 3, -step 4). +The spine retains the failure-cause narration — where a slice can fail, the rule +that a verdict comes only from a validated envelope, and the SKIPPED and +other-stop-condition narration. This file holds the mechanical actions the +orchestrator performs **on a FAILED slice**: first the deterministic +failure-class-to-label mapping, then the actions that apply it. The bounded +continuation loop that decides when an `incomplete` slice fails is the slice +executor's, and lives in its own operating procedure. + +## The failure-class to triage-label mapping + +A slice executor classifies its own failure — that is where the evidence is — and +reports one **failure class** on its envelope. The orchestrator maps that class +to a tracker label, because it is the single writer of tracker state. This table +is that mapping's **only** home; the class set itself is defined once in +`validate_envelope`'s schema and is not restated here. + +| `failureClass` | Label | Why | +|---|---|---| +| `incomplete-budget-exhausted` | `needs-info` | Resumable partial work is preserved in the worktree — "resume me", not "diagnose me". | +| `no-progress-stall` | `needs-triage` | Repeated attempts converged on nothing; a human has to look. | +| `unrecoverable-obstacle` | `needs-triage` | A blocker with no safe workaround. | +| `invalid-or-missing-worker-envelope` | `needs-triage` | A worker's result could not be trusted. | +| `changeset-mismatch` | `needs-triage` | Declared files did not match the worktree. | +| `empty-changeset` | `needs-triage` | The slice produced no file changes. | +| `model-refusal` | `needs-triage` | A spawned model refused the task. | + +When **no class exists at all** — the executor's envelope was itself invalid or +missing and `recover_slice_progress` could not rescue the slice — use +`needs-triage`. The one case that is **not** a slice failure and must not be +labelled: an `unrecoverable-obstacle` whose `failureReason` names a missing +operating procedure is an environment fault; report it to the operator instead. + +The label vocabulary is the project's, not this tool's — swap the label column if +a project uses different triage labels. + +## The mechanical actions On a FAILED slice: -- When the failure cause is a validated worker envelope with `status: "blocked"` - (implementer) or `status: "failed"` (reviewer), that envelope now carries a - validated `rootCause` (`verified` | `hypothesis` + `claim` + optional - `evidence`) — surface it in the failure artifact alongside the `failureReason` - so a developer reads the subagent's own labelled diagnosis. +- When the failure cause is a validated envelope carrying a `rootCause` + (`verified` | `hypothesis` + `claim` + optional `evidence`) — surface it in the + failure artifact alongside the `failureReason` so a developer reads the + subagent's own labelled diagnosis. - Set its `state` to `failed` with a `failureReason`, checkpoint, and - transition the issue's tracker label. For a slice that failed because the - continue-in-place loop **exhausted the continuation budget** — partial, - resumable work — `needs-info` better signals "resume me" than `needs-triage`: - `gh issue edit --remove-label ready-for-agent --add-label needs-info`. - For the **no-progress** terminal cause (a continuation that changed nothing) - and every other failure cause, use `needs-triage`: + transition the issue's tracker label to the one the mapping above yields: + `gh issue edit --remove-label ready-for-agent --add-label needs-info` + or `gh issue edit --remove-label ready-for-agent --add-label needs-triage`. - Do **not** merge it. **Preserve its worktree** — leave it on disk for a developer to inspect. Do not call `remove_worktree`. diff --git a/plugins/orchestrate/skills/orchestrate/references/preflight-mode.md b/plugins/orchestrate/skills/orchestrate/references/preflight-mode.md index 3c0419e1..c54fe053 100644 --- a/plugins/orchestrate/skills/orchestrate/references/preflight-mode.md +++ b/plugins/orchestrate/skills/orchestrate/references/preflight-mode.md @@ -191,7 +191,7 @@ dirty — with `core.autocrlf` on, a regenerated file can surface as modified in Call `run_typecheck`, `run_build`, `run_tests`, and `run_lint` with the probe path as `repoPath`. **All four** — not the `run_build` + `run_tests` subset the -pre-merge gate uses (`references/slice-pipeline.md` step 5a). That subset +slice executor's own **Capability gate** uses. That subset deliberately narrows a gate on a worktree the reviewer has already seen; the probe is answering whether verification happens **at all**, so every verb the project relies on has to be exercised. diff --git a/plugins/orchestrate/skills/orchestrate/references/run-state.md b/plugins/orchestrate/skills/orchestrate/references/run-state.md index 5789c68a..9a155522 100644 --- a/plugins/orchestrate/skills/orchestrate/references/run-state.md +++ b/plugins/orchestrate/skills/orchestrate/references/run-state.md @@ -128,14 +128,19 @@ metadata, not source — the target project should gitignore - `blockedBy` — issue-id strings this slice depends on (drives the graph view). - `state` — see *Slice states* below. - `subState` — fine-grained position **within** §3 processing of an - `in-progress` slice, one of - `implemented|verified|reviewed|pushed|pr-open|merged`, written at every §3 - transition; the key is **absent before §3 step 4 completes — omit the key + `in-progress` slice. The schema accepts + `implemented|verified|reviewed|pushed|pr-open|merged`, but a **delegating** + orchestrator (ADR-0017) only ever *writes* the last three: `implemented`, + `verified` and `reviewed` marked intra-slice stages it no longer performs or + observes, and are retained in the enum solely so a checkpoint written before + the delegation layer still validates. The key is **absent for the whole span + between spawning the slice executor and its envelope validating — omit it entirely; an explicit `null` is rejected** (the schema `subState: subStateEnum.optional()` accepts an absent key but rejects a - literal `null`, so `"subState": null` fails `validate_run_state`). It is - the **resume anchor** for an interrupted in-progress slice - (see *Resume*). `pushed` is + literal `null`, so `"subState": null` fails `validate_run_state`). Across that + span the resume anchor is not this field but the slice's **progress record** + (below), reached through `recover_slice_progress`; from `pushed` onward + `subState` is the resume anchor again (see *Resume*). `pushed` is recorded **only after** `git ls-remote` confirms the branch landed; `merged` (slice PR squash-merged into the umbrella, step 8) precedes the slice reaching coarse `state: passed` (step 9). @@ -149,6 +154,9 @@ metadata, not source — the target project should gitignore field was introduced (backward-compatible). When present, it captures the routing that was frozen at slice creation so a resumed run routes the slice from the checkpoint rather than from live GitHub labels. Shape: + - `slice-executor` — `{model, variant}` for the slice executor, the role the + orchestrator actually spawns per slice (ADR-0017). A resumed run re-spawns + the executor from this entry rather than re-resolving routing. - `investigator` — `{model, variant}` for the investigator role, or `null` when this tier skips the investigation pass. - `implementer` — `{model, variant}` for the implementer role. diff --git a/plugins/orchestrate/skills/orchestrate/references/slice-pipeline.md b/plugins/orchestrate/skills/orchestrate/references/slice-pipeline.md index bfe0635a..fc5b50b5 100644 --- a/plugins/orchestrate/skills/orchestrate/references/slice-pipeline.md +++ b/plugins/orchestrate/skills/orchestrate/references/slice-pipeline.md @@ -1,16 +1,32 @@ -# Processing one slice — the per-slice pipeline - -These are the per-slice steps the wave loop invokes (section 3). The spine -retains the Result-envelope trust-chain narration (how every subagent outcome is -read **only** from its validated envelope); this file holds the step-by-step -procedure. Update the slice's entry in `run-state.json` and write the file at -every state change. - -Every subagent ends its turn with a result envelope; after each subagent -(investigator, implementer, reviewer, conflict-resolver) returns, call the -`validate_envelope` MCP tool with the subagent's verbatim returned text and its -`role`, then act on the validated `status`/`envelope` as the trust-chain -narration in the spine describes. +# Processing one slice — worktree, routing, and integration + +These are the per-slice steps the **orchestrator** performs around the slice +executor (section 3): the two that run **before** the executor is spawned — +creating the worktree and freezing the slice's routing — and the integration tail +that runs **after** its envelope validates. + +The intra-slice stages — investigation, implementation and its bounded +continuation loop, the changeset scope check, review, and the capability gate — +are **no longer here**. They belong to the slice executor, whose operating +procedure is the `slice-pipeline` skill preloaded by its subagent definition. The +orchestrator does not perform them and learns their outcome only from the +executor's validated result envelope. The step numbers are therefore +**deliberately non-contiguous**: steps 6–9 keep the numbers they have always had, +because other references cite them by number, and the gap at 3–5a is where the +executor's stages went. + +Update the slice's entry in `run-state.json` and write the file at every state +change. + +## Contents + +- **Step 1 — Create the worktree** — `create_worktree`, the bundled install +- **Step 2 — Resolve routing** — `resolve_routing`, and freezing the result +- **Step 6 — Commit and push** — `finalize_slice` phase `commit-push` +- **Step 7 — Open the slice pull request** — `gh pr create` +- **Step 8 — Merge the slice** — the mergeability gate +- **Step 8a — Resolve a merge conflict (once)** — `resolve_merge_conflict` +- **Step 9 — Finish the slice** — `finalize_slice` phase `post-merge`, pass label 1. **Create the worktree.** Set the slice `state` to `in-progress`, write its `sliceBranch` (`orchestrate/slice-`) and the `worktreePath` you will use @@ -30,16 +46,21 @@ narration in the spine describes. spawn, plus any label-resolved `fallbacks` and `warnings`: - `status: "ok"` — use the returned `routing`. **Freeze it into the slice's `resolvedRouting` checkpoint field** in `run-state.json` at slice creation: - the per-role `{model, variant}` blocks, and — because the Fable lane is - implementer-only — the implementer's entry from the returned `fallbacks` - array mapped into the single `resolvedRouting.fallback` `{model, maxRetries}` - (with `fallbackTaken: false`). Every later spawn and every resume routes - from this frozen checkpoint, never by re-reading labels (the resume-routing - principle in the spine's resume matrix). + the per-role `{model, variant}` blocks — **including the `slice-executor` + role**, which is what step 3 spawns and what a resumed run must re-spawn + without re-resolving — and, because the Fable lane is implementer-only, the + implementer's entry from the returned `fallbacks` array mapped into the + single `resolvedRouting.fallback` `{model, maxRetries}` (with + `fallbackTaken: false`). Every later spawn and every resume routes from this + frozen checkpoint, never by re-reading labels (the resume-routing principle + in the spine's resume matrix). Capture the result's `continuationBudget` + too — the executor's briefing carries it. - **`warnings[]`** (present on `ok`) — surface each in the run report. An unconfigured `route:*` label present on the slice (in the config's `labels` block) yields a loud **WARNING** here: the label had no effect, so the - operator can fix `routing.json` or drop the label. + operator can fix `routing.json` or drop the label. A routing config written + before the `slice-executor` role existed also warns here, having had that + role defaulted from the tier's own `implementer` entry. - `errorCode: "LABEL_CONFLICT"` — **two applied labels patch the same role** (there is no precedence rule). This is a loud **ERROR**: the slice has **FAILED**; report the conflicting labels so the operator resolves it in @@ -50,217 +71,11 @@ narration in the spine describes. no `fallback` (an unrouted slice has no premium lane). - `errorCode: "CONFIG_INVALID"` — the routing config is broken; the slice has **FAILED**. -3. **Run the investigator (higher tiers only).** If `routing.investigator` is - non-null, spawn the `orchestrate:investigator-` subagent — `` - and the Agent `model` override both come from `routing.investigator`. Its - prompt must carry the issue number/title/body **and the slice's acceptance - criteria explicitly named as the hard scope boundary** — the canonical per- - slice scope established by the backlog partitioner. The investigator must - not propose work that falls outside those acceptance criteria. Validate its - returned text with `validate_envelope` (role `investigator`); on `valid`, - **diff the returned brief against the acceptance criteria before forwarding - it to the implementer**: inspect the brief's `relevantFiles`, `approach`, - and `notes` for any work that does not trace to at least one acceptance - criterion. If the brief includes work from a sibling or downstream slice — - files, approaches, or recommendations that the acceptance criteria do not - require — the brief is over-scoped: treat it as a failed investigation pass - (the slice has **FAILED**). A brief that is correctly scoped to the - acceptance criteria is forwarded to the implementer as the research brief. - An `invalid` or `missing` envelope is a failed investigation pass — the - slice has **FAILED** (the investigator is read-only, so no worktree fallback - applies). If `routing.investigator` is null, skip this step. -4. **Run the implementer.** Spawn the `orchestrate:implementer-` - subagent — `` and the `model` override from `routing.implementer`. Its - prompt must carry the issue number/title/body, the worktree path (every - change goes there), the investigator's brief if one was produced, an - instruction to verify with the capability tools using the worktree path as - `repoPath`, a note that it MAY call `run_install` (worktree path as - `repoPath`) to fetch a newly-added dependency before re-verifying — and that - any lockfile that install mutates MUST be reported in `filesChanged` so it - lands in the slice diff — and a reminder not to commit, push, or run git. - Validate its returned text with `validate_envelope` (role `implementer`). On - a `valid` envelope, classify the envelope `status`: - - `completed` — proceed to the worktree scope check in step 4a. - - `incomplete` — the implementer's graceful turn-budget self-report: it - foresaw it could not finish within its remaining turns and stopped cleanly - with partial work recorded **and a `remainingWork` handoff**. Do **not** - fail the slice immediately. Instead run the **bounded continue-in-place - loop** below: re-spawn the implementer in the *same* worktree carrying the - `remainingWork`, until it returns `completed` or the continuation budget is - exhausted. The slice FAILs from `incomplete` only when the budget runs out - or the no-progress guard trips — see the loop and *Failure handling*. - - `blocked` — the implementer hit an unrecoverable obstacle: the slice has - **FAILED**. - - **Continue-in-place loop (on a `valid` `incomplete` envelope):** - - Read `budget = continuationBudget` from the step-2 `resolve_routing` result - (the resolved run-wide budget, default `2`; `0` disables continuation — - legacy immediate-FAIL). When `resolve_routing` returned `CONFIG_NOT_FOUND` - (no routing configured), there is no budget — use **0**. - - Initialize an in-session `continuationsUsed = 0` and capture a - **content-level fingerprint** of the worktree's uncommitted state: a hash - of `git -C diff HEAD` concatenated with the contents of the - untracked files listed by - `git -C ls-files --others --exclude-standard`. A filename-set - comparison is insufficient — the same file may be rewritten with real - progress or returned byte-identical. - - While `continuationsUsed < budget`: re-spawn `orchestrate:implementer-` - in the **same** worktree (same routing/model/variant) with a continuation - prompt = the issue, the worktree path, the PRIOR envelope's `remainingWork`, - the standard verify/no-git reminders, and an explicit "partial work is - already in the worktree — continue it, do not restart." Validate the - returned text with `validate_envelope` (role `implementer`). - - `completed` — proceed to the worktree scope check in step 4a. Loop done. - - `blocked`, or an `invalid`/`missing` envelope — the slice has **FAILED** - (record the precise cause). Loop done. - - `incomplete` again — recompute the fingerprint. If it **equals** the - prior fingerprint, the **no-progress guard trips**: the slice FAILs, its - `failureReason` names the no-progress stall, label `needs-triage`. - Otherwise increment `continuationsUsed`, update the stored fingerprint and - `remainingWork`, and loop. - - When `continuationsUsed === budget` and the last envelope is still - `incomplete`: the slice FAILs with a budget-exhausted `failureReason` - ("implementer reported `incomplete` after exhausting the continuation budget - of N; partial work preserved in the worktree for resumption"), label - `needs-info` (resumable). - - The counter and fingerprint are **loop-local** — nothing is persisted to - `run-state.json`. A mid-continuation context handoff/resume discards the - in-progress slice and rebuilds its worktree (§1), restarting the slice - clean; this is intentional. - - An `invalid` or `missing` envelope also means the slice has **FAILED** — a - hard turn-limit cutoff that truncates the envelope mid-emission lands here as - `invalid`, distinct from the graceful `incomplete` self-report above. - - **Model fallback (premium spawns only — runs BEFORE the FAILED verdict).** - This interception is **separate from** the continue-in-place loop above: the - loop re-spawns the *same* model on `incomplete` and counts against - `continuationBudget`; this step **swaps** the model exactly once on a *model - failure* and does **not** touch `continuationBudget`. It applies only to a - **premium-spawned** implementer — one whose frozen `resolvedRouting.fallback` - is set because a `route:*` label (e.g. `route:fable`) patched the implementer - in step 2. Before declaring such a slice FAILED from a step-4 or loop failure, - check whether the failure is one the fallback covers and whether the rescue is - still available: - - **Trigger set** — the implementer **refused**, returned a retention/safety - **400**, or emitted an **invalid/missing envelope**. A *valid* `blocked` - envelope is **excluded**: it is a genuine obstacle (e.g. a missing - dependency) the fallback model would not fix — keep it on the immediate - FAILED path. - - **Guard** — proceed only when `resolvedRouting.fallback` is set **and** - `resolvedRouting.fallbackTaken` is still `false`. If there is no fallback - (an ordinary, non-premium spawn) or it is already spent - (`fallbackTaken: true`), skip this step and apply the ordinary FAILED - taxonomy. - - **Re-spawn** — spawn `orchestrate:implementer-` **once** in the - **same** worktree, overriding the Agent `model` to - `resolvedRouting.fallback.model` (e.g. `opus`), with the standard prompt - (issue, worktree path, the investigator brief if any, verify/no-git - reminders; carry the prior `remainingWork` if the failure came from the - continuation loop). Set `resolvedRouting.fallbackTaken: true` in the slice - checkpoint and write `run-state.json` **before** the re-spawn, so the - once-only guard survives a mid-spawn handoff — `fallbackTaken` is a - **persisted slice-level** flag, not a loop-local counter, and the fallback - fires at most once across the initial spawn and every continuation. - - **Classify the fallback envelope** with `validate_envelope` (role - `implementer`) exactly as the initial spawn: `completed` → step 4a; - `incomplete` → re-enter the continue-in-place loop (the fallback model now - drives it, still bounded by `continuationBudget`); `blocked`, or an - `invalid`/`missing` envelope → the slice has **FAILED** (the one rescue is - spent). Record the swap for the final-report narration ("fable declined → - served by opus"; see `references/wave-loop.md`). -4a. **Verify the changeset against the worktree.** After a `completed` - implementer envelope — and before trusting it — call the `verify_changeset` - MCP tool with the slice's `worktreePath` and the implementer envelope's - `filesChanged` as `declaredFiles`. It inspects the worktree directly with - `git status` and compares the declared file set against what actually - changed on disk: - - `match: "matched"` or `"clean"` — the declared set agrees with the - worktree; proceed to step 5. - - `match: "empty-but-declared"` — the implementer declared files but the - worktree is clean: its edits never landed. The slice has **FAILED**. - - `match: "suspiciously-empty"` — the implementer declared nothing but the - worktree HAS changes: the work was under-reported. The slice has - **FAILED**; record the `presentButUndeclared` paths in the `failureReason`. - - `match: "mismatch"` — the declared set and the worktree changeset diverge. - Trust the worktree: use the **union** of the implementer's declared - `filesChanged` and the tool's `actualFiles` as the changed-file set for the - reviewer and the commit (step 6), and note the divergence - (`declaredButAbsent` / `presentButUndeclared`) so the reviewer sees it. - - `status: "error"` — the worktree could not be inspected; the slice has - **FAILED**. - Once the changed-file set is established (a `matched`/`clean`/`mismatch` - verdict), set the slice's `subState` to `implemented` and checkpoint - `run-state.json`; the completed implementer envelope also satisfies the - pre-review gate, so set `subState` to `verified` and checkpoint again before - spawning the reviewer. (These two adjacent checkpoints differ only in - resume granularity — the resume matrix in section 1 re-runs the capability - gate for both.) -5. **Run the reviewer.** Spawn the `orchestrate:reviewer-` subagent — - `` and the `model` override from `routing.reviewer` — in the same - worktree. Its prompt must carry the issue, the worktree path, the - changed-file set agreed on by step 4a — the implementer envelope's - `filesChanged` when `verify_changeset` matched, the union of declared and - `actualFiles` on a `mismatch` — the implementer envelope's `notes`, and the - investigator's brief if one was produced. Validate its returned text with - `validate_envelope` (role `reviewer`). On a `valid` envelope, an envelope - `status` of `failed` means the slice has **FAILED**; `passed` proceeds. An - `invalid` or `missing` envelope also means the slice has **FAILED**. On a - `passed` envelope, set the slice's `subState` to `reviewed` and checkpoint - `run-state.json` before proceeding to step 6. -5a. **Pre-merge capability gate.** After the reviewer returns `passed` (step 5), - and **before any commit, push, or GitHub state exists**, the orchestrator - independently runs the correctness capability tools on the slice worktree — - this is the pre-merge capability gate. It does **not** trust the reviewer's - envelope `verification`: the reviewer's re-run is a subagent self-report; - this step is the orchestrator's own deterministic check, the last link in the - `implementer → reviewer → orchestrator` trust chain. - - Call the `run_build` and `run_tests` MCP tools with the slice's - `` as `repoPath` (the same pattern step 8a's `clean`-verdict - re-verify uses). Each tool - returns a `status` enum (`passed | failed | not-configured | error`); handle - all four: - - `passed` on **both** verbs → proceed to step 6. - - `not-configured` (either verb) → **tolerated**, treated as a pass for that - verb (consistent with the prerequisites note that a missing-command - `not-configured` is tolerated). The gate must not fail a project that has - not configured `build`/`tests`. - - `failed` or `error` (either verb) → the slice has **FAILED** (the existing - FAILED semantics defined throughout section 3 — no new failure handling). - - **Known-baseline-failure hint (`knownFailureMatches`).** When a capability - tool returns `status: "failed"` and the project's `commands.json` configures a - `knownFailures` pattern list, the result carries - `knownFailureMatches.matched` (configured patterns that appeared in the - failing output) and `.unmatched` (configured patterns that did not). Use it - only as a **hint**, never as a verdict — it is a best-effort L1 annotation, - not a deterministic "zero new failures" assertion (`run_tests` returns capped - exit-code output, not a structured test-result list). When every failure - indicator in the output is explained by a `matched` pattern and `unmatched` - holds only not-present baseline cases, treat the failure as a **likely known - baseline** and proceed per this gate's baseline handling. When the failing - output contains indicators NOT covered by any `matched` pattern, - **spot-check** before treating it as baseline — L1 cannot deterministically - assert "0 new failures." A `knownFailures` entry in `commands.json` looks - like, e.g.: - - ```json - { "tests": ["npm", "test"], "knownFailures": ["flaky-network timeout", "ECONNRESET"] } - ``` +*Steps 3 through 5a are the slice executor's. See section 3 of the spine for +what the orchestrator does between step 2 and step 6: it spawns the executor, +validates one envelope, and acts on it.* - The verb set is exactly `run_build` + `run_tests` — a deliberate subset: - build+test is the correctness trust boundary, while `typecheck`/`lint` remain - the reviewer's quality remit and are intentionally **not** re-run here. The - step 8a `clean`-verdict (post-`prepare`) re-verify running all four - `run_tests`/`run_typecheck`/`run_build`/`run_lint` verbs is a **known, - intentional asymmetry** — and is left unchanged: this pre-merge gate is - focused correctness on a worktree the reviewer already saw, whereas the - conflict re-verify is max-confidence on a never-before-tested merged - combination. "Pre-merge" names what the gate controls (whether the merge - proceeds); mechanically it runs pre-commit, on the same worktree state the - reviewer validated. 6. **Commit and push.** Run the slice's commit + verified-push mechanics with the **`finalize_slice` MCP tool** in phase `commit-push` — not raw `git`. It stages the file set, guards an empty changeset, commits, composes @@ -270,19 +85,19 @@ narration in the spine describes. worktree), `runId`, `sliceId` = the slice's issue-id-string key, `branch` = `orchestrate/slice-`, `remote` = `origin`, `setUpstream: true`, `commitSubject` = `(): `, `issueNumber` = ``, and - `files` = the union of the `filesChanged` arrays from the validated - implementer and reviewer envelopes. + `files` = the slice executor envelope's `filesChanged` — every worker's edits + combined, as the executor reported them. `finalize_slice` stages **exactly** that `files` set (`git add -- ...files`, never `git add -A` — the capability tools leave untracked build artifacts in - the worktree). When the implementer fetched a new dependency with + the worktree). When an implementer fetched a new dependency with `run_install`, install ran **in its turn before this commit** and mutated the - lockfile (`pnpm-lock.yaml` / `package-lock.json` / `Cargo.lock`); because the - implementer declared that lockfile in `filesChanged`, it is in this staged - union and the commit captures it — so the new dependency lands in the slice - diff. The commit preserves the two-`-m` form (subject + `Closes #` - trailer), and the push goes through `push_and_verify`'s SHA-matched - `git ls-remote` landing check. + lockfile (`pnpm-lock.yaml` / `package-lock.json` / `Cargo.lock`); because that + lockfile was declared in `filesChanged` and carried into the executor's + envelope, it is in this staged set and the commit captures it — so the new + dependency lands in the slice diff. The commit preserves the two-`-m` form + (subject + `Closes #` trailer), and the push goes through + `push_and_verify`'s SHA-matched `git ls-remote` landing check. On `status: "ok"` (`verdict: "committed-pushed"`) the slice's `subState` was set to `pushed` and `run-state.json` checkpointed by the tool — and **only** @@ -337,7 +152,9 @@ narration in the spine describes. slice on a conflict without attempting resolution. 8a. **Resolve a merge conflict (once).** Attempt resolution exactly once — a - conflict the resolver cannot fix is a FAILED slice. + conflict the resolver cannot fix is a FAILED slice. This is the + orchestrator's own work, never the executor's: a conflict is between the + slice branch and the umbrella branch, so it is git. 1. Prepare the worktree for resolution with the **`resolve_merge_conflict` MCP tool** in operation `prepare`. Call it with `operation: "prepare"`, @@ -407,8 +224,8 @@ narration in the spine describes. then removes the worktree (force — it may hold untracked build artifacts), then force-reclaims the **local** slice branch (`git branch -D`, ordered after the removal because a checked-out branch refuses the delete). `merged` is the - integration-boundary anchor: a run resumed at `subState: merged` skips every - subagent and re-enters here at step 9 only, never re-merging. + integration-boundary anchor: a run resumed at `subState: merged` re-enters + here at step 9 only, never re-merging and never re-spawning the executor. On `status: "ok"` (`verdict: "committed-pushed"`) the merged checkpoint, worktree removal, and local-branch reclaim are all done. The tool is diff --git a/plugins/orchestrate/skills/orchestrate/references/wave-loop.md b/plugins/orchestrate/skills/orchestrate/references/wave-loop.md index 63c73f00..97efc657 100644 --- a/plugins/orchestrate/skills/orchestrate/references/wave-loop.md +++ b/plugins/orchestrate/skills/orchestrate/references/wave-loop.md @@ -261,8 +261,9 @@ pull request. **Narrate the routing notes in the per-slice summary.** For each slice whose `resolvedRouting.fallbackTaken` is `true`, note the **model-fallback swap** in its summary line — e.g. "slice #N: fable declined → served by opus" — so the -premium-lane fallover is visible in the report (the swap itself runs in -`references/slice-pipeline.md` step 4). Surface any routing-label +premium-lane fallover is visible in the report (the swap itself runs inside the +slice executor and is reported in its envelope's `fallbackTaken`). Surface any +routing-label **WARNING** (an unconfigured `route:*` label) or **ERROR** (a same-role `LABEL_CONFLICT`) from `resolve_routing` in the same summary, and — where the orchestrator judges a slice would have benefited from a premium lane — it may