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

test(#4517): migrate hook/guard invocation batch to named timeout constants (#4608)

Browse Source 
Batch 6 of 17 in the ad hoc timeout literal migration (epic #4445).
Replaces every bare numeric timeout/timeoutMs object-literal property in
tests/read-guard.test.cjs, tests/feat-2483-review-claude-mds-guard.test.cjs,
tests/gsd-write-guard.test.cjs, tests/gsd-secret-read-guard.test.cjs, and
tests/hooks-crash-policy.test.cjs with a named constant, per
eslint-rules/no-adhoc-timeout-literal.cjs. Removes the 5 files from the
rule's allowlist.

Reuses existing tests/helpers/timeouts.cjs class norms where the shape
matches (QUICK_SPAWN_TIMEOUT_MS x2, PROBE_TIMEOUT_MS x1). Adds two
file-local constants for classes not shared across files:
READ_GUARD_HOOK_TIMEOUT_MS (read-guard.test.cjs's 4 sites, a tighter
no-fan-out bound than QUICK_SPAWN_TIMEOUT_MS with no bench data to widen
it) and FIXTURE_PROBE_CAPABILITY_TIMEOUT_MS (fixture data, not a real
spawn timeout).

The fifth site (feat-2483-review-claude-mds-guard.test.cjs's review-lane
invoke spawn) was initially classified onto HOOK_FANOUT_TIMEOUT_MS;
isolated Spec-axis review caught that this call is one nested spawn, not
the multi-spawn git-hook fan-out shape that constant's own doc comment
defines. Corrected to a new file-local REVIEW_LANE_INVOKE_TIMEOUT_MS
holding the exact pre-existing 60000ms value under an honest name,
rather than narrowing to PROBE_TIMEOUT_MS with no bench justification.

No src/bin file touched, no numeric value changed anywhere.

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-10 15:36:43 -04:00
committed by GitHub
parent 93ba63aeff
commit 138e70d734
6 changed files with 60 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -29,7 +29,6 @@
"tests/emitted-ack-trailer.test.cjs",
"tests/emitted-attribution.test.cjs",
"tests/execute-wave-post-gate-pipeline-e2e.test.cjs",
"tests/feat-2483-review-claude-mds-guard.test.cjs",
"tests/federated-config.test.cjs",
"tests/gate-predicate-evaluator.test.cjs",
"tests/gen-context-index.test.cjs",
@@ -39,11 +38,8 @@
"tests/graphify.test.cjs",
"tests/gsd-check-update-worker-platform-gate.test.cjs",
"tests/gsd-mcp-server-bin.test.cjs",
"tests/gsd-secret-read-guard.test.cjs",
"tests/gsd-statusline.test.cjs",
"tests/gsd-write-guard.test.cjs",
"tests/health-validation.test.cjs",
"tests/hooks-crash-policy.test.cjs",
"tests/init.test.cjs",
"tests/lint-docs-command-form.test.cjs",
"tests/locking-bugs-1909-1916-1925-1927.test.cjs",
@@ -68,7 +64,6 @@
"tests/prompt-injection-scan.security.test.cjs",
"tests/qa/tdd-walk.cjs",
"tests/quick-batch.test.cjs",
"tests/read-guard.test.cjs",
"tests/read-injection-scanner.property.test.cjs",
"tests/read-injection-scanner.security.test.cjs",
"tests/representative-corpus.test.cjs",

30
tests/feat-2483-review-claude-mds-guard.test.cjs
View File

@@ -43,6 +43,32 @@ const { cleanup } = require('./helpers.cjs');
const REPO_ROOT = path.join(__dirname, '..');
const TOOLS = path.join(REPO_ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs');
/**
* NOT a subprocess spawn timeout. This is fixture DATA standing in for a
* capability manifest's own declared `probe.timeoutMs` config field, passed
* only into the pure `discloseExecutableSurfaces` function under test — never
* a live spawn timeout, never pass this into a real subprocess options object.
*/
const FIXTURE_PROBE_CAPABILITY_TIMEOUT_MS = 1000;
/**
* `gsd-tools.cjs review-lane invoke` spawned directly, which itself spawns
* the reviewer's `invoke.binary` exactly ONCE (here, the shimmed `claude`
* bash script, which does no further spawning of its own — just file I/O
* and echo). This is ONE level of nested spawn, not the multi-spawn
* git-hook-under-bash shape `HOOK_FANOUT_TIMEOUT_MS`'s own doc comment
* defines ("roughly four Git Bash spawns") — that constant does not
* describe this call, despite the coincidentally-matching pre-existing
* value. Heavier than a bare `PROBE_TIMEOUT_MS` site, though: the outer
* `gsd-tools.cjs` process itself does real work beyond a version-string
* read-back (resolves the lane plan, spawns the reviewer binary, waits on
* its output, parses JSON) before the nested spawn even starts. No bench
* data exists to justify a different number, so the pre-existing 60000ms
* literal is preserved exactly under its own file-local name rather than
* folded into either shared class it does not truly match.
*/
const REVIEW_LANE_INVOKE_TIMEOUT_MS = 60000;
const GUARD = Object.freeze({
CLAUDE_CODE_DISABLE_CLAUDE_MDS: '1',
CLAUDE_CODE_DISABLE_AUTO_MEMORY: '1',
@@ -174,7 +200,7 @@ describe('#2483 the claude reviewer lane suppresses CLAUDE.md + auto-memory inje
'--repo-root', REPO_ROOT, '--json'],
{
encoding: 'utf8',
timeout: 60_000,
timeout: REVIEW_LANE_INVOKE_TIMEOUT_MS,
killSignal: 'SIGKILL',
env: {
...process.env,
@@ -373,7 +399,7 @@ describe('#2483 the claude reviewer lane suppresses CLAUDE.md + auto-memory inje
slug: 'probe-reviewer',
transport: 'spawn',
handler: null,
probe: { kind: 'command-capability', binary: '/tmp/evil-probe', needle: 'x', timeoutMs: 1000 },
probe: { kind: 'command-capability', binary: '/tmp/evil-probe', needle: 'x', timeoutMs: FIXTURE_PROBE_CAPABILITY_TIMEOUT_MS },
invoke: { binary: 'node', args: ['--version'], promptChannel: 'stdin' },
},
};

3
tests/gsd-secret-read-guard.test.cjs
View File

@@ -32,6 +32,7 @@ const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const path = require('node:path');
const { runHook: runHookSeam } = require('./helpers/process-seam.cjs');
const { QUICK_SPAWN_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-secret-read-guard.js');
@@ -39,7 +40,7 @@ function runHook(payload) {
const r = runHookSeam(HOOK_PATH, [], {
input: typeof payload === 'string' ? payload : JSON.stringify(payload),
env: { ...process.env },
timeoutMs: 10_000,
timeoutMs: QUICK_SPAWN_TIMEOUT_MS,
});
return { status: r.exitCode, stdout: r.stdout, stderr: r.stderr };
}

11
tests/gsd-write-guard.test.cjs
View File

@@ -29,6 +29,7 @@ const os = require('node:os');
const path = require('node:path');
const { createTempDir, cleanup } = require('./helpers.cjs');
const { runHook: runHookSeam } = require('./helpers/process-seam.cjs');
const { QUICK_SPAWN_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-write-guard.js');
@@ -40,10 +41,10 @@ const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-write-guard.js');
* Returns an object shaped like the raw spawnSync() result (status/stdout/
* stderr) because every call site in this file was written against that
* shape; the seam itself returns exitCode, not status, so it is mapped here.
* 10_000ms: gsd-write-guard.js does no subprocess work of its own (pure
* fs reads + JSON, no execFileSync/spawnSync inside the hook) — generous
* headroom over the fs-bound workload without matching the 30_000ms figure
* sibling suites use for guards that shell out to git.
* QUICK_SPAWN_TIMEOUT_MS: gsd-write-guard.js does no subprocess work of its
* own (pure fs reads + JSON, no execFileSync/spawnSync inside the hook) —
* generous headroom over the fs-bound workload without matching the
* 30_000ms figure sibling suites use for guards that shell out to git.
*/
function runHook(payload, extraEnv = {}) {
const env = { ...process.env };
@@ -52,7 +53,7 @@ function runHook(payload, extraEnv = {}) {
const r = runHookSeam(HOOK_PATH, [], {
input: typeof payload === 'string' ? payload : JSON.stringify(payload),
env,
timeoutMs: 10_000,
timeoutMs: QUICK_SPAWN_TIMEOUT_MS,
});
return { status: r.exitCode, stdout: r.stdout, stderr: r.stderr };
}

3
tests/hooks-crash-policy.test.cjs
View File

@@ -45,6 +45,7 @@ const os = require('node:os');
const { createTempDir, cleanup, TEST_ENV_BASE } = require('./helpers.cjs');
const { runHook: runHookSeam, runNode, OUTCOME } = require('./helpers/process-seam.cjs');
const { gitOrThrow, GIT_FIXTURE_TIMEOUT_MS } = require('./helpers/git-fixture.cjs');
const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const { ensureBuiltHooks } = require('../scripts/run-tests.cjs');
const HOOKS_DIR = path.join(__dirname, '..', 'hooks');
@@ -682,7 +683,7 @@ describe('hooks-crash-policy: hooks/dist/lib parity (#3911 review finding)', ()
`const { terminateNow } = require(${JSON.stringify(distCliExitPath)});`,
`terminateNow('HOOK_DENY', { x: 1 });`,
].join('\n');
const r = runNode(['-e', script], { timeoutMs: 15000 });
const r = runNode(['-e', script], { timeoutMs: PROBE_TIMEOUT_MS });
assert.equal(r.outcome, OUTCOME.EXITED, `expected a clean exit; got ${r.outcome} stderr=${r.stderr}`);
assert.equal(r.exitCode, 2, `expected HOOK_DENY's registered exit code 2; stdout=${r.stdout} stderr=${r.stderr}`);
});

26
tests/read-guard.test.cjs
View File

@@ -28,6 +28,12 @@ const { createTempDir, cleanup } = require('./helpers.cjs');
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-guard.js');
// Bounds a single direct invocation of the advisory-only, no-subprocess
// gsd-read-guard.js hook; tighter than the shared QUICK_SPAWN_TIMEOUT_MS norm
// (10000ms) despite a similar "no fan-out" class — kept as its own constant
// since this migration never widens a bound without a fresh bench citation.
const READ_GUARD_HOOK_TIMEOUT_MS = 5000;
/**
* Run the read guard hook with a given tool input payload.
* Returns { exitCode, stdout, stderr }.
@@ -45,7 +51,7 @@ function runHook(payload, envOverrides = {}) {
CLAUDE_PROJECT_DIR: '',
...envOverrides,
};
const r = runHookSeam(HOOK_PATH, [], { input, env, timeoutMs: 5000 });
const r = runHookSeam(HOOK_PATH, [], { input, env, timeoutMs: READ_GUARD_HOOK_TIMEOUT_MS });
if (r.exitCode === 0) {
return { exitCode: 0, stdout: r.stdout.trim(), stderr: '' };
}
@@ -152,7 +158,7 @@ describe('gsd-read-guard hook', () => {
const stdout = execFileSync(process.execPath, [HOOK_PATH], {
input: 'not json',
encoding: 'utf-8',
timeout: 5000,
timeout: READ_GUARD_HOOK_TIMEOUT_MS,
stdio: ['pipe', 'pipe', 'pipe'],
});
// Should exit 0 silently
@@ -284,6 +290,12 @@ const { createTempDir, cleanup } = require('./helpers.cjs');
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-guard.js');
// Bounds a single direct invocation of the advisory-only, no-subprocess
// gsd-read-guard.js hook; tighter than the shared QUICK_SPAWN_TIMEOUT_MS norm
// (10000ms) despite a similar "no fan-out" class — kept as its own constant
// since this migration never widens a bound without a fresh bench citation.
const READ_GUARD_HOOK_TIMEOUT_MS = 5000;
function runHook(payload, envOverrides = {}) {
const input = JSON.stringify(payload);
const env = {
@@ -295,7 +307,7 @@ function runHook(payload, envOverrides = {}) {
CLAUDE_PROJECT_DIR: '',
...envOverrides,
};
const r = runHookSeam(HOOK_PATH, [], { input, env, timeoutMs: 5000 });
const r = runHookSeam(HOOK_PATH, [], { input, env, timeoutMs: READ_GUARD_HOOK_TIMEOUT_MS });
if (r.exitCode === 0) {
return { exitCode: 0, stdout: r.stdout.trim(), stderr: '' };
}
@@ -394,6 +406,12 @@ const { createTempDir, cleanup } = require('./helpers.cjs');
const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-guard.js');
// Bounds a single direct invocation of the advisory-only, no-subprocess
// gsd-read-guard.js hook; tighter than the shared QUICK_SPAWN_TIMEOUT_MS norm
// (10000ms) despite a similar "no fan-out" class — kept as its own constant
// since this migration never widens a bound without a fresh bench citation.
const READ_GUARD_HOOK_TIMEOUT_MS = 5000;
/**
* Spawn the hook with an env that mirrors the actual Claude Code hook
* subprocess env: CLAUDECODE and CLAUDE_SESSION_ID are stripped, only
@@ -419,7 +437,7 @@ function runHookInClaudeCodeSubprocess(payload, envOverrides = {}) {
const stdout = execFileSync(process.execPath, [HOOK_PATH], {
input,
encoding: 'utf-8',
timeout: 5000,
timeout: READ_GUARD_HOOK_TIMEOUT_MS,
stdio: ['pipe', 'pipe', 'pipe'],
env,
});