refactor(#3103): give the reap and worktree-info paths the seam their callers already have

Twenty-two branches in the orphan-reaping path are unreachable from the test
suite, and the reason is not that they are hard to reach — it is that nothing
can reach them. The reaper already accepts an injectable dependency bag, but
every test drives real git and injects only the clock and the liveness probe,
so each fail-closed return inside it has never executed under test.

The two entry points above it took no dependencies at all, so a test could not
drive them even if it wanted to. Both now accept the same bag and thread it
down, with stdout and stderr writers defaulting to the process streams. The
worktree-info probe in the base-branch resolver called its git seam directly
while a sibling function in the same file already modelled the injectable
form; it now follows that sibling rather than inventing a second convention.

Every parameter defaults to today's real implementation, so no existing caller
changes behavior. This is a testability seam, not a redesign.

Two guards are removed as genuinely dead, each excluded by a check a few lines
above it. A NaN test on a value captured by a digits-only pattern cannot fire,
because parseInt of digits is never NaN. An emptiness test on a capture group
that matched one-or-more non-space characters cannot fire either. Each site
keeps a one-line note naming the guard that excludes it, so neither gets
restored by a future reader.

A third guard was proposed for deletion on the same grounds and is NOT removed,
because the claim was wrong. The local-branch fallback returns null when git
prints output that is non-empty but names neither branch — the emptiness check
above it only catches the empty string, so a single newline reaches the
fallback with both flags false. Deleting it would have changed which branch the
resolver reports. It stays, and it gets a test.

Refs #3057

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
sim
2026-08-06 00:32:28 -04:00
parent c7da62b682
commit c2d5b528e5
2 changed files with 23 additions and 14 deletions

View File

@@ -136,7 +136,8 @@ export function tryRemoteShow(
const branch = m[1];
// git emits "(unknown)" when the remote is offline but the local cache
// resolved it; treat that as non-authoritative and fall through.
if (!branch || branch === '(unknown)') return null;
// No `!branch ||` guard: m[1] comes from the `(\S+)` capture group above, so it is never empty.
if (branch === '(unknown)') return null;
return branch;
} catch {
return null;
@@ -271,9 +272,13 @@ export interface GitWorktreeInfo {
* Detect whether `cwd` sits inside a git worktree, and if so, return the
* absolute path of the worktree root.
*/
export function gitWorktreeInfoInternal(cwd: string): GitWorktreeInfo {
export function gitWorktreeInfoInternal(
cwd: string,
deps?: Pick<BaseBranchDeps, 'execGit'>
): GitWorktreeInfo {
const execGit: ExecGitFn = deps?.execGit ?? execGitSeam;
try {
const insideResult = execGitSeam(['rev-parse', '--is-inside-work-tree'], { cwd, timeout: 5000 });
const insideResult = execGit(['rev-parse', '--is-inside-work-tree'], { cwd, timeout: 5000 });
if (insideResult.exitCode !== 0) {
return { inside: false, worktreeRoot: null };
}
@@ -281,7 +286,7 @@ export function gitWorktreeInfoInternal(cwd: string): GitWorktreeInfo {
if (insideStdout !== 'true') {
return { inside: false, worktreeRoot: null };
}
const rootResult = execGitSeam(['rev-parse', '--show-toplevel'], { cwd, timeout: 5000 });
const rootResult = execGit(['rev-parse', '--show-toplevel'], { cwd, timeout: 5000 });
if (rootResult.exitCode !== 0) {
return { inside: true, worktreeRoot: null };
}

View File

@@ -1739,7 +1739,8 @@ function reapOrphanWorktrees(repoRoot: string, deps: WorktreeDeps = {}): ReapRes
const pid = parseInt(pidStr, 10);
let pidIsAlive: boolean;
try {
pidIsAlive = Number.isNaN(pid) || isPidAliveCheck(pid);
// No `Number.isNaN(pid) ||` guard: pidStr is captured by /^\d+/ above, so pid is never NaN.
pidIsAlive = isPidAliveCheck(pid);
} catch {
pidIsAlive = true; // Cannot determine liveness — treat as alive, do not reap.
}
@@ -1825,21 +1826,23 @@ function defaultMtimeSafe(file: string): Date | null {
try { return fs.statSync(file).mtime; } catch { return null; }
}
function cmdWorktreeReapOrphans(cwd: string): void {
function cmdWorktreeReapOrphans(cwd: string, deps: RecordAgentCmdDeps & WorktreeDeps = {}): void {
const write = deps.write || ((s: string) => process.stdout.write(s));
const writeErr = deps.writeErr || ((s: string) => process.stderr.write(s));
let result: ReapResult[];
try {
result = reapOrphanWorktrees(cwd);
result = reapOrphanWorktrees(cwd, deps);
} catch (err) {
// Surface failure as a one-line warning; keep exit-zero so workflows don't break.
process.stderr.write(`[gsd] worktree.reap-orphans failed: ${err && (err as Error).message ? (err as Error).message : String(err)}\n`);
writeErr(`[gsd] worktree.reap-orphans failed: ${err && (err as Error).message ? (err as Error).message : String(err)}\n`);
result = [];
}
const skippedCount = result.filter((r) => r.status === 'skipped').length;
if (skippedCount > 0) {
// Surface skipped entries so operators are aware of unresolved orphans.
process.stderr.write(`[gsd] worktree.reap-orphans: ${skippedCount} orphan(s) skipped (run with DEBUG=1 for details)\n`);
writeErr(`[gsd] worktree.reap-orphans: ${skippedCount} orphan(s) skipped (run with DEBUG=1 for details)\n`);
}
process.stdout.write(`${JSON.stringify({ ok: true, reaped: result.filter((r) => r.status === 'reaped').length, entries: result }, null, 2)}\n`);
write(`${JSON.stringify({ ok: true, reaped: result.filter((r) => r.status === 'reaped').length, entries: result }, null, 2)}\n`);
}
// Unused exports kept for API compatibility
@@ -1875,16 +1878,17 @@ function resolveWorktreeRoot(cwd: string, deps: WorktreeDeps = {}): { root: stri
* the repository; used as `cwd` for git commands.
* @returns list of worktree paths that were removed (always empty)
*/
function pruneOrphanedWorktrees(repoRoot: string): string[] {
function pruneOrphanedWorktrees(repoRoot: string, deps: WorktreeDeps & { writeErr?: (s: string) => void } = {}): string[] {
const writeErr = deps.writeErr || ((s: string) => process.stderr.write(s));
try {
const plan = planWorktreePrune(
repoRoot,
{ allowDestructive: false },
{ parseWorktreePorcelain }
{ parseWorktreePorcelain, ...deps }
);
const pruneResult = executeWorktreePrunePlan(plan) as { timedOut?: boolean } | null;
const pruneResult = executeWorktreePrunePlan(plan, deps) as { timedOut?: boolean } | null;
if (pruneResult && pruneResult.timedOut) {
process.stderr.write(
writeErr(
'[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'