Repository navigation
Give sibling Session catalog isolation checks a 60-second timeout - #995
Conversation
The Test starts six real test-agent workers across its two nested Executions and inherited the 20-second default; on the Node CI runner that startup cost reached the default (main-health #994). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR #995: Give sibling Session catalog checks a finite integration timeout1 files, +1 / -1 Scope✅ PR scope looks good. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static Analysis✅ Oxlint found no issues. Correctness✅ Small PR — correctness review skipped. |
taras
left a comment
There was a problem hiding this comment.
Review of 32d1e0b — correctness, performance impact, minimalism. No blocking findings.
The complete diff is one attribute: the Test at packages/test-agent/src/SessionConfiguration.test.md:39 gains timeout="60s". Nothing else in the tree changes.
Correctness. The failure this repairs is the one on main at dfe4c73: in the red run's test-node (5/10) job (https://github.com/taras/executable.md/actions/runs/40372864525/job/121747941691) the in-process consumer reports sibling nested Executions have independent catalogs: test timed out after 20 seconds, while the other four Tests and the CLI pass succeed. The fix uses the mechanism specs/testing-spec.md already defines for exactly this case: a per-Test timeout duration replacing the 20-second default. packages/testing/src/test-component.ts parses it and hands it to the same timebox; the installed default in packages/testing/src/components.ts stays at 20 seconds, so no other Test's budget moves. Both nested <Execution> elements, their separate <TestAgent> declarations and both AssertEquals are byte-identical to the base, so the sibling-independence negative control is unchanged: the first child moves effort to high, the second still prompts against the declared low-effort defaults. Two other Tests in this fixture family already carry the same 60-second budget (SessionConfiguration.test.md:9 from b3a4197, SessionConfiguration.refusals.md:4 from dd308df), so this is the established shape, not a new convention.
Evidence on the fixed revision. Both pull_request runs on this SHA pass test-node (5/10), the shard that runs this consumer: run 40382163496 (subtest 3 at 92.2 s) and run 40382259777 (subtest 3 at 96.4 s, the labelled run that also ran composability). The red run's same subtest took 96.1 s while halting the sibling Test at its 20-second bound. The near-identical wall clock with and without the timeout is consistent with the sibling Test finishing at roughly 20 s on the Linux runner, which puts the 60-second bound at about 3× the observed duration. That is an inference: the harness logs no per-Test timing, and the PR's controlled worker-start delay reproduction establishes the startup-cost mechanism rather than CI's exact numbers.
Performance impact. None on the passing path: the bound is only consulted when the body overruns it. On a genuinely hung run, detection of this one Test moves from 20 s to 60 s, so a red test-node shard can finish at most 40 s later. No production code is touched.
Minimalism. One attribute, no comment, no retry, pooling, cache or shared state. There is nothing to remove, and a tighter figure such as 45 s would have no measured basis while departing from the file's existing budgets.
One non-blocking observation is posted inline. Delivery stays with the maintainer and the required CI checks; this review does not approve merge or close #994.
| </TestAgent> | ||
|
|
||
| <Test name="sibling nested Executions have independent catalogs"> | ||
| <Test name="sibling nested Executions have independent catalogs" timeout="60s"> |
There was a problem hiding this comment.
Non-blocking observation: the next-tightest bound on this fixture is now the consumer's 120-second CLI process budget, not a Test budget.
Consequence: the Node consumer at packages/test-agent/tests/session-configuration.test.ts runs this whole document once through the CLI under RUN = { timeout: 120_000 }. On the Linux runner that subtest measured 90.7 s, 93.5 s and 93.9 s across the three recent test-node (5/10) jobs (runs 40382163496, 40372864525, 40382259777), so about 26 s of headroom remains. If worker startup cost grows further, that bound trips before any Test budget and reports as a CLI exit code rather than a named Test, which is a worse diagnostic than the one #994 produced.
Evidence: the duration_ms of runs configured Sessions and nested declarations through the source CLI and ACPX worker in each job log linked above. Related: packages/test-agent/tests/session-configuration.test.ts is still unmeasured in test-weights.json and is charged the heaviest recorded Node weight (176 s) while it actually ran 223 s in run 40382259777; the shard log says so on its first lines.
Recommendation: no change in this PR. If the CLI bound trips, raise that consumer's RUN.timeout or split the CLI pass; do not raise the 20-second <Test> default. The next measure-test-weights run records this file's real weight.
Why
The Session configuration integration fixture timed out on main's Node CI job while starting independent source workers for two nested Executions. Its catalog-isolation Test inherited the 20-second default.
Refs #994.
What changes
Before:
sibling nested Executions have independent catalogscould exhaust its default budget during worker startup.After: that Test declares
timeout="60s"and retains both nested Executions and all assertions.How to verify it
npx tsx --tsconfig tsconfig.node.json --test --test-concurrency=1 packages/test-agent/tests/session-configuration.test.ts— passed all three tests, covering the source CLI, configuration refusals, native identity, isolation and worker-free replay.deno task test packages/test-agent/tests/session-configuration.test.ts— passed one suite with three steps covering the same consumer.32d1e0ba0a5255fb88e5c07822ff3352ac2d3863, including composability.Scope
Included
packages/test-agent/src/SessionConfiguration.test.md.Intentionally unchanged
Risks and limitations
The focused local runs used Darwin; the original CI failure used Node on Linux. The controlled delay demonstrates the startup-cost mechanism, not the CI runner's exact timing breakdown. The Test remains bounded at 60 seconds.
Scope confirmation