diff --git a/eslint-rules/no-adhoc-timeout-literal.allowlist.json b/eslint-rules/no-adhoc-timeout-literal.allowlist.json index 175dab7f0..a233b9861 100644 --- a/eslint-rules/no-adhoc-timeout-literal.allowlist.json +++ b/eslint-rules/no-adhoc-timeout-literal.allowlist.json @@ -18,8 +18,5 @@ "tests/reviewer-trust-disclosure.test.cjs", "tests/run-tests-temp-root.test.cjs", "tests/shell-command-projection-dispatch.test.cjs", - "tests/task-command-router-resolve-content.test.cjs", - "tests/task-content-resolution.test.cjs", - "tests/task-content-resolver-grammar-parity.test.cjs", "tests/teams-status.test.cjs" ] diff --git a/tests/helpers/timeouts.cjs b/tests/helpers/timeouts.cjs index b9ea0d00d..5b35c27dc 100644 --- a/tests/helpers/timeouts.cjs +++ b/tests/helpers/timeouts.cjs @@ -300,6 +300,25 @@ const PATHOLOGICAL_INPUT_TEST_TIMEOUT_MS = 60000; */ const GSD_TOOLS_CLI_MODERATE_TIMEOUT_MS = 30000; +/** + * NOT a subprocess spawn timeout. Fixture DATA -- the default, plausible- + * looking `invoke.timeoutMs` value for a well-formed task-content-resolver + * manifest, used as the base fixture across most "valid resolver" tests on + * the RESOLUTION side of that system (tests/task-content-resolution.cjs and + * its grammar-parity sibling). Coincides in value and role with epic #4445 + * batch 10's file-local `TASK_RESOLVER_FIXTURE_TIMEOUT_MS` in + * tests/capability-validator-task-content-resolver.test.cjs (the VALIDATION + * side of the same resolver system, a different file, not exported) -- + * disclosed, not merged; both independently describe the same manifest + * field's plausible default, unsurprising for two suites covering the same + * resolver. + * + * Shared across 2 files in batch #4524 of the ad hoc timeout literal + * migration, epic #4445 -- every site independently arrived at this exact + * value -- that is why it lives here rather than as a file-local constant. + */ +const TASK_RESOLVER_INVOKE_TIMEOUT_MS = 10000; + module.exports = { PROBE_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS, @@ -318,4 +337,5 @@ module.exports = { REAL_REPO_GIT_TIMEOUT_MS, PATHOLOGICAL_INPUT_TEST_TIMEOUT_MS, GSD_TOOLS_CLI_MODERATE_TIMEOUT_MS, + TASK_RESOLVER_INVOKE_TIMEOUT_MS, }; diff --git a/tests/task-command-router-resolve-content.test.cjs b/tests/task-command-router-resolve-content.test.cjs index d6e8e1285..e7a4fadba 100644 --- a/tests/task-command-router-resolve-content.test.cjs +++ b/tests/task-command-router-resolve-content.test.cjs @@ -27,6 +27,13 @@ const { ResolverFailedError, } = require('../gsd-core/bin/lib/task-content-resolution.cjs'); +/** + * NOT a subprocess spawn timeout. Fixture DATA inside a fakeCapabilities + * manifest consumed by a fully-injected resolveTaskContentFn -- neither + * test using this constant ever reaches a real child process. + */ +const FAKE_RESOLVER_INVOKE_TIMEOUT_MS = 5000; + function writePlan(dir, taskXml) { const planPath = path.join(dir, '01-PLAN.md'); fs.writeFileSync(planPath, `# Plan\n\n${taskXml}\n`, 'utf8'); @@ -123,7 +130,7 @@ describe('task resolve-content (rows 17-19)', () => { id: 'fake-tracker', taskContentResolver: { trackerPrefix: 'test', - invoke: { binary: 'fake-cli', args: ['show', '{{id}}'], timeoutMs: 5000 }, + invoke: { binary: 'fake-cli', args: ['show', '{{id}}'], timeoutMs: FAKE_RESOLVER_INVOKE_TIMEOUT_MS }, }, }, ]; @@ -176,7 +183,7 @@ describe('task resolve-content (rows 17-19)', () => { id: 'fake-tracker', taskContentResolver: { trackerPrefix: 'test', - invoke: { binary: 'fake-cli', args: ['show', '{{id}}'], timeoutMs: 5000 }, + invoke: { binary: 'fake-cli', args: ['show', '{{id}}'], timeoutMs: FAKE_RESOLVER_INVOKE_TIMEOUT_MS }, }, }, ]; diff --git a/tests/task-content-resolution.test.cjs b/tests/task-content-resolution.test.cjs index 9396bd022..87fab1640 100644 --- a/tests/task-content-resolution.test.cjs +++ b/tests/task-content-resolution.test.cjs @@ -8,6 +8,7 @@ const { test } = require('node:test'); const assert = require('node:assert'); const fc = require('fast-check'); +const { TASK_RESOLVER_INVOKE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const m = require('../gsd-core/bin/lib/task-content-resolution.cjs'); const { @@ -22,6 +23,22 @@ const { ResolverMalformedOutputError, } = m; +/** + * NOT a subprocess spawn timeout. Fixture DATA: a plausible-looking + * invoke.timeoutMs value used as filler in tests whose actual assertion is + * about something ELSE in the manifest (a substring-vs-placeholder + * distinction, a missing {{id}} placeholder, an empty binary field) -- the + * timeout value itself is never the thing under test at these sites. + */ +const TASK_RESOLVER_INVOKE_FILLER_TIMEOUT_MS = 1000; + +/** NOT a subprocess spawn timeout. Fixture DATA: a deliberately-invalid (zero) invoke.timeoutMs value in a findResolver garbage-shapes test, proving a zero timeout is rejected. */ +const INVALID_TIMEOUT_ZERO_MS = 0; +/** NOT a subprocess spawn timeout. Fixture DATA: a deliberately-invalid (negative) invoke.timeoutMs value in a findResolver garbage-shapes test, proving a negative timeout is rejected. */ +const INVALID_TIMEOUT_NEGATIVE_MS = -5; +/** NOT a subprocess spawn timeout. Fixture DATA: a deliberately-invalid (non-integer) invoke.timeoutMs value in a findResolver garbage-shapes test, proving a non-integer timeout is rejected. */ +const INVALID_TIMEOUT_NON_INTEGER_MS = 1.5; + function beadsCapability(overrides = {}) { return { id: 'beads-capability', @@ -30,7 +47,7 @@ function beadsCapability(overrides = {}) { invoke: { binary: 'bd', args: ['show', '{{id}}', '--json'], - timeoutMs: 10000, + timeoutMs: TASK_RESOLVER_INVOKE_TIMEOUT_MS, }, ...overrides, }, @@ -68,16 +85,16 @@ test('id containing colons splits on first colon only (row 16)', () => { // ─── buildInvocation — pure ───────────────────────────────────────────────────── test('buildInvocation replaces every {{id}} entry with the literal id', () => { - const resolver = { invoke: { binary: 'bd', args: ['show', '{{id}}', '--json'], timeoutMs: 10000 } }; + const resolver = { invoke: { binary: 'bd', args: ['show', '{{id}}', '--json'], timeoutMs: TASK_RESOLVER_INVOKE_TIMEOUT_MS } }; assert.deepStrictEqual(buildInvocation(resolver, 'GSD-42'), { binary: 'bd', args: ['show', 'GSD-42', '--json'], - timeoutMs: 10000, + timeoutMs: TASK_RESOLVER_INVOKE_TIMEOUT_MS, }); }); test('buildInvocation does not touch args that merely contain {{id}} as a substring', () => { - const resolver = { invoke: { binary: 'bd', args: ['prefix-{{id}}-suffix'], timeoutMs: 1000 } }; + const resolver = { invoke: { binary: 'bd', args: ['prefix-{{id}}-suffix'], timeoutMs: TASK_RESOLVER_INVOKE_FILLER_TIMEOUT_MS } }; assert.deepStrictEqual(buildInvocation(resolver, 'X').args, ['prefix-{{id}}-suffix']); }); @@ -104,7 +121,7 @@ test('findResolver returns "ambiguous" for two matching capabilities (row 7)', ( test('findResolver ignores a declaration missing the {{id}} placeholder (row 15)', () => { const bad = { id: 'bad-capability', - taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['show'], timeoutMs: 1000 } }, + taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['show'], timeoutMs: TASK_RESOLVER_INVOKE_FILLER_TIMEOUT_MS } }, }; assert.strictEqual(findResolver('beads', [bad]), null); }); @@ -115,10 +132,10 @@ test('findResolver ignores garbage taskContentResolver shapes without throwing', { id: 'b', taskContentResolver: 'not-an-object' }, { id: 'c', taskContentResolver: [] }, { id: 'd', taskContentResolver: { trackerPrefix: 'beads' } }, // no invoke - { id: 'e', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: '', args: ['{{id}}'], timeoutMs: 1000 } } }, - { id: 'f', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['{{id}}'], timeoutMs: 0 } } }, - { id: 'g', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['{{id}}'], timeoutMs: -5 } } }, - { id: 'h', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['{{id}}'], timeoutMs: 1.5 } } }, + { id: 'e', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: '', args: ['{{id}}'], timeoutMs: TASK_RESOLVER_INVOKE_FILLER_TIMEOUT_MS } } }, + { id: 'f', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['{{id}}'], timeoutMs: INVALID_TIMEOUT_ZERO_MS } } }, + { id: 'g', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['{{id}}'], timeoutMs: INVALID_TIMEOUT_NEGATIVE_MS } } }, + { id: 'h', taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['{{id}}'], timeoutMs: INVALID_TIMEOUT_NON_INTEGER_MS } } }, ]; assert.strictEqual(findResolver('beads', garbage), null); }); @@ -216,7 +233,7 @@ test('row 12: resolver exceeding timeoutMs throws ResolverTimeoutError (simulate () => resolveTaskContent({ trackerId: 'beads:GSD-42', capabilities: [beadsCapability()], execFn }), (err) => { assert.ok(err instanceof ResolverTimeoutError); - assert.strictEqual(err.timeoutMs, 10000); + assert.strictEqual(err.timeoutMs, TASK_RESOLVER_INVOKE_TIMEOUT_MS); return true; }, ); @@ -245,7 +262,7 @@ test('row 14: valid JSON non-object stdout (null/array/string/number/bool) throw test('row 15: invoke.args without {{id}} placeholder is rejected — resolves no-resolver, never spawns', () => { const badCapability = { id: 'bad-capability', - taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['show'], timeoutMs: 1000 } }, + taskContentResolver: { trackerPrefix: 'beads', invoke: { binary: 'bd', args: ['show'], timeoutMs: TASK_RESOLVER_INVOKE_FILLER_TIMEOUT_MS } }, }; const execFn = () => { throw new Error('must not spawn a rejected declaration'); }; const result = resolveTaskContent({ trackerId: 'beads:GSD-42', capabilities: [badCapability], execFn }); diff --git a/tests/task-content-resolver-grammar-parity.test.cjs b/tests/task-content-resolver-grammar-parity.test.cjs index 43276a31c..139237aa6 100644 --- a/tests/task-content-resolver-grammar-parity.test.cjs +++ b/tests/task-content-resolver-grammar-parity.test.cjs @@ -21,9 +21,10 @@ const assert = require('node:assert/strict'); const { validateTaskContentResolver } = require('../gsd-core/bin/lib/capability-validator.cjs'); const { findResolver } = require('../gsd-core/bin/lib/task-content-resolution.cjs'); +const { TASK_RESOLVER_INVOKE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); function validInvoke() { - return { binary: 'bd', args: ['show', '{{id}}', '--json'], timeoutMs: 10000 }; + return { binary: 'bd', args: ['show', '{{id}}', '--json'], timeoutMs: TASK_RESOLVER_INVOKE_TIMEOUT_MS }; } function featureCapWithResolver(trackerPrefix) {