[freeqaz]

Problem

#3149 moved two lane-deadline tests in tests/integration/ci-runner.test.mjs onto a held runner clock (tests/helpers/ci-runner-held-clock.mjs, finding 2164). Other tests in the file still race real time against a runner limit, and fail on main when the host is very busy. On 2 CPUs (taskset -c) with stress -c 256 on the same CPUs, 10 runs of the waiting block plus the cancelled-job test on main:

Test	Limit it races	Main
run kills a timed-out file's whole process group after the timeout record	5 s file timeout vs. the fixture recording its pids	fails 10/10 (no pid record)
run records a timed-out file's interrupted test and output tail in the diagnostics report	5 s file timeout vs. the fixture printing its 503 lines	fails 10/10 (no interrupted test)
run gives up on a preparation still running at the lane deadline and names it	lane deadline 5 s after the test starts vs. the runner reaching the preparation	fails 10/10 (lane-deadline instead of prepare-timeout)
run gives up on a hung cleanup after its allowance, ends its commands and starts no later file	2 s cleanup allowance vs. the cleanup's command starting its descendant	fails 10/10 (a recorded command outlives the runner)
run reports a cleanup that fails once a cancelled job's shell is gone as cancelled	the test's own 30 s spawnSync guard vs. both files running	fails 6/10 (shell ended by the guard's SIGTERM)
a runner timeout while no test was running says so	8 s file timeout vs. passes finishing	passes 10/10, but only by its margin

The other waiting tests ("timeout before any test started", "under a second left", and the two #3149 moved) passed 10/10; the rest of the file passed one full loaded run (842 s).

Change

Test-only; scripts/ci/run-tests.mjs is unchanged.

• The five waiting tests above start the runner with the held clock from fix(test): hold the ci runner's clock until deadline fixtures are in place #3149, held from the start. Each fixture creates a gate file once it reaches the state its assertions need: after printing its line or last line, when the after hook starts, when the hung preparation starts, and once the cleanup's command and its descendant are on record (now written with one rename, so the gate and the cleanup see both pids at once). Each limit then runs its full length after the gate.
• Assertions keep their meaning, and two get stricter:
  • The hung preparation's limit is now exactly the 3 s left before the deadline (was "between 0 and 5 s").
  • The diagnostics test's elapsedMs upper bound counts from when the fixture printed its last line, because the runner records real time and the clock stood still until then. Its 30 s wait is still far past the bound.
  • Each waiting test now also asserts the runner was not stopped by the test's own guard, and prints the runner's stderr when it fails.
• "a runner timeout while no test was running says so": the file timeout drops from 8 s to 3 s, since the gate now guarantees passes finished.
• "...cancelled job's shell...": the spawnSync guard goes from 30 s to 120 s. It only stops a hang; a guard SIGTERM still fails the SIGKILL assertion.

Verification

• Main, 2 CPUs + stress -c 256, 10 runs: see the table.
• This branch, same load, waiting block plus the cancelled-job test: 50/50 (61-178 s per run).
• Whole file, 2 CPUs, no load: 20/20. File runtime: main median 16.5 s (5 runs), branch median 16.4 s (20 runs).
• Planted runner bugs, each caught by the moved test: no SIGKILL to a timed-out file's group; file timeout 4x too long; preparation limit 0.5 s short of the deadline; abandoned cleanup commands not killed.
• Prettier and ESLint on the touched file.
• Internal review: approve with nits (hold from the start rather than one second in; comment timings). Applied.

Risks

• Test-only. A fixture that never reaches its gate holds the runner until the test's 60 s guard, and the held clock reports the missing gate on stderr every 10 s.
• The cleanup test's failure on main most likely points at a runner gap too: a descendant started while the runner lists its process tree at exit is not killed. Filed as a separate finding; not changed here.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR holds the runner clock until five fixtures are ready and increases the cancelled-job test’s outer timeout.

Example: A busy host takes more than 5 seconds to reach the hung preparation.

• Before: The lane deadline expires before preparation starts, producing lane-deadline instead of the expected prepare-timeout.
• After: The clock waits for preparation to start, then allows 3 seconds before asserting prepare-timeout.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	A focused, source-supported test repair with successful exact-head CI and harness-level validation.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored test-only PR is outside the ordinary contributor proof gate; exact-head CI succeeded, while the body still marks loaded head runs as in progress.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Contributors can receive CI failures because fixture startup loses a scheduling race rather than because runner behavior is wrong.
Reason: The maintainer-authored change addresses documented failures using an existing test mechanism and preserves distinct behavioral coverage.

Merge readiness

✅ Ready for maintainer review

Keep open: this maintainer-authored PR addresses concrete test timing races, preserves the runner assertions, and introduces no identified correctness defect.

Priority: P2
Reviewed head: 99f8df97336d8cab2a39b5607719248afcd87656

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: The reported post-change stress comparison remains in progress; no changed product behavior requires additional shipped-entry-point proof.

Agent review details

How this fits together

The CI runner integration suite launches the real runner against temporary fixture manifests and checks its result files, diagnostics, and surviving processes.

flowchart LR
  A[Integration test] --> B[Runner subprocess]
  C[Held clock preload] --> B
  B --> D[Fixture preparation and tests]
  D --> E[Readiness gate files]
  E --> C
  B --> F[Diagnostics and process cleanup assertions]

Loading

Technical review

Best possible solution:

Reuse the existing held clock at fixture readiness boundaries while retaining real runner timeout, diagnostics, and process cleanup assertions.

Do we have a high-confidence way to reproduce the issue?

Source confirms that fixture startup competes with absolute deadlines and timeout allowances; the author reports repeated failures under two-CPU contention, but this review did not execute tests.

Is this the best way to solve the issue?

The change targets fixture readiness through the established clock helper and retains meaningful assertions against the real runner rather than replacing its behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 8316ff9031a7.

Provenance checked

• Concurrent waiting tests and slow-host margins changes intended behavior with a stated reason (aa49634: perf(test): wait out the ci-runner test's real runner limits side by side #3071 ran waiting tests concurrently while retaining margins intended to let slow hosts reach fixture readiness.)
• Preparation and cleanup limits keeps the original intent (84f182d: ci(runner): limit each file's preparation and cleanup #2314 added coverage that hung steps produce results and prevent later work from starting; the patch retains those outcomes.)
• Abandoned cleanup command termination keeps the original intent (b13be62: ci(runner): end abandoned steps' commands and bound teardown, cleanup and routing waits #2338 extended the cleanup test to verify command descendants terminate; the new atomic PID gate preserves that assertion.)
• Cancelled-job cleanup classification keeps the original intent (f890b9d: test(worker): harden the convergence deadline tests and split the revision file #2298 distinguished cancellation after parent-shell death from ordinary cleanup failure; the larger outer guard retains the SIGKILL and result assertions.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Verified patch scope: The introduced patch and verified test merge both change only the integration test file; the runner and existing clock helper are unchanged. (tests/integration/ci-runner.test.mjs, 99f8df97336d)
• Existing clock owner: The helper holds Date.now and global timeout callbacks until fixture gates exist, while recorded performance durations retain real elapsed time; this PR reuses that mechanism. (tests/helpers/ci-runner-held-clock.mjs:1, 99f8df97336d)
• Current main still has the race: The preparation test on fetched main starts its 5-second absolute deadline before launching the runner and does not wait for fixture readiness. (tests/integration/ci-runner.test.mjs:2986, 8316ff9031a7)
• Prior implementation and intent: fix(test): hold the ci runner's clock until deadline fixtures are in place #3149 established the same clock mechanism for two other deadline tests and explicitly identified the remaining waiting-test failures as outside its scope. (tests/helpers/ci-runner-held-clock.mjs:1, 7961d0952de5)
• Exact-head CI: CI Required, Checks and Conformance 2, Static Checks, and Suite Audit succeeded for the reviewed head; the changed test is registered in checks-baseline-2. (scripts/ci/test-suites/checks-baseline-2.json:160, 99f8df97336d)
• Inspection limits: No target code was executed. GitHub job-log retrieval redirected to a blocked Azure host, so leaf logs were not inspected; file-follow history reached an unavailable old object after the relevant recent history.

Likely related people:

• Free Wortley: Raw commit aa49634 adds tests/integration/ci-runner.test.mjs:2930 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: aa49634a9f95; files: tests/integration/ci-runner.test.mjs)

Review metrics

Metric	Value	Why it matters
Patch scope	Production +0/-0; tests +74/-24	The changes extend existing fixtures and reuse the existing clock helper without adding production machinery.

Labels

Label changes:

• add P2: The PR addresses reproducible CI fixture races under heavy host contention, with no product runtime change.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P2: The PR addresses reproducible CI fixture races under heavy host contention, with no product runtime change.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Complete the reported loaded before/after comparison to confirm that the repaired fixtures withstand the original contention scenario.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 21, 2026, 8:46 PM ET / October 22, 2026, 00:46 UTC.