fix(#3659): make the worktree base-check trust evidence, not baseRef (#3736)

* test(#3659): baseref-head suppress must be mode-aware regression rows

* fix(#3659): make baseref-head suppress mode-aware and thread isolation mode

* fix(#3659): review fixes - stale advice purge, message pins, mode alias

* fix(#3659): pick-interceptable emit seam, ack merge, writeSync pin

* test(#3659): rewrite set-baseref pin, fix writeSync row stub

* chore(#3659): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-21 04:58:33 -04:00
committed by GitHub
parent 1f76861202
commit 2b42b28687
18 changed files with 339 additions and 76 deletions

View File

@@ -2,7 +2,7 @@
"version": 1,
"paths": {
"execute-phase.md": {
"reason": "#2856 \u2014 step 5.75 now dispatches every `kind == \"step\"` at execute:wave:post. That point previously dispatched only contribution and gate, so any registered step was declared and silently never run; the capability validator refuses to generate a registry containing such a step. Also adds the validate-ref.command-before-shell warning matching the sibling gate-dispatch line. Supersedes the spent #3370 fragment entry (merged into next, so its ripple is already absorbed at the base and it can no longer clear anything), which also named execute-phase.md and would otherwise double-ack the same path \u2014 the same supersede that entry itself performed on the spent #3324 fragment."
"reason": "#2856 \u2014 step 5.75 now dispatches every `kind == \"step\"` at execute:wave:post. That point previously dispatched only contribution and gate, so any registered step was declared and silently never run; the capability validator refuses to generate a registry containing such a step. Also adds the validate-ref.command-before-shell warning matching the sibling gate-dispatch line. Supersedes the spent #3370 fragment entry (merged into next, so its ripple is already absorbed at the base and it can no longer clear anything), which also named execute-phase.md and would otherwise double-ack the same path \u2014 the same supersede that entry itself performed on the spent #3324 fragment. #3659 corrects the auto-degrade paragraph's restore-advice to the orchestrator/harness split (+~300B): worktree.baseRef:\"head\" restores parallel execution only where GSD itself creates worktrees; harness-isolated runtimes do not read the setting (#48, upstream claude-code#44965). Deliberate growth."
}
}
}

View File

@@ -1,6 +1,6 @@
{
"version": 1,
"paths": {
"execute-plan.md": "#3370: the Pattern A dispatch prompt spec gained the gate-semantics clause (gate=\"blocking\" (the default) is auto-approvable in auto-mode per the executor's own checkpoint protocol, gate=\"blocking-human\" always surfaces to a human; add no instruction overriding that protocol), closing the identically-shaped dispatch-time gap on the single-plan path named in the issue. Growth ~248 bytes (38913 -> 39161, still under the DEFAULT 40 KiB ceiling). Supersedes the spent #2652 fragment (merged into next), which also named execute-plan.md and would otherwise double-ack the same path."
"execute-plan.md": "#3370: the Pattern A dispatch prompt spec gained the gate-semantics clause (gate=\"blocking\" (the default) is auto-approvable in auto-mode per the executor's own checkpoint protocol, gate=\"blocking-human\" always surfaces to a human; add no instruction overriding that protocol), closing the identically-shaped dispatch-time gap on the single-plan path named in the issue. Growth ~248 bytes (38913 -> 39161, still under the DEFAULT 40 KiB ceiling). Supersedes the spent #2652 fragment (merged into next), which also named execute-plan.md and would otherwise double-ack the same path. #3659 appends --mode \"$ISOLATION\" to the #2649 pre-dispatch base-check and corrects the baseRef restore-advice to the orchestrator/harness split (+154B; the harness does not read baseRef, #48). Deliberate growth."
}
}

View File

@@ -2,16 +2,16 @@
"version": 1,
"paths": {
"audit-fix.md": {
"reason": "#3602: the issue's file-level grep audit missed this file — its gsd-executor spawn is dispatch-shaped. Gains an EXECUTOR_MODEL resolve-model binding, the #2517 marker + rule block, and model= on the executor Agent block. Deliberate growth, not converter drift."
"reason": "#3602: the issue's file-level grep audit missed this file \u2014 its gsd-executor spawn is dispatch-shaped. Gains an EXECUTOR_MODEL resolve-model binding, the #2517 marker + rule block, and model= on the executor Agent block. Deliberate growth, not converter drift."
},
"diagnose-issues.md": {
"reason": "#3602: same class — gsd-debugger spawned with no model resolution. Gains a DEBUGGER_MODEL resolve-model assignment in the pre-spawn block, the #2517 marker + rule block, and model=\"{DEBUGGER_MODEL}\" on the debugger Agent block. Deliberate growth, not converter drift."
"reason": "#3602: same class \u2014 gsd-debugger spawned with no model resolution. Gains a DEBUGGER_MODEL resolve-model assignment in the pre-spawn block, the #2517 marker + rule block, and model=\"{DEBUGGER_MODEL}\" on the debugger Agent block. Deliberate growth, not converter drift. #3659 appends --mode \"$ISOLATION\" to both #2649 base-check calls (+40B): the isolation mode now gates the baseref-head suppress (harness ignores the setting, #48). Deliberate growth."
},
"profile-user.md": {
"reason": "#3602: same class via a Task-tool prose spawn the issue's Agent()-shaped audit could not see. Gains a PROFILER_MODEL resolve-model binding, the #2517 marker + rule block, and a model=\"{PROFILER_MODEL}\" pass/omit instruction on the Task-tool spawn. Deliberate growth, not converter drift."
},
"docs-update.md": {
"reason": "#3602 (isolated adversarial review finding): the --verify-only step's gsd-doc-verifier spawn had no model resolution — doc_writer_model covers only the writer spawns, and docs-init emits no verifier field. Gains a DOC_VERIFIER_MODEL resolve-model assignment in the init block, a model=\"{DOC_VERIFIER_MODEL}\" pass/omit instruction at the verifier spawn, and the #2517 blockquote's variable list extended. Deliberate growth, not converter drift."
"reason": "#3602 (isolated adversarial review finding): the --verify-only step's gsd-doc-verifier spawn had no model resolution \u2014 doc_writer_model covers only the writer spawns, and docs-init emits no verifier field. Gains a DOC_VERIFIER_MODEL resolve-model assignment in the init block, a model=\"{DOC_VERIFIER_MODEL}\" pass/omit instruction at the verifier spawn, and the #2517 blockquote's variable list extended. Deliberate growth, not converter drift."
}
}
}

View File

@@ -2,7 +2,7 @@
"version": 1,
"paths": {
"quick.md": {
"reason": "#3606: the execute:post consumer gains generic step dispatch (deferral to loop-hook-dispatch.md) before the code-review specialization, so refactor-trigger's ref.command step and any other registered execute:post steps actually run on /gsd:quick runs instead of being filtered out by the one-skill narrow. Deliberate growth, not converter drift."
"reason": "#3606: the execute:post consumer gains generic step dispatch (deferral to loop-hook-dispatch.md) before the code-review specialization, so refactor-trigger's ref.command step and any other registered execute:post steps actually run on /gsd:quick runs instead of being filtered out by the one-skill narrow. Deliberate growth, not converter drift. #3659 appends --mode \"$ISOLATION\" to both #1941 base-check calls (+40B): the isolation mode now gates the baseref-head suppress \u2014 the runtime harness does not read project-settings baseRef (#48), so harness mode must compare instead of suppressing. Deliberate growth."
}
}
}

View File

@@ -784,9 +784,17 @@ describe('execute-phase: between-wave manifest reset (#1369, #3384)', () => {
assert.ok(content.includes('#3384'), 'step 7c must reference #3384');
});
test('step 7c calls worktree.set-baseref to re-assert head config', () => {
test('step 7c runs the mode-threaded base-check and does NOT re-assert set-baseref (#3659)', () => {
// The former pin required the set-baseref re-assert, whose stated mechanism
// (#1369: "so the Claude Code harness re-reads the live HEAD") was fiction —
// the harness does not read project-settings baseRef (#48, verified 5/5;
// upstream claude-code#44965). Rewritten per the #3659 sanction: the
// between-wave re-check threads --mode and never re-asserts the dead call.
const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8');
assert.ok(content.includes('worktree.set-baseref'), 'step 7c must call worktree.set-baseref');
assert.ok(content.includes('worktree.base-check --mode "$ISOLATION"'),
'step 7c must thread the isolation mode through the base-check');
assert.ok(!content.includes('worktree.set-baseref'),
'step 7c must not re-assert set-baseref — the harness never read it (#48/#3659)');
});
test('step 7c appears after step 7b and before step 8 in the wave loop', () => {

View File

@@ -267,11 +267,22 @@ describe('evaluateWorktreeBaseDegrade', () => {
};
}
test('effectiveBaseRef="head" → no degrade, reason baseref-head, execGit never called', () => {
// #3659 rows share the diverged-HEAD stub shape — one builder keeps the
// four fixtures from drifting apart.
function makeDivergedExecGit(headSha, forkSha) {
const ok = (stdout) => ({ exitCode: 0, stdout, stderr: '', signal: null, error: null });
return makeExecGit({
'rev-parse HEAD': ok(headSha),
'rev-parse --verify --quiet origin/HEAD': ok(forkSha),
});
}
test('effectiveBaseRef="head" + orchestrator mode → no degrade, reason baseref-head, execGit never called (#3659)', () => {
let called = false;
const result = evaluateWorktreeBaseDegrade({
execGit: () => { called = true; return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null }; },
effectiveBaseRef: 'head',
isolationMode: 'orchestrator-worktree',
});
assert.strictEqual(result.shouldDegrade, false);
assert.strictEqual(result.reason, 'baseref-head');
@@ -279,7 +290,64 @@ describe('evaluateWorktreeBaseDegrade', () => {
assert.strictEqual(result.headSha, null);
assert.strictEqual(result.forkRef, null);
assert.strictEqual(result.forkSha, null);
assert.strictEqual(called, false, 'execGit must not be called when effectiveBaseRef is head');
assert.strictEqual(called, false, 'orchestrator mode: GSD controls the fork start-point, head is honored by construction');
});
test('effectiveBaseRef="head" + harness mode (default) + diverged HEAD → degrade, reason baseref-head-ignored-by-harness (#3659)', () => {
// #48 verified 5/5 that the harness dispatch path never routes through
// project-settings baseRef — with head set on a diverged branch the check
// must compare and degrade, not trust the setting.
const HEAD_SHA = '11111111223344aa11111111223344aa11111111';
const FORK_SHA = '99999999223344bb99999999223344bb99999999';
const result = evaluateWorktreeBaseDegrade({
execGit: makeDivergedExecGit(HEAD_SHA, FORK_SHA),
effectiveBaseRef: 'head',
});
assert.strictEqual(result.shouldDegrade, true,
'head must not suppress the comparison in harness mode (#3659)');
assert.strictEqual(result.reason, 'baseref-head-ignored-by-harness');
assert.strictEqual(result.headSha, HEAD_SHA);
assert.strictEqual(result.forkRef, 'origin/HEAD');
assert.strictEqual(result.forkSha, FORK_SHA);
});
test('effectiveBaseRef="head" + explicit harness-worktree mode + diverged → degrade (#3659)', () => {
const HEAD_SHA = 'aaaa1111223344ccaaaa1111223344ccaaaa1111';
const FORK_SHA = 'bbbb1111223344ddbbbb1111223344ddbbbb1111';
const result = evaluateWorktreeBaseDegrade({
execGit: makeDivergedExecGit(HEAD_SHA, FORK_SHA),
effectiveBaseRef: 'head',
isolationMode: 'harness-worktree',
});
assert.strictEqual(result.shouldDegrade, true);
assert.strictEqual(result.reason, 'baseref-head-ignored-by-harness');
});
test('effectiveBaseRef="head" + harness + HEAD == origin/HEAD → no degrade, reason head-matches-fork (#3659)', () => {
const SAME_SHA = 'cccc1111223344eecccc1111223344eecccc1111';
const result = evaluateWorktreeBaseDegrade({
execGit: makeExecGit({
'rev-parse HEAD': { exitCode: 0, stdout: SAME_SHA, stderr: '', signal: null, error: null },
'rev-parse --verify --quiet origin/HEAD': { exitCode: 0, stdout: SAME_SHA, stderr: '', signal: null, error: null },
}),
effectiveBaseRef: 'head',
isolationMode: 'harness-worktree',
});
assert.strictEqual(result.shouldDegrade, false,
'when the harness fork base happens to equal HEAD there is no mismatch to degrade for');
assert.strictEqual(result.reason, 'head-matches-fork');
});
test('harness head-diverge message cites the verified harness limitation (#3659)', () => {
const HEAD_SHA = 'dddd1111223344ffdddd1111223344ffdddd1111';
const FORK_SHA = 'eeee1111223344abeeceeee1111223344abeeceee';
const result = evaluateWorktreeBaseDegrade({
execGit: makeDivergedExecGit(HEAD_SHA, FORK_SHA),
effectiveBaseRef: 'head',
});
assert.ok(result.message !== null, 'divergence under head must carry the explanatory message');
assert.ok(result.message.includes('#48'), 'message must cite the verified harness limitation');
assert.ok(result.message.includes('sequentially'), 'message must state the sequential fallback');
});
test('git rev-parse HEAD fails → no degrade, reason no-head', () => {
@@ -414,7 +482,7 @@ describe('evaluateWorktreeBaseDegrade', () => {
});
test('headAbsenceVerified is null (not applicable) for a reason other than no-head', () => {
const result = evaluateWorktreeBaseDegrade({ effectiveBaseRef: 'head' });
const result = evaluateWorktreeBaseDegrade({ effectiveBaseRef: 'head', isolationMode: 'orchestrator-worktree' });
assert.strictEqual(result.reason, 'baseref-head');
assert.strictEqual(result.headAbsenceVerified, null);
});
@@ -449,8 +517,10 @@ describe('evaluateWorktreeBaseDegrade', () => {
assert.strictEqual(result.headSha, HEAD_SHA);
assert.strictEqual(result.forkRef, 'origin/HEAD');
assert.strictEqual(result.forkSha, FORK_SHA);
// Verify message contains the short SHAs and the issue reference
const expectedMsg = `⚠ Worktree base mismatch: HEAD (${HEAD_SHA.slice(0, 8)}) differs from origin/HEAD (${FORK_SHA.slice(0, 8)}). Running this phase sequentially on the main working tree. To keep parallel worktrees, set worktree.baseRef:"head" in .claude/settings.local.json (or run: gsd-tools worktree set-baseref). See #683.`;
// Verify message contains the short SHAs and the corrected remediation
// (#3659: the old text advised setting baseRef:"head", which the harness
// does not read).
const expectedMsg = `⚠ Worktree base mismatch: HEAD (${HEAD_SHA.slice(0, 8)}) differs from origin/HEAD (${FORK_SHA.slice(0, 8)}). Running this phase sequentially on the main working tree. Parallel worktrees return once HEAD is merged/pushed so origin/HEAD matches it. (worktree.baseRef:"head" applies only where GSD itself creates the worktree — the runtime harness does not read it; #48, #3659.)`;
assert.strictEqual(result.message, expectedMsg);
});
@@ -487,7 +557,7 @@ describe('evaluateWorktreeBaseDegrade', () => {
assert.strictEqual(result.reason, 'fork-ref-unknown');
assert.strictEqual(result.forkRef, null);
assert.strictEqual(result.forkSha, null);
const expectedMsg = `⚠ Cannot determine the worktree fork base (origin/HEAD unresolved). Running this phase sequentially on the main working tree to avoid a base mismatch. To keep parallel worktrees, set worktree.baseRef:"head" in .claude/settings.local.json (or run: gsd-tools worktree set-baseref). See #683.`;
const expectedMsg = `⚠ Cannot determine the worktree fork base (origin/HEAD unresolved). Running this phase sequentially on the main working tree to avoid a base mismatch. Parallel worktrees return once origin/HEAD resolves and matches HEAD. See #683, #3659.`;
assert.strictEqual(result.message, expectedMsg);
});
@@ -541,7 +611,7 @@ describe('cmdWorktreeBaseCheck', () => {
};
}
test('baseRef=head in settings → shouldDegrade false, reason baseref-head; write emits valid JSON', () => {
test('baseRef=head in settings + --mode orchestrator-worktree → shouldDegrade false, reason baseref-head; write emits valid JSON (#3659)', () => {
const cwd = '/repo';
const claudeDir = '/repo/.claude';
let written = '';
@@ -555,13 +625,84 @@ describe('cmdWorktreeBaseCheck', () => {
// Hermetic: point userClaudeDir at a non-existent path so real ~/.claude is never read
userClaudeDir: '/nonexistent-hermetic-user-dir',
};
const result = cmdWorktreeBaseCheck(cwd, [], deps);
const result = cmdWorktreeBaseCheck(cwd, ['--mode', 'orchestrator-worktree'], deps);
assert.strictEqual(result.shouldDegrade, false);
assert.strictEqual(result.reason, 'baseref-head');
const parsed = JSON.parse(written);
assert.deepStrictEqual(parsed, result);
});
test('baseRef=head in settings + default (harness) mode + diverged HEAD → shouldDegrade true (#3659)', () => {
const cwd = '/repo';
const claudeDir = '/repo/.claude';
const HEAD_SHA = 'fade1111223344cafade1111223344cafade1111';
const FORK_SHA = 'bead1111223344dbbead1111223344dbbead1111';
const deps = {
readFile: (p) => {
if (p === path.join(claudeDir, 'settings.local.json')) return JSON.stringify({ worktree: { baseRef: 'head' } });
return null;
},
execGit: makeExecGitCheck({
'rev-parse HEAD': { exitCode: 0, stdout: HEAD_SHA, stderr: '', signal: null, error: null },
'rev-parse --verify --quiet origin/HEAD': { exitCode: 0, stdout: FORK_SHA, stderr: '', signal: null, error: null },
}),
write: () => {},
userClaudeDir: '/nonexistent-hermetic-user-dir',
};
const result = cmdWorktreeBaseCheck(cwd, [], deps);
assert.strictEqual(result.shouldDegrade, true,
'settings head must not suppress the harness-mode comparison (#3659)');
assert.strictEqual(result.reason, 'baseref-head-ignored-by-harness');
});
test('--mode rejects invalid values — no silent default that would re-open the #3659 hole', () => {
const cwd = '/repo';
const deps = {
readFile: () => null,
execGit: makeExecGitCheck({}),
write: () => {},
userClaudeDir: '/nonexistent-hermetic-user-dir',
};
assert.throws(
() => cmdWorktreeBaseCheck(cwd, ['--mode', 'bogus-mode'], deps),
/--mode must be harness-worktree or orchestrator-worktree/
);
assert.throws(
() => cmdWorktreeBaseCheck(cwd, ['--mode'], deps),
/--mode must be harness-worktree or orchestrator-worktree/
);
});
test('default emit goes through fs.writeSync(1, …) — the seam --pick intercepts (#3659)', (t) => {
// The CLI's --pick capture patches fs.writeSync, not process.stdout.write;
// under $(…) command substitution the stdout.write default made --pick
// emit the full JSON, so the workflow auto-degrade guards never matched.
const fds = [];
const chunks = [];
const original = fs.writeSync;
fs.writeSync = (fd, buf, offset, length) => {
fds.push(fd);
const start = offset ?? 0;
const end = length ?? (typeof buf === 'string' ? buf.length : buf.length);
chunks.push(typeof buf === 'string' ? buf.slice(start, end) : buf.toString('utf8', start, end));
return end - start;
};
t.after(() => { fs.writeSync = original; });
const result = cmdWorktreeBaseCheck('/repo', [], {
readFile: () => null,
execGit: makeExecGitCheck({
'rev-parse HEAD': { exitCode: 128, stdout: '', stderr: 'fatal: not a git repository', signal: null, error: null },
}),
userClaudeDir: '/nonexistent-hermetic-user-dir',
// no deps.write — the default seam under test
});
assert.ok(fds.length > 0, 'default emit must call fs.writeSync');
assert.ok(fds.every((fd) => fd === 1), 'every write must target fd 1 (stdout)');
const emitted = chunks.join('');
assert.ok(emitted.includes('"reason"'), 'emitted payload is the JSON result');
assert.strictEqual(result.reason, 'no-head');
});
test('diverged shas → shouldDegrade true; captured JSON parses correctly', () => {
const cwd = '/repo';
const HEAD_SHA = 'deadbeef11223344deadbeef11223344deadbeef';
@@ -1000,9 +1141,11 @@ describe('cmdWorktreeBaseCheck — user/global cascade (#1013)', () => {
const cwd = '/repo';
const claudeDir = '/repo/.claude';
test('(e positive) user/global head + phase lane → shouldDegrade:false (KEY REGRESSION)', () => {
// This is the exact bug: user set worktree.baseRef:"head" in their global settings,
// but without the fix that setting was invisible and the phase lane triggered degrade.
test('(e positive) user/global head + phase lane + orchestrator mode → shouldDegrade:false (KEY REGRESSION #1013)', () => {
// This is the exact bug #1013 fixed: user set worktree.baseRef:"head" in their global
// settings, but the setting was invisible and the phase lane triggered degrade. The
// suppress now applies only where GSD manages the fork (--mode orchestrator-worktree),
// which is also what keeps this cascade proof meaningful post-#3659.
const deps = {
execGit: makePhaseLaneExecGit(HEAD_SHA),
readFile: (p) => {
@@ -1018,12 +1161,34 @@ describe('cmdWorktreeBaseCheck — user/global cascade (#1013)', () => {
write: () => {},
userClaudeDir: USER_CLAUDE_DIR,
};
const result = cmdWorktreeBaseCheck(cwd, [], deps);
const result = cmdWorktreeBaseCheck(cwd, ['--mode', 'orchestrator-worktree'], deps);
assert.strictEqual(result.shouldDegrade, false,
'user/global worktree.baseRef:"head" must suppress degrade on a phase lane');
'user/global worktree.baseRef:"head" must suppress degrade on a phase lane where GSD manages the fork');
assert.strictEqual(result.reason, 'baseref-head');
});
test('user/global head + phase lane + default (harness) mode → shouldDegrade:true (#3659)', () => {
// The mirror of the KEY REGRESSION row: in harness mode the setting cannot
// suppress anything (#48), so the same lane degrades.
const deps = {
execGit: makePhaseLaneExecGit(HEAD_SHA),
readFile: (p) => {
if (p === path.join(claudeDir, 'settings.local.json')) return null;
if (p === path.join(claudeDir, 'settings.json')) return null;
if (p === path.join(USER_CLAUDE_DIR, 'settings.json')) {
return JSON.stringify({ worktree: { baseRef: 'head' } });
}
return null;
},
write: () => {},
userClaudeDir: USER_CLAUDE_DIR,
};
const result = cmdWorktreeBaseCheck(cwd, [], deps);
assert.strictEqual(result.shouldDegrade, true,
'harness mode: head must not suppress the lane degrade (#3659)');
assert.strictEqual(result.reason, 'fork-ref-unknown');
});
test('(e negative) NO user/global head + same phase lane → shouldDegrade:true (proves lane degrades)', () => {
// Without a user/global head, the phase lane must still degrade (proves the positive test is real)
const deps = {