diff --git a/.changeset/lucky-foxes-jump.md b/.changeset/lucky-foxes-jump.md new file mode 100644 index 000000000..282fded4c --- /dev/null +++ b/.changeset/lucky-foxes-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3192 +--- +**`review-lane` rejects an unknown subcommand instantly instead of after a dozen subprocess spawns** — an unrecognized subcommand fell through to the usage-error branch only after loading the capability registry and building a per-lane plan, which spawns one child process per lane. The error now fires before any of that work starts (~119ms instead of ~1288ms). (#3148) diff --git a/CONTEXT.md b/CONTEXT.md index bca0fa553..d9abc3d6d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -421,13 +421,13 @@ Five-axis story decomposition discipline (**S**pike, **P**aths, **I**nterfaces, An injectable time abstraction accepted as an optional parameter by production code (`{ clock = Date } = {}`). Test code substitutes `node:test` `mock.timers` to control time deterministically without waiting for real OS scheduler events. Canonical pattern established by ADR 456 (`docs/adr/456-test-rigor-architecture.md`). ### Process seam -The single subprocess-spawning primitive test code uses (`tests/helpers/process-seam.cjs`, #3055): `runNode` / `runGit` / `runHook`, each returning one discriminated union `{ outcome, exitCode, stdout, stderr, timedOut, signal, killed, code }` where `outcome` is the frozen `OUTCOME` enum (`EXITED` / `KILLED` / `TIMED_OUT` / `BUFFER_OVERFLOW` / `SPAWN_FAILED`). Every call is timeout-bounded — there is no unbounded code path — and nothing throws for a child's exit code, kill, timeout, buffer overflow, or spawn failure; all five are data. KILLED is a child terminated by a signal the seam did not send (a genuine OOM kill): `spawnSync` reports no `error` for that case, so it must be distinguished from EXITED, and the `runGsdTools` adapter retries it exactly as the pre-seam `isKilled()` did. This is what makes `timedOut` and `signal` assertable, so a fail-open guard's degraded verdict can be tested instead of merely observing that the call did not throw. Discrimination order is forced by runtime behavior: a timeout and a maxBuffer overflow are identical on both `status` (`null`) and `signal` (`SIGTERM`) and differ only by `code` (`ETIMEDOUT` vs `ENOBUFS`), so overflow is classified first. Per-suite wrappers remain and bind fixtures (cwd, env, payload); only the spawn body delegates here. Deliberately **not** a fault-injection surface — it cannot distinguish an injected timeout from a genuine bench OOM and would retry it; injection is in-process via `deps` (#3056). `runGsdTools` is an adapter over it that preserves its own legacy `{ success, output, error, exitCode }` shape and retry-once-on-kill behavior. +The single subprocess-spawning primitive test code uses (`tests/helpers/process-seam.cjs`, #3055): `runNode` / `runGit` / `runHook`, each returning one discriminated union `{ outcome, exitCode, stdout, stderr, timedOut, signal, killed, code }` where `outcome` is the frozen `OUTCOME` enum (`EXITED` / `KILLED` / `TIMED_OUT` / `BUFFER_OVERFLOW` / `SPAWN_FAILED`). Every call is timeout-bounded — there is no unbounded code path — and nothing throws for a child's exit code, kill, timeout, buffer overflow, or spawn failure; all five are data. KILLED is a child terminated by a signal the seam did not send (a genuine OOM kill): `spawnSync` reports no `error` for that case, so it must be distinguished from EXITED, and the `runGsdTools` adapter retries it exactly as the pre-seam `isKilled()` did. This is what makes `timedOut` and `signal` assertable, so a fail-open guard's degraded verdict can be tested instead of merely observing that the call did not throw. Discrimination order is forced by runtime behavior: a timeout and a maxBuffer overflow are identical on both `status` (`null`) and `signal` (`SIGTERM`) and differ only by `code` (`ETIMEDOUT` vs `ENOBUFS`), so overflow is classified first — but that ordering assumes `status === null`, which is checked ahead of it: at the exact timeout boundary `spawnSync` can report `error.code === 'ETIMEDOUT'` on a result that ALSO carries a real `status` (the child finished on its own just as the timer fired), so `status !== null` is classified EXITED before any error-code branch runs, keeping `exitCode` coherent with the reported outcome. Per-suite wrappers remain and bind fixtures (cwd, env, payload); only the spawn body delegates here. Deliberately **not** a fault-injection surface — it cannot distinguish an injected timeout from a genuine bench OOM and would retry it; injection is in-process via `deps` (#3056). `runGsdTools` is an adapter over it that preserves its own legacy `{ success, output, error, exitCode }` shape and retry-once-on-kill behavior. ### Git fixture wrapper The throw-preserving companion to the process seam (`tests/helpers/git-fixture.cjs`, #3143): `gitOrThrow(args, options)` runs `runGit` and returns `stdout` as a string on a clean exit, but throws on any other outcome. It exists because the seam **deliberately never throws** while `execSync` and `execFileSync` — the two forms 237 migrating call sites use — both throw on a non-zero exit. Migrating those mechanically onto `runGit` would convert a loud failure into a silent one: fixture setup that failed would return an empty string and surface as a baffling assertion failure further down. The thrown error carries `status` **and** `exitCode` as deliberate aliases (`status` is what the legacy `execSync` catch idiom reads, e.g. `tests/worktree-safety.test.cjs:1361`), plus `stdout`, `stderr`, `signal`, `timedOut` and `outcome`. Use `runGit` when every outcome is data you branch on; use `gitOrThrow` for fixture setup that must abort loudly. The seam module is **not** modified to add this — a throwing export would falsify the never-throws contract stated in its own header and in the `### Process seam` entry above. The module also exports `throwIfFailed(result, displayName)`, the single implementation of that throw shape: `gitOrThrow` itself is `throwIfFailed` specialized to `runGit`, so it routes through the same code path and the two cannot drift apart. Per-suite wrappers driving non-git targets — a node CLI via `runNode`, a bash snippet via `runHook` — call `throwIfFailed` directly rather than hand-rolling their own copy of this shape, which is exactly how five call sites had drifted from each other before this module exported it (#3144). It also exports `toLegacyResult(result)`, the non-throwing counterpart: a bare mapping onto the legacy `{ status, stdout, stderr }` shape (`status` aliasing the seam's `exitCode`) for call sites that already branch on exit status as data rather than wanting a throw — ~8 test files hand-rolled that identical three-line mapping before this module exported it too (#3147). Callers needing an extra field beyond that shape (e.g. a parsed-JSON body, a fixture-specific path) compose it — `{ ...toLegacyResult(result), extra }` — rather than folding the extra behavior into the shared helper. ### Unbounded-spawn guard -The lint rule enforcing `DEFECT.UNBOUNDED-SUBPROCESS` across the test suite (`eslint-rules/no-unbounded-spawn.cjs`, #3143, wired into the `tests/**/*.cjs` block of `eslint.config.mjs`). Flags `spawnSync` / `execFileSync` / `execSync` whose options carry no usable `timeout`. It resolves renamed destructures (`const { execSync: exec } = require('node:child_process')`) and chained requires (`require('node:child_process').execSync(...)`) rather than matching literal callee names — both forms exist in the suite today and a name-only matcher leaves them permanently invisible. It resolves an options object held in a single-write `const`, which is what keeps `process-seam.cjs` — the bounded reference implementation — from flagging itself. Two values are rejected as *nominally* bounded: `timeout: 0` (Node reads zero as no timeout) and anything above the 600000 ms ceiling (effectively unbounded); a non-literal value is trusted, since the target shape is one named constant with a comment. `no-unbounded-spawn.allowlist.json` grandfathers pre-existing violations and ratchets **down only** — a listed file with zero violations reports its own entry as stale, so the list cannot go quiet while the class survives. Companion tests assert the list never grows, carries no dead entries, and that no `eslint-disable` for this rule exists anywhere under `tests/`. #3145 added an auditable ceiling escape: a literal timeout over the 600000 ms ceiling is permitted, without `timeoutTooLarge`, only when the call carries an inline `// allow-spawn-timeout-ceiling: ` marker comment (the `// allow-test-rule: ` idiom) with a non-empty reason, on the line immediately above the call or anywhere inside the call's own source range. It binds to that call only — a marker on one call never suppresses a different over-ceiling call elsewhere in the file — and it is deliberately narrow: it raises the ceiling for a call that already has a resolvable numeric timeout, it never waives the requirement for a bound, so a marked call with no `timeout` at all still reports `unboundedSpawn`. The real load-tested exception this exists for is `fragment-single-edit-propagation.install.test.cjs`'s `timeout: 900000` on `npm run regen:derived` (a full build plus eight generators), where 300000 was observed killing a genuinely-completed run near the end on a loaded bench. +The lint rule enforcing `DEFECT.UNBOUNDED-SUBPROCESS` across the test suite (`eslint-rules/no-unbounded-spawn.cjs`, #3143, wired into the `tests/**/*.cjs` block of `eslint.config.mjs`). Flags `spawnSync` / `execFileSync` / `execSync` whose options carry no usable `timeout`. It resolves renamed destructures (`const { execSync: exec } = require('node:child_process')`) and chained requires (`require('node:child_process').execSync(...)`) rather than matching literal callee names — both forms exist in the suite today and a name-only matcher leaves them permanently invisible. It resolves an options object held in a single-write `const`, which is what keeps `process-seam.cjs` — the bounded reference implementation — from flagging itself. Two values are rejected as *nominally* bounded: `timeout: 0` (Node reads zero as no timeout) and anything above the 600000 ms ceiling (effectively unbounded); a non-literal value is trusted, since the target shape is one named constant with a comment. #3148 (the terminal wave of epic #3064) deleted `no-unbounded-spawn.allowlist.json` and its wiring from `eslint.config.mjs` once the migration reached zero remaining violations — the rule now runs with **no exemption surface** across `tests/**`; there is no allowlist to add a file to. The one companion guard that survives (`tests/no-unbounded-spawn-allowlist.test.cjs`) asserts that no `eslint-disable` naming this rule exists anywhere under `tests/` — with the allowlist gone, that inline disable is the *only* remaining way to silence the rule, so this guard is now the sole remaining defense. The only sanctioned escapes are an explicit `timeout` on a raw spawn (for a call shape the process seam cannot express, e.g. a `shell: true` invocation for `npm.cmd` on Windows, or `stdio` redirection to a real fd) and the ceiling-escape marker below. #3145 added an auditable ceiling escape: a literal timeout over the 600000 ms ceiling is permitted, without `timeoutTooLarge`, only when the call carries an inline `// allow-spawn-timeout-ceiling: ` marker comment (the `// allow-test-rule: ` idiom) with a non-empty reason, on the line immediately above the call or anywhere inside the call's own source range. It binds to that call only — a marker on one call never suppresses a different over-ceiling call elsewhere in the file — and it is deliberately narrow: it raises the ceiling for a call that already has a resolvable numeric timeout, it never waives the requirement for a bound, so a marked call with no `timeout` at all still reports `unboundedSpawn`. The real load-tested exception this exists for is `fragment-single-edit-propagation.install.test.cjs`'s `timeout: 900000` on `npm run regen:derived` (a full build plus eight generators), where 300000 was observed killing a genuinely-completed run near the end on a loaded bench. ### Deterministic scheduler Test-execution model in which all timing and concurrency outcomes are fully controlled by the test (via clock seam, explicit `await` ordering, or synchronous stepping) rather than by the OS thread scheduler. Opposed to real-race tests, which are non-deterministic on loaded CI runners. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0a7100309..42f667d08 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -525,10 +525,16 @@ escape only ever raises the ceiling for a call that already resolves to a numeri never waives the requirement for a bound. A marked call with no `timeout` at all still reports `unboundedSpawn`. -`eslint-rules/no-unbounded-spawn.allowlist.json` grandfathers files that predate the rule. It only -ratchets **down**: once a file is clean, the rule reports its allowlist line as stale and you delete -it. Never add an entry, and never reach for `eslint-disable` on this rule — a test asserts that no -such comment exists. +There is no allowlist. `eslint-rules/no-unbounded-spawn.allowlist.json` grandfathered files that +predated the rule; the epic that introduced it (#3064) migrated every site across four waves and +deleted the file in its terminal wave (#3148), so `local/no-unbounded-spawn` now runs with **no +exemption surface** across `tests/**`. There is no file to add an entry to — fix the timeout at +the call site instead. The only sanctioned escapes are an explicit `timeout` on a raw spawn (for a +call shape the process seam cannot express, e.g. a `shell: true` invocation for `npm.cmd` on +Windows, or `stdio` redirection to a real fd) and the `// allow-spawn-timeout-ceiling: ` +marker above for a bound over the 600000 ms ceiling. Never reach for `eslint-disable` on this rule +— with the allowlist gone, that is the only remaining way to silence it, and a test asserts that no +such comment exists anywhere under `tests/`. ### Test Structure diff --git a/eslint-rules/no-unbounded-spawn.allowlist.json b/eslint-rules/no-unbounded-spawn.allowlist.json deleted file mode 100644 index 45f0f9bc0..000000000 --- a/eslint-rules/no-unbounded-spawn.allowlist.json +++ /dev/null @@ -1,51 +0,0 @@ -[ - "tests/adr-15-progress-converge.test.cjs", - "tests/agent-skills.test.cjs", - "tests/autonomous-converge.test.cjs", - "tests/bugs-1656-1657.test.cjs", - "tests/check-tdd-review-checkpoint-e2e.test.cjs", - "tests/check-ui-safety-gate.test.cjs", - "tests/check-update-config-dir.test.cjs", - "tests/ci-test-scope.test.cjs", - "tests/cli-exit.test.cjs", - "tests/close-phase-todos-padded-resolves.test.cjs", - "tests/code-review-pipeline-regression.test.cjs", - "tests/commands.test.cjs", - "tests/commonjs-marker.test.cjs", - "tests/config-get-default.test.cjs", - "tests/config-loader.test.cjs", - "tests/config.test.cjs", - "tests/configuration-migrate-config.test.cjs", - "tests/drift-detection.test.cjs", - "tests/edge-probe.test.cjs", - "tests/execute-wave-post-gate-pipeline-e2e.test.cjs", - "tests/fix-2136-clock-local-today.test.cjs", - "tests/fix-2590-workflow-script-contract.test.cjs", - "tests/fix-2650-plan-phase-stall-detection.test.cjs", - "tests/fix-2657-untrack-compiled-artifacts.test.cjs", - "tests/fix-3045-cursor-subagent-isolation.test.cjs", - "tests/fix-3045-dispatch-isolation-resolver.test.cjs", - "tests/fixture-builder.test.cjs", - "tests/frontmatter-cli.test.cjs", - "tests/gsd-agent-isolation-guard.test.cjs", - "tests/gsd-statusline.test.cjs", - "tests/helpers.cjs", - "tests/host-integration.test.cjs", - "tests/init.test.cjs", - "tests/io.test.cjs", - "tests/issue-2765-brace-expansion-lockfile.test.cjs", - "tests/issue-498-update-context.test.cjs", - "tests/new-milestone-clear-phases.test.cjs", - "tests/pause-work-improvements.test.cjs", - "tests/phase.test.cjs", - "tests/process-seam.test.cjs", - "tests/prohibition-enforcement.test.cjs", - "tests/project-instruction-file-parity.test.cjs", - "tests/run-tests-harness.test.cjs", - "tests/runtime-launcher-parity.test.cjs", - "tests/smart-entry.unit.test.cjs", - "tests/spec-section.test.cjs", - "tests/state-rebuild-cli.test.cjs", - "tests/state.test.cjs", - "tests/workflow-guard.test.cjs" -] diff --git a/eslint.config.mjs b/eslint.config.mjs index 9d37c0798..1db9ea096 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -5,7 +5,6 @@ import pluginN from 'eslint-plugin-n'; import noOnlyTests from 'eslint-plugin-no-only-tests'; import { dirname } from 'path'; import { fileURLToPath } from 'url'; -import { createRequire } from 'module'; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -27,9 +26,6 @@ import normalizePathInContent from './eslint-rules/normalize-path-in-content.cjs import requireFsOpFallback from './eslint-rules/require-fs-op-fallback.cjs'; import noUnboundedSpawn from './eslint-rules/no-unbounded-spawn.cjs'; -const require = createRequire(import.meta.url); -const unboundedSpawnAllowlist = require('./eslint-rules/no-unbounded-spawn.allowlist.json'); - const localPlugin = { rules: { 'no-source-grep': noSourceGrep, @@ -399,8 +395,11 @@ export default tseslint.config( 'local/no-bare-npm-exec': 'error', // Require USERPROFILE alongside HOME assignments (ADR-1703 Phase 4) 'local/require-userprofile-with-home': 'error', - // Ban unbounded sync child_process spawns in tests (DEFECT.UNBOUNDED-SUBPROCESS) - 'local/no-unbounded-spawn': ['error', { allowlist: unboundedSpawnAllowlist }], + // Ban unbounded sync child_process spawns in tests (DEFECT.UNBOUNDED-SUBPROCESS). + // No allowlist: the epic (#3064) migrated every site; the rule runs with no + // exemption surface. The only sanctioned escapes are an explicit `timeout` on + // a raw spawn or the `// allow-spawn-timeout-ceiling: ` marker. + 'local/no-unbounded-spawn': 'error', // Ban raw setTimeout sync + elapsed/duration-style assertions via no-restricted-syntax 'no-restricted-syntax': [ 'error', diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index b06c604de..a2d0d3dd9 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1190,6 +1190,22 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load return i !== -1 && args[i + 1] && !String(args[i + 1]).startsWith('--') ? args[i + 1] : null; }; const sub = args[1]; + // Fail fast on an unrecognized subcommand. Without this check, `sub` fell through + // to the `sub !== 'invoke'` usage-error branch far below (after loading the + // capability registry AND building a per-lane plan for every lane — which itself + // spawns one child `query resolve-execution` process per lane via `effortFor`, + // up to 12 subprocess spawns for the default lane set) before ever reporting the + // error. That made an invalid subcommand slow instead of instant, and under bench + // load (many sequential node spawns) `review-lane bogus` could exceed a caller's + // spawn timeout and be killed before writing anything to stderr — the CI-observed + // failure was empty stdout AND stderr, not the expected usage message (#3148). + // `plan`/`invoke` are the only subs that need the expensive plan-building path + // below; `sections`/`flags` return earlier still. Anything else errors here, before + // any of that work starts. + if (!['plan', 'invoke', 'sections', 'flags'].includes(sub)) { + error("Usage: review-lane [--selected a,b] [--run-dir D] [--repo-root R]"); + return; + } const runDir = flag('--run-dir') || '.'; const repoRoot = flag('--repo-root') || cwd; diff --git a/tests/adr-15-progress-converge.test.cjs b/tests/adr-15-progress-converge.test.cjs index cd59c3ce8..b5ce56b96 100644 --- a/tests/adr-15-progress-converge.test.cjs +++ b/tests/adr-15-progress-converge.test.cjs @@ -8,7 +8,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const cp = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const COMMAND_PATH = path.join(REPO_ROOT, 'commands', 'gsd', 'progress.md'); @@ -141,10 +143,9 @@ describe('ADR-15: /gsd:progress --next --auto --converge (#1190)', () => { // Behavioral coverage: prove the roster the workflow derives from actually // yields the flags this test used to hardcode, so the derivation is not vacuous. - const laneFlags = cp - .execFileSync(process.execPath, [TOOLS, 'review-lane', 'flags'], { encoding: 'utf8' }) - .split('\n') - .filter(Boolean); + const laneFlagsResult = runNode([TOOLS, 'review-lane', 'flags'], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(laneFlagsResult, `node ${TOOLS} review-lane flags`); + const laneFlags = laneFlagsResult.stdout.split('\n').filter(Boolean); for (const flag of formerlyHardcodedLaneFlags) { assert.ok(laneFlags.includes(flag), `review-lane flags should include ${flag}`); } diff --git a/tests/agent-skills.test.cjs b/tests/agent-skills.test.cjs index f1c4ea679..40063bc1f 100644 --- a/tests/agent-skills.test.cjs +++ b/tests/agent-skills.test.cjs @@ -17,6 +17,8 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { runGsdTools, createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const TEST_ENV_BASE = { GSD_SESSION_KEY: '', CODEX_THREAD_ID: '', @@ -41,21 +43,13 @@ const TEST_ENV_BASE = { */ function runGsdToolsWithStderr(args, cwd, env) { const childEnv = { ...process.env, ...TEST_ENV_BASE, ...(env || {}) }; - try { - const result = spawnSync(process.execPath, [TOOLS_PATH, ...args], { - cwd, - encoding: 'utf-8', - env: childEnv, - }); - return { - success: result.status === 0, - stdout: (result.stdout || '').trim(), - stderr: (result.stderr || '').trim(), - exitCode: result.status, - }; - } catch (err) { - return { success: false, stdout: '', stderr: String(err), exitCode: 1 }; - } + const result = runNode([TOOLS_PATH, ...args], { cwd, env: childEnv, timeoutMs: PROBE_TIMEOUT_MS }); + return { + success: result.exitCode === 0, + stdout: result.stdout.trim(), + stderr: result.stderr.trim(), + exitCode: result.exitCode, + }; } const { loadTrustedGlobalRoots, validatePath } = require('../gsd-core/bin/lib/security.cjs'); @@ -1759,6 +1753,10 @@ describe('#1400 regression: plain agent-skills output survives pipe/file stdout' const outPath = path.join(tmpDir, 'agent-skills.out'); const fd = fs.openSync(outPath, 'w'); try { + // Kept as a raw spawnSync (not the process-seam): the seam does not + // forward a `stdio` option, and this test needs stdout wired directly + // to a real file descriptor to reproduce the exit-before-flush + // truncation bug — capturing via a pipe would defeat the point. const result = spawnSync( process.execPath, [TOOLS_PATH, 'query', 'agent-skills', agentType], @@ -1766,6 +1764,7 @@ describe('#1400 regression: plain agent-skills output survives pipe/file stdout' cwd: tmpDir, env: { ...process.env, ...TEST_ENV_BASE, HOME: tmpDir, USERPROFILE: tmpDir }, stdio: ['ignore', fd, 'pipe'], + timeout: PROBE_TIMEOUT_MS, }, ); return { status: result.status, contents: fs.readFileSync(outPath, 'utf-8') }; @@ -1822,20 +1821,16 @@ describe('#1400 regression: plain agent-skills output survives pipe/file stdout' writeConfig(tmpDir, { agent_skills: { 'gsd-executor': skillPaths } }); // stdout to a pipe (the truncation-prone case the bug is about), captured - // by spawnSync — proves writeAllSync drained every byte before exit. - const result = spawnSync( - process.execPath, - [TOOLS_PATH, 'query', 'agent-skills', 'gsd-executor'], - { - cwd: tmpDir, - encoding: 'utf-8', - maxBuffer: 8 * 1024 * 1024, - env: { ...process.env, ...TEST_ENV_BASE, HOME: tmpDir, USERPROFILE: tmpDir }, - stdio: ['ignore', 'pipe', 'pipe'], - }, - ); + // via the process seam — proves writeAllSync drained every byte before + // exit. Actual output here is well under the seam's implicit 1MB + // spawnSync maxBuffer default, so no override is needed. + const result = runNode([TOOLS_PATH, 'query', 'agent-skills', 'gsd-executor'], { + cwd: tmpDir, + env: { ...process.env, ...TEST_ENV_BASE, HOME: tmpDir, USERPROFILE: tmpDir }, + timeoutMs: PROBE_TIMEOUT_MS, + }); const out = result.stdout || ''; - assert.strictEqual(result.status, 0, `command must exit 0; stderr=${result.stderr}`); + assert.strictEqual(result.exitCode, 0, `command must exit 0; stderr=${result.stderr}`); assert.ok( Buffer.byteLength(out, 'utf-8') > PIPE_BUFFER, `block must exceed the ${PIPE_BUFFER}-byte pipe buffer to exercise partial writes (got ${Buffer.byteLength(out, 'utf-8')} bytes)`, diff --git a/tests/autonomous-converge.test.cjs b/tests/autonomous-converge.test.cjs index ff5c45e0f..fd349b80d 100644 --- a/tests/autonomous-converge.test.cjs +++ b/tests/autonomous-converge.test.cjs @@ -8,7 +8,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const cp = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const COMMAND_PATH = path.join(REPO_ROOT, 'commands', 'gsd', 'autonomous.md'); @@ -156,10 +158,9 @@ describe('autonomous --converge flag (#711)', () => { // Behavioral coverage: prove the roster the workflow derives from actually // yields the flags this test used to hardcode, so the derivation is not vacuous. - const laneFlags = cp - .execFileSync(process.execPath, [TOOLS, 'review-lane', 'flags'], { encoding: 'utf8' }) - .split('\n') - .filter(Boolean); + const laneFlagsResult = runNode([TOOLS, 'review-lane', 'flags'], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(laneFlagsResult, `node ${TOOLS} review-lane flags`); + const laneFlags = laneFlagsResult.stdout.split('\n').filter(Boolean); for (const flag of formerlyHardcodedLaneFlags) { assert.ok(laneFlags.includes(flag), `review-lane flags should include ${flag}`); } diff --git a/tests/bugs-1656-1657.test.cjs b/tests/bugs-1656-1657.test.cjs index 0964991b3..e6c6ce038 100644 --- a/tests/bugs-1656-1657.test.cjs +++ b/tests/bugs-1656-1657.test.cjs @@ -12,7 +12,9 @@ const { test, describe, before } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { execFileSync } = require('child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { BUILD_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HOOKS_DIST = path.join(__dirname, '..', 'hooks', 'dist'); const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); @@ -25,10 +27,8 @@ describe('#1656: community .sh hooks must be present in hooks/dist', () => { // hooks/dist/ is gitignored so it must be generated; this mirrors what // `npm run build:hooks` (prepublishOnly) does before publish. before(() => { - execFileSync(process.execPath, [BUILD_SCRIPT], { - encoding: 'utf-8', - stdio: 'pipe', - }); + const result = runNode([BUILD_SCRIPT], { timeoutMs: BUILD_TIMEOUT_MS }); + throwIfFailed(result, `node ${BUILD_SCRIPT}`); }); test('gsd-session-state.sh exists in hooks/dist', () => { diff --git a/tests/check-tdd-review-checkpoint-e2e.test.cjs b/tests/check-tdd-review-checkpoint-e2e.test.cjs index 83a18fda1..1a01e8aa6 100644 --- a/tests/check-tdd-review-checkpoint-e2e.test.cjs +++ b/tests/check-tdd-review-checkpoint-e2e.test.cjs @@ -28,9 +28,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { execFileSync, spawnSync } = require('node:child_process'); +const { execFileSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -47,9 +48,8 @@ function createTddGitFixture({ planFiles = [] } = {}) { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tdd-e2e-')); function git(...args) { - const result = spawnSync('git', args, { + return gitOrThrow(args, { cwd: tmpDir, - encoding: 'utf-8', env: { ...process.env, GIT_AUTHOR_NAME: 'Test', @@ -57,11 +57,7 @@ function createTddGitFixture({ planFiles = [] } = {}) { GIT_COMMITTER_NAME: 'Test', GIT_COMMITTER_EMAIL: 'test@test.com', }, - }); - if (result.status !== 0) { - throw new Error(`git ${args.join(' ')} failed: ${result.stderr}`); - } - return result.stdout.trim(); + }).trim(); } git('init', '--initial-branch=main'); diff --git a/tests/check-ui-safety-gate.test.cjs b/tests/check-ui-safety-gate.test.cjs index 84b6a8cfb..45d5d22a8 100644 --- a/tests/check-ui-safety-gate.test.cjs +++ b/tests/check-ui-safety-gate.test.cjs @@ -294,9 +294,11 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { spawnSync } = require('node:child_process'); const os = require('node:os'); const { cleanup } = require('./helpers.cjs'); +const { runNode, runHook } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HELPER_PATH = path.join(__dirname, '..', 'bin', 'lib', 'ui-safety-gate.cjs'); const PLAN_PHASE_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'plan-phase.md'); @@ -335,11 +337,8 @@ function hasUiGate(text) { * Returns the spawnSync result object. */ function spawnGate(input) { - return spawnSync(process.execPath, [HELPER_PATH], { - shell: false, - encoding: 'utf-8', - input: input, - }); + const result = runNode([HELPER_PATH], { input, timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(result); } // ── Structural guard — workflow files now invoke Node via stdin ─────────────── @@ -579,11 +578,13 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep ].join('\n'); function runGateFrom(consumingDir, phaseSection) { - return spawnSync('bash', ['-c', GATE_SNIPPET], { + const result = runHook('-c', [GATE_SNIPPET], { + interpreter: 'bash', cwd: consumingDir, - encoding: 'utf-8', env: { ...process.env, RUNTIME_DIR: REPO_ROOT, PHASE_SECTION: phaseSection }, + timeoutMs: PROBE_TIMEOUT_MS, }); + return toLegacyResult(result); } test('UI text is detected (HAS_UI=0) from a project without bin/lib', () => { @@ -621,11 +622,14 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep path.join(installedLibDir, 'ui-safety-gate.cjs') ); - const res = spawnSync('bash', ['-c', GATE_SNIPPET], { - cwd: consumingProject, - encoding: 'utf-8', - env: { ...process.env, RUNTIME_DIR: fakeRuntime, PHASE_SECTION: 'Build the analytics dashboard' }, - }); + const res = toLegacyResult( + runHook('-c', [GATE_SNIPPET], { + interpreter: 'bash', + cwd: consumingProject, + env: { ...process.env, RUNTIME_DIR: fakeRuntime, PHASE_SECTION: 'Build the analytics dashboard' }, + timeoutMs: PROBE_TIMEOUT_MS, + }) + ); assert.strictEqual(res.status, 0, `bash failed: ${res.stderr}`); assert.strictEqual(res.stdout.trim(), '0', 'helper must be found via gsd-core/bin/lib/ in installed layout and report UI present'); diff --git a/tests/check-update-config-dir.test.cjs b/tests/check-update-config-dir.test.cjs index 0406723d2..430ee8814 100644 --- a/tests/check-update-config-dir.test.cjs +++ b/tests/check-update-config-dir.test.cjs @@ -17,8 +17,10 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const { execFileSync } = require('child_process'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const CHECK_UPDATE_PATH = path.join(__dirname, '..', 'hooks', 'gsd-check-update.js'); @@ -101,9 +103,9 @@ describe('detectConfigDir runtime behavior (#1860)', () => { "process.stdout.write(result);", ].join('\n'); - const result = execFileSync(process.execPath, ['-e', testScript], { - encoding: 'utf8', - }); + const nodeResult = runNode(['-e', testScript], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(nodeResult, `node -e `); + const result = nodeResult.stdout; const expectedDir = path.join(tmpHome, '.claude'); assert.strictEqual( @@ -137,9 +139,9 @@ describe('detectConfigDir runtime behavior (#1860)', () => { "process.stdout.write(result);", ].join('\n'); - const result = execFileSync(process.execPath, ['-e', testScript], { - encoding: 'utf8', - }); + const nodeResult = runNode(['-e', testScript], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(nodeResult, `node -e `); + const result = nodeResult.stdout; const expectedDir = path.join(tmpHome, '.config', 'opencode'); assert.strictEqual( diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index c39839836..161d777bb 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -2,22 +2,21 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { spawnSync } = require('child_process'); const path = require('path'); const fs = require('fs'); const os = require('node:os'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'ci-test-scope.cjs'); const WORKFLOWS_DIR = path.join(ROOT, '.github', 'workflows'); function scopeFor(files) { - const r = spawnSync(process.execPath, [SCRIPT, '--files', files.join(' ')], { - cwd: ROOT, - encoding: 'utf8', - }); - assert.strictEqual(r.status, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); + const r = runNode([SCRIPT, '--files', files.join(' ')], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(r.exitCode, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); return JSON.parse(r.stdout); } @@ -171,11 +170,8 @@ describe('ci-test-scope.cjs', () => { }); test('missing required CLI values fail with usage', () => { - const r = spawnSync(process.execPath, [SCRIPT, '--files'], { - cwd: ROOT, - encoding: 'utf8', - }); - assert.notStrictEqual(r.status, 0); + const r = runNode([SCRIPT, '--files'], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.notStrictEqual(r.exitCode, 0); // allow-test-rule: pending-migration-to-typed-ir [#3090] // Regex-matches the CLI's human-readable stderr formatter (usage banner + // arg-parser Error#message) — CONTRIBUTING's own BAD example verbatim. @@ -226,11 +222,7 @@ describe('ci-test-scope.cjs', () => { // GitHub's PR semantics) must see ONLY the docs change. const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ci-scope-837-')); try { - const git = (...a) => { - const r = spawnSync('git', a, { cwd: tmp, encoding: 'utf8' }); - assert.strictEqual(r.status, 0, `git ${a.join(' ')} failed: ${r.stderr}`); - return r.stdout.trim(); - }; + const git = (...a) => gitOrThrow(a, { cwd: tmp }).trim(); git('init', '-q'); git('config', 'user.email', 'test@example.com'); git('config', 'user.name', 'Test'); @@ -259,11 +251,8 @@ describe('ci-test-scope.cjs', () => { git('commit', '-qm', 'chore: bump version on next'); const base = git('rev-parse', 'HEAD'); - const r = spawnSync(process.execPath, [SCRIPT, '--base', base, '--head', head], { - cwd: tmp, - encoding: 'utf8', - }); - assert.strictEqual(r.status, 0, `script failed: stderr=${r.stderr}\nstdout=${r.stdout}`); + const r = runNode([SCRIPT, '--base', base, '--head', head], { cwd: tmp, timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(r.exitCode, 0, `script failed: stderr=${r.stderr}\nstdout=${r.stdout}`); const result = JSON.parse(r.stdout); assert.deepStrictEqual( @@ -897,11 +886,13 @@ describe('code_changed=false implies clean output invariant', () => { const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); -const { spawnSync } = require('child_process'); const fs = require('fs'); const path = require('path'); const { createTempDir, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HARNESS = path.join(__dirname, '..', 'scripts', 'run-tests.cjs'); @@ -919,11 +910,12 @@ function seed(dir, names) { function runHarness(testDir, args = [], extraEnv = {}) { const env = { ...process.env, GSD_TEST_DIR: testDir, ...extraEnv }; delete env.NODE_TEST_CONTEXT; - return spawnSync(process.execPath, [HARNESS, ...args], { + const result = runNode([HARNESS, ...args], { cwd: path.join(__dirname, '..'), env, - encoding: 'utf8', + timeoutMs: PROBE_TIMEOUT_MS, }); + return toLegacyResult(result); } describe('bug #641 — --files-from with bare suite token', () => { @@ -1114,12 +1106,15 @@ describe('bug #1329 — ci-prepare-test-scope fallback never emits a deleted fil for (const f of FALLBACK) { fs.writeFileSync(path.join(tmpDir, f), PASS_BODY, 'utf8'); } - const prep = spawnSync( - process.execPath, + const prep = runNode( [path.join(REPO_ROOT, 'scripts', 'ci-prepare-test-scope.cjs')], - { cwd: tmpDir, env: { ...process.env, TEST_SCOPE: 'targeted', TARGETED_TESTS: '', WINDOWS_TESTS: '' }, encoding: 'utf8' }, + { + cwd: tmpDir, + env: { ...process.env, TEST_SCOPE: 'targeted', TARGETED_TESTS: '', WINDOWS_TESTS: '' }, + timeoutMs: PROBE_TIMEOUT_MS, + }, ); - assert.strictEqual(prep.status, 0, `prepare step failed: ${prep.stderr}`); + assert.strictEqual(prep.exitCode, 0, `prepare step failed: ${prep.stderr}`); const selected = fs.readFileSync(path.join(tmpDir, '.ci-selected-tests.txt'), 'utf8'); for (const line of selected.split(/\r?\n/).filter(Boolean)) { diff --git a/tests/cli-exit.test.cjs b/tests/cli-exit.test.cjs index 21823bd14..bb2cef742 100644 --- a/tests/cli-exit.test.cjs +++ b/tests/cli-exit.test.cjs @@ -2,10 +2,12 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { spawnSync } = require('node:child_process'); const path = require('node:path'); const { ExitError, runMain } = require('../scripts/lib/cli-exit.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // Paths to the compiled product seam (src/cli-exit.cts → gsd-core/bin/lib/cli-exit.cjs) // used for json-error mode regression tests which require io.cjs integration. @@ -197,7 +199,7 @@ describe('regressions', () => { runMain(() => { throw ${throwExpr}; }); setImmediate(() => {}); `; - return spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + return toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS })); } describe('bug-965: unexpected throw in json-error mode emits structured envelope', () => { diff --git a/tests/close-phase-todos-padded-resolves.test.cjs b/tests/close-phase-todos-padded-resolves.test.cjs index 831c258ca..b1d3ed445 100644 --- a/tests/close-phase-todos-padded-resolves.test.cjs +++ b/tests/close-phase-todos-padded-resolves.test.cjs @@ -10,8 +10,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +const { runHook } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const EXECUTE_PHASE = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md'); @@ -116,7 +118,9 @@ describe('#2576: close_phase_todos normalizes padded vs unpadded resolves_phase const script = path.join(tmp, 'normalize.sh'); // argv array (no shell string) so a quoted input like '"05"' is passed verbatim. fs.writeFileSync(script, `${helper}\nnormalize_phase_num "$1"\n`); - return execFileSync('bash', [script, input], { encoding: 'utf8' }); + const result = runHook(script, [input], { interpreter: 'bash', timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(result, `bash ${script} ${input}`); + return result.stdout; } // The headline #2576 case: single-digit phase, padded vs unpadded. diff --git a/tests/code-review-pipeline-regression.test.cjs b/tests/code-review-pipeline-regression.test.cjs index a57b5933d..2c864c5f2 100644 --- a/tests/code-review-pipeline-regression.test.cjs +++ b/tests/code-review-pipeline-regression.test.cjs @@ -25,7 +25,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); +const { runHook } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { createTempDir, cleanup, readFileNormalized } = require('./helpers.cjs'); const ROOT = path.resolve(__dirname, '..'); @@ -627,10 +629,13 @@ describe('Bug 4 (#2352) — compute_file_scope tilde-path expansion', () => { function runPostProcessing(homeDir, files) { const script = extractPostProcessingScript(); // "bash" as $0 so the real REVIEW_FILES entries land in "$@" from $1. - return spawnSync('bash', ['-c', script, 'bash', ...files], { - encoding: 'utf8', - env: { ...process.env, HOME: homeDir }, - }); + return toLegacyResult( + runHook('-c', [script, 'bash', ...files], { + interpreter: 'bash', + env: { ...process.env, HOME: homeDir }, + timeoutMs: PROBE_TIMEOUT_MS, + }) + ); } let tmpHome; diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 86a0901ec..031509c06 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -8,11 +8,12 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); -const { execSync, execFileSync } = require('node:child_process'); const fs = require('fs'); const path = require('path'); const { runGsdTools, createTempProject, createTempDir, cleanup } = require('./helpers.cjs'); const fc = require('./helpers/fast-check-setup.cjs'); +const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); describe('history-digest command', () => { let tmpDir; @@ -1303,7 +1304,8 @@ describe('resolve-model command', () => { describe('commit command', () => { const { createTempGitProject } = require('./helpers.cjs'); - const { execSync, execFileSync } = require('child_process'); + const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); + const { runNode } = require('./helpers/process-seam.cjs'); let tmpDir; beforeEach(() => { @@ -1332,8 +1334,8 @@ describe('commit command', () => { test('skips when .planning is gitignored', () => { // Add .planning/ to .gitignore and commit it so git recognizes the ignore fs.writeFileSync(path.join(tmpDir, '.gitignore'), '.planning/\n'); - execSync('git add .gitignore', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "add gitignore"', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['add', '.gitignore'], { cwd: tmpDir }); + gitOrThrow(['commit', '-m', 'add gitignore'], { cwd: tmpDir }); const result = runGsdTools('commit "test message"', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); @@ -1366,7 +1368,7 @@ describe('commit command', () => { assert.strictEqual(output.reason, 'committed'); // Verify via git log - const gitLog = execSync('git log --oneline -1', { cwd: tmpDir, encoding: 'utf-8' }).trim(); + const gitLog = gitOrThrow(['log', '--oneline', '-1'], { cwd: tmpDir }).trim(); assert.ok(gitLog.includes('test: add test file'), 'git log should contain the commit message'); assert.ok(gitLog.includes(output.hash), 'git log should contain the returned hash'); }); @@ -1374,8 +1376,8 @@ describe('commit command', () => { test('amend mode works without crashing', () => { // Create a file and commit it first fs.writeFileSync(path.join(tmpDir, '.planning', 'amend-file.md'), '# Initial\n'); - execSync('git add .planning/amend-file.md', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "initial file"', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['add', '.planning/amend-file.md'], { cwd: tmpDir }); + gitOrThrow(['commit', '-m', 'initial file'], { cwd: tmpDir }); // Modify the file and amend fs.writeFileSync(path.join(tmpDir, '.planning', 'amend-file.md'), '# Amended\n'); @@ -1387,7 +1389,7 @@ describe('commit command', () => { assert.strictEqual(output.committed, true, 'amend should succeed'); // Verify only 2 commits total (initial setup + amended) - const logCount = execSync('git log --oneline', { cwd: tmpDir, encoding: 'utf-8' }).trim().split('\n').length; + const logCount = gitOrThrow(['log', '--oneline'], { cwd: tmpDir }).trim().split('\n').length; assert.strictEqual(logCount, 2, 'should have 2 commits (initial + amended)'); }); test('creates strategy branch before first commit when branching_strategy is milestone (#3079: no switch)', () => { @@ -1416,11 +1418,10 @@ describe('commit command', () => { assert.strictEqual(output.committed, true, 'should have committed'); // #3079: the branch should be CREATED but NOT switched to. - const { execFileSync } = require('child_process'); - const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir, encoding: 'utf-8' }).trim(); + const branch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); assert.notStrictEqual(branch, 'gsd/v1.0-initial-release', '#3079: must NOT switch to the milestone branch'); // Verify the branch WAS created (exists as a ref) - const branchExists = execFileSync('git', ['rev-parse', '--verify', 'gsd/v1.0-initial-release'], { cwd: tmpDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); + const branchExists = gitOrThrow(['rev-parse', '--verify', 'gsd/v1.0-initial-release'], { cwd: tmpDir }); assert.ok(branchExists.trim(), 'milestone branch should be created even without switching'); }); @@ -1456,11 +1457,10 @@ describe('commit command', () => { // #3079: the branch should be CREATED but NOT switched to. The commit // lands on the current branch (master/main), and the phase branch exists // as a ref but HEAD did not move. - const { execFileSync } = require('child_process'); - const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir, encoding: 'utf-8' }).trim(); + const branch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); assert.notStrictEqual(branch, 'gsd/phase-01-setup', '#3079: must NOT switch to the phase branch'); // Verify the branch WAS created (exists as a ref) - const branchExists = execFileSync('git', ['rev-parse', '--verify', 'gsd/phase-01-setup'], { cwd: tmpDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); + const branchExists = gitOrThrow(['rev-parse', '--verify', 'gsd/phase-01-setup'], { cwd: tmpDir }); assert.ok(branchExists.trim(), 'phase branch should be created even without switching'); }); @@ -1494,11 +1494,10 @@ describe('commit command', () => { assert.strictEqual(output.committed, true, 'should have committed'); // #3079: verify branch is created but NOT switched to (decimal phase) - const { execFileSync } = require('child_process'); - const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir, encoding: 'utf-8' }).trim(); + const branch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); assert.notStrictEqual(branch, 'gsd/phase-45.14-golden-capture', '#3079: must NOT switch to the phase branch'); // Verify the correct branch name was resolved (not integer-only) - const branchExists = execFileSync('git', ['rev-parse', '--verify', 'gsd/phase-45.14-golden-capture'], { cwd: tmpDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); + const branchExists = gitOrThrow(['rev-parse', '--verify', 'gsd/phase-45.14-golden-capture'], { cwd: tmpDir }); assert.ok(branchExists.trim(), 'decimal phase branch should be created (45.14, not 14)'); }); @@ -1559,24 +1558,23 @@ describe('commit command', () => { // #3079: the commit no longer switches to the phase branch. The phase-07 // branch should be CREATED (resolving correctly to 07, not the archived 02), // but the commit lands on the current branch. - const branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir, encoding: 'utf-8' }).trim(); + const branch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); assert.notStrictEqual( branch, 'gsd/phase-02-archived-phase', `must NOT be on the archived phase-02 branch (got ${branch})` ); // Verify the correct phase-07 branch was created (not the archived 02) - const phase07Exists = execFileSync( - 'git', ['rev-parse', '--verify', 'gsd/phase-07-active-phase'], - { cwd: tmpDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] } + const phase07Exists = gitOrThrow( + ['rev-parse', '--verify', 'gsd/phase-07-active-phase'], + { cwd: tmpDir } ); assert.ok(phase07Exists.trim(), 'phase-07 branch should be created (not the archived phase-02)'); // The committed file must exist on HEAD, proving the commit landed. - const committedFile = execFileSync( - 'git', + const committedFile = gitOrThrow( ['show', 'HEAD:.planning/phases/PROJECT_V2-07-active-phase/07-CONTEXT.md'], - { cwd: tmpDir, encoding: 'utf-8' } + { cwd: tmpDir } ); assert.ok(committedFile.includes('# Context'), 'phase-07 file must be in the commit'); }); @@ -1611,36 +1609,29 @@ describe('commit command', () => { // default branch so the working tree is NOT on the phase branch when commit // runs. The resolved branch already exists; the pre-fix code silently // switched onto it. - execFileSync('git', ['branch', 'gsd/phase-01-first-phase'], { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['branch', 'gsd/phase-01-first-phase'], { cwd: tmpDir }); // Ensure the file is staged only by the commit command itself (it must run // from the current/default branch and must not be force-switched). - const beforeBranch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: tmpDir, encoding: 'utf-8', - }).trim(); + const beforeBranch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); - // Invoke gsd-tools via spawnSync so stderr is observable on the success - // path — the warning that proves the no-switch path is not silent (#2539 - // AC2) is written to stderr, which execFileSync discards on success. + // Invoke gsd-tools via the process seam so stderr is observable on the + // success path — the warning that proves the no-switch path is not silent + // (#2539 AC2) is written to stderr, which execFileSync discards on success. const { TOOLS_PATH } = require('./helpers.cjs'); - const { spawnSync } = require('child_process'); - const proc = spawnSync(process.execPath, [ + const proc = runNode([ TOOLS_PATH, 'commit', 'docs(01): add context', '--files', '.planning/phases/01-first-phase/01-CONTEXT.md', - ], { cwd: tmpDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); + ], { cwd: tmpDir }); + throwIfFailed(proc, 'gsd-tools commit (#2539 no-switch fixture)'); const stdout = proc.stdout || ''; const stderr = proc.stderr || ''; - if (proc.status !== 0) { - throw new Error(`gsd-tools commit exited ${proc.status}: stdout=${stdout} stderr=${stderr}`); - } const output = JSON.parse(stdout.trim()); assert.strictEqual(output.committed, true, 'should have committed'); // The command must NOT have silently switched the working tree onto the // pre-existing phase branch. The commit lands on the branch we were on. - const afterBranch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: tmpDir, encoding: 'utf-8', - }).trim(); + const afterBranch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); assert.strictEqual( afterBranch, beforeBranch, @@ -2163,15 +2154,14 @@ describe('stats command', () => { }); test('reports git commit count and first commit date from repository history', () => { - execSync('git init', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config user.email "test@example.com"', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config user.name "Test User"', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.name', 'Test User'], { cwd: tmpDir }); fs.writeFileSync(path.join(tmpDir, '.planning', 'PROJECT.md'), '# Project\n'); - execSync('git add -A', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "initial commit"', { + gitOrThrow(['add', '-A'], { cwd: tmpDir }); + gitOrThrow(['commit', '-m', 'initial commit'], { cwd: tmpDir, - stdio: 'pipe', env: { ...process.env, GIT_AUTHOR_DATE: '2026-01-01T00:00:00Z', @@ -2180,10 +2170,9 @@ describe('stats command', () => { }); fs.writeFileSync(path.join(tmpDir, 'README.md'), '# Updated\n'); - execSync('git add README.md', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "second commit"', { + gitOrThrow(['add', 'README.md'], { cwd: tmpDir }); + gitOrThrow(['commit', '-m', 'second commit'], { cwd: tmpDir, - stdio: 'pipe', env: { ...process.env, GIT_AUTHOR_DATE: '2026-02-01T00:00:00Z', @@ -2425,7 +2414,7 @@ describe('check-commit command', () => { ); // Stage a non-planning file fs.writeFileSync(path.join(tmpDir, 'src.js'), 'console.log("hi")'); - execSync('git add src.js', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['add', 'src.js'], { cwd: tmpDir }); const result = runGsdTools('check-commit', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); @@ -2439,7 +2428,7 @@ describe('check-commit command', () => { JSON.stringify({ commit_docs: false }) ); fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State'); - execSync('git add .planning/STATE.md', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['add', '.planning/STATE.md'], { cwd: tmpDir }); const result = runGsdTools('check-commit', tmpDir); assert.ok(!result.success, 'should block commit'); @@ -2650,25 +2639,23 @@ describe('pr-subrepo', () => { function initPrSubrepo(dir) { fs.mkdirSync(dir, { recursive: true }); - execFileSync('git', ['init'], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: dir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: dir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: dir }); fs.writeFileSync(path.join(dir, '.gitkeep'), ''); fs.writeFileSync(path.join(dir, 'feature.js'), '// initial\n'); fs.writeFileSync(path.join(dir, 'a.js'), '// initial\n'); fs.writeFileSync(path.join(dir, 'b.js'), '// initial\n'); - execFileSync('git', ['add', '.gitkeep', 'feature.js', 'a.js', 'b.js'], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['commit', '-m', 'chore: initial commit'], { cwd: dir, stdio: 'pipe' }); + gitOrThrow(['add', '.gitkeep', 'feature.js', 'a.js', 'b.js'], { cwd: dir }); + gitOrThrow(['commit', '-m', 'chore: initial commit'], { cwd: dir }); } function wirePrSubrepoRemote(repoDir, bareDir) { fs.mkdirSync(bareDir, { recursive: true }); - execFileSync('git', ['init', '--bare'], { cwd: bareDir, stdio: 'pipe' }); - execFileSync('git', ['remote', 'add', 'origin', bareDir], { cwd: repoDir, stdio: 'pipe' }); - const branch = execFileSync('git', ['branch', '--show-current'], { - cwd: repoDir, encoding: 'utf8', - }).trim(); - execFileSync('git', ['push', 'origin', branch], { cwd: repoDir, stdio: 'pipe' }); + gitOrThrow(['init', '--bare'], { cwd: bareDir }); + gitOrThrow(['remote', 'add', 'origin', bareDir], { cwd: repoDir }); + const branch = gitOrThrow(['branch', '--show-current'], { cwd: repoDir }).trim(); + gitOrThrow(['push', 'origin', branch], { cwd: repoDir }); } describe('regressions (#666 — cmdPrSubrepo seam)', () => { @@ -2827,13 +2814,13 @@ describe('pr-subrepo', () => { // Wire a bare remote with a pre-receive hook that rejects all pushes. const rejectingBare = path.join(rootDir, '_rejecting-bare.git'); fs.mkdirSync(rejectingBare, { recursive: true }); - execFileSync('git', ['init', '--bare'], { cwd: rejectingBare, stdio: 'pipe' }); + gitOrThrow(['init', '--bare'], { cwd: rejectingBare }); const hookPath = path.join(rejectingBare, 'hooks', 'pre-receive'); fs.writeFileSync(hookPath, '#!/bin/sh\nexit 1\n'); fs.chmodSync(hookPath, 0o755); // Point origin at the rejecting bare (overwrite the working one wired in beforeEach). - execFileSync('git', ['remote', 'set-url', 'origin', rejectingBare], { cwd: subDir, stdio: 'pipe' }); + gitOrThrow(['remote', 'set-url', 'origin', rejectingBare], { cwd: subDir }); fs.writeFileSync(path.join(subDir, 'feature.js'), 'IMPORTANT USER WORK\n'); @@ -2847,21 +2834,17 @@ describe('pr-subrepo', () => { assert.ok(!res.success, `Expected failure on rejected push, got success: ${res.output}`); // The local branch must still exist — work must not be lost. - const branches = execFileSync('git', ['branch', '--list', branch], { - cwd: subDir, encoding: 'utf8', - }); + const branches = gitOrThrow(['branch', '--list', branch], { cwd: subDir }); assert.ok(branches.trim().length > 0, `Branch ${branch} was deleted after push failure — user work lost`); // The commit on that branch must contain the user's changes. - const log = execFileSync('git', ['log', branch, '--oneline', '-1'], { - cwd: subDir, encoding: 'utf8', - }); + const log = gitOrThrow(['log', branch, '--oneline', '-1'], { cwd: subDir }); assert.ok(log.trim().length > 0, `No commit on ${branch} — staged work was lost`); }); test('pr-subrepo porcelain: staged rename — both old and new paths in result.files', () => { // git mv produces "R old -> new" in porcelain v1; both paths must be staged. - execFileSync('git', ['mv', 'feature.js', 'renamed-feature.js'], { cwd: subDir, stdio: 'pipe' }); + gitOrThrow(['mv', 'feature.js', 'renamed-feature.js'], { cwd: subDir }); const res = runGsdTools( ['query', 'pr-subrepo', 'fix(backend): rename', @@ -2877,8 +2860,8 @@ describe('pr-subrepo', () => { test('pr-subrepo porcelain: non-ASCII filename (core.quotePath=false)', () => { // Without -c core.quotePath=false, "café.js" is C-escaped → slice(2) parse breaks. fs.writeFileSync(path.join(subDir, 'café.js'), '// initial\n'); - execFileSync('git', ['add', 'café.js'], { cwd: subDir, stdio: 'pipe' }); - execFileSync('git', ['commit', '-m', 'chore: add café.js'], { cwd: subDir, stdio: 'pipe' }); + gitOrThrow(['add', 'café.js'], { cwd: subDir }); + gitOrThrow(['commit', '-m', 'chore: add café.js'], { cwd: subDir }); fs.writeFileSync(path.join(subDir, 'café.js'), 'updated\n'); const res = runGsdTools( @@ -2986,12 +2969,12 @@ describe('pr-subrepo', () => { // making the assertions vacuous. A tracked modification ensures that WITHOUT the // guard the repo WOULD be reported dirty, so the test genuinely fails-first. const initDirtyRepo = (dir, file) => { - execFileSync('git', ['init'], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: dir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: dir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: dir }); fs.writeFileSync(path.join(dir, file), 'committed\n'); - execFileSync('git', ['add', file], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['-c', 'commit.gpgsign=false', 'commit', '-m', 'init'], { cwd: dir, stdio: 'pipe' }); + gitOrThrow(['add', file], { cwd: dir }); + gitOrThrow(['-c', 'commit.gpgsign=false', 'commit', '-m', 'init'], { cwd: dir }); fs.writeFileSync(path.join(dir, file), 'modified\n'); }; @@ -3020,7 +3003,8 @@ describe('pr-subrepo', () => { const subReposJson = JSON.stringify(entries); try { - execFileSync('node', ['-e', script, subReposJson, scanRoot, dirtyFile], { stdio: 'pipe' }); + const scanResult = runNode(['-e', script, subReposJson, scanRoot, dirtyFile]); + throwIfFailed(scanResult, 'node -e '); const dirty = fs.existsSync(dirtyFile) ? fs.readFileSync(dirtyFile, 'utf-8') : ''; const lines = dirty.split('\n').filter(Boolean); assert.ok( @@ -3453,18 +3437,23 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { spawnSync } = require('node:child_process'); - const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); const GSD_TOOLS = path.resolve(__dirname, '../gsd-core/bin/gsd-tools.cjs'); +// This is a DISTINCT, independently-scoped `runCli` — not the file's other +// local helper of a similar shape (`runGsdTools` above, folded from +// feat-3251, which already carries its own `timeout: 30000`). This one +// returns the legacy `{status, stdout, stderr}` shape its callers below read +// directly (never a throw contract), so it is bounded via `runNode` + +// `toLegacyResult` rather than `gitOrThrow`/`throwIfFailed`. function runCli(args, env = {}) { - const result = spawnSync(process.execPath, [GSD_TOOLS, ...args], { - encoding: 'utf8', + const result = runNode([GSD_TOOLS, ...args], { env: { ...process.env, GSD_TEST_MODE: '1', ...env }, }); - return result; + return toLegacyResult(result); } function makeTmpDir(prefix) { diff --git a/tests/commonjs-marker.test.cjs b/tests/commonjs-marker.test.cjs index 80c514492..d3375cf2f 100644 --- a/tests/commonjs-marker.test.cjs +++ b/tests/commonjs-marker.test.cjs @@ -33,9 +33,10 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const crypto = require('node:crypto'); -const { spawnSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { BUILD_TIMEOUT_MS, INSTALL_TIMEOUT_MS, PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { COMMONJS_MARKER, @@ -75,15 +76,14 @@ function mkTmp(prefix) { function runInstall(root, runtime, extraArgs = []) { const env = { ...process.env, HOME: root, USERPROFILE: root, CLAUDE_CONFIG_DIR: root }; delete env.GSD_TEST_MODE; - const result = spawnSync( - process.execPath, + const result = runNode( [INSTALL_SCRIPT, `--${runtime}`, '--global', '--config-dir', root, ...extraArgs], - { cwd: root, encoding: 'utf8', env }, + { cwd: root, env, timeoutMs: INSTALL_TIMEOUT_MS }, ); assert.equal( - result.status, + result.exitCode, 0, - `installer exited ${result.status}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`, + `installer exited ${result.exitCode}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`, ); return result; } @@ -354,8 +354,8 @@ describe('#2544 regression: install must not clobber the config-root package.jso // hooks/dist is gitignored and built; scoped CI lanes do not run build:hooks, // so build it idempotently before driving a real install. before(() => { - const build = spawnSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf8' }); - assert.equal(build.status, 0, `build:hooks failed: ${build.stderr}`); + const build = runNode([BUILD_SCRIPT], { timeoutMs: BUILD_TIMEOUT_MS }); + assert.equal(build.exitCode, 0, `build:hooks failed: ${build.stderr}`); }); for (const runtime of ['opencode', 'claude']) { @@ -410,13 +410,12 @@ describe('#2544 regression: install must not clobber the config-root package.jso // ERR_REQUIRE_ESM / "require is not defined" — the regression AC3 forbids. const target = path.join(root, 'hooks', 'lib', 'git-cmd.js'); assert.ok(fs.existsSync(target), 'hooks/lib/git-cmd.js must be staged'); - const probe = spawnSync( - process.execPath, + const probe = runNode( ['-e', `const m = require(${JSON.stringify(target)}); if (typeof m.isGitSubcommand !== 'function') { throw new Error('unexpected exports'); } console.log('loaded');`], - { cwd: root, encoding: 'utf8' }, + { cwd: root, timeoutMs: PROBE_TIMEOUT_MS }, ); assert.equal( - probe.status, + probe.exitCode, 0, `staged hook helper must load as CommonJS under an ESM config root\nstderr: ${probe.stderr}`, ); diff --git a/tests/config-get-default.test.cjs b/tests/config-get-default.test.cjs index 948807f52..e5e0e3e0b 100644 --- a/tests/config-get-default.test.cjs +++ b/tests/config-get-default.test.cjs @@ -674,29 +674,22 @@ 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 { execFileSync } = require('node:child_process'); const { createTempProject, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const SDK_CLI = path.join(REPO_ROOT, 'sdk', 'dist', 'cli.js'); function runConfigSet(key, value, projectDir) { const argv = ['query', 'config-set', key, String(value), '--project-dir', projectDir]; - let stdout = ''; - let exitCode = 0; - try { - stdout = execFileSync(process.execPath, [SDK_CLI, ...argv], { - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], - env: { ...process.env, GSD_SESSION_KEY: '' }, - }); - } catch (err) { - exitCode = err.status ?? 1; - stdout = err.stdout?.toString() ?? ''; - } + const result = runNode([SDK_CLI, ...argv], { + env: { ...process.env, GSD_SESSION_KEY: '' }, + timeoutMs: PROBE_TIMEOUT_MS, + }); let json = null; - try { json = JSON.parse(stdout.trim()); } catch { /* ok */ } - return { exitCode, json }; + try { json = JSON.parse(result.stdout.trim()); } catch { /* ok */ } + return { exitCode: result.exitCode, json }; } describe('bug-2798: context_window is a valid config key', () => { diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index f9c8f4e55..1e3e89cdf 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -569,15 +569,15 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { execFileSync } = require('child_process'); const { createTempProject, cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const { loadConfig } = require('../gsd-core/bin/lib/config-loader.cjs'); function makeSubRepo(parent, name) { const dir = path.join(parent, name); fs.mkdirSync(dir, { recursive: true }); - execFileSync('git', ['init'], { cwd: dir, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: dir }); } function readConfig(tmpDir) { @@ -740,8 +740,9 @@ const { describe, test, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); const { createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const TEST_ENV_BASE = { GSD_SESSION_KEY: '', @@ -765,15 +766,15 @@ const TEST_ENV_BASE = { * Always captures stderr even when exit code is 0. */ function runWithStderr(args, cwd, env = {}) { - const result = spawnSync(process.execPath, [TOOLS_PATH, ...args], { + const result = runNode([TOOLS_PATH, ...args], { cwd, - encoding: 'utf-8', env: { ...process.env, ...TEST_ENV_BASE, ...env }, + timeoutMs: PROBE_TIMEOUT_MS, }); return { stdout: result.stdout || '', stderr: result.stderr || '', - status: result.status, + status: result.exitCode, }; } diff --git a/tests/config.test.cjs b/tests/config.test.cjs index 2f05699bd..b51f793d5 100644 --- a/tests/config.test.cjs +++ b/tests/config.test.cjs @@ -2290,11 +2290,13 @@ describe('feat-3210 / H5: enum validation for code_quality.fallow.scope and .pro const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { spawnSync } = require('node:child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.join(__dirname, '..'); @@ -2303,10 +2305,11 @@ function read(relativePath) { } function runGsd(args, cwd) { - return spawnSync(process.execPath, [path.join(ROOT, 'gsd-core/bin/gsd-tools.cjs'), ...args], { + const result = runNode([path.join(ROOT, 'gsd-core/bin/gsd-tools.cjs'), ...args], { cwd, - encoding: 'utf8', + timeoutMs: PROBE_TIMEOUT_MS, }); + return toLegacyResult(result); } describe('bug #3212 execute-phase stall detection and safe resume', () => { diff --git a/tests/configuration-migrate-config.test.cjs b/tests/configuration-migrate-config.test.cjs index 3ea1537c3..d98baea6e 100644 --- a/tests/configuration-migrate-config.test.cjs +++ b/tests/configuration-migrate-config.test.cjs @@ -15,8 +15,9 @@ const { describe, test, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); const { createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const TEST_ENV_BASE = { GSD_SESSION_KEY: '', @@ -36,15 +37,15 @@ const TEST_ENV_BASE = { }; function runMigrateConfig(cwd, extraArgs = [], env = {}) { - const result = spawnSync(process.execPath, [TOOLS_PATH, 'migrate-config', ...extraArgs], { + const result = runNode([TOOLS_PATH, 'migrate-config', ...extraArgs], { cwd, - encoding: 'utf-8', env: { ...process.env, ...TEST_ENV_BASE, ...env }, + timeoutMs: PROBE_TIMEOUT_MS, }); return { stdout: result.stdout || '', stderr: result.stderr || '', - status: result.status, + status: result.exitCode, }; } diff --git a/tests/drift-detection.test.cjs b/tests/drift-detection.test.cjs index 1f52a1c35..f7967d923 100644 --- a/tests/drift-detection.test.cjs +++ b/tests/drift-detection.test.cjs @@ -17,13 +17,14 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); const { createTempProject, createTempGitProject, cleanup, runGsdTools, } = require('./helpers.cjs'); +const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { runHook } = require('./helpers/process-seam.cjs'); const DRIFT_PATH = path.join( __dirname, @@ -52,9 +53,11 @@ const { DRIFT_CATEGORIES, } = require(DRIFT_PATH); -// Small wrapper around execFileSync so tests don't sprinkle shell=true calls. +// Small wrapper so tests don't sprinkle shell=true calls. Routed through +// gitOrThrow (bounded, throw-on-failure) rather than bare `runGit` — this +// helper's 16 callers all rely on the throw-on-failure contract. function git(cwd, ...args) { - return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }).trim(); + return gitOrThrow(args, { cwd }).trim(); } // ─── Unit: classifyFile ────────────────────────────────────────────────────── @@ -823,7 +826,6 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); const { cleanup, readFileNormalized } = require('./helpers.cjs'); const GATE_MD = path.join( @@ -832,9 +834,9 @@ const GATE_MD = path.join( const SNIPPET_FILE = path.join(__dirname, '..', 'gsd-core', 'workflows', '_runtime-launcher.snippet.sh'); // readFileNormalized() strips \r\n -> \n before bashBlock() slices a fence -// out of the result and hands it to execFileSync('bash', ...) below — an -// un-normalized read on a Windows checkout would break bash mid-script -// (DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE, #2650). +// out of the result and hands it to runHook('-c', ..., {interpreter:'bash'}) +// below — an un-normalized read on a Windows checkout would break bash +// mid-script (DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE, #2650). function readGate() { return readFileNormalized(GATE_MD); } @@ -923,10 +925,12 @@ describe('bug #619 — codebase-drift-gate resolves gsd-tools via the runtime sh ); const block = bashBlock(readGate(), 0) + '\nprintf "%s" "$DRIFT"\n'; - const out = execFileSync('bash', ['-c', block], { + const shimResult = runHook('-c', [block], { + interpreter: 'bash', env: { ...process.env, RUNTIME_DIR: tmp }, - encoding: 'utf8', }); + throwIfFailed(shimResult, 'bash -c '); + const out = shimResult.stdout; assert.match(out, /SHIM_RAN/, 'the drift check must execute the resolved shim, proving gsd_run resolution'); assert.doesNotMatch(out, /sdk-failed/, 'the gate must NOT silently skip when the shim is present (#619)'); @@ -940,10 +944,12 @@ describe('bug #619 — codebase-drift-gate resolves gsd-tools via the runtime sh // hits the 127 → `|| echo` skip path even though the shim (gsd-tools.cjs) exists. const oldForm = 'DRIFT=$(gsd-tools verify codebase-drift 2>/dev/null || echo \'{"skipped":true,"reason":"sdk-failed"}\'); printf "%s" "$DRIFT"'; - const out = execFileSync('bash', ['-c', 'export PATH=/nonexistent-empty-path; ' + oldForm], { + const oldFormResult = runHook('-c', ['export PATH=/nonexistent-empty-path; ' + oldForm], { + interpreter: 'bash', env: { ...process.env }, - encoding: 'utf8', }); + throwIfFailed(oldFormResult, 'bash -c <#619 bare-binary red-proof>'); + const out = oldFormResult.stdout; assert.match(out, /sdk-failed/, 'sanity: the bare-binary form skips without gsd-tools on PATH — the bug the fix removes'); }); }); diff --git a/tests/edge-probe.test.cjs b/tests/edge-probe.test.cjs index 660ef4ec9..90f1a4e30 100644 --- a/tests/edge-probe.test.cjs +++ b/tests/edge-probe.test.cjs @@ -20,8 +20,10 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const fs = require('node:fs'); const os = require('node:os'); -const { execFileSync, spawnSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const BUILT_SCRIPT = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'edge-probe.cjs'); const ep = require(BUILT_SCRIPT); @@ -148,19 +150,14 @@ describe('edge-probe: CLI (built artifact)', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'edge-probe-')); const reqPath = path.join(dir, 'requirements.json'); fs.writeFileSync(reqPath, JSON.stringify([{ id: 'R1', text: 'Round a number to N decimal places' }])); - const out = execFileSync('node', [BUILT_SCRIPT, reqPath], { encoding: 'utf8' }); - const rep = JSON.parse(out); + const nodeResult = runNode([BUILT_SCRIPT, reqPath], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(nodeResult, `node ${BUILT_SCRIPT} ${reqPath}`); + const rep = JSON.parse(nodeResult.stdout); assert.deepEqual(rep.coverage, { applicable: 2, resolved: 0, unresolved: 2, byVerification: { explicit: 0, backstop: 0 } }); }); test('with no args exits with status 2 (assert on exit code, not stderr prose)', () => { - let status; - try { - execFileSync('node', [BUILT_SCRIPT], { stdio: 'pipe' }); - status = 0; - } catch (error) { - status = error.status; - } - assert.equal(status, 2); + const result = runNode([BUILT_SCRIPT], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(result.exitCode, 2); }); }); @@ -170,8 +167,8 @@ describe('edge-probe: CLI JSON.parse error handling (RR-10)', () => { const badJson = path.join(dir, 'bad-req.json'); fs.writeFileSync(badJson, 'not valid json {{{'); try { - const r = spawnSync(process.execPath, [BUILT_SCRIPT, badJson], { stdio: 'pipe' }); - assert.equal(r.status, 2); + const r = runNode([BUILT_SCRIPT, badJson], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(r.exitCode, 2); } finally { cleanup(dir); } @@ -183,8 +180,8 @@ describe('edge-probe: CLI JSON.parse error handling (RR-10)', () => { fs.writeFileSync(goodReq, JSON.stringify([{ id: 'R1', text: 'Round a number to N decimal places' }])); fs.writeFileSync(badRes, 'not valid json {{{'); try { - const r = spawnSync(process.execPath, [BUILT_SCRIPT, goodReq, badRes], { stdio: 'pipe' }); - assert.equal(r.status, 2); + const r = runNode([BUILT_SCRIPT, goodReq, badRes], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(r.exitCode, 2); } finally { cleanup(dir); } @@ -194,8 +191,8 @@ describe('edge-probe: CLI JSON.parse error handling (RR-10)', () => { const reqPath = path.join(dir, 'req.json'); fs.writeFileSync(reqPath, JSON.stringify([{ id: 'R1', text: 'Round a number to N decimal places' }])); try { - const r = spawnSync(process.execPath, [BUILT_SCRIPT, reqPath], { stdio: 'pipe', encoding: 'utf8' }); - assert.equal(r.status, 0); + const r = runNode([BUILT_SCRIPT, reqPath], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(r.exitCode, 0); const rep = JSON.parse(r.stdout); assert.deepEqual(rep.coverage, { applicable: 2, resolved: 0, unresolved: 2, byVerification: { explicit: 0, backstop: 0 } }); } finally { @@ -303,8 +300,8 @@ describe('edge-probe: proposeEdges — invalid authored shapes fail closed (re-r const reqPath = path.join(dir, 'req.json'); fs.writeFileSync(reqPath, JSON.stringify([{ id: 'R1', text: 'Round a number', shapes: ['numeric'] }])); try { - const r = spawnSync(process.execPath, [BUILT_SCRIPT, reqPath], { stdio: 'pipe' }); - assert.equal(r.status, 2); + const r = runNode([BUILT_SCRIPT, reqPath], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(r.exitCode, 2); } finally { cleanup(dir); } diff --git a/tests/execute-wave-post-gate-pipeline-e2e.test.cjs b/tests/execute-wave-post-gate-pipeline-e2e.test.cjs index d92fcef6a..64e7c2363 100644 --- a/tests/execute-wave-post-gate-pipeline-e2e.test.cjs +++ b/tests/execute-wave-post-gate-pipeline-e2e.test.cjs @@ -30,6 +30,7 @@ const path = require('node:path'); const { spawnSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const GSD_TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -37,9 +38,7 @@ const GSD_TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs') // Inline — NOT modifying tests/helpers.cjs per task rules. function gitSync(args, cwd) { - const r = spawnSync('git', args, { cwd, encoding: 'utf8', env: { ...process.env, GIT_AUTHOR_NAME: 'Test', GIT_AUTHOR_EMAIL: 'test@test.com', GIT_COMMITTER_NAME: 'Test', GIT_COMMITTER_EMAIL: 'test@test.com' } }); - if (r.status !== 0) throw new Error(`git ${args.join(' ')} failed: ${r.stderr}`); - return r.stdout.trim(); + return gitOrThrow(args, { cwd, env: { ...process.env, GIT_AUTHOR_NAME: 'Test', GIT_AUTHOR_EMAIL: 'test@test.com', GIT_COMMITTER_NAME: 'Test', GIT_COMMITTER_EMAIL: 'test@test.com' } }).trim(); } function initGitRepo(dir) { diff --git a/tests/fix-2136-clock-local-today.test.cjs b/tests/fix-2136-clock-local-today.test.cjs index 108871f0b..4b5582db6 100644 --- a/tests/fix-2136-clock-local-today.test.cjs +++ b/tests/fix-2136-clock-local-today.test.cjs @@ -21,13 +21,15 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { execFileSync } = require('node:child_process'); const path = require('node:path'); const CLOCK_CJS = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'clock.cjs'); const STATE_TRANSITION_CJS = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'state-transition.cjs'); const { stateExtractField } = require('../gsd-core/bin/lib/state-document.cjs'); const { makeFakeClock } = require('./helpers/clock.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // 2020-06-15T02:00:00.000Z. Under America/Chicago (UTC−5 in June) this instant // is 2020-06-14 21:00 local — the local calendar day is 2020-06-14 while the UTC @@ -40,10 +42,9 @@ const PINNED_MS = '1592186400000'; * versions). GSD_TEST_MODE + GSD_NOW_MS pin realClock via the documented seam. */ function clockInSubprocess(expr, env) { - return execFileSync(process.execPath, ['-e', expr], { - env, - encoding: 'utf8', - }).trim(); + const result = runNode(['-e', expr], { env, timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(result, `node -e ${expr}`); + return result.stdout.trim(); } describe('#2136 realClock.localToday() — host-local calendar day', () => { diff --git a/tests/fix-2590-workflow-script-contract.test.cjs b/tests/fix-2590-workflow-script-contract.test.cjs index 38721627f..c4bc17a12 100644 --- a/tests/fix-2590-workflow-script-contract.test.cjs +++ b/tests/fix-2590-workflow-script-contract.test.cjs @@ -32,8 +32,10 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); const { createTempDir, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const core = require('../gsd-core/bin/lib/claude-orchestration.cjs'); const TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -74,9 +76,8 @@ describe('#2590: emitted Workflow scripts satisfy the Workflow tool contract', ( const f = path.join(dir, 'emitted.mjs'); fs.writeFileSync(f, script); try { - execFileSync(process.execPath, ['--check', f], { stdio: 'pipe' }); - } catch (e) { - assert.fail(`emitted script does not parse: ${e.stderr ? e.stderr.toString() : e.message}`); + const result = runNode(['--check', f], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(result, `node --check ${f} (emitted script must parse)`); } finally { cleanup(dir); } @@ -211,13 +212,14 @@ describe('#2590: the backend is reachable without hand-passed flags', () => { } function resolve(dir, extraArgs) { - const out = execFileSync(process.execPath, [ + const result = runNode([ TOOLS, 'claude-orchestration', 'resolve-wave-dispatch', '--waves', 'waves.json', '--run-id', 'execute-1', '--phase-dir', '.planning/phases/01', '--raw', ...(extraArgs || []), - ], { cwd: dir, encoding: 'utf8' }); - return JSON.parse(out); + ], { cwd: dir, timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(result, 'gsd-tools claude-orchestration resolve-wave-dispatch'); + return JSON.parse(result.stdout); } test('5+6. no --runtime and no --agent-sdk-version still reaches the version gate', () => { @@ -264,12 +266,13 @@ describe('#2590: the backend is reachable without hand-passed flags', () => { test('GSD_AGENT_SDK_VERSION is honored between the flag and the installed version', () => { const dir = repro(); try { - const out = execFileSync(process.execPath, [ + const result = runNode([ TOOLS, 'claude-orchestration', 'resolve-wave-dispatch', '--waves', 'waves.json', '--run-id', 'execute-1', '--phase-dir', '.planning/phases/01', '--raw', - ], { cwd: dir, encoding: 'utf8', env: { ...process.env, GSD_AGENT_SDK_VERSION: '0.3.149' } }); - assert.equal(JSON.parse(out).backend, 'workflow'); + ], { cwd: dir, env: { ...process.env, GSD_AGENT_SDK_VERSION: '0.3.149' }, timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(result, 'gsd-tools claude-orchestration resolve-wave-dispatch (GSD_AGENT_SDK_VERSION)'); + assert.equal(JSON.parse(result.stdout).backend, 'workflow'); } finally { cleanup(dir); } diff --git a/tests/fix-2650-plan-phase-stall-detection.test.cjs b/tests/fix-2650-plan-phase-stall-detection.test.cjs index a384c342c..a33adffc3 100644 --- a/tests/fix-2650-plan-phase-stall-detection.test.cjs +++ b/tests/fix-2650-plan-phase-stall-detection.test.cjs @@ -37,6 +37,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const { spawnSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); @@ -407,8 +410,8 @@ describe('bug #2650 config schema — planner.stall_* keys mirror executor.stall fs.writeFileSync(path.join(tmp, '.planning/config.json'), '{}\n'); const toolsPath = path.join(REPO_ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs'); - const interval = spawnSync(process.execPath, [toolsPath, 'config-get', 'planner.stall_detect_interval_minutes', '--raw'], { cwd: tmp, encoding: 'utf-8' }); - const threshold = spawnSync(process.execPath, [toolsPath, 'config-get', 'planner.stall_threshold_minutes', '--raw'], { cwd: tmp, encoding: 'utf-8' }); + const interval = toLegacyResult(runNode([toolsPath, 'config-get', 'planner.stall_detect_interval_minutes', '--raw'], { cwd: tmp, timeoutMs: PROBE_TIMEOUT_MS })); + const threshold = toLegacyResult(runNode([toolsPath, 'config-get', 'planner.stall_threshold_minutes', '--raw'], { cwd: tmp, timeoutMs: PROBE_TIMEOUT_MS })); assert.equal(interval.status, 0, interval.stderr); assert.equal(interval.stdout.trim(), '5'); diff --git a/tests/fix-2657-untrack-compiled-artifacts.test.cjs b/tests/fix-2657-untrack-compiled-artifacts.test.cjs index c2f01444a..e3ef77898 100644 --- a/tests/fix-2657-untrack-compiled-artifacts.test.cjs +++ b/tests/fix-2657-untrack-compiled-artifacts.test.cjs @@ -36,7 +36,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); +const { runGit, runNode, OUTCOME } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { trackedCompiledArtifacts } = require('../scripts/lint-compiled-artifact-sync.cjs'); @@ -56,19 +58,22 @@ const NINE_ARTIFACTS = [ ].map((name) => `${LIB_DIR}/${name}`); /** - * Run a command via spawnSync (never throws) and return the raw result. - * Throws immediately, with full context, only on a genuine spawn failure - * (binary not found, etc.) — a condition no caller here can meaningfully - * interpret as a match/no-match answer. + * Run a command via the process seam (never throws) and return a legacy + * `{status, stdout, stderr, signal}` shape. `cmd` is either `'git'` (routed + * through `runGit`) or `process.execPath` (routed through `runNode`) — the + * only two callers below. Throws immediately, with full context, only on a + * genuine spawn failure (binary not found, etc.) — a condition no caller + * here can meaningfully interpret as a match/no-match answer. */ function run(cmd, args, opts) { - const result = spawnSync(cmd, args, { cwd: REPO_ROOT, encoding: 'utf8', ...opts }); - if (result.error) { + const options = { cwd: REPO_ROOT, timeoutMs: PROBE_TIMEOUT_MS, ...opts }; + const result = cmd === 'git' ? runGit(args, options) : runNode(args, options); + if (result.outcome === OUTCOME.SPAWN_FAILED) { throw new Error( - `${cmd} ${args.join(' ')} failed to spawn (cwd=${REPO_ROOT}): ${result.error.message}`, + `${cmd} ${args.join(' ')} failed to spawn (cwd=${REPO_ROOT}): ${result.stderr || result.code}`, ); } - return result; + return { ...toLegacyResult(result), signal: result.signal }; } /** Render a failed command's full context for an assertion message. */ diff --git a/tests/fix-3045-cursor-subagent-isolation.test.cjs b/tests/fix-3045-cursor-subagent-isolation.test.cjs index 20d74b400..437fb8c03 100644 --- a/tests/fix-3045-cursor-subagent-isolation.test.cjs +++ b/tests/fix-3045-cursor-subagent-isolation.test.cjs @@ -27,9 +27,11 @@ const { describe, test, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { execFileSync, spawnSync } = require('node:child_process'); const { createTempDir, cleanup } = require('./helpers.cjs'); const { SENTINEL_RELATIVE_PATH, SENTINEL_STALE_MS } = require('../hooks/lib/isolation-sentinel.js'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { gitOrThrow, toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-cursor-subagent-start.js'); @@ -56,12 +58,12 @@ function runHook(payload, extraEnv = {}) { // `USERPROFILE` too so that redirection actually takes effect on Windows // instead of silently leaking the real CI runner's profile directory. if ('HOME' in extraEnv) env.USERPROFILE = extraEnv.HOME; - return spawnSync(process.execPath, [HOOK_PATH], { + return toLegacyResult(runNode([HOOK_PATH], { input: typeof payload === 'string' ? payload : JSON.stringify(payload), - encoding: 'utf8', cwd: require('node:os').tmpdir(), env, - }); + timeoutMs: PROBE_TIMEOUT_MS, + })); } function subagentPayload(workspaceRoots, overrides = {}) { @@ -82,7 +84,7 @@ function subagentPayload(workspaceRoots, overrides = {}) { } function git(args, cwd) { - return execFileSync('git', args, { cwd, stdio: 'pipe', encoding: 'utf8' }); + return gitOrThrow(args, { cwd }); } /** A real git repo with a committed .planning/config.json. */ @@ -600,17 +602,17 @@ describe('executor-identity parity: hooks/gsd-agent-isolation-guard.js (Claude) test(`subagent_type="${subagentType}": Claude block-decision and Cursor deny-decision agree`, () => { const claudeEnv = { ...process.env }; delete claudeEnv.GSD_RUNTIME; - const claudeResult = spawnSync(process.execPath, [CLAUDE_HOOK_PATH], { + const claudeResult = runNode([CLAUDE_HOOK_PATH], { input: JSON.stringify({ hook_event_name: 'PreToolUse', tool_name: 'Agent', tool_input: { subagent_type: subagentType }, }), - encoding: 'utf8', cwd: claudeProject, env: claudeEnv, + timeoutMs: PROBE_TIMEOUT_MS, }); - const claudeBlocked = claudeResult.status === 2; + const claudeBlocked = claudeResult.exitCode === 2; const cursorResult = runHook(subagentPayload([cursorProject], { subagent_type: subagentType })); const cursorOut = JSON.parse(cursorResult.stdout); diff --git a/tests/fix-3045-dispatch-isolation-resolver.test.cjs b/tests/fix-3045-dispatch-isolation-resolver.test.cjs index 1eeb50507..ca93c2517 100644 --- a/tests/fix-3045-dispatch-isolation-resolver.test.cjs +++ b/tests/fix-3045-dispatch-isolation-resolver.test.cjs @@ -24,8 +24,8 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const { SENTINEL_RELATIVE_PATH, readSentinel } = require('../hooks/lib/isolation-sentinel.js'); const { runtimes } = require('../gsd-core/bin/lib/capability-registry.cjs'); @@ -263,7 +263,7 @@ describe('#3045 MAJOR — --harness-flag can now accept a bare CLI-flag value (C describe('#3045 MINOR — writer/reader sentinel path derivation now agrees for a linked worktree without its own .planning/', () => { function git(args, cwd) { - return execFileSync('git', args, { cwd, stdio: 'pipe', encoding: 'utf8' }); + gitOrThrow(args, { cwd }); } test('a sentinel written from a linked worktree (via --cwd) is found by readSentinel() called with that SAME worktree path', () => { diff --git a/tests/fixture-builder.test.cjs b/tests/fixture-builder.test.cjs index 3e8d991e6..b933fe52d 100644 --- a/tests/fixture-builder.test.cjs +++ b/tests/fixture-builder.test.cjs @@ -2,10 +2,10 @@ const { test, describe, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { execSync } = require('child_process'); const { createFixture, seedPhase, seedWorkstream, writeState } = require('./fixtures/index.cjs'); const { cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const created = []; afterEach(() => { @@ -72,10 +72,10 @@ describe('fixture builder module', () => { const dir = createFixture({ git: true }); created.push(dir); - const isWorkTree = execSync('git rev-parse --is-inside-work-tree', { cwd: dir, encoding: 'utf8' }).trim(); + const isWorkTree = gitOrThrow(['rev-parse', '--is-inside-work-tree'], { cwd: dir }).trim(); assert.strictEqual(isWorkTree, 'true', 'fixture should be a git worktree'); - const head = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + const head = gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir }).trim(); assert.ok(head.length > 0, 'fixture should include initial commit'); assert.ok( diff --git a/tests/frontmatter-cli.test.cjs b/tests/frontmatter-cli.test.cjs index ad19e3341..8c39d0f33 100644 --- a/tests/frontmatter-cli.test.cjs +++ b/tests/frontmatter-cli.test.cjs @@ -18,7 +18,9 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const cp = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { runGsdTools, parseFrontmatter } = require('./helpers.cjs'); // Track temp files for cleanup @@ -735,11 +737,12 @@ describe('frontmatter get — truncated vs absent frontmatter (#1882)', () => { const TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); function runCapturingStderr(file) { - const r = cp.spawnSync(process.execPath, [TOOLS, 'frontmatter', 'get', file, '--raw'], { - encoding: 'utf8', + const r = runNode([TOOLS, 'frontmatter', 'get', file, '--raw'], { env: { ...process.env, GSD_TEST_MODE: '1' }, + timeoutMs: PROBE_TIMEOUT_MS, }); - return { status: r.status, stdout: (r.stdout || '').trim(), stderr: (r.stderr || '').trim() }; + const legacy = toLegacyResult(r); + return { status: legacy.status, stdout: legacy.stdout.trim(), stderr: legacy.stderr.trim() }; } // This is the wired keystone for #1882: the diagnostic is only "delivered" if it reaches diff --git a/tests/git-fixture.test.cjs b/tests/git-fixture.test.cjs index a381bb135..69d16832a 100644 --- a/tests/git-fixture.test.cjs +++ b/tests/git-fixture.test.cjs @@ -126,8 +126,27 @@ describe('git-fixture: E — gitOrThrow', () => { assert.equal(caught.outcome, OUTCOME.SPAWN_FAILED); }); - test('E9: timeout throws and reports timedOut', () => { - const caught = captureThrown(() => gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir, timeoutMs: 1 })); + test('E9: gitOrThrow propagates a TIMED_OUT seam result as a throw', () => { + // Not an integration timeout test: a real `git rev-parse HEAD` racing a + // 1ms bound is a real-race test (on a warm/unloaded container the + // command can finish first, spawnSync then reports a clean EXITED with + // no error, and gitOrThrow correctly does not throw — see #3148). This + // drives `gitOrThrow` with a synthetic TIMED_OUT result from the + // `runGit` spy installed at module scope (line 30), so it exercises the + // exact propagation behavior with zero timing dependence. Deterministic + // coverage of `throwIfFailed`'s TIMED_OUT branch itself already lives in + // the `throwIfFailed` describe block below (the `for (const outcome of + // [OUTCOME.TIMED_OUT, ...])` case). + runGitSpy.mock.mockImplementationOnce(() => ({ + outcome: OUTCOME.TIMED_OUT, + exitCode: null, + stdout: '', + stderr: '', + timedOut: true, + signal: 'SIGTERM', + code: 'ETIMEDOUT', + })); + const caught = captureThrown(() => gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir })); assert.equal(caught.timedOut, true); assert.equal(caught.outcome, OUTCOME.TIMED_OUT); }); diff --git a/tests/gsd-agent-isolation-guard.test.cjs b/tests/gsd-agent-isolation-guard.test.cjs index de25e6ddc..c7a0a82c2 100644 --- a/tests/gsd-agent-isolation-guard.test.cjs +++ b/tests/gsd-agent-isolation-guard.test.cjs @@ -47,8 +47,10 @@ const { describe, test, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); const fc = require('./helpers/fast-check-setup.cjs'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); const { SENTINEL_RELATIVE_PATH, SENTINEL_STALE_MS } = require('../hooks/lib/isolation-sentinel.js'); @@ -82,12 +84,13 @@ function runHook(payload, cwd, extraEnv = {}) { // `USERPROFILE` too so that redirection actually takes effect on Windows // instead of silently leaking the real CI runner's profile directory. if ('HOME' in extraEnv) env.USERPROFILE = extraEnv.HOME; - return spawnSync(process.execPath, [HOOK_PATH], { + const r = runHookSeam(HOOK_PATH, [], { input: typeof payload === 'string' ? payload : JSON.stringify(payload), - encoding: 'utf8', cwd, env, + timeoutMs: PROBE_TIMEOUT_MS, }); + return toLegacyResult(r); } function agentPayload(overrides = {}) { diff --git a/tests/gsd-statusline.test.cjs b/tests/gsd-statusline.test.cjs index 6944b482b..6a90992d9 100644 --- a/tests/gsd-statusline.test.cjs +++ b/tests/gsd-statusline.test.cjs @@ -1969,8 +1969,8 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); - const { execFileSync } = require('node:child_process'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); + const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const statusline = require('../hooks/gsd-statusline.js'); const { parseGitStatus, buildGitSegment, readGitStatus, composeStatusline } = statusline; @@ -2077,8 +2077,7 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { describe('readGitStatus + parseGitStatus against a real repo', () => { function makeGitRepo() { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'git-seg-')); - const run = (args) => execFileSync('git', ['-C', dir, ...args], { - encoding: 'utf8', + const run = (args) => gitOrThrow(['-C', dir, ...args], { env: { ...process.env, GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_SYSTEM: '/dev/null' }, }); run(['init', '-q', '-b', 'main']); @@ -2203,7 +2202,7 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { test('flag=true renders the branch segment', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'git-seg-e2e-')); try { - execFileSync('git', ['-C', dir, 'init', '-q', '-b', 'main']); + gitOrThrow(['-C', dir, 'init', '-q', '-b', 'main']); fs.mkdirSync(path.join(dir, '.planning'), { recursive: true }); fs.writeFileSync( path.join(dir, '.planning', 'config.json'), @@ -2219,7 +2218,7 @@ test('config-set statusline.show_context_tokens yes → rejected', () => { test('default (flag absent) has no git segment', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'git-seg-e2e-')); try { - execFileSync('git', ['-C', dir, 'init', '-q', '-b', 'main']); + gitOrThrow(['-C', dir, 'init', '-q', '-b', 'main']); fs.mkdirSync(path.join(dir, '.planning'), { recursive: true }); const out = runHook(dir); assert.ok(!out.includes('│ main'), `expected no git segment; got: ${out}`); diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 56496b866..812c257f1 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -611,14 +611,21 @@ function runNpm(args, options = {}) { const defaults = { encoding: 'utf-8', shell: isWindows, - timeout: 180000, env: isolatedEnv, }; // Merge options; if caller passes their own env, merge it on top of isolatedEnv // so the isolation is preserved unless the caller explicitly overrides HOME. - const { env: callerEnv, ...otherOptions } = options; + // `timeout` is destructured with a default (not left inside `defaults`) so an + // explicit `timeout: undefined` in `options` — an own key, not an omission — + // cannot silently erase the bound via the spread below; a destructure default + // only applies on `undefined`, whereas `{ ...defaults, ...otherOptions }` + // would let that own key win and fall through to no bound at all. 180000ms: + // npm install/pack against an isolated HOME; this is the pre-existing value, + // preserved. + const NPM_TIMEOUT_MS = 180000; + const { env: callerEnv, timeout = NPM_TIMEOUT_MS, ...otherOptions } = options; const mergedEnv = callerEnv ? { ...isolatedEnv, ...callerEnv } : isolatedEnv; - return execFileSync(npmCmd, args, { ...defaults, ...otherOptions, env: mergedEnv }).trim(); + return execFileSync(npmCmd, args, { ...defaults, ...otherOptions, timeout, env: mergedEnv }).trim(); } /** diff --git a/tests/helpers/process-seam.cjs b/tests/helpers/process-seam.cjs index cde3c68e7..6ab0a6cac 100644 --- a/tests/helpers/process-seam.cjs +++ b/tests/helpers/process-seam.cjs @@ -34,6 +34,19 @@ * would silently drop the #969 kill-discrimination retry for the exact * case it exists to catch. KILLED is reported as its own outcome so the * `runGsdTools` adapter can retry it exactly like TIMED_OUT. + * - `status !== null` with a populated `result.error` -> EXITED, evidence + * over classification, checked before any error-code branch below. At + * the exact timeout boundary, spawnSync can report `error.code === + * 'ETIMEDOUT'` (the timer fired) on a result that ALSO carries + * `status: 0` (the child finished on its own first) — verified + * empirically via `tests/process-seam.test.cjs`'s at-the-bound case. + * `status` is only ever populated by a real exit, so it outranks an + * attached error: a process that returned a real exit code did not time + * out in any sense the caller cares about, and reporting TIMED_OUT while + * passing that exit code through as `exitCode` would be an incoherent + * shape. This also means every branch below may assume `status === + * null`, which is exactly the assumption the BUFFER_OVERFLOW vs + * TIMED_OUT ordering (next) already relies on. * - `result.error.code` is a buffer-overflow code (`ENOBUFS` on this * runtime, or the `ERR_CHILD_PROCESS_STDIO_MAXBUFFER` code documented * for the async exec()/execFile() family, accepted defensively in case @@ -99,6 +112,11 @@ function toSeamResult(result) { let outcome; if (!error) { outcome = signal === null ? OUTCOME.EXITED : OUTCOME.KILLED; + } else if (status !== null) { + // Evidence over classification: `status` is only ever populated by a + // real exit, so it outranks an attached `error` — see the header + // comment's at-the-timeout-boundary case. + outcome = OUTCOME.EXITED; } else if (BUFFER_OVERFLOW_CODES.has(errorCode)) { outcome = OUTCOME.BUFFER_OVERFLOW; } else if (errorCode === 'ETIMEDOUT' || signal !== null) { @@ -223,4 +241,4 @@ function runHook(target, args = [], options = {}) { return spawnSeam(interpreter, [target, ...args], spawnOptions); } -module.exports = { runNode, runGit, runHook, OUTCOME }; +module.exports = { runNode, runGit, runHook, OUTCOME, toSeamResult }; diff --git a/tests/host-integration.test.cjs b/tests/host-integration.test.cjs index 1e5aca7e3..d3ff09599 100644 --- a/tests/host-integration.test.cjs +++ b/tests/host-integration.test.cjs @@ -1860,15 +1860,18 @@ describe('#2584 orchestratorExec — validator', () => { // consumer's only entry point — execute-phase branches on exactly this output. // --------------------------------------------------------------------------- describe('#2627 dispatch-isolation CLI route', () => { - const { execFileSync } = require('node:child_process'); + const { runNode } = require('./helpers/process-seam.cjs'); + const { throwIfFailed } = require('./helpers/git-fixture.cjs'); + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const GSD_TOOLS = path.join(REPO_ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs'); function query(runtimeId, extraArgs = []) { - return execFileSync( - process.execPath, + const r = runNode( [GSD_TOOLS, 'query', 'dispatch-isolation', ...extraArgs], - { cwd: REPO_ROOT, encoding: 'utf8', env: { ...process.env, GSD_RUNTIME: runtimeId } }, + { cwd: REPO_ROOT, env: { ...process.env, GSD_RUNTIME: runtimeId }, timeoutMs: PROBE_TIMEOUT_MS }, ); + throwIfFailed(r, `gsd-tools query dispatch-isolation ${extraArgs.join(' ')}`); + return r.stdout; } const queryJson = (runtimeId, extraArgs = []) => JSON.parse(query(runtimeId, ['--json', ...extraArgs])); diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 7236a2ad3..5eb3b2c38 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -2921,9 +2921,8 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { execSync } = require('node:child_process'); - const { runGsdTools, cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const WORKFLOW_PATH = path.join( __dirname, @@ -2946,13 +2945,13 @@ function createOuterRepoWithSubdir(prefix = 'bug-3491-') { // short-name (RUNNER~1) and the runtime resolves to the long form. // realpathSync.native handles both; then normalize separators for compare. const outerReal = fs.realpathSync.native(outer); - execSync('git init', { cwd: outerReal, stdio: 'pipe' }); - execSync('git config user.email "test@test.com"', { cwd: outerReal, stdio: 'pipe' }); - execSync('git config user.name "Test"', { cwd: outerReal, stdio: 'pipe' }); - execSync('git config commit.gpgsign false', { cwd: outerReal, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: outerReal }); + gitOrThrow(['config', 'user.email', 'test@test.com'], { cwd: outerReal }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: outerReal }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd: outerReal }); fs.writeFileSync(path.join(outerReal, 'README.md'), '# outer\n'); - execSync('git add -A', { cwd: outerReal, stdio: 'pipe' }); - execSync('git commit -m "initial"', { cwd: outerReal, stdio: 'pipe' }); + gitOrThrow(['add', '-A'], { cwd: outerReal }); + gitOrThrow(['commit', '-m', 'initial'], { cwd: outerReal }); const subdir = path.join(outerReal, 'workstreams', 'my-project'); fs.mkdirSync(subdir, { recursive: true }); diff --git a/tests/io.test.cjs b/tests/io.test.cjs index 6735184ea..fc1be4989 100644 --- a/tests/io.test.cjs +++ b/tests/io.test.cjs @@ -13,12 +13,18 @@ const { test, describe, afterEach } = require('node:test'); const assert = require('node:assert/strict'); -const { spawnSync } = require('node:child_process'); const path = require('node:path'); const os = require('node:os'); const fs = require('node:fs'); const io = require('../gsd-core/bin/lib/io.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + +function runScript(script) { + return toLegacyResult(runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS })); +} // ─── ERROR_REASON constants ─────────────────────────────────────────────────── @@ -98,7 +104,7 @@ describe('output()', () => { const io = require(${JSON.stringify(ioPath)}); io.output({ ok: true, value: 42 }, false); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); const parsed = JSON.parse(result.stdout); assert.deepStrictEqual(parsed, { ok: true, value: 42 }); @@ -109,7 +115,7 @@ describe('output()', () => { const io = require(${JSON.stringify(ioPath)}); io.output({ ignored: true }, true, 'raw-text-output'); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); assert.strictEqual(result.stdout, 'raw-text-output'); }); @@ -119,7 +125,7 @@ describe('output()', () => { const io = require(${JSON.stringify(ioPath)}); io.output({ fallback: true }, true); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); const parsed = JSON.parse(result.stdout); assert.deepStrictEqual(parsed, { fallback: true }); @@ -130,7 +136,7 @@ describe('output()', () => { const io = require(${JSON.stringify(ioPath)}); io.output(null, false); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); assert.strictEqual(result.stdout, 'null'); }); @@ -156,7 +162,7 @@ describe('output()', () => { const largeString = 'x'.repeat(60000); io.output({ large: largeString }, false); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); const stdout = result.stdout.trim(); @@ -187,7 +193,7 @@ describe('error()', () => { io.setJsonErrorMode(false); io.error('something went wrong'); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 1); assert.ok(result.stderr.includes('Error: something went wrong'), `stderr was: ${result.stderr}`); assert.strictEqual(result.stdout, ''); @@ -199,7 +205,7 @@ describe('error()', () => { io.setJsonErrorMode(false); io.error('no reason code expected'); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 1); // plain mode does NOT include the reason field assert.ok(!result.stderr.includes('"reason"'), `stderr unexpectedly contained reason: ${result.stderr}`); @@ -211,7 +217,7 @@ describe('error()', () => { io.setJsonErrorMode(true); io.error('structured error', io.ERROR_REASON.SDK_FAIL_FAST); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 1); assert.strictEqual(result.stdout, ''); const payload = JSON.parse(result.stderr.trim()); @@ -226,7 +232,7 @@ describe('error()', () => { io.setJsonErrorMode(true); io.error('no reason given'); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 1); const payload = JSON.parse(result.stderr.trim()); assert.strictEqual(payload.reason, 'unknown'); @@ -246,7 +252,7 @@ describe('error()', () => { io.setJsonErrorMode(true); io.error('test', io.ERROR_REASON.${key}); `; - const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + const result = runScript(script); assert.strictEqual(result.status, 1, `key=${key}`); const payload = JSON.parse(result.stderr.trim()); assert.strictEqual(payload.reason, expected, `key=${key}`); diff --git a/tests/issue-2765-brace-expansion-lockfile.test.cjs b/tests/issue-2765-brace-expansion-lockfile.test.cjs index 8f04a9584..061e0f6b4 100644 --- a/tests/issue-2765-brace-expansion-lockfile.test.cjs +++ b/tests/issue-2765-brace-expansion-lockfile.test.cjs @@ -15,11 +15,20 @@ const path = require('node:path'); const ROOT = path.join(__dirname, '..'); +// `npm` is not process.execPath, git, or a bash script/hook, so this does not +// route through tests/helpers/process-seam.cjs (whose runNode/runGit/runHook +// primitives cover exactly those three shapes and forward no `shell` option) +// — `npm` needs `shell: true` on Windows (npm.cmd), which the seam has no +// surface for. Bounding this directly with an explicit `timeout` is the +// documented alternative in eslint-rules/no-unbounded-spawn.cjs. +const NPM_LS_TIMEOUT_MS = 30000; + function npmLs(pkg) { // `npm ls --json --all` lists every installed copy with its version. Collect // the version of every node whose key is `pkg` (not the parent packages). const out = execFileSync('npm', ['ls', pkg, '--json', '--all'], { cwd: ROOT, encoding: 'utf8', shell: true, stdio: ['ignore', 'pipe', 'ignore'], + timeout: NPM_LS_TIMEOUT_MS, }); const versions = []; const walk = (node) => { diff --git a/tests/issue-498-update-context.test.cjs b/tests/issue-498-update-context.test.cjs index 1ff5f5c37..33d47b75e 100644 --- a/tests/issue-498-update-context.test.cjs +++ b/tests/issue-498-update-context.test.cjs @@ -12,7 +12,9 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const nodeFs = require('node:fs'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.join(__dirname, '..'); const GSD_TOOLS = path.join(ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -127,12 +129,12 @@ describe('gsd-tools update-context (CLI): emits the JSON contract', () => { nodeFs.mkdirSync(path.join(tmp, 'gsd-core', 'workflows'), { recursive: true }); nodeFs.writeFileSync(path.join(tmp, 'gsd-core', 'VERSION'), '1.42.0\n'); nodeFs.writeFileSync(path.join(tmp, 'gsd-core', 'workflows', 'update.md'), 'x'); - const out = execFileSync( - process.execPath, + const r = runNode( [GSD_TOOLS, 'update-context', '--config-dir', tmp, '--runtime', 'kilo', '--json'], - { encoding: 'utf8', env: { ...process.env, GSD_TEST_MODE: '1' } }, + { env: { ...process.env, GSD_TEST_MODE: '1' }, timeoutMs: PROBE_TIMEOUT_MS }, ); - const ctx = JSON.parse(out); + throwIfFailed(r, 'gsd-tools update-context --json'); + const ctx = JSON.parse(r.stdout); assert.deepEqual(Object.keys(ctx).sort(), ['gsdDir', 'installedVersion', 'runtime', 'scope']); assert.equal(ctx.installedVersion, '1.42.0'); assert.equal(ctx.scope, 'GLOBAL'); diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index f32838734..fcc3bcf06 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -10,9 +10,10 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); -const { execSync, execFileSync } = require('child_process'); const fs = require('fs'); const path = require('path'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); const { runGsdTools, createTempProject, createTempGitProject, cleanup, readFileNormalized } = require('./helpers.cjs'); const { writeState } = require('./fixtures/index.cjs'); @@ -176,7 +177,7 @@ describe('phases clear: uncommitted-changes guard (#1447)', () => { fs.mkdirSync(phase1, { recursive: true }); fs.writeFileSync(path.join(phase1, 'PLAN.md'), '# Plan (staged)'); // Stage the file but do not commit - execSync('git add .planning/phases/', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['add', '.planning/phases/'], { cwd: tmpDir }); const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(!result.success, 'phases clear should fail when staged-but-uncommitted changes exist'); @@ -207,8 +208,8 @@ describe('phases clear: uncommitted-changes guard (#1447)', () => { fs.mkdirSync(phase1, { recursive: true }); fs.writeFileSync(path.join(phase1, 'PLAN.md'), '# Plan (committed)'); // Commit the phase files - execSync('git add .planning/phases/', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "add phase"', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['add', '.planning/phases/'], { cwd: tmpDir }); + gitOrThrow(['commit', '-m', 'add phase'], { cwd: tmpDir }); const result = runGsdTools('phases clear --confirm', tmpDir); assert.ok(result.success, `should succeed when phase files are committed: ${result.error}`); @@ -645,7 +646,9 @@ describe('new-milestone.md: workstream-aware PROJECT.md guard (#2308)', () => { function runStep1(argumentsValue) { const script = `ARGUMENTS=${JSON.stringify(argumentsValue)}\n${step1Fence}\n` + 'printf \'GSD_WS=[%s]\\nMILESTONE_ARG=[%s]\\n\' "$GSD_WS" "$MILESTONE_ARG"'; - const out = execFileSync('bash', ['-c', script], { encoding: 'utf8' }); + const r = runHookSeam('-c', [script], { interpreter: 'bash' }); + throwIfFailed(r, 'bash '); + const out = r.stdout; return { gsdWs: /GSD_WS=\[(.*)\]/.exec(out)[1], milestoneArg: /MILESTONE_ARG=\[(.*)\]/.exec(out)[1], @@ -676,7 +679,9 @@ describe('new-milestone.md: workstream-aware PROJECT.md guard (#2308)', () => { function runStep6Commit(argumentsValue) { const gsdRunStub = 'gsd_run() { printf "%s\\n" "gsd_run_call:$*"; }\n'; const script = `ARGUMENTS=${JSON.stringify(argumentsValue)}\n${gsdRunStub}${step6CommitFence}`; - return execFileSync('bash', ['-c', script], { encoding: 'utf8' }); + const r = runHookSeam('-c', [script], { interpreter: 'bash' }); + throwIfFailed(r, 'bash '); + return r.stdout; } // Step 4 Part A's guard — not this commit — is what protects the shared diff --git a/tests/no-unbounded-spawn-allowlist.test.cjs b/tests/no-unbounded-spawn-allowlist.test.cjs index a35941d87..78f14aae1 100644 --- a/tests/no-unbounded-spawn-allowlist.test.cjs +++ b/tests/no-unbounded-spawn-allowlist.test.cjs @@ -2,9 +2,18 @@ /** * no-unbounded-spawn-allowlist.test.cjs * - * Structural guards on `eslint-rules/no-unbounded-spawn.allowlist.json` — - * matrix D4-D8 of - * .gsd/phase/chore-3143-no-unbounded-spawn-guard/50-test-matrix.md. + * `eslint-rules/no-unbounded-spawn.allowlist.json` was deleted by #3148 (the + * terminal wave of epic #3064): the migration reached zero remaining + * violations, so the allowlist option was dropped from the rule's wiring in + * `eslint.config.mjs` and `local/no-unbounded-spawn` now runs with no + * exemption surface at all under `tests/**`. The former D4/D5/D6/D8 guards + * here (dead entries, baseline ratchet, separator normalization, canonical + * sort/dedupe) all referenced that now-deleted file and are gone with it. + * + * What remains — D7, the inline-disable ban — matters MORE now, not less: + * with no allowlist to grandfather a file, an inline `eslint-disable` + * naming this rule is the ONLY remaining way to silence it. This guard is + * the sole remaining defense against that, so it stays. * * D7 needs to inspect test-file *contents* for an inline directive that * disables this rule by name — the absence of that pattern is the contract @@ -14,6 +23,16 @@ * exists to catch — hence the * `// allow-test-rule: structural-regression-guard [#3143]` annotation on * its own line above, per CONTRIBUTING.md's documented exemption. + * + * The scan covers ALL of `tests/`, RECURSIVELY — not just the top-level + * directory. `listTestFiles()` walks every subdirectory (`tests/helpers/`, + * `tests/qa/`, `tests/observability/`, `tests/fixtures/`, `tests/dispatch/`, + * etc.) via `fs.readdirSync(..., { recursive: true })`. A non-recursive scan + * left ~37 nested `.cjs` files completely unchecked: a file in a subdirectory + * could carry both an unbounded spawn AND an inline eslint-disable directive + * naming this rule (see `GUARDED_RULE` below), and this guard would never see + * it. With the allowlist gone, this is the SOLE remaining defense against + * silencing the rule — recursion is not optional. */ 'use strict'; @@ -23,61 +42,32 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const ALLOWLIST_PATH = path.join(__dirname, '..', 'eslint-rules', 'no-unbounded-spawn.allowlist.json'); const TESTS_DIR = path.join(__dirname); const REPO_ROOT = path.join(__dirname, '..'); -// The allowlist only ratchets DOWN. This baseline is the length observed at -// the time this guard was written (139 entries) — each future migration -// wave lowers it as files are moved off the allowlist by adding real -// timeouts; it must never grow back up. Lowered to 120 by the #3144 Wave-1 -// process-seam migration (19 files' unbounded spawns bounded), then to 73 -// by the #3145 Wave-2 migration (47 files' unbounded spawns bounded), then -// to 49 by the #3147 Wave-3 migration (24 files' unbounded spawns bounded). -const BASELINE = 49; - -function readAllowlist() { - const raw = fs.readFileSync(ALLOWLIST_PATH, 'utf8'); - return JSON.parse(raw); -} - +/** + * Recursively list every `.cjs` file under `tests/`, including subdirectories + * (`tests/helpers/`, `tests/qa/`, `tests/observability/`, `tests/fixtures/`, + * `tests/dispatch/`, etc.). `fs.readdirSync(dir, { recursive: true, + * withFileTypes: true })` is available on the repo's Node floor (>=22.0.0 per + * package.json `engines`; the option landed in Node 20.1). Each returned + * `Dirent` carries `parentPath` — its containing directory, which for a + * nested entry is the subdirectory, not `TESTS_DIR` — so the joined path is + * correct at any depth. `node_modules` is skipped defensively in case one is + * ever vendored under `tests/`. + */ function listTestFiles() { const out = []; - for (const entry of fs.readdirSync(TESTS_DIR, { withFileTypes: true })) { - if (entry.isFile() && entry.name.endsWith('.cjs')) { - out.push(path.join(TESTS_DIR, entry.name)); - } + const entries = fs.readdirSync(TESTS_DIR, { withFileTypes: true, recursive: true }); + for (const entry of entries) { + if (!entry.isFile() || !entry.name.endsWith('.cjs')) continue; + const dir = entry.parentPath || entry.path || TESTS_DIR; + if (dir.split(path.sep).includes('node_modules')) continue; + out.push(path.join(dir, entry.name)); } return out; } -// Pure detection helpers, extracted so each is unit-testable against a -// synthetic fixture — proving the check itself can fail, not only that -// today's real data happens to pass it (a check that never runs against an -// injected violation is a vacuous-truth risk). - -function findDeadEntries(list, root) { - return list.filter((entry) => !fs.existsSync(path.join(root, entry))); -} - -function findBackslashEntries(list) { - return list.filter((entry) => entry.includes('\\')); -} - -function isSorted(list) { - return JSON.stringify(list) === JSON.stringify([...list].sort()); -} - -function findDuplicates(list) { - const seen = new Set(); - const dupes = new Set(); - for (const entry of list) { - if (seen.has(entry)) dupes.add(entry); - seen.add(entry); - } - return [...dupes]; -} - // Built via concatenation, not a string literal, so this file does not // itself contain the literal directive text (`local/no-unbounded-spawn`) // that D7 below scans every test file for — a literal here would make this @@ -88,45 +78,6 @@ function containsDisableDirective(contents, ruleName) { return new RegExp(`eslint-disable[^\\n]*${ruleName}`).test(contents); } -describe('no-unbounded-spawn allowlist: D4 — no dead entries', () => { - test('every allowlist entry resolves to a file that exists on disk', () => { - const list = readAllowlist(); - const dead = findDeadEntries(list, REPO_ROOT); - assert.deepEqual(dead, [], `dead allowlist entries (file does not exist): ${JSON.stringify(dead)}`); - }); - - test('detection logic actually flags a synthetic dead entry', () => { - const synthetic = ['tests/does-not-exist-xyz.test.cjs', 'tests/no-unbounded-spawn.test.cjs']; - const dead = findDeadEntries(synthetic, REPO_ROOT); - assert.deepEqual(dead, ['tests/does-not-exist-xyz.test.cjs']); - }); -}); - -describe('no-unbounded-spawn allowlist: D5 — never grows', () => { - test('allowlist length is at or under the recorded baseline', () => { - const list = readAllowlist(); - assert.ok( - list.length <= BASELINE, - `allowlist grew to ${list.length} entries, exceeding the BASELINE of ${BASELINE}. ` + - `The allowlist only ratchets down — if this is a legitimate new violation, ` + - `lower BASELINE only after confirming it, never raise it to paper over growth.` - ); - }); -}); - -describe('no-unbounded-spawn allowlist: D6 — separator normalization', () => { - test('every entry uses / separators, never \\', () => { - const list = readAllowlist(); - const withBackslash = findBackslashEntries(list); - assert.deepEqual(withBackslash, [], `entries with a backslash separator: ${JSON.stringify(withBackslash)}`); - }); - - test('detection logic actually flags a synthetic backslash entry', () => { - const synthetic = ['tests\\foo.test.cjs', 'tests/bar.test.cjs']; - assert.deepEqual(findBackslashEntries(synthetic), ['tests\\foo.test.cjs']); - }); -}); - describe('no-unbounded-spawn allowlist: D7 — no inline disable of this rule', () => { test('no test file inline-disables the unbounded-spawn guard', () => { const offenders = []; @@ -152,18 +103,74 @@ describe('no-unbounded-spawn allowlist: D7 — no inline disable of this rule', assert.equal(containsDisableDirective(syntheticContents, GUARDED_RULE), true); assert.equal(containsDisableDirective("'use strict';\nspawnSync('git', ['status'], {});", GUARDED_RULE), false); }); -}); -describe('no-unbounded-spawn allowlist: D8 — canonical form', () => { - test('allowlist is valid JSON, sorted, with no duplicates', () => { - const list = readAllowlist(); - assert.ok(Array.isArray(list), 'allowlist.json must parse to an array'); - assert.ok(isSorted(list), 'allowlist entries must be sorted'); - assert.deepEqual(findDuplicates(list), [], 'allowlist entries must be unique'); + test('listTestFiles() recurses into subdirectories, not just the top level', (t) => { + // Regression for the reviewer-proven hole: a non-recursive scan sees + // only TESTS_DIR itself and is blind to tests/helpers/, tests/qa/, + // tests/observability/, tests/fixtures/, tests/dispatch/, etc. + const probeDir = path.join(TESTS_DIR, 'helpers'); + const probePath = path.join(probeDir, '__probe_recursion_3148.cjs'); + fs.writeFileSync( + probePath, + [ + "'use strict';", + '// eslint-disable-next-line ' + GUARDED_RULE, + "spawnSync('git', ['status'], {});", + '', + ].join('\n'), + ); + t.after(() => { + // helpers.cleanup() refuses any path outside a recognized temp root; + // this probe deliberately lives under tests/helpers/ (the thing under + // test is recursion into a real subdirectory of tests/, not a temp + // dir), so a raw, force-flagged, single-file rmSync of a path this + // same test just created is the correct tool here. + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- probe file lives under tests/helpers/, not a temp root; helpers.cleanup() would refuse it + fs.rmSync(probePath, { force: true }); + }); + + const found = listTestFiles(); + assert.ok( + found.includes(probePath), + 'listTestFiles() must include .cjs files nested in subdirectories of tests/', + ); }); - test('detection logic actually flags a synthetic unsorted/duplicate list', () => { - assert.equal(isSorted(['b.test.cjs', 'a.test.cjs']), false); - assert.deepEqual(findDuplicates(['a.test.cjs', 'b.test.cjs', 'a.test.cjs']), ['a.test.cjs']); + test('a subdirectory file inline-disabling the rule is caught end-to-end', (t) => { + // Same probe, but exercised through the actual D7 detection path (the + // same offenders-collection loop the first test in this describe runs), + // proving the fix closes the hole rather than just listTestFiles(). + const probeDir = path.join(TESTS_DIR, 'helpers'); + const probePath = path.join(probeDir, '__probe_recursion_detect_3148.cjs'); + fs.writeFileSync( + probePath, + [ + "'use strict';", + '// eslint-disable-next-line ' + GUARDED_RULE, + "spawnSync('git', ['status'], {});", + '', + ].join('\n'), + ); + t.after(() => { + // helpers.cleanup() refuses any path outside a recognized temp root; + // this probe deliberately lives under tests/helpers/ (the thing under + // test is recursion into a real subdirectory of tests/, not a temp + // dir), so a raw, force-flagged, single-file rmSync of a path this + // same test just created is the correct tool here. + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- probe file lives under tests/helpers/, not a temp root; helpers.cleanup() would refuse it + fs.rmSync(probePath, { force: true }); + }); + + const offenders = []; + for (const filePath of listTestFiles()) { + const contents = fs.readFileSync(filePath, 'utf8'); + if (containsDisableDirective(contents, GUARDED_RULE)) { + offenders.push(path.relative(REPO_ROOT, filePath)); + } + } + assert.ok( + offenders.includes(path.relative(REPO_ROOT, probePath)), + `expected the nested inline-disable probe to be caught; offenders: ${JSON.stringify(offenders)}`, + ); }); }); diff --git a/tests/pause-work-improvements.test.cjs b/tests/pause-work-improvements.test.cjs index 40e8b8876..fad4ae9f3 100644 --- a/tests/pause-work-improvements.test.cjs +++ b/tests/pause-work-improvements.test.cjs @@ -89,7 +89,8 @@ const { test, describe, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); const { createTempDir, cleanup, readFileNormalized } = require('./helpers.cjs'); const WORKFLOW_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'resume-project.md'); @@ -120,11 +121,11 @@ function extractCheckBlock() { function runSnippet(cwd, snippet) { // has_interrupted_agent is a downstream-orchestrator variable; default it // to "false" so the embedded `if` branch is a no-op during this test. - return spawnSync('bash', ['-c', snippet], { + return toLegacyResult(runHookSeam('-c', [snippet], { + interpreter: 'bash', cwd, - encoding: 'utf8', env: { ...process.env, has_interrupted_agent: 'false', interrupted_agent_id: '' }, - }); + })); } describe('bug #3446: resume-project detects non-phase and legacy continue-here handoffs', () => { @@ -242,7 +243,8 @@ const { describe, test, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); @@ -256,7 +258,7 @@ const FIND_SNIPPET = [ ].join('\n'); function hasShell(name) { - const result = spawnSync('which', [name], { encoding: 'utf8' }); + const result = toLegacyResult(runHookSeam(name, [], { interpreter: 'which' })); return result.status === 0 && result.stdout.trim().length > 0; } @@ -281,10 +283,10 @@ describe('bug #3689 — resume-project.md continue-here scan under zsh NOMATCH', }); test('zsh -o nomatch lists the .planning/.continue-here-* checkpoint', { skip: !hasShell('zsh') }, () => { - const result = spawnSync('zsh', ['-o', 'nomatch', '-c', FIND_SNIPPET], { + const result = toLegacyResult(runHookSeam('-o', ['nomatch', '-c', FIND_SNIPPET], { + interpreter: 'zsh', cwd: tmpDir, - encoding: 'utf8', - }); + })); assert.equal(result.status, 0, `zsh exited ${result.status}; stderr=${result.stderr}`); assert.match( result.stdout, @@ -294,10 +296,10 @@ describe('bug #3689 — resume-project.md continue-here scan under zsh NOMATCH', }); test('bash default lists the .planning/.continue-here-* checkpoint', { skip: !hasShell('bash') }, () => { - const result = spawnSync('bash', ['-c', FIND_SNIPPET], { + const result = toLegacyResult(runHookSeam('-c', [FIND_SNIPPET], { + interpreter: 'bash', cwd: tmpDir, - encoding: 'utf8', - }); + })); assert.equal(result.status, 0, `bash exited ${result.status}; stderr=${result.stderr}`); assert.match( result.stdout, @@ -320,10 +322,10 @@ describe('bug #3689 — empty workspace exits cleanly', () => { }); test('zsh -o nomatch with no checkpoints exits 0, empty output', { skip: !hasShell('zsh') }, () => { - const result = spawnSync('zsh', ['-o', 'nomatch', '-c', FIND_SNIPPET], { + const result = toLegacyResult(runHookSeam('-o', ['nomatch', '-c', FIND_SNIPPET], { + interpreter: 'zsh', cwd: tmpDir, - encoding: 'utf8', - }); + })); assert.equal(result.status, 0, `zsh exited ${result.status}; stderr=${result.stderr}`); assert.equal(result.stdout.trim(), '', `expected no stdout, got: ${JSON.stringify(result.stdout)}`); }); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index cfadc479a..a0b69f482 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -15,7 +15,14 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('node:os'); -const { execFileSync, spawnSync } = require('node:child_process'); +const { execFileSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); + +// `phase complete` against a real STATE.md rewrite; matches the 60000ms bound +// already used for the same CLI call elsewhere in this file (runPhaseComplete +// above, and `run()` below at 15000ms for a lighter query-only call). +const PHASE_COMPLETE_TIMEOUT_MS = 60000; const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); const GSD_TOOLS_BIN = path.resolve(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -6138,11 +6145,11 @@ describe('bug-3287 — init plan-phase exposes expected_phase_dir with project_c const { planningDir } = setupPhase1316Project(tmpDir); writePassedVerificationForPhase(tmpDir, '32'); - const result = spawnSync(process.execPath, [GSD_TOOLS_BIN, 'phase', 'complete', '32'], { + const result = toLegacyResult(runNode([GSD_TOOLS_BIN, 'phase', 'complete', '32'], { cwd: tmpDir, - encoding: 'utf8', env: process.env, - }); + timeoutMs: PHASE_COMPLETE_TIMEOUT_MS, + })); assert.strictEqual(result.status, 0, `phase complete failed: ${result.stderr || result.stdout}`); assert.ok( diff --git a/tests/process-seam.test.cjs b/tests/process-seam.test.cjs index 6fce82cae..25da755ca 100644 --- a/tests/process-seam.test.cjs +++ b/tests/process-seam.test.cjs @@ -20,11 +20,10 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); const { createTempDir, cleanup, runGsdTools, TOOLS_PATH } = require('./helpers.cjs'); const processSeam = require('./helpers/process-seam.cjs'); -const { runNode, runGit, runHook, OUTCOME } = processSeam; +const { runNode, runGit, runHook, OUTCOME, toSeamResult } = processSeam; // ---- fixture sources ------------------------------------------------- @@ -265,6 +264,24 @@ describe('process-seam', () => { } }); + test('toSeamResult classifies a raced status+ETIMEDOUT as EXITED, not TIMED_OUT', () => { + // Synthetic reproduction of the exact-bound race: spawnSync's timer + // fired (error.code === 'ETIMEDOUT') just as the child finished on its + // own (status: 0). Evidence (a real status) must outrank the attached + // error — the old discrimination order checked error.code first and + // reported TIMED_OUT with exitCode: 0, an incoherent shape. + const result = toSeamResult({ + status: 0, + error: { code: 'ETIMEDOUT' }, + signal: null, + stdout: '', + stderr: '', + }); + assert.equal(result.outcome, OUTCOME.EXITED); + assert.equal(result.timedOut, false); + assert.equal(result.exitCode, 0); + }); + test('child overrunning the bound is TIMED_OUT', () => { const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER); const result = runNode([fixture, '5000'], { timeoutMs: 200 }); @@ -443,8 +460,8 @@ describe('runHook interpreter option', () => { // node-only container may not have bash on PATH. function isBashAvailable() { if (process.platform === 'win32') return false; - const probeResult = spawnSync('bash', ['-c', 'exit 0']); - return !probeResult.error; + const probeResult = runHook('-c', ['exit 0'], { interpreter: 'bash' }); + return probeResult.outcome !== OUTCOME.SPAWN_FAILED; } const bashAvailable = isBashAvailable(); diff --git a/tests/prohibition-enforcement.test.cjs b/tests/prohibition-enforcement.test.cjs index 440351159..0aa485627 100644 --- a/tests/prohibition-enforcement.test.cjs +++ b/tests/prohibition-enforcement.test.cjs @@ -332,7 +332,9 @@ describe('prohibition-enforcement: deterministic test-tier producer (#1259 / ADR test('routeProhibitionEnforcement parses a JSON request file and emits a structured result', (t) => { const fs = require('node:fs'); - const { execFileSync } = require('node:child_process'); + const { runNode } = require('./helpers/process-seam.cjs'); + const { throwIfFailed } = require('./helpers/git-fixture.cjs'); + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // Write a request file; the route reads it and runs the node-test descriptor's default runner // (its target does not exist, so it fail-closes deterministically — we assert the JSON SHAPE, // not a green verdict). We invoke the built CLI surface in a child process so output() @@ -351,8 +353,9 @@ describe('prohibition-enforcement: deterministic test-tier producer (#1259 / ADR ".routeProhibitionEnforcement(['check','prohibition-enforcement'," + JSON.stringify(reqPath) + "], false);\n"); t.after(() => cleanup(dir)); - const captured = execFileSync('node', [runnerPath], { encoding: 'utf-8' }); - const parsed = JSON.parse(captured); + const r = runNode([runnerPath], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(r, `node ${runnerPath}`); + const parsed = JSON.parse(r.stdout); assert.equal(typeof parsed, 'object', 'route emits a JSON object'); assert.equal(parsed.tier, 'test', 'tier is preserved through the CLI surface'); assert.equal(parsed.located, true, 'the check descriptor was located'); diff --git a/tests/project-instruction-file-parity.test.cjs b/tests/project-instruction-file-parity.test.cjs index bae91ae4e..907ad2652 100644 --- a/tests/project-instruction-file-parity.test.cjs +++ b/tests/project-instruction-file-parity.test.cjs @@ -25,7 +25,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.join(__dirname, '..'); const RUNTIME_NAME_POLICY_PATH = path.join( @@ -66,11 +68,13 @@ function queryInstructionFile(runtime) { '--runtime', runtime, ]; - return execFileSync('node', args, { + const r = runNode(args, { cwd: ROOT, - encoding: 'utf8', env: { ...process.env, GSD_RUNTIME: '' }, - }).trim(); + timeoutMs: PROBE_TIMEOUT_MS, + }); + throwIfFailed(r, `node ${args.join(' ')}`); + return r.stdout.trim(); } describe('bug #1529: getProjectInstructionFile ↔ gsd-tools query parity', () => { diff --git a/tests/release-tarball-smoke.install.test.cjs b/tests/release-tarball-smoke.install.test.cjs index 5fb5c8bb1..559478ef4 100644 --- a/tests/release-tarball-smoke.install.test.cjs +++ b/tests/release-tarball-smoke.install.test.cjs @@ -459,6 +459,69 @@ describe('bug-131: runNpm isolates HOME from the caller environment', () => { 'isolatedNpmEnv() should disable npm update-notifier notices for npm versions that honor NO_UPDATE_NOTIFIER', ); }); + + // ── Test 4 — runNpm's 180000ms bound survives an explicit `timeout: undefined` ── + // (#3148 wave 4) runNpm() used to spread `...defaults` (which carries + // `timeout: 180000`) and then `...otherOptions` AFTER it, so a caller + // passing an own `timeout: undefined` key (not an omission) silently won + // the spread and erased the bound, leaving the underlying execFileSync call + // unbounded. The fix destructures `timeout` off `options` with a default + // and passes it explicitly after both spreads, so an own `undefined` key + // resolves to the default instead of erasing it. + // + // Proof is behavioral, not textual: a fresh child process monkeypatches + // `child_process.execFileSync` to capture the options object it actually + // receives — before `helpers.cjs` is required in that child, so its + // top-level `const { execFileSync } = require('child_process')` picks up + // the patched function — then calls `runNpm(['--version'], { timeout: + // undefined })` and reports back the captured `timeout` value. Reverting + // the destructure-with-default fix (restoring the plain `{ ...defaults, + // ...otherOptions, env: mergedEnv }` spread order) makes this test fail: + // the captured `timeout` becomes `undefined` instead of `180000`. + test('runNpm resolves an explicit `timeout: undefined` to the 180000ms bound, not unbounded', () => { + const script = ` + const cp = require('node:child_process'); + const seen = []; + cp.execFileSync = (cmd, args, options) => { + seen.push(options); + return '9.9.9'; + }; + const { runNpm } = require(${JSON.stringify(path.join(__dirname, 'helpers.cjs'))}); + // An own \`timeout: undefined\` key — not an omitted key — is the exact + // hazard: a caller-controlled property that must not erase the bound. + runNpm(['--version'], { timeout: undefined }); + process.stdout.write(JSON.stringify({ timeout: seen[0] && seen[0].timeout })); + `; + + let stdout = ''; + let stderr = ''; + let exitCode = 0; + try { + stdout = execFileSync(process.execPath, ['-e', script], { + encoding: 'utf-8', + timeout: 30_000, + }); + } catch (err) { + stdout = err.stdout || ''; + stderr = err.stderr || ''; + exitCode = err.status ?? 1; + } + + assert.equal( + exitCode, + 0, + `runNpm timeout-capture probe failed with exit ${exitCode}. stderr: ${stderr}`, + ); + + const captured = JSON.parse(stdout.trim()); + assert.equal( + captured.timeout, + 180000, + `runNpm must resolve an explicit timeout: undefined to the 180000ms bound; ` + + `captured options.timeout was ${JSON.stringify(captured.timeout)} — an unset bound ` + + `is not a bound (DEFECT.UNBOUNDED-SUBPROCESS)`, + ); + }); }); }); } diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index d0970a73b..07a822f64 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -17,14 +17,23 @@ const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); -const { spawnSync } = require('child_process'); const fs = require('fs'); const path = require('path'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); const HARNESS = path.join(__dirname, '..', 'scripts', 'run-tests.cjs'); +// The harness under test enforces its OWN per-chunk timeout internally +// (RUN_TESTS_CHUNK_TIMEOUT_MS, default 600000ms; this file's slowest explicit +// override below is 30000ms). This outer bound must stay comfortably above +// whatever the harness itself is configured to wait for a hung chunk, plus +// `node --test` child-process startup overhead — otherwise this seam would +// kill the harness before its own timeout diagnostic fires. +const HARNESS_TIMEOUT_MS = 120000; + // Minimal valid node:test file. Each fixture file passes when executed. const PASS_BODY = `'use strict'; const { test } = require('node:test'); @@ -44,11 +53,15 @@ function runHarness(testDir, args = [], extraEnv = {}) { // doesn't refuse to run with "recursive run() skipping running files". const env = { ...process.env, GSD_TEST_DIR: testDir, ...extraEnv }; delete env.NODE_TEST_CONTEXT; - return spawnSync(process.execPath, [HARNESS, ...args], { + const r = runNode([HARNESS, ...args], { cwd: path.join(__dirname, '..'), env, - encoding: 'utf8', + timeoutMs: HARNESS_TIMEOUT_MS, }); + // toLegacyResult() alone drops `signal` (several assertions below embed it + // in their failure message) — compose it back on top, per git-fixture.cjs's + // documented "extra field" composition pattern. + return { ...toLegacyResult(r), signal: r.signal }; } describe('run-tests.cjs harness (issue #3597)', () => { diff --git a/tests/runtime-launcher-parity.test.cjs b/tests/runtime-launcher-parity.test.cjs index cc54cdb60..45ff4bc89 100644 --- a/tests/runtime-launcher-parity.test.cjs +++ b/tests/runtime-launcher-parity.test.cjs @@ -29,13 +29,25 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); +/** + * Run a bash script FILE via the process seam, preserving the throw-on- + * nonzero-exit semantics of the execFileSync('bash', [path], ...) idiom + * this replaces. + */ +function runBashFile(scriptPath, options = {}) { + const r = runHookSeam(scriptPath, [], { interpreter: 'bash', ...options }); + throwIfFailed(r, `bash ${scriptPath}`); + return r.stdout; +} + /** * Read the canonical preamble from the snippet file (all lines, no trailing newline). */ @@ -293,7 +305,7 @@ describe('runtime-launcher-parity (#373)', () => { const scriptPath = path.join(base, 'test-space.sh'); fs.writeFileSync(scriptPath, scriptContent); - const stdout = execFileSync('bash', [scriptPath], { encoding: 'utf8' }); + const stdout = runBashFile(scriptPath); assert.ok( stdout.includes('STUB:query,state.json'), `Expected stdout to contain "STUB:query,state.json" but got: ${stdout.trim()}`, @@ -334,18 +346,12 @@ describe('runtime-launcher-parity (#373)', () => { }); const isolatedPath = [noToolsBin, ...systemPaths].join(path.delimiter); - let threw = false; - let stderrOutput = ''; - try { - execFileSync('bash', [scriptPath], { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - env: { ...process.env, PATH: isolatedPath, HOME: base }, - }); - } catch (err) { - threw = true; - stderrOutput = err.stderr || ''; - } + const r = runHookSeam(scriptPath, [], { + interpreter: 'bash', + env: { ...process.env, PATH: isolatedPath, HOME: base }, + }); + const threw = r.exitCode !== 0; + const stderrOutput = r.stderr || ''; assert.ok(threw, 'Expected the script to exit non-zero when gsd-tools.cjs is missing and gsd-tools is not on PATH'); assert.ok( @@ -383,8 +389,7 @@ describe('runtime-launcher-parity (#373)', () => { const scriptPath = path.join(base, 'test-pathfb.sh'); fs.writeFileSync(scriptPath, scriptContent); - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: `${pathBinDir}${path.delimiter}${process.env.PATH || ''}` }, }); @@ -535,8 +540,7 @@ describe('runtime-launcher-parity (#373)', () => { systemPaths.unshift(nodeShimDir); } - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: systemPaths.join(path.delimiter), HOME: fakeHome }, }); @@ -614,9 +618,10 @@ describe('runtime-launcher-parity — standalone executable (#381)', () => { `console.log('GSD_TOOLS_STUB:' + process.argv.slice(2).join(' '))`, ); - const stdout = execFileSync('sh', [path.join(binDir, 'gsd_run'), 'query', 'x'], { - encoding: 'utf8', - }); + const gsdRunPath = path.join(binDir, 'gsd_run'); + const r = runHookSeam(gsdRunPath, ['query', 'x'], { interpreter: 'sh' }); + throwIfFailed(r, `sh ${gsdRunPath} query x`); + const stdout = r.stdout; assert.ok( stdout.includes('GSD_TOOLS_STUB:query x'), `Expected stdout to contain "GSD_TOOLS_STUB:query x", got: ${stdout.trim()}`, @@ -650,8 +655,7 @@ describe('runtime-launcher-parity — standalone executable (#381)', () => { const preambleScript = path.join(baseParent, 'run-preamble.sh'); fs.writeFileSync(preambleScript, snippet); - execFileSync('bash', [preambleScript], { - encoding: 'utf8', + runBashFile(preambleScript, { env: { RUNTIME_DIR: base, CLAUDE_ENV_FILE: envFile, @@ -685,13 +689,15 @@ describe('runtime-launcher-parity — standalone executable (#381)', () => { // Simulate a LATER fresh block that SOURCES the env file to get gsd_run on PATH. // The later shell does NOT have the bin dir on PATH beforehand — it only gets it // by sourcing the env file. We use a minimal PATH (no temp bin dir pre-injected). - const stdout = execFileSync('bash', ['-c', '. "$CLAUDE_ENV_FILE"; gsd_run hello'], { - encoding: 'utf8', + const inlineResult = runHookSeam('-c', ['. "$CLAUDE_ENV_FILE"; gsd_run hello'], { + interpreter: 'bash', env: { CLAUDE_ENV_FILE: envFile, PATH: process.env.PATH, }, }); + throwIfFailed(inlineResult, 'bash -c '); + const stdout = inlineResult.stdout; assert.ok( stdout.includes('GSD_RUN_STUB:hello'), `Expected stdout to contain "GSD_RUN_STUB:hello" after sourcing env file, got: ${stdout.trim()}`, @@ -801,7 +807,8 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); @@ -809,6 +816,17 @@ const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); // Representative propagated workflow file (has a gsd_run call): const REPRESENTATIVE_FILE = path.join(WORKFLOWS_DIR, 'add-backlog.md'); +/** + * Run a bash script FILE via the process seam, preserving the throw-on- + * nonzero-exit semantics of the execFileSync('bash', [path], ...) idiom + * this replaces. + */ +function runBashFile(scriptPath, options = {}) { + const r = runHookSeam(scriptPath, [], { interpreter: 'bash', ...options }); + throwIfFailed(r, `bash ${scriptPath}`); + return r.stdout; +} + const CLAUDE_HOME_PROBE = '.claude/gsd-core/bin/'; describe('bug-211: launcher ~/.claude home fallback', () => { @@ -866,7 +884,9 @@ describe('bug-211: launcher ~/.claude home fallback', () => { // Filter out directories that contain a gsd-tools executable. If node lives // in the same directory as gsd-tools, create a dedicated shim dir with a // symlink to node only (no gsd-tools there). - const nodeBin = execFileSync('which', ['node'], { encoding: 'utf8' }).trim(); + const nodeBinResult = runHookSeam('node', [], { interpreter: 'which' }); + throwIfFailed(nodeBinResult, 'which node'); + const nodeBin = nodeBinResult.stdout.trim(); const systemPaths = (process.env.PATH || '/usr/bin:/bin') .split(path.delimiter) .filter((p) => { @@ -889,8 +909,7 @@ describe('bug-211: launcher ~/.claude home fallback', () => { systemPaths.unshift(nodeShimDir); } - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: systemPaths.join(path.delimiter), HOME: fakeHome }, }); @@ -943,18 +962,12 @@ describe('bug-211: launcher ~/.claude home fallback', () => { }); const isolatedPath = [noToolsBin, ...systemPaths].join(path.delimiter); - let threw = false; - let stderrOutput = ''; - try { - execFileSync('bash', [scriptPath], { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - env: { ...process.env, PATH: isolatedPath, HOME: fakeHome }, - }); - } catch (err) { - threw = true; - stderrOutput = err.stderr || ''; - } + const r = runHookSeam(scriptPath, [], { + interpreter: 'bash', + env: { ...process.env, PATH: isolatedPath, HOME: fakeHome }, + }); + const threw = r.exitCode !== 0; + const stderrOutput = r.stderr || ''; assert.ok(threw, 'Expected non-zero exit when all three resolution arms miss'); assert.ok( @@ -1011,12 +1024,24 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); +/** + * Run a bash script FILE via the process seam, preserving the throw-on- + * nonzero-exit semantics of the execFileSync('bash', [path], ...) idiom + * this replaces. + */ +function runBashFile(scriptPath, options = {}) { + const r = runHookSeam(scriptPath, [], { interpreter: 'bash', ...options }); + throwIfFailed(r, `bash ${scriptPath}`); + return r.stdout; +} + // Every non-Claude runtime home probe the snippet must contain. // Key: runtime name (for diagnostics). Value: the substring that must appear // in the snippet (the env-var-with-default expansion that probes that runtime's @@ -1291,8 +1316,7 @@ describe('bug-891: non-Claude runtime home fallback arms', () => { const scriptPath = path.join(fakeRuntime, 'test-hermes-home.sh'); fs.writeFileSync(scriptPath, scriptContent); - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: isolatedPath, HOME: fakeHome, HERMES_HOME: fakeHermesHome }, }); @@ -1341,8 +1365,7 @@ describe('bug-891: non-Claude runtime home fallback arms', () => { const scriptPath = path.join(fakeRuntime, 'test-hermes-default.sh'); fs.writeFileSync(scriptPath, scriptContent); - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: isolatedPath, HOME: fakeHome }, }); @@ -1436,7 +1459,8 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { readFileNormalized } = require('./helpers.cjs'); const WORKFLOW_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'next.md'); @@ -1519,15 +1543,17 @@ function runResolver({ cwd, runtimeDir, pathDir }) { // suite is guarded to POSIX (matches the host suite's own bash -c guard). if (process.platform === 'win32') return ''; - return execFileSync('bash', ['-c', script], { + const r = runHookSeam('-c', [script], { + interpreter: 'bash', cwd, env: { ...process.env, PATH: `${pathDir}${path.delimiter}${process.env.PATH || ''}`, RUNTIME_DIR: runtimeDir || '', }, - encoding: 'utf8', }); + throwIfFailed(r, 'bash -c '); + return r.stdout; } function writeExecutable(file, content) { @@ -1619,12 +1645,24 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { execFileSync } = require('node:child_process'); +const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); +/** + * Run a bash script FILE via the process seam, preserving the throw-on- + * nonzero-exit semantics of the execFileSync('bash', [path], ...) idiom + * this replaces. + */ +function runBashFile(scriptPath, options = {}) { + const r = runHookSeam(scriptPath, [], { interpreter: 'bash', ...options }); + throwIfFailed(r, `bash ${scriptPath}`); + return r.stdout; +} + // The probe string that must appear in the snippet for the new repo-local check. // The snippet uses _GSD_RUNTIME_ROOT as the intermediate variable. const LOCAL_CLAUDE_PROBE = '_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/'; @@ -1732,8 +1770,7 @@ describe('bug-444: resolver finds repo-local .claude install', () => { // Keep node in PATH (needed to run the .cjs stub); remove gsd-tools const isolatedPath = makeIsolatedPath([noToolsBin]); - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: isolatedPath, HOME: fakeHome }, }); @@ -1796,8 +1833,7 @@ describe('bug-444: resolver finds repo-local .claude install', () => { const isolatedPath = makeIsolatedPath([noToolsBin]); - const stdout = execFileSync('bash', [scriptPath], { - encoding: 'utf8', + const stdout = runBashFile(scriptPath, { env: { ...process.env, PATH: isolatedPath, HOME: fakeHome }, }); diff --git a/tests/smart-entry.unit.test.cjs b/tests/smart-entry.unit.test.cjs index 2a54e3f2c..1f864a7d6 100644 --- a/tests/smart-entry.unit.test.cjs +++ b/tests/smart-entry.unit.test.cjs @@ -15,8 +15,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { gitOrThrow, throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const smartEntry = require('../gsd-core/bin/lib/smart-entry.cjs'); const { classify, classifyProject, detectSignals, SITUATIONS } = smartEntry; @@ -43,9 +45,9 @@ function makeProject({ state, roadmap = false, git = false, verifyFail = false } fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), content); } if (git) { - execFileSync('git', ['init'], { cwd: tmpDir, stdio: 'pipe' }); - execFileSync('git', ['config', 'user.email', 't@t.com'], { cwd: tmpDir, stdio: 'pipe' }); - execFileSync('git', ['config', 'user.name', 'T'], { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.email', 't@t.com'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.name', 'T'], { cwd: tmpDir }); } if (verifyFail) { const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-feat'); @@ -133,8 +135,8 @@ describe('smart-entry: idle-stranded (git-dependent)', () => { })); // Commit the .planning files so the working tree is clean (untracked files // would make git_dirty true and mask the stranded signal). - execFileSync('git', ['add', '-A'], { cwd: dir, stdio: 'pipe' }); - execFileSync('git', ['commit', '-m', 'init'], { cwd: dir, stdio: 'pipe' }); + gitOrThrow(['add', '-A'], { cwd: dir }); + gitOrThrow(['commit', '-m', 'init'], { cwd: dir }); const base = detectSignals(dir); assert.equal(base.git_dirty, false); assert.equal(base.git_unpushed, false); @@ -413,10 +415,9 @@ describe('smart-entry: CLI dispatch (gsd-tools smart-entry)', () => { // A bare tmpdir with no .planning is a true no-project. const bare = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-se-bare-')); track(bare); - const out = execFileSync(process.execPath, [TOOLS, 'smart-entry', '--json', '--cwd', bare], { - encoding: 'utf-8', - }); - const j = JSON.parse(out); + const r = runNode([TOOLS, 'smart-entry', '--json', '--cwd', bare], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(r, 'gsd-tools smart-entry --json'); + const j = JSON.parse(r.stdout); assert.equal(j.situation, 'no-project'); assert.equal(j.recommended, 'new-project'); assert.equal(j.actions[0].command, '/gsd:new-project'); @@ -425,9 +426,9 @@ describe('smart-entry: CLI dispatch (gsd-tools smart-entry)', () => { test('default (human) mode prints a plain summary line, not JSON', () => { const bare = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-se-human-')); track(bare); - const out = execFileSync(process.execPath, [TOOLS, 'smart-entry', '--cwd', bare], { - encoding: 'utf-8', - }); + const r = runNode([TOOLS, 'smart-entry', '--cwd', bare], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(r, 'gsd-tools smart-entry'); + const out = r.stdout; assert.ok(!out.startsWith('{'), 'human mode is not JSON'); assert.match(out, /No project yet/); assert.match(out, /Recommended:/); diff --git a/tests/spec-section.test.cjs b/tests/spec-section.test.cjs index 17fa00e9d..5df2521cd 100644 --- a/tests/spec-section.test.cjs +++ b/tests/spec-section.test.cjs @@ -20,7 +20,9 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const fs = require('node:fs'); const os = require('node:os'); -const { spawnSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const BUILT_SCRIPT = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'spec-section.cjs'); const ss = require(BUILT_SCRIPT); @@ -131,7 +133,7 @@ describe('spec-section: countSectionDataRows / specSectionStatus', () => { describe('spec-section: CLI', () => { test('prints JSON status and exits 0 for a valid key', () => { const p = writeTmp('cli.md', PROHIB_SUFFIX_SPEC); - const r = spawnSync(process.execPath, [BUILT_SCRIPT, p, 'prohibitions'], { encoding: 'utf8' }); + const r = toLegacyResult(runNode([BUILT_SCRIPT, p, 'prohibitions'], { timeoutMs: PROBE_TIMEOUT_MS })); assert.equal(r.status, 0); const out = JSON.parse(r.stdout); assert.equal(out.supplied, true); @@ -140,7 +142,7 @@ describe('spec-section: CLI', () => { test('exits 2 on a bad key', () => { const p = writeTmp('cli2.md', PROHIB_SUFFIX_SPEC); - const r = spawnSync(process.execPath, [BUILT_SCRIPT, p, 'bogus'], { encoding: 'utf8' }); + const r = toLegacyResult(runNode([BUILT_SCRIPT, p, 'bogus'], { timeoutMs: PROBE_TIMEOUT_MS })); assert.equal(r.status, 2); }); }); diff --git a/tests/state-rebuild-cli.test.cjs b/tests/state-rebuild-cli.test.cjs index 4d72f4a15..44fcc3c97 100644 --- a/tests/state-rebuild-cli.test.cjs +++ b/tests/state-rebuild-cli.test.cjs @@ -10,7 +10,9 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const { execFileSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { createTempProject, @@ -295,12 +297,14 @@ describe('ADR-1817 Phase 2: `state rebuild` CLI subcommand dispatch (criterion # // runGsdTools returns stdout only on success; --verbose writes to stderr, // so invoke gsd-tools directly to capture both streams separately. - const stdout = execFileSync( - process.execPath, + const r = runNode( [TOOLS_PATH, 'state', 'rebuild', '--verbose'], - { cwd, encoding: 'utf8' }, + { cwd, timeoutMs: PROBE_TIMEOUT_MS }, ); - // execFileSync does not separate stderr — stderr is inherited by default. + throwIfFailed(r, 'gsd-tools state rebuild --verbose'); + const stdout = r.stdout; + // The seam's runNode captures stderr separately (unlike execFileSync's + // default inherited stderr) but this test does not need it — see below. // Assert via the canonical record written to STATE.md: the audit log // section is always appended (mutated fixture), and --verbose merely // tees the same entries to stderr. The functional guarantee (audit log diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 7def31cf7..8f0d61499 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -7164,7 +7164,8 @@ function readState(dir) { } function runGsdState(args, cwd) { - const { execFileSync } = require('child_process'); + const { runNode } = require('./helpers/process-seam.cjs'); + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const env = { ...process.env, GSD_SESSION_KEY: '', @@ -7173,17 +7174,9 @@ function runGsdState(args, cwd) { CLAUDE_CODE_SSE_PORT: '', OPENCODE_SESSION_ID: '', }; - try { - execFileSync(process.execPath, [TOOLS_PATH, 'state', ...args], { - cwd, - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], - env, - }); - return { success: true }; - } catch (err) { - return { success: false, error: err.stderr?.toString().trim() || err.message }; - } + const r = runNode([TOOLS_PATH, 'state', ...args], { cwd, env, timeoutMs: PROBE_TIMEOUT_MS }); + if (r.exitCode === 0) return { success: true }; + return { success: false, error: r.stderr.trim() || `exited with outcome=${r.outcome} exitCode=${r.exitCode}` }; } // --------------------------------------------------------------------------- diff --git a/tests/workflow-guard.test.cjs b/tests/workflow-guard.test.cjs index 0e93f5c97..3b8eba9b8 100644 --- a/tests/workflow-guard.test.cjs +++ b/tests/workflow-guard.test.cjs @@ -16,11 +16,11 @@ process.env.GSD_TEST_MODE = '1'; const { test, describe, before, after } = require('node:test'); const assert = require('node:assert/strict'); -const { execSync } = require('node:child_process'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); @@ -47,10 +47,12 @@ describe('#2304: Kimi tool vocabulary engages the workflow guard', () => { before(() => { repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-workflow-guard-')); - execSync( - 'git init -q -b worktree-agent-test && git config user.email t@t && git config user.name t', - { cwd: repoDir, stdio: 'ignore' } + const initResult = runHookSeam( + '-c', + ['git init -q -b worktree-agent-test && git config user.email t@t && git config user.name t'], + { interpreter: 'bash', cwd: repoDir }, ); + throwIfFailed(initResult, 'bash -c '); fs.mkdirSync(path.join(repoDir, '.planning')); fs.writeFileSync( path.join(repoDir, '.planning', 'config.json'),