diff --git a/eslint-rules/no-adhoc-timeout-literal.allowlist.json b/eslint-rules/no-adhoc-timeout-literal.allowlist.json index 47efb5bee..50676ff8a 100644 --- a/eslint-rules/no-adhoc-timeout-literal.allowlist.json +++ b/eslint-rules/no-adhoc-timeout-literal.allowlist.json @@ -1,15 +1,8 @@ [ "tests/adr-612-bracket-coherence.test.cjs", "tests/adr-612-bracket-read-tolerance.test.cjs", - "tests/adr857-core-without-capabilities.test.cjs", "tests/assumption-delta-checkpoint-e2e.test.cjs", "tests/assumption-delta.test.cjs", - "tests/capability-cli.test.cjs", - "tests/capability-probe-fallback.test.cjs", - "tests/capability-state.test.cjs", - "tests/capability-trust.test.cjs", - "tests/capability-validator-task-content-resolver.test.cjs", - "tests/capability-writer.test.cjs", "tests/check-env.test.cjs", "tests/check-predicate.test.cjs", "tests/cjs-command-router-adapter.test.cjs", diff --git a/tests/adr857-core-without-capabilities.test.cjs b/tests/adr857-core-without-capabilities.test.cjs index 729089cd6..dcb56df43 100644 --- a/tests/adr857-core-without-capabilities.test.cjs +++ b/tests/adr857-core-without-capabilities.test.cjs @@ -105,6 +105,15 @@ function buildTddOnlyConfig() { // ── Helpers ─────────────────────────────────────────────────────────────────── +/** + * A single gsd-tools.cjs CLI subcommand invocation via execFileSync, no + * fan-out. Same class as tests/helpers/timeouts.cjs's + * LOOP_HOOK_POINT_CLI_TIMEOUT_MS, but at half that constant's pre-existing + * bound -- not equalized without bench data. Kept file-local: this batch's + * only site at this exact class+value. + */ +const GSD_TOOLS_CLI_TIMEOUT_MS = 30000; + /** * Run gsd-tools subprocess and return { exitCode, output }. * Does NOT throw on non-zero exit — let the test assert. @@ -114,7 +123,7 @@ function runCli(args, cwd) { const stdout = execFileSync(process.execPath, [GSD_TOOLS, ...args], { cwd, encoding: 'utf-8', - timeout: 30000, + timeout: GSD_TOOLS_CLI_TIMEOUT_MS, }); return { exitCode: 0, output: stdout.trim() }; } catch (err) { diff --git a/tests/capability-cli.test.cjs b/tests/capability-cli.test.cjs index 70b3cfaf5..fca7b498b 100644 --- a/tests/capability-cli.test.cjs +++ b/tests/capability-cli.test.cjs @@ -19,6 +19,7 @@ const os = require('node:os'); const path = require('node:path'); const { runGsdTools, cleanup } = require('./helpers.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // ─── Fixtures ─────────────────────────────────────────────────────────────── @@ -1013,7 +1014,7 @@ describe('capability consent store (#1459)', () => { const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const fifoPath = path.join(dir, 'capability.json'); throwIfFailed( - runHook(fifoPath, [], { interpreter: 'mkfifo', timeoutMs: 15000 }), + runHook(fifoPath, [], { interpreter: 'mkfifo', timeoutMs: PROBE_TIMEOUT_MS }), `mkfifo ${fifoPath}`, ); // A committed project ledger so the list iterates this entry (the FIFO is on the metadata-read path). diff --git a/tests/capability-probe-fallback.test.cjs b/tests/capability-probe-fallback.test.cjs index b41183fd6..2c6ca287f 100644 --- a/tests/capability-probe-fallback.test.cjs +++ b/tests/capability-probe-fallback.test.cjs @@ -38,6 +38,19 @@ const DELTA_FRAGMENT = path.join( REPO_ROOT, 'capabilities', 'assumption-delta', 'fragments', 'plan-pre.md'); const TOOLS_PATH = path.join(REPO_ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs'); +/** + * The OUTER runNode() timeoutMs bound for a node -e wrapper that itself + * spawns ONE bash subprocess to execute an extracted capability-fragment + * probe snippet (a two-level nested spawn). Distinct from + * HOOK_FANOUT_TIMEOUT_MS (a git-hook-shaped multi-spawn fan-out, ~4 spawns) + * and LOOP_HOOK_POINT_CLI_TIMEOUT_MS (specifically gsd-tools.cjs, not a + * generic node -e wrapper) despite the coincidentally-matching value. Does + * NOT touch the inner `timeout: 60000` inside the node-e script's own + * string literal a few lines above -- that is JS text content passed to a + * spawned child, not a real object-literal property in this file's AST. + */ +const FRAGMENT_PROBE_SNIPPET_TIMEOUT_MS = 60000; + /** * Pull the fragment's probe snippet out of its markdown: the first fenced * ```bash block that assigns `varName`. The block is returned verbatim so the @@ -84,7 +97,7 @@ function runSnippet({ block, varName, prelude, cwd }) { const { spawnSync } = require('node:child_process'); const r = spawnSync('bash', [process.argv[1]], { cwd: process.argv[2], encoding: 'utf8', timeout: 60000 }); process.stdout.write(r.stdout || ''); - `, scriptFile, cwd], { cwd, timeoutMs: 60000 }); + `, scriptFile, cwd], { cwd, timeoutMs: FRAGMENT_PROBE_SNIPPET_TIMEOUT_MS }); assert.strictEqual(r.outcome, OUTCOME.EXITED, `snippet runner outcome: ${r.outcome}`); return r.stdout; } finally { diff --git a/tests/capability-state.test.cjs b/tests/capability-state.test.cjs index bb53614db..a5efa2d3b 100644 --- a/tests/capability-state.test.cjs +++ b/tests/capability-state.test.cjs @@ -16,6 +16,7 @@ const os = require('node:os'); const path = require('node:path'); const { cleanup, runGsdTools } = require('./helpers.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { resolveCapabilityState, @@ -786,7 +787,7 @@ function runCapabilityState(cwd, configDir) { const result = spawnSync( process.execPath, [gsdToolsPath, 'capability', 'state', '--config-dir', configDir, '--raw', '--cwd', cwd], - { encoding: 'utf8', timeout: 15000 }, + { encoding: 'utf8', timeout: PROBE_TIMEOUT_MS }, ); return result; } @@ -1324,7 +1325,7 @@ describe('regressions: installed-runtime capability surface (#1160)', () => { '--config-dir', tmpInstalledConfigDir, '--cwd', tmpInstalledProjectDir, ], - { encoding: 'utf8', timeout: 15000 }, + { encoding: 'utf8', timeout: PROBE_TIMEOUT_MS }, ); assert.strictEqual(result.status, 0, `gsd-tools exited ${result.status}:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); const envelope = JSON.parse(result.stdout.trim()); @@ -1353,7 +1354,7 @@ describe('regressions: installed-runtime capability surface (#1160)', () => { '--config-dir', tmpInstalledConfigDir, '--cwd', tmpInstalledProjectDir, ], - { encoding: 'utf8', timeout: 15000 }, + { encoding: 'utf8', timeout: PROBE_TIMEOUT_MS }, ); assert.strictEqual(result.status, 0, `gsd-tools exited ${result.status}:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); const envelope = JSON.parse(result.stdout.trim()); @@ -1381,6 +1382,19 @@ describe('regressions: installed-runtime capability surface (#1160)', () => { // is genuinely empty, so pre-fix the '*' profile materialized to an empty // surfaced set → enabled=false → verify:post activeHooks: []. This test FAILS // before the fix and PASSES after. + + /** + * gsd-tools.cjs spawned against a COPIED install-root tree (fs.cpSync of the + * executable runtime into a temp dir simulating a global skills-runtime + * install with no commands/ sibling) rather than the bare source checkout. + * Same CLI-query shape as PROBE_TIMEOUT_MS (15000ms) elsewhere in this file, + * but a genuinely heavier pre-existing bound -- not equalized without bench + * data. Coincides numerically with tests/helpers/timeouts.cjs's + * STAGED_HOOK_SCRIPT_TIMEOUT_MS but describes a different operation (a + * gsd-tools.cjs CLI subcommand, not a staged hook script) -- kept local. + */ + const INSTALLED_RUNTIME_CLI_TIMEOUT_MS = 20000; + describe('true installed layout (commands/gsd unreachable)', () => { let installRoot; let installedConfigDir; @@ -1449,7 +1463,7 @@ describe('regressions: installed-runtime capability surface (#1160)', () => { '--cwd', installedProjectDir, '--raw', ], - { encoding: 'utf8', timeout: 20000 }, + { encoding: 'utf8', timeout: INSTALLED_RUNTIME_CLI_TIMEOUT_MS }, ); assert.strictEqual(result.status, 0, `gsd-tools exited ${result.status}:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); const envelope = JSON.parse(result.stdout); @@ -1471,7 +1485,7 @@ describe('regressions: installed-runtime capability surface (#1160)', () => { '--config-dir', installedConfigDir, '--cwd', installedProjectDir, ], - { encoding: 'utf8', timeout: 20000 }, + { encoding: 'utf8', timeout: INSTALLED_RUNTIME_CLI_TIMEOUT_MS }, ); assert.strictEqual(result.status, 0, `gsd-tools exited ${result.status}:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); const envelope = JSON.parse(result.stdout.trim()); @@ -1506,7 +1520,7 @@ describe('regressions: installed-runtime capability surface (#1160)', () => { '--config-dir', installedConfigDir, '--cwd', disabledProj, ], - { encoding: 'utf8', timeout: 20000 }, + { encoding: 'utf8', timeout: INSTALLED_RUNTIME_CLI_TIMEOUT_MS }, ); assert.strictEqual(result.status, 0, `gsd-tools exited ${result.status}:\nstderr: ${result.stderr}`); const envelope = JSON.parse(result.stdout.trim()); diff --git a/tests/capability-trust.test.cjs b/tests/capability-trust.test.cjs index 9124a7f79..63c8219f3 100644 --- a/tests/capability-trust.test.cjs +++ b/tests/capability-trust.test.cjs @@ -20,6 +20,14 @@ function tmpDir() { return fs.mkdtempSync(path.join(os.tmpdir(), 'cap-trust-test-')); } +/** + * NOT a subprocess spawn timeout. Fixture DATA inside a synthetic MCP + * server config object passed to trust.signatureForManifest() -- proves + * key-reordering doesn't change a config's signature. Never passed into a + * real spawnSync/execFileSync options object. + */ +const FIXTURE_MCP_SERVER_TIMEOUT_VALUE = 30; + // --------------------------------------------------------------------------- // discloseExecutableSurfaces // --------------------------------------------------------------------------- @@ -491,8 +499,8 @@ test('finding-5: reordering keys WITHIN the full mcp config does NOT change the // revert-fails: if the full config were folded in via a NON-stable JSON (insertion-order // dependent), a mere key reorder would change the signature and this strictEqual would FAIL. The // full-config hash must use the stable (recursively key-sorted) encoding. - const a = { id: 'x', mcpServers: { srv: { command: 'node', envFile: '.env', timeout: 30, extra: { z: 1, a: 2 } } } }; - const b = { id: 'x', mcpServers: { srv: { extra: { a: 2, z: 1 }, timeout: 30, envFile: '.env', command: 'node' } } }; + const a = { id: 'x', mcpServers: { srv: { command: 'node', envFile: '.env', timeout: FIXTURE_MCP_SERVER_TIMEOUT_VALUE, extra: { z: 1, a: 2 } } } }; + const b = { id: 'x', mcpServers: { srv: { extra: { a: 2, z: 1 }, timeout: FIXTURE_MCP_SERVER_TIMEOUT_VALUE, envFile: '.env', command: 'node' } } }; assert.strictEqual( trust.signatureForManifest(a), trust.signatureForManifest(b), diff --git a/tests/capability-validator-task-content-resolver.test.cjs b/tests/capability-validator-task-content-resolver.test.cjs index 2fcd7f2af..5297b01ca 100644 --- a/tests/capability-validator-task-content-resolver.test.cjs +++ b/tests/capability-validator-task-content-resolver.test.cjs @@ -22,6 +22,39 @@ const { validateCrossCapability, } = require('../gsd-core/bin/lib/capability-validator.cjs'); +/** + * NOT a subprocess spawn timeout. Fixture DATA -- the default valid + * `invoke.timeoutMs` value used across most tests in this file's + * `validResolver()` base fixture, validated by validateTaskContentResolver() + * as pure manifest data. Coincides numerically with + * tests/helpers/timeouts.cjs's QUICK_SPAWN_TIMEOUT_MS but is not a real + * timeout in this file's own execution -- kept local. + */ +const TASK_RESOLVER_FIXTURE_TIMEOUT_MS = 10000; + +/** + * NOT a subprocess spawn timeout. Fixture DATA -- the exact ceiling value a + * dedicated describe block ("invoke.timeoutMs upper ceiling") asserts is + * still ACCEPTED by validateTaskContentResolver(). Mirrors a business-logic + * ceiling constant in gsd-core/bin/lib/capability-validator.cjs (production + * code, untouched by this migration). Coincides numerically with + * tests/helpers/timeouts.cjs's INSTALL_TIMEOUT_MS but is not a real timeout + * in this file's own execution -- kept local. Forms a CLAUDE.md + * limit/limit+1 boundary pair with TASK_RESOLVER_TIMEOUT_CEILING_PLUS_ONE_MS + * below -- do not change either value independently. + */ +const TASK_RESOLVER_TIMEOUT_CEILING_MS = 120000; + +/** + * NOT a subprocess spawn timeout. Fixture DATA -- the boundary-violating + * companion to TASK_RESOLVER_TIMEOUT_CEILING_MS above, asserting exactly + * one ms past the ceiling is REJECTED. A CLAUDE.md "Boundary Coverage" + * limit+1 value -- must always equal TASK_RESOLVER_TIMEOUT_CEILING_MS + 1. + * Declared as its own literal (not computed via +1 arithmetic) to match + * this migration's convention of plain-literal constants throughout. + */ +const TASK_RESOLVER_TIMEOUT_CEILING_PLUS_ONE_MS = 120001; + // ─── Fixture builders ────────────────────────────────────────────────────── // House convention (tests/capability-manifest-version.test.cjs): builder // functions return a VALID fixture, which each test then mutates. Every call @@ -34,7 +67,7 @@ function validResolver() { invoke: { binary: 'bd', args: ['show', '{{id}}', '--json'], - timeoutMs: 10000, + timeoutMs: TASK_RESOLVER_FIXTURE_TIMEOUT_MS, }, }; } @@ -233,7 +266,7 @@ describe('row 23 — invoke.timeoutMs must be a positive integer', () => { describe('invoke.timeoutMs upper ceiling (120000ms)', () => { test('timeoutMs 120000 (exactly at the ceiling) is accepted', () => { const resolver = validResolver(); - resolver.invoke = { ...resolver.invoke, timeoutMs: 120000 }; + resolver.invoke = { ...resolver.invoke, timeoutMs: TASK_RESOLVER_TIMEOUT_CEILING_MS }; const cap = featureCap({ taskContentResolver: resolver }); const errs = validateTaskContentResolver(cap); assert.ok( @@ -244,7 +277,7 @@ describe('invoke.timeoutMs upper ceiling (120000ms)', () => { test('timeoutMs 120001 (one past the ceiling) is rejected', () => { const resolver = validResolver(); - resolver.invoke = { ...resolver.invoke, timeoutMs: 120001 }; + resolver.invoke = { ...resolver.invoke, timeoutMs: TASK_RESOLVER_TIMEOUT_CEILING_PLUS_ONE_MS }; const cap = featureCap({ taskContentResolver: resolver }); const errs = validateTaskContentResolver(cap); assert.ok( diff --git a/tests/capability-writer.test.cjs b/tests/capability-writer.test.cjs index 220d37868..b07ad6f44 100644 --- a/tests/capability-writer.test.cjs +++ b/tests/capability-writer.test.cjs @@ -14,6 +14,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const { cleanup } = require('./helpers.cjs'); @@ -303,7 +304,7 @@ describe('capability-writer: setCapabilityState', () => { // Test: capability set ui --off (--config-dir rcd) exits 0 const offResult = runNode([gsdToolsBin, 'capability', 'set', 'ui', '--off', '--config-dir', rcd], { cwd, - timeoutMs: 15000, + timeoutMs: PROBE_TIMEOUT_MS, }); assert.equal(offResult.exitCode, 0, `capability set ui --off should exit 0, got ${String(offResult.exitCode)}. stderr: ${offResult.stderr}`); @@ -311,7 +312,7 @@ describe('capability-writer: setCapabilityState', () => { // Test: capability set ui --on (--config-dir rcd) exits 0 const onResult = runNode([gsdToolsBin, 'capability', 'set', 'ui', '--on', '--config-dir', rcd], { cwd, - timeoutMs: 15000, + timeoutMs: PROBE_TIMEOUT_MS, }); assert.equal(onResult.exitCode, 0, `capability set ui --on should exit 0, got ${String(onResult.exitCode)}. stderr: ${onResult.stderr}`); @@ -319,7 +320,7 @@ describe('capability-writer: setCapabilityState', () => { // Test: unknown id exits non-zero const unknownResult = runNode([gsdToolsBin, 'capability', 'set', 'does-not-exist', '--off', '--config-dir', rcd], { cwd, - timeoutMs: 15000, + timeoutMs: PROBE_TIMEOUT_MS, }); assert.notEqual(unknownResult.exitCode, 0, `capability set does-not-exist --off should exit non-zero, got ${String(unknownResult.exitCode)}`); @@ -502,7 +503,7 @@ describe('capability-writer: setCapabilityState', () => { const conflictResult = runNode( [gsdToolsBin, 'capability', 'set', 'ui', '--on', '--off', '--config-dir', rcd], - { cwd, timeoutMs: 15000 }, + { cwd, timeoutMs: PROBE_TIMEOUT_MS }, ); assert.notEqual( conflictResult.exitCode, 0,