golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

test(#4512): migrate process-seam batch to named timeout constants

Browse Source 
Batch 1 of the ad hoc timeout literal migration (epic #4445). Replaces
every bare numeric timeout/timeoutMs object-literal property in
tests/process-seam.test.cjs, tests/helpers-process-isolation.test.cjs,
tests/run-with-timeout.test.cjs, and tests/helpers.cjs with a named
constant, per eslint-rules/no-adhoc-timeout-literal.cjs. Removes the
4 files from the rule's allowlist.

Adds SEAM_DEFAULT_TIMEOUT_MS to tests/helpers/timeouts.cjs (shared
across 2 batch files, mirroring process-seam.cjs's own un-exported
default). File-local constants elsewhere for values not shared across
files or not a bench-derived class norm. No src/bin file touched, no
numeric value changed anywhere.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
sim
2026-09-08 14:03:22 -04:00
committed by Tom Boucher
parent c9016b2e35
commit ff5c5782ee
6 changed files with 112 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
eslint-rules/no-adhoc-timeout-literal.allowlist.json
View File

@@ -56,8 +56,6 @@
"tests/gsd-validate-commit-crash-policy.test.cjs",
"tests/gsd-write-guard.test.cjs",
"tests/health-validation.test.cjs",
"tests/helpers-process-isolation.test.cjs",
"tests/helpers.cjs",
"tests/hooks-commonjs-marker.test.cjs",
"tests/hooks-crash-policy.test.cjs",
"tests/init.test.cjs",
@@ -93,7 +91,6 @@
"tests/plugin-manifest.test.cjs",
"tests/policy-160-route0-resume.test.cjs",
"tests/pr-branch-planning-filter.test.cjs",
"tests/process-seam.test.cjs",
"tests/prohibition-enforcement.test.cjs",
"tests/prompt-injection-scan.security.test.cjs",
"tests/qa/tdd-walk.cjs",
@@ -108,7 +105,6 @@
"tests/reviewer-manifest-body.test.cjs",
"tests/reviewer-trust-disclosure.test.cjs",
"tests/run-tests-temp-root.test.cjs",
"tests/run-with-timeout.test.cjs",
"tests/secret-scan-lint.security.test.cjs",
"tests/security-prompt-injection.security.test.cjs",
"tests/security-scan.security.test.cjs",

33
tests/helpers-process-isolation.test.cjs
View File

@@ -9,6 +9,18 @@ const {
CONFIG_LOCATION_ENV_KEYS,
scrubConfigLocationEnv,
} = require('./helpers.cjs');
const { INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
/**
* A cold `node -e` require-probe in this file (not the shared
* `PROBE_TIMEOUT_MS` class — that constant is 15000ms and consolidating
* onto it would silently halve this file's pre-existing bound, a real
* behavior change a rename-only migration must not make). The comment at
* each use site already documents the observed duration as sub-second, so
* 30000ms leaves ~30x headroom. Pre-existing value, unchanged by this
* migration.
*/
const COLD_REQUIRE_PROBE_TIMEOUT_MS = 30000;
describe('#2665: the built-lib require is deferred', () => {
// The scrub set derives from gsd-core/bin/lib, which is BUILT. Requiring it at
@@ -24,9 +36,9 @@ describe('#2665: the built-lib require is deferred', () => {
"const needle = path.join('gsd-core', 'bin', 'lib', 'capability-registry.cjs');",
'process.stdout.write(String(Object.keys(require.cache).some((m) => m.endsWith(needle))));',
].join('\n');
// Bounded per local/no-unbounded-spawn (#3143): a cold require is sub-second,
// so 30s is ~30x headroom and still fails loudly instead of hanging a lane.
const r = spawnSync(process.execPath, ['-e', src], { encoding: 'utf8', timeout: 30_000 });
// Bounded per local/no-unbounded-spawn (#3143): a cold require is sub-second, so
// COLD_REQUIRE_PROBE_TIMEOUT_MS leaves ~30x headroom and fails loudly instead of hanging a lane.
const r = spawnSync(process.execPath, ['-e', src], { encoding: 'utf8', timeout: COLD_REQUIRE_PROBE_TIMEOUT_MS });
assert.strictEqual(r.status, 0, `probe failed: ${r.stderr}`);
return r.stdout === 'true';
};
@@ -339,7 +351,8 @@ describe('#2665 round 4: the skillsHome walk is reversion-sensitive', () => {
cwd: __dirname,
encoding: 'utf-8',
stdio: ['pipe', 'pipe', 'pipe'],
timeout: 30_000,
// Same class as the probe above (#2665) — see COLD_REQUIRE_PROBE_TIMEOUT_MS's own comment.
timeout: COLD_REQUIRE_PROBE_TIMEOUT_MS,
});
void out; // exit 0 is the assertion; execFileSync throws on nonzero
});
@@ -406,7 +419,7 @@ describe('#3156: a raw installer spawn cannot write into the ambient HOME', () =
encoding: 'utf-8',
stdio: ['pipe', 'pipe', 'pipe'],
env: installerEnv(),
timeout: 120_000,
timeout: INSTALL_TIMEOUT_MS,
});
assert.ok(!fs.existsSync(path.join(canaryHome, '.gsd')),
@@ -432,6 +445,14 @@ describe('#3712: a raw installer spawn cannot reach the ambient HOME\'s shared s
const { cleanup } = require('./helpers.cjs');
const { installerEnv } = require('./helpers/install-shared.cjs');
const INSTALL_PATH = path.join(__dirname, '..', 'bin', 'install.js');
/**
* A GLOBAL codex install additionally walks and copies the shared skills
* tree across the ambient-HOME boundary (#3712) on top of a normal
* install run, so it keeps a larger cap than INSTALL_TIMEOUT_MS. No fresh
* bench measurement justifies a new number here; the existing multiple is
* preserved rather than re-guessed on migration.
*/
const INSTALL_GLOBAL_TIMEOUT_MS = INSTALL_TIMEOUT_MS * 2.5;
// #3156's canary asserts on <canaryHome>/.gsd. That is not the only ambient-HOME
// surface: a kind may declare a global `home` override resolved from os.homedir()
@@ -480,7 +501,7 @@ describe('#3712: a raw installer spawn cannot reach the ambient HOME\'s shared s
encoding: 'utf-8',
stdio: ['pipe', 'pipe', 'pipe'],
env: installerEnv(),
timeout: 300_000,
timeout: INSTALL_GLOBAL_TIMEOUT_MS,
});
assert.strictEqual(inventory(), before,

3
tests/helpers.cjs
View File

@@ -8,6 +8,7 @@ const os = require('os');
const path = require('path');
const { createFixture } = require('./fixtures/index.cjs');
const processSeam = require('./helpers/process-seam.cjs');
const { SEAM_DEFAULT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs');
@@ -226,7 +227,7 @@ function runGsdTools(args, cwd = process.cwd(), env = {}) {
return processSeam.runNode([TOOLS_PATH, ...argv], {
cwd,
env: childEnv,
timeoutMs: 60000,
timeoutMs: SEAM_DEFAULT_TIMEOUT_MS,
});
}

25
tests/helpers/timeouts.cjs
View File

@@ -19,6 +19,12 @@
* tests/fragment-single-edit-propagation.install.test.cjs) keeps its own
* local constant with its own justifying comment — do not force those
* sites onto a shared value that doesn't describe them.
*
* One exception to "bench-derived class norm": `SEAM_DEFAULT_TIMEOUT_MS`
* below is a structural mirror of another module's un-exported default,
* not an independently bench-measured bound — see its own doc comment for
* why it still belongs here (shared by ≥2 call sites) rather than as a
* local constant.
*/
const { DEFAULT_GIT_TIMEOUT_MS, GIT_FIXTURE_TIMEOUT_MS } = require('./git-fixture.cjs');
@@ -86,6 +92,24 @@ const BUILD_TIMEOUT_MS = 30000;
*/
const INSTALL_TIMEOUT_MS = 120000;
/**
* Mirrors `tests/helpers/process-seam.cjs`'s own internal fallback for an
* omitted `timeoutMs` (#4512, batch 1 of the ad hoc timeout literal
* migration, epic #4445). That module does not export its fallback —
* exporting it is out of this batch's scope, since `process-seam.cjs`
* itself is not one of the batch's files — so this constant restates the
* SAME pre-existing number under a name, purely so call sites asserting
* parity with the seam's own fallback (rather than a measured class norm)
* have something to import instead of a bare literal.
*
* Deliberately a separate name from `HOOK_FANOUT_TIMEOUT_MS`, even though
* the two currently coincide: that constant is a bench-justified bound for
* a heavier, specific subprocess shape (nested shell fan-out), while this
* one is only "no unbounded path" — collapsing them would let a future,
* independent tune of either value silently move the other.
*/
const SEAM_DEFAULT_TIMEOUT_MS = 60000;
module.exports = {
PROBE_TIMEOUT_MS,
HOOK_FANOUT_TIMEOUT_MS,
@@ -93,4 +117,5 @@ module.exports = {
GIT_FIXTURE_TIMEOUT_MS,
BUILD_TIMEOUT_MS,
INSTALL_TIMEOUT_MS,
SEAM_DEFAULT_TIMEOUT_MS,
};

53
tests/process-seam.test.cjs
View File

@@ -24,6 +24,29 @@ const path = require('node:path');
const { createTempDir, cleanup, runGsdTools, TOOLS_PATH } = require('./helpers.cjs');
const processSeam = require('./helpers/process-seam.cjs');
const { runNode, runGit, runHook, OUTCOME, toSeamResult } = processSeam;
const {
SEAM_DEFAULT_TIMEOUT_MS,
} = require('./helpers/timeouts.cjs');
/**
* `FIXTURE_SLEEPER` in this file sleeps 5000ms by default. Both constants
* below are comfortably under that, chosen only to reliably fire TIMED_OUT
* with margin — the exact numbers are not independently meaningful beyond
* "far under 5000ms", and are kept as two distinct pre-existing values (not
* consolidated to one) so this migration does not silently change any
* test's timing behavior. `SEAM_TIGHT_TIMEOUT_MS` (200) is the SMALLER /
* tighter of the two; `SEAM_SHORT_TIMEOUT_MS` (300) is the LARGER of the
* two — the names alone don't convey that ordering, hence spelling it out
* here.
*/
const SEAM_TIGHT_TIMEOUT_MS = 200;
const SEAM_SHORT_TIMEOUT_MS = 300;
/**
* Generous headroom for a fixture in this file that exits quickly or
* synchronously (not exercising the timeout boundary itself).
*/
const SEAM_GENEROUS_TIMEOUT_MS = 5000;
// ---- fixture sources -------------------------------------------------
@@ -155,7 +178,7 @@ describe('process-seam', () => {
test('a child that overruns is TIMED_OUT as data, not a throw', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '5000'], { timeoutMs: 300 });
const result = runNode([fixture, '5000'], { timeoutMs: SEAM_SHORT_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.TIMED_OUT);
assert.equal(result.timedOut, true);
assert.equal(result.killed, true);
@@ -164,7 +187,7 @@ describe('process-seam', () => {
test('timedOut does not depend on signal presence (Windows)', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '5000'], { timeoutMs: 300 });
const result = runNode([fixture, '5000'], { timeoutMs: SEAM_SHORT_TIMEOUT_MS });
// The assertion below is intentionally the whole point of this test: it
// proves timedOut alone, without ever branching on result.signal. See
// 40-design.md row 6 — signal is null on Windows and must not be
@@ -175,7 +198,7 @@ describe('process-seam', () => {
test('a timeout still returns string stdout/stderr (partial content is platform-dependent)', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const marker = JSON.stringify({ partial: true });
const result = runNode([fixture, '5000', marker], { timeoutMs: 300 });
const result = runNode([fixture, '5000', marker], { timeoutMs: SEAM_SHORT_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.TIMED_OUT);
assert.equal(result.timedOut, true);
assert.equal(typeof result.stdout, 'string');
@@ -230,7 +253,7 @@ describe('process-seam', () => {
test('omitting timeoutMs still bounds the call', () => {
const fixture = writeFixture(tmpDir, 'exit.cjs', FIXTURE_EXIT);
const withDefault = runNode([fixture, '0']);
const withExplicitDefault = runNode([fixture, '0'], { timeoutMs: 60000 });
const withExplicitDefault = runNode([fixture, '0'], { timeoutMs: SEAM_DEFAULT_TIMEOUT_MS });
// Omitting timeoutMs must resolve to the same bounded code path as
// explicitly passing the documented default — never a distinct
// "unbounded" branch.
@@ -242,14 +265,14 @@ describe('process-seam', () => {
test('child finishing just under the bound is EXITED', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '50'], { timeoutMs: 5000 });
const result = runNode([fixture, '50'], { timeoutMs: SEAM_GENEROUS_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.EXITED);
assert.equal(result.timedOut, false);
});
test('at-the-bound child yields one deterministic outcome', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '300'], { timeoutMs: 300 });
const result = runNode([fixture, '300'], { timeoutMs: SEAM_SHORT_TIMEOUT_MS });
// Either outcome is acceptable at the exact bound (OS/scheduler
// jitter decides which side of the race wins) — what must never happen
// is an outcome outside the pair, or fields inconsistent with whichever
@@ -284,7 +307,7 @@ describe('process-seam', () => {
test('child overrunning the bound is TIMED_OUT', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '5000'], { timeoutMs: 200 });
const result = runNode([fixture, '5000'], { timeoutMs: SEAM_TIGHT_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.TIMED_OUT);
});
@@ -346,7 +369,7 @@ describe('process-seam', () => {
test('a custom killSignal is reported, not normalized', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '5000'], { timeoutMs: 200, killSignal: 'SIGINT' });
const result = runNode([fixture, '5000'], { timeoutMs: SEAM_TIGHT_TIMEOUT_MS, killSignal: 'SIGINT' });
assert.equal(result.outcome, OUTCOME.TIMED_OUT);
if (process.platform !== 'win32') {
assert.equal(result.signal, 'SIGINT');
@@ -355,7 +378,7 @@ describe('process-seam', () => {
test('timeout with stderr reports one outcome, keeps both fields', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const result = runNode([fixture, '5000', '', 'err-marker'], { timeoutMs: 300 });
const result = runNode([fixture, '5000', '', 'err-marker'], { timeoutMs: SEAM_SHORT_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.TIMED_OUT);
assert.equal(typeof result.stdout, 'string');
assert.equal(typeof result.stderr, 'string');
@@ -377,8 +400,8 @@ describe('process-seam', () => {
test('consecutive timeouts do not share state', () => {
const fixture = writeFixture(tmpDir, 'sleeper.cjs', FIXTURE_SLEEPER);
const first = runNode([fixture, '5000'], { timeoutMs: 200 });
const second = runNode([fixture, '5000'], { timeoutMs: 200 });
const first = runNode([fixture, '5000'], { timeoutMs: SEAM_TIGHT_TIMEOUT_MS });
const second = runNode([fixture, '5000'], { timeoutMs: SEAM_TIGHT_TIMEOUT_MS });
assert.equal(first.outcome, OUTCOME.TIMED_OUT);
assert.equal(second.outcome, OUTCOME.TIMED_OUT);
});
@@ -410,7 +433,7 @@ describe('process-seam', () => {
return;
}
const fixture = writeFixture(tmpDir, 'suicide.cjs', FIXTURE_SUICIDE);
const result = runNode([fixture], { timeoutMs: 5000 });
const result = runNode([fixture], { timeoutMs: SEAM_GENEROUS_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.KILLED);
assert.equal(result.killed, true);
assert.equal(result.timedOut, false);
@@ -432,7 +455,7 @@ describe('process-seam', () => {
CLAUDE_CODE_SSE_PORT: '',
CLAUDE_PROJECT_DIR: '',
};
const result = runHook(hookPath, [], { input: payload, env, timeoutMs: 5000 });
const result = runHook(hookPath, [], { input: payload, env, timeoutMs: SEAM_GENEROUS_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.EXITED);
assert.equal(typeof result.stdout, 'string');
});
@@ -492,7 +515,7 @@ describe('runHook interpreter option', () => {
// unexpected string-valued option alongside a valid timeoutMs; the
// seam's contract-validation for timeoutMs must still pass through
// untouched and the call must complete without throwing.
const result = runHook(hookPath, ['0'], { interpreter: process.execPath, timeoutMs: 5000 });
const result = runHook(hookPath, ['0'], { interpreter: process.execPath, timeoutMs: SEAM_GENEROUS_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.EXITED);
assert.equal(result.exitCode, 0);
});
@@ -510,7 +533,7 @@ describe('runHook interpreter option', () => {
'sleep 5',
].join('\n')
);
const result = runHook(scriptPath, [], { interpreter: 'bash', timeoutMs: 300 });
const result = runHook(scriptPath, [], { interpreter: 'bash', timeoutMs: SEAM_SHORT_TIMEOUT_MS });
assert.equal(result.outcome, OUTCOME.TIMED_OUT);
assert.equal(result.timedOut, true);
assert.equal(result.exitCode, null);

23
tests/run-with-timeout.test.cjs
View File

@@ -25,6 +25,23 @@ const fs = require('node:fs');
const { setTimeout: sleep } = require('node:timers/promises');
const { createTempDir, cleanup } = require('./helpers.cjs');
/**
* Backstop for spawnSync's OWN hard-kill, guarding against run-with-timeout's
* internal wall-clock cap failing to fire. Not a measured probe duration —
* this suite's verb-internal budgets top out at 10s (Windows .cmd/.bat
* mediation tests), so this preserves ~3x headroom over that ceiling, the
* pre-existing value unchanged by this migration.
*/
const RUN_WITH_TIMEOUT_HARNESS_BACKSTOP_MS = 30000;
/**
* Tighter backstop for the C1 reap test specifically: its own verb-internal
* budget is 3s plus ~900ms of heartbeat-settle waits, so this keeps a
* smaller margin than the file default while still comfortably covering it.
* Pre-existing value, unchanged.
*/
const RUN_WITH_TIMEOUT_C1_REAP_BACKSTOP_MS = 20000;
const ROOT = path.join(__dirname, '..');
const GSD_TOOLS = path.join(ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs');
const NODE = process.execPath;
@@ -35,7 +52,7 @@ function runVerb(args, opts = {}) {
return spawnSync(NODE, [GSD_TOOLS, 'run-with-timeout', ...args], {
cwd: os.tmpdir(),
encoding: 'utf8',
timeout: 30000, // test-harness backstop; the verb's own cap is what we assert
timeout: RUN_WITH_TIMEOUT_HARNESS_BACKSTOP_MS, // test-harness backstop; the verb's own cap is what we assert
...opts,
});
}
@@ -139,7 +156,7 @@ describe('#2351 run-with-timeout — argument handling (negative matrix)', () =>
test('the `query` meta-prefix form is accepted', () => {
const r = spawnSync(NODE, [GSD_TOOLS, 'query', 'run-with-timeout', '5', '--', ...OK], {
cwd: os.tmpdir(), encoding: 'utf8', timeout: 30000,
cwd: os.tmpdir(), encoding: 'utf8', timeout: RUN_WITH_TIMEOUT_HARNESS_BACKSTOP_MS,
});
assert.equal(r.status, 0);
});
@@ -183,7 +200,7 @@ describe('#2351 run-with-timeout — kill semantics (POSIX process groups)', ()
'process.on("SIGTERM", () => process.exit(0));',
'setInterval(() => {}, 1000);',
].join('\n'));
const r = runVerb(['3', '--', NODE, parentFile, hbFile], { timeout: 20000 });
const r = runVerb(['3', '--', NODE, parentFile, hbFile], { timeout: RUN_WITH_TIMEOUT_C1_REAP_BACKSTOP_MS });
assert.equal(r.status, 124, 'must report a timeout (124), not hang');
assert.ok(fs.existsSync(hbFile), 'child heartbeat should exist');
await sleep(300); // let any in-flight write settle after the SIGKILL