Files
msd-core/tests/bug-3281-worktree-git-timeout.test.cjs
Tom Boucher 639e4d603a refactor(shell-projection): migrate all subprocess call sites to exec*/probeTty seam (Phase 2, #3466) (#3476)
* refactor(shell-projection): migrate planning-workspace.cjs tty probe to probeTty seam (#3466)

Replaces direct execFileSync('tty') with probeTty() from the shell-projection
seam. Removes try/catch — probeTty() returns null on error/non-tty/win32.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): migrate commands.cjs to execGit seam (#3466)

- Replaces execSync('git diff --cached --name-only') with execGit array call
- Migrates 14 existing execGit(cwd, args) callers from core.cjs's local
  wrapper to the seam's execGit(args, { cwd }) signature
- Drops execGit from the core.cjs destructure to resolve naming collision

Drops try/catch around git diff — execGit returns exitCode without throwing,
so the no-staged-files / not-a-git-repo case is detected by exitCode !== 0.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): migrate check-latest-version.cjs to execNpm seam (#3466)

Routes the default-spawn path through execNpm — execNpm owns the win32
shell-flag policy. The injection point remains spawnSync-shaped for test
compatibility; an internal adapter translates { exitCode } → { status }.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): migrate init.cjs git calls to execGit seam (#3466)

Replaces 3 execSync calls with execGit array-args:
- detectChildRepos: git status --porcelain
- cmdInitNewWorkspace: git --version (worktree availability probe)
- cmdRemoveWorkspace: git status --porcelain

Drops 3 try/catch blocks — execGit returns exitCode without throwing, so
best-effort handling becomes a clean exitCode === 0 check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): migrate core.cjs to execGit seam delegation (#3466)

- Removes direct require('child_process') from core.cjs
- Replaces execFileSync('git check-ignore') with seam's execGit
- Local execGit wrapper now a thin adapter delegating to seam — keeps the
  legacy (cwd, args) positional signature and derived timedOut field for
  the verify.cjs and worktree-safety.cjs consumers that are out of Phase 2
  scope (the wrapper proper would only be removed once those consumers
  migrate, tracked separately)

Extends the seam's _spawnResult to expose signal and error fields so callers
can compute timedOut without bypassing the seam. The Phase 1 test suite
asserts on required field presence only, so the extension is
backward-compatible.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): migrate graphify.cjs to execTool seam (#3466)

- execGraphify: spawnSync('graphify', ...) → execTool with env passthrough,
  preserving the ENOENT/TIMEOUT/EXIT_NONZERO typed reason mapping using the
  seam's signal/error fields
- checkGraphifyInstalled: spawnSync('graphify', ['--help']) → execTool
- checkGraphifyVersion strategy 1: graphify --version via execTool
- checkGraphifyVersion strategy 2: python3 importlib.metadata via execTool

Adds env option to execTool — graphify needs PYTHONUNBUFFERED=1 to drain
buffered stdout on long-running operations.

Changes seam internals to access spawnSync/execFileSync via the non-destructured
childProcess module reference. Destructured imports capture references at load
time and are un-mockable by mock.method(childProcess, 'spawnSync', ...) — which
breaks all the graphify subprocess tests. Non-destructured access restores
mockability without changing public behavior.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(shell-projection): execGit defaults to non-interactive git env (#3466)

Bakes GIT_TERMINAL_PROMPT=0 and GCM_INTERACTIVE=never into execGit's default
env. Without these, a credential prompt or terminal-input probe blocks the
git subprocess indefinitely until our 10s timeout kills it — surfacing as
a generic timeout instead of the actual auth-prompt cause.

These were previously set ad-hoc in worktree-safety.cjs's local execGitDefault
wrapper. Moving them to the seam makes them the consistent default for every
git call across the codebase. Callers can override via opts.env.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): remove core.cjs local execGit wrapper; migrate verify + worktree-safety (#3466)

Completes the Phase 2 "remove local execGit wrapper" criterion. All callers
now use the shell-projection seam's execGit(args, opts) signature directly.

- core.cjs: delete the local execGit wrapper and the execGit export. The
  isGitIgnored seam check now calls execGit from the seam. Worktree-safety
  function calls drop their execGit DI passthrough — worktree-safety's
  internal execGitDefault now delegates to the seam and adds timedOut.
- verify.cjs: 6 callers migrate from execGit(cwd, args) to
  execGit(args, { cwd }). Imports execGit from the seam directly. The
  inspectWorktreeHealth DI passes the seam's execGit (worktree-safety now
  matches that shape).
- worktree-safety.cjs: local execGitDefault becomes a thin adapter over
  the seam — no more direct spawnSync. 11 internal callers migrate to the
  new shape. DI contract for tests changes from (cwd, args) → (args, opts).
- graphify.cjs: 2 remaining execGit callers migrate from core.cjs (now
  removed) to the seam directly.
- test mocks updated in 3 worktree-safety test files to match the new
  (args, opts) DI shape — most mocks were shape-agnostic and required no
  changes; only those that destructured cwd/args needed updates.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(changeset): add entry for shell-projection Phase 2 migration (#3466)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(pr3476): address CodeRabbit review findings

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-13 18:33:32 -04:00

263 lines
10 KiB
JavaScript

/**
* Regression tests for #3281:
* Worktree health paths can hang indefinitely due to unbounded git subprocess calls.
*
* Acceptance criteria:
* AC1 — Worktree git subprocess calls use bounded execution (timeout + deterministic failure).
* AC2 — Timeout/failure outcomes produce structured non-fatal warning signals.
* AC3 — validate health and init progress remain non-crashing when git is unavailable/stalled,
* but report degraded worktree health-check status.
* AC4 — Regression tests cover timeout/degraded-git behavior for worktree safety checks.
*/
'use strict';
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const path = require('path');
// ─── Module paths ─────────────────────────────────────────────────────────────
const WORKTREE_SAFETY_PATH = path.join(
__dirname, '..', 'get-shit-done', 'bin', 'lib', 'worktree-safety.cjs'
);
// ─── Shared timeout stub ──────────────────────────────────────────────────────
/**
* Returns an execGit stub that simulates what spawnSync returns when the
* subprocess is killed by SIGTERM after exceeding its timeout option.
* Per Node.js docs: result.status === null, result.signal === 'SIGTERM',
* result.error?.code === 'ETIMEDOUT'.
*
* The production execGit implementation must detect this shape and:
* - return { ..., timedOut: true } so callers can distinguish timeout from auth failure
* - not throw
*/
function makeTimeoutStub() {
return function stubTimedOutExecGit(_args, _opts) {
return {
exitCode: null,
stdout: '',
stderr: '',
timedOut: true,
signal: 'SIGTERM',
error: Object.assign(new Error('spawnSync git ETIMEDOUT'), { code: 'ETIMEDOUT' }),
};
};
}
// ─── AC1 / AC4: degraded health via exported functions ───────────────────────
describe('bug-3281 AC1: worktree functions return degraded-ok on timeout, not throw', () => {
test('planWorktreePrune returns action=skip when execGit times out', () => {
const { planWorktreePrune } = require(WORKTREE_SAFETY_PATH);
let threw = false;
let result;
try {
result = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() });
} catch {
threw = true;
}
assert.strictEqual(threw, false, 'planWorktreePrune must not throw on timeout');
assert.strictEqual(typeof result, 'object', 'planWorktreePrune must return an object');
assert.strictEqual(result.action, 'skip', 'planWorktreePrune must return action=skip when git times out');
assert.ok(
typeof result.reason === 'string' && result.reason.length > 0,
'planWorktreePrune must return a non-empty reason when git times out'
);
});
test('executeWorktreePrunePlan returns ok:false when plan is skip (timeout path)', () => {
const { planWorktreePrune, executeWorktreePrunePlan } = require(WORKTREE_SAFETY_PATH);
const plan = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() });
const result = executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() });
assert.strictEqual(typeof result, 'object', 'executeWorktreePrunePlan must return an object');
assert.strictEqual(result.ok, false, 'executeWorktreePrunePlan must return ok:false on timeout');
});
test('inspectWorktreeHealth returns ok:false when git times out', () => {
const { inspectWorktreeHealth } = require(WORKTREE_SAFETY_PATH);
let threw = false;
let result;
try {
result = inspectWorktreeHealth('/tmp', {}, { execGit: makeTimeoutStub() });
} catch {
threw = true;
}
assert.strictEqual(threw, false, 'inspectWorktreeHealth must not throw on timeout');
assert.strictEqual(typeof result, 'object');
assert.strictEqual(result.ok, false, 'inspectWorktreeHealth must return ok:false on timeout');
});
test('listLinkedWorktreePaths returns ok:false on timeout, not throw', () => {
const { listLinkedWorktreePaths } = require(WORKTREE_SAFETY_PATH);
let threw = false;
let result;
try {
result = listLinkedWorktreePaths('/tmp', { execGit: makeTimeoutStub() });
} catch {
threw = true;
}
assert.strictEqual(threw, false, 'listLinkedWorktreePaths must not throw on timeout');
assert.strictEqual(result.ok, false, 'listLinkedWorktreePaths must return ok:false on timeout');
assert.ok(
typeof result.reason === 'string' && result.reason.length > 0,
'listLinkedWorktreePaths must return non-empty reason on timeout'
);
});
test('snapshotWorktreeInventory returns ok:false with reason on timeout, not throw', () => {
const { snapshotWorktreeInventory } = require(WORKTREE_SAFETY_PATH);
let threw = false;
let result;
try {
result = snapshotWorktreeInventory('/tmp', {}, { execGit: makeTimeoutStub() });
} catch {
threw = true;
}
assert.strictEqual(threw, false, 'snapshotWorktreeInventory must not throw on timeout');
assert.strictEqual(typeof result, 'object');
assert.strictEqual(result.ok, false, 'snapshotWorktreeInventory must return ok:false on timeout');
assert.ok(
typeof result.reason === 'string' && result.reason.length > 0,
'snapshotWorktreeInventory must return non-empty reason on timeout'
);
});
test('resolveWorktreeContext returns a valid result on timeout, not throw', () => {
const { resolveWorktreeContext } = require(WORKTREE_SAFETY_PATH);
let threw = false;
let result;
try {
result = resolveWorktreeContext('/tmp', { execGit: makeTimeoutStub() });
} catch {
threw = true;
}
assert.strictEqual(threw, false, 'resolveWorktreeContext must not throw on timeout');
assert.strictEqual(typeof result, 'object');
assert.ok(
typeof result.effectiveRoot === 'string',
'resolveWorktreeContext must return effectiveRoot string even on timeout'
);
});
});
// ─── AC2 / AC4: timedOut is a first-class field in results ───────────────────
describe('bug-3281 AC2+AC4: timedOut is a first-class field in results', () => {
test('planWorktreePrune reason is git_timed_out when execGit returns timedOut:true', () => {
const { planWorktreePrune } = require(WORKTREE_SAFETY_PATH);
const result = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() });
// AC4 strict: must use the specific reason string 'git_timed_out'
// (not the generic 'git_list_failed') to distinguish timeout from auth failure
assert.strictEqual(
result.reason,
'git_timed_out',
[
'AC4 (strict): planWorktreePrune must use reason=git_timed_out',
'when execGit returns timedOut:true — not the generic git_list_failed',
].join(' ')
);
});
test('listLinkedWorktreePaths reason is git_timed_out when execGit returns timedOut:true', () => {
const { listLinkedWorktreePaths } = require(WORKTREE_SAFETY_PATH);
const result = listLinkedWorktreePaths('/tmp', { execGit: makeTimeoutStub() });
assert.strictEqual(
result.reason,
'git_timed_out',
[
'AC4 (strict): listLinkedWorktreePaths must use reason=git_timed_out',
'when execGit returns timedOut:true',
].join(' ')
);
});
test('executeWorktreePrunePlan result.timedOut is true when prune git call times out', () => {
const { executeWorktreePrunePlan } = require(WORKTREE_SAFETY_PATH);
// Use a plan that bypasses readWorktreeList (action=metadata_prune_only)
// so the prune execGit call itself can time out
const plan = {
repoRoot: '/tmp',
action: 'metadata_prune_only',
reason: 'no_worktrees',
destructiveModeRequested: false,
};
const result = executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() });
assert.strictEqual(result.ok, false, 'executeWorktreePrunePlan must return ok:false when prune times out');
// AC4 strict: timedOut must be surfaced as a first-class field
assert.strictEqual(
result.timedOut,
true,
[
'AC4 (strict): executeWorktreePrunePlan must include timedOut:true in result',
'when the execGit call returns timedOut:true',
].join(' ')
);
});
test('snapshotWorktreeInventory reason is git_timed_out on timeout', () => {
const { snapshotWorktreeInventory } = require(WORKTREE_SAFETY_PATH);
const result = snapshotWorktreeInventory('/tmp', {}, { execGit: makeTimeoutStub() });
assert.strictEqual(
result.reason,
'git_timed_out',
[
'AC4 (strict): snapshotWorktreeInventory must use reason=git_timed_out',
'when execGit returns timedOut:true',
].join(' ')
);
});
});
// ─── AC3: non-crashing under degraded git — worktree prune flow ───────────────
describe('bug-3281 AC3: worktree prune flow is non-crashing under degraded git', () => {
test('full prune flow (plan -> execute) completes without throwing on timeout', () => {
const { planWorktreePrune, executeWorktreePrunePlan } = require(WORKTREE_SAFETY_PATH);
let threw = false;
try {
const plan = planWorktreePrune('/tmp', {}, { execGit: makeTimeoutStub() });
executeWorktreePrunePlan(plan, { execGit: makeTimeoutStub() });
} catch {
threw = true;
}
assert.strictEqual(threw, false, 'full prune flow must not throw on timeout — must degrade gracefully');
});
test('inspectWorktreeHealth findings is empty array (not undefined) on timeout', () => {
const { inspectWorktreeHealth } = require(WORKTREE_SAFETY_PATH);
const result = inspectWorktreeHealth('/tmp', {}, { execGit: makeTimeoutStub() });
// ok:false is expected — but findings must still be an array (not undefined)
// so callers that iterate findings do not crash
assert.strictEqual(Array.isArray(result.findings), true, 'findings must be an array even when ok:false');
});
});