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/docs/explanation/context-engineering.md b/docs/explanation/context-engineering.md index 799a7b2a3..77869c33c 100644 --- a/docs/explanation/context-engineering.md +++ b/docs/explanation/context-engineering.md @@ -90,13 +90,11 @@ Claude Code exposes a `FileChanged` event in addition to session-lifecycle hooks Requiring a `/clear` to pick up a config edit would destroy the very continuity the context-engineering design is trying to protect. By watching for `FileChanged` on `config.json`, GSD can reload configuration mid-session — adjusting model profiles, context-window thresholds, or routing preferences — without the user losing their place. The working context survives; the configuration updates beneath it. -### Forked context for heavy skills +### Effort signals for heavy and light skills -Beyond passive monitoring, GSD uses an active strategy for skills whose work is large and bursty: they run in a **forked context** (`context: fork` in the skill definition). +Beyond passive monitoring, GSD uses `effort:` frontmatter to signal the token budget appropriate for each skill. Heavy orchestrator skills (`plan-phase`, `execute-phase`, `autonomous`) declare `effort: xhigh`; quick-status skills (`progress`, `stats`) declare `effort: low`. -Skills like `plan-phase`, `execute-phase`, and `autonomous` do a great deal of work — spawning multiple subagents, reading large files, iterating over multiple plans. If that work happened in the main session, it would consume a substantial share of the orchestrator's context budget. The forked context prevents this: the skill runs in an isolated context of its own, does its heavy lifting there, and the main session's headroom is preserved. - -This is the same context-engineering principle as the fresh-context subagent model — applied not at the session boundary, but at the skill-invocation boundary. The difference is one of granularity. The phase loop spawns fresh subagents to protect each agent from its siblings' noise. Forked context protects the orchestrating session from the skill's own accumulated noise while the skill runs. +Note: an earlier version of GSD also applied `context: fork` to these three heavy skills to protect the main session's context budget. This was removed (#921) because `plan-phase`, `execute-phase`, and `autonomous` are **spawning orchestrators** — their core function is to spawn subagents (`gsd-planner`, `gsd-executor`, etc.), and a forked subagent context does not have the `Agent` tool. Context isolation for these skills comes from the subagents they spawn, not from forking the orchestrator itself. Complementing this, quick-status skills explicitly declare low effort in their definitions. This is a budget-conscious signal in the opposite direction: these skills read minimal state and return concise output, keeping their own footprint small by design. @@ -108,7 +106,7 @@ This machinery is worth being honest about. **Headroom tracking is a heuristic.** The hooks give GSD a signal, not a guarantee. A single model call can consume tokens unpredictably depending on the response length, tool use, and caching behaviour. GSD uses headroom estimates to warn and steer, not to make hard guarantees about what will fit. -**Forked context is isolated.** The forked work cannot see uncommitted state in the main session. This is not a bug — it is necessary for isolation — but it means anything the forked skill needs to know must be on disk before the fork occurs. This is precisely why `.planning/` exists as the shared substrate: plan files, `STATE.md`, `CONTEXT.md`, and `config.json` are all durable, file-system artefacts that any context — main or forked — can read. The context-engineering design is self-consistent: the same principle that makes fresh-context subagents work (shared state lives in files, not in a conversation) is what makes forked context viable. See also [Multi-agent orchestration](multi-agent-orchestration.md) for how `.planning/` serves the same role across the orchestrator → agent boundary. +**Subagents are isolated.** A spawned subagent cannot see uncommitted state in the orchestrating session. This is not a bug — it is necessary for independence — but it means anything the subagent needs must be on disk before it is spawned. This is precisely why `.planning/` exists as the shared substrate: plan files, `STATE.md`, `CONTEXT.md`, and `config.json` are all durable, file-system artefacts that any context — orchestrator or subagent — can read. The context-engineering design is self-consistent: the same principle that makes fresh-context subagents work (shared state lives in files, not in a conversation) is what makes the multi-agent architecture viable. See also [Multi-agent orchestration](multi-agent-orchestration.md) for how `.planning/` serves the same role across the orchestrator → agent boundary. --- diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index caf56c32a..ec0001033 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -46,10 +46,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 422128ec7..a7a658592 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 */ @@ -91,11 +99,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', () => { @@ -104,10 +116,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', () => { @@ -116,10 +128,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', () => { @@ -238,7 +250,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; @@ -252,12 +266,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 = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'autonomous'); 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', () => { @@ -268,12 +282,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 = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'execute-phase'); 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', () => { @@ -284,12 +298,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 = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'plan-phase'); 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 ]);