fix(#3533): effort inherit — expressible, omitted at writers, never re-added (#3541)

This commit is contained in:
Tom Boucher
2026-08-15 07:00:44 -04:00
committed by GitHub
parent 500df8e37f
commit 50d5368add
10 changed files with 284 additions and 8 deletions

View File

@@ -3726,6 +3726,85 @@ description: Executes GSD phase plans
Body of the agent.
`;
describe('#3533 effort sync: inherit means the key must not exist', () => {
test('10d: sync does not re-add a hand-stripped key when inherit is configured', () => {
const tmpDir = makeTmpDir('effort-sync-inherit-absent-');
const agentsDir = makeAgentsDir(tmpDir);
fs.writeFileSync(path.join(agentsDir, 'gsd-executor.md'), AGENT_WITHOUT_EFFORT);
// Tier standard -> inherit.
writePlanningConfig(tmpDir, { routing_tier_defaults: { light: 'high', standard: 'inherit', heavy: 'xhigh' } });
const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs');
const result = captureOutput(() =>
cmdEffortSync(tmpDir, false, { dryRun: false, configDir: tmpDir, runtime: 'claude' })
);
assert.equal(result.synced, 0, `absent key + inherit is IN SYNC, not drift: ${JSON.stringify(result.changes)}`);
assert.equal(result.changes.length, 0, 'no change may be reported for an absent key under inherit');
const after = fs.readFileSync(path.join(agentsDir, 'gsd-executor.md'), 'utf8');
assert.ok(!/^effort:/m.test(after), 'the effort: key must NOT be re-added');
cleanup(tmpDir);
});
test('10d: sync strips the key when inherit is configured and a value is present', () => {
const tmpDir = makeTmpDir('effort-sync-inherit-strip-');
const agentsDir = makeAgentsDir(tmpDir);
// Fixture carries its own name so the survivor assertion below is
// satisfiable (AGENT_WITH_EFFORT names gsd-planner — wrong file).
fs.writeFileSync(path.join(agentsDir, 'gsd-executor.md'), AGENT_WITH_EFFORT.replace('name: gsd-planner', 'name: gsd-executor'));
writePlanningConfig(tmpDir, { default: 'inherit' });
const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs');
const result = captureOutput(() =>
cmdEffortSync(tmpDir, false, { dryRun: false, configDir: tmpDir, runtime: 'claude' })
);
assert.equal(result.synced, 1);
assert.equal(result.changes[0].agent, 'gsd-executor');
assert.equal(result.changes[0].from, 'medium');
assert.equal(result.changes[0].to, null, 'to: null is the typed IR for omission');
const after = fs.readFileSync(path.join(agentsDir, 'gsd-executor.md'), 'utf8');
assert.ok(!/^effort:/m.test(after), 'the effort: line must be stripped');
assert.ok(after.includes('name: gsd-executor'), 'every other frontmatter line survives');
assert.ok(after.includes('Body of the agent.'), 'the body survives');
cleanup(tmpDir);
});
test('10d: strip preserves CRLF files and leaves comments and sibling keys intact', () => {
const tmpDir = makeTmpDir('effort-sync-inherit-crlf-');
const agentsDir = makeAgentsDir(tmpDir);
const crlfAgent = [
'---',
'name: gsd-executor',
'# a hand comment that must survive',
'effort: high',
'description: Executes GSD phase plans',
'---',
'Body.',
'',
].join('\r\n');
const agentPath = path.join(agentsDir, 'gsd-executor.md');
fs.writeFileSync(agentPath, crlfAgent);
writePlanningConfig(tmpDir, { agent_overrides: { 'gsd-executor': 'inherit' } });
const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs');
const result = captureOutput(() =>
cmdEffortSync(tmpDir, false, { dryRun: false, configDir: tmpDir, runtime: 'claude' })
);
assert.equal(result.synced, 1, `expected one strip: ${JSON.stringify(result.changes)}`);
const after = fs.readFileSync(agentPath, 'utf8');
assert.ok(!/^effort:/m.test(after), 'effort line gone');
assert.ok(after.includes('\r\n'), 'CRLF endings preserved');
assert.ok(after.includes('# a hand comment that must survive'), 'comment preserved');
assert.ok(/^description: Executes GSD phase plans\r?$/m.test(after), 'sibling key preserved');
cleanup(tmpDir);
});
});
describe('feat-488: effort sync command', () => {
test('dry-run mode reports pending changes without writing files', () => {
const tmpDir = makeTmpDir('effort-sync-dry-');

View File

@@ -409,6 +409,23 @@ describe('#2481 live path — resolve-execution carries invocation-time effort',
});
});
describe('#3533 inherit renders no host argv argument', () => {
test('a project configuring inherit resolves effort inherit and renders NO argv', (t2) => {
const dir = createTempProject();
t2.after(() => cleanup(dir));
fs.writeFileSync(
path.join(dir, '.planning', 'config.json'),
JSON.stringify({ effort: { agent_overrides: { 'gsd-planner': 'inherit' } } }, null, 2),
);
const out = JSON.parse(
runGsdTools('query resolve-execution gsd-planner --host claude', dir).output,
);
assert.equal(out.effort, 'inherit');
assert.deepEqual(out.effort_argv, [], 'inherit must render no argument');
assert.equal(out.effort_propagation, null);
});
});
describe('#2481 — the escalation surface renders argv (CLI-level, not a workflow claim)', () => {
// NAMING IS DELIBERATE. This exercises `resolve-execution --attempt` directly,
// which is the CLI surface ADR-443's blocker explicitly EXCLUDES when it asks

View File

@@ -4040,6 +4040,61 @@ describe('#443 resolveInstallTimeEffort: invalid tokens fall through to valid ef
});
});
// ─── describe 5d: #3533 (10d) — inherit omits the effort key at install ──────
describe('#3533 inherit: install writes NO effort key when the agent resolves to inherit', () => {
let tmpDir;
let claudeHome;
let codexHome;
beforeEach(() => {
tmpDir = makeTmpDir('gsd-3533-inherit-');
const projectDir = path.join(tmpDir, 'project');
claudeHome = path.join(projectDir, '.claude');
codexHome = path.join(projectDir, '.codex');
fs.mkdirSync(claudeHome, { recursive: true });
fs.mkdirSync(codexHome, { recursive: true });
fs.mkdirSync(path.join(projectDir, '.planning'), { recursive: true });
});
afterEach(() => {
cleanup(tmpDir);
});
function writeProjectConfig(config) {
const projectDir = path.dirname(claudeHome);
fs.writeFileSync(
path.join(projectDir, '.planning', 'config.json'),
JSON.stringify(config, null, 2)
);
}
test('claude .md frontmatter has no effort: key for an inherit-resolving agent', () => {
writeProjectConfig({ effort: { agent_overrides: { 'gsd-planner': 'inherit' } } });
runGlobalInstall('claude', claudeHome);
const fm = readFrontmatter(path.join(claudeHome, 'agents', 'gsd-planner.md'));
assert.doesNotMatch(fm, /^effort:/m,
`an inherit-resolving agent must carry NO effort key\nActual:\n${fm}`);
// A non-inherit agent still gets its concrete value.
const fmExecutor = readFrontmatter(path.join(claudeHome, 'agents', 'gsd-executor.md'));
const m = fmExecutor.match(/^effort:\s*(\S+)$/m);
assert.ok(m && m[1] === 'high', `gsd-executor keeps its concrete tier value, got: ${m && m[1]}`);
});
test('codex .toml omits model_reasoning_effort for an inherit-resolving pinned agent', () => {
writeProjectConfig({
runtime: 'codex',
model_overrides: { 'gsd-planner': 'gpt-5.6-sol' },
effort: { agent_overrides: { 'gsd-planner': 'inherit' } },
});
runGlobalInstall('codex', codexHome);
const toml = fs.readFileSync(path.join(codexHome, 'agents', 'gsd-planner.toml'), 'utf8');
assert.match(toml, /^model\s*=\s*"gpt-5.6-sol"$/m, 'the model pin stays');
assert.doesNotMatch(toml, /^model_reasoning_effort\s*=/m,
`inherit must not pin a literal effort level\nActual:\n${toml.slice(0, 400)}`);
});
});
// ─── describe 5: Source stays clean ──────────────────────────────────────────
describe('#443 Source purity: agents/gsd-planner.md has no effort: key', () => {

View File

@@ -290,13 +290,63 @@ describe('resolveEffortInternal', () => {
test('VALID_EFFORTS and EFFORT_SET are consistent', () => {
assert.ok(Array.isArray(VALID_EFFORTS));
assert.ok(EFFORT_SET instanceof Set);
assert.strictEqual(EFFORT_SET.size, VALID_EFFORTS.length);
// #3533 (10d): the VOCABULARY (EFFORT_SET) carries one more member than
// the LADDER (VALID_EFFORTS) — 'inherit' is a declarable effort choice
// but not a level nextEffort may step into.
assert.strictEqual(EFFORT_SET.size, VALID_EFFORTS.length + 1);
assert.ok(EFFORT_SET.has('inherit'), "EFFORT_SET must accept 'inherit'");
assert.ok(!VALID_EFFORTS.includes('inherit'), "the escalation ladder must NOT contain 'inherit'");
for (const e of VALID_EFFORTS) {
assert.ok(EFFORT_SET.has(e), `EFFORT_SET missing: ${e}`);
}
});
});
// ─── #3533 (10d): effort inheritance ──────────────────────────────────────────
describe('#3533 effort inherit: expressible at every layer, never a wire level', () => {
let tmpDir;
beforeEach(() => { tmpDir = makeTempProject(); });
afterEach(() => { cleanup(tmpDir); });
test('inherit accepted at every cascade layer (runtime)', () => {
writeConfig(tmpDir, { effort: { agent_overrides: { 'gsd-executor': 'inherit' } } });
assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-executor'), 'inherit');
writeConfig(tmpDir, { effort: { routing_tier_defaults: { heavy: 'inherit' } } });
assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'inherit');
writeConfig(tmpDir, { effort: { default: 'inherit' } });
assert.strictEqual(resolveEffortInternal(tmpDir, 'completely-unknown-agent-xyz'), 'inherit');
// A tiered agent under an inherit tier default also inherits (tier layer won).
assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-planner'), 'inherit');
assert.strictEqual(resolveEffortInternal(tmpDir, 'gsd-executor', { override: 'inherit' }), 'inherit');
});
test('explicit inherit does not escalate', () => {
writeConfig(tmpDir, {
effort: { routing_tier_defaults: { heavy: 'inherit' } },
dynamic_routing: { enabled: true, escalate_on_failure: true, max_escalations: 3 },
});
assert.strictEqual(resolveEffortForTier(tmpDir, 'gsd-planner', 2), 'inherit');
});
test('renderEffortForRuntime inherit never yields a wire level', () => {
const { renderEffortForRuntime } = require('../gsd-core/bin/lib/model-catalog.cjs');
for (const runtime of ['claude', 'codex', 'something-unknown']) {
const r = renderEffortForRuntime(runtime, 'inherit');
assert.strictEqual(r.value, 'inherit', `${runtime}: value`);
assert.strictEqual(r.param, null, `${runtime}: param`);
assert.strictEqual(r.channel, null, `${runtime}: channel`);
}
// Concrete levels unchanged.
assert.strictEqual(renderEffortForRuntime('claude', 'minimal').value, 'low');
assert.strictEqual(renderEffortForRuntime('codex', 'max').value, 'xhigh');
assert.strictEqual(renderEffortForRuntime('claude', 'xhigh').value, 'xhigh');
});
});
// ─── nextEffort ────────────────────────────────────────────────────────────────
describe('nextEffort', () => {