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 <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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"
|
||||
]
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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 },
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user