diff --git a/.changeset/sturdy-elks-march.md b/.changeset/sturdy-elks-march.md new file mode 100644 index 000000000..cf61836c0 --- /dev/null +++ b/.changeset/sturdy-elks-march.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 4742 +--- +**A workflow can no longer declare a loop-host agent role belonging to another step family** — the Loop Host Contract assigns each of the five loop steps a role family (orchestration, planning, execution), but nothing enforced it: adding `orchestrator` to an execute step `agent-roles` line compiled cleanly, and capability contributions could then target the orchestrator at execution points. Contract generation now rejects a cross-family role, a role outside the vocabulary, and an unknown step, reporting every offender rather than the first. No existing workflow or capability changes. (#4740) diff --git a/CONTEXT.md b/CONTEXT.md index 9094f16f6..7240d0e54 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -357,7 +357,7 @@ Projects a pure, typed install plan for a given runtime by composing artifact pl A bundle delivering one optional GSD feature, toggled as a unit at install or after install. Owns its skills, agents, hooks, federated config-key schema (keys + defaults + validation), and loop extension-point registrations, plus a `requires` list of other Capabilities, plus an optional `activationKey` (a dotted config key, e.g. `graphify.enabled`) naming the config toggle that gates the whole capability — consumed by the Capability State Resolver's per-capability `active` (absent → no config gate; see Capability State Resolver tri-state deepening). Declared co-located in the Capability's own folder and compiled into a generated central Capability Registry at build time. The five-step loop (Discuss → Plan → Execute → Verify → Ship) and shared-infrastructure skills (phase, config, help, update, surface, progress) are the privileged host, not Capabilities, in v1 — but host extension points are data so a loop step can become a Capability under a future uniform kernel. Supersedes the implicit feature-scattering across clusters, install-profiles, and config-schema. Generalizes the Skill Surface Budget Module and Runtime Install Policy Module. ### Loop Host Contract -Generated description of what the five-step loop (Discuss → Plan → Execute → Verify → Ship) exposes as extension points: per-step loop points, agent roles, and core artifacts. Sourced from structured `` HTML-comment markers embedded near the top of each of the five step workflow files (`discuss-phase.md`, `plan-phase.md`, `execute-phase.md`, `verify-work.md`, `ship.md`). Generated by `scripts/gen-loop-host-contract.cjs` → `gsd-core/bin/lib/loop-host-contract.cjs` (ADR-894 §3 phase 3a-impl-2). Covers exactly the 12 canonical points (discuss:pre/post, plan:pre/post, execute:pre/wave:pre/wave:post/post, verify:pre/post, ship:pre/post). The generator enforces a drift guard: every declared non-orchestrator agent role must correspond to an actual agent reference in the workflow file. Consumed by `gen-capability-registry.cjs` (replaces the former inline `LOOP_HOST_CONTRACT` constant). Run `node scripts/gen-loop-host-contract.cjs --write` after editing a workflow step marker. +Generated description of what the five-step loop (Discuss → Plan → Execute → Verify → Ship) exposes as extension points: per-step loop points, agent roles, and core artifacts. Sourced from structured `` HTML-comment markers embedded near the top of each of the five step workflow files (`discuss-phase.md`, `plan-phase.md`, `execute-phase.md`, `verify-work.md`, `ship.md`). Generated by `scripts/gen-loop-host-contract.cjs` → `gsd-core/bin/lib/loop-host-contract.cjs` (ADR-894 §3 phase 3a-impl-2). Covers exactly the 12 canonical points (discuss:pre/post, plan:pre/post, execute:pre/wave:pre/wave:post/post, verify:pre/post, ship:pre/post). The generator enforces a drift guard: every declared non-orchestrator agent role must correspond to an actual agent reference in the workflow file. Since #4740 it additionally enforces a **role-family partition** (ADR-894 §3, Amendment 2026-09-14): every role belongs to exactly one of three disjoint families — orchestration (`orchestrator`), planning (`researcher`/`planner`/`checker`), execution (`executor`/`verifier`) — and each step admits exactly one family, so `execute:*` never admits `orchestrator` and the orchestration steps (`discuss`, `verify`, `ship`) never admit `executor`/`verifier`. A step MAY declare a strict subset of its family; it may never declare outside it, and an unknown step fails CLOSED (unlike the points coverage check, which defers an unknown step to its canonical-set and duplicate nets — roles have no second net). This governs what a WORKFLOW may DECLARE, and is distinct from §3's separate rule that a capability's `contribution.into` must be a member of the step's `agentRoles`, which governs what a CAPABILITY may TARGET and is unchanged. Consumed by `gen-capability-registry.cjs` (replaces the former inline `LOOP_HOST_CONTRACT` constant). Run `node scripts/gen-loop-host-contract.cjs --write` after editing a workflow step marker. ### Gate Predicate Evaluator Module Pure, deps-injected evaluator for capability gate `check.predicate` blocks (#2008, ADR-2008). Prior to #2008 the registry validator accepted `check.predicate` (one of exactly-one-of `query`/`predicate`/`agentVerdict`) and the loop-resolver rendered it, but nothing EVALUATED a declared predicate — only `check.query` was enforced (dispatched via `gsd_run check `), and built-in gates like `security` worked only via hard-coded `capId` prose branches in `ship.md`/`execute-phase.md`/`verify-work.md`. This module is the generic evaluation path: `evaluatePredicate(predicate, context, deps) → { block, message, details? }` dispatches by `predicate.kind` through a `KIND_TABLE`. Built-in kinds: `command-exit-zero` runs a declared command in a bounded `sh -c` subprocess (production binding: `shell-command-projection.execTool`) at the project root, inheriting env; exit 0 ⇒ pass, non-zero ⇒ block, timeout (SIGTERM) ⇒ block; `artifact-frontmatter-equals` resolves a phase or project-level Markdown artifact through injected dependencies and compares a frontmatter field by scalar string value. Command predicates interpolate `${PHASE_NUMBER}`/`${PHASE_DIR}`/`${PHASE_REQ_IDS}` from gate context. A THROWN error (malformed predicate, non-positive/non-finite timeout, command >4096 chars, missing dependencies, unknown kind) maps at the CLI seam to a non-zero check-command exit, which the workflow's two-step gate contract treats as a step-1 command failure routed per `onError` — so an evaluator bug is never conflated with a legitimate block decision. Leaf pure module (no fs/child_process/config — subprocess and artifact seams injected). CLI entry: `gsd_run check predicate --predicate '' [--phase-dir …] [--phase-number …] [--phase-req-ids …] --raw`, wired into `check-command-router.cts:cmdCheckPredicate`. `--phase-dir` is CONFINED to the project root at that router boundary (#4354, epic #4636 Phase 2): an unconfined value let a BLOCKING gate return `block: false` on an artifact in a directory the caller chose, and the same value interpolates into `${PHASE_DIR}` for `command-exit-zero`, so confining at the boundary covers BOTH kinds while keeping this module's fs-free leaf contract intact — a fix inside the evaluator would have required giving a pure module a filesystem dependency and would still have missed the interpolation path. The four generic workflow gate-dispatch sites (`execute:wave:post`, `execute:post`, `plan:post`, and `ship:pre` — the last wired by #3559, which had resolved gates generically but enforced only two hardcoded `capId`s) branch on `check` shape (`query` vs `predicate`). Source of truth: `src/gate-predicate-evaluator.cts`. Docs: `docs/reference/gate-predicates.md`, `docs/how-to/command-exit-zero-gate.md`. @@ -404,7 +404,7 @@ ADR-1244 Phase 4 (D5+D6) orchestration seam (`gsd-core/bin/lib/capability-lifecy ADR-1244 Phase 5 (D7) registry-driven dispatch of capability command families. First-party families (`graphify`/`intel`/`audit`, shipped in `bin/lib/`) dispatch via `dispatchCapabilityCommand` (`gsd-core/bin/gsd-tools.cjs`) against the FROZEN `capability-registry.cjs` `commandFamilies` (confined to `bin/lib/`) — unchanged. Third-party (installed overlay) families dispatch via `dispatchOverlayCapabilityCommand`: after the first-party path returns false, it calls `loadRegistry({ includeInstalled, cwd })` and dispatches a family iff its `capId` is in `_overlay.commandRoots` — which `capability-loader.cjs` populates ONLY for accepted overlay capabilities that declare `commands` AND pass the loader's activation gate (a **committed** ledger entry, present and non-`_pending`, PLUS — for PROJECT scope — a matching user consent record in the Capability Consent Store; GLOBAL scope needs no consent record). A bundle dropped on disk with no install (no ledger entry) or no on-this-machine consent is NOT command-dispatchable. The router module is `require()`'d FROM the capability's install root via `defaultRequireFromInstallRoot` (bare-`.cjs` basename + `realpath` containment, rejecting `..` traversal and symlink escape); same own-property/function/sync-only guards as the first-party path. Wired into the `runCommand` default arm before "Unknown command". A repo-planted project ledger no longer activates anything on its own (#1459) — see `docs/explanation/capability-trust-model.md` "project-scope trust boundary". ### Claude Orchestration Capability -Default-off, BETA, claude-only Capability (`capabilities/claude-orchestration/`, `role: feature`, `runtimeCompat.supported: ["claude"]`, `tier: full`, `activationKey: claude_orchestration.enabled`) adopting Claude Code's Workflow tool (the engine behind `/effort ultracode`, Agent SDK ≥ v0.3.149) as an optional parallel-execution backend for the GSD loop, and folding the `gsd-ultraplan-phase` plan-offload under the same runtime gate (#1143; ADR-1143). Pure, fail-closed core in `gsd-core/bin/lib/claude-orchestration.cjs`: `detectWorkflowBackend({ runtimeId, hostIntegration, config, agentSdkVersion }) → { available, backend:'workflow'|'inline', reason }` (gate ladder: enabled → Claude → execution_backend ≠ inline → host dispatch nested+background → valid Agent SDK → SDK ≥ floor; every miss degrades to `inline`, never throws); `emitWorkflowScript({ phaseDir, waves, runId, budgetTokens?, executorModel? }) → { ok, script, summary }` mapping waves → `parallel()` stage barriers, plans → `agent({ agentType:'gsd-executor', isolation:'worktree', model? })`, `files_modified` overlap → separate sequential stages (greedy first-fit), `resumeFromRunId` wired to the run id, shared `budget(tokens)`; all interpolated identifiers validated script-safe (no `"`,`\`,control chars) and briefs JSON-quoted (review anti-injection). #2686: `executorModel` — resolved for `gsd-executor` from the same config the inline path reads, defaulted by the router so no caller change is needed, `--executor-model` to pin — is emitted per plan and OMITTED when it resolves to `inherit`/empty/whitespace/non-string (#2517: an empty model 404s on runtimes without native tier aliases); a value carrying an unscriptable character is rejected outright (`ok:false`) rather than quoted, because the ADR-1411 provenance header interpolates it into a `//` comment where U+2028/U+2029 would terminate the comment and execute the remainder. `resolveWaveDispatch` forwards it. Registers two loop contributions at WIRED points only (execute:wave:pre/execute:pre are declared but not rendered, same constraint external-job documents): `execute:wave:post into:executor` (Workflow-backend guidance) and `plan:post into:planner` (ultraplan ownership declaration), both `when: claude_orchestration.enabled`, `onError: skip`. Federated config keys (`claude_orchestration.enabled` default false, `execution_backend` enum auto|workflow|inline default auto, `min_agent_sdk_version` string default "0.3.149") live only in the registry — uninstall removes them cleanly. Pre-release versions of the floor compare below GA (SemVer precedence). Restores the wave parallelism + plan-checker + verifier that #853 forces inline on Claude Code; on any runtime lacking the Workflow tool, behaviour is byte-identical to today. BETA v1 ships detection + emission + declarative ultraplan ownership + a `claude-orchestration` command family (`gsd-tools claude-orchestration detect-backend|emit-workflow`, router `gsd-core/bin/lib/claude-orchestration-command-router.cjs` from `src/claude-orchestration-command-router.cts`); full install-profile migration of the ultraplan skill into `skills[]` is a follow-up (CLUSTERS/profile gate). Test anchors: `tests/claude-orchestration.test.cjs`, `tests/claude-orchestration-command-router.test.cjs`. +Default-off, BETA, claude-only Capability (`capabilities/claude-orchestration/`, `role: feature`, `runtimeCompat.supported: ["claude"]`, `tier: full`, `activationKey: claude_orchestration.enabled`) adopting Claude Code's Workflow tool (the engine behind `/effort ultracode`, Agent SDK ≥ v0.3.149) as an optional parallel-execution backend for the GSD loop, and folding the `gsd-ultraplan-phase` plan-offload under the same runtime gate (#1143; ADR-1143). Pure, fail-closed core in `gsd-core/bin/lib/claude-orchestration.cjs`: `detectWorkflowBackend({ runtimeId, hostIntegration, config, agentSdkVersion }) → { available, backend:'workflow'|'inline', reason }` (gate ladder: enabled → Claude → execution_backend ≠ inline → host dispatch nested+background → valid Agent SDK → SDK ≥ floor; every miss degrades to `inline`, never throws); `emitWorkflowScript({ phaseDir, waves, runId, budgetTokens?, executorModel? }) → { ok, script, summary }` mapping waves → `parallel()` stage barriers, plans → `agent({ agentType:'gsd-executor', isolation:'worktree', model? })`, `files_modified` overlap → separate sequential stages (greedy first-fit), `resumeFromRunId` wired to the run id, shared `budget(tokens)`; all interpolated identifiers validated script-safe (no `"`,`\`,control chars) and briefs JSON-quoted (review anti-injection). #2686: `executorModel` — resolved for `gsd-executor` from the same config the inline path reads, defaulted by the router so no caller change is needed, `--executor-model` to pin — is emitted per plan and OMITTED when it resolves to `inherit`/empty/whitespace/non-string (#2517: an empty model 404s on runtimes without native tier aliases); a value carrying an unscriptable character is rejected outright (`ok:false`) rather than quoted, because the ADR-1411 provenance header interpolates it into a `//` comment where U+2028/U+2029 would terminate the comment and execute the remainder. `resolveWaveDispatch` forwards it. Registers one loop contribution at a WIRED point: `plan:post into:planner` (ultraplan ownership declaration), `when: claude_orchestration.enabled`, `onError: skip`. #4740 removed the earlier `execute:wave:pre into:executor` contribution outright (not retargeted) — its body was pure orchestrator procedure (build a wave manifest, resolve the dispatch backend, spawn executor agents) with nothing an executor agent could act on, so injecting it into executor prompts violated the loop's role partition; orchestration is not delivered through an agent contribution. The procedure text is preserved, unwired, at `capabilities/claude-orchestration/docs/workflow-backend-dispatch.md` as reference for a future host-level mechanism. Federated config keys (`claude_orchestration.enabled` default false, `execution_backend` enum auto|workflow|inline default auto, `min_agent_sdk_version` string default "0.3.149") live only in the registry — uninstall removes them cleanly. Pre-release versions of the floor compare below GA (SemVer precedence). Restores the wave parallelism + plan-checker + verifier that #853 forces inline on Claude Code; on any runtime lacking the Workflow tool, behaviour is byte-identical to today. BETA v1 ships detection + emission + declarative ultraplan ownership + a `claude-orchestration` command family (`gsd-tools claude-orchestration detect-backend|emit-workflow`, router `gsd-core/bin/lib/claude-orchestration-command-router.cjs` from `src/claude-orchestration-command-router.cts`); full install-profile migration of the ultraplan skill into `skills[]` is a follow-up (CLUSTERS/profile gate). Test anchors: `tests/claude-orchestration.test.cjs`, `tests/claude-orchestration-command-router.test.cjs`. ### Loop Extension Point A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in Execute; 12 total) where Capabilities register hooks. Three hook kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks via a declared `blocking` flag). Each hook declares the artifacts it produces and consumes; hook order is derived by topological sort of that produces/consumes graph (capability-id tiebreak), which also defines data flow — file-artifact based, surviving `/clear` and fresh executor contexts. Hooks are surfaced by runtime resolution with concrete projection: the workflow calls a query that resolves the active hooks and returns fully-rendered, ordered markdown for the executor. Failure is default-resilient — a non-gate hook that errors is skipped with a warning; a hook may opt into `onError: halt`. Part of the Capability system. ADR-857 phase 3c ships the registry-consuming query layer: `gsd-core/bin/lib/loop-resolver.cjs` exposes `resolveLoopHooks({ point, registry, config })` (pure, no I/O), `renderLoopHooks(resolved)` (pure markdown renderer), and `cmdLoopRenderHooks(cwd, point, raw, opts)` (I/O entry point); activated via `gsd-tools loop render-hooks ` which emits `{ point, activeHooks[], rendered }`. Activation is driven by `when` (dotted config key resolved against `loadConfig`), with inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard. The first phase-6 cutovers wiring workflows to this query have landed — ui-phase at `plan:pre` and ui-review at `verify:post` (in `plan-phase.md`/`autonomous.md`); further per-feature cutovers are ongoing. diff --git a/capabilities/claude-orchestration/capability.json b/capabilities/claude-orchestration/capability.json index 116783048..c624bc2e0 100644 --- a/capabilities/claude-orchestration/capability.json +++ b/capabilities/claude-orchestration/capability.json @@ -55,19 +55,6 @@ }, "steps": [], "contributions": [ - { - "point": "execute:wave:pre", - "into": "executor", - "fragment": { - "path": "fragments/execute-wave-pre.md" - }, - "produces": [], - "consumes": [ - "PLAN.md" - ], - "when": "claude_orchestration.enabled", - "onError": "skip" - }, { "point": "plan:post", "into": "planner", diff --git a/capabilities/claude-orchestration/fragments/execute-wave-pre.md b/capabilities/claude-orchestration/docs/workflow-backend-dispatch.md similarity index 94% rename from capabilities/claude-orchestration/fragments/execute-wave-pre.md rename to capabilities/claude-orchestration/docs/workflow-backend-dispatch.md index e14499375..9e19b6f77 100644 --- a/capabilities/claude-orchestration/fragments/execute-wave-pre.md +++ b/capabilities/claude-orchestration/docs/workflow-backend-dispatch.md @@ -1,7 +1,20 @@ # Claude orchestration — Workflow execution backend (BETA) -> Injected at `execute:wave:pre` `into: executor` only when -> `claude_orchestration.enabled` is true. Default-off; `onError: skip`. +> This is the orchestrator-side procedure for the Workflow execution backend. +> It is NOT a loop contribution and is not injected anywhere. It was +> previously declared as an `execute:wave:pre` contribution with +> `into: "executor"`, which routed orchestrator instructions into executor +> prompts — a role-partition violation (the orchestrator only orchestrates, +> the executor only executes). It is retained here as the reference for +> whoever wires the orchestrator side through a host-level mechanism. See +> issue #4740. + +> **Editor's note:** the body below is preserved byte-for-byte from when this +> file WAS the `execute:wave:pre` contribution fragment, so its section +> headings and prose ("this contribution", "injected", etc.) still speak in +> those terms. That is intentional — it is not being rewritten to match its +> new status — and it is retained purely as the orchestrator-side reference +> described above. ## When this contribution is active diff --git a/docs/adr/1143-claude-orchestration-capability.md b/docs/adr/1143-claude-orchestration-capability.md index 34c1571e6..593a586a2 100644 --- a/docs/adr/1143-claude-orchestration-capability.md +++ b/docs/adr/1143-claude-orchestration-capability.md @@ -130,3 +130,48 @@ execution path (actual orchestration via the Workflow tool inside Claude Code) i not verifiable outside that runtime. The capability is structurally complete and tested at the contract level; flipping to Accepted follows maintainer sign-off on the E2E behaviour once exercised on Claude Code with the Workflow tool present. + +## Amendment (2026-09-14): the `execute:wave:*` contribution is removed — orchestration is not an agent contribution + +Issue [#4740](https://github.com/open-gsd/gsd-core/issues/4740). + +Decision §2 and the 2026-07-06 amendment above are **historical record and are not rewritten**; +this section supersedes them on the single point of loop registration. + +`capabilities/claude-orchestration/capability.json` declared a contribution at `execute:wave:pre` +with `into: "executor"`. `gsd-core/references/loop-hook-dispatch.md` defines a contribution as +*"Inject `fragment.inline` verbatim into the context for the role named in `into`"*, so that +fragment was injected into **executor** prompts whenever `claude_orchestration.enabled`. + +Its 267 lines are orchestration end to end: construct a wave manifest, resolve the dispatch +backend, invoke the Workflow tool to spawn executors, bridge per-agent results into the merge +chain. An executor can act on none of it. + +**Retargeting it to `into: "orchestrator"` would not have been a fix.** `ROLE_TO_AGENT` carries no +`orchestrator` entry by design — the orchestrator IS the host, not an agent, and the host's +procedure lives in `gsd-core/workflows/execute-phase.md`. A step's `agentRoles` enumerates agents a +capability may inject context INTO. Adding `orchestrator` there would model the host as an +injectable agent: the same category error pointed the other way, and it would have required +carving an exception into the role partition ADR-894 §3's 2026-09-14 amendment had just made +normative. + +The defect is therefore the **mechanism**, not the label. A `contribution` injects into an agent's +context; *"replace step 3's inline dispatch loop"* (`execute-phase.md:588`) is a change to what the +**host** does. The contribution channel was being used as a host-behaviour directive because it was +the only channel available at an `execute:*` point. + +**What changed:** the `execute:wave:pre` contribution is removed. The `plan:post` / +`into: "planner"` contribution is correct and is untouched. The orchestrator-side procedure is +preserved verbatim at `capabilities/claude-orchestration/docs/workflow-backend-dispatch.md` — it is +the only copy in the repo — and is no longer injected anywhere. + +**Consequence, stated plainly:** the Workflow execution backend now has **no loop wiring**. Its +detection and emission code (`detectWorkflowBackend`, `emitWorkflowScript`) and its federated +config remain, and its design is intact in the preserved document, but nothing dispatches it. Under +the orchestration/execution separation this ADR itself asserts — *"only the orchestrator (the +script's return) writes shared files; executors stay in their worktrees"* — it never had a +legitimate channel. This makes that visible rather than changing it, and the "Why this is still +Proposed" audit above already records that the end-to-end path has never been exercised. + +Wiring it properly needs a **host-level mechanism** for a capability to alter the orchestrator's +own dispatch procedure. That does not exist today and is not proposed here. diff --git a/docs/adr/894-capability-declaration-format.md b/docs/adr/894-capability-declaration-format.md index 1dd42f8b4..de7b95e16 100644 --- a/docs/adr/894-capability-declaration-format.md +++ b/docs/adr/894-capability-declaration-format.md @@ -267,3 +267,81 @@ This ADR was stress-tested in two rounds before merge; the format changed materi - The declarative-predicate vocabulary for gates (`artifact-exists`, `config-equals`, …). - The exact `commandStyle`/`sandboxTier`/`hooksSurface` enums for `role: runtime` (phase 5, against the 15 runtimes). - Point/role-set deprecation policy once third-party capabilities exist (additive-only holds until then; a rename/removal needs a major bump + deprecation window — deferred with third-party loading per ADR-857). + +## Amendment (2026-09-14): the role assignment in §3 is normative, and the families are disjoint + +Issue [#4740](https://github.com/open-gsd/gsd-core/issues/4740). + +§3 presents the per-step role assignment parenthesised as *"(illustrative roles)"*. That word was +accurate about the list's **purpose** — it illustrated the shape of a generated contract entry — and +wrong about its **status**, because the assignment was load-bearing from the moment the generator +consumed it. Read literally it makes the partition an example rather than a rule, which is the +reading this amendment closes. **The assignment in §3 is normative.** + +### The partition + +Every role belongs to exactly one of three disjoint families: + +| family | roles | +|---|---| +| orchestration | `orchestrator` | +| planning | `researcher`, `planner`, `checker` | +| execution | `executor`, `verifier` | + +Every loop step admits exactly one family: + +| step | admits | +|---|---| +| `discuss` | orchestration | +| `plan` | planning | +| `execute` | execution | +| `verify` | orchestration | +| `ship` | orchestration | + +**No step may declare roles from more than one family.** `execute:*` never admits `orchestrator`; +the orchestration steps never admit `executor` or `verifier`. A step MAY declare a strict subset of +its family (`execute: [executor]` is legal); it may never declare outside it. + +Orchestration and execution are distinct functions of the loop. The orchestrator decides what runs +and owns the shared-state writes; an executor does one plan's work inside its own worktree. A +contribution aimed at one is, by construction, not usable by the other — so a step that admitted +both would be publishing an interface no reader could resolve. + +### Why this is a clarification, not a new decision + +§3's own mechanism already made the partition true in fact. The contract is generated from the +workflow markers *"so it cannot drift into a lie"*, and all five step workflows have always declared +single-family role sets. What was absent was any statement that this is **required**, and any check +that it **holds**. This amendment supplies the first; the enforcement below supplies the second. +No shipped behavior changes. + +### Enforcement + +`scripts/gen-loop-host-contract.cjs` gains `crossCheckRoleFamilies(step, agentRoles, fileName)`, +which fails contract generation on a cross-family declaration, a role outside the vocabulary, or an +unknown step. `lint:generated-sync` already runs `gen-loop-host-contract.cjs --check` in CI, so a +violation blocks a PR. + +It **fails closed on an unknown step**, deliberately diverging from `assertPointsCoverage`'s +`if (!expected) continue; // unknown step — caught elsewhere`. For *points* that comment is true — +the canonical-set and cross-step duplicate checks catch it. For *roles* there is no second net, so +failing open would leave an unknown step as the single input that bypasses the gate. + +This is **additive to §3's existing validation rule** (*"every `contribution.into` ∈ that step's +`agentRoles`"*), which is unchanged. The two govern different actors and do not interact: + +| rule | governs | enforced by | +|---|---|---| +| `contribution.into ∈ agentRoles` (§3, unchanged) | what a **capability** may target | `capability-validator.cjs` | +| role family matches the step (this amendment) | what a **workflow** may declare | `gen-loop-host-contract.cjs` | + +### Scope and limits + +- **Declarations only.** No capability is affected: a contribution targeting a legitimately-declared + role is valid before and after this amendment. +- **No workflow changes.** All five already declare single-family sets, so the gate is green on the + same commit that introduces it. It constrains future edits. +- **It does not make the partition unfalsifiable.** Someone may still edit + `EXPECTED_FAMILY_BY_STEP`. The value is that the boundary moves from an unstated property of five + workflow files to a named, reviewed constant with a test that fails when it changes — the same + bar §3 set for points. diff --git a/docs/explanation/claude-orchestration-capability.md b/docs/explanation/claude-orchestration-capability.md index c14c15dc5..fce8125b2 100644 --- a/docs/explanation/claude-orchestration-capability.md +++ b/docs/explanation/claude-orchestration-capability.md @@ -36,13 +36,18 @@ gate. It is blocked-on-nothing now that the ADR-857 capability system is release - **`role: feature`**, `runtimeCompat.supported: ["claude"]`, `tier: full`. - **`activationKey: claude_orchestration.enabled`** — default `false`. Nothing changes until you opt in. -- Registers at two **wired** loop points: `execute:wave:pre` (into the executor) - and `plan:post` (into the planner). Both are `onError: skip` and gated by the - `enabled` key. The dispatch-backend selector fires at `execute:wave:pre` — the - seam that runs immediately BEFORE a wave's agents are dispatched — because a - selector fired *after* a wave already dispatched inline (the original - `execute:wave:post` placement, [#2285]) is structurally too late to change how - dispatch happens. +- Registers at one **wired** loop point: `plan:post` (into the planner), + `onError: skip` and gated by the `enabled` key. The capability previously + also contributed at `execute:wave:pre` (`into: executor`), but that fragment + was pure orchestrator procedure — build a wave manifest, resolve the + dispatch backend, spawn executor agents — with nothing an executor agent can + act on. Injecting orchestrator instructions into executor prompts violates + the loop's role partition (the orchestrator orchestrates, the executor + executes), so the contribution was removed ([#4740]). The procedure text is + preserved at + [`capabilities/claude-orchestration/docs/workflow-backend-dispatch.md`](../../capabilities/claude-orchestration/docs/workflow-backend-dispatch.md) + as reference for wiring the orchestrator side through a host-level + mechanism; it is not injected anywhere. ## How it decides whether to activate @@ -106,3 +111,4 @@ own runtime gate continues to no-op on non-Claude runtimes. [#1143]: https://github.com/open-gsd/gsd-core/issues/1143 [#2772]: https://github.com/open-gsd/gsd-core/issues/2772 [#2285]: https://github.com/open-gsd/gsd-core/issues/2285 +[#4740]: https://github.com/open-gsd/gsd-core/issues/4740 diff --git a/docs/reference/capability-manifest.md b/docs/reference/capability-manifest.md index b12732583..b8eaf3894 100644 --- a/docs/reference/capability-manifest.md +++ b/docs/reference/capability-manifest.md @@ -308,7 +308,7 @@ The following invariants are enforced at **build time** by `scripts/gen-capabili - **`requires` exist and are acyclic.** Every `id` listed in `requires` must exist in the registry; the dependency graph must be acyclic. - **`requires` is tier-monotone.** A `core` capability may not require a `standard` or `full` capability. A `standard` capability may not require a `full` capability. - **`point` values are from the closed set.** Every `point` in `steps`, `contributions`, and `gates` must be one of the 12 identifiers above. -- **`contribution.into` is a published agent role.** The `into` value must be an agent role declared by the host contract for that loop extension point. +- **`contribution.into` is a published agent role.** The `into` value must be an agent role declared by the host contract for that loop extension point. The published roles are **family-partitioned** (ADR-894 §3, Amendment 2026-09-14): every role belongs to exactly one of orchestration (`orchestrator`), planning (`researcher`, `planner`, `checker`) or execution (`executor`, `verifier`), and each loop step publishes exactly one family. So `execute:*` publishes `executor`/`verifier` and never `orchestrator`, and `discuss:*`/`verify:*`/`ship:*` publish `orchestrator` and never `executor`/`verifier`. The partition is enforced when the host contract is generated, so the role set a point publishes is stable rather than incidental. - **Config key exclusivity.** A federated config key must be owned by exactly one capability and absent from the central `config-schema`. Presence in both is a collision; a half-migrated key fails the build gate. - **Artefact production uniqueness per point.** No two capability steps may `produces` the same artefact name at the same loop extension point. - **`engines.gsd` is a hard gate.** A capability whose `engines.gsd` range does not satisfy the installed GSD version is blocked at install and skipped (with a warning) at load time. diff --git a/docs/reference/capability-matrix.md b/docs/reference/capability-matrix.md index 6e2817bd2..7e387aaae 100644 --- a/docs/reference/capability-matrix.md +++ b/docs/reference/capability-matrix.md @@ -56,7 +56,7 @@ points. | `assumption-delta` | feature | full | `>=1.6.0` | `plan:pre` | contribution | first-party | | `audit` | feature | full | `>=1.6.0` | — | — | first-party | | `broken-windows` | feature | full | `>=1.7.0` | `ship:pre` | gate | first-party | -| `claude-orchestration` | feature | full | `>=1.7.0` | `plan:post`, `execute:wave:pre` | contribution | first-party | +| `claude-orchestration` | feature | full | `>=1.7.0` | `plan:post` | contribution | first-party | | `code-review` | feature | full | `>=1.6.0` | `execute:wave:post`, `execute:post` | step | first-party | | `drift` | feature | full | `>=1.6.0` | `plan:pre`, `execute:wave:post` | gate | first-party | | `external-job` | feature | full | `>=1.7.0` | `plan:post`, `execute:wave:post` | contribution | first-party | diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index d7e56882d..229ab5fd1 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -736,20 +736,6 @@ const capabilities = { }, "steps": [], "contributions": [ - { - "point": "execute:wave:pre", - "into": "executor", - "fragment": { - "path": "fragments/execute-wave-pre.md", - "inline": "# Claude orchestration — Workflow execution backend (BETA)\n\n> Injected at `execute:wave:pre` `into: executor` only when\n> `claude_orchestration.enabled` is true. Default-off; `onError: skip`.\n\n## When this contribution is active\n\nThe Claude orchestration capability is **default-off and BETA**. It activates only\nwhen ALL of the following hold:\n\n1. `claude_orchestration.enabled` is `true` in `.planning/config.json`, AND\n2. the active runtime is **Claude Code** (the Workflow tool is Claude / Agent\n SDK-specific), AND\n3. `claude_orchestration.execution_backend` resolves to `workflow` — either\n explicitly, or via `auto` — **and** the Agent SDK version is\n `>= claude_orchestration.min_agent_sdk_version` (default `0.3.149`). The SDK\n floor applies in both `auto` and `workflow` modes (fail-closed: a pre-release\n or older SDK never activates the preview backend).\n\nDetection is fail-closed: any miss degrades to **inline, manual, one-agent-per-\nmessage dispatch** — exactly today's behaviour. On a non-Claude runtime this\ncontribution is a no-op.\n\n## Why `execute:wave:pre` (not `execute:wave:post`)\n\nThis is a **dispatch-backend selector** — it decides HOW a wave's executor agents\nare spawned. That decision has to be made BEFORE the wave's `Agent()` calls in\n`execute-phase.md` step 3, not after the wave has already finished (#2285). The\ncapability previously registered at `execute:wave:post`, which fires only after\nworktree merge/post-merge tests/tracking updates — by then the wave was already\ndispatched inline, so the contribution was structurally unable to change how\ndispatch happened. This fragment is injected at the point that actually precedes\ndispatch.\n\n## What the orchestrator does when the Workflow backend is active\n\nBefore spawning executor agents for the current wave (execute-phase.md step 3),\nresolve the dispatch backend through the single composed CLI seam:\n\n```bash\ngsd-tools claude-orchestration resolve-wave-dispatch \\\n --waves \"$WAVE_MANIFEST_PATH\" --run-id \"$PHASE_RUN_ID\" \\\n --runtime \"$RUNTIME\" \\\n --phase-dir \"$PHASE_DIR\" --raw\n```\n\n`--agent-sdk-version` is no longer passed here (#2590). The router resolves the\ninstalled Agent SDK version itself; see **Agent SDK version** below. The former\n`${AGENT_SDK_VERSION:+--agent-sdk-version \"$AGENT_SDK_VERSION\"}` line was also\n**shell-dependent**: zsh does not word-split unquoted parameter expansions, so it\ncollapsed to a SINGLE argv element there, `argValue()` never matched, and the run\nfailed into `agent_sdk_version_unknown` — indistinguishable from genuinely\nunknown. Pass `--agent-sdk-version ` explicitly only to pin a version.\n\nThis composes `detectWorkflowBackend` (the gate ladder above) with\n`emitWorkflowScript` (the wave→plan mapping below) in ONE call — the pure\nfunction backing it is `resolveWaveDispatch` in\n`gsd-core/bin/lib/claude-orchestration.cjs`. Response shape:\n`{ backend: 'inline'|'workflow', reason, script?, summary? }`.\n\n### Manifest construction (`$WAVE_MANIFEST_PATH`, `$PHASE_RUN_ID`, `$PHASE_DIR`)\n\nThese are NOT pre-existing execute-phase.md variables — the orchestrator builds\nthem at this step, from data it already has in-context from `discover_and_group_plans`\n(the `PLAN_INDEX` JSON) and step 2.5 (the per-plan `USE_WORKTREES_FOR_PLAN` decision):\n\n1. **`$PHASE_DIR`** — reuse `{phase_dir}` from the `INIT` bundle (already loaded\n in the `initialize` step). No new value needed.\n\n2. **`$PHASE_RUN_ID`** — a stable identifier for THIS phase-execution attempt, so\n `resumeFromRunId` can resume an interrupted run without re-dispatching plans\n the Workflow tool already completed. Construct it deterministically —\n `execute-{phase_number}-{phase_slug}` — from `INIT`'s `phase_number`/`phase_slug`\n (both are already validated identifiers used elsewhere in this workflow, so\n they satisfy `emitWorkflowScript`'s `isScriptableIdentifier` check). Do NOT\n mint a new random id per wave — the SAME `$PHASE_RUN_ID` is reused for every\n wave in the phase so the Workflow tool can correctly track cross-wave resume\n state.\n\n3. **`$WAVE_MANIFEST_PATH`** — a fresh temp file for THIS wave's manifest (one\n wave = one `waves` array with a single entry, matching the wave-by-wave\n dispatch loop; do not batch multiple waves into one manifest — waves are\n dispatched in wave order, not all at once):\n\n ```bash\n WAVE_MANIFEST_PATH=$(mktemp \"${TMPDIR:-/tmp}/gsd-wave-dispatch-XXXXXX\") && mv \"$WAVE_MANIFEST_PATH\" \"$WAVE_MANIFEST_PATH.json\" && WAVE_MANIFEST_PATH=\"$WAVE_MANIFEST_PATH.json\"\n ```\n\n Then **use the Write tool** (not a bash/jq pipeline — the orchestrator already\n has every field parsed in-context) to write the manifest JSON to\n `$WAVE_MANIFEST_PATH`:\n\n ```json\n {\n \"waves\": [\n {\n \"id\": \"wave-{N}\",\n \"plans\": [\n {\n \"id\": \"{plan_id}\",\n \"brief\": \"{the SAME ... prompt block step 3 builds for this plan's inline Agent() call}\",\n \"files_modified\": [\"{from PLAN_INDEX.plans[].files_modified for this plan}\"],\n \"use_worktree\": {true unless step 2.5 set USE_WORKTREES_FOR_PLAN=false for this plan}\n }\n ]\n }\n ]\n }\n ```\n\n - **`id`** — the plan id from `PLAN_INDEX`, e.g. `\"01-01\"`.\n - **`brief`** — MUST carry the same task content as step 3's inline `Agent()`\n prompt (the ``/``/``/\n `` block, with `{plan_number}`/`{phase_number}`/\n `{phase_name}` substituted) — a short summary here would NOT reproduce\n step 3's behavior and would violate the \"identical artifacts\" contract.\n - **`files_modified`** — copy verbatim from the plan's `PLAN_INDEX` entry.\n - **`use_worktree`** — `true` for every plan UNLESS step 2.5's per-plan\n worktree gate (`execute-phase/steps/per-plan-worktree-gate.md`) set\n `USE_WORKTREES_FOR_PLAN=false` for that plan (submodule-touching plan, or\n project-level `USE_WORKTREES=false`) — in which case pass `false` here so\n `emitWorkflowScript` omits `isolation: \"worktree\"` for that plan (#2772 /\n #2285 finding 1). **Never** hardcode `true` — that would force worktree\n isolation on a plan the inline path explicitly keeps out of worktrees.\n\n4. **`$AGENT_SDK_VERSION`** — no longer built here; the router resolves it.\n\n**Agent SDK version:** the orchestrator has no *bash-computable* way to\nintrospect the live Agent SDK version — but the router runs in Node, so it\nresolves the version itself (#2590), in this order:\n\n1. an explicit `--agent-sdk-version ` (pin a version),\n2. `GSD_AGENT_SDK_VERSION`,\n3. the **installed** `@anthropic-ai/claude-agent-sdk` package version, read from\n its `package.json` on disk by walking `node_modules` up the tree. (Read\n directly rather than via `require.resolve`: the SDK's `exports` map does not\n expose `./package.json`, so `require.resolve` throws\n `ERR_PACKAGE_PATH_NOT_EXPORTED`.)\n\nPreviously nothing computed this at all, so gate 5 returned\n`agent_sdk_version_unknown` on **every** automated run and the Workflow backend\ncould never activate — while `gsd-tools capability state` still reported the\ncapability `active: true`. Fail-closed is preserved: when no version can be\nresolved, gate 5 still declines to `inline`. What changed is that a resolvable\nversion is now actually found, so a genuinely-too-old SDK reports\n`agent_sdk_version_below_floor` — the truthful reason — instead of `unknown`.\n\n**If `backend == \"workflow\"`:** run the emitted `script` via the Workflow tool\nfor THIS wave instead of the per-message `Agent()` loop in step 3. The script\ncomposes the SAME `gsd-executor` agent type the inline path uses, with\nworktree isolation applied PER PLAN from the manifest's `use_worktree` field\n(see `emitWorkflowScript`):\n\n- **waves → one or more sequential `parallel()` barriers** — each wave is a\n barrier group; when plans within a wave share `files_modified`, they are split\n into separate sequential stages within that wave's barrier.\n- **plans → `agent(brief, { agentType: 'gsd-executor', isolation: 'worktree' })`**\n when `use_worktree` is not `false`, or `agent(brief, { agentType: 'gsd-executor' })`\n (no isolation) when it is — so the produced `SUMMARY.md` and commits are\n identical to inline dispatch, INCLUDING the inline path's submodule safety\n gate (#2772 / #2285 finding 1).\n- **`files_modified` overlap → separate sequential stages** — the same overlap\n rule execute-phase already applies inline (step 1 of the wave loop).\n- **`resumeFromRunId`** — **pass `summary.resumeRunId` as the Workflow tool's\n `resumeFromRunId` INPUT when you invoke the tool.** It is a tool parameter,\n not a script function; the script deliberately does not call it (#2590 — doing\n so threw \"resumeFromRunId is not defined\" and rejected the entire script).\n Omitting it from the tool invocation silently regresses phase-resume to a\n no-op: an interrupted phase re-runs completed plans.\n\n### After the run: manifest bridge into the merge chain (#3302)\n\nThe single Workflow tool call replaces step 3's per-plan `Agent()` loop — which also\nmeans step 3's manifest bookkeeping (creation + per-agent recording) does NOT happen on\nthis path. The orchestrator MUST bridge the run's per-agent results into the SAME\nmanifest-scoped merge chain inline dispatch uses, before steps 4–5.8, which then run\nunchanged:\n\n1. **Create the manifest BEFORE invoking the tool** (this is step 3's creation block,\n which this path skips). When ANY plan in the wave has `use_worktree` not `false`:\n\n ```bash\n if [ -z \"${WAVE_WORKTREE_MANIFEST:-}\" ]; then\n M=$(mktemp \"${TMPDIR:-/tmp}/gsd-worktree-wave-XXXXXX\") && mv \"$M\" \"$M.json\" && WAVE_WORKTREE_MANIFEST=\"$M.json\" || exit 1 # XXXXXX must be path-final on BSD/macOS (#1520)\n # Persist the dispatch-time orchestrator worktree root so wave-cleanup pins back\n # to the orchestrator's OWN worktree (#630), exactly as inline dispatch does.\n ORCH_ROOT=$(git rev-parse --show-toplevel)\n ORCH_ROOT=\"$ORCH_ROOT\" MANIFEST=\"$WAVE_WORKTREE_MANIFEST\" node -e 'const fs=require(\"fs\");fs.writeFileSync(process.env.MANIFEST,JSON.stringify({orchestrator_root:process.env.ORCH_ROOT||null,worktrees:[]})+\"\\n\")'\n export WAVE_WORKTREE_MANIFEST\n fi\n ```\n\n2. **Invoke the Workflow tool with the emitted script and\n `resumeFromRunId: summary.resumeRunId`.** The script top-level `return`s one entry\n per dispatched plan: `{ plan, expects_worktree, metadata }`. `metadata` is that\n plan's executor `` JSON (`{agent_id, worktree_path, branch,\n expected_base}` — captured by the executor itself per\n `agents/gsd-executor.md`), or `null` when the agent's result carried none\n (interrupted agent, resumed-from-cache plan, or a non-worktree plan).\n\n3. **Record every worktree plan** exactly as inline dispatch does at step 3's\n \"After each `Agent()` returns\" — one `worktree.record-agent` per returned entry\n with `expects_worktree: true` and complete metadata:\n\n ```bash\n gsd_run query worktree.record-agent --manifest \"$WAVE_WORKTREE_MANIFEST\" \\\n --agent-id \"\" --path \"\" \\\n --branch \"\" --base \"\" \\\n --files \"\" \\\n --deletions \"\"\n ```\n\n `--deletions` (#3003) carries the plan's declared `files_deleted` so a plan that scoped a file\n removal merges through `cleanup-wave` instead of being blocked. Unlike `--files` it is not\n advisory: omitting it leaves the deletions guard blocking on any deletion at all, so this\n dispatch path must pass it or plans declaring a removal fail to merge here while succeeding on\n the inline path.\n\n The verb's write-strict validation applies as inline: on a non-zero exit or any\n missing field, stop and ask for recovery — do not append an under-populated entry.\n\n4. **HALT on uncapturable metadata — never a silently-empty manifest (#3302).**\n After recording, the manifest must hold one entry per `expects_worktree: true`\n outcome (`summary.worktreePlans` from `resolve-wave-dispatch` is the expected\n count). Any shortfall — a `null` `metadata`, a missing/empty field, or a count\n mismatch — means commits are stranded on their `worktree-wf_*` branches and\n `worktree.cleanup-wave` would merge nothing while the phase looks green. STOP the\n phase with the failing plan id and the recovery hint below; do NOT run\n `worktree.cleanup-wave` and do NOT proceed to step 4.\n\n **Recovery hint:** the unmerged `worktree-wf_*` branch still holds the work. Recover\n the missing metadata from the run's per-agent result journal (`journal.jsonl` — one\n `{\"type\":\"result\",…}` line per agent — in the Workflow run's transcript dir), re-run\n `worktree.record-agent` by hand, then re-run cleanup. If the journal cannot be\n recovered either, merge the branch manually after review — never discard it.\n\n5. **Resume (`resumeFromRunId`).** Cached/resumed agents do not re-emit their final\n messages, so a previously-completed plan can return with `metadata: null`. Recover\n that plan's metadata from the ORIGINAL run's journal (same hint as above). If it\n cannot be recovered, fail loudly per rule 4 — a resumed run must never report\n success over silently-dropped agent work.\n\n6. **Non-worktree plans** (`expects_worktree: false` — `use_worktree: false` in the\n manifest): they ran without isolation; their commits are already on the main working\n tree. No record-agent entry, no manifest write.\n\nWith the manifest populated, steps 4–5.8 (wait/completion bookkeeping, step 5.5's\nmanifest-scoped `worktree.cleanup-wave`, post-merge gate, tracking update) run\nUNCHANGED — the Workflow backend replaces HOW agents are spawned and returns their\nmetadata; the merge chain itself is the inline path's own, now with real input.\n\n**If `backend == \"inline\"`** (any gate miss, or `resolve-wave-dispatch` itself\nunavailable/erroring): proceed to step 3's standard per-message `Agent()`\ndispatch — the default, byte-identical-to-today path. `onError: skip` on this\ncontribution means a `resolve-wave-dispatch` command failure is treated exactly\nlike an `inline` result, never as a fatal wave error.\n\n## Fallback contract\n\nDetection is fail-closed end-to-end: capability disabled, non-Claude runtime,\n`execution_backend:\"inline\"`, missing/incapable host descriptor, unknown or\nbelow-floor Agent SDK version, or an `emitWorkflowScript` failure on a malformed\nwave manifest — ANY of these degrades to `backend:\"inline\"` and execute-phase's\nstandard inline dispatch (step 3) runs unmodified. The Workflow backend never\npartially activates; the executor MUST NOT assume parallelism, a shared budget,\nor resume-from-run-id semantics when `backend == \"inline\"`.\n" - }, - "produces": [], - "consumes": [ - "PLAN.md" - ], - "when": "claude_orchestration.enabled", - "onError": "skip" - }, { "point": "plan:post", "into": "planner", @@ -4537,23 +4523,7 @@ const byLoopPoint = { }, "execute:wave:pre": { "steps": [], - "contributions": [ - { - "capId": "claude-orchestration", - "point": "execute:wave:pre", - "into": "executor", - "fragment": { - "path": "fragments/execute-wave-pre.md", - "inline": "# Claude orchestration — Workflow execution backend (BETA)\n\n> Injected at `execute:wave:pre` `into: executor` only when\n> `claude_orchestration.enabled` is true. Default-off; `onError: skip`.\n\n## When this contribution is active\n\nThe Claude orchestration capability is **default-off and BETA**. It activates only\nwhen ALL of the following hold:\n\n1. `claude_orchestration.enabled` is `true` in `.planning/config.json`, AND\n2. the active runtime is **Claude Code** (the Workflow tool is Claude / Agent\n SDK-specific), AND\n3. `claude_orchestration.execution_backend` resolves to `workflow` — either\n explicitly, or via `auto` — **and** the Agent SDK version is\n `>= claude_orchestration.min_agent_sdk_version` (default `0.3.149`). The SDK\n floor applies in both `auto` and `workflow` modes (fail-closed: a pre-release\n or older SDK never activates the preview backend).\n\nDetection is fail-closed: any miss degrades to **inline, manual, one-agent-per-\nmessage dispatch** — exactly today's behaviour. On a non-Claude runtime this\ncontribution is a no-op.\n\n## Why `execute:wave:pre` (not `execute:wave:post`)\n\nThis is a **dispatch-backend selector** — it decides HOW a wave's executor agents\nare spawned. That decision has to be made BEFORE the wave's `Agent()` calls in\n`execute-phase.md` step 3, not after the wave has already finished (#2285). The\ncapability previously registered at `execute:wave:post`, which fires only after\nworktree merge/post-merge tests/tracking updates — by then the wave was already\ndispatched inline, so the contribution was structurally unable to change how\ndispatch happened. This fragment is injected at the point that actually precedes\ndispatch.\n\n## What the orchestrator does when the Workflow backend is active\n\nBefore spawning executor agents for the current wave (execute-phase.md step 3),\nresolve the dispatch backend through the single composed CLI seam:\n\n```bash\ngsd-tools claude-orchestration resolve-wave-dispatch \\\n --waves \"$WAVE_MANIFEST_PATH\" --run-id \"$PHASE_RUN_ID\" \\\n --runtime \"$RUNTIME\" \\\n --phase-dir \"$PHASE_DIR\" --raw\n```\n\n`--agent-sdk-version` is no longer passed here (#2590). The router resolves the\ninstalled Agent SDK version itself; see **Agent SDK version** below. The former\n`${AGENT_SDK_VERSION:+--agent-sdk-version \"$AGENT_SDK_VERSION\"}` line was also\n**shell-dependent**: zsh does not word-split unquoted parameter expansions, so it\ncollapsed to a SINGLE argv element there, `argValue()` never matched, and the run\nfailed into `agent_sdk_version_unknown` — indistinguishable from genuinely\nunknown. Pass `--agent-sdk-version ` explicitly only to pin a version.\n\nThis composes `detectWorkflowBackend` (the gate ladder above) with\n`emitWorkflowScript` (the wave→plan mapping below) in ONE call — the pure\nfunction backing it is `resolveWaveDispatch` in\n`gsd-core/bin/lib/claude-orchestration.cjs`. Response shape:\n`{ backend: 'inline'|'workflow', reason, script?, summary? }`.\n\n### Manifest construction (`$WAVE_MANIFEST_PATH`, `$PHASE_RUN_ID`, `$PHASE_DIR`)\n\nThese are NOT pre-existing execute-phase.md variables — the orchestrator builds\nthem at this step, from data it already has in-context from `discover_and_group_plans`\n(the `PLAN_INDEX` JSON) and step 2.5 (the per-plan `USE_WORKTREES_FOR_PLAN` decision):\n\n1. **`$PHASE_DIR`** — reuse `{phase_dir}` from the `INIT` bundle (already loaded\n in the `initialize` step). No new value needed.\n\n2. **`$PHASE_RUN_ID`** — a stable identifier for THIS phase-execution attempt, so\n `resumeFromRunId` can resume an interrupted run without re-dispatching plans\n the Workflow tool already completed. Construct it deterministically —\n `execute-{phase_number}-{phase_slug}` — from `INIT`'s `phase_number`/`phase_slug`\n (both are already validated identifiers used elsewhere in this workflow, so\n they satisfy `emitWorkflowScript`'s `isScriptableIdentifier` check). Do NOT\n mint a new random id per wave — the SAME `$PHASE_RUN_ID` is reused for every\n wave in the phase so the Workflow tool can correctly track cross-wave resume\n state.\n\n3. **`$WAVE_MANIFEST_PATH`** — a fresh temp file for THIS wave's manifest (one\n wave = one `waves` array with a single entry, matching the wave-by-wave\n dispatch loop; do not batch multiple waves into one manifest — waves are\n dispatched in wave order, not all at once):\n\n ```bash\n WAVE_MANIFEST_PATH=$(mktemp \"${TMPDIR:-/tmp}/gsd-wave-dispatch-XXXXXX\") && mv \"$WAVE_MANIFEST_PATH\" \"$WAVE_MANIFEST_PATH.json\" && WAVE_MANIFEST_PATH=\"$WAVE_MANIFEST_PATH.json\"\n ```\n\n Then **use the Write tool** (not a bash/jq pipeline — the orchestrator already\n has every field parsed in-context) to write the manifest JSON to\n `$WAVE_MANIFEST_PATH`:\n\n ```json\n {\n \"waves\": [\n {\n \"id\": \"wave-{N}\",\n \"plans\": [\n {\n \"id\": \"{plan_id}\",\n \"brief\": \"{the SAME ... prompt block step 3 builds for this plan's inline Agent() call}\",\n \"files_modified\": [\"{from PLAN_INDEX.plans[].files_modified for this plan}\"],\n \"use_worktree\": {true unless step 2.5 set USE_WORKTREES_FOR_PLAN=false for this plan}\n }\n ]\n }\n ]\n }\n ```\n\n - **`id`** — the plan id from `PLAN_INDEX`, e.g. `\"01-01\"`.\n - **`brief`** — MUST carry the same task content as step 3's inline `Agent()`\n prompt (the ``/``/``/\n `` block, with `{plan_number}`/`{phase_number}`/\n `{phase_name}` substituted) — a short summary here would NOT reproduce\n step 3's behavior and would violate the \"identical artifacts\" contract.\n - **`files_modified`** — copy verbatim from the plan's `PLAN_INDEX` entry.\n - **`use_worktree`** — `true` for every plan UNLESS step 2.5's per-plan\n worktree gate (`execute-phase/steps/per-plan-worktree-gate.md`) set\n `USE_WORKTREES_FOR_PLAN=false` for that plan (submodule-touching plan, or\n project-level `USE_WORKTREES=false`) — in which case pass `false` here so\n `emitWorkflowScript` omits `isolation: \"worktree\"` for that plan (#2772 /\n #2285 finding 1). **Never** hardcode `true` — that would force worktree\n isolation on a plan the inline path explicitly keeps out of worktrees.\n\n4. **`$AGENT_SDK_VERSION`** — no longer built here; the router resolves it.\n\n**Agent SDK version:** the orchestrator has no *bash-computable* way to\nintrospect the live Agent SDK version — but the router runs in Node, so it\nresolves the version itself (#2590), in this order:\n\n1. an explicit `--agent-sdk-version ` (pin a version),\n2. `GSD_AGENT_SDK_VERSION`,\n3. the **installed** `@anthropic-ai/claude-agent-sdk` package version, read from\n its `package.json` on disk by walking `node_modules` up the tree. (Read\n directly rather than via `require.resolve`: the SDK's `exports` map does not\n expose `./package.json`, so `require.resolve` throws\n `ERR_PACKAGE_PATH_NOT_EXPORTED`.)\n\nPreviously nothing computed this at all, so gate 5 returned\n`agent_sdk_version_unknown` on **every** automated run and the Workflow backend\ncould never activate — while `gsd-tools capability state` still reported the\ncapability `active: true`. Fail-closed is preserved: when no version can be\nresolved, gate 5 still declines to `inline`. What changed is that a resolvable\nversion is now actually found, so a genuinely-too-old SDK reports\n`agent_sdk_version_below_floor` — the truthful reason — instead of `unknown`.\n\n**If `backend == \"workflow\"`:** run the emitted `script` via the Workflow tool\nfor THIS wave instead of the per-message `Agent()` loop in step 3. The script\ncomposes the SAME `gsd-executor` agent type the inline path uses, with\nworktree isolation applied PER PLAN from the manifest's `use_worktree` field\n(see `emitWorkflowScript`):\n\n- **waves → one or more sequential `parallel()` barriers** — each wave is a\n barrier group; when plans within a wave share `files_modified`, they are split\n into separate sequential stages within that wave's barrier.\n- **plans → `agent(brief, { agentType: 'gsd-executor', isolation: 'worktree' })`**\n when `use_worktree` is not `false`, or `agent(brief, { agentType: 'gsd-executor' })`\n (no isolation) when it is — so the produced `SUMMARY.md` and commits are\n identical to inline dispatch, INCLUDING the inline path's submodule safety\n gate (#2772 / #2285 finding 1).\n- **`files_modified` overlap → separate sequential stages** — the same overlap\n rule execute-phase already applies inline (step 1 of the wave loop).\n- **`resumeFromRunId`** — **pass `summary.resumeRunId` as the Workflow tool's\n `resumeFromRunId` INPUT when you invoke the tool.** It is a tool parameter,\n not a script function; the script deliberately does not call it (#2590 — doing\n so threw \"resumeFromRunId is not defined\" and rejected the entire script).\n Omitting it from the tool invocation silently regresses phase-resume to a\n no-op: an interrupted phase re-runs completed plans.\n\n### After the run: manifest bridge into the merge chain (#3302)\n\nThe single Workflow tool call replaces step 3's per-plan `Agent()` loop — which also\nmeans step 3's manifest bookkeeping (creation + per-agent recording) does NOT happen on\nthis path. The orchestrator MUST bridge the run's per-agent results into the SAME\nmanifest-scoped merge chain inline dispatch uses, before steps 4–5.8, which then run\nunchanged:\n\n1. **Create the manifest BEFORE invoking the tool** (this is step 3's creation block,\n which this path skips). When ANY plan in the wave has `use_worktree` not `false`:\n\n ```bash\n if [ -z \"${WAVE_WORKTREE_MANIFEST:-}\" ]; then\n M=$(mktemp \"${TMPDIR:-/tmp}/gsd-worktree-wave-XXXXXX\") && mv \"$M\" \"$M.json\" && WAVE_WORKTREE_MANIFEST=\"$M.json\" || exit 1 # XXXXXX must be path-final on BSD/macOS (#1520)\n # Persist the dispatch-time orchestrator worktree root so wave-cleanup pins back\n # to the orchestrator's OWN worktree (#630), exactly as inline dispatch does.\n ORCH_ROOT=$(git rev-parse --show-toplevel)\n ORCH_ROOT=\"$ORCH_ROOT\" MANIFEST=\"$WAVE_WORKTREE_MANIFEST\" node -e 'const fs=require(\"fs\");fs.writeFileSync(process.env.MANIFEST,JSON.stringify({orchestrator_root:process.env.ORCH_ROOT||null,worktrees:[]})+\"\\n\")'\n export WAVE_WORKTREE_MANIFEST\n fi\n ```\n\n2. **Invoke the Workflow tool with the emitted script and\n `resumeFromRunId: summary.resumeRunId`.** The script top-level `return`s one entry\n per dispatched plan: `{ plan, expects_worktree, metadata }`. `metadata` is that\n plan's executor `` JSON (`{agent_id, worktree_path, branch,\n expected_base}` — captured by the executor itself per\n `agents/gsd-executor.md`), or `null` when the agent's result carried none\n (interrupted agent, resumed-from-cache plan, or a non-worktree plan).\n\n3. **Record every worktree plan** exactly as inline dispatch does at step 3's\n \"After each `Agent()` returns\" — one `worktree.record-agent` per returned entry\n with `expects_worktree: true` and complete metadata:\n\n ```bash\n gsd_run query worktree.record-agent --manifest \"$WAVE_WORKTREE_MANIFEST\" \\\n --agent-id \"\" --path \"\" \\\n --branch \"\" --base \"\" \\\n --files \"\" \\\n --deletions \"\"\n ```\n\n `--deletions` (#3003) carries the plan's declared `files_deleted` so a plan that scoped a file\n removal merges through `cleanup-wave` instead of being blocked. Unlike `--files` it is not\n advisory: omitting it leaves the deletions guard blocking on any deletion at all, so this\n dispatch path must pass it or plans declaring a removal fail to merge here while succeeding on\n the inline path.\n\n The verb's write-strict validation applies as inline: on a non-zero exit or any\n missing field, stop and ask for recovery — do not append an under-populated entry.\n\n4. **HALT on uncapturable metadata — never a silently-empty manifest (#3302).**\n After recording, the manifest must hold one entry per `expects_worktree: true`\n outcome (`summary.worktreePlans` from `resolve-wave-dispatch` is the expected\n count). Any shortfall — a `null` `metadata`, a missing/empty field, or a count\n mismatch — means commits are stranded on their `worktree-wf_*` branches and\n `worktree.cleanup-wave` would merge nothing while the phase looks green. STOP the\n phase with the failing plan id and the recovery hint below; do NOT run\n `worktree.cleanup-wave` and do NOT proceed to step 4.\n\n **Recovery hint:** the unmerged `worktree-wf_*` branch still holds the work. Recover\n the missing metadata from the run's per-agent result journal (`journal.jsonl` — one\n `{\"type\":\"result\",…}` line per agent — in the Workflow run's transcript dir), re-run\n `worktree.record-agent` by hand, then re-run cleanup. If the journal cannot be\n recovered either, merge the branch manually after review — never discard it.\n\n5. **Resume (`resumeFromRunId`).** Cached/resumed agents do not re-emit their final\n messages, so a previously-completed plan can return with `metadata: null`. Recover\n that plan's metadata from the ORIGINAL run's journal (same hint as above). If it\n cannot be recovered, fail loudly per rule 4 — a resumed run must never report\n success over silently-dropped agent work.\n\n6. **Non-worktree plans** (`expects_worktree: false` — `use_worktree: false` in the\n manifest): they ran without isolation; their commits are already on the main working\n tree. No record-agent entry, no manifest write.\n\nWith the manifest populated, steps 4–5.8 (wait/completion bookkeeping, step 5.5's\nmanifest-scoped `worktree.cleanup-wave`, post-merge gate, tracking update) run\nUNCHANGED — the Workflow backend replaces HOW agents are spawned and returns their\nmetadata; the merge chain itself is the inline path's own, now with real input.\n\n**If `backend == \"inline\"`** (any gate miss, or `resolve-wave-dispatch` itself\nunavailable/erroring): proceed to step 3's standard per-message `Agent()`\ndispatch — the default, byte-identical-to-today path. `onError: skip` on this\ncontribution means a `resolve-wave-dispatch` command failure is treated exactly\nlike an `inline` result, never as a fatal wave error.\n\n## Fallback contract\n\nDetection is fail-closed end-to-end: capability disabled, non-Claude runtime,\n`execution_backend:\"inline\"`, missing/incapable host descriptor, unknown or\nbelow-floor Agent SDK version, or an `emitWorkflowScript` failure on a malformed\nwave manifest — ANY of these degrades to `backend:\"inline\"` and execute-phase's\nstandard inline dispatch (step 3) runs unmodified. The Workflow backend never\npartially activates; the executor MUST NOT assume parallelism, a shared budget,\nor resume-from-run-id semantics when `backend == \"inline\"`.\n" - }, - "produces": [], - "consumes": [ - "PLAN.md" - ], - "when": "claude_orchestration.enabled", - "onError": "skip" - } - ], + "contributions": [], "gates": [] }, "execute:wave:post": { diff --git a/scripts/gen-loop-host-contract.cjs b/scripts/gen-loop-host-contract.cjs index 1be932336..0cbc97a20 100644 --- a/scripts/gen-loop-host-contract.cjs +++ b/scripts/gen-loop-host-contract.cjs @@ -78,6 +78,23 @@ const ROLE_TO_AGENT = { verifier: 'gsd-verifier', }; +// #4740 — Role → family mapping. Three families: orchestration, planning, +// execution. This constrains what a workflow may DECLARE in agent-roles for +// a given step (admissibility), a separate concern from ROLE_TO_AGENT's +// agent-file presence check above. +const ROLE_FAMILY = { + orchestrator: 'orchestration', + researcher: 'planning', planner: 'planning', checker: 'planning', + executor: 'execution', verifier: 'execution', +}; +const EXPECTED_FAMILY_BY_STEP = { + discuss: 'orchestration', + plan: 'planning', + execute: 'execution', + verify: 'orchestration', + ship: 'orchestration', +}; + // ─── Parser ─────────────────────────────────────────────────────────────────── /** @@ -228,6 +245,52 @@ function crossCheckRoles(content, agentRoles, fileName) { return errors; } +// ─── Cross-check: declared roles vs. their permitted family (#4740) ────────── + +/** + * For a step, verify every role declared in agentRoles belongs to that step's + * expected family (orchestration / planning / execution). + * + * Unlike `assertPointsCoverage`'s `if (!expected) continue // caught + * elsewhere` guard, an unknown step here fails CLOSED: for points + * there is a second net (the canonical-set and duplicate checks), but nothing + * else in the repo validates role families, so failing open on an unknown + * step would make it the one input that silently bypasses this gate — the + * "unknown resolving to a safe known" failure mode this check exists to close. + * + * Pure: never sorts, de-dupes, or otherwise mutates `agentRoles` — the same + * array `buildContract` puts into the generated contract. + * + * @param {string} step The step named in the marker block. + * @param {string[]} agentRoles Roles declared in the block. + * @param {string} fileName For error messages. + * @returns {string[]} Array of error strings; empty = OK. + */ +function crossCheckRoleFamilies(step, agentRoles, fileName) { + const expectedFamily = EXPECTED_FAMILY_BY_STEP[step]; + if (!expectedFamily) { + return [fileName + ': step "' + step + '" has no entry in EXPECTED_FAMILY_BY_STEP']; + } + + const errors = []; + for (const role of agentRoles) { + const family = ROLE_FAMILY[role]; + if (!family) { + errors.push( + fileName + ': declared agent-role "' + role + '" has no entry in ROLE_FAMILY mapping', + ); + continue; + } + if (family !== expectedFamily) { + errors.push( + fileName + ': declared agent-role "' + role + '" (family "' + family + + '") is not permitted at step "' + step + '" (expected family "' + expectedFamily + '")', + ); + } + } + return errors; +} + // ─── 12-points coverage assertion ──────────────────────────────────────────── /** @@ -331,6 +394,10 @@ function buildContract(workflowsDir) { const roleErrors = crossCheckRoles(content, entry.agentRoles, file); allErrors.push(...roleErrors); + // Cross-check role families (#4740) + const roleFamilyErrors = crossCheckRoleFamilies(entry.step, entry.agentRoles, file); + allErrors.push(...roleFamilyErrors); + for (const auxiliary of auxiliaryHosts) { let auxiliaryContent; try { @@ -667,6 +734,7 @@ function getWiredLoopPoints(repoRoot) { module.exports = { parseLoopHostBlock, crossCheckRoles, + crossCheckRoleFamilies, assertPointsCoverage, buildContract, serializeContract, @@ -676,6 +744,7 @@ module.exports = { CANONICAL_POINTS, EXPECTED_POINTS_BY_STEP, ROLE_TO_AGENT, + ROLE_FAMILY, scanWiredPoints, getWiredLoopPoints, coveredKindsInRegion, diff --git a/tests/claude-orchestration.test.cjs b/tests/claude-orchestration.test.cjs index 86fa0bcff..dec26bab3 100644 --- a/tests/claude-orchestration.test.cjs +++ b/tests/claude-orchestration.test.cjs @@ -806,7 +806,7 @@ describe('capability declaration (capabilities/claude-orchestration/capability.j assert.strictEqual(slice.default, 'auto'); }); - test('registers at WIRED points only (execute:wave:pre, plan:post)', () => { + test('registers at WIRED points only (plan:post) — #4740 removed execute:wave:pre', () => { const cap = loadCap(); const points = cap.contributions.map((c) => c.point); for (const p of points) { @@ -815,10 +815,10 @@ describe('capability declaration (capabilities/claude-orchestration/capability.j 'contribution point ' + p + ' must be a wired point', ); } - // #2285: the dispatch-backend selector moved from execute:wave:post (fires - // AFTER the wave already dispatched inline — too late to select a backend) - // to execute:wave:pre (fires BEFORE step 3's Agent() dispatch). - assert.ok(points.includes('execute:wave:pre'), 'registers the pre-wave dispatch-selector hook'); + // #4740: the execute:wave:pre / into:executor contribution was pure + // orchestrator procedure with nothing an executor agent could act on — + // it was removed, not retargeted. Only plan:post (into:planner) remains. + assert.ok(!points.includes('execute:wave:pre'), 'no longer registers at execute:wave:pre (#4740)'); assert.ok(points.includes('plan:post'), 'declares plan:* ownership for ultraplan (criterion 5)'); }); @@ -852,16 +852,15 @@ describe('registry integration', () => { assert.strictEqual(registry.configSchema['claude_orchestration.execution_backend'].default, 'auto'); }); - test('byLoopPoint[execute:wave:pre].contributions includes our capability (#2285)', () => { + test('byLoopPoint[execute:wave:pre] no longer carries our contribution (#4740 removed it)', () => { const { capMap } = loadAndValidate(new Set()); const registry = buildRegistry(capMap); const contribs = registry.byLoopPoint['execute:wave:pre'].contributions; const ours = contribs.find((c) => c.capId === 'claude-orchestration'); - assert.ok(ours, 'our execute:wave:pre contribution is registered'); - assert.strictEqual(ours.into, 'executor'); + assert.strictEqual(ours, undefined, 'claude-orchestration must not remain at execute:wave:pre (#4740)'); }); - test('byLoopPoint[execute:wave:post] no longer carries our contribution (#2285 moved it to wave:pre)', () => { + test('byLoopPoint[execute:wave:post] does not carry our contribution', () => { const { capMap } = loadAndValidate(new Set()); const registry = buildRegistry(capMap); const contribs = registry.byLoopPoint['execute:wave:post'].contributions; @@ -1515,28 +1514,30 @@ describe('C. resolveWaveDispatch composes detectWorkflowBackend + emitWorkflowSc }); }); -// ─── Section D: capability declaration now targets execute:wave:pre ───────── +// ─── Section D: capability declaration no longer contributes at execute:wave:pre (#4740) ── -describe('D. capability.json declares the contribution at execute:wave:pre (#2285)', () => { - test('[happy] contribution point is execute:wave:pre, not execute:wave:post', () => { +describe('D. capability.json no longer declares a contribution at execute:wave:pre (#4740)', () => { + test('[happy] neither execute:wave:pre nor execute:wave:post carries a contribution', () => { + // #4740: the execute:wave:pre / into:executor contribution was pure + // orchestrator procedure (build a wave manifest, resolve the dispatch + // backend, spawn executor agents) with nothing an executor agent can + // act on — injecting it into executor prompts violated the loop's role + // partition. It was removed outright, not retargeted. const cap = JSON.parse(fs.readFileSync(CAP_PATH, 'utf8')); const wavePreContrib = cap.contributions.find((c) => c.point === 'execute:wave:pre'); - assert.ok(wavePreContrib, 'capability.json must declare a contribution at execute:wave:pre'); - assert.strictEqual(wavePreContrib.into, 'executor'); - assert.strictEqual(wavePreContrib.when, 'claude_orchestration.enabled'); - assert.strictEqual(wavePreContrib.onError, 'skip'); - assert.strictEqual(wavePreContrib.fragment.path, 'fragments/execute-wave-pre.md'); + assert.strictEqual(wavePreContrib, undefined, 'capability.json must no longer declare a contribution at execute:wave:pre'); const wavePostContrib = cap.contributions.find((c) => c.point === 'execute:wave:post'); - assert.strictEqual(wavePostContrib, undefined, 'the capability must no longer contribute at execute:wave:post'); + assert.strictEqual(wavePostContrib, undefined, 'the capability must not contribute at execute:wave:post either'); }); - test('[happy] the declared fragment file exists on disk', () => { - const fragPath = path.join(ROOT, 'capabilities', 'claude-orchestration', 'fragments', 'execute-wave-pre.md'); - assert.ok(fs.existsSync(fragPath), 'fragments/execute-wave-pre.md must exist'); - const content = fs.readFileSync(fragPath, 'utf8'); + test('[happy] the preserved procedure doc exists on disk (moved, not deleted)', () => { + const docPath = path.join(ROOT, 'capabilities', 'claude-orchestration', 'docs', 'workflow-backend-dispatch.md'); + assert.ok(fs.existsSync(docPath), 'capabilities/claude-orchestration/docs/workflow-backend-dispatch.md must exist'); + const content = fs.readFileSync(docPath, 'utf8'); assert.match(content, /execute:wave:pre/); assert.match(content, /resolve-wave-dispatch/); + assert.match(content, /#4740/, 'header must reference the issue that removed the contribution'); }); }); @@ -1759,9 +1760,11 @@ describe('G. missing top-level `waves` key never silently exits 0 with no output // ─── Section H: orthogonal-review finding 3 — manifest construction guidance is concrete ── -describe('H. the execute:wave:pre fragment documents concrete manifest construction (finding 3)', () => { - test('[happy] the fragment explains how to build WAVE_MANIFEST_PATH, PHASE_RUN_ID, and per-plan use_worktree', () => { - const fragPath = path.join(ROOT, 'capabilities', 'claude-orchestration', 'fragments', 'execute-wave-pre.md'); +describe('H. the workflow-backend-dispatch doc documents concrete manifest construction (finding 3)', () => { + test('[happy] the doc explains how to build WAVE_MANIFEST_PATH, PHASE_RUN_ID, and per-plan use_worktree', () => { + // #4740: moved out of capabilities/claude-orchestration/fragments (no + // longer a loop contribution) to capabilities/claude-orchestration/docs. + const fragPath = path.join(ROOT, 'capabilities', 'claude-orchestration', 'docs', 'workflow-backend-dispatch.md'); const content = fs.readFileSync(fragPath, 'utf8'); assert.match(content, /Manifest construction/, 'fragment must have concrete manifest-construction guidance, not just reference undefined vars'); assert.match(content, /PHASE_RUN_ID/); @@ -1797,17 +1800,22 @@ describe('H. the execute:wave:pre fragment documents concrete manifest construct }); }); -// ─── Section I: orthogonal-review finding 4 — stale doc fixed ─────────────── +// ─── Section I: docs/explanation/claude-orchestration-capability.md reflects the #4740 removal ── -describe('I. docs/explanation/claude-orchestration-capability.md reflects the execute:wave:pre move (finding 4)', () => { - test('[happy] the doc no longer claims the capability registers at execute:wave:post', () => { +describe('I. docs/explanation/claude-orchestration-capability.md reflects the #4740 removal', () => { + test('[happy] the doc no longer claims the capability registers at execute:wave:pre or execute:wave:post', () => { const docPath = path.join(ROOT, 'docs', 'explanation', 'claude-orchestration-capability.md'); const content = fs.readFileSync(docPath, 'utf8'); - assert.match(content, /execute:wave:pre/, 'doc must mention execute:wave:pre as the wired point'); + assert.match( + content, + /Registers at one \*\*wired\*\* loop point: `plan:post`/, + 'doc must state the capability registers at exactly one wired loop point (plan:post)', + ); assert.ok( !/execute:wave:post.*\(into the executor\)/.test(content), 'doc must not still claim the wired point is execute:wave:post', ); + assert.match(content, /plan:post/, 'doc must still mention the surviving plan:post contribution'); }); }); @@ -2231,8 +2239,11 @@ describe('#3302: emitted Workflow script returns per-agent outcomes for the mani }); }); -describe('#3302: the execute:wave:pre fragment bridges Workflow results into the manifest merge chain', () => { - const FRAG_3302 = path.join(ROOT, 'capabilities', 'claude-orchestration', 'fragments', 'execute-wave-pre.md'); +describe('#3302: the workflow-backend-dispatch doc bridges Workflow results into the manifest merge chain', () => { + // #4740: this content is no longer an injected loop contribution (it was + // orchestrator procedure wrongly targeted `into: executor`); it now lives + // as a plain reference doc, content preserved verbatim. + const FRAG_3302 = path.join(ROOT, 'capabilities', 'claude-orchestration', 'docs', 'workflow-backend-dispatch.md'); function fragContent() { return fs.readFileSync(FRAG_3302, 'utf8'); diff --git a/tests/execute-wave-post-gate-pipeline-e2e.test.cjs b/tests/execute-wave-post-gate-pipeline-e2e.test.cjs index 42ef519bd..1643d7fde 100644 --- a/tests/execute-wave-post-gate-pipeline-e2e.test.cjs +++ b/tests/execute-wave-post-gate-pipeline-e2e.test.cjs @@ -652,9 +652,9 @@ describe('F. Real registry execute:wave:post shape — guard against accidental `execute:wave:post step ref must be { agent: 'gsd-dom-verifier' }; got ${JSON.stringify(domUatStep.ref)}`); assert.strictEqual(domUatStep.onError, 'skip', `execute:wave:post step onError must be 'skip'; got ${domUatStep.onError}`); - // #2285: claude-orchestration's dispatch-backend-selector contribution moved - // from execute:wave:post to execute:wave:pre — wave:post fires AFTER the - // wave already dispatched inline, too late to select a dispatch backend. + // #4740: claude-orchestration never contributed here (external-job + + // mempalace are unrelated capabilities), so this assertion is unaffected + // by #4740 removing claude-orchestration's execute:wave:pre contribution. assert.strictEqual(point.contributions.length, 2, `execute:wave:post must have 2 contributions (external-job + mempalace); got ${point.contributions.length}`); const capIds = point.contributions.map(c => c.capId).sort(); @@ -662,15 +662,21 @@ describe('F. Real registry execute:wave:post shape — guard against accidental `execute:wave:post contributions must be external-job + mempalace; got ${capIds.join(',')}`); }); - test('[happy] real registry: execute:wave:pre has 1 contribution (claude-orchestration dispatch-backend selector, #2285)', () => { + test('[happy] real registry: execute:wave:pre has 0 contributions (#4740 removed claude-orchestration\'s)', () => { + // #4740: the execute:wave:pre / into:executor contribution was pure + // orchestrator procedure (build a wave manifest, resolve the dispatch + // backend, spawn executor agents) with nothing an executor agent could + // act on — orchestration is not delivered through an agent contribution, + // so it was removed outright rather than retargeted. No capability + // contributes at execute:wave:pre anymore. const point = realRegistry.byLoopPoint['execute:wave:pre']; assert.strictEqual(point.steps.length, 0, `execute:wave:pre steps must be empty; got ${point.steps.length}`); - assert.strictEqual(point.contributions.length, 1, - `execute:wave:pre must have 1 contribution (claude-orchestration); got ${point.contributions.length}`); + assert.strictEqual(point.contributions.length, 0, + `execute:wave:pre must have 0 contributions (#4740); got ${point.contributions.length}`); const capIds = point.contributions.map(c => c.capId).sort(); - assert.deepStrictEqual(capIds, ['claude-orchestration'], - `execute:wave:pre contributions must be claude-orchestration; got ${capIds.join(',')}`); + assert.deepStrictEqual(capIds, [], + `execute:wave:pre contributions must be empty; got ${capIds.join(',')}`); }); }); diff --git a/tests/loop-host-contract.test.cjs b/tests/loop-host-contract.test.cjs index 3435c8eda..a7053d0f4 100644 --- a/tests/loop-host-contract.test.cjs +++ b/tests/loop-host-contract.test.cjs @@ -13,12 +13,14 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const fc = require('fast-check'); const { cleanup } = require('./helpers.cjs'); const { parseLoopHostBlock, crossCheckRoles, + crossCheckRoleFamilies, assertPointsCoverage, buildContract, serializeContract, @@ -27,6 +29,7 @@ const { CANONICAL_POINTS, EXPECTED_POINTS_BY_STEP, ROLE_TO_AGENT, + ROLE_FAMILY, } = require('../scripts/gen-loop-host-contract.cjs'); const { LOOP_HOST_CONTRACT } = require('../gsd-core/bin/lib/loop-host-contract.cjs'); @@ -202,6 +205,158 @@ describe('crossCheckRoles', () => { }); }); +// ─── 2b. crossCheckRoleFamilies (#4740) ────────────────────────────────────── + +describe('crossCheckRoleFamilies (#4740)', () => { + test('accepts the execution family at execute', () => { + const errors = crossCheckRoleFamilies('execute', ['executor', 'verifier'], 'execute-phase.md'); + assert.deepEqual(errors, []); + }); + + test('accepts orchestrator at discuss', () => { + const errors = crossCheckRoleFamilies('discuss', ['orchestrator'], 'discuss-phase.md'); + assert.deepEqual(errors, []); + }); + + test('accepts the planning family at plan', () => { + const errors = crossCheckRoleFamilies('plan', ['researcher', 'planner', 'checker'], 'plan-phase.md'); + assert.deepEqual(errors, []); + }); + + test('accepts orchestrator at verify and ship', () => { + assert.deepEqual(crossCheckRoleFamilies('verify', ['orchestrator'], 'verify-work.md'), []); + assert.deepEqual(crossCheckRoleFamilies('ship', ['orchestrator'], 'ship.md'), []); + }); + + test('a strict subset of the family is legal', () => { + const errors = crossCheckRoleFamilies('execute', ['executor'], 'execute-phase.md'); + assert.deepEqual(errors, []); + }); + + test('rejects orchestrator added to execute', () => { + const errors = crossCheckRoleFamilies('execute', ['executor', 'verifier', 'orchestrator'], 'execute-phase.md'); + assert.strictEqual(errors.length, 1, 'expected exactly 1 error'); + assert.match(errors[0], /orchestrator/); + assert.match(errors[0], /execute/); + }); + + test('rejects an execution role at an orchestration step', () => { + const errors = crossCheckRoleFamilies('discuss', ['orchestrator', 'executor'], 'discuss-phase.md'); + assert.strictEqual(errors.length, 1, 'expected exactly 1 error'); + assert.match(errors[0], /executor/); + }); + + test('rejects an execution role at plan', () => { + const errors = crossCheckRoleFamilies('plan', ['planner', 'executor'], 'plan-phase.md'); + assert.strictEqual(errors.length, 1, 'expected exactly 1 error'); + assert.match(errors[0], /executor/); + }); + + test('rejects a planning role at execute', () => { + const errors = crossCheckRoleFamilies('execute', ['executor', 'planner'], 'execute-phase.md'); + assert.strictEqual(errors.length, 1, 'expected exactly 1 error'); + assert.match(errors[0], /planner/); + }); + + test('rejects a role with no family', () => { + const errors = crossCheckRoleFamilies('execute', ['executer'], 'execute-phase.md'); + assert.ok(errors.length >= 1, 'expected at least 1 error'); + assert.ok(errors.some((e) => e.includes('executer'))); + }); + + test('fails closed on an unknown step', () => { + const errors = crossCheckRoleFamilies('audit', ['orchestrator'], 'audit.md'); + assert.strictEqual(errors.length, 1, 'expected exactly 1 error for unknown step'); + assert.match(errors[0], /audit/); + }); + + test('rejects a role whose capitalization does not match', () => { + const errors = crossCheckRoleFamilies('execute', ['Orchestrator'], 'execute-phase.md'); + assert.ok(errors.length >= 1, 'capitalized role must not silently match'); + assert.ok(errors.some((e) => e.includes('Orchestrator'))); + }); + + test('a duplicated in-family role is not an error', () => { + const errors = crossCheckRoleFamilies('execute', ['executor', 'executor'], 'execute-phase.md'); + assert.deepEqual(errors, []); + }); + + test('reports every offending role, not just the first', () => { + const errors = crossCheckRoleFamilies('execute', ['executor', 'orchestrator', 'planner'], 'execute-phase.md'); + assert.strictEqual(errors.length, 2, 'expected one error per offending role'); + const combined = errors.join('\n'); + assert.ok(combined.includes('orchestrator')); + assert.ok(combined.includes('planner')); + }); + + test('is pure and does not mutate its arguments', () => { + // Deliberately NOT alphabetically sorted — an in-place agentRoles.sort() + // must be caught by THIS test, not merely coincide with already-sorted + // input (#4740 review finding). + const agentRoles = ['verifier', 'executor', 'orchestrator']; + const snapshot = agentRoles.slice(); + const first = crossCheckRoleFamilies('execute', agentRoles, 'execute-phase.md'); + assert.deepEqual(agentRoles, snapshot, 'input array must not be mutated'); + const second = crossCheckRoleFamilies('execute', agentRoles, 'execute-phase.md'); + assert.deepEqual(first, second, 'repeated calls must produce identical results'); + assert.deepEqual(agentRoles, snapshot, 'input array must still not be mutated after second call'); + }); + + test('ROLE_FAMILY and ROLE_TO_AGENT cover the exact same role-name domain (#4740)', () => { + // Generative-fix-divergence guard: ROLE_FAMILY and ROLE_TO_AGENT are parallel + // constants over the same role-name domain (every non-orchestrator role, plus + // orchestrator which is family-only). Exact set equality both directions — + // not a subset check — so adding a role to only one map is caught here. + const familyKeys = new Set(Object.keys(ROLE_FAMILY)); + const expectedKeys = new Set([...Object.keys(ROLE_TO_AGENT), 'orchestrator']); + assert.deepEqual( + [...familyKeys].sort(), + [...expectedKeys].sort(), + 'ROLE_FAMILY keys must equal ROLE_TO_AGENT keys plus "orchestrator", exactly', + ); + }); + + test('property: in-family subsets pass, any foreign role fails', () => { + const ROLE_FAMILY_LOCAL = { + orchestrator: 'orchestration', + researcher: 'planning', planner: 'planning', checker: 'planning', + executor: 'execution', verifier: 'execution', + }; + const familyRoles = { + orchestration: ['orchestrator'], + planning: ['researcher', 'planner', 'checker'], + execution: ['executor', 'verifier'], + }; + const stepsByFamily = { + discuss: 'orchestration', plan: 'planning', execute: 'execution', + verify: 'orchestration', ship: 'orchestration', + }; + const steps = Object.keys(stepsByFamily); + + fc.assert( + fc.property( + fc.constantFrom(...steps), + fc.subarray(Object.keys(ROLE_FAMILY_LOCAL), { minLength: 1 }), + (step, roles) => { + const family = stepsByFamily[step]; + const inFamilySubset = familyRoles[family]; + const errors = crossCheckRoleFamilies(step, inFamilySubset, 'x.md'); + assert.deepEqual(errors, []); + + const hasForeign = roles.some((r) => ROLE_FAMILY_LOCAL[r] !== family); + const errors2 = crossCheckRoleFamilies(step, roles, 'x.md'); + if (hasForeign) { + assert.ok(errors2.length >= 1); + } else { + assert.deepEqual(errors2, []); + } + }, + ), + { seed: 4740, numRuns: 200 }, + ); + }); +}); + // ─── 3. assertPointsCoverage ───────────────────────────────────────────────── describe('assertPointsCoverage', () => { @@ -333,6 +488,19 @@ describe('buildContract from real workflows', () => { assert.deepEqual(ship.coreArtifacts.produces, []); assert.deepEqual(ship.coreArtifacts.consumes, ['UAT.md']); }); + + test('the shipped workflows declare no cross-family role (#4740)', () => { + // Pins something real about THIS change, distinct from the per-step field + // assertions above: running crossCheckRoleFamilies directly against every + // entry of the REAL contract returns zero errors for every step, not merely + // that buildContract() didn't throw (which every other test in this describe + // block already exercises incidentally). + const contract = buildContract(); + for (const entry of contract) { + const errors = crossCheckRoleFamilies(entry.step, entry.agentRoles, entry.step + '.md'); + assert.deepEqual(errors, [], 'step "' + entry.step + '" must declare no cross-family role'); + } + }); }); // ─── 5. cross-check rejects nonexistent agent-role ───────────────────────────