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

fix(#4758): rescue resolves a relative worktree_path against repoRoot, not process.cwd() (#4872)

Browse Source 
* test(#4758): failing-first — rescue must resolve a relative worktree_path against repoRoot

* fix(#4758): rescue resolves a relative worktree_path against repoRoot, not process.cwd()

* test(#4758): review fold-ins — t.after cleanup pattern, post-resolution reader contract comment

* chore(#4758): changeset fragment

* chore(#4758): backfill changeset PR number (4872)

* test(#4758): windows lanes key rescue fakes on resolved path identity, not verbatim strings

win32 path.resolve rewrites driveless-absolute POSIX-style fixture values to the
current drive, so the rescue's (correct) resolved-path handoff stopped matching
verbatim string keys: #3804/#245/#2556/B7/#2852 fakes silently skipped the
rescue and my seam test compared against a POSIX literal. Fakes now key on
path.resolve(repoRoot, …) identity — the same semantics the code and git -C
use — so every rescue test exercises the rescue on every platform.

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-19 09:41:51 -04:00
committed by GitHub
parent 63edc777e6
commit d36514b816
3 changed files with 270 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/happy-quails-romp.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4872
---
**Worktree cleanup-wave rescues SUMMARY artifacts from relative worktree paths** — a manifest entry carrying a relative `worktree_path` made the rescue walk the CLI's working directory instead of the repo root, finding nothing and blocking the entry `worktree_dirty` instead of rescuing; the rescue now resolves the path against the repo root, the same way every git consumer of the field already does. (#4758)

18
src/worktree-safety.cts
View File

@@ -926,7 +926,19 @@ function rescueSummaryArtifacts(
const mkdirSync = deps.mkdirSync || ((d: string, o?: { recursive?: boolean }) => fs.mkdirSync(d, o));
const copyFileSync = deps.copyFileSync || fs.copyFileSync;
const summaryPaths = findSummaryFiles(worktreePath);
// #4758: resolve the manifest's worktree_path against repoRoot once, at this
// boundary, so every reader of the field here — the fs walker below and the
// `git -C` calls — resolves it the way the caller's git consumers already do
// (`-C <path>` with `{ cwd: repoRoot }`). A relative value passed to the
// walker verbatim made `path.join` emit a relative directory whose reads
// resolved against process.cwd(), silently walking the wrong tree.
// With this resolution, the remaining deps-injectable readers (existsSync,
// readFileSync, mkdirSync, copyFileSync) only ever see absolute paths:
// `dest` is built from repoRoot, `absPath` from the walker's join off this
// resolved base.
const resolvedWorktreePath = path.resolve(repoRoot, worktreePath);
const summaryPaths = findSummaryFiles(resolvedWorktreePath);
const rescuedRelPaths = new Set<string>();
const failures: Array<{ relPath: string; error: string }> = [];
@@ -934,7 +946,7 @@ function rescueSummaryArtifacts(
// relPath is the path relative to the worktree root (e.g. ".planning/q1-SUMMARY.md")
// Normalize to forward slashes so the Set comparison against `git status --porcelain`
// output works on Windows too (git always emits forward slashes in porcelain output).
const relPath = posixNormalize(absPath.slice(worktreePath.length).replace(/^[/\\]/, ''));
const relPath = posixNormalize(absPath.slice(resolvedWorktreePath.length).replace(/^[/\\]/, ''));
// #706: skip rescue when the SUMMARY is already committed on the branch.
// Use `git cat-file -e HEAD:<relPath>` (not `ls-files --error-unmatch`) so
@@ -953,7 +965,7 @@ function rescueSummaryArtifacts(
// copy when the main tree already holds identical content (which is also what
// guards the #706 merge collision). A divergent dest is overwritten, but only
// uncommitted main-tree content could be lost (committed content is git-recoverable).
const catFileResult = execGit(['-C', worktreePath, 'cat-file', '-e', `HEAD:${relPath}`], { cwd: repoRoot });
const catFileResult = execGit(['-C', resolvedWorktreePath, 'cat-file', '-e', `HEAD:${relPath}`], { cwd: repoRoot });
if (catFileResult.exitCode === 0) {
// exit 0 → the SUMMARY is committed on HEAD; the merge will carry it, so skip rescue.
continue;

317
tests/worktree-safety.test.cjs
View File

@@ -62,6 +62,24 @@ const isWindows = process.platform === 'win32';
// ─── Shared stubs ─────────────────────────────────────────────────────────────
/**
* #4758 (windows conformance lanes): the SUMMARY rescue resolves the manifest's
* worktree_path against repoRoot before handing it to the fs walker and its own
* `git -C` calls. A driveless-absolute fixture value ('/repo/...') rewrites to
* '<cwd-drive>:\repo\...' under win32 path.resolve, so fakes must key on the
* RESOLVED path — a verbatim string key silently never matches there and the
* rescue stops being exercised on Windows. gitKeyFor mirrors git's own
* `-C <relative>` resolution (against the plan's repoRoot) so one fake answers
* both the caller's calls (verbatim values, resolved by git's cwd) and the
* rescue's calls (already-resolved values) through the same branch set.
*/
function gitKeyFor(repoRoot, args) {
if (args[0] === '-C') {
return `-C ${path.resolve(repoRoot, args[1])} ${args.slice(2).join(' ')}`;
}
return args.join(' ');
}
/**
* Returns an execGit stub that simulates what spawnSync returns when the
* subprocess is killed by SIGTERM after exceeding its timeout.
@@ -2930,6 +2948,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
// (copy to main tree) and succeed, not return worktree_dirty.
const calls = [];
const rescued = [];
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -2945,8 +2964,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
calls.push(args.join(' '));
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -2958,10 +2977,10 @@ describe('executeWorktreeWaveCleanupPlan', () => {
// SUMMARY is NOT committed on the branch. `git cat-file -e HEAD:<path>` returns
// exit 128 (NOT 1) for an absent path (#2556): "fatal: path '...' does not exist
// in 'HEAD'". Rescue must fire on this real exit code.
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// Only the SUMMARY is dirty — no other modified files
return { exitCode: 0, stdout: '?? .planning/q1-SUMMARY.md', stderr: '' };
}
@@ -2976,15 +2995,18 @@ describe('executeWorktreeWaveCleanupPlan', () => {
}
return { exitCode: 0, stdout: '', stderr: '' };
},
// Inject FS deps so tests don't touch the real filesystem
// Inject FS deps so tests don't touch the real filesystem.
// Key on the RESOLVED path identity and return paths joined off it — the
// walker contract the default walker honors, and the only form whose
// slice-derived relPath is correct on every platform (#4758).
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
readFileSync: (p) => {
if (p === '/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md') return 'summary content';
if (p === path.join(wtResolved, '.planning', 'q1-SUMMARY.md')) return 'summary content';
return '';
},
existsSync: (_p) => false,
@@ -2994,7 +3016,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
// SUMMARY was rescued into the main tree
assert.equal(rescued.length, 1, 'SUMMARY.md must be rescued (copied) to main tree');
assert.equal(rescued[0].src, '/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md');
assert.equal(rescued[0].src, path.join(wtResolved, '.planning', 'q1-SUMMARY.md'));
// Normalize to forward slashes for cross-platform assertion (path.join uses \ on Windows)
assert.equal(rescued[0].dest.replace(/\\/g, '/'), '/repo/main/.planning/q1-SUMMARY.md');
@@ -3006,6 +3028,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
test('#3804: still blocks when worktree has non-SUMMARY dirty files alongside SUMMARY', () => {
// If there are OTHER dirty files (not SUMMARY), cleanup must still block.
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3020,8 +3043,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3032,22 +3055,25 @@ describe('executeWorktreeWaveCleanupPlan', () => {
}
// SUMMARY is NOT committed on the branch (uncommitted, per quick.md contract).
// cat-file -e returns 128 for an absent path (#2556).
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// SUMMARY plus another dirty file
return { exitCode: 0, stdout: '?? .planning/q1-SUMMARY.md\nM src/foo.js', stderr: '' };
}
throw new Error(`unexpected git call after dirty check: ${key}`);
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
readFileSync: () => 'summary content',
readFileSync: (p) => {
if (p === path.join(wtResolved, '.planning', 'q1-SUMMARY.md')) return 'summary content';
return '';
},
existsSync: () => false,
mkdirSync: () => {},
copyFileSync: () => {},
@@ -3056,12 +3082,149 @@ describe('executeWorktreeWaveCleanupPlan', () => {
assert.equal(result.entries[0].reason, 'worktree_dirty');
});
test('#4758: rescue resolves a relative worktree_path against repoRoot, not process.cwd()', (t) => {
const fs = require('node:fs');
// The manifest's worktree_path is RELATIVE and repoRoot (a temp dir) differs from
// process.cwd(). Every git consumer of the field resolves `-C <relative>` against
// its cwd=repoRoot; the rescue's filesystem walk must resolve the same field the
// same way. Before the fix the walker resolved against process.cwd(), found
// nothing, and the entry blocked worktree_dirty instead of rescuing.
// The fs deps are deliberately NOT injected: the real default walker and the real
// copy are the subjects under test.
const repoRoot = createTempDir('gsd-4758-repo-');
t.after(() => cleanup(repoRoot));
const worktreePath = '.claude/worktrees/agent-rel-4758';
const absWorktree = path.join(repoRoot, worktreePath);
fs.mkdirSync(path.join(absWorktree, '.planning'), { recursive: true });
fs.writeFileSync(path.join(absWorktree, '.planning', 'q1-SUMMARY.md'), 'summary content');
const plan = {
ok: true,
repoRoot,
action: 'cleanup_wave',
discovery: 'manifest',
entries: [{
agent_id: 'a1',
worktree_path: worktreePath,
branch: 'worktree-agent-a1',
expected_base: 'abc123',
}],
};
// Resolution-agnostic fake: behavior keys on the repoRoot-resolved -C operand —
// the same resolution git itself applies to `-C <relative>`.
const resolveGitKey = (args) => (args[0] === '-C'
? `-C ${path.resolve(repoRoot, args[1])} ${args.slice(2).join(' ')}`
: args.join(' '));
const wtKey = `-C ${absWorktree}`;
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = resolveGitKey(args);
if (key === `${wtKey} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
return { exitCode: 0, stdout: 'abc123', stderr: '' };
}
if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') {
return { exitCode: 0, stdout: '', stderr: '' };
}
// SUMMARY is NOT committed on the branch (#2556: cat-file -e returns 128).
if (key === `${wtKey} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
if (key === `${wtKey} status --porcelain --untracked-files=all`) {
return { exitCode: 0, stdout: '?? .planning/q1-SUMMARY.md', stderr: '' };
}
return { exitCode: 0, stdout: '', stderr: '' };
},
});
const rescuedDest = path.join(repoRoot, '.planning', 'q1-SUMMARY.md');
assert.equal(fs.readFileSync(rescuedDest, 'utf8'), 'summary content',
'rescue must copy the worktree SUMMARY into repoRoot despite the relative manifest path');
assert.equal(result.ok, true, 'cleanup must succeed when only the SUMMARY was dirty');
assert.equal(result.entries[0].status, 'merged_removed',
'a rescued SUMMARY must not block the entry as worktree_dirty');
assert.equal(result.entries[0].reason, 'ok');
});
test('#4758: every rescue reader sees the repoRoot-resolved worktree path', () => {
// Seam contract: the rescue resolves entry.worktree_path ONCE against repoRoot and
// hands the same absolute path to every reader — the injected fs walker and its own
// `git -C` calls — instead of passing the manifest value verbatim to fs reads
// (which then resolve against process.cwd()).
const seenWalker = [];
const seenGit = [];
const repoRoot = '/repo/main';
// Computed, not literal: on win32 path.resolve rewrites driveless-absolute
// and forward-slash inputs to the current drive's backslash form — exactly
// the value the rescue must hand its readers there.
const resolvedWt = path.resolve(repoRoot, 'wt/agent-a1');
const plan = {
ok: true,
repoRoot,
action: 'cleanup_wave',
discovery: 'manifest',
entries: [{
agent_id: 'a1',
worktree_path: 'wt/agent-a1',
branch: 'worktree-agent-a1',
expected_base: 'abc123',
}],
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
seenGit.push(key);
if (key === `-C wt/agent-a1 rev-parse --abbrev-ref HEAD`) {
// The CALLER's branch check passes the manifest value verbatim to git;
// its cwd=repoRoot resolves `-C <relative>`. Unchanged by the fix.
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === `-C ${resolvedWt} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
return { exitCode: 0, stdout: 'abc123', stderr: '' };
}
if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') {
return { exitCode: 0, stdout: '', stderr: '' };
}
if (key === `-C ${resolvedWt} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
if (key === `-C wt/agent-a1 status --porcelain --untracked-files=all`) {
// The CALLER's post-rescue dirty check passes the manifest value verbatim to
// git (cwd=repoRoot resolves it) — unchanged by the fix, so keep it answerable.
return { exitCode: 0, stdout: '?? .planning/q1-SUMMARY.md', stderr: '' };
}
return { exitCode: 0, stdout: '', stderr: '' };
},
findSummaryFiles: (p) => {
seenWalker.push(p);
return p === resolvedWt ? [path.join(resolvedWt, '.planning/q1-SUMMARY.md')] : [];
},
readFileSync: (p) => (String(p).endsWith('q1-SUMMARY.md') ? 'summary content' : ''),
existsSync: () => false,
mkdirSync: () => {},
copyFileSync: () => {},
});
assert.deepEqual(seenWalker, [resolvedWt],
'the fs walker must receive the repoRoot-resolved path, not the verbatim relative value');
assert.ok(
seenGit.includes(`-C ${resolvedWt} cat-file -e HEAD:.planning/q1-SUMMARY.md`),
`the rescue's own git calls must use the same resolved path; saw: ${JSON.stringify(seenGit)}`);
assert.equal(result.entries[0].status, 'merged_removed');
});
test('#245: blocks with summary_rescue_failed when copyFileSync throws during rescue', () => {
// Fixture: the only dirty file is .planning/q1-SUMMARY.md, but copyFileSync throws
// (simulating ENOSPC / permission error). The path must NOT be added to rescuedRelPaths,
// so the entry must be blocked with status='blocked', reason='summary_rescue_failed',
// and the worktree must NOT be merged or removed.
const calls = [];
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3077,8 +3240,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
calls.push(args.join(' '));
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3089,10 +3252,10 @@ describe('executeWorktreeWaveCleanupPlan', () => {
}
// SUMMARY is NOT committed — cat-file -e returns exit 128 for an absent path (#2556);
// rescue proceeds and copyFileSync throws (ENOSPC).
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// Only the SUMMARY is dirty
return { exitCode: 0, stdout: '?? .planning/q1-SUMMARY.md', stderr: '' };
}
@@ -3103,13 +3266,13 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
readFileSync: (p) => {
if (p === '/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md') return 'summary content';
if (p === path.join(wtResolved, '.planning', 'q1-SUMMARY.md')) return 'summary content';
return '';
},
existsSync: () => false,
@@ -3137,6 +3300,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
// The rescue step must skip this file entirely. The merge must succeed.
const calls = [];
const rescued = [];
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3152,8 +3316,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
calls.push(args.join(' '));
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3163,10 +3327,10 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
}
// SUMMARY is committed on the branch — cat-file -e HEAD:<path> succeeds (exit 0)
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 0, stdout: '.planning/q1-SUMMARY.md', stderr: '' };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// Worktree is clean — SUMMARY is committed, not dirty
return { exitCode: 0, stdout: '', stderr: '' };
}
@@ -3182,8 +3346,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
@@ -3206,6 +3370,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
test('#706: SUMMARY committed on branch + untracked non-SUMMARY dirty file still blocks', () => {
// Even when SUMMARY is committed (no rescue needed), a non-SUMMARY dirty file must block.
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3220,8 +3385,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3231,18 +3396,18 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
}
// SUMMARY is committed on the branch
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 0, stdout: '.planning/q1-SUMMARY.md', stderr: '' };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// Another untracked file exists alongside the committed SUMMARY
return { exitCode: 0, stdout: '?? scratch.txt', stderr: '' };
}
throw new Error(`unexpected git call after dirty check: ${key}`);
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
@@ -3265,6 +3430,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
// but git status shows 'A .planning/q1-SUMMARY.md' (staged). Rescue must
// copy it into the main tree and the cleanup must proceed.
const rescued = [];
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3279,8 +3445,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3290,10 +3456,10 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
}
// SUMMARY is staged but NOT committed — absent from HEAD, cat-file returns 128 (#2556)
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// File is staged ('A .planning/q1-SUMMARY.md')
return { exitCode: 0, stdout: 'A .planning/q1-SUMMARY.md', stderr: '' };
}
@@ -3309,8 +3475,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
@@ -3341,6 +3507,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
//
// Fixture: cat-file returns exitCode:128. Rescue MUST fire (copy into main tree).
const rescued = [];
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3355,8 +3522,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3367,10 +3534,10 @@ describe('executeWorktreeWaveCleanupPlan', () => {
}
// cat-file returns 128 — the SUMMARY is absent from HEAD (#2556: the normal
// uncommitted state, NOT a fatal error)
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'", timedOut: false };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// Worktree appears clean (SUMMARY is committed on branch)
return { exitCode: 0, stdout: '', stderr: '' };
}
@@ -3386,8 +3553,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
@@ -3415,6 +3582,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
//
// Fixture: cat-file returns timedOut:true. Rescue MUST fire (copy into main tree).
const rescued = [];
const wtResolved = path.resolve('/repo/main', '/repo/.claude/worktrees/agent-a1');
const plan = {
ok: true,
repoRoot: '/repo/main',
@@ -3429,8 +3597,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3440,7 +3608,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
}
// cat-file times out — cannot determine if SUMMARY is committed
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return {
exitCode: null,
stdout: '',
@@ -3450,7 +3618,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
error: Object.assign(new Error('spawnSync git ETIMEDOUT'), { code: 'ETIMEDOUT' }),
};
}
if (key === '-C /repo/.claude/worktrees/agent-a1 status --porcelain --untracked-files=all') {
if (key === `-C ${wtResolved} status --porcelain --untracked-files=all`) {
// Worktree appears clean (SUMMARY is committed on branch)
return { exitCode: 0, stdout: '', stderr: '' };
}
@@ -3466,8 +3634,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
return { exitCode: 0, stdout: '', stderr: '' };
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},
@@ -3554,8 +3722,11 @@ describe('executeWorktreeWaveCleanupPlan', () => {
const result = executeWorktreeWaveCleanupPlan(waveCleanupPlanFixture, {
execGit,
findSummaryFiles: (worktreePath) => (
worktreePath === '/repo/.claude/worktrees/agent-a1'
? ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md']
path.resolve(waveCleanupPlanFixture.repoRoot, worktreePath)
=== path.resolve(waveCleanupPlanFixture.repoRoot, '/repo/.claude/worktrees/agent-a1')
? [path.join(
path.resolve(waveCleanupPlanFixture.repoRoot, '/repo/.claude/worktrees/agent-a1'),
'.planning', 'q1-SUMMARY.md')]
: []
),
readFileSync: () => 'summary content',
@@ -3577,8 +3748,11 @@ describe('executeWorktreeWaveCleanupPlan', () => {
const result = executeWorktreeWaveCleanupPlan(waveCleanupPlanFixture, {
execGit,
findSummaryFiles: (worktreePath) => (
worktreePath === '/repo/.claude/worktrees/agent-a1'
? ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md']
path.resolve(waveCleanupPlanFixture.repoRoot, worktreePath)
=== path.resolve(waveCleanupPlanFixture.repoRoot, '/repo/.claude/worktrees/agent-a1')
? [path.join(
path.resolve(waveCleanupPlanFixture.repoRoot, '/repo/.claude/worktrees/agent-a1'),
'.planning', 'q1-SUMMARY.md')]
: []
),
readFileSync: () => 'summary content',
@@ -3661,8 +3835,19 @@ describe('executeWorktreeWaveCleanupPlan', () => {
// mismatch, no deletions, no dirty files. Returns undefined for an unmatched key
// so callers can layer entry-specific overrides in front of this fallback.
function cleanEntryResponse(key, branch, worktreePath) {
if (key === `-C ${worktreePath} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: branch, stderr: '' };
// #4758: answer both `-C` spellings — the caller passes the manifest value
// verbatim (git resolves it against its cwd=repoRoot) while tests that
// normalize keys hand the path.resolve(repoRoot, …) form; the two are
// identical on POSIX and drive-rewritten on win32. '/repo/main' is this
// describe's shared fixture repoRoot.
const wtForms = [worktreePath, path.resolve('/repo/main', worktreePath)];
for (const wt of wtForms) {
if (key === `-C ${wt} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: branch, stderr: '' };
}
if (key === `-C ${wt} status --porcelain --untracked-files=all`) {
return { exitCode: 0, stdout: '', stderr: '' };
}
}
if (key === `merge-base HEAD ${branch}`) {
return { exitCode: 0, stdout: 'abc123', stderr: '' };
@@ -3670,9 +3855,6 @@ describe('executeWorktreeWaveCleanupPlan', () => {
if (key === `diff --diff-filter=D --name-only HEAD...${branch}`) {
return { exitCode: 0, stdout: '', stderr: '' };
}
if (key === `-C ${worktreePath} status --porcelain --untracked-files=all`) {
return { exitCode: 0, stdout: '', stderr: '' };
}
if (key === `merge ${branch} --no-ff --no-edit -m chore: merge executor worktree (${branch})`) {
return { exitCode: 0, stdout: '', stderr: '' };
}
@@ -3904,10 +4086,11 @@ describe('executeWorktreeWaveCleanupPlan', () => {
const e1 = makeEntry('a1', 'worktree-agent-a1');
const e2 = makeEntry('a2', 'worktree-agent-a2');
const plan = { ok: true, repoRoot: '/repo/main', action: 'cleanup_wave', discovery: 'manifest', entries: [e1, e2] };
const wtResolved = path.resolve(plan.repoRoot, e1.worktree_path);
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
const key = gitKeyFor(plan.repoRoot, args);
if (key === `-C ${wtResolved} rev-parse --abbrev-ref HEAD`) {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
}
if (key === 'merge-base HEAD worktree-agent-a1') {
@@ -3916,7 +4099,7 @@ describe('executeWorktreeWaveCleanupPlan', () => {
if (key === 'diff --diff-filter=D --name-only HEAD...worktree-agent-a1') {
return { exitCode: 0, stdout: '', stderr: '' };
}
if (key === '-C /repo/.claude/worktrees/agent-a1 cat-file -e HEAD:.planning/q1-SUMMARY.md') {
if (key === `-C ${wtResolved} cat-file -e HEAD:.planning/q1-SUMMARY.md`) {
return { exitCode: 128, stdout: '', stderr: "fatal: path '.planning/q1-SUMMARY.md' does not exist in 'HEAD'" };
}
const clean2 = cleanEntryResponse(key, e2.branch, e2.worktree_path);
@@ -3924,8 +4107,8 @@ describe('executeWorktreeWaveCleanupPlan', () => {
throw new Error(`unexpected git call: ${key}`);
},
findSummaryFiles: (worktreePath) => {
if (worktreePath === '/repo/.claude/worktrees/agent-a1') {
return ['/repo/.claude/worktrees/agent-a1/.planning/q1-SUMMARY.md'];
if (path.resolve(plan.repoRoot, worktreePath) === wtResolved) {
return [path.join(wtResolved, '.planning', 'q1-SUMMARY.md')];
}
return [];
},