golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(#48): verify-only worktree_branch_check + orchestrator cwd-drift guard (#590)

Browse Source 
Closes #48. Makes the canonical worktree_branch_check fragment verify-only/fail-closed (exit 42, no git reset self-recovery), adds an orchestrator fail-closed collection rule and a cwd-drift guard at execute_waves entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-01 20:10:59 -04:00
committed by GitHub
parent b88d6d6ef1
commit 0c1084ba88
9 changed files with 159 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
tests/bug-3384-secondary-defects.test.cjs
View File

@@ -22,11 +22,12 @@ describe('bug #3384: adjacent worktree data-loss guards', () => {
assert.match(skipSet, /'worktree'/);
});
test('diagnose-issues agents assert disposable worktree branch before reset --hard', () => {
test('diagnose-issues references canonical fragment; fragment is verify-only and fails closed (#48)', () => {
// diagnose-issues.md now references the canonical fragment rather than
// inlining the block. Verify (a) it references the fragment and (b) the
// fragment itself has the correct ordering: symbolic-ref/HEAD assertion and
// ^worktree-agent- allow-list appear BEFORE git reset --hard.
// ^worktree-agent- allow-list appear before any work, and (c) the fragment
// is verify-only — no destructive self-recovery.
const diagnoseSource = read('get-shit-done/workflows/diagnose-issues.md');
assert.ok(
diagnoseSource.includes('worktree-branch-check.md'),
@@ -36,11 +37,12 @@ describe('bug #3384: adjacent worktree data-loss guards', () => {
const fragmentSource = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf8');
const branchCheck = fragmentSource.indexOf('HEAD_REF=$(git symbolic-ref --quiet HEAD || echo');
const namespaceCheck = fragmentSource.indexOf('^worktree-agent-');
const reset = fragmentSource.indexOf('git reset --hard {EXPECTED_BASE}');
assert.ok(branchCheck > 0, 'canonical fragment must assert HEAD before repair');
assert.ok(namespaceCheck > branchCheck, 'canonical fragment must require disposable worktree-agent branch before reset');
assert.ok(reset > namespaceCheck, 'reset --hard must come only after branch namespace check in canonical fragment');
assert.ok(branchCheck > 0, 'canonical fragment must assert HEAD before any work');
assert.ok(namespaceCheck > branchCheck, 'canonical fragment must require disposable worktree-agent branch');
// #48: verify-only — the destructive self-recovery is gone; the fragment fails closed instead.
assert.ok(!fragmentSource.includes('git reset --hard {EXPECTED_BASE}'), 'canonical fragment must not self-recover via reset --hard — orchestrator owns recovery (#48)');
assert.ok(fragmentSource.includes('exit 42'), 'canonical fragment must fail closed with exit 42 on base mismatch (#48)');
});
test('remove-workspace fails closed when git worktree remove fails', () => {

83
tests/worktree-cleanup.test.cjs
View File

@@ -146,21 +146,14 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => {
);
});
test('HEAD-attachment assertion runs BEFORE `git reset --hard`', () => {
test('block is verify-only: HEAD assertion present, no git reset, fails closed (#48)', () => {
const codeBlocks = extractFencedCodeBlocks(block);
const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body));
const symbolicRefIdx = findCommandIndex(allStatements, (cmd) =>
cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD')
);
const resetHardIdx = findCommandIndex(allStatements, (cmd) =>
cmd[0] === 'git' && cmd[1] === 'reset' && cmd.includes('--hard')
);
assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref check must exist');
assert.notStrictEqual(resetHardIdx, -1, 'reset --hard must exist');
assert.ok(
symbolicRefIdx < resetHardIdx,
'HEAD attachment assertion (symbolic-ref) must precede `git reset --hard` so a stale HEAD never moves a protected branch'
);
const symbolicRefIdx = findCommandIndex(allStatements, (cmd) => cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD'));
const resetIdx = findCommandIndex(allStatements, (cmd) => cmd[0] === 'git' && cmd[1] === 'reset');
assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref HEAD-attachment check must exist');
assert.strictEqual(resetIdx, -1, 'fragment must be verify-only — no git reset self-recovery (#48)');
assert.ok(/exit 42/.test(block), 'fragment must fail closed with exit 42 (#48)');
});
test('block names protected branches that must NOT be the agent branch', () => {
@@ -306,23 +299,19 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => {
);
});
test('HEAD assertion precedes `git reset --hard`', () => {
// Use shell-statement ordering on the fenced code block to avoid false matches
// from the preamble text that mentions `git reset --hard` for context.
test('block is verify-only: HEAD assertion present, no git reset, fails closed (#48)', () => {
// Verify-only contract: symbolic-ref exists, no git reset at all, fails closed with exit 42.
const codeBlocks = extractFencedCodeBlocks(block);
const allStatements = codeBlocks.flatMap(({ body }) => shellStatements(body));
const symbolicRefIdx = findCommandIndex(allStatements, (cmd) =>
cmd[0] === 'git' && cmd[1] === 'symbolic-ref' && cmd.includes('HEAD')
);
const resetHardIdx = findCommandIndex(allStatements, (cmd) =>
cmd[0] === 'git' && cmd[1] === 'reset' && cmd.includes('--hard')
);
assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref check must exist');
assert.notStrictEqual(resetHardIdx, -1, 'reset --hard must exist');
assert.ok(
symbolicRefIdx < resetHardIdx,
'symbolic-ref HEAD assertion must appear before `git reset --hard` in quick.md worktree_branch_check'
const resetIdx = findCommandIndex(allStatements, (cmd) =>
cmd[0] === 'git' && cmd[1] === 'reset'
);
assert.notStrictEqual(symbolicRefIdx, -1, 'symbolic-ref HEAD-attachment check must exist');
assert.strictEqual(resetIdx, -1, 'fragment must be verify-only — no git reset self-recovery (#48)');
assert.ok(/exit 42/.test(block), 'fragment must fail closed with exit 42 (#48)');
});
test('block forbids `git update-ref` self-recovery', () => {
@@ -733,3 +722,49 @@ test('#3425: cleanup-tail snippet carries the same primary-worktree pin before r
assert.match(content, /FATAL: cannot cd to primary worktree \$PRIMARY_WT/);
assert.match(content, /# Cleanup-tail: remove residual agent worktrees after a cross-wave-dependency deviation\./);
});
describe('bug #48: orchestrator cwd-drift guard at execute_waves entry', () => {
const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8');
const stepStart = content.indexOf('<step name="execute_waves">');
const nextStep = content.indexOf('<step ', stepStart + 1);
const stepBody = content.slice(stepStart, nextStep === -1 ? undefined : nextStep);
test('execute_waves step exists', () => {
assert.notStrictEqual(stepStart, -1, 'execute-phase.md must contain a <step name="execute_waves"> step');
});
test('execute_waves contains a labelled cwd-drift guard (#48)', () => {
assert.ok(stepBody.includes('cwd-drift guard') && /#48/.test(stepBody), 'execute_waves entry must contain a cwd-drift guard tagged #48');
});
test('cwd-drift guard resolves the worktree root via git rev-parse --show-toplevel (#48)', () => {
const g = stepBody.indexOf('cwd-drift guard');
assert.notStrictEqual(g, -1);
const region = stepBody.slice(g, g + 1600);
assert.ok(/git rev-parse --show-toplevel/.test(region), 'cwd-drift guard must resolve the worktree ROOT via git rev-parse --show-toplevel (#48)');
});
test('cwd-drift guard discriminates agent worktrees by branch namespace and fails closed (#48)', () => {
const g = stepBody.indexOf('cwd-drift guard');
assert.notStrictEqual(g, -1);
const region = stepBody.slice(g, g + 1600);
assert.ok(/worktree-agent-/.test(region), 'guard must use the worktree-agent-* branch namespace as the drift discriminator (#48)');
assert.ok(/exit 1/.test(region), 'cwd-drift guard must fail closed with exit 1 on drift (#48)');
});
test('cwd-drift guard does NOT blanket-refuse .claude/worktrees/ paths (#48)', () => {
const g = stepBody.indexOf('cwd-drift guard');
assert.notStrictEqual(g, -1);
const region = stepBody.slice(g, g + 1600);
assert.ok(!region.includes('*.claude/worktrees/*') && !region.includes('.claude/worktrees/*)'), 'guard must not blanket-refuse .claude/worktrees/ paths — would break legitimate worktree invocations (#48)');
});
});
describe('bug #48: orchestrator fail-closed handling of verify-only halts', () => {
const content = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8');
const withoutDispatchNote = content.replace(/<worktree_branch_check>[\s\S]*?<\/worktree_branch_check>/g, '');
test('orchestrator documents a fail-closed rule for executor exit 42 / FATAL (#48)', () => {
assert.ok(/exit 42|FATAL/.test(withoutDispatchNote), 'execute-phase.md must reference executor exit 42 / FATAL outside the dispatch note (#48)');
assert.ok(/(blocked|do NOT merge|not merge)/i.test(withoutDispatchNote), 'execute-phase.md must document an orchestrator-side rule that an executor FATAL/exit 42 marks the plan blocked and is not merged (#48)');
});
});

54
tests/worktree.test.cjs
View File

@@ -139,8 +139,8 @@ describe('canonical worktree-branch-check fragment is the single source of truth
assert.ok(blockMatch, 'worktree-branch-check.md must contain a <worktree_branch_check> block');
});
test('fragment block contains reset --hard', () => {
assert.ok(block.includes('reset --hard'), 'fragment block must use reset --hard');
test('fragment block is verify-only (no git reset)', () => {
assert.ok(!/git\s+reset/.test(block), 'fragment block must NOT run git reset — orchestrator owns base recovery (#48)');
});
test('fragment block does NOT contain reset --soft', () => {
@@ -176,8 +176,20 @@ describe('canonical worktree-branch-check fragment is the single source of truth
assert.ok(block.includes('update-ref'), 'fragment block must reference update-ref prohibition');
});
test('fragment block contains merge-base HEAD {EXPECTED_BASE}', () => {
assert.ok(block.includes('merge-base HEAD {EXPECTED_BASE}'), 'fragment block must contain git merge-base HEAD {EXPECTED_BASE}');
test('fragment block asserts exact base and fails closed with exit 42 (#48)', () => {
assert.ok(block.includes('git rev-parse HEAD') && block.includes('{EXPECTED_BASE}'), 'fragment must assert HEAD equals {EXPECTED_BASE} exactly (#48)');
assert.ok(/exit 42/.test(block), 'fragment must fail closed with exit 42 on mismatch (#48)');
});
});
// ─── #48: execute-plan mandate is verify-only ───────────────────────────────
describe('bug #48: execute-plan.md worktree mandate is verify-only', () => {
test('execute-plan.md worktree mandate is verify-only (no reset --hard self-recovery) (#48)', () => {
const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8');
assert.ok(content.includes('worktree-branch-check.md'), 'execute-plan.md must reference the canonical fragment');
assert.ok(!/hard-reset/.test(content) && !/reset --hard/.test(content), 'execute-plan.md must not describe reset --hard self-recovery — verify-only per #48');
assert.ok(/exit 42/.test(content), 'execute-plan.md mandate must specify fail-closed exit 42 (#48)');
});
});
@@ -214,7 +226,7 @@ function makeTempUpstreamRepo(prefix) {
// ─── #2015: reset --hard not --soft ─────────────────────────────────────────
describe('worktree_branch_check must use reset --hard not reset --soft (#2015)', () => {
describe('verify-only: worktree_branch_check must NOT run git reset (#48, supersedes #2015)', () => {
test('execute-phase.md worktree_branch_check does not use reset --soft', () => {
const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8');
@@ -226,19 +238,19 @@ describe('worktree_branch_check must use reset --hard not reset --soft (#2015)',
const block = blockMatch[1];
assert.ok(
!block.includes('reset --soft'),
'worktree_branch_check must not use reset --soft (leaves working tree files unchanged). Use reset --hard instead.'
'worktree_branch_check must not use reset --soft (leaves working tree files unchanged).'
);
});
test('execute-phase.md worktree_branch_check uses reset --hard for base correction', () => {
test('verify-only: execute-phase.md worktree_branch_check must not run git reset at all (#48, supersedes #2015)', () => {
const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8');
const blockMatch = fragmentContent.match(/<worktree_branch_check>([\s\S]*?)<\/worktree_branch_check>/);
assert.ok(blockMatch, 'worktree-branch-check.md must contain a <worktree_branch_check> block');
const block = blockMatch[1];
assert.ok(
block.includes('reset --hard'),
'worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree to the expected base'
!/git\s+reset/.test(block),
'worktree_branch_check must NOT run git reset — orchestrator owns base recovery (#48, supersedes #2015)'
);
});
@@ -250,19 +262,19 @@ describe('worktree_branch_check must use reset --hard not reset --soft (#2015)',
const block = blockMatch[1];
assert.ok(
!block.includes('reset --soft'),
'quick.md worktree_branch_check must not use reset --soft. Use reset --hard instead.'
'quick.md worktree_branch_check must not use reset --soft.'
);
});
test('quick.md worktree_branch_check uses reset --hard for base correction', () => {
test('verify-only: quick.md worktree_branch_check must not run git reset at all (#48, supersedes #2015)', () => {
const fragmentContent = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf-8');
const blockMatch = fragmentContent.match(/<worktree_branch_check>([\s\S]*?)<\/worktree_branch_check>/);
assert.ok(blockMatch, 'worktree-branch-check.md must contain a <worktree_branch_check> block');
const block = blockMatch[1];
assert.ok(
block.includes('reset --hard'),
'quick.md worktree_branch_check must use reset --hard to correctly reset both HEAD and working tree'
!/git\s+reset/.test(block),
'quick.md worktree_branch_check must NOT run git reset — orchestrator owns base recovery (#48, supersedes #2015)'
);
});
@@ -340,7 +352,7 @@ describe('bug-2075: worktree deletion safeguards', () => {
});
describe('Failure Mode A: worktree_branch_check audit across all worktree-spawning workflows', () => {
test('execute-phase.md has worktree_branch_check block with --hard reset', () => {
test('execute-phase.md has worktree_branch_check block (verify-only, no git reset) (#48)', () => {
const executePhaseContent = fs.readFileSync(EXECUTE_PHASE_PATH, 'utf-8');
assert.ok(
executePhaseContent.includes('worktree-branch-check.md'),
@@ -356,8 +368,8 @@ describe('bug-2075: worktree deletion safeguards', () => {
const block = blockMatch[1];
assert.ok(
block.includes('reset --hard'),
'execute-phase.md worktree_branch_check must use git reset --hard (not --soft)'
!/git\s+reset/.test(block),
'execute-phase.md worktree_branch_check must NOT run git reset — verify-only per #48'
);
assert.ok(
!block.includes('reset --soft'),
@@ -365,7 +377,7 @@ describe('bug-2075: worktree deletion safeguards', () => {
);
});
test('quick.md has worktree_branch_check block with --hard reset', () => {
test('quick.md has worktree_branch_check block (verify-only, no git reset) (#48)', () => {
const quickContent = fs.readFileSync(QUICK_PATH, 'utf-8');
assert.ok(
quickContent.includes('worktree-branch-check.md'),
@@ -381,8 +393,8 @@ describe('bug-2075: worktree deletion safeguards', () => {
const block = blockMatch[1];
assert.ok(
block.includes('reset --hard'),
'quick.md worktree_branch_check must use git reset --hard (not --soft)'
!/git\s+reset/.test(block),
'quick.md worktree_branch_check must NOT run git reset — verify-only per #48'
);
assert.ok(
!block.includes('reset --soft'),
@@ -402,8 +414,8 @@ describe('bug-2075: worktree deletion safeguards', () => {
assert.ok(blockMatch, 'worktree-branch-check.md must contain a <worktree_branch_check> block');
const block = blockMatch[1];
assert.ok(
block.includes('reset --hard'),
'diagnose-issues.md worktree_branch_check must instruct agents to use git reset --hard'
!/git\s+reset/.test(block),
'diagnose-issues.md worktree_branch_check must NOT run git reset — verify-only per #48'
);
});
});