diff --git a/eslint-rules/no-adhoc-timeout-literal.allowlist.json b/eslint-rules/no-adhoc-timeout-literal.allowlist.json index 1a29ae3f4..485e13b5a 100644 --- a/eslint-rules/no-adhoc-timeout-literal.allowlist.json +++ b/eslint-rules/no-adhoc-timeout-literal.allowlist.json @@ -1,17 +1,12 @@ [ "tests/assumption-delta-checkpoint-e2e.test.cjs", "tests/assumption-delta.test.cjs", - "tests/check-predicate.test.cjs", "tests/cjs-command-router-adapter.test.cjs", "tests/code-review-pipeline-regression.test.cjs", "tests/code-review.test.cjs", "tests/dispatcher.test.cjs", - "tests/gate-predicate-evaluator.test.cjs", - "tests/phase6-capstone-conformance.test.cjs", "tests/plan-phase-stall-detection.test.cjs", "tests/plan-review-convergence.test.cjs", - "tests/policy-160-route0-resume.test.cjs", - "tests/prohibition-enforcement.test.cjs", "tests/review-lane-invocation.test.cjs", "tests/reviewer-manifest-body.test.cjs", "tests/reviewer-trust-disclosure.test.cjs", diff --git a/tests/check-predicate.test.cjs b/tests/check-predicate.test.cjs index c890e7bbd..d72d30828 100644 --- a/tests/check-predicate.test.cjs +++ b/tests/check-predicate.test.cjs @@ -23,6 +23,21 @@ const { evaluatePredicate } = require('../gsd-core/bin/lib/gate-predicate-evalua const { buildPredicateDeps, parsePredicateFlags } = require('../gsd-core/bin/lib/check-command-router.cjs'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +/** + * A real, bounded `sh -c` subprocess spawned via the production + * runBoundedShell dependency -- the describe block's own name is "real + * bounded sh -c subprocess." + */ +const BOUNDED_SHELL_PROBE_TIMEOUT_MS = 5000; + +/** + * The same runBoundedShell call as BOUNDED_SHELL_PROBE_TIMEOUT_MS, but + * deliberately tiny (not generous headroom) to force a `sleep 1` command + * past the bound within this test's own lifetime, proving "timeout kills + * the subprocess (SIGTERM => timedOut:true)." + */ +const BOUNDED_SHELL_FORCED_TIMEOUT_MS = 100; + // ─── buildPredicateDeps: real subprocess exit mapping ───────────────────────── describe('buildPredicateDeps — real bounded sh -c subprocess', () => { @@ -30,30 +45,30 @@ describe('buildPredicateDeps — real bounded sh -c subprocess', () => { const cwd = process.cwd(); test('`true` => exitCode 0, not timed out', () => { - const r = deps.runBoundedShell({ command: 'true', cwd, timeoutMs: 5000 }); + const r = deps.runBoundedShell({ command: 'true', cwd, timeoutMs: BOUNDED_SHELL_PROBE_TIMEOUT_MS }); assert.equal(r.exitCode, 0); assert.equal(r.timedOut, false); }); test('`false` => exitCode 1, not timed out', () => { - const r = deps.runBoundedShell({ command: 'false', cwd, timeoutMs: 5000 }); + const r = deps.runBoundedShell({ command: 'false', cwd, timeoutMs: BOUNDED_SHELL_PROBE_TIMEOUT_MS }); assert.equal(r.exitCode, 1); assert.equal(r.timedOut, false); }); test('`exit 3` => exitCode 3', () => { - const r = deps.runBoundedShell({ command: 'exit 3', cwd, timeoutMs: 5000 }); + const r = deps.runBoundedShell({ command: 'exit 3', cwd, timeoutMs: BOUNDED_SHELL_PROBE_TIMEOUT_MS }); assert.equal(r.exitCode, 3); }); test('stderr is captured from the subprocess', () => { - const r = deps.runBoundedShell({ command: 'echo oops >&2; exit 4', cwd, timeoutMs: 5000 }); + const r = deps.runBoundedShell({ command: 'echo oops >&2; exit 4', cwd, timeoutMs: BOUNDED_SHELL_PROBE_TIMEOUT_MS }); assert.equal(r.exitCode, 4); assert.match(r.stderr, /oops/); }); test('timeout kills the subprocess (SIGTERM => timedOut:true)', () => { - const r = deps.runBoundedShell({ command: 'sleep 1', cwd, timeoutMs: 100 }); + const r = deps.runBoundedShell({ command: 'sleep 1', cwd, timeoutMs: BOUNDED_SHELL_FORCED_TIMEOUT_MS }); assert.equal(r.timedOut, true); assert.equal(r.signal, 'SIGTERM'); }); diff --git a/tests/gate-predicate-evaluator.test.cjs b/tests/gate-predicate-evaluator.test.cjs index 955a37043..651a9afed 100644 --- a/tests/gate-predicate-evaluator.test.cjs +++ b/tests/gate-predicate-evaluator.test.cjs @@ -28,6 +28,24 @@ const { EVALUATOR_KINDS, } = require('../gsd-core/bin/lib/gate-predicate-evaluator.cjs'); +/** + * NOT a subprocess spawn timeout. Fixture DATA: an arbitrary declarative + * `timeout` value (seconds) feeding a canned timedOut:true fake-shell + * response -- the value itself is not asserted on, only the resulting + * block/message behavior. + */ +const PREDICATE_TIMEOUT_FIXTURE_SECONDS = 5; +/** + * NOT a subprocess spawn timeout. Fixture DATA: specifically tests the + * predicate's own seconds-to-milliseconds conversion -- the assertion + * below checks the converted value is exactly this times 1000. + */ +const PREDICATE_TIMEOUT_CUSTOM_SECONDS = 90; +/** NOT a subprocess spawn timeout. Fixture DATA: a deliberately-invalid (zero) declarative timeout value, proving the predicate validator rejects a non-positive timeout. */ +const INVALID_PREDICATE_TIMEOUT_ZERO = 0; +/** NOT a subprocess spawn timeout. Fixture DATA: a deliberately-invalid (negative) declarative timeout value, proving the predicate validator rejects a non-positive timeout. */ +const INVALID_PREDICATE_TIMEOUT_NEGATIVE = -5; + // ─── Fake bounded-shell seam ────────────────────────────────────────────────── /** Build a fake runBoundedShell that records the invocation and returns a preset result. */ @@ -113,7 +131,7 @@ describe('evaluatePredicate — command-exit-zero timeout', () => { test('timedOut => block:true with timed-out message', () => { const shell = fakeShell({ timedOut: true, exitCode: null, signal: 'SIGTERM' }); const res = evaluatePredicate( - { kind: 'command-exit-zero', command: 'sleep 100', timeout: 5 }, + { kind: 'command-exit-zero', command: 'sleep 100', timeout: PREDICATE_TIMEOUT_FIXTURE_SECONDS }, baseCtx, { runBoundedShell: shell.run }, ); @@ -134,7 +152,7 @@ describe('evaluatePredicate — command-exit-zero timeout', () => { test('custom timeout (seconds) honored and converted to ms', () => { const shell = fakeShell({ exitCode: 0 }); evaluatePredicate( - { kind: 'command-exit-zero', command: 'true', timeout: 90 }, + { kind: 'command-exit-zero', command: 'true', timeout: PREDICATE_TIMEOUT_CUSTOM_SECONDS }, baseCtx, { runBoundedShell: shell.run }, ); @@ -240,11 +258,11 @@ describe('evaluatePredicate — malformed predicate throws (maps to check-cmd fa test('non-positive timeout throws', () => { assert.throws( - () => evaluatePredicate({ kind: 'command-exit-zero', command: 'x', timeout: 0 }, baseCtx, { runBoundedShell: fakeShell({}).run }), + () => evaluatePredicate({ kind: 'command-exit-zero', command: 'x', timeout: INVALID_PREDICATE_TIMEOUT_ZERO }, baseCtx, { runBoundedShell: fakeShell({}).run }), /timeout/i, ); assert.throws( - () => evaluatePredicate({ kind: 'command-exit-zero', command: 'x', timeout: -5 }, baseCtx, { runBoundedShell: fakeShell({}).run }), + () => evaluatePredicate({ kind: 'command-exit-zero', command: 'x', timeout: INVALID_PREDICATE_TIMEOUT_NEGATIVE }, baseCtx, { runBoundedShell: fakeShell({}).run }), /timeout/i, ); }); diff --git a/tests/phase6-capstone-conformance.test.cjs b/tests/phase6-capstone-conformance.test.cjs index acc34ea9e..91a80e0aa 100644 --- a/tests/phase6-capstone-conformance.test.cjs +++ b/tests/phase6-capstone-conformance.test.cjs @@ -22,6 +22,22 @@ const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); const { isCentralConfigKey } = require('../gsd-core/bin/lib/config-schema.cjs'); const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); +/** + * A single gsd-tools.cjs `check --raw` CLI subcommand spawn, no + * fan-out, doing real registry lookup and gate-predicate evaluation work + * through the full CLI dispatch path -- "the real dispatch form used by + * the host loop." Coincides numerically with tests/helpers/timeouts.cjs's + * QUICK_SPAWN_TIMEOUT_MS and epic #4445 batch 13's + * TASK_RESOLVER_INVOKE_TIMEOUT_MS, but describes neither of those + * operations -- kept local. Also distinct from + * LOOP_HOOK_POINT_CLI_TIMEOUT_MS (60000ms), whose own doc comment lists + * `check ` as one of its representative verbs at a heavier + * bound -- this site's pre-existing value (10000ms) was not bench- + * remeasured against that class norm and is preserved as-is, not + * reclassified. + */ +const GATE_CHECK_CLI_TIMEOUT_MS = 10000; + function readRepoFile(relativePath) { return fs.readFileSync(path.join(ROOT, relativePath), 'utf8'); } @@ -421,7 +437,7 @@ describe('ADR-857 phase 6 — capabilities must not bake install paths into the rawOut = execFileSync( process.execPath, [gsdTools, 'check', query, '1', '--raw'], - { cwd: tmpDir, encoding: 'utf-8', timeout: 10000 }, + { cwd: tmpDir, encoding: 'utf-8', timeout: GATE_CHECK_CLI_TIMEOUT_MS }, ); const parsed = JSON.parse(rawOut.trim()); if (typeof parsed.block !== 'boolean') { @@ -437,7 +453,7 @@ describe('ADR-857 phase 6 — capabilities must not bake install paths into the rawOut = execFileSync( process.execPath, [gsdTools, 'check', query, tmpDir, '--raw'], - { cwd: tmpDir, encoding: 'utf-8', timeout: 10000 }, + { cwd: tmpDir, encoding: 'utf-8', timeout: GATE_CHECK_CLI_TIMEOUT_MS }, ); const parsed = JSON.parse(rawOut.trim()); if (typeof parsed.block !== 'boolean') { diff --git a/tests/policy-160-route0-resume.test.cjs b/tests/policy-160-route0-resume.test.cjs index bb236197f..d6e07c786 100644 --- a/tests/policy-160-route0-resume.test.cjs +++ b/tests/policy-160-route0-resume.test.cjs @@ -20,6 +20,7 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { QUICK_SPAWN_TIMEOUT_MS, LOOP_HOOK_POINT_CLI_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); describe('Route 0: resume_incomplete_phase invariant (#160)', () => { const nextMdPath = path.join(__dirname, '..', 'gsd-core', 'workflows', 'next.md'); @@ -471,7 +472,7 @@ describe('Route 0: resume_incomplete_phase invariant (#160)', () => { // (Route 0 CAN fire), not permanently 0 (the bug this closes). const { execFileSync } = require('node:child_process'); let jqAvailable = false; - try { execFileSync('jq', ['--version'], { stdio: 'ignore', timeout: 10000, killSignal: 'SIGKILL' }); jqAvailable = true; } catch { /* no jq on PATH */ } + try { execFileSync('jq', ['--version'], { stdio: 'ignore', timeout: QUICK_SPAWN_TIMEOUT_MS, killSignal: 'SIGKILL' }); jqAvailable = true; } catch { /* no jq on PATH */ } if (!jqAvailable) { t.skip('jq not on PATH — this contract test applies progress.md\'s literal jq expression to real roadmap.analyze JSON; the source-text assertions above still validate the fix'); return; } const { createTempProject, cleanup } = require('./helpers.cjs'); @@ -492,7 +493,7 @@ describe('Route 0: resume_incomplete_phase invariant (#160)', () => { const roadmapJson = execFileSync(process.execPath, [toolsBin, 'roadmap', 'analyze'], { cwd: tmpDir, encoding: 'utf8', - timeout: 60000, + timeout: LOOP_HOOK_POINT_CLI_TIMEOUT_MS, }); const phase1 = JSON.parse(roadmapJson).phases.find((p) => String(p.number) === '1'); assert.ok(phase1, 'roadmap.analyze must report phase 1'); @@ -503,13 +504,13 @@ describe('Route 0: resume_incomplete_phase invariant (#160)', () => { const planCountOut = execFileSync('jq', ['-r', '.plan_count // 0'], { input: JSON.stringify(phase1), encoding: 'utf8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, killSignal: 'SIGKILL', }).trim(); const summaryCountOut = execFileSync('jq', ['-r', '.summary_count // 0'], { input: JSON.stringify(phase1), encoding: 'utf8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, killSignal: 'SIGKILL', }).trim(); diff --git a/tests/prohibition-enforcement.test.cjs b/tests/prohibition-enforcement.test.cjs index 94587f7bf..3223daebe 100644 --- a/tests/prohibition-enforcement.test.cjs +++ b/tests/prohibition-enforcement.test.cjs @@ -13,6 +13,22 @@ const { createTempDir, cleanup } = require('./helpers.cjs'); const ENFORCEMENT_LIB = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'prohibition-enforcement.cjs'); +/** + * A deliberately-short (not generous headroom) enforcement bound, forcing + * a real hung node --test fixture (whose own internal block is 10 + * seconds) past the bound quickly within this test's own runtime -- + * proving "a HANGING node-test fails closed via the bounded timeout." + */ +const HANG_TEST_ENFORCEMENT_TIMEOUT_MS = 1500; + +/** + * The same deliberately-short enforcement-bound pattern as + * HANG_TEST_ENFORCEMENT_TIMEOUT_MS, but for a different regression + * (#3660: no orphaned descendant survives a runner-only kill) at a + * different pre-existing bound -- kept separate, never equalized. + */ +const ORPHAN_HANG_TEST_ENFORCEMENT_TIMEOUT_MS = 1200; + const TEST_TIER = Object.freeze({ requirement_id: 'R1', category: 'safety', @@ -710,7 +726,7 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => { const result = enforce.runProhibitionEnforcement( TEST_TIER, { kind: 'node-test', target: tf, failFirst: true }, - { cwd: dir, timeoutMs: 1500 }, + { cwd: dir, timeoutMs: HANG_TEST_ENFORCEMENT_TIMEOUT_MS }, ); assert.notEqual(result.status, 'green', 'a hung check must be killed and fail closed — never hang verify or green'); assert.equal(result.located, true); @@ -875,7 +891,7 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => { const result = enforce.runProhibitionEnforcement( TEST_TIER, { kind: 'node-test', target: tf, failFirst: true }, - { cwd: dir, timeoutMs: 1200 }, + { cwd: dir, timeoutMs: ORPHAN_HANG_TEST_ENFORCEMENT_TIMEOUT_MS }, ); assert.notEqual(result.status, 'green', 'a hung check must fail closed (unchanged pre-existing contract)'); const workerPid = await readPidWithRetry(pidfilePath);