diff --git a/.changeset/happy-quails-romp.md b/.changeset/happy-quails-romp.md new file mode 100644 index 000000000..85c453074 --- /dev/null +++ b/.changeset/happy-quails-romp.md @@ -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) diff --git a/src/worktree-safety.cts b/src/worktree-safety.cts index 0d3e4fd1a..bfa4c3357 100644 --- a/src/worktree-safety.cts +++ b/src/worktree-safety.cts @@ -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 ` 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(); 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:` (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; diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index 800824955..bfb1a36db 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -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 + * ':\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 ` 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:` 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 ` 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 `. + 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 `. 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: 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 []; },