From 56b706a07a3b08a10e338d2c0b0d780bff4cd539 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 12 Sep 2026 23:21:21 -0400 Subject: [PATCH] test(#4524): migrate task-content resolution batch to named timeout constants (#4675) Batch 13 of the ad hoc timeout literal migration (epic #4445). Replaces every bare numeric timeoutMs object-literal property in tests/task-content-resolution.test.cjs, tests/task-command-router-resolve-content.test.cjs, and tests/task-content-resolver-grammar-parity.test.cjs with a named constant, per eslint-rules/no-adhoc-timeout-literal.cjs. Removes these 3 files from the rule's allowlist. Every one of the 13 sites describes the same field -- a task-content- resolver manifest's invoke.timeoutMs -- as fixture/validation data; none is a real subprocess spawn timeout, verified by tracing each site to a pure function, a garbage-shape rejection path, or a fully-injected fake exec function. Adds one new shared constant, TASK_RESOLVER_INVOKE_TIMEOUT_MS, used by 2 files in this batch (crossing the promotion bar). Adds file-local constants for a value used by only 1 file, plus three deliberately-invalid values (zero, negative, non-integer) inside one findResolver garbage-shapes test proving the validator rejects a malformed manifest regardless of which way its timeout is invalid. Also fixes a review-caught defect outside the mechanical rule's own scope: a bare-literal duplicate of the new shared constant inside a ResolverTimeoutError assertion (a call argument, not an object-literal property, so the lint rule never flagged it) -- renamed in this same PR per the no-deferrals rule. No src/bin file touched, no numeric value changed anywhere. Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- .../no-adhoc-timeout-literal.allowlist.json | 3 -- tests/helpers/timeouts.cjs | 20 ++++++++++ ...sk-command-router-resolve-content.test.cjs | 11 +++++- tests/task-content-resolution.test.cjs | 39 +++++++++++++------ ...k-content-resolver-grammar-parity.test.cjs | 3 +- 5 files changed, 59 insertions(+), 17 deletions(-) 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) {