Harden flaky integration tests: poll for state instead of fixed sleeps - #14
Merged
Conversation
CI's integration job has been flaky across recent runs, failing on a different script each time. Root cause: several scripts assert a supervisor state change (limit->wait->inject, detach, kill) after a fixed `sleep N`, racing the real timing budget (poll_interval + reset wait + buffer + process/IO overhead), which gets tight under a loaded CI runner. Reproduced the integration_attach.sh failure locally under simulated CPU load (5/10 failures on "normal attach injected the resume prompt" with the old fixed sleep 7 vs. 0/15 with the new polling loop, and 0/10 clean). Could not reproduce the integration_m2_autodetach.sh failure despite 60+ local runs (clean, CPU-stressed, and concurrent), so it's hardened defensively: require the simulated attach to be seen on two consecutive checks before treating it as settled, widen the polling ceilings, and print extra diagnostics (ps/tmux state) on failure to make any future recurrence debuggable. Also fixed the same fixed-sleep-racing-a-state-change pattern in integration.sh, integration_codex.sh, integration_reprompt.sh (sleep 14 before checking the injected marker) and integration_m2.sh (sleep 5 after detach/stop --kill), all replaced with bounded polling loops. integration_pty.sh and integration_dead_session.sh already polled correctly and were left alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CI's "integration (tmux + pty)" job has been flaky across recent runs on
main, failing on a different script each time (different assertions, same shape of problem).Root cause: several
test/integration_*.shscripts assert a supervisor state change (limit detected → wait → inject, or detach/kill) after a fixedsleep N, racing the real timing budget (poll_interval+ reset wait + buffer + process/IO overhead). That budget gets tight under a loaded CI runner.test/integration_attach.sh's fixedsleep 7reproduced the exact reported failure ("normal attach injected the resume prompt") — 5/10 failures under simulated CPU load with the old code, 0/15 with the new polling loop (plus 0/10 clean). Replaced with a bounded poll of the marker file.test/integration.sh,test/integration_codex.sh,test/integration_reprompt.sh(all hadsleep 14before checking an injected marker) andtest/integration_m2.sh(sleep 5afterdetach/stop --kill) — all converted to bounded polling loops.test/integration_m2_autodetach.shalready polled (from an earlier flake fix) but still failed once onmain. Could not reproduce despite 60+ local runs (clean, CPU-stressed, concurrent). Hardened defensively anyway: require the simulated tmux attach to be seen on two consecutive checks 100ms apart (guards a hypothetical flapping attach), widened both polling ceilings, and added diagnostics (ps,tmux list-clients) on failure so any future recurrence is debuggable. Being upfront: this one is not a verified fix, since the failure never reproduced locally — just defensive hardening plus better failure diagnostics.test/integration_pty.shandtest/integration_dead_session.shalready used correct polling and were left untouched.No Go source changed — this is test-script-only.
Test plan
test/run_all.sh: 5/5 clean runs, 40/40 individual script passes (WSL + real tmux)go build ./...,go vet ./...,gofmt -l .clean (no Go files touched)mainover the next several CI runs forintegration_m2_autodetach.shspecifically, since that one's fix is unverified🤖 Generated with Claude Code