Skip to content

Give sibling Session catalog isolation checks a 60-second timeout - #995

Merged
taras merged 1 commit into
mainfrom
agent/ci-main-994-session-catalog
Oct 26, 2026
Merged

taras merged 1 commit into
mainfrom
agent/ci-main-994-session-catalog

Conversation

@taras

@taras taras commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

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 catalogs could 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

  • The retained controlled reproduction adds a two-second delay before each nested worker starts. The original fixture times out at 20 seconds; the corrected fixture passes all five Tests under the same delay. This distinguishes startup cost from a weakened assertion.
  • 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.
  • The retained delivery receipt records the required green check passing for 32d1e0ba0a5255fb88e5c07822ff3352ac2d3863, including composability.

Scope

Included

  • One explicit timeout on the sibling catalog-isolation Test in 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

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified (none).
  • The description matches the final diff and test results.

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>
@taras taras added the ci-main-red-fix Maintainer-controlled: lands one PR while main is red, only with the full clean-checkout proof label Oct 26, 2026
@github-actions

Copy link
Copy Markdown

PR #995: Give sibling Session catalog checks a finite integration timeout

1 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 taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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">

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@taras taras changed the title Give sibling Session catalog checks a finite integration timeout Give sibling Session catalog isolation checks a 60-second timeout Oct 26, 2026
@taras
taras marked this pull request as ready for review October 26, 2026 19:56
@taras
taras merged commit 71083a5 into main Oct 26, 2026
84 of 86 checks passed
@taras
taras deleted the agent/ci-main-994-session-catalog branch October 26, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-main-red-fix Maintainer-controlled: lands one PR while main is red, only with the full clean-checkout proof

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant