diff --git a/tests/worktree-safety-reap.test.cjs b/tests/worktree-safety-reap.test.cjs new file mode 100644 index 000000000..d390136c6 --- /dev/null +++ b/tests/worktree-safety-reap.test.cjs @@ -0,0 +1,821 @@ +'use strict'; + +/** + * `reapOrphanWorktrees` — fault-injected verdict coverage (#3057, wave 3). + * + * Seam: gsd-core/bin/lib/worktree-safety.cjs + * Interface: reapOrphanWorktrees, cmdWorktreeReapOrphans, pruneOrphanedWorktrees + * + * WHY A SECOND FILE FOR THIS MODULE + * `tests/worktree-safety.test.cjs` is ~6.4k lines and its `reapOrphanWorktrees` + * suites live inside a folded block with their own local fixture helpers. The + * negative-space work below needs a different fixture shape (an injected + * `execGit` that delegates to real git, plus per-test mutation of the + * `.git/worktrees//` admin directory), so it gets its own module-bucketed + * file rather than a third set of helpers wedged into the folded block. + * + * WHAT THIS FILE PINS THAT NOTHING ELSE DID + * Every pre-existing test drove the DEFAULT `execGit` against real git and + * injected only `mtimeSafe` / `nowMs` / `isPidAlive`. `reapOrphanWorktrees` + * accepts `execGit`, `readDirSafe` and `readFileSafe` in the same `deps` bag, + * and nothing used them — so every fail-closed `return` inside the function was + * unreachable from the suite. Each test here injects exactly the one fault that + * selects one branch and asserts the SPECIFIC `{status, reason}` verdict that + * branch produces, never merely that the call returned an array. + * + * Determinism: no wall clock is read (`mtimeSafe`/`nowMs` are injected), and no + * live PID is probed (`isPidAlive` is injected), so the only real-world + * dependency is git itself. + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); +const { runGit } = require('./helpers/process-seam.cjs'); +const { makeFaultyGit, withFaultyFs } = require('./helpers/faulty-deps.cjs'); + +const { + reapOrphanWorktrees, + cmdWorktreeReapOrphans, + pruneOrphanedWorktrees, +} = require('../gsd-core/bin/lib/worktree-safety.cjs'); + +// ─── Fixed clock values (ADR-456 clock seam) ───────────────────────────────── + +/** Older than any staleness threshold, at any real point in time. */ +const STALE_MTIME = new Date(0); + +/** The lock-owner PID written into every fixture; liveness is always injected. */ +const LOCK_OWNER_PID = '4242'; + +const GIT_TIMEOUT_MS = 30000; + +// ─── Path + git helpers ────────────────────────────────────────────────────── + +function canonicalPath(p) { + try { return fs.realpathSync.native(path.resolve(p)); } catch { return path.resolve(p); } +} + +/** + * Long-form os.tmpdir(). Windows CI reports 8.3 short names that git does not + * echo back, so every fixture path is built from the resolved form. + */ +function resolvedTmpDir() { + try { return fs.realpathSync.native(os.tmpdir()); } catch { return os.tmpdir(); } +} + +/** Run git for FIXTURE SETUP; throws on anything but a clean exit. */ +function git(args, cwd) { + const r = runGit(args, { cwd, timeoutMs: GIT_TIMEOUT_MS }); + if (r.exitCode !== 0) { + throw new Error(`git ${args.join(' ')} failed (${r.outcome}/${r.exitCode}): ${r.stderr}`); + } + return r.stdout; +} + +/** + * An `execGit`-shaped delegate that runs REAL git. Used as `makeFaultyGit`'s + * `passthrough` so a test can fault one argv and leave every other call intact. + */ +function realExecGit(args, opts = {}) { + const r = runGit(args, { cwd: opts.cwd, timeoutMs: GIT_TIMEOUT_MS }); + return { + exitCode: r.exitCode, + stdout: r.stdout, + stderr: r.stderr, + signal: r.signal, + error: r.code === null ? null : Object.assign(new Error(r.code), { code: r.code }), + timedOut: r.timedOut, + }; +} + +/** A benign zero-exit result carrying `stdout`. */ +function okResult(stdout) { + return { exitCode: 0, stdout, stderr: '', signal: null, error: null, timedOut: false }; +} + +function argvOf(faultyGit) { + return faultyGit.calls.map((c) => c.args.join(' ')); +} + +function calledWith(faultyGit, prefix) { + return faultyGit.calls.some((c) => prefix.every((token, i) => c.args[i] === token)); +} + +// ─── Fixture construction ──────────────────────────────────────────────────── + +function initRepo(dir) { + fs.mkdirSync(dir, { recursive: true }); + git(['init'], dir); + git(['config', 'user.email', 'test@test.com'], dir); + git(['config', 'user.name', 'Test'], dir); + git(['config', 'commit.gpgsign', 'false'], dir); + fs.writeFileSync(path.join(dir, 'README.md'), '# Test\n'); + git(['add', '-A'], dir); + git(['commit', '-m', 'initial commit'], dir); + // Exit code deliberately unchecked: the rename fails harmlessly when the + // repo was already initialised on `main`. + runGit(['branch', '-m', 'master', 'main'], { cwd: dir, timeoutMs: GIT_TIMEOUT_MS }); +} + +/** Locate `.git/worktrees//` for a linked worktree. */ +function adminDirFor(repoDir, wtDir) { + const commonDir = path.resolve(repoDir, git(['rev-parse', '--git-common-dir'], repoDir).trim()); + const worktreesDir = path.join(commonDir, 'worktrees'); + const wanted = canonicalPath(wtDir); + for (const entry of fs.readdirSync(worktreesDir)) { + const gitdirFile = path.join(worktreesDir, entry, 'gitdir'); + if (!fs.existsSync(gitdirFile)) continue; + const pointer = fs.readFileSync(gitdirFile, 'utf8').trim(); + const root = path.resolve(worktreesDir, entry, pointer).replace(/[/\\]\.git$/, ''); + if (canonicalPath(root) === wanted) return path.join(worktreesDir, entry); + } + throw new Error(`no .git/worktrees/ admin dir for ${wtDir}`); +} + +/** + * Build a repo with one linked, locked worktree whose branch is merged into + * `main` unless `merge:false`. The lock owner is a fixed PID string; liveness is + * always supplied through `deps.isPidAlive`, never probed against the OS. + */ +function makeFixture(tmpBase, name, options = {}) { + const repoDir = path.join(tmpBase, `repo-${name}`); + const wtDir = path.join(tmpBase, `wt-${name}`); + const branch = `worktree-agent-${name}`; + + initRepo(repoDir); + git(['worktree', 'add', wtDir, '-b', branch], repoDir); + fs.writeFileSync(path.join(wtDir, 'work.txt'), 'content\n'); + git(['add', '-A'], wtDir); + git(['commit', '-m', `work in ${name}`], wtDir); + if (options.merge !== false) { + git(['merge', branch, '--no-ff', '-m', `merge ${branch}`], repoDir); + } + + const adminDir = adminDirFor(repoDir, wtDir); + if (options.lock !== false) { + fs.writeFileSync(path.join(adminDir, 'locked'), LOCK_OWNER_PID); + } + return { repoDir, wtDir, branch, adminDir }; +} + +/** Deps every "owner is dead, lock is stale" test shares. */ +function deadOwnerDeps(extra = {}) { + return { isPidAlive: () => false, mtimeSafe: () => STALE_MTIME, ...extra }; +} + +/** Assert exactly one result row, and return it. */ +function onlyRow(result) { + assert.strictEqual(result.length, 1, `expected exactly one result row, got ${JSON.stringify(result)}`); + return result[0]; +} + +// ─── Suite: default-branch discovery — fail-closed verdicts ────────────────── + +describe('#3057 reapOrphanWorktrees: default-branch discovery verdicts', () => { + let tmpBase; + + beforeEach(() => { + tmpBase = fs.mkdtempSync(path.join(resolvedTmpDir(), 'gsd-3057-reap-disc-')); + }); + + afterEach(() => { + cleanup(tmpBase); + }); + + test('returns no rows and never reads the admin directory when git cannot resolve --git-dir', () => { + const f = makeFixture(tmpBase, 'nogitdir'); + const probed = []; + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'exit', exitCode: 128, when: ['rev-parse', '--git-dir'] }], + passthrough: realExecGit, + }); + + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ + execGit: faultyGit, + readDirSafe: (dir) => { probed.push(dir); return fs.readdirSync(dir); }, + })); + + assert.deepStrictEqual(result, []); + assert.deepStrictEqual(probed, [], 'admin directory must not be read once --git-dir failed'); + assert.deepStrictEqual(argvOf(faultyGit), ['rev-parse --git-dir']); + assert.ok(fs.existsSync(f.wtDir), 'the worktree must survive a fail-closed bail-out'); + }); + + test('returns no rows when the worktrees admin directory cannot be listed', () => { + const f = makeFixture(tmpBase, 'nodir'); + const probed = []; + const faultyGit = makeFaultyGit({ passthrough: realExecGit }); + + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ + execGit: faultyGit, + readDirSafe: (dir) => { probed.push(dir); return null; }, + })); + + assert.deepStrictEqual(result, []); + assert.strictEqual(probed.length, 1, 'the admin directory must be probed exactly once'); + assert.strictEqual(path.basename(probed[0]), 'worktrees'); + // Distinguishes this bail-out from the --git-dir one above: --git-dir DID + // run and succeed, and nothing after the admin listing was attempted. + assert.deepStrictEqual(argvOf(faultyGit), ['rev-parse --git-dir']); + }); + + test('returns no rows for a repo that has no linked worktrees at all', () => { + // Exercises the real `defaultReadDirSafe` catch: `.git/worktrees/` does not + // exist, so readdirSync throws and the helper returns null. + const repoDir = path.join(tmpBase, 'repo-bare-of-worktrees'); + initRepo(repoDir); + + assert.deepStrictEqual(reapOrphanWorktrees(repoDir), []); + }); + + test('reaps from origin/HEAD alone and never consults local branch candidates', () => { + const f = makeFixture(tmpBase, 'remotehead'); + const mainTip = git(['rev-parse', 'main'], f.repoDir).trim(); + const faultyGit = makeFaultyGit({ + passthrough: (args, opts) => { + if (args[0] === 'symbolic-ref' && args[args.length - 1] === 'refs/remotes/origin/HEAD') { + return okResult('origin/main\n'); + } + if (args[0] === 'rev-parse' && args[1] === 'refs/remotes/origin/main') { + return okResult(`${mainTip}\n`); + } + return realExecGit(args, opts); + }, + }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'reaped'); + assert.strictEqual(row.reason, 'pid_dead_and_merged'); + // The remote-exclusive arm is what makes this distinguishable from the + // local-candidate arm that every other fixture in the tree takes. + assert.strictEqual(calledWith(faultyGit, ['remote']), false, 'must not fall back to remote enumeration'); + assert.strictEqual( + calledWith(faultyGit, ['config', '--get', 'init.defaultBranch']), + false, + 'must not build a local candidate list when origin/HEAD resolved' + ); + }); + + test('returns no rows when origin/HEAD names a remote ref that will not resolve', () => { + const f = makeFixture(tmpBase, 'badremoteref'); + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'exit', exitCode: 128, when: ['rev-parse', 'refs/remotes/origin/main'] }], + passthrough: (args, opts) => ( + args[0] === 'symbolic-ref' && args[args.length - 1] === 'refs/remotes/origin/HEAD' + ? okResult('origin/main\n') + : realExecGit(args, opts) + ), + }); + + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit })); + + assert.deepStrictEqual(result, []); + assert.strictEqual( + calledWith(faultyGit, ['worktree', 'list']), + false, + 'must fail closed before building the canonical index' + ); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('returns no rows when a remote exists but origin/HEAD is unset', () => { + const f = makeFixture(tmpBase, 'ambiguousremote'); + const originSrc = path.join(tmpBase, 'origin-src'); + initRepo(originSrc); + git(['remote', 'add', 'origin', originSrc], f.repoDir); + const faultyGit = makeFaultyGit({ passthrough: realExecGit }); + + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit })); + + assert.deepStrictEqual(result, [], 'an ambiguous default branch must not be guessed'); + assert.strictEqual(calledWith(faultyGit, ['remote']), true); + assert.strictEqual( + calledWith(faultyGit, ['config', '--get', 'init.defaultBranch']), + false, + 'the candidate list must not be built once a remote is known to exist' + ); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('returns no rows when not one default-branch candidate resolves', () => { + const f = makeFixture(tmpBase, 'nocandidate'); + const faultyGit = makeFaultyGit({ + faults: [{ + kind: 'exit', + exitCode: 128, + when: (args) => args[0] === 'rev-parse' && args[1] !== '--git-dir', + }], + passthrough: realExecGit, + }); + + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit })); + + assert.deepStrictEqual(result, []); + assert.strictEqual(calledWith(faultyGit, ['rev-parse', 'main']), true); + assert.strictEqual(calledWith(faultyGit, ['rev-parse', 'master']), true); + assert.strictEqual( + calledWith(faultyGit, ['worktree', 'list']), + false, + 'must fail closed before building the canonical index' + ); + }); +}); + +// ─── Suite: canonical-index construction ───────────────────────────────────── + +describe('#3057 reapOrphanWorktrees: canonical-index degradation verdicts', () => { + let tmpBase; + + beforeEach(() => { + tmpBase = fs.mkdtempSync(path.join(resolvedTmpDir(), 'gsd-3057-reap-idx-')); + }); + + afterEach(() => { + cleanup(tmpBase); + }); + + test('still reaps when git worktree list fails and the canonical index stays empty', () => { + const f = makeFixture(tmpBase, 'listfails'); + const wtCanonical = canonicalPath(f.wtDir); + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'timeout', when: ['worktree', 'list'] }], + passthrough: realExecGit, + }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + // A failed listing must degrade to the gitdir-derived path, NOT abort the + // sweep — an empty index is not "there is nothing to reap". + assert.strictEqual(row.status, 'reaped'); + assert.strictEqual(row.reason, 'pid_dead_and_merged'); + assert.strictEqual(canonicalPath(row.path), wtCanonical); + assert.strictEqual(calledWith(faultyGit, ['worktree', 'list']), true); + assert.strictEqual(fs.existsSync(f.wtDir), false); + }); + + test('still reaps when a porcelain block carries no worktree line', () => { + const f = makeFixture(tmpBase, 'headlessblock'); + const realPorcelain = git(['worktree', 'list', '--porcelain'], f.repoDir); + const faultyGit = makeFaultyGit({ + passthrough: (args, opts) => ( + args[0] === 'worktree' && args[1] === 'list' + ? okResult(`bare\n\n${realPorcelain}`) + : realExecGit(args, opts) + ), + }); + + // Without the `continue`, `wtLine.slice(...)` would throw on the leading + // block and the whole sweep would die. + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'reaped'); + assert.strictEqual(row.reason, 'pid_dead_and_merged'); + }); + + test('still reaps when the porcelain lists a path that no longer exists on disk', () => { + const f = makeFixture(tmpBase, 'ghostpath'); + const realPorcelain = git(['worktree', 'list', '--porcelain'], f.repoDir); + const ghost = path.join(tmpBase, 'ghost-worktree'); + const faultyGit = makeFaultyGit({ + passthrough: (args, opts) => ( + args[0] === 'worktree' && args[1] === 'list' + ? okResult(`worktree ${ghost}\nHEAD 0000000000000000000000000000000000000000\n\n${realPorcelain}`) + : realExecGit(args, opts) + ), + }); + + // realpathSync.native throws for the ghost block; the catch must skip that + // one entry and keep indexing the rest. + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'reaped'); + assert.strictEqual(row.reason, 'pid_dead_and_merged'); + }); +}); + +// ─── Suite: admin-directory shape ──────────────────────────────────────────── + +describe('#3057 reapOrphanWorktrees: admin-entry verdicts', () => { + let tmpBase; + + beforeEach(() => { + tmpBase = fs.mkdtempSync(path.join(resolvedTmpDir(), 'gsd-3057-reap-admin-')); + }); + + afterEach(() => { + cleanup(tmpBase); + }); + + test('reports no row at all for a linked worktree that carries no lock file', () => { + const f = makeFixture(tmpBase, 'locked'); + const unlockedDir = path.join(tmpBase, 'wt-unlocked'); + git(['worktree', 'add', unlockedDir, '-b', 'worktree-agent-unlocked'], f.repoDir); + + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps()); + + const row = onlyRow(result); + assert.strictEqual(canonicalPath(row.path), canonicalPath(f.wtDir)); + assert.strictEqual(row.status, 'reaped'); + assert.ok(fs.existsSync(unlockedDir), 'an unlocked worktree is not the reaper concern'); + }); + + test('reports no row for a locked admin entry whose gitdir pointer is missing', () => { + const f = makeFixture(tmpBase, 'nopointer'); + fs.unlinkSync(path.join(f.adminDir, 'gitdir')); + + // The lock file is present and stale and the owner is dead, so a row WOULD + // be emitted if the missing pointer were not a hard skip. + assert.deepStrictEqual(reapOrphanWorktrees(f.repoDir, deadOwnerDeps()), []); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports no row when an injected readFileSafe reports the gitdir pointer as empty', () => { + const f = makeFixture(tmpBase, 'blankpointer'); + const gitdirFile = path.join(f.adminDir, 'gitdir'); + + // Covers the `deps.readFileSafe` seam arm AND the empty-string half of the + // falsy-pointer guard (the missing-file half returns null, not ''). + const result = reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ + readFileSafe: (file) => { + if (path.resolve(file) === path.resolve(gitdirFile)) return ''; + try { return fs.readFileSync(file, 'utf8'); } catch { return null; } + }, + })); + + assert.deepStrictEqual(result, []); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports lock_too_fresh when the real mtime helper cannot stat the lock file', () => { + const f = makeFixture(tmpBase, 'statfails'); + + // No `mtimeSafe` injection: this drives the module's own default helper and + // pins its catch arm. nowMs is the far future, so a readable mtime would + // read as stale and reap. + const result = withFaultyFs( + { statSync: () => { throw Object.assign(new Error('EIO'), { code: 'EIO' }); } }, + () => reapOrphanWorktrees(f.repoDir, { isPidAlive: () => false, nowMs: 8640000000000000 }) + ); + + const row = onlyRow(result); + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'lock_too_fresh'); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports remove_failed against the raw gitdir pointer when its basename is not .git', () => { + const f = makeFixture(tmpBase, 'oddpointer'); + const pointerTarget = path.join(f.wtDir, 'notgit'); + fs.writeFileSync(path.join(f.adminDir, 'gitdir'), `${pointerTarget}\n`); + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'exit', exitCode: 1, when: ['worktree', 'remove'] }], + passthrough: realExecGit, + }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + // `path` is the load-bearing assertion: a pointer that does not end in + // `/.git` is used verbatim (no dirname()), and because it does not exist, + // the canonical lookup throws and the raw path is what reaches git. + assert.strictEqual(row.path, pointerTarget); + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'remove_failed'); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports lock_too_fresh when the lock file cannot be stat-ed', () => { + const f = makeFixture(tmpBase, 'nomtime'); + + // nowMs is the far future, so a REAL mtime would read as stale and the + // entry would be reaped. Only the null-mtime arm can produce this verdict. + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { + isPidAlive: () => false, + mtimeSafe: () => null, + nowMs: 8640000000000000, + })); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'lock_too_fresh'); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports lock_too_fresh for a zero-age lock under the default guard', () => { + const f = makeFixture(tmpBase, 'defaultguard'); + const now = 1000000; + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { + isPidAlive: () => false, + mtimeSafe: () => new Date(now), + nowMs: now, + })); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'lock_too_fresh'); + }); + + test('reaps the same zero-age lock when an injected reapMtimeGuardMs of 0 retires the guard', () => { + const f = makeFixture(tmpBase, 'zeroguard'); + const now = 1000000; + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { + isPidAlive: () => false, + mtimeSafe: () => new Date(now), + nowMs: now, + reapMtimeGuardMs: 0, + })); + + assert.strictEqual(row.status, 'reaped'); + assert.strictEqual(row.reason, 'pid_dead_and_merged'); + }); +}); + +// ─── Suite: liveness and ancestry verdicts ─────────────────────────────────── + +describe('#3057 reapOrphanWorktrees: liveness and ancestry verdicts', () => { + let tmpBase; + + beforeEach(() => { + tmpBase = fs.mkdtempSync(path.join(resolvedTmpDir(), 'gsd-3057-reap-live-')); + }); + + afterEach(() => { + cleanup(tmpBase); + }); + + test('reports pid_alive when the lock owner is alive', () => { + const f = makeFixture(tmpBase, 'alive'); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { + isPidAlive: () => true, + mtimeSafe: () => STALE_MTIME, + })); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'pid_alive'); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports pid_alive when the liveness probe throws', () => { + const f = makeFixture(tmpBase, 'probethrows'); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { + isPidAlive: () => { throw Object.assign(new Error('EPERM'), { code: 'EPERM' }); }, + mtimeSafe: () => STALE_MTIME, + })); + + // An undeterminable owner is treated as alive — same verdict as a genuinely + // live owner, which is the intended fail-closed conflation. + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'pid_alive'); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports pid_alive from the default isPidAlive helper when process.kill throws EPERM', (t) => { + // No `isPidAlive` injection: this drives the module's OWN default helper + // (`defaultIsPidAlive`), whose EPERM arm every other test in this tree + // bypasses by injecting `isPidAlive` directly. `process.kill` is + // monkeypatched per CONTRIBUTING's cross-platform IO-fault-injection rule + // rather than run against a real cross-user PID. + const f = makeFixture(tmpBase, 'defaultkill-eperm'); + const originalKill = process.kill; + t.after(() => { process.kill = originalKill; }); + process.kill = () => { throw Object.assign(new Error('EPERM'), { code: 'EPERM' }); }; + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { mtimeSafe: () => STALE_MTIME })); + + assert.deepStrictEqual({ status: row.status, reason: row.reason }, { status: 'skipped', reason: 'pid_alive' }); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports pid_dead_and_merged from the default isPidAlive helper when process.kill throws ESRCH', (t) => { + // Same default helper as above, but its dead-owner arm: ESRCH means "no + // such process", so `defaultIsPidAlive` returns false and the sweep falls + // through to the (merged, by fixture default) ancestry check and reaps. + const f = makeFixture(tmpBase, 'defaultkill-esrch'); + const originalKill = process.kill; + t.after(() => { process.kill = originalKill; }); + process.kill = () => { throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' }); }; + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { mtimeSafe: () => STALE_MTIME })); + + assert.deepStrictEqual( + { status: row.status, reason: row.reason }, + { status: 'reaped', reason: 'pid_dead_and_merged' } + ); + assert.strictEqual(fs.existsSync(f.wtDir), false); + }); + + test('reports pid_alive from the default isPidAlive helper when process.kill returns without throwing', (t) => { + // The non-throwing arm of `defaultIsPidAlive`: a live owner's `kill(pid, + // 0)` returns normally, so the helper returns true directly, with no + // catch block involved at all. + const f = makeFixture(tmpBase, 'defaultkill-alive'); + const originalKill = process.kill; + t.after(() => { process.kill = originalKill; }); + process.kill = () => true; + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, { mtimeSafe: () => STALE_MTIME })); + + assert.deepStrictEqual({ status: row.status, reason: row.reason }, { status: 'skipped', reason: 'pid_alive' }); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports cannot_resolve_branch_tip when the admin HEAD file is absent', () => { + const f = makeFixture(tmpBase, 'noheadfile'); + fs.unlinkSync(path.join(f.adminDir, 'HEAD')); + const faultyGit = makeFaultyGit({ passthrough: realExecGit }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'cannot_resolve_branch_tip'); + assert.strictEqual( + calledWith(faultyGit, ['merge-base']), + false, + 'ancestry must not be probed once the tip is unknown' + ); + assert.ok(fs.existsSync(f.wtDir)); + }); + + test('reports cannot_resolve_branch_tip when the admin HEAD names an unresolvable branch', () => { + const f = makeFixture(tmpBase, 'deadsymref'); + fs.writeFileSync(path.join(f.adminDir, 'HEAD'), 'ref: refs/heads/does-not-exist\n'); + const faultyGit = makeFaultyGit({ passthrough: realExecGit }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'cannot_resolve_branch_tip'); + // Distinguishes the symbolic-ref arm from the missing-file and + // unrecognised-content arms, which all share this one reason string. + assert.strictEqual(calledWith(faultyGit, ['rev-parse', 'refs/heads/does-not-exist']), true); + }); + + test('reports cannot_resolve_branch_tip for an admin HEAD that is neither a symref nor a sha', () => { + const f = makeFixture(tmpBase, 'garbagehead'); + const headFile = path.join(f.adminDir, 'HEAD'); + fs.writeFileSync(headFile, 'not-a-ref\n'); + const faultyGit = makeFaultyGit({ passthrough: realExecGit }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'cannot_resolve_branch_tip'); + assert.ok(fs.existsSync(headFile), 'the HEAD file is present — this is not the missing-file arm'); + assert.strictEqual( + faultyGit.calls.some((c) => c.args[0] === 'rev-parse' && String(c.args[1]).startsWith('refs/heads/')), + false, + 'unrecognised HEAD content must not be handed to rev-parse' + ); + }); + + test('reaps a detached admin HEAD without resolving any branch ref', () => { + const f = makeFixture(tmpBase, 'detached'); + const branchTip = git(['rev-parse', f.branch], f.repoDir).trim(); + fs.writeFileSync(path.join(f.adminDir, 'HEAD'), `${branchTip}\n`); + const faultyGit = makeFaultyGit({ passthrough: realExecGit }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(row.status, 'reaped'); + assert.strictEqual(row.reason, 'pid_dead_and_merged'); + assert.strictEqual( + faultyGit.calls.some((c) => c.args[0] === 'rev-parse' && String(c.args[1]).startsWith('refs/heads/')), + false, + 'a bare 40-hex HEAD is the tip; no ref resolution is needed' + ); + assert.strictEqual(fs.existsSync(f.wtDir), false); + }); + + test('reports branch_not_merged for an unmerged branch whose lock owner is dead', () => { + const f = makeFixture(tmpBase, 'unmerged', { merge: false }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps())); + + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'branch_not_merged'); + assert.ok(fs.existsSync(f.wtDir), 'unmerged work must survive the sweep'); + }); + + test('reports remove_failed and leaves the worktree on disk when git worktree remove fails', () => { + const f = makeFixture(tmpBase, 'removefails'); + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'exit', exitCode: 1, when: ['worktree', 'remove'] }], + passthrough: realExecGit, + }); + + const row = onlyRow(reapOrphanWorktrees(f.repoDir, deadOwnerDeps({ execGit: faultyGit }))); + + assert.strictEqual(canonicalPath(row.path), canonicalPath(f.wtDir)); + assert.strictEqual(row.status, 'skipped'); + assert.strictEqual(row.reason, 'remove_failed'); + assert.ok(fs.existsSync(f.wtDir)); + assert.strictEqual(calledWith(faultyGit, ['worktree', 'unlock']), true, 'unlock precedes remove'); + }); +}); + +// ─── Suite: CLI wrappers ───────────────────────────────────────────────────── + +describe('#3057 cmdWorktreeReapOrphans / pruneOrphanedWorktrees output verdicts', () => { + let tmpBase; + + beforeEach(() => { + tmpBase = fs.mkdtempSync(path.join(resolvedTmpDir(), 'gsd-3057-reap-cli-')); + }); + + afterEach(() => { + cleanup(tmpBase); + }); + + test('cmdWorktreeReapOrphans reports ok with zero entries after the reaper throws', () => { + const out = []; + const err = []; + + cmdWorktreeReapOrphans(tmpBase, { + write: (s) => out.push(s), + writeErr: (s) => err.push(s), + execGit: () => { throw new Error('boom'); }, + }); + + assert.deepStrictEqual(err, ['[gsd] worktree.reap-orphans failed: boom\n']); + assert.deepStrictEqual(JSON.parse(out.join('')), { ok: true, reaped: 0, entries: [] }); + }); + + test('cmdWorktreeReapOrphans warns with the skipped count and emits the skipped row as JSON', () => { + const f = makeFixture(tmpBase, 'cliskip', { merge: false }); + const out = []; + const err = []; + + cmdWorktreeReapOrphans(f.repoDir, { + write: (s) => out.push(s), + writeErr: (s) => err.push(s), + ...deadOwnerDeps(), + }); + + assert.deepStrictEqual(err, [ + '[gsd] worktree.reap-orphans: 1 orphan(s) skipped (run with DEBUG=1 for details)\n', + ]); + const payload = JSON.parse(out.join('')); + assert.strictEqual(payload.ok, true); + assert.strictEqual(payload.reaped, 0); + assert.strictEqual(payload.entries.length, 1); + assert.strictEqual(payload.entries[0].status, 'skipped'); + assert.strictEqual(payload.entries[0].reason, 'branch_not_merged'); + }); + + test('cmdWorktreeReapOrphans stays silent on stderr when nothing is skipped', () => { + const f = makeFixture(tmpBase, 'cliclean'); + const out = []; + const err = []; + + cmdWorktreeReapOrphans(f.repoDir, { + write: (s) => out.push(s), + writeErr: (s) => err.push(s), + ...deadOwnerDeps(), + }); + + assert.deepStrictEqual(err, []); + const payload = JSON.parse(out.join('')); + assert.strictEqual(payload.reaped, 1); + assert.strictEqual(payload.entries[0].reason, 'pid_dead_and_merged'); + }); + + test('pruneOrphanedWorktrees warns that the health check degraded when git worktree prune times out', () => { + const f = makeFixture(tmpBase, 'prunetimeout'); + const err = []; + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'timeout', when: ['worktree', 'prune'] }], + passthrough: realExecGit, + }); + + const removed = pruneOrphanedWorktrees(f.repoDir, { + execGit: faultyGit, + writeErr: (s) => err.push(s), + }); + + assert.deepStrictEqual(removed, []); + assert.deepStrictEqual(err, [ + '[gsd-tools] WARNING: worktree health check degraded' + + ' — git worktree prune timed out after 10s.' + + ' Orphaned worktree metadata may remain until the next successful run.\n', + ]); + }); + + test('pruneOrphanedWorktrees returns an empty list and warns nothing when git throws', () => { + const f = makeFixture(tmpBase, 'prunethrows'); + const err = []; + + const removed = pruneOrphanedWorktrees(f.repoDir, { + execGit: () => { throw new Error('boom'); }, + writeErr: (s) => err.push(s), + }); + + assert.deepStrictEqual(removed, [], 'a throwing git must never crash the caller'); + assert.deepStrictEqual(err, [], 'the degraded-health warning belongs to the timeout arm only'); + }); +}); diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index 917bf8774..29cbae989 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -3582,10 +3582,7 @@ describe('worktree-safety: pruneOrphanedWorktrees behaviour', () => { test('pruneOrphanedWorktrees(temp dir) returns [] and does not throw', (t) => { const dir = createTempDir('gsd-prune-'); t.after(() => cleanup(dir)); - let result; - assert.doesNotThrow(() => { - result = worktreeSafety.pruneOrphanedWorktrees(dir); - }); + const result = worktreeSafety.pruneOrphanedWorktrees(dir); assert.deepStrictEqual(result, []); }); }); @@ -3912,10 +3909,14 @@ describe('bug-3707: reapOrphanWorktrees', () => { // ensuring the live-PID check is the only reason the entry is skipped. const result = reapOrphanWorktrees(repoDir, { mtimeSafe: () => STALE_MTIME }); + // #3057: assert the SPECIFIC verdict unconditionally. The previous form + // guarded on `if (skipped)`, so it passed vacuously whenever the entry was + // absent from the results entirely — the exact failure this test exists to + // catch. const skipped = result.find((r) => canonicalPath(r.path) === canonicalPath(wtDir)); - if (skipped) { - assert.notEqual(skipped.status, 'reaped', 'live-pid worktree must not be reaped'); - } + assert.ok(skipped, 'live-pid worktree must appear in the results'); + assert.equal(skipped.status, 'skipped', 'live-pid worktree must not be reaped'); + assert.equal(skipped.reason, 'pid_alive', 'reason must be pid_alive'); assert.ok(fs.existsSync(wtDir), 'worktree directory must still exist for live-pid worktree'); }); @@ -3938,10 +3939,12 @@ describe('bug-3707: reapOrphanWorktrees', () => { // ensuring the unmerged-branch check is the only reason the entry is skipped. const result = reapOrphanWorktrees(repoDir, { mtimeSafe: () => STALE_MTIME }); + // #3057: unconditional verdict assertion — the old `if (entry)` form passed + // vacuously when no row was produced at all. const entry = result.find((r) => canonicalPath(r.path) === canonicalPath(wtDir)); - if (entry) { - assert.notEqual(entry.status, 'reaped', 'unmerged worktree must not be reaped (data loss guard)'); - } + assert.ok(entry, 'unmerged worktree must appear in the results'); + assert.equal(entry.status, 'skipped', 'unmerged worktree must not be reaped (data loss guard)'); + assert.equal(entry.reason, 'branch_not_merged', 'reason must be branch_not_merged'); assert.ok(fs.existsSync(wtDir), 'unmerged worktree directory must still exist'); }); @@ -3967,10 +3970,12 @@ describe('bug-3707: reapOrphanWorktrees', () => { // within 5 minutes) which is fragile on heavily-loaded CI hosts. const result = reapOrphanWorktrees(repoDir, { mtimeSafe: () => FRESH_MTIME }); + // #3057: unconditional verdict assertion — the old `if (entry)` form passed + // vacuously when no row was produced at all. const entry = result.find((r) => canonicalPath(r.path) === canonicalPath(wtDir)); - if (entry) { - assert.notEqual(entry.status, 'reaped', 'fresh-mtime worktree must not be reaped (race guard)'); - } + assert.ok(entry, 'fresh-mtime worktree must appear in the results'); + assert.equal(entry.status, 'skipped', 'fresh-mtime worktree must not be reaped (race guard)'); + assert.equal(entry.reason, 'lock_too_fresh', 'reason must be lock_too_fresh'); assert.ok(fs.existsSync(wtDir), 'fresh-lock worktree directory must still exist'); }); @@ -4189,16 +4194,12 @@ describe('bug-3707: reapOrphanWorktrees — adversarial edge cases', () => { // ensuring the non-numeric content check is the only reason the entry is skipped. const result = reapOrphanWorktrees(repoDir, { mtimeSafe: () => STALE_MTIME }); + // #3057: unconditional verdict assertion — the old `if (entry)` form passed + // vacuously when no row was produced at all. const entry = result.find((r) => canonicalPath(r.path) === canonicalPath(wtDir)); - if (entry) { - assert.notEqual( - entry.status, - 'reaped', - 'non-numeric Claude Code lock must NOT be reaped (fail-closed: owner unknown)' - ); - assert.equal(entry.status, 'skipped', 'non-numeric lock entry should have status=skipped'); - assert.equal(entry.reason, 'lock_owner_unknown', 'reason must be lock_owner_unknown'); - } + assert.ok(entry, 'Claude-Code-locked worktree must appear in the results'); + assert.equal(entry.status, 'skipped', 'non-numeric lock entry should have status=skipped'); + assert.equal(entry.reason, 'lock_owner_unknown', 'reason must be lock_owner_unknown'); assert.ok(fs.existsSync(wtDir), 'worktree with Claude Code lock must NOT be removed'); }); @@ -4232,10 +4233,13 @@ describe('bug-3707: reapOrphanWorktrees — adversarial edge cases', () => { mtimeSafe: () => STALE_MTIME, }); + // #3057: unconditional verdict assertion. An undeterminable owner takes the + // same fail-closed exit as a genuinely live one, so the reason string is + // pid_alive in both cases — production deliberately conflates them. const entry = result.find((r) => canonicalPath(r.path) === canonicalPath(wtDir)); - if (entry) { - assert.notEqual(entry.status, 'reaped', 'EPERM from isPidAlive must be treated as ALIVE — must not reap'); - } + assert.ok(entry, 'EPERM worktree must appear in the results'); + assert.equal(entry.status, 'skipped', 'EPERM from isPidAlive must be treated as ALIVE — must not reap'); + assert.equal(entry.reason, 'pid_alive', 'reason must be pid_alive'); assert.ok(fs.existsSync(wtDir), 'worktree must still exist when isPidAlive throws EPERM'); }); @@ -4282,11 +4286,12 @@ describe('bug-3707: reapOrphanWorktrees — adversarial edge cases', () => { // OR skip it for a safe reason — it must NOT return an empty result (which // would mean it bailed out entirely, silently skipping all orphan detection). assert.ok(Array.isArray(result), 'reapOrphanWorktrees must return an array'); - assert.ok(result.length > 0, 'reaper must not bail out entirely for trunk-default repos — must inspect the worktree'); + assert.equal(result.length, 1, 'reaper must inspect exactly the one worktree in this trunk-default repo — not bail out entirely, and not report extras'); const entry = result.find((r) => canonicalPath(r.path) === wtDirCanonical); assert.ok(entry, 'worktree must appear in results (reaped or skipped with reason)'); // The branch IS merged into trunk, and the PID is dead, so it should be reaped. assert.equal(entry.status, 'reaped', 'worktree with dead pid merged into trunk must be reaped'); + assert.equal(entry.reason, 'pid_dead_and_merged', 'reason must be pid_dead_and_merged (using trunk as the default branch)'); }); }); }); @@ -4755,8 +4760,7 @@ describe('bug #260: gsd-worktree-path-guard.js', () => { }; const result = runHook(worktreeDir, payload); assert.strictEqual(result.status, 2, `Expected exit 2 (block), got ${result.status}. stderr: ${result.stderr}`); - let parsed; - assert.doesNotThrow(() => { parsed = JSON.parse(result.stdout); }, 'stdout must be valid JSON'); + const parsed = JSON.parse(result.stdout); assert.strictEqual(parsed.decision, 'block', 'Expected decision:"block" in output'); }); @@ -5341,8 +5345,7 @@ describe('#1342 — GSD-activity gate + fail-open for no-repo targets', () => { `GSD-managed worktree targeting main repo root must be blocked (exit 2). ` + `Got exit ${result.status}. stderr: ${result.stderr}` ); - let parsed; - assert.doesNotThrow(() => { parsed = JSON.parse(result.stdout); }, 'stdout must be valid JSON'); + const parsed = JSON.parse(result.stdout); assert.strictEqual(parsed.decision, 'block', 'Expected decision:"block" in output'); }); @@ -5407,8 +5410,7 @@ describe('#1342 — GSD-activity gate + fail-open for no-repo targets', () => { `GSD-managed worktree targeting .git/config of another repo must be blocked (exit 2). ` + `Got exit ${result.status}. stderr: ${result.stderr}` ); - let parsed; - assert.doesNotThrow(() => { parsed = JSON.parse(result.stdout); }, 'stdout must be valid JSON'); + const parsed = JSON.parse(result.stdout); assert.strictEqual(parsed.decision, 'block', 'Expected decision:"block" in output'); assert.ok( parsed.reason && parsed.reason.includes('.git'),