diff --git a/eslint-rules/no-adhoc-timeout-literal.allowlist.json b/eslint-rules/no-adhoc-timeout-literal.allowlist.json index 7ae12bc80..92636865c 100644 --- a/eslint-rules/no-adhoc-timeout-literal.allowlist.json +++ b/eslint-rules/no-adhoc-timeout-literal.allowlist.json @@ -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", diff --git a/tests/feat-2483-review-claude-mds-guard.test.cjs b/tests/feat-2483-review-claude-mds-guard.test.cjs index 38f5d94a8..aa26ffdfb 100644 --- a/tests/feat-2483-review-claude-mds-guard.test.cjs +++ b/tests/feat-2483-review-claude-mds-guard.test.cjs @@ -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' }, }, }; diff --git a/tests/gsd-secret-read-guard.test.cjs b/tests/gsd-secret-read-guard.test.cjs index 73b3589f7..1d8a3cbb4 100644 --- a/tests/gsd-secret-read-guard.test.cjs +++ b/tests/gsd-secret-read-guard.test.cjs @@ -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 }; } diff --git a/tests/gsd-write-guard.test.cjs b/tests/gsd-write-guard.test.cjs index c555d26e8..4979b45a5 100644 --- a/tests/gsd-write-guard.test.cjs +++ b/tests/gsd-write-guard.test.cjs @@ -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 }; } diff --git a/tests/hooks-crash-policy.test.cjs b/tests/hooks-crash-policy.test.cjs index dcde48122..a7f91a67a 100644 --- a/tests/hooks-crash-policy.test.cjs +++ b/tests/hooks-crash-policy.test.cjs @@ -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}`); }); diff --git a/tests/read-guard.test.cjs b/tests/read-guard.test.cjs index 07a5108e4..58f0dd2aa 100644 --- a/tests/read-guard.test.cjs +++ b/tests/read-guard.test.cjs @@ -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, });