diff --git a/.changeset/913-plan-phase-toplevel-spawn-guard.md b/.changeset/913-plan-phase-toplevel-spawn-guard.md new file mode 100644 index 000000000..3a4b1ade8 --- /dev/null +++ b/.changeset/913-plan-phase-toplevel-spawn-guard.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 913 +--- +**Top-level Claude Code `/gsd-plan-phase` now always spawns the researcher/planner/plan-checker agents instead of collapsing them inline** — a `` block after `` makes the Agent-availability requirement explicit and documents that the workflow fails-closed (stops with a clear log message) in genuinely Agent-less contexts; seven "ORCHESTRATOR RULE — CODEX RUNTIME" labels are renamed to "ALL RUNTIMES" so the guard applies universally; `execute-phase.md` scopes its existing "Other runtimes" inline-fallback prose to non-Claude contexts, preserving the #853 backgrounded-agent behaviour. (#913) diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 19921aea8..0a748fb2f 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -14,8 +14,11 @@ Orchestrator coordinates, not executes. Each subagent loads the full execute-pla instead of spawning parallel agents. Only attempt parallel spawning if the user explicitly requests it — and in that case, rely on the spot-check fallback in step 3 to detect completion. -- **Other runtimes:** If `Agent`/`agent` tool is unavailable, use sequential inline execution as the - fallback. Check for tool availability at runtime rather than assuming based on runtime name. +- **Other runtimes:** If `Agent`/`agent` tool is genuinely unavailable (e.g. a backgrounded + Claude Code agent per #853, or a non-Claude runtime), use sequential inline execution as + the fallback for executor parallelization only. If `Agent` IS available (top-level Claude + Code), you MUST spawn gsd-executor agents — inline execution is not authorized. Check for + actual tool availability, not runtime name. **Fallback rule:** If a spawned agent completes its work (commits visible, SUMMARY.md exists) but the orchestrator never receives the completion signal, treat it as successful based on spot-checks diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index 4b1b702f5..303b5d299 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -20,6 +20,31 @@ Valid GSD subagent types (use exact names — do not fall back to 'general-purpo - gsd-plan-checker — Reviews plan quality before execution + +**Subagent spawning — top-level Claude Code:** +The Agent tool IS available in a top-level Claude Code session. Always spawn +gsd-phase-researcher, gsd-planner, and gsd-plan-checker as separate Agent() calls. +Never absorb these roles inline. Role separation is required regardless of `--chain` +or `--auto` — those options suppress interactive prompts only; they NEVER authorize +collapsing plan roles into the orchestrator context. + +**Backgrounded Claude Code (via manager/autonomous):** +The calling workflow (manager.md / autonomous.md) already runs plan-phase inline via +Skill() on Claude Code so that the plan-checker subagent can still spawn. plan-phase +itself does not need to detect this case. + +**#1009 caveat (discuss-phase early-exit):** +The "display the command and exit" instruction near `## 4` applies only to the +discuss-phase early-exit path. It does NOT authorize inline role performance for any +plan-phase agents. + +**Other runtimes:** +If the Agent tool is genuinely absent (e.g. a backgrounded Claude Code agent per +#853, or a non-Claude runtime that does not expose Agent/agent), log the gap and +stop — do NOT perform researcher/planner/checker roles inline. Independent agent +contexts are required for the plan-checker gate to be meaningful. + + ## 0. Git Branch Invariant @@ -530,7 +555,7 @@ Agent( ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. +> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. ### Handle Researcher Return @@ -849,7 +874,7 @@ Agent( ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. +> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. **Handle return:** - **`## PATTERN MAPPING COMPLETE`:** Update `PATTERNS_PATH` to the created file path, continue to step 8. @@ -1012,7 +1037,7 @@ Agent( ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. +> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. **If `CHUNKED_MODE` is `true`:** Skip the Agent() call above — proceed to step 8.5 instead. @@ -1068,7 +1093,7 @@ Agent( ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. +> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. Handle return: - **`## OUTLINE COMPLETE`:** Read `PLAN-OUTLINE.md`, extract plan list. Continue to 8.5.2. @@ -1112,7 +1137,7 @@ For each plan entry extracted from `PLAN-OUTLINE.md`: ) ``` - > **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. + > **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. 4. **Verify disk:** Check `${PHASE_DIR}/{plan_id}-PLAN.md` exists. If missing: offer 1) Retry, 2) Stop. @@ -1270,7 +1295,7 @@ Agent( ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. +> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. ## 11. Handle Checker Return @@ -1385,7 +1410,7 @@ Agent( ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. +> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available. After planner returns -> spawn checker again (step 10), increment iteration_count. diff --git a/tests/plan-phase-drift-guard.test.cjs b/tests/plan-phase-drift-guard.test.cjs index 2e8b84794..9ef1e5bf3 100644 --- a/tests/plan-phase-drift-guard.test.cjs +++ b/tests/plan-phase-drift-guard.test.cjs @@ -135,3 +135,61 @@ describe('plan-phase workflow: Artifacts this phase produces section (#22)', () ); }); }); + +// ─── (C) Top-level spawn guard (#913) ──────────────────────────────────────── + +describe('plan-phase workflow: top-level spawn guard (#913)', () => { + // Extract the runtime_compatibility block for targeted assertions + const rtBlock = (() => { + const m = workflow.match(/([\s\S]*?)<\/runtime_compatibility>/); + return m ? m[1] : ''; + })(); + + test('workflow has a runtime_compatibility block asserting Agent is available at top-level', () => { + assert.ok( + rtBlock.length > 0, + 'plan-phase must have a block — prevents role-collapse regression (#913)' + ); + assert.ok( + rtBlock.includes('Agent tool IS available') || rtBlock.includes('Agent IS available'), + 'plan-phase runtime_compatibility must assert that the Agent tool IS available at top-level Claude Code (#913)' + ); + assert.ok( + rtBlock.toLowerCase().includes('top-level'), + 'plan-phase runtime_compatibility must scope the IS-available assertion to top-level Claude Code (#913)' + ); + assert.ok( + rtBlock.includes('Always spawn') || rtBlock.includes('always spawn'), + 'plan-phase runtime_compatibility must state that plan roles must always be spawned (#913)' + ); + assert.ok( + rtBlock.includes('Never absorb') || rtBlock.includes('never absorb'), + 'plan-phase runtime_compatibility must state that roles must never be absorbed inline (#913)' + ); + }); + + test('workflow states --chain/--auto suppress prompts only, not spawns', () => { + assert.ok( + rtBlock.includes('suppress') && + (rtBlock.includes('prompts only') || rtBlock.includes('interactive prompts only')), + 'plan-phase runtime_compatibility must document that --chain/--auto suppress prompts only, not spawns (#913)' + ); + }); + + test('workflow does not contain unscoped CODEX RUNTIME orchestrator rule labels', () => { + // All "wait for subagent" rules must apply to ALL RUNTIMES, not just Codex + assert.ok( + !workflow.includes('ORCHESTRATOR RULE — CODEX RUNTIME'), + 'plan-phase must not label orchestrator wait rules as "CODEX RUNTIME" — they apply to all runtimes including top-level Claude Code (#913)' + ); + }); + + test('workflow contains ALL RUNTIMES orchestrator rule labels (count preserved)', () => { + // Must have all 7 agent-spawn wait rules still present (none dropped during rename) + const allRuntimesCount = (workflow.match(/ORCHESTRATOR RULE — ALL RUNTIMES/g) || []).length; + assert.ok( + allRuntimesCount >= 7, + `plan-phase must have at least 7 "ORCHESTRATOR RULE — ALL RUNTIMES" labels (one per agent spawn site); found ${allRuntimesCount} (#913)` + ); + }); +}); diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index 1c75523f4..7713b2be7 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -81,8 +81,8 @@ const GRACE = 3000; // current high-water mark within GRACE (#597 tighten-only ratchet). // XL high-water mark is execute-phase.md — note that under LINES it was // plan-phase; bytes genuinely re-rank the tier, which is the point of #717. -// actualMax=91161 (execute-phase, #891 launcher shim expansion — added 17 runtime home arms); -// slack=1839 ≤ GRACE. plan-phase.md=88120, new-project.md=58110; both well under ceiling. +// actualMax=92525 (execute-phase, #913 inline-fallback scope clarification); +// slack=475 ≤ GRACE. plan-phase.md=90501 (#913 runtime_compatibility block + label rename), new-project.md=58110. const XL_BUDGET = 93000; // LARGE high-water mark is docs-update.md. actualMax=54410 (#891 launcher shim expansion); // slack=1590 ≤ GRACE. quick.md=45710, autonomous.md=38030. @@ -95,8 +95,8 @@ const DEFAULT_BUDGET = 40000; // Grandfathered at current sizes — see PR #2551 for the progressive-disclosure // pattern that future shrinks should follow. Byte counts noted for reference. const XL_WORKFLOWS = new Set([ - 'execute-phase', // 91161 bytes (tier high-water mark; grew in #891 launcher shim expansion) - 'plan-phase', // 85068 bytes + 'execute-phase', // 92525 bytes (tier high-water mark; grew in #913 inline-fallback scope clarification) + 'plan-phase', // 90501 bytes (grew in #913 runtime_compatibility block + label rename) 'new-project', // 55850 bytes ]);