diff --git a/.changeset/quick-birds-munch.md b/.changeset/quick-birds-munch.md new file mode 100644 index 000000000..fad095b21 --- /dev/null +++ b/.changeset/quick-birds-munch.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3077 +--- +**`execGit` now reports `timedOut` on every result, and its return type is no longer misdeclared** — three modules hand-copied the shape of `execGit`'s result because the canonical type was not exported, and two of those copies declared `exitCode` as nullable when it can never be null. The shape is now declared once and reused, so a consumer can no longer be written against a contract the function does not honor. (#3071) diff --git a/src/check-command-router.cts b/src/check-command-router.cts index 556cabe75..c74c66ad9 100644 --- a/src/check-command-router.cts +++ b/src/check-command-router.cts @@ -968,7 +968,7 @@ function buildPredicateDeps() { stdout: r.stdout, stderr: r.stderr, signal: r.signal, - timedOut: r.signal === 'SIGTERM', + timedOut: r.timedOut, }; }, findPhaseArtifact(phaseDir: string, artifactSuffix: string): string | null { diff --git a/src/git-base-branch.cts b/src/git-base-branch.cts index 4b560ca28..1a0c36cb8 100644 --- a/src/git-base-branch.cts +++ b/src/git-base-branch.cts @@ -29,10 +29,7 @@ import { execGit as execGitSeam } from './shell-command-projection.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── -type ExecGitFn = ( - args: string[], - opts?: { cwd?: string; env?: Record; timeout?: number } -) => { exitCode: number | null; stdout: string; stderr: string; signal: string | null; error: unknown }; +type ExecGitFn = typeof execGitSeam; export interface BaseBranchDeps { /** Override the git runner (default: execGit from shell-command-projection) */ diff --git a/src/graphify.cts b/src/graphify.cts index 2dd33ce87..c3a55d7e0 100644 --- a/src/graphify.cts +++ b/src/graphify.cts @@ -84,8 +84,12 @@ function execGraphify(cwd: string, args: string[], options: { timeout?: number } }; } - // Timeout — seam exposes signal; spawnSync sets SIGTERM when killed by timeout. - if (result.signal === 'SIGTERM') { + // Timeout — result.timedOut is derived by the shared isSpawnTimeout predicate + // (shell-command-projection.cts), keyed on error.code === 'ETIMEDOUT' rather + // than signal === 'SIGTERM': Windows does not reliably report SIGTERM on a + // timeout kill, and an externally-delivered SIGTERM (error is null) is not + // a timeout at all. + if (result.timedOut) { return { exitCode: 124, stdout: result.stdout, diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index 801d580ea..3ccaad438 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -491,12 +491,13 @@ export function projectPersistentPathExportActions({ targetDir, platform = proce // ─── Subprocess dispatch ────────────────────────────────────────────────────── -interface SpawnResultOutput { +export interface SpawnResultOutput { exitCode: number; stdout: string; stderr: string; signal: NodeJS.Signals | null; error: Error | null; + timedOut: boolean; } /** @@ -522,14 +523,20 @@ export function isSpawnTimeout(result: { error?: unknown }): boolean { function _spawnResult(result: { error?: NodeJS.ErrnoException | null; status?: number | null; stdout?: Buffer | string | null; stderr?: Buffer | string | null; signal?: NodeJS.Signals | null }, program: string): SpawnResultOutput { if (result.error && result.error.code === 'ENOENT') { - return { exitCode: 127, stdout: '', stderr: `${program}: not found`, signal: null, error: result.error }; + return { exitCode: 127, stdout: '', stderr: `${program}: not found`, signal: null, error: result.error, timedOut: false }; } + const signal = result.signal ?? null; + const error = result.error ?? null; return { exitCode: result.status ?? 1, stdout: (result.stdout ?? '').toString().trim(), stderr: (result.stderr ?? '').toString().trim(), - signal: result.signal ?? null, - error: result.error ?? null, + signal, + error, + // Reuse the single shared timeout predicate (isSpawnTimeout, below) rather + // than re-deriving it here — see that function's docstring for why only + // error.code === 'ETIMEDOUT' is checked (not signal === 'SIGTERM'). + timedOut: isSpawnTimeout({ error }), }; } diff --git a/src/worktree-base-ref.cts b/src/worktree-base-ref.cts index 45a47120c..f2350677f 100644 --- a/src/worktree-base-ref.cts +++ b/src/worktree-base-ref.cts @@ -85,10 +85,7 @@ function parseJsonc(text: string): unknown { // ─── Internal types ─────────────────────────────────────────────────────────── -type ExecGitFn = ( - args: string[], - opts?: { cwd?: string; env?: Record; timeout?: number } -) => { exitCode: number | null; stdout: string; stderr: string; signal: string | null; error: unknown }; +type ExecGitFn = typeof execGitSeam; // ─── Message constants (verbatim — downstream docs/tests depend on these) ───── diff --git a/src/worktree-safety.cts b/src/worktree-safety.cts index 64896bad6..26433a046 100644 --- a/src/worktree-safety.cts +++ b/src/worktree-safety.cts @@ -10,7 +10,7 @@ import fs from 'node:fs'; import path from 'node:path'; -import { execGit as execGitSeam, posixNormalize, isSpawnTimeout } from './shell-command-projection.cjs'; +import { execGit as execGitSeam, posixNormalize, type SpawnResultOutput } from './shell-command-projection.cjs'; // Default timeout for worktree-related git subprocess calls. // 10 s is generous enough for normal git operations on large repos while still @@ -21,31 +21,22 @@ const DEFAULT_GIT_TIMEOUT_MS = 10000; const WORKTREE_AGENT_BRANCH_RE = /^(worktree-)?agent-[A-Za-z0-9._/-]+$/; const WORKTREE_AGENT_BRANCH_PATTERN = WORKTREE_AGENT_BRANCH_RE.source; -interface GitResult { - exitCode: number; - stdout: string; - stderr: string; - signal?: string | null; - error?: NodeJS.ErrnoException | null; - timedOut: boolean; -} +// GitResult now aliases the canonical SpawnResultOutput (shell-command-projection.cts), +// which already carries `timedOut` — kept as a local name since it is referenced +// below (gitResultOk). +type GitResult = SpawnResultOutput; -type ExecGitFn = (args: string[], opts?: { cwd?: string; timeout?: number }) => GitResult; +type ExecGitFn = typeof execGitSeam; /** - * Execute a git command via the shell-projection seam, with a derived - * `timedOut` field. Tests inject mocks via deps.execGit using the new + * Execute a git command via the shell-projection seam, applying the module's + * default timeout. `timedOut` is now derived by the seam itself + * (shell-command-projection.cts's `_spawnResult`), so this is a thin + * passthrough. Tests inject mocks via deps.execGit using the same * (args, opts) shape — see worktree-safety-policy.test.cjs. - * - * Return shape: { exitCode, stdout, stderr, timedOut, error, signal } - * - timedOut: derived via the shared `isSpawnTimeout` predicate - * (shell-command-projection.cts) — true when spawnSync's `error.code` - * is `ETIMEDOUT`; does not require `signal === 'SIGTERM'` (#3050). */ -function execGitDefault(args: string[], opts: { cwd?: string; timeout?: number } = {}): GitResult { - const result = execGitSeam(args, { ...opts, timeout: opts.timeout ?? DEFAULT_GIT_TIMEOUT_MS }); - const timedOut = isSpawnTimeout(result); - return { ...result, timedOut }; +function execGitDefault(args: string[], opts: { cwd?: string; env?: Record; timeout?: number } = {}): GitResult { + return execGitSeam(args, { ...opts, timeout: opts.timeout ?? DEFAULT_GIT_TIMEOUT_MS }); } interface WorktreeBranchEntry { diff --git a/tests/faulty-deps.test.cjs b/tests/faulty-deps.test.cjs new file mode 100644 index 000000000..ba36aebf3 --- /dev/null +++ b/tests/faulty-deps.test.cjs @@ -0,0 +1,322 @@ +'use strict'; + +/** + * Phase 2 test matrix for issue #3056 (fault-injection adapter) and #3071 + * (execGit normalization, folded in). See + * .gsd/phase/test-3056-fault-injection-adapter/50-test-matrix.md. + * + * Section D (row 24 — injection through the process seam is unsupported) has + * no runtime assertion: it is documented in tests/helpers/faulty-deps.cjs's + * module JSDoc and in CONTRIBUTING.md, per the matrix's own note that this + * row is "asserted by review + absence, not a runtime test." Likewise row 23 + * (no chmod anywhere in the helper) is asserted by review of + * tests/helpers/faulty-deps.cjs, not by a runtime test. + */ + +const { describe, test, mock } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const childProcess = require('node:child_process'); + +const { + execGit, + execNpm, + execTool, + isSpawnTimeout, +} = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'shell-command-projection.cjs')); + +const worktreeSafety = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'worktree-safety.cjs')); +const { trySymbolicRef } = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'git-base-branch.cjs')); +const { evaluateWorktreeBaseDegrade } = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'worktree-base-ref.cjs')); +const { defaultPhaseCleanCommitTimesMs } = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'verification.cjs')); + +const { createTempGitProject, createTempDir, cleanup } = require('./helpers.cjs'); +const { makeFaultyGit, withFaultyFs } = require('./helpers/faulty-deps.cjs'); + +// ─── A. execGit normalization (#3071) ────────────────────────────────────── + +describe('A. execGit normalization (#3071)', () => { + let tmpDir; + + test('1. execGit result carries timedOut', (t) => { + tmpDir = createTempGitProject(); + t.after(() => cleanup(tmpDir)); + const result = execGit(['status', '--porcelain'], { cwd: tmpDir }); + assert.strictEqual(result.timedOut, false); + assert.strictEqual(typeof result.exitCode, 'number'); + }); + + test('2. every exec* result carries timedOut', () => { + const npmResult = execNpm(['--version']); + const toolResult = execTool(process.execPath, ['--version']); + assert.strictEqual(typeof npmResult.timedOut, 'boolean'); + assert.strictEqual(npmResult.timedOut, false); + assert.strictEqual(typeof toolResult.timedOut, 'boolean'); + assert.strictEqual(toolResult.timedOut, false); + }); + + test('3. ENOENT path still sets timedOut:false', () => { + // Reached via a real execTool call to a nonexistent binary — the early + // ENOENT return in _spawnResult (shell-command-projection.cts) must not + // omit the field the rest of the shape now always carries. + const result = execTool('definitely-not-a-real-program-fault-3056', []); + assert.strictEqual(result.exitCode, 127); + assert.strictEqual(result.timedOut, false); + }); + + test('4. a timed-out git reports timedOut', (t) => { + tmpDir = createTempGitProject(); + t.after(() => cleanup(tmpDir)); + // timeout:1 is real (no mocking) — process creation alone cannot + // complete inside 1ms, so the kill path is deterministic, matching the + // existing "wall-clock timeout" pattern used for dispatchGsdCommand in + // shell-command-projection-dispatch.test.cjs. + const result = execGit(['status', '--porcelain'], { cwd: tmpDir, timeout: 1 }); + assert.strictEqual(result.timedOut, true); + assert.strictEqual(result.error && result.error.code, 'ETIMEDOUT'); + }); + + test('5. an external SIGTERM is not reported as a timeout', (t) => { + // Direct predicate check: a SIGTERM with no accompanying ETIMEDOUT error + // (the shape an externally-delivered kill produces) must not trip + // isSpawnTimeout — proves dropping the SIGTERM conjunct (#3050) did not + // widen the predicate into a false positive. + assert.strictEqual(isSpawnTimeout({ error: null }), false); + + // End-to-end: mock spawnSync to return the externally-killed shape and + // confirm the real execGit seam reports timedOut:false. Same + // mock.method(childProcess, 'spawnSync', ...) technique already + // established in shell-command-projection-dispatch.test.cjs for the + // Windows-shaped-timeout case, exercising the same non-destructured + // childProcess import for the opposite direction. + mock.method(childProcess, 'spawnSync', () => ({ + status: null, + stdout: '', + stderr: '', + signal: 'SIGTERM', + error: null, + })); + t.after(() => mock.restoreAll()); + + const result = execGit(['status', '--porcelain']); + assert.strictEqual(result.signal, 'SIGTERM'); + assert.strictEqual(result.timedOut, false); + }); + + test('6. execGit env opt actually reaches the child process', (t) => { + tmpDir = createTempGitProject(); + t.after(() => cleanup(tmpDir)); + // worktree-safety.cts's execGitDefault is now a thin passthrough to this + // exact seam (see worktree-safety.cts:38 docstring), and its widened opts + // type ({cwd, env, timeout} together) is proven at build time by the + // strict tsc build, not here. This test instead proves the runtime + // behavior the type describes: `git var GIT_EDITOR` reflects the + // GIT_EDITOR env var, so a sentinel value passed via opts.env must come + // back verbatim on stdout — a real proof env reaches the child, not a + // liveness check that passes regardless of whether env is wired through. + const result = execGit(['var', 'GIT_EDITOR'], { + cwd: tmpDir, + env: { GIT_EDITOR: 'fault-3056-sentinel-editor' }, + timeout: 5000, + }); + assert.strictEqual(result.exitCode, 0); + assert.strictEqual(result.stdout, 'fault-3056-sentinel-editor'); + }); + + test('7. exitCode is always a number', (t) => { + tmpDir = createTempGitProject(); + t.after(() => cleanup(tmpDir)); + const success = execGit(['status', '--porcelain'], { cwd: tmpDir }); + const enoent = execTool('definitely-not-a-real-program-fault-3056', []); + const timeout = execGit(['status', '--porcelain'], { cwd: tmpDir, timeout: 1 }); + assert.strictEqual(typeof success.exitCode, 'number'); + assert.strictEqual(typeof enoent.exitCode, 'number'); + assert.strictEqual(typeof timeout.exitCode, 'number'); + }); + + test('8. one git stub satisfies every ExecGitFn seam', (t) => { + tmpDir = createTempDir(); + t.after(() => cleanup(tmpDir)); + + // Single shared stub, deliberately configured with only the default + // benign passthrough — the point of this row is that ONE value is + // accepted everywhere, not what any one fault produces. + const sharedStub = makeFaultyGit(); + + // worktree-safety.cts:33 — via resolveWorktreeContext, the exported + // entry point that threads deps.execGit. + const wsResult = worktreeSafety.resolveWorktreeContext(tmpDir, { + execGit: sharedStub, + existsSync: () => false, + }); + assert.strictEqual(typeof wsResult.effectiveRoot, 'string'); + assert.strictEqual(typeof wsResult.mode, 'string'); + assert.strictEqual(typeof wsResult.reason, 'string'); + + // git-base-branch.cts:32 — trySymbolicRef takes execGit directly as its + // second positional argument (no deps wrapper). + assert.doesNotThrow(() => trySymbolicRef(tmpDir, sharedStub)); + + // worktree-base-ref.cts:88 — evaluateWorktreeBaseDegrade threads + // deps.execGit. + const wbrResult = evaluateWorktreeBaseDegrade({ execGit: sharedStub, cwd: tmpDir }); + assert.strictEqual(typeof wbrResult.shouldDegrade, 'boolean'); + assert.strictEqual(typeof wbrResult.reason, 'string'); + + // verification.cts:226 — defaultPhaseCleanCommitTimesMs takes execGitFn + // directly as its third positional argument, typed `= typeof execGit`. + assert.doesNotThrow(() => { + const map = defaultPhaseCleanCommitTimesMs(tmpDir, ['a.md', 'b.md'], sharedStub); + assert.ok(map instanceof Map); + }); + }); +}); + +// ─── B. FaultyGit ─────────────────────────────────────────────────────────── + +describe('B. FaultyGit', () => { + test('9. FaultyGit timeout trips isSpawnTimeout', () => { + const faultyGit = makeFaultyGit({ faults: [{ kind: 'timeout' }] }); + const result = faultyGit(['status']); + assert.strictEqual(result.exitCode, 1); + assert.strictEqual(result.timedOut, true); + assert.strictEqual(result.error && result.error.code, 'ETIMEDOUT'); + assert.strictEqual(isSpawnTimeout(result), true); + }); + + test('10. FaultyGit non-zero exit', () => { + const faultyGit = makeFaultyGit({ faults: [{ kind: 'exit', exitCode: 3, stderr: 'boom' }] }); + const result = faultyGit(['status']); + assert.strictEqual(result.exitCode, 3); + assert.strictEqual(result.stderr, 'boom'); + assert.strictEqual(result.timedOut, false); + }); + + test('11. FaultyGit spawn failure', () => { + const faultyGit = makeFaultyGit({ faults: [{ kind: 'spawnFail' }] }); + const result = faultyGit(['status']); + assert.strictEqual(result.exitCode, 127); + assert.strictEqual(result.error && result.error.code, 'ENOENT'); + assert.strictEqual(result.timedOut, false); + }); + + test('12. FaultyGit faults only what it was told to', () => { + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'timeout', when: ['worktree', 'list'] }], + }); + const unmatched = faultyGit(['status', '--porcelain']); + assert.strictEqual(unmatched.timedOut, false); + assert.strictEqual(unmatched.exitCode, 0); + }); + + test('13. FaultyGit scopes a fault to one argv', () => { + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'exit', exitCode: 9, when: ['worktree', 'list'] }], + }); + const matched = faultyGit(['worktree', 'list', '--porcelain']); + const unmatched = faultyGit(['status', '--porcelain']); + assert.strictEqual(matched.exitCode, 9); + assert.strictEqual(unmatched.exitCode, 0); + }); + + test('14. FaultyGit faults the Nth call only', () => { + const faultyGit = makeFaultyGit({ + faults: [{ kind: 'exit', exitCode: 5, onCall: 2 }], + }); + const first = faultyGit(['rev-parse', 'HEAD']); + const second = faultyGit(['rev-parse', 'HEAD']); + const third = faultyGit(['rev-parse', 'HEAD']); + assert.strictEqual(first.exitCode, 0); + assert.strictEqual(second.exitCode, 5); + assert.strictEqual(third.exitCode, 0); + }); + + test('15. FaultyGit records its calls', () => { + const faultyGit = makeFaultyGit(); + faultyGit(['rev-parse', 'HEAD'], { cwd: '/a' }); + faultyGit(['status'], { cwd: '/b' }); + assert.strictEqual(faultyGit.calls.length, 2); + assert.deepStrictEqual(faultyGit.calls[0].args, ['rev-parse', 'HEAD']); + assert.strictEqual(faultyGit.calls[0].opts.cwd, '/a'); + assert.deepStrictEqual(faultyGit.calls[1].args, ['status']); + assert.strictEqual(faultyGit.calls[1].opts.cwd, '/b'); + }); + + test('16. FaultyGit result is a valid execGit result', () => { + const faultyGit = makeFaultyGit({ faults: [{ kind: 'timeout' }] }); + const result = faultyGit(['status']); + for (const key of ['exitCode', 'stdout', 'stderr', 'signal', 'error', 'timedOut']) { + assert.ok(Object.prototype.hasOwnProperty.call(result, key), `missing ${key}`); + } + assert.strictEqual(typeof result.exitCode, 'number'); + assert.strictEqual(typeof result.stdout, 'string'); + assert.strictEqual(typeof result.stderr, 'string'); + assert.strictEqual(typeof result.timedOut, 'boolean'); + }); +}); + +// ─── C. FaultyFs ──────────────────────────────────────────────────────────── + +describe('C. FaultyFs', () => { + test('17. FaultyFs read throws', () => { + const fs = require('node:fs'); + const injected = new Error('injected read failure'); + withFaultyFs({ readFileSync: () => { throw injected; } }, () => { + assert.throws(() => fs.readFileSync('/whatever'), /injected read failure/); + }); + }); + + test('18. FaultyFs write throws', () => { + const fs = require('node:fs'); + const injected = new Error('injected write failure'); + withFaultyFs({ writeFileSync: () => { throw injected; } }, () => { + assert.throws(() => fs.writeFileSync('/whatever', 'x'), /injected write failure/); + }); + }); + + test('19. FaultyFs restores on success', () => { + const fs = require('node:fs'); + const original = fs.readFileSync; + withFaultyFs({ readFileSync: () => { throw new Error('injected'); } }, () => { + assert.notStrictEqual(fs.readFileSync, original); + }); + assert.strictEqual(fs.readFileSync, original); + }); + + test('20. FaultyFs restores when the body throws', () => { + const fs = require('node:fs'); + const original = fs.readFileSync; + assert.throws(() => { + withFaultyFs({ readFileSync: () => { throw new Error('injected'); } }, () => { + throw new Error('body exploded'); + }); + }, /body exploded/); + assert.strictEqual(fs.readFileSync, original); + }); + + test('21. nested FaultyFs restore in order', () => { + const fs = require('node:fs'); + const original = fs.readFileSync; + const outerPatch = () => { throw new Error('outer'); }; + const innerPatch = () => { throw new Error('inner'); }; + + withFaultyFs({ readFileSync: outerPatch }, () => { + assert.strictEqual(fs.readFileSync, outerPatch); + withFaultyFs({ readFileSync: innerPatch }, () => { + assert.strictEqual(fs.readFileSync, innerPatch); + }); + // Inner restored WITHOUT clobbering the outer's still-active patch. + assert.strictEqual(fs.readFileSync, outerPatch); + }); + assert.strictEqual(fs.readFileSync, original); + }); + + test('22. FaultyFs patches only the named method', () => { + const fs = require('node:fs'); + const originalWrite = fs.writeFileSync; + withFaultyFs({ readFileSync: () => { throw new Error('injected'); } }, () => { + assert.strictEqual(fs.writeFileSync, originalWrite); + }); + assert.strictEqual(fs.writeFileSync, originalWrite); + }); +}); diff --git a/tests/graphify.test.cjs b/tests/graphify.test.cjs index 4d4104925..1b12ded44 100644 --- a/tests/graphify.test.cjs +++ b/tests/graphify.test.cjs @@ -476,7 +476,10 @@ describe('build', () => { status: null, stdout: 'partial', stderr: '', - error: undefined, + // Real spawnSync timeouts set error.code === 'ETIMEDOUT'; the + // timeout verdict keys on that, because SIGTERM alone is also + // produced by an external kill and is unreliable on Windows. + error: Object.assign(new Error('spawnSync ETIMEDOUT'), { code: 'ETIMEDOUT' }), signal: 'SIGTERM', })); @@ -487,6 +490,22 @@ describe('build', () => { assert.strictEqual(result.timeout_ms, 30000); }); + test('an externally-delivered SIGTERM is not reported as a timeout', () => { + // Before the shared isSpawnTimeout predicate was adopted, this shape + // (SIGTERM with no error.code) was misclassified as a timeout. + mock.method(childProcess, 'spawnSync', () => ({ + status: null, + stdout: '', + stderr: '', + error: undefined, + signal: 'SIGTERM', + })); + + const result = execGraphify('/tmp', ['build']); + assert.notStrictEqual(result.exitCode, 124); + assert.notStrictEqual(result.reason, GRAPHIFY_REASON.TIMEOUT); + }); + test('passes PYTHONUNBUFFERED=1 in env', () => { let captured; mock.method(childProcess, 'spawnSync', (_cmd, _args, opts) => { diff --git a/tests/helpers/faulty-deps.cjs b/tests/helpers/faulty-deps.cjs new file mode 100644 index 000000000..ee7e82648 --- /dev/null +++ b/tests/helpers/faulty-deps.cjs @@ -0,0 +1,199 @@ +'use strict'; + +/** + * In-process fault-injection adapter (issue #3056, Phase 2 of epic #3051). + * + * `makeFaultyGit()` and `withFaultyFs()` inject faults ONLY via the `deps` + * parameters that production seams already accept (`execGit`-shaped + * functions, `fs` module methods). This is in-process injection ONLY. + * + * The Phase 1 subprocess-dispatch seam (`tests/helpers/process-seam.cjs`, + * issue #3055) is DELIBERATELY NOT a fault-injection surface and must never + * be used to simulate a timeout, an ENOENT, or any other fault: the + * subprocess path has no way to distinguish an injected failure from a + * genuine bench OOM or a real flaky CI host, and — per the process seam's + * own retry policy — it would retry an injected timeout exactly as if it + * were a real one, silently corrupting the test's intent (design matrix + * row 24). Fault injection belongs at the `deps` seam, in-process, where the + * test controls exactly what is faulted and exactly once. + * + * `makeFaultyGit(options)` returns a function structurally assignable to + * `typeof execGit` from `shell-command-projection.cjs` — i.e. it satisfies + * every `ExecGitFn` declaration in the tree (worktree-safety.cts, + * git-base-branch.cts, worktree-base-ref.cts, verification.cts) because all + * four are structural subsets/supersets of that one shape (#3071). + * + * `withFaultyFs(patches, body)` monkeypatches named `fs` methods and + * guarantees restoration in a `finally` — this file is a standalone helper + * with no test context, which is the ONLY place CONTRIBUTING.md ("Setup and + * Cleanup") permits a `try/finally`: "try/finally is only permitted inside + * standalone utility or helper functions that have no access to test + * context." Test bodies must never use try/finally directly. + * + * NEVER use chmod to simulate an fs fault. `chmod 0o000` no-ops under root + * (root bypasses mode bits), so a chmod-based fault silently passes with + * zero coverage in root Docker/CI — see CONTRIBUTING.md and the Phase 2 + * design doc's "Not-corruption / negative space" section. + */ + +const DEFAULT_PASSTHROUGH_RESULT = Object.freeze({ + exitCode: 0, + stdout: '', + stderr: '', + signal: null, + error: null, + timedOut: false, +}); + +/** + * Build the result shape for a single fault `kind`. Mirrors the shapes + * `_spawnResult` in shell-command-projection.cts produces, so a faulted + * result is indistinguishable in shape from a real one. + * + * @param {{kind: string, exitCode?: number, stderr?: string}} fault + * @returns {{exitCode:number, stdout:string, stderr:string, signal:(string|null), error:(Error|null), timedOut:boolean}} + */ +function _buildFaultResult(fault) { + switch (fault.kind) { + case 'timeout': { + const error = Object.assign(new Error('timed out'), { code: 'ETIMEDOUT' }); + return { + exitCode: 1, + stdout: '', + stderr: '', + signal: 'SIGTERM', + error, + timedOut: true, + }; + } + case 'exit': { + return { + exitCode: typeof fault.exitCode === 'number' ? fault.exitCode : 1, + stdout: '', + stderr: fault.stderr || '', + signal: null, + error: null, + timedOut: false, + }; + } + case 'spawnFail': { + const error = Object.assign(new Error('spawn ENOENT'), { code: 'ENOENT' }); + return { + exitCode: 127, + stdout: '', + stderr: '', + signal: null, + error, + timedOut: false, + }; + } + default: + throw new Error(`makeFaultyGit: unknown fault kind "${fault.kind}"`); + } +} + +/** + * Return true when `fault.when` matches this invocation's argv. + * `when` may be an argv predicate `(args) => boolean` or an array treated as + * a prefix match against `args`. + * + * @param {*} when + * @param {string[]} args + * @returns {boolean} + */ +function _whenMatches(when, args) { + if (when === undefined) return true; + if (typeof when === 'function') return Boolean(when(args)); + if (Array.isArray(when)) { + if (when.length > args.length) return false; + return when.every((token, i) => args[i] === token); + } + return false; +} + +/** + * Build a fault-injecting stand-in for `execGit` (and, by structural typing, + * for every `ExecGitFn` site in the tree — see module doc). Faults are + * configured, never global: a call only faults when it matches every + * constraint on a configured fault; every other call delegates to + * `options.passthrough` (default: a benign zero-exit result). + * + * @param {object} [options] + * @param {Array<{ + * kind: 'timeout'|'exit'|'spawnFail', + * exitCode?: number, + * stderr?: string, + * when?: ((args: string[]) => boolean) | string[], + * onCall?: number, + * }>} [options.faults] - fault configurations, evaluated in order; the first + * whose `when`/`onCall` match the current call wins. + * @param {(args: string[], opts?: object) => object} [options.passthrough] - + * delegate for calls that do not match any configured fault. Defaults to a + * benign `{exitCode:0, stdout:'', stderr:'', signal:null, error:null, timedOut:false}`. + * @returns {((args: string[], opts?: object) => object) & { calls: Array<{args:string[], opts:object}> }} + */ +function makeFaultyGit(options) { + const opts = options || {}; + const faults = Array.isArray(opts.faults) ? opts.faults : []; + const passthrough = typeof opts.passthrough === 'function' + ? opts.passthrough + : () => ({ ...DEFAULT_PASSTHROUGH_RESULT }); + + const calls = []; + + function faultyGit(args, callOpts) { + const recordedArgs = Array.isArray(args) ? args.slice() : args; + const recordedOpts = callOpts || {}; + calls.push({ args: recordedArgs, opts: recordedOpts }); + const callNumber = calls.length; + + for (const fault of faults) { + if (fault.onCall !== undefined && fault.onCall !== callNumber) continue; + if (!_whenMatches(fault.when, recordedArgs)) continue; + return _buildFaultResult(fault); + } + + return passthrough(recordedArgs, recordedOpts); + } + + faultyGit.calls = calls; + return faultyGit; +} + +/** + * Monkeypatch named `fs` methods for the duration of `body()`, guaranteeing + * restoration via `finally` even when `body` throws. Nestable: each call + * saves its own originals into a local (not module-level) map, so an inner + * `withFaultyFs` restoring first does not clobber an outer call's saved + * original. + * + * This is the ONLY place in the test tree permitted to use try/finally — + * see the module doc and CONTRIBUTING.md "Setup and Cleanup": "try/finally + * is only permitted inside standalone utility or helper functions that have + * no access to test context." This helper has no test context; it is a + * plain function. + * + * @template T + * @param {Record any>} patches - maps an `fs` + * method name to its replacement (typically a function that throws). + * @param {() => T} body + * @returns {T} + */ +function withFaultyFs(patches, body) { + const fs = require('node:fs'); + const methodNames = Object.keys(patches); + const originals = new Map(); + for (const name of methodNames) { + originals.set(name, fs[name]); + fs[name] = patches[name]; + } + try { + return body(); + } finally { + for (const name of methodNames) { + fs[name] = originals.get(name); + } + } +} + +module.exports = { makeFaultyGit, withFaultyFs };