From cbd180c5cd8464f882ba9ef272505b987070bb4e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 7 Aug 2026 15:45:51 -0400 Subject: [PATCH] test(#3147): bound the lint/changeset/docs cluster onto the process seam (#3181) * test(#3147): bound the lint/changeset/docs cluster onto the process seam Migrates 69 unbounded sync spawn sites across 24 files. Allowlist 73 to 49. Two shared helpers move: tests/helpers/graphify.cjs (6 importing suites) and tests/fixtures/index.cjs, whose three quoted-argument shell strings became single argv elements rather than whitespace splits. changeset-lint's throw-native git() helper routes to gitOrThrow; migrating it to bare runGit would have silently swallowed a failure that is loud today. ingest-docs goes the other way -- its catch never rethrew, it degraded failure into data every call site asserts on, so throwIfFailed would have thrown where the original returned. The design doc said otherwise and was corrected. tsconfig-noemit runs a real tsc --noEmit and takes a bespoke 180000ms per the ensure-runtime-build precedent, not the 30000ms build-hooks norm -- that norm is for a file copy, and sizing against a label rather than the work is the same error in the opposite direction. * test(#3147): add toLegacyResult and settle review findings The seam exposed a throwing adapter (throwIfFailed) but no non-throwing one, so eight files independently re-derived the same unwrap back to the legacy {status, stdout, stderr} shape. That is the third time this epic produced N copies of one mechanism -- seven throw wrappers in Wave 1, fifty-two timeout constants in Wave 2, eight result adapters here. The pattern is that whenever the seam does not expose a mechanism, every suite re-derives it. toLegacyResult now sits beside throwIfFailed, with its own tests. Two sites are deliberately NOT converted: changeset-cli's runRender and runRenderIn return {status, report, stderr} from parsed JSON and never a raw stdout, so they are a different shape family. lint-legacy-dir-name keeps its local GUARD_TIMEOUT_MS: 30000 matches the build norm numerically but bounds a lint probe, not hooks bundling, and importing it would encode a coincidence as a relationship. --------- Co-authored-by: sim --- CONTEXT.md | 2 +- CONTRIBUTING.md | 24 +++++++ .../no-unbounded-spawn.allowlist.json | 24 ------- tests/changeset-cli.test.cjs | 66 ++++++++----------- tests/changeset-github-release-notes.test.cjs | 47 +++++++------ tests/changeset-lint.test.cjs | 13 ++-- tests/emitted-sizes.test.cjs | 7 +- tests/fixtures/index.cjs | 14 ++-- tests/gen-registry.test.cjs | 7 +- tests/git-fixture.test.cjs | 52 ++++++++++++++- tests/graphify-auto-update.slow.test.cjs | 38 ++++++----- tests/graphify-visualization.test.cjs | 35 ++++++---- tests/helpers/git-fixture.cjs | 40 ++++++++++- tests/helpers/graphify.cjs | 16 ++--- tests/ingest-docs.test.cjs | 36 +++++----- .../issue-844-manifest-version-sync.test.cjs | 9 ++- tests/lint-docs-command-form.test.cjs | 10 +-- tests/lint-legacy-dir-name.test.cjs | 18 +++-- tests/lint-pr-check-project-dir.test.cjs | 11 ++-- tests/lint-regression-test-names.test.cjs | 18 +++-- tests/lint-skill-deps.test.cjs | 11 ++-- tests/lint-test-file-count.test.cjs | 14 ++-- tests/mutation-matrix-ratchet.test.cjs | 11 ++-- tests/no-unbounded-spawn-allowlist.test.cjs | 5 +- tests/repo-layout.test.cjs | 16 ++--- tests/reviewer-docs-parity.test.cjs | 12 ++-- tests/skill-frontmatter-contract.test.cjs | 15 +++-- tests/slash-command-namespace.test.cjs | 20 +++--- tests/tsconfig-noemit.test.cjs | 14 ++-- tests/validate-registry.test.cjs | 7 +- 30 files changed, 381 insertions(+), 231 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index ac8af7a86..bca0fa553 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -424,7 +424,7 @@ An injectable time abstraction accepted as an optional parameter by production c 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. ### 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). +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. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6f095cfab..0a7100309 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -468,6 +468,30 @@ const r = runNode([BUILD_SCRIPT], { timeoutMs: BUILD_TIMEOUT_MS }); throwIfFailed(r, 'build-hooks.js (before install tests)'); ``` +#### When you want the legacy shape without a throw: `toLegacyResult` + +Some call sites never wanted a throw in the first place — they already branch on exit status as +data, reading `.status`/`.stdout`/`.stderr` off the result themselves. Those still need the seam's +`exitCode` renamed to the legacy `status` field their assertions expect. Use `toLegacyResult` +instead of hand-rolling the three-line mapping — ~8 test files did exactly that independently +before this export existed (#3147): + +```javascript +const { toLegacyResult } = require('./helpers/git-fixture.cjs'); + +function runLint(args = []) { + const r = runNode([LINT_SCRIPT, ...args], { timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); // { status, stdout, stderr } +} +``` + +It is a bare mapping and nothing more. If your call site needs an extra field beyond that shape (a +parsed-JSON body, a fixture-specific path alongside the result), compose it rather than extending +the helper: `{ ...toLegacyResult(result), extra }`. And if your site's return shape genuinely +diverges from `{ status, stdout, stderr }` — e.g. it substitutes a parsed report object for raw +`stdout` — leave it as its own local mapping; forcing every result-reshaping helper onto one shared +function is the same drift `toLegacyResult` exists to prevent, just in the other direction. + #### The lint rule that enforces it `local/no-unbounded-spawn` (`eslint-rules/no-unbounded-spawn.cjs`) fails any `spawnSync`, diff --git a/eslint-rules/no-unbounded-spawn.allowlist.json b/eslint-rules/no-unbounded-spawn.allowlist.json index 48cc1c283..45f0f9bc0 100644 --- a/eslint-rules/no-unbounded-spawn.allowlist.json +++ b/eslint-rules/no-unbounded-spawn.allowlist.json @@ -3,9 +3,6 @@ "tests/agent-skills.test.cjs", "tests/autonomous-converge.test.cjs", "tests/bugs-1656-1657.test.cjs", - "tests/changeset-cli.test.cjs", - "tests/changeset-github-release-notes.test.cjs", - "tests/changeset-lint.test.cjs", "tests/check-tdd-review-checkpoint-e2e.test.cjs", "tests/check-ui-safety-gate.test.cjs", "tests/check-update-config-dir.test.cjs", @@ -21,7 +18,6 @@ "tests/configuration-migrate-config.test.cjs", "tests/drift-detection.test.cjs", "tests/edge-probe.test.cjs", - "tests/emitted-sizes.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", @@ -30,46 +26,26 @@ "tests/fix-3045-cursor-subagent-isolation.test.cjs", "tests/fix-3045-dispatch-isolation-resolver.test.cjs", "tests/fixture-builder.test.cjs", - "tests/fixtures/index.cjs", "tests/frontmatter-cli.test.cjs", - "tests/gen-registry.test.cjs", - "tests/graphify-auto-update.slow.test.cjs", - "tests/graphify-visualization.test.cjs", "tests/gsd-agent-isolation-guard.test.cjs", "tests/gsd-statusline.test.cjs", "tests/helpers.cjs", - "tests/helpers/graphify.cjs", "tests/host-integration.test.cjs", - "tests/ingest-docs.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/issue-844-manifest-version-sync.test.cjs", - "tests/lint-docs-command-form.test.cjs", - "tests/lint-legacy-dir-name.test.cjs", - "tests/lint-pr-check-project-dir.test.cjs", - "tests/lint-regression-test-names.test.cjs", - "tests/lint-skill-deps.test.cjs", - "tests/lint-test-file-count.test.cjs", - "tests/mutation-matrix-ratchet.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/repo-layout.test.cjs", - "tests/reviewer-docs-parity.test.cjs", "tests/run-tests-harness.test.cjs", "tests/runtime-launcher-parity.test.cjs", - "tests/skill-frontmatter-contract.test.cjs", - "tests/slash-command-namespace.test.cjs", "tests/smart-entry.unit.test.cjs", "tests/spec-section.test.cjs", "tests/state-rebuild-cli.test.cjs", "tests/state.test.cjs", - "tests/tsconfig-noemit.test.cjs", - "tests/validate-registry.test.cjs", "tests/workflow-guard.test.cjs" ] diff --git a/tests/changeset-cli.test.cjs b/tests/changeset-cli.test.cjs index 03247aa0c..5d214c20e 100644 --- a/tests/changeset-cli.test.cjs +++ b/tests/changeset-cli.test.cjs @@ -6,8 +6,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const cp = require('node:child_process'); 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, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'cli.cjs'); @@ -24,21 +26,20 @@ function writeFragment(name, type, pr, body) { } function runRender(args = []) { - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'render', '--repo', tmp, ...args, '--json'], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); return { - status: r.status, + status: r.exitCode, report: r.stdout && r.stdout.length ? JSON.parse(r.stdout) : null, stderr: r.stderr || '', }; } function runRenderRaw(args = []) { - const r = cp.spawnSync(process.execPath, [SCRIPT, 'render', '--repo', tmp, ...args], { encoding: 'utf8' }); - return { status: r.status, stdout: r.stdout || '', stderr: r.stderr || '' }; + const r = runNode([SCRIPT, 'render', '--repo', tmp, ...args], { timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); } before(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-changeset-')); }); @@ -82,15 +83,15 @@ function runExtract(args = [], changelogText = null) { if (changelogText !== null) { fs.writeFileSync(changelogFile, changelogText); } - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'extract', '--changelog', changelogFile, ...args], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); + // Composes toLegacyResult with the site-specific parsed-JSON `json` field + // rather than folding it into the shared helper — see git-fixture.cjs's + // toLegacyResult JSDoc. return { - status: r.status, - stdout: r.stdout || '', - stderr: r.stderr || '', + ...toLegacyResult(r), json: (() => { try { return JSON.parse(r.stdout); } catch { return null; } })(), @@ -450,13 +451,12 @@ describe('changeset cli #690 regression: CHANGELOG.md has 1.3.0 and 1.3.1 entrie }); test('extract 1.2.0->1.3.1 against repo CHANGELOG returns both 1.3.x releases (regression #690)', () => { - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'extract', '--from', '1.2.0', '--to', '1.3.1', '--changelog', CHANGELOG_PATH, '--json'], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); const json = (() => { try { return JSON.parse(r.stdout); } catch { return null; } })(); - assert.equal(r.status, 0, `expected exit 0 but got ${r.status}; stderr=${r.stderr}; stdout=${r.stdout}`); + assert.equal(r.exitCode, 0, `expected exit 0 but got ${r.exitCode}; stderr=${r.stderr}; stdout=${r.stdout}`); assert.ok(json, 'stdout must be valid JSON'); const versions = (json.releases || []).map((rel) => rel.version); assert.ok(versions.includes('1.3.0'), `releases array must include 1.3.0; got: ${JSON.stringify(versions)}`); @@ -471,16 +471,11 @@ describe('changeset cli #690 regression: CHANGELOG.md has 1.3.0 and 1.3.1 entrie function runVerify(args, changelogText) { const changelogFile = path.join(tmp, 'CHANGELOG-verify-test.md'); fs.writeFileSync(changelogFile, changelogText); - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'verify', '--changelog', changelogFile, ...args], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); - return { - status: r.status, - stdout: r.stdout || '', - stderr: r.stderr || '', - }; + return toLegacyResult(r); } describe('changeset cli verify subcommand (not yet implemented — TDD red step)', () => { @@ -603,12 +598,11 @@ describe('changeset cli verify subcommand (not yet implemented — TDD red step) ].join('\n'); const changelogFile = path.join(tmp, 'CHANGELOG-verify-test.md'); fs.writeFileSync(changelogFile, FIXTURE_DATED); - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'verify', '--version', '1.3.1', '--json', '--changelog', changelogFile], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); - assert.equal(r.status, 0, `expected exit 0; stderr=${r.stderr}`); + assert.equal(r.exitCode, 0, `expected exit 0; stderr=${r.stderr}`); const json = (() => { try { return JSON.parse(r.stdout); } catch { return null; } })(); assert.ok(json, 'stdout must be valid JSON'); assert.strictEqual(json.ok, true, 'json.ok must be true'); @@ -630,13 +624,12 @@ describe('changeset cli verify subcommand (not yet implemented — TDD red step) describe('changeset cli render --allow-empty', () => { // Helper: run render for a specific test-local tmp directory. function runRenderIn(dir, args = []) { - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'render', '--repo', dir, ...args, '--json'], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); return { - status: r.status, + status: r.exitCode, report: r.stdout && r.stdout.length ? JSON.parse(r.stdout) : null, stderr: r.stderr || '', }; @@ -644,12 +637,11 @@ describe('changeset cli render --allow-empty', () => { function runVerifyIn(dir, version) { const changelogPath = path.join(dir, 'CHANGELOG.md'); - const r = cp.spawnSync( - process.execPath, + const r = runNode( [SCRIPT, 'verify', '--version', version, '--changelog', changelogPath], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); - return { status: r.status, stdout: r.stdout || '', stderr: r.stderr || '' }; + return toLegacyResult(r); } // Test 1: --allow-empty with zero fragments writes a dated heading + diff --git a/tests/changeset-github-release-notes.test.cjs b/tests/changeset-github-release-notes.test.cjs index 4ffc973e0..d56f68b09 100644 --- a/tests/changeset-github-release-notes.test.cjs +++ b/tests/changeset-github-release-notes.test.cjs @@ -6,8 +6,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const cp = require('node:child_process'); const helpers = 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', 'changeset', 'cli.cjs'); @@ -18,10 +20,14 @@ const { renderGithubReleaseNotes, } = require(path.join(ROOT, 'scripts', 'changeset', 'github-release-notes.cjs')); -function run(command, args, cwd, env) { - const result = cp.spawnSync(command, args, { cwd, encoding: 'utf8', env: env || process.env }); - assert.equal(result.status, 0, `${command} ${args.join(' ')}\nstdout=${result.stdout}\nstderr=${result.stderr}`); - return result.stdout; +// Every call site in this file runs `git` — this helper is git-only +// (gitOrThrow), which preserves its original throw-on-failure semantics (it +// used to assert.equal(status, 0)). The generic `command` parameter this +// used to carry was dropped (#3147 pre-PR review finding): every call site +// passed the literal 'git' and immediately asserted it, so the parameter +// carried no information. +function run(args, cwd, env) { + return gitOrThrow(args, { cwd, env: env || process.env }); } function writeFragment(repo, name, type, pr, body) { @@ -43,23 +49,23 @@ function createTaggedRepo() { GIT_CONFIG_GLOBAL: emptyGitConfig, GIT_CONFIG_SYSTEM: emptyGitConfig, }; - run('git', ['init', '-q'], repo, gitEnv); + run(['init', '-q'], repo, gitEnv); // Belt-and-suspenders: also set local repo config to disable signing - run('git', ['config', 'user.email', 'test@example.com'], repo, gitEnv); - run('git', ['config', 'user.name', 'Test User'], repo, gitEnv); - run('git', ['config', 'commit.gpgSign', 'false'], repo, gitEnv); - run('git', ['config', 'tag.gpgSign', 'false'], repo, gitEnv); - run('git', ['config', 'tag.forceSignAnnotated', 'false'], repo, gitEnv); + run(['config', 'user.email', 'test@example.com'], repo, gitEnv); + run(['config', 'user.name', 'Test User'], repo, gitEnv); + run(['config', 'commit.gpgSign', 'false'], repo, gitEnv); + run(['config', 'tag.gpgSign', 'false'], repo, gitEnv); + run(['config', 'tag.forceSignAnnotated', 'false'], repo, gitEnv); fs.writeFileSync(path.join(repo, 'README.md'), 'fixture\n'); - run('git', ['add', 'README.md'], repo, gitEnv); - run('git', ['commit', '-q', '-m', 'initial'], repo, gitEnv); - run('git', ['tag', 'v1.0.0'], repo, gitEnv); + run(['add', 'README.md'], repo, gitEnv); + run(['commit', '-q', '-m', 'initial'], repo, gitEnv); + run(['tag', 'v1.0.0'], repo, gitEnv); writeFragment(repo, 'fix-install-sdk', 'Fixed', 101, '**`gsd-sdk` now installs reliably** — persistent PATH is checked.'); writeFragment(repo, 'remove-intel-noise', 'Removed', 102, '**`gsd-intel-updater` no longer emits layout detection noise** — ordinary projects stay quiet.'); - run('git', ['add', '.changeset'], repo, gitEnv); - run('git', ['commit', '-q', '-m', 'add changesets'], repo, gitEnv); - run('git', ['tag', 'v1.0.1'], repo, gitEnv); + run(['add', '.changeset'], repo, gitEnv); + run(['commit', '-q', '-m', 'add changesets'], repo, gitEnv); + run(['tag', 'v1.0.1'], repo, gitEnv); return repo; } @@ -101,8 +107,7 @@ describe('changeset github release notes: tag-range renderer (#3382)', () => { test('CLI writes a notes file suitable for gh release edit --notes-file', () => { const repo = (_repo = createTaggedRepo()); const output = path.join(repo, 'release-notes.md'); - const result = cp.spawnSync( - process.execPath, + const result = runNode( [ SCRIPT, 'github-release-notes', @@ -113,10 +118,10 @@ describe('changeset github release notes: tag-range renderer (#3382)', () => { '--output', output, '--json', ], - { encoding: 'utf8' }, + { timeoutMs: PROBE_TIMEOUT_MS }, ); - assert.equal(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); + assert.equal(result.exitCode, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); const report = JSON.parse(result.stdout); assert.deepEqual( { consumed: report.consumed, output: report.output, hasBodyInJson: report.body !== null }, diff --git a/tests/changeset-lint.test.cjs b/tests/changeset-lint.test.cjs index 6775d142d..eca70c164 100644 --- a/tests/changeset-lint.test.cjs +++ b/tests/changeset-lint.test.cjs @@ -6,7 +6,6 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const fs = require('node:fs'); const os = require('node:os'); -const cp = require('node:child_process'); const { evaluateLint, LINT_REASON, DEFAULT_BASE: CHANGESET_DEFAULT_BASE } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'lint.cjs')); const { DEFAULT_BASE: DOCS_DEFAULT_BASE } = require(path.join(__dirname, '..', 'scripts', 'lint-docs-required.cjs')); @@ -14,6 +13,9 @@ const { DEFAULT_BASE: DOCS_DEFAULT_BASE } = require(path.join(__dirname, '..', ' const ROOT = path.join(__dirname, '..'); const LINT_SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'lint.cjs'); const { cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); /** * Build a minimal temp git repo shaped like a PR branch: @@ -26,7 +28,7 @@ const { cleanup } = require('./helpers.cjs'); * @returns {string} path to the temp repo (same as tmpDir) */ function buildTempRepo(tmpDir, prFiles, baseFiles = []) { - const git = (...args) => cp.execFileSync('git', args, { cwd: tmpDir, encoding: 'utf8' }); + const git = (...args) => gitOrThrow(args, { cwd: tmpDir }); git('init', '-q', '-b', 'main'); git('config', 'user.email', 'test@example.com'); @@ -72,18 +74,17 @@ function buildTempRepo(tmpDir, prFiles, baseFiles = []) { * @returns {{ status: number, report: object }} */ function runLint(repoDir) { - const result = cp.spawnSync( - process.execPath, + const result = runNode( [LINT_SCRIPT, '--json'], { cwd: repoDir, env: { ...process.env, GITHUB_BASE_REF: 'main', GITHUB_EVENT_PATH: '' }, - encoding: 'utf8', + timeoutMs: PROBE_TIMEOUT_MS, }, ); let report = {}; try { report = JSON.parse(result.stdout); } catch { /* leave as empty object */ } - return { status: result.status, report }; + return { status: result.exitCode, report }; } // evaluateLint is a pure function over file lists + label list — no fs, no git. diff --git a/tests/emitted-sizes.test.cjs b/tests/emitted-sizes.test.cjs index 733c68325..9481ec77e 100644 --- a/tests/emitted-sizes.test.cjs +++ b/tests/emitted-sizes.test.cjs @@ -16,9 +16,11 @@ 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 { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { BUILD_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { BUILD_SCRIPT, runMinimalInstall, @@ -30,7 +32,8 @@ const { // idempotently before the shared real-install fixture, mirroring // tests/golden-install-tree.test.cjs. before(() => { - execFileSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf-8', stdio: 'pipe' }); + const r = runNode([BUILD_SCRIPT], { timeoutMs: BUILD_TIMEOUT_MS }); + throwIfFailed(r, `node ${BUILD_SCRIPT}`); }); // ─── Shared real-install fixture (B1, B2, B7) ───────────────────────────────── diff --git a/tests/fixtures/index.cjs b/tests/fixtures/index.cjs index 2bea76185..019afec89 100644 --- a/tests/fixtures/index.cjs +++ b/tests/fixtures/index.cjs @@ -1,7 +1,7 @@ -const { execSync } = require('child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { gitOrThrow } = require('../helpers/git-fixture.cjs'); /** * Create a temp test fixture directory with canonical planning layout. @@ -36,16 +36,16 @@ function createFixture(options = {}) { } if (git) { - execSync('git init', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config user.email "test@test.com"', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config user.name "Test"', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git config commit.gpgsign false', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git add -A', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['init'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.email', 'test@test.com'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: tmpDir }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd: tmpDir }); + gitOrThrow(['add', '-A'], { cwd: tmpDir }); // `--allow-empty`: a fixture with `git: true, planning: false, // projectDoc: false` (e.g. a "greenfield" starting world) stages nothing, // so a plain `git commit` would fail with "nothing to commit" and the // caller would never get a usable repo (there'd be no HEAD at all). - execSync('git commit --allow-empty -m "initial commit"', { cwd: tmpDir, stdio: 'pipe' }); + gitOrThrow(['commit', '--allow-empty', '-m', 'initial commit'], { cwd: tmpDir }); } return tmpDir; diff --git a/tests/gen-registry.test.cjs b/tests/gen-registry.test.cjs index 7202feb82..9f9fb8744 100644 --- a/tests/gen-registry.test.cjs +++ b/tests/gen-registry.test.cjs @@ -6,8 +6,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -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 { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const SCRIPT_PATH = path.join(__dirname, '..', 'scripts', 'gen-registry.cjs'); const { renderMarkdown } = require(path.join(__dirname, '..', 'scripts', 'registry-schema.cjs')); @@ -127,7 +129,8 @@ function withReviewerFixture(capabilityEntries, reviewerEntries, fn) { } function runGen(cwd, args = []) { - return spawnSync(process.execPath, [SCRIPT_PATH, ...args], { cwd, encoding: 'utf8' }); + const r = runNode([SCRIPT_PATH, ...args], { cwd, timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); } describe('gen-registry CLI (subprocess)', () => { diff --git a/tests/git-fixture.test.cjs b/tests/git-fixture.test.cjs index 2572040af..a381bb135 100644 --- a/tests/git-fixture.test.cjs +++ b/tests/git-fixture.test.cjs @@ -30,7 +30,7 @@ const { OUTCOME } = processSeam; const runGitSpy = mock.method(processSeam, 'runGit'); after(() => mock.restoreAll()); -const { gitOrThrow, throwIfFailed, DEFAULT_GIT_TIMEOUT_MS } = require('./helpers/git-fixture.cjs'); +const { gitOrThrow, throwIfFailed, toLegacyResult, DEFAULT_GIT_TIMEOUT_MS } = require('./helpers/git-fixture.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); /** @@ -283,3 +283,53 @@ describe('throwIfFailed', () => { assert.ok(caught.message.includes('my distinctive display name')); }); }); + +/** + * `toLegacyResult` is the non-throwing sibling of `throwIfFailed` (#3147): + * a bare mapping onto `{ status, stdout, stderr }`, never throwing. Tested + * directly with literal result objects — no subprocess needed. + */ +describe('toLegacyResult', () => { + test('never throws, unlike throwIfFailed, for a non-zero exit', () => { + assert.doesNotThrow(() => + toLegacyResult({ outcome: OUTCOME.EXITED, exitCode: 1, stdout: '', stderr: 'boom' }) + ); + }); + + test('maps exitCode to .status (the legacy field name)', () => { + const r = toLegacyResult({ outcome: OUTCOME.EXITED, exitCode: 1, stdout: 'out', stderr: 'err' }); + assert.equal(r.status, 1); + }); + + test('exitCode: 0 maps to .status: 0, not falsy-coerced', () => { + const r = toLegacyResult({ outcome: OUTCOME.EXITED, exitCode: 0, stdout: '', stderr: '' }); + assert.equal(r.status, 0); + assert.notEqual(r.status, undefined); + }); + + test('exitCode: null (non-EXITED outcome) propagates to .status as null, not coerced', () => { + const r = toLegacyResult({ outcome: OUTCOME.TIMED_OUT, exitCode: null, stdout: '', stderr: '' }); + assert.equal(r.status, null); + assert.notEqual(r.status, undefined); + }); + + test('stdout and stderr pass through unchanged', () => { + const r = toLegacyResult({ outcome: OUTCOME.EXITED, exitCode: 0, stdout: 'the stdout', stderr: 'the stderr' }); + assert.equal(r.stdout, 'the stdout'); + assert.equal(r.stderr, 'the stderr'); + }); + + test('returns exactly the three legacy fields, nothing extra from the seam result', () => { + const r = toLegacyResult({ + outcome: OUTCOME.EXITED, + exitCode: 0, + stdout: '', + stderr: '', + signal: null, + timedOut: false, + killed: false, + code: null, + }); + assert.deepEqual(Object.keys(r).sort(), ['status', 'stderr', 'stdout']); + }); +}); diff --git a/tests/graphify-auto-update.slow.test.cjs b/tests/graphify-auto-update.slow.test.cjs index 5866e5438..fd5b20486 100644 --- a/tests/graphify-auto-update.slow.test.cjs +++ b/tests/graphify-auto-update.slow.test.cjs @@ -12,9 +12,10 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('node:os'); -const { execFileSync, spawnSync } = require('child_process'); const { createTempProject, cleanup, runGsdTools, delay } = require('./helpers.cjs'); -const { runHook: seamRunHook } = require('./helpers/process-seam.cjs'); +const { runGit, runHook: seamRunHook } = require('./helpers/process-seam.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { graphifyStatus, @@ -206,15 +207,15 @@ describe('auto-update', () => { function createTempGitRepo(opts = {}) { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3347-')); - spawnSync('git', ['init', '-b', opts.defaultBranch || 'main'], { - cwd: tmpDir, - stdio: 'ignore', - }); - execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: tmpDir }); - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: tmpDir }); + // Original discarded the result unchecked (stdio: 'ignore', never a + // throw-native execFileSync) — bare runGit preserves that never-throws, + // ignored-result shape. + runGit(['init', '-b', opts.defaultBranch || 'main'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: tmpDir }); fs.writeFileSync(path.join(tmpDir, 'README.md'), '# test\n'); - execFileSync('git', ['add', 'README.md'], { cwd: tmpDir }); - execFileSync('git', ['commit', '-m', 'init'], { cwd: tmpDir, stdio: 'ignore' }); + gitOrThrow(['add', 'README.md'], { cwd: tmpDir }); + gitOrThrow(['commit', '-m', 'init'], { cwd: tmpDir }); fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); if (opts.config !== undefined) { @@ -254,6 +255,9 @@ describe('auto-update', () => { // suite and the hook itself dispatches a detached graphify rebuild that // some tests wait on separately; the hook's own synchronous return (gate // checks + status-file write) is fast, so 30s stays generous headroom. + // Original invoked the hook with stdio: 'ignore' — the seam always + // captures stdout/stderr instead, but every call site of this wrapper + // below reads only `.status`; the captured output is simply unread. const r = seamRunHook(HOOK, [], { interpreter: 'bash', cwd: tmpDir, @@ -305,7 +309,14 @@ describe('auto-update', () => { try { const pid = parseInt(fs.readFileSync(lockPath, 'utf8'), 10); if (!Number.isFinite(pid) || pid <= 0) break; - execFileSync('kill', ['-0', String(pid)], { stdio: 'ignore' }); + // `kill -0` on a live PID exits 0; on a dead PID (or missing `kill`) + // it exits non-zero — this loop only cares about that distinction, + // so it reads exitCode as data instead of preserving a throw. + // Original ran with stdio: 'ignore' — the seam always captures + // stdout/stderr instead, but only `probe.exitCode` is read below; + // the captured output is simply unread. + const probe = seamRunHook('-0', [String(pid)], { interpreter: 'kill', timeoutMs: PROBE_TIMEOUT_MS }); + if (probe.exitCode !== 0) break; // PID dead → safe to clean up } catch { break; // PID dead → safe to clean up } @@ -386,10 +397,7 @@ describe('auto-update', () => { config: { graphify: { enabled: true, auto_update: true } }, }); t.after(() => cleanupHookRepo(tmpDir)); - execFileSync('git', ['checkout', '-b', 'worktree-agent-abc'], { - cwd: tmpDir, - stdio: 'ignore', - }); + gitOrThrow(['checkout', '-b', 'worktree-agent-abc'], { cwd: tmpDir }); const mockBin = makeMockGraphifyBin(tmpDir); const r = runHook( tmpDir, diff --git a/tests/graphify-visualization.test.cjs b/tests/graphify-visualization.test.cjs index cd2c7002e..99cea213d 100644 --- a/tests/graphify-visualization.test.cjs +++ b/tests/graphify-visualization.test.cjs @@ -8,7 +8,9 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('node:os'); -const { execFileSync } = require('child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { BUILD_TIMEOUT_MS, INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { createTempProject, createTempGitProject, cleanup } = require('./helpers.cjs'); const { @@ -490,7 +492,8 @@ describe('regressions', () => { // Regression for #3579: Gap 1 — build-hooks.js packages every top-level hooks/*.sh describe('#3579 Gap 1: build-hooks.js packages every top-level hooks/*.sh into dist', () => { before(() => { - execFileSync(process.execPath, [BUILD_SCRIPT_3579], { encoding: 'utf-8', stdio: 'pipe' }); + const r = runNode([BUILD_SCRIPT_3579], { timeoutMs: BUILD_TIMEOUT_MS }); + throwIfFailed(r, `node ${BUILD_SCRIPT_3579}`); }); test('every top-level hooks/*.sh is emitted to hooks/dist/ by the build', () => { @@ -532,17 +535,18 @@ describe('regressions', () => { let installStdout; before(() => { - execFileSync(process.execPath, [BUILD_SCRIPT_3579], { encoding: 'utf-8', stdio: 'pipe' }); + const r1 = runNode([BUILD_SCRIPT_3579], { timeoutMs: BUILD_TIMEOUT_MS }); + throwIfFailed(r1, `node ${BUILD_SCRIPT_3579}`); tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3579-install-')); - installStdout = execFileSync( - process.execPath, + const r2 = runNode( [INSTALL_SCRIPT_3579, '--claude', '--global', '--yes', '--no-sdk'], { - encoding: 'utf-8', - stdio: 'pipe', env: { ...process.env, CLAUDE_CONFIG_DIR: tmpDir }, + timeoutMs: INSTALL_TIMEOUT_MS, } ); + throwIfFailed(r2, `node ${INSTALL_SCRIPT_3579}`); + installStdout = r2.stdout; }); after(() => { @@ -608,7 +612,9 @@ const { describe, test, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { spawnSync } = require('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'); @@ -623,8 +629,9 @@ const GRAPHIFY_MD = path.join(__dirname, '..', 'commands', 'gsd', 'graphify.md') * Returns the bash source text (without the fence lines themselves). * * readFileNormalized() strips \r\n -> \n before the match below runs — the - * extracted block is later spawned via spawnSync('bash', ...) in runBlock(), - * so an un-normalized read on a Windows checkout would break bash mid-script + * extracted block is later spawned via runHook('-c', ..., {interpreter: + * 'bash'}) in runBlock(), so an un-normalized read on a Windows checkout + * would break bash mid-script * (DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE, #2650). */ function extractStep3Block() { @@ -703,15 +710,19 @@ function populateSandbox(includeHtml) { * Execute the extracted Step 3 block in the sandbox. */ function runBlock(block) { - return spawnSync('bash', ['-c', block], { + // The extracted block is a shell chain (&&, [ -f ] guards, ||) — it stays + // a `bash -c` invocation rather than being decomposed into argv. + const r = runHook('-c', [block], { + interpreter: 'bash', cwd: sandbox, env: { ...process.env, PATH: fakeBin + ':' + process.env.PATH, HOME: fakeHome, }, - encoding: 'utf8', + timeoutMs: PROBE_TIMEOUT_MS, }); + return toLegacyResult(r); } // ─── tests ─────────────────────────────────────────────────────────────────── diff --git a/tests/helpers/git-fixture.cjs b/tests/helpers/git-fixture.cjs index c480f7543..081b51a81 100644 --- a/tests/helpers/git-fixture.cjs +++ b/tests/helpers/git-fixture.cjs @@ -2,7 +2,8 @@ /** * git-fixture — the shared throw-on-failure mechanism for process-seam - * results, plus a throw-preserving wrapper over the seam's `runGit`. + * results, its non-throwing counterpart, plus a throw-preserving wrapper + * over the seam's `runGit`. * * Why this exists: `execSync`/`execFileSync` throw on any non-zero exit, * and 237+ sites in this repo's test suite are written against that throw — @@ -24,6 +25,13 @@ * etc.) calls `throwIfFailed` directly instead of hand-rolling its own copy * of this shape — five call sites did exactly that before this module * exported it, and drifted from each other in the process (#3144). + * + * `toLegacyResult` is the non-throwing sibling: call sites that already + * branch on exit status as data (never wanted a throw) still need the + * result reshaped onto the legacy `{ status, stdout, stderr }` field names + * their assertions read — ~8 test files hand-rolled that identical mapping + * before this module exported it too (#3147). + * * `tests/helpers/process-seam.cjs` itself is NOT modified by this module — * its never-throws contract is intact; this is a layer on top, not a change * underneath. @@ -111,4 +119,32 @@ function gitOrThrow(args, options = {}) { return r.stdout; } -module.exports = { gitOrThrow, throwIfFailed, DEFAULT_GIT_TIMEOUT_MS }; +/** + * The NON-throwing counterpart to `throwIfFailed`: maps any process-seam + * result onto the legacy `execSync`/`execFileSync` `{ status, stdout, + * stderr }` shape, without ever throwing. For call sites that already read + * exit status as data (they branch on `.status`/`.stdout`/`.stderr` + * themselves) rather than wanting a throw on failure — the same "the shape + * is defined once rather than re-derived per suite" motivation as + * `throwIfFailed`, just for the non-throwing half of the split. Before this + * export existed, ~8 test files hand-rolled the identical three-line mapping + * (#3147 pre-PR review finding). + * + * This is intentionally a bare mapping and nothing more: some call sites + * layer additional site-specific behavior on top (an extra field, a parsed + * JSON body in place of raw `stdout`, etc.) — those compose `toLegacyResult` + * as a building block (e.g. `{ ...toLegacyResult(result), extra }`) rather + * than folding their extra behavior into this helper, so this shape stays + * exactly one thing everywhere it's used. + * + * @param {object} result - a process-seam result: `{outcome, exitCode, + * stdout, stderr, timedOut, signal}` (plus any seam-specific fields). + * @returns {{status: number|null, stdout: string, stderr: string}} — + * `status` is the legacy `spawnSync`/`execFileSync` field name for + * `result.exitCode`; `stdout`/`stderr` pass through unchanged. + */ +function toLegacyResult(result) { + return { status: result.exitCode, stdout: result.stdout, stderr: result.stderr }; +} + +module.exports = { gitOrThrow, throwIfFailed, toLegacyResult, DEFAULT_GIT_TIMEOUT_MS }; diff --git a/tests/helpers/graphify.cjs b/tests/helpers/graphify.cjs index 1b2dbb08b..eb7fc0be1 100644 --- a/tests/helpers/graphify.cjs +++ b/tests/helpers/graphify.cjs @@ -7,7 +7,7 @@ const fs = require('fs'); const path = require('path'); const os = require('node:os'); -const { execFileSync } = require('child_process'); +const { gitOrThrow } = require('./git-fixture.cjs'); function enableGraphify(planningDir) { const configPath = path.join(planningDir, 'config.json'); @@ -39,11 +39,11 @@ function writeSnapshotJson(planningDir, data) { } function gitHead(cwd) { - return execFileSync('git', ['rev-parse', 'HEAD'], { cwd, encoding: 'utf-8' }).trim(); + return gitOrThrow(['rev-parse', 'HEAD'], { cwd }).trim(); } function commitEmpty(cwd, message) { - execFileSync('git', ['commit', '--allow-empty', '-m', message], { cwd, stdio: 'pipe' }); + gitOrThrow(['commit', '--allow-empty', '-m', message], { cwd }); } // Helper for auto-update status tests: builds a temp git project with @@ -51,12 +51,12 @@ function commitEmpty(cwd, message) { // autoUpdateValue === null means no status file is written. function makeStatusProject(autoUpdateValue) { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3347-status-')); - execFileSync('git', ['init', '-q', '-b', 'main'], { cwd: tmpDir }); - execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: tmpDir }); - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: tmpDir }); + gitOrThrow(['init', '-q', '-b', 'main'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: tmpDir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: tmpDir }); fs.writeFileSync(path.join(tmpDir, 'README.md'), '# t\n'); - execFileSync('git', ['add', '.'], { cwd: tmpDir }); - execFileSync('git', ['commit', '-qm', 'init'], { cwd: tmpDir }); + gitOrThrow(['add', '.'], { cwd: tmpDir }); + gitOrThrow(['commit', '-qm', 'init'], { cwd: tmpDir }); fs.mkdirSync(path.join(tmpDir, '.planning/graphs'), { recursive: true }); fs.writeFileSync( path.join(tmpDir, '.planning/config.json'), diff --git a/tests/ingest-docs.test.cjs b/tests/ingest-docs.test.cjs index 40330ca27..7d4b4ff8a 100644 --- a/tests/ingest-docs.test.cjs +++ b/tests/ingest-docs.test.cjs @@ -342,29 +342,31 @@ 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 childProc = 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 REPO_ROOT = path.join(__dirname, '..'); const WORKFLOW_FILE = path.join(REPO_ROOT, 'gsd-core', 'workflows', 'ingest-docs.md'); +// Note (#3147 migration): the original execFileSync-based implementation +// caught its own throw and degraded it into `{exitCode, stdout}` data — +// nothing here ever propagated err.status/err.stdout/err.stderr to a +// caller, and every call site below asserts exitCode === 0. That is the +// "already reads exit status as data" shape (never-throwing runX, map +// .exitCode), not a throw-preserving one, so this uses `runNode` directly +// rather than `throwIfFailed`. stdout/stderr are concatenated on a non-zero +// exit to match the original catch-block behavior exactly. function spawnGsdTools(args, projectDir) { - let stdout = ''; - let exitCode = 0; - try { - stdout = childProc.execFileSync( - process.execPath, - [TOOLS_PATH, ...args, '--cwd', projectDir], - { - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], - env: { ...process.env, GSD_SESSION_KEY: '' }, - } - ); - } catch (err) { - exitCode = err.status ?? 1; - stdout = (err.stdout?.toString() ?? '') + (err.stderr?.toString() ?? ''); - } + const result = runNode( + [TOOLS_PATH, ...args, '--cwd', projectDir], + { + env: { ...process.env, GSD_SESSION_KEY: '' }, + timeoutMs: PROBE_TIMEOUT_MS, + } + ); + const exitCode = result.exitCode ?? 1; + const stdout = exitCode === 0 ? result.stdout : result.stdout + result.stderr; return { exitCode, stdout }; } diff --git a/tests/issue-844-manifest-version-sync.test.cjs b/tests/issue-844-manifest-version-sync.test.cjs index ca8dd2b3a..7d645d155 100644 --- a/tests/issue-844-manifest-version-sync.test.cjs +++ b/tests/issue-844-manifest-version-sync.test.cjs @@ -18,7 +18,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const { execFileSync } = require('child_process'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const ROOT = path.resolve(__dirname, '..'); const helpers = require(path.join(__dirname, 'helpers.cjs')); @@ -279,8 +279,11 @@ describe('C: regression guard — version-bearing JSON files must be registered' // filter to .json in JS instead). let lines; try { - const out = execFileSync('git', ['ls-files'], { cwd: ROOT }); - lines = out.toString().split('\n').filter((f) => f.endsWith('.json')); + // `gitOrThrow` returns a string (the seam is always utf-8), so no + // `.toString()` is needed here — the original Buffer#toString() call + // this replaces was a no-op on the seam's already-string stdout. + const out = gitOrThrow(['ls-files'], { cwd: ROOT }); + lines = out.split('\n').filter((f) => f.endsWith('.json')); } catch (err) { t.skip('git unavailable: ' + err.message); return; diff --git a/tests/lint-docs-command-form.test.cjs b/tests/lint-docs-command-form.test.cjs index 713e37d91..cf91f4fc4 100644 --- a/tests/lint-docs-command-form.test.cjs +++ b/tests/lint-docs-command-form.test.cjs @@ -10,19 +10,19 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { execFileSync } = require('node:child_process'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { runNode } = require('./helpers/process-seam.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const GUARD_SCRIPT = path.resolve(__dirname, '..', 'scripts', 'lint-docs-command-form.cjs'); function createTempRepo() { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lint-docs-command-form-test-')); - execFileSync('git', ['init', '--initial-branch=main'], { cwd: dir }); - execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir }); - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir }); + gitOrThrow(['init', '--initial-branch=main'], { cwd: dir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: dir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: dir }); // Roster source: the guard reads commands/gsd/*.md filenames as valid // command names, regardless of tracked/staged status. writeFile(dir, 'commands/gsd/plan-phase.md', '# plan-phase\n'); @@ -36,7 +36,7 @@ function writeFile(dir, relPath, content) { } function gitAdd(dir, relPath) { - execFileSync('git', ['add', relPath], { cwd: dir }); + gitOrThrow(['add', relPath], { cwd: dir }); } function cleanup(dir) { diff --git a/tests/lint-legacy-dir-name.test.cjs b/tests/lint-legacy-dir-name.test.cjs index b31fc04d2..14221018a 100644 --- a/tests/lint-legacy-dir-name.test.cjs +++ b/tests/lint-legacy-dir-name.test.cjs @@ -9,11 +9,11 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { execFileSync } = require('node:child_process'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { runNode } = require('./helpers/process-seam.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const GUARD_SCRIPT = path.resolve(__dirname, '..', 'scripts', 'lint-legacy-dir-name.cjs'); @@ -26,13 +26,21 @@ const GUARD_SCRIPT = path.resolve(__dirname, '..', 'scripts', 'lint-legacy-dir-n // suite already uses for the sibling check-npm-integrity.cjs script // invocation in tests/npm-integrity-gate.test.cjs (also a small-fixture, // single-subprocess CLI script). +// +// NOT sourced from `tests/helpers/timeouts.cjs`'s `BUILD_TIMEOUT_MS` even +// though the literal happens to coincide (both 30000): that shared constant +// is scoped to hooks bundling via `scripts/build-hooks.js` — a heavier, +// different class of work than this lint/guard script's single small-fixture +// git-and-regex pass. Aliasing onto BUILD_TIMEOUT_MS would tie this value's +// meaning to hook-bundling duration, which is not what bounds this call +// (#3147 pre-PR review finding). const GUARD_TIMEOUT_MS = 30_000; function createTempRepo() { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-lint-legacy-test-')); - execFileSync('git', ['init', '--initial-branch=main'], { cwd: dir }); - execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir }); - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir }); + gitOrThrow(['init', '--initial-branch=main'], { cwd: dir }); + gitOrThrow(['config', 'user.email', 'test@example.com'], { cwd: dir }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd: dir }); return dir; } @@ -43,7 +51,7 @@ function writeFile(dir, relPath, content) { } function gitAdd(dir, relPath) { - execFileSync('git', ['add', relPath], { cwd: dir }); + gitOrThrow(['add', relPath], { cwd: dir }); } function cleanup(dir) { diff --git a/tests/lint-pr-check-project-dir.test.cjs b/tests/lint-pr-check-project-dir.test.cjs index 6e347a3d9..b4cda472f 100644 --- a/tests/lint-pr-check-project-dir.test.cjs +++ b/tests/lint-pr-check-project-dir.test.cjs @@ -5,8 +5,10 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const { spawnSync } = require('child_process'); 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, '..'); const LINT_SCRIPT = path.join(ROOT, 'scripts', 'lint-pr-check-project-dir.cjs'); @@ -23,7 +25,8 @@ function createFixtureDir() { } function runLint(args = []) { - return spawnSync(process.execPath, [LINT_SCRIPT, ...args], { encoding: 'utf8' }); + const r = runNode([LINT_SCRIPT, ...args], { timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); } describe('lint-pr-check-project-dir', () => { @@ -116,7 +119,7 @@ describe('lint-pr-check-project-dir', () => { }); test('script parses without syntax errors', () => { - const result = spawnSync(process.execPath, ['--check', LINT_SCRIPT], { encoding: 'utf8' }); - assert.strictEqual(result.status, 0, result.stderr); + const result = runNode(['--check', LINT_SCRIPT], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(result.exitCode, 0, result.stderr); }); }); diff --git a/tests/lint-regression-test-names.test.cjs b/tests/lint-regression-test-names.test.cjs index 9fbd46e27..144cc28d3 100644 --- a/tests/lint-regression-test-names.test.cjs +++ b/tests/lint-regression-test-names.test.cjs @@ -7,10 +7,12 @@ const { describe, test, before, after } = 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 ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'lint-regression-test-names.cjs'); @@ -25,17 +27,19 @@ function runLint({ files, allowlist, args = [] }) { for (const f of files) fs.writeFileSync(path.join(testsDir, f), ''); const allowlistPath = path.join(testsDir, 'allowlist.json'); fs.writeFileSync(allowlistPath, JSON.stringify(allowlist)); - const r = spawnSync(process.execPath, [SCRIPT, ...args], { + const result = runNode([SCRIPT, ...args], { cwd: ROOT, - encoding: 'utf8', + timeoutMs: PROBE_TIMEOUT_MS, env: { ...process.env, GSD_LINT_REGRESSION_TESTS_DIR: testsDir, GSD_LINT_REGRESSION_ALLOWLIST: allowlistPath, }, }); - r.allowlistPath = allowlistPath; - return r; + // Composes toLegacyResult with the site-specific allowlistPath rather than + // folding it into the shared helper — see git-fixture.cjs's toLegacyResult + // JSDoc. + return { ...toLegacyResult(result), allowlistPath }; } describe('lint-regression-test-names', () => { @@ -92,8 +96,8 @@ describe('lint-regression-test-names', () => { }); test('repo baseline passes (real tests/ dir against real allowlist)', () => { - const r = spawnSync(process.execPath, [SCRIPT], { cwd: ROOT, encoding: 'utf8' }); - assert.strictEqual(r.status, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); + const r = runNode([SCRIPT], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(r.exitCode, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); }); test('novel-offender failure names the --update drift-repair path', () => { diff --git a/tests/lint-skill-deps.test.cjs b/tests/lint-skill-deps.test.cjs index 0e6254f1f..14aaedf33 100644 --- a/tests/lint-skill-deps.test.cjs +++ b/tests/lint-skill-deps.test.cjs @@ -12,13 +12,16 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const { spawnSync } = require('child_process'); 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 LINT_SCRIPT = path.join(__dirname, '..', 'scripts', 'lint-skill-deps.cjs'); function runLint(args = []) { - return spawnSync(process.execPath, [LINT_SCRIPT, ...args], { encoding: 'utf8' }); + const r = runNode([LINT_SCRIPT, ...args], { timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); } function createFixtureDir() { @@ -126,8 +129,8 @@ describe('lint-skill-deps: profile closure satisfaction', () => { describe('lint-skill-deps: script basics', () => { test('script is executable (no syntax errors)', () => { - const result = spawnSync(process.execPath, ['--check', LINT_SCRIPT], { encoding: 'utf8' }); - assert.strictEqual(result.status, 0, `Syntax error in lint script: ${result.stderr}`); + const result = runNode(['--check', LINT_SCRIPT], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(result.exitCode, 0, `Syntax error in lint script: ${result.stderr}`); }); test('prints ok message on success', () => { diff --git a/tests/lint-test-file-count.test.cjs b/tests/lint-test-file-count.test.cjs index c34b12bad..735b37553 100644 --- a/tests/lint-test-file-count.test.cjs +++ b/tests/lint-test-file-count.test.cjs @@ -10,7 +10,8 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const path = require('path'); -const { spawnSync } = require('child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.join(__dirname, '..'); const LINT_SCRIPT = path.join(ROOT, 'scripts', 'lint-test-file-count.cjs'); @@ -30,13 +31,12 @@ function makeFiles(prefix, names) { } function runCliJson(extraArgs = []) { - const result = spawnSync( - process.execPath, + const result = runNode( [LINT_SCRIPT, '--json', ...extraArgs], - { encoding: 'utf8' } + { timeoutMs: PROBE_TIMEOUT_MS } ); const parsed = JSON.parse(result.stdout); - return { status: result.status, data: parsed }; + return { status: result.exitCode, data: parsed }; } // --------------------------------------------------------------------------- @@ -302,8 +302,8 @@ describe('testEffectivePrefix', () => { describe('CLI --json', () => { test('script parses without syntax errors', () => { - const result = spawnSync(process.execPath, ['--check', LINT_SCRIPT], { encoding: 'utf8' }); - assert.strictEqual(result.status, 0, result.stderr); + const result = runNode(['--check', LINT_SCRIPT], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(result.exitCode, 0, result.stderr); }); test('exits 0 against real repo (allowlist covers all current violations)', () => { diff --git a/tests/mutation-matrix-ratchet.test.cjs b/tests/mutation-matrix-ratchet.test.cjs index bbed74283..79c1be2e5 100644 --- a/tests/mutation-matrix-ratchet.test.cjs +++ b/tests/mutation-matrix-ratchet.test.cjs @@ -22,8 +22,10 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); -const { execFileSync } = require('node:child_process'); const path = require('node:path'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const MATRIX_SCRIPT = path.resolve(__dirname, '../scripts/mutation-matrix.cjs'); const matrix = require(MATRIX_SCRIPT); @@ -116,15 +118,16 @@ describe('mutation-matrix ratchet: matrix JSON output includes minScore', () => const moduleNames = Object.keys(covered); const stdinLines = moduleNames.map(name => `src/${name}.cts`).join('\n'); - const raw = execFileSync( - process.execPath, + const spawnResult = runNode( [MATRIX_SCRIPT], { input: stdinLines + '\n', - encoding: 'utf8', cwd: path.resolve(__dirname, '..'), + timeoutMs: PROBE_TIMEOUT_MS, } ); + throwIfFailed(spawnResult, `node ${MATRIX_SCRIPT}`); + const raw = spawnResult.stdout; let result; try { diff --git a/tests/no-unbounded-spawn-allowlist.test.cjs b/tests/no-unbounded-spawn-allowlist.test.cjs index 62282078d..a35941d87 100644 --- a/tests/no-unbounded-spawn-allowlist.test.cjs +++ b/tests/no-unbounded-spawn-allowlist.test.cjs @@ -32,8 +32,9 @@ const REPO_ROOT = path.join(__dirname, '..'); // 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). -const BASELINE = 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'); diff --git a/tests/repo-layout.test.cjs b/tests/repo-layout.test.cjs index e54f37523..1bb3240b0 100644 --- a/tests/repo-layout.test.cjs +++ b/tests/repo-layout.test.cjs @@ -23,7 +23,8 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { execFileSync } = require('child_process'); +const { runGit } = require('./helpers/process-seam.cjs'); +const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const ROOT = path.resolve(__dirname, '..'); @@ -33,15 +34,10 @@ test('repo-layout: root AGENTS.md is not git-tracked — no ad-hoc AI instructio // may exist on disk — that's expected after a local install. What must NOT // happen is committing it to git, where editors and AI tools would silently // pick up the installer-generated stub instead of CONTEXT.md. - let tracked; - try { - execFileSync('git', ['ls-files', '--error-unmatch', 'AGENTS.md'], { - cwd: ROOT, encoding: 'utf8', stdio: 'pipe', - }); - tracked = true; - } catch { - tracked = false; - } + const r = runGit(['ls-files', '--error-unmatch', 'AGENTS.md'], { + cwd: ROOT, timeoutMs: GIT_TIMEOUT_MS, + }); + const tracked = r.exitCode === 0; assert.equal( tracked, false, diff --git a/tests/reviewer-docs-parity.test.cjs b/tests/reviewer-docs-parity.test.cjs index 7324a0b82..cced14904 100644 --- a/tests/reviewer-docs-parity.test.cjs +++ b/tests/reviewer-docs-parity.test.cjs @@ -722,10 +722,14 @@ describe('reviewer docs parity — the shipped repo', () => { }); describe('review-lane flags — emitted shape', () => { - 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 TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); - const runFlags = (args = []) => - cp.spawnSync(process.execPath, [TOOLS, 'review-lane', 'flags', ...args], { encoding: 'utf8' }); + const runFlags = (args = []) => { + const r = runNode([TOOLS, 'review-lane', 'flags', ...args], { timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); + }; test('emitsEveryDeclaredFlagInDescriptorOrder', () => { const r = runFlags(); @@ -789,7 +793,7 @@ describe('review-lane flags — emitted shape', () => { }); test('anUnknownSubcommandErrorsWithoutAStackTrace', () => { - const r = cp.spawnSync(process.execPath, [TOOLS, 'review-lane', 'bogus'], { encoding: 'utf8' }); + const r = runNode([TOOLS, 'review-lane', 'bogus'], { timeoutMs: PROBE_TIMEOUT_MS }); const combined = `${r.stdout || ''}${r.stderr || ''}`; assert.match(combined, /flags/); assert.ok(!combined.includes('at Object.')); diff --git a/tests/skill-frontmatter-contract.test.cjs b/tests/skill-frontmatter-contract.test.cjs index 5a2f32685..7802b9a77 100644 --- a/tests/skill-frontmatter-contract.test.cjs +++ b/tests/skill-frontmatter-contract.test.cjs @@ -244,9 +244,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 os = require('node:os'); const { cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const COMMANDS_DIR = path.join(__dirname, '../commands/gsd'); const LINT_SCRIPT = path.join(__dirname, '../scripts/lint-descriptions.cjs'); @@ -380,11 +381,11 @@ describe('lint-descriptions.cjs', () => { const tmpFile = path.join(tmpDir, 'long-desc.md'); fs.writeFileSync(tmpFile, content, 'utf-8'); - const result = spawnSync(process.execPath, [LINT_SCRIPT, tmpFile], { - encoding: 'utf-8', + const result = runNode([LINT_SCRIPT, tmpFile], { + timeoutMs: PROBE_TIMEOUT_MS, }); - assert.notStrictEqual(result.status, 0, [ + assert.notStrictEqual(result.exitCode, 0, [ 'lint-descriptions.cjs should exit non-zero for description > 100 chars', 'stdout: ' + result.stdout, 'stderr: ' + result.stderr, @@ -405,11 +406,11 @@ describe('lint-descriptions.cjs', () => { const tmpFile = path.join(tmpDir, 'short-desc.md'); fs.writeFileSync(tmpFile, content, 'utf-8'); - const result = spawnSync(process.execPath, [LINT_SCRIPT, tmpFile], { - encoding: 'utf-8', + const result = runNode([LINT_SCRIPT, tmpFile], { + timeoutMs: PROBE_TIMEOUT_MS, }); - assert.strictEqual(result.status, 0, [ + assert.strictEqual(result.exitCode, 0, [ 'lint-descriptions.cjs should exit 0 for description <= 100 chars', 'stdout: ' + result.stdout, 'stderr: ' + result.stderr, diff --git a/tests/slash-command-namespace.test.cjs b/tests/slash-command-namespace.test.cjs index acedc6b30..8a200275d 100644 --- a/tests/slash-command-namespace.test.cjs +++ b/tests/slash-command-namespace.test.cjs @@ -501,8 +501,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 { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const INSTALL_PATH = path.join(REPO_ROOT, 'bin', 'install.js'); @@ -521,12 +523,12 @@ const { readCmdNames } = require(path.join(REPO_ROOT, 'scripts', 'fix-slash-comm function runClaudeLocalInstall(cwd) { const env = { ...process.env }; delete env.GSD_TEST_MODE; - execFileSync(process.execPath, [INSTALL_PATH, '--claude', '--local', '--no-sdk'], { + const r = runNode([INSTALL_PATH, '--claude', '--local', '--no-sdk'], { cwd, - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], env, + timeoutMs: INSTALL_TIMEOUT_MS, }); + throwIfFailed(r, `node ${INSTALL_PATH} --claude --local --no-sdk`); } /** @@ -750,8 +752,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 { throwIfFailed } = require('./helpers/git-fixture.cjs'); +const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const INSTALL_PATH = path.join(REPO_ROOT, 'bin', 'install.js'); @@ -770,12 +774,12 @@ const { readCmdNames } = require(path.join(REPO_ROOT, 'scripts', 'fix-slash-comm function runClaudeLocalInstall(cwd) { const env = { ...process.env }; delete env.GSD_TEST_MODE; - execFileSync(process.execPath, [INSTALL_PATH, '--claude', '--local', '--no-sdk'], { + const r = runNode([INSTALL_PATH, '--claude', '--local', '--no-sdk'], { cwd, - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], env, + timeoutMs: INSTALL_TIMEOUT_MS, }); + throwIfFailed(r, `node ${INSTALL_PATH} --claude --local --no-sdk`); } /** diff --git a/tests/tsconfig-noemit.test.cjs b/tests/tsconfig-noemit.test.cjs index 99bc0790d..cebe4ebe8 100644 --- a/tests/tsconfig-noemit.test.cjs +++ b/tests/tsconfig-noemit.test.cjs @@ -3,19 +3,25 @@ const { test } = require('node:test'); const assert = require('node:assert/strict'); const path = require('node:path'); -const { spawnSync } = require('node:child_process'); +const { runNode } = require('./helpers/process-seam.cjs'); + +// 180000: this is a real `tsc --noEmit` compile of the whole project, not a +// short probe or the hooks-only bundle `BUILD_TIMEOUT_MS` (30000) norm +// covers — matches the documented real-compile precedent at +// tests/ensure-runtime-build.test.cjs:35. +const TSC_NOEMIT_TIMEOUT_MS = 180000; test('root tsconfig supports the default no-emit typecheck command', () => { const root = path.join(__dirname, '..'); const tscBin = path.join(root, 'node_modules', 'typescript', 'bin', 'tsc'); - const result = spawnSync(process.execPath, [tscBin, '--noEmit'], { + const result = runNode([tscBin, '--noEmit'], { cwd: root, - encoding: 'utf8', + timeoutMs: TSC_NOEMIT_TIMEOUT_MS, }); assert.equal( - result.status, + result.exitCode, 0, [ 'Expected the default root TypeScript typecheck to pass.', diff --git a/tests/validate-registry.test.cjs b/tests/validate-registry.test.cjs index 457c82a07..558a65a9c 100644 --- a/tests/validate-registry.test.cjs +++ b/tests/validate-registry.test.cjs @@ -6,8 +6,10 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -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 { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const SCRIPT_PATH = path.join(__dirname, '..', 'scripts', 'validate-registry.cjs'); @@ -95,7 +97,8 @@ function withReviewerFixture(capabilityEntries, reviewerEntries, fn) { } function runValidate(cwd, args = []) { - return spawnSync(process.execPath, [SCRIPT_PATH, ...args], { cwd, encoding: 'utf8' }); + const r = runNode([SCRIPT_PATH, ...args], { cwd, timeoutMs: PROBE_TIMEOUT_MS }); + return toLegacyResult(r); } describe('validate-registry CLI (subprocess)', () => {