diff --git a/.changeset/921-922-orchestrators-no-fork.md b/.changeset/921-922-orchestrators-no-fork.md new file mode 100644 index 000000000..92e481ef7 --- /dev/null +++ b/.changeset/921-922-orchestrators-no-fork.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 921 +--- +**`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous` no longer carry `context: fork`** — these are spawning orchestrators; a forked subagent context has no `Agent` tool, preventing them from spawning the subagents they require. `effort: xhigh` is preserved. Fixes `/gsd:autonomous` halting with "running as a forked subagent" on 1.4.1 (#921). Also replaces the introspection-based Agent-availability check in `plan-phase`'s `` block with an attempt-based gate: the workflow now always attempts the `Agent()` call and only stops if a real tool-unavailable error is returned, eliminating false-negative aborts in top-level sessions (#922). diff --git a/commands/gsd/autonomous.md b/commands/gsd/autonomous.md index fce955925..9a13ebf21 100644 --- a/commands/gsd/autonomous.md +++ b/commands/gsd/autonomous.md @@ -2,7 +2,6 @@ name: gsd:autonomous description: Run all remaining phases autonomously — discuss→plan→execute per phase argument-hint: "[--from N] [--to N] [--only N] [--interactive]" -context: fork effort: xhigh allowed-tools: - Read diff --git a/commands/gsd/execute-phase.md b/commands/gsd/execute-phase.md index b7acb5885..c7eb7bdca 100644 --- a/commands/gsd/execute-phase.md +++ b/commands/gsd/execute-phase.md @@ -2,7 +2,6 @@ name: gsd:execute-phase description: Execute all plans in a phase with wave-based parallelization argument-hint: " [--wave N] [--gaps-only] [--interactive] [--tdd]" -context: fork effort: xhigh allowed-tools: - Read diff --git a/commands/gsd/plan-phase.md b/commands/gsd/plan-phase.md index 47549dfbf..d4f071145 100644 --- a/commands/gsd/plan-phase.md +++ b/commands/gsd/plan-phase.md @@ -2,7 +2,6 @@ name: gsd:plan-phase description: Create detailed phase plan (PLAN.md) with verification loop argument-hint: "[phase] [--auto] [--research] [--skip-research] [--research-phase ] [--view] [--gaps] [--skip-verify] [--prd ] [--ingest ] [--ingest-format ] [--reviews] [--text] [--tdd] [--mvp]" -context: fork effort: xhigh allowed-tools: - Read diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 438bcb6dd..4b3b67ba0 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -14,7 +14,7 @@ The hyphen and colon forms are *runtime-specific spellings of the same command*. ### Skill Runtime Behavior (Claude Code) -Heavy workflow skills (`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous`) carry `context: fork` in their frontmatter. On Claude Code, this runs each skill in an isolated subagent context window, protecting the main session's context budget. The skills also declare `effort: xhigh`, signalling maximum token budget to the runtime. +Heavy workflow skills (`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous`) declare `effort: xhigh`, signalling maximum token budget to the runtime. These skills are spawning orchestrators — they must run at top level so they retain the `Agent` tool needed to spawn subagents. They do **not** carry `context: fork` (see #921). Quick-status skills (`/gsd-progress`, `/gsd-stats`) declare `effort: low`, directing the runtime to use a minimal token budget for fast reads. diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index 303b5d299..e3989395a 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -39,10 +39,13 @@ discuss-phase early-exit path. It does NOT authorize inline role performance for 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. +Do not pre-judge Agent availability by introspection. Always attempt the actual +Agent() call for gsd-phase-researcher, gsd-planner, and gsd-plan-checker. Only +a real tool-unavailable error returned by Agent() is a reliable absence signal — +never stop based on a self-assessed "I think Agent is unavailable." If the call +fails with a tool-unavailable error, log the gap and stop — do NOT collapse +researcher/planner/checker roles inline. Independent agent contexts are required +for the plan-checker gate to be meaningful. diff --git a/tests/enh-769-context-fork-effort.install.test.cjs b/tests/enh-769-context-fork-effort.install.test.cjs index 125730723..6fff8c9df 100644 --- a/tests/enh-769-context-fork-effort.install.test.cjs +++ b/tests/enh-769-context-fork-effort.install.test.cjs @@ -4,20 +4,28 @@ // transformation is asserted — not inspected for string presence. /** - * #769 — context:fork + effort: frontmatter on heavy workflow skills. + * #769 — effort: frontmatter on heavy workflow skills. + * #921 — spawning orchestrators must NOT carry context: fork. + * + * Context: context:fork was added by #769 to protect context budget, but + * plan-phase, execute-phase, and autonomous are spawning orchestrators — a + * forked subagent has no Agent/Task tool, breaking their core function. + * effort: xhigh is preserved; context: fork is removed from these three. + * The converter still passes context: fork through if a source file has it + * (for any future leaf skill that legitimately needs isolation). * * Verifies: - * 1. Source commands/gsd/autonomous.md has context: fork and effort: xhigh - * 2. Source commands/gsd/execute-phase.md has context: fork and effort: xhigh - * 3. Source commands/gsd/plan-phase.md has context: fork and effort: xhigh + * 1. Source commands/gsd/autonomous.md does NOT have context: fork, has effort: xhigh + * 2. Source commands/gsd/execute-phase.md does NOT have context: fork, has effort: xhigh + * 3. Source commands/gsd/plan-phase.md does NOT have context: fork, has effort: xhigh * 4. Source commands/gsd/progress.md has effort: low * 5. Source commands/gsd/stats.md has effort: low - * 6. Claude global install: SKILL.md for autonomous has context: fork and effort: xhigh - * 7. Claude global install: SKILL.md for execute-phase has context: fork and effort: xhigh - * 8. Claude global install: SKILL.md for plan-phase has context: fork and effort: xhigh + * 6. Claude global install: SKILL.md for autonomous has effort: xhigh, NOT context: fork + * 7. Claude global install: SKILL.md for execute-phase has effort: xhigh, NOT context: fork + * 8. Claude global install: SKILL.md for plan-phase has effort: xhigh, NOT context: fork * 9. Claude global install: SKILL.md for progress has effort: low * 10. Claude global install: SKILL.md for stats has effort: low - * 11. convertClaudeCommandToClaudeSkill preserves context: fork field + * 11. convertClaudeCommandToClaudeSkill still passes context: fork through (for non-orchestrator skills) * 12. convertClaudeCommandToClaudeSkill preserves effort: field */ @@ -90,11 +98,15 @@ function runClaudeGlobalInstall(claudeHome) { // ─── describe 1: Source command files have correct frontmatter ──────────────── -describe('#769 source commands: heavy skills have context: fork and effort: xhigh', () => { - test('commands/gsd/autonomous.md has context: fork', () => { +// #921/#922: spawning orchestrators must NOT carry context: fork — a forked +// subagent has no Agent/Task tool, making it impossible for orchestrators to +// spawn their required subagents. context: fork is appropriate only for leaf +// skills that do not themselves dispatch agents. effort: xhigh is preserved. +describe('#769/#921 source commands: spawning orchestrators have effort: xhigh but NOT context: fork', () => { + test('commands/gsd/autonomous.md does NOT have context: fork (#921)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'autonomous.md')); - assert.match(fm, /^context:[ \t]*fork$/m, - `autonomous.md frontmatter must have context: fork\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, + `autonomous.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`); }); test('commands/gsd/autonomous.md has effort: xhigh', () => { @@ -103,10 +115,10 @@ describe('#769 source commands: heavy skills have context: fork and effort: xhig `autonomous.md frontmatter must have effort: xhigh\nActual:\n${fm}`); }); - test('commands/gsd/execute-phase.md has context: fork', () => { + test('commands/gsd/execute-phase.md does NOT have context: fork (#921)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'execute-phase.md')); - assert.match(fm, /^context:[ \t]*fork$/m, - `execute-phase.md frontmatter must have context: fork\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, + `execute-phase.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`); }); test('commands/gsd/execute-phase.md has effort: xhigh', () => { @@ -115,10 +127,10 @@ describe('#769 source commands: heavy skills have context: fork and effort: xhig `execute-phase.md frontmatter must have effort: xhigh\nActual:\n${fm}`); }); - test('commands/gsd/plan-phase.md has context: fork', () => { + test('commands/gsd/plan-phase.md does NOT have context: fork (#921)', () => { const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'plan-phase.md')); - assert.match(fm, /^context:[ \t]*fork$/m, - `plan-phase.md frontmatter must have context: fork\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, + `plan-phase.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`); }); test('commands/gsd/plan-phase.md has effort: xhigh', () => { @@ -237,7 +249,9 @@ describe('#769 convertClaudeCommandToClaudeSkill: preserves context and effort f // ─── describe 3: Claude global install — SKILL.md files include new fields ──── -describe('#769 Claude global install: SKILL.md files preserve context: fork and effort:', () => { +// #921/#922: after install, spawning orchestrators must NOT carry context: fork +// in their emitted SKILL.md. effort: xhigh is still emitted (preserved from source). +describe('#769/#921 Claude global install: spawning-orchestrator SKILL.md files have effort: xhigh but NOT context: fork', () => { let tmpDir; let claudeHome; @@ -251,12 +265,12 @@ describe('#769 Claude global install: SKILL.md files preserve context: fork and cleanup(tmpDir); }); - test('gsd-autonomous SKILL.md has context: fork after global install', () => { + test('gsd-autonomous SKILL.md does NOT have context: fork after global install (#921)', () => { runClaudeGlobalInstall(claudeHome); const skillPath = path.join(claudeHome, 'skills', 'gsd-autonomous', 'SKILL.md'); const fm = readFrontmatter(skillPath); - assert.match(fm, /^context:[ \t]*fork$/m, - `gsd-autonomous SKILL.md must have context: fork\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, + `gsd-autonomous is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`); }); test('gsd-autonomous SKILL.md has effort: xhigh after global install', () => { @@ -267,12 +281,12 @@ describe('#769 Claude global install: SKILL.md files preserve context: fork and `gsd-autonomous SKILL.md must have effort: xhigh\nActual:\n${fm}`); }); - test('gsd-execute-phase SKILL.md has context: fork after global install', () => { + test('gsd-execute-phase SKILL.md does NOT have context: fork after global install (#921)', () => { runClaudeGlobalInstall(claudeHome); const skillPath = path.join(claudeHome, 'skills', 'gsd-execute-phase', 'SKILL.md'); const fm = readFrontmatter(skillPath); - assert.match(fm, /^context:[ \t]*fork$/m, - `gsd-execute-phase SKILL.md must have context: fork\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, + `gsd-execute-phase is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`); }); test('gsd-execute-phase SKILL.md has effort: xhigh after global install', () => { @@ -283,12 +297,12 @@ describe('#769 Claude global install: SKILL.md files preserve context: fork and `gsd-execute-phase SKILL.md must have effort: xhigh\nActual:\n${fm}`); }); - test('gsd-plan-phase SKILL.md has context: fork after global install', () => { + test('gsd-plan-phase SKILL.md does NOT have context: fork after global install (#921)', () => { runClaudeGlobalInstall(claudeHome); const skillPath = path.join(claudeHome, 'skills', 'gsd-plan-phase', 'SKILL.md'); const fm = readFrontmatter(skillPath); - assert.match(fm, /^context:[ \t]*fork$/m, - `gsd-plan-phase SKILL.md must have context: fork\nActual:\n${fm}`); + assert.doesNotMatch(fm, /^context:[ \t]*fork$/m, + `gsd-plan-phase is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`); }); test('gsd-plan-phase SKILL.md has effort: xhigh after global install', () => { diff --git a/tests/plan-phase-drift-guard.test.cjs b/tests/plan-phase-drift-guard.test.cjs index 9ef1e5bf3..3ae66f09d 100644 --- a/tests/plan-phase-drift-guard.test.cjs +++ b/tests/plan-phase-drift-guard.test.cjs @@ -193,3 +193,70 @@ describe('plan-phase workflow: top-level spawn guard (#913)', () => { ); }); }); + +// ─── (D) Attempt-based Agent gate (#922) ───────────────────────────────────── + +describe('plan-phase workflow: attempt-based Agent availability gate (#922)', () => { + // Extract the runtime_compatibility block for targeted assertions + const rtBlock = (() => { + const m = workflow.match(/([\s\S]*?)<\/runtime_compatibility>/); + return m ? m[1] : ''; + })(); + + // Extract the "Other runtimes" clause specifically + const otherRuntimesClause = (() => { + const m = rtBlock.match(/\*\*Other runtimes[^*]*\*\*[^\n]*\n([\s\S]*?)(?=\n\*\*|$)/); + return m ? m[0] : rtBlock; + })(); + + test('Other runtimes clause does not authorize stopping on a self-assessed absence (#922)', () => { + // The pre-#922 wording ("if the Agent tool is genuinely absent") let the model + // self-assess and stop without ever attempting a call. The fixed wording must + // not contain phrasing that authorizes that pattern. + const forbiddenPatterns = [ + /if the Agent tool is genuinely absent/i, + /if.*Agent.*genuinely absent/i, + ]; + for (const pattern of forbiddenPatterns) { + assert.ok( + !pattern.test(otherRuntimesClause), + `plan-phase "Other runtimes" clause must not authorize stopping on a self-assessed Agent absence — ` + + `use attempt-based gate instead (#922). Found: ${otherRuntimesClause.trim()}` + ); + } + }); + + test('Other runtimes clause pins "Always attempt the actual Agent() call" language (#922)', () => { + // Pin the exact contract phrase so a future edit that changes to "try to determine + // availability" or "check if Agent is available" does not silently reintroduce introspection. + assert.ok( + otherRuntimesClause.includes('Always attempt the actual') || + otherRuntimesClause.includes('always attempt the actual'), + `plan-phase "Other runtimes" clause must pin "Always attempt the actual Agent() call" (or equivalent) (#922). ` + + `Found: ${otherRuntimesClause.trim()}` + ); + }); + + test('Other runtimes clause pins "real tool-unavailable error" as the only valid stop signal (#922)', () => { + // Must tie the stop to a real returned error, not a self-assessed absence. + assert.ok( + otherRuntimesClause.includes('real tool-unavailable error') || + otherRuntimesClause.includes('tool-unavailable error returned'), + `plan-phase "Other runtimes" clause must state only a real tool-unavailable error from Agent() authorizes stopping (#922). ` + + `Found: ${otherRuntimesClause.trim()}` + ); + }); + + test('Other runtimes clause still prohibits inline role collapse (#922 preserves #913)', () => { + // Even after the attempt-based rewrite the clause must keep the no-inline-collapse guard. + const hasNoInline = + otherRuntimesClause.toLowerCase().includes('do not') && + (otherRuntimesClause.toLowerCase().includes('inline') || + otherRuntimesClause.toLowerCase().includes('collapse')); + assert.ok( + hasNoInline, + `plan-phase "Other runtimes" clause must still prohibit inline role collapse even with the attempt-based gate (#922). ` + + `Found: ${otherRuntimesClause.trim()}` + ); + }); +}); diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index 7713b2be7..c8b96bd9d 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -82,7 +82,7 @@ const GRACE = 3000; // 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=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. +// slack=475 ≤ GRACE. plan-phase.md=90748 (#922 attempt-based Agent gate), 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. @@ -96,7 +96,7 @@ const DEFAULT_BUDGET = 40000; // pattern that future shrinks should follow. Byte counts noted for reference. const XL_WORKFLOWS = new Set([ '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) + 'plan-phase', // 90748 bytes (grew in #922 attempt-based Agent gate) 'new-project', // 55850 bytes ]);