fix(#921,#922): orchestrators must not fork; plan-phase Agent gate is attempt-based (#926)
`context: fork` strips the `Agent` tool from a subagent's environment.
Spawning orchestrators (`/gsd-autonomous`, `/gsd-execute-phase`,
`/gsd-plan-phase`) depend on `Agent` to dispatch sub-agents; running
them forked silently disables the core capability they exist to provide
(#921). Remove `context: fork` from all three command frontmatter files.
`effort: xhigh` (introduced by #769) is preserved.
The `<runtime_compatibility>` Agent-availability guard added by #913 was
checking whether `Agent` was present *before* attempting the call. On
runtimes where the tool list is dynamically resolved this produced
false-negative aborts in sessions that have the tool (#922). Replace
the introspection-based pattern with an attempt-based gate: always
attempt the `Agent()` call; stop only if a real tool-unavailable error
is returned. This preserves #853's backgrounded-session close-off and
false negatives.
Tests updated: enh-769-context-fork-effort.install.test.cjs asserts the
three orchestrators lack `context: fork` and that the converter still
passes the field through for non-orchestrator commands; plan-phase-drift-
guard.test.cjs adds four assertions for the attempt-based gate language;
workflow-size-budget unchanged (budgets not exceeded).
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit b866b95296)
This commit is contained in:
Tom Boucher
committed by
github-actions[bot]
github-actions[bot]
parent
27af818312
commit
a6229ec123
@@ -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', () => {
|
||||
|
||||
@@ -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(/<runtime_compatibility>([\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()}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
]);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user