Skip to content

Stop harness waits failing on a slow machine - #227

Merged
JacobStephens2 merged 2 commits into
mainfrom
issue-200
Oct 1, 2026
Merged

JacobStephens2 merged 2 commits into
mainfrom
issue-200

Conversation

@JacobStephens2

Copy link
Copy Markdown
Owner

Summary

The harness's two waits on a Run, for the fake agent to start (Scenario::run_and_signal) and for a prompt on the terminal (Scenario::run_on_terminal), no longer give up after a fixed 10s.

 wait for the Run to reach a point (agent started / prompt shown)
   loop
+    exited = has the Run's process exited?
     if the point is reached: stop waiting
+    if exited: fail now, with the Run's stderr / what the terminal shows
-    if 10s have passed: fail
+    if WAIT_BOUND (120s) has passed: fail
     sleep

Three sibling timings in the tests went the same way:

  • tests/failed_run.rs, tests/spec_run.rs: the interrupt tests asserted the whole Run took under 20s, which now includes however long the agent took to start. They assert instead that the session's script never got past its sleep 30.
  • tests/spec_run.rs: sessions that wait in bash for another Ticket's session to start waited 10s; they now wait WAIT_BOUND.

Seams under test: Scenario::run_and_signal and Scenario::run_on_terminal, in the new tests/harness.rs.

Evidence

  • Before: a Run that exits in Pre-flight (issue Numbered Issue branches after a merged or closed PR #7 closed) while the test waits for the agent:
    waiting_for_an_agent_that_never_starts_fails_with_the_runs_stderr_once_it_exits ... FAILED
    panicked at tests/support/mod.rs:259:13: the agent never started
    finished in 11.36s
    
    After: it fails at once with "the agent never started: the Run exited first; stderr: ... issue Numbered Issue branches after a merged or closed PR #7 is closed", and the same for a prompt the terminal never shows:
    test waiting_for_an_agent_that_never_starts_fails_with_the_runs_stderr_once_it_exits ... ok
    test waiting_for_a_prompt_that_never_shows_fails_with_the_terminal_once_the_run_exits ... ok
    test result: ok. 2 passed; finished in 0.12s
    
  • Slow but progressing Run: a throwaway test (not committed) whose agent sleeps 12s before touching its started marker passed in 12.50s; the old 10s deadline would have failed it.
  • Under load: with 24 busy loops on the 4 cores (load average climbing past 15), ctrl_c_after_the_merge_finishes_the_post_merge_steps_and_exits_as_merged passed three times out of three. This stood in for "cargo test in two other checkouts", which was not run.
  • Full suite: cargo test passed, 173.81s wall including test compilation, with load average about 4 from other sessions, so there is no clean idle before/after comparison. The new tests/harness.rs adds 0.17s; nothing else adds time. The full suite ran before the review fixes; after them, harness, failed_run and spec_run (every file the fixes touched) were rerun and passed, and cargo clippy --all-targets is clean.

Unaddressed findings

Standards

  • Duplicated Code: the two wait loops in tests/support/mod.rs share a shape. Skipped: on exit one collects piped output and the other drains the terminal before searching it, so a shared helper would take a callback per step and read worse than two short loops.
  • Duplicated Code: the same interruptible agent script in tests/failed_run.rs and tests/spec_run.rs. Skipped: the duplication predates this change, and one sets the script for every session while the other sets it for one Ticket.
  • Duplicated Code: SETUP_OFFER in tests/harness.rs repeats OFFER in tests/setup_offer.rs. Skipped: each test file is its own crate, and the harness module is not the place for one prompt's wording.
  • let exited = loop { let exited = ... } shadows itself. Skipped: both hold the same fact, and the inner one is the value the loop breaks with.

Spec

  • No committed test for "a wait no longer fails on a slow but progressing Run". Skipped: such a test has to outlast the old 10s deadline, adding over 10s to a suite the issue says must take no longer. Checked by hand instead (see Evidence).
  • Scope: the 20s elapsed assertions and the in-script 10s waits are not harness helpers. Kept: with a 120s bound on the agent starting, the 20s assertions would fail on a slow machine in place of "the agent never started", and the in-script waits are the same fixed 10s wait for a session to start.
  • The in-script waits cannot tell the other session's Run has exited, so a real regression there takes two minutes per session to fail, not 10s. Skipped: a bash script inside one session has no handle on another Ticket's Run; the cost falls only on a failing test.
  • After the Run exits, reader.join() and wait_with_output() have no deadline if a descendant still holds the terminal or the pipe. Skipped: the harness already made both calls unbounded at the end of each helper, and a Run that exits before starting an agent leaves no descendants.
  • A Run that exits just after the marker appears gets no signal, so a signal test could pass unsignalled. Skipped: this matches the old behaviour, where the signal went to a zombie and did nothing; it is now documented on run_and_signal.
  • "Passes with cargo test running in two other checkouts" and "takes no longer on an idle machine" not verified as written. See Evidence for what was run instead.

Merge Danger

Door: two-way

Tests only; no change to the binary.

Blast Radius: tests

A Run that truly hangs before reaching the awaited point now holds its test for 120s, not 10s, before failing.

Closes #200

🤖 Generated with Claude Code

JacobStephens2 and others added 2 commits October 1, 2026 01:47
A test that waits for a Run to start the agent, or to show a prompt on
the terminal, now fails as soon as the Run exits without getting there,
with the Run's stderr, and otherwise waits up to two minutes instead of
ten seconds.

The interrupt tests prove the session was stopped by a marker its script
would write after its sleep, not by how long the test took, and the
Spec run sessions that wait for each other allow two minutes too.

Closes #200

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The harness tests assert the wait failed because the Run exited, not how
long it took. The Spec run scripts derive their polling from WAIT_BOUND,
and the harness helpers document when they panic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@JacobStephens2
JacobStephens2 merged commit e913d2a into main Oct 1, 2026
13 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-200 branch October 1, 2026 01:56
@JacobStephens2 JacobStephens2 mentioned this pull request Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harness waits fail under load on a fixed 10s "the agent never started" deadline

1 participant