From ab2ca9b4f50febc5c8a6a57cf97d917201cbad20 Mon Sep 17 00:00:00 2001 From: Aman Jaiman Date: Sat, 4 Jul 2026 19:26:11 -0400 Subject: [PATCH] Harden flaky integration tests: poll for state instead of fixed sleeps 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 --- test/integration.sh | 16 ++++++++++++++-- test/integration_attach.sh | 24 +++++++++++++++++++++++- test/integration_codex.sh | 15 ++++++++++++++- test/integration_m2.sh | 15 +++++++++++++-- test/integration_m2_autodetach.sh | 29 +++++++++++++++++++++++------ test/integration_reprompt.sh | 16 +++++++++++++++- 6 files changed, 102 insertions(+), 13 deletions(-) diff --git a/test/integration.sh b/test/integration.sh index 74bf6d6..41dfa47 100644 --- a/test/integration.sh +++ b/test/integration.sh @@ -43,8 +43,20 @@ echo "== launching supervisor against real tmux ==" "$BIN" run --agent fake --name "$SESSION" --config "$CFG" --prompt "continue" -- "$AGENT" & SUP=$! -# Give it time to: detect (1s) + wait (~6s+1s buffer) + inject + echo. -sleep 14 +# Poll for the limit -> wait -> inject -> echo cycle to complete instead of a +# fixed sleep: the cycle is detect (1 poll) + wait (~6s reset + 1s buffer) + +# inject + the agent echoing it back, and a fixed duration raced this too +# tightly under a loaded/CI runner. Ceiling is generous; the marker or an early +# supervisor exit both end the wait promptly. +for _ in $(seq 1 40); do + if grep -q "continue" "$MARKER" 2>/dev/null; then + break + fi + if ! kill -0 "$SUP" 2>/dev/null; then + break + fi + sleep 0.5 +done echo "== final pane ==" tmux capture-pane -p -t "$SESSION" 2>/dev/null || echo "(session gone)" diff --git a/test/integration_attach.sh b/test/integration_attach.sh index 2d20752..cf2043e 100644 --- a/test/integration_attach.sh +++ b/test/integration_attach.sh @@ -43,7 +43,29 @@ sleep 2 echo "== attach-existing (normal) on the same live session SHOULD inject ==" "$BIN" attach-existing --agent fake --target "$SESSION" --config "$CFG" --no-notify --prompt real-prompt >/tmp/ak_inj.log 2>&1 & INJ=$! -sleep 7 + +# The fake agent emits its limit line ~1s after start with a reset ~6s later, +# and poll_interval here is 1s (see $CFG above), so the supervisor needs at +# least a few poll cycles after the reset to notice, wait out the buffer, and +# inject. A fixed `sleep 7` raced this tightly on a loaded/CI runner (the +# prompt sometimes hadn't landed yet), so poll for the marker instead of +# guessing a duration — matching the pattern in integration_m2_autodetach.sh. +injected=0 +for _ in $(seq 1 60); do + if grep -q "real-prompt" "$MARKER" 2>/dev/null; then + injected=1 + break + fi + if ! kill -0 "$INJ" 2>/dev/null; then + # process exited early (e.g. crashed) — no point polling further + break + fi + sleep 0.5 +done +if [ "$injected" -ne 1 ]; then + echo " FAIL: resume prompt not seen in marker after 30s of polling" +fi + kill "$INJ" 2>/dev/null; wait "$INJ" 2>/dev/null check "normal attach injected the resume prompt" 'grep -q "real-prompt" "$MARKER" 2>/dev/null' diff --git a/test/integration_codex.sh b/test/integration_codex.sh index a4f45db..a092f1d 100644 --- a/test/integration_codex.sh +++ b/test/integration_codex.sh @@ -38,7 +38,20 @@ chmod +x "$AGENT" echo "== launching supervisor with --agent codex ==" "$BIN" run --agent codex --name "$SESSION" --config "$CFG" --prompt "codex-continue" -- "$AGENT" >/tmp/ak_codex.log 2>&1 & SUP=$! -sleep 14 + +# Poll for the limit -> wait -> inject -> echo cycle instead of a fixed sleep +# (detect + ~6s reset + 1s buffer + inject can lag noticeably on a loaded/CI +# runner). Ceiling is generous; the marker+log or an early supervisor exit +# both end the wait promptly. +for _ in $(seq 1 40); do + if grep -q "codex-continue" "$MARKER" 2>/dev/null && grep -qi "source=relative" /tmp/ak_codex.log 2>/dev/null; then + break + fi + if ! kill -0 "$SUP" 2>/dev/null; then + break + fi + sleep 0.5 +done echo "== final pane ==" tmux capture-pane -p -t "$SESSION" 2>/dev/null || echo "(session gone)" diff --git a/test/integration_m2.sh b/test/integration_m2.sh index 85f3dfe..90b1dd7 100644 --- a/test/integration_m2.sh +++ b/test/integration_m2.sh @@ -34,7 +34,13 @@ check "status shows RUNNING" '"$BIN" status | grep -q RUNNING' echo "== detach via command ==" "$BIN" detach --name "$N1" -sleep 5 +# Poll instead of a fixed sleep: detach goes through the control-file poller +# (pollControl) plus the supervisor's own poll_interval, so a fixed duration +# can race this on a loaded/CI runner. Ceiling is generous. +for _ in $(seq 1 20); do + stopped "$SUP1" && break + sleep 0.5 +done check "supervisor exited after detach" 'stopped "$SUP1"' check "session still alive after detach" 'tmux has-session -t "$N1" 2>/dev/null' check "status now shows DETACHED" '"$BIN" status --name "$N1" | grep -q DETACHED' @@ -44,7 +50,12 @@ echo "== stop --kill on a second instance ==" SUP2=$! sleep 3 "$BIN" stop --name "$N2" --kill -sleep 5 +# Same rationale as the detach poll above: give the control-file poller and +# supervisor poll_interval a generous, polled ceiling instead of a fixed sleep. +for _ in $(seq 1 20); do + stopped "$SUP2" && break + sleep 0.5 +done check "supervisor-2 exited after kill" 'stopped "$SUP2"' check "session-2 terminated by kill" '! tmux has-session -t "$N2" 2>/dev/null' diff --git a/test/integration_m2_autodetach.sh b/test/integration_m2_autodetach.sh index f729faa..167429e 100644 --- a/test/integration_m2_autodetach.sh +++ b/test/integration_m2_autodetach.sh @@ -29,27 +29,44 @@ ATT=$! # anything about the supervisor's reaction — on a loaded/CI runner both the # `script` pty attach and the supervisor's next poll can lag noticeably past a # fixed sleep, which made this flaky. Poll instead of guessing a duration. +# +# Require the client to be seen on two consecutive checks (100ms apart) so a +# transient/flapping attach (script reattaching, or a race in when the pty is +# fully set up) doesn't get counted as "attached" before it has really settled. attached=0 -for _ in $(seq 1 20); do +for _ in $(seq 1 40); do if [ -n "$(tmux list-clients -t "$N" 2>/dev/null)" ]; then - attached=1 - break + sleep 0.1 + if [ -n "$(tmux list-clients -t "$N" 2>/dev/null)" ]; then + attached=1 + break + fi fi sleep 0.5 done if [ "$attached" -ne 1 ]; then - echo " FAIL: tmux never saw an attached client (script/tmux attach didn't take)" + echo " FAIL: tmux never saw a sustained attached client (script/tmux attach didn't take)" echo "---- script log ----"; cat /tmp/ak_auto_script.log + echo "---- tmux list-clients (final) ----"; tmux list-clients -t "$N" 2>&1 fail=1 fi -# Give the supervisor a few poll cycles to notice and detach. -for _ in $(seq 1 20); do +# Give the supervisor several poll cycles (default poll_interval is 3s; budget +# a generous multiple of that plus process/IO overhead on a loaded runner) +# to notice and detach. +detached=0 +for _ in $(seq 1 60); do if ! kill -0 "$SUP" 2>/dev/null; then + detached=1 break fi sleep 0.5 done +if [ "$detached" -ne 1 ]; then + echo " FAIL: supervisor (pid $SUP) never exited after 30s of polling" + echo "---- ps of supervisor tree ----"; ps -ef 2>/dev/null | grep -i sleeperagent + echo "---- tmux list-clients (final) ----"; tmux list-clients -t "$N" 2>&1 +fi check "supervisor auto-detached" '! kill -0 "$SUP" 2>/dev/null' check "session still alive (handed to user)" 'tmux has-session -t "$N" 2>/dev/null' diff --git a/test/integration_reprompt.sh b/test/integration_reprompt.sh index ca52ade..a2f1819 100644 --- a/test/integration_reprompt.sh +++ b/test/integration_reprompt.sh @@ -70,7 +70,21 @@ chmod +x "$AGENT" echo "== launching with --reprompt ollama:test ==" "$BIN" run --agent fake --name "$SESSION" --config "$CFG" --reprompt ollama:test -- "$AGENT" >/tmp/ak_rp.log 2>&1 & SUP=$! -sleep 14 + +# Poll for the limit -> wait -> reprompt-generate -> inject -> echo cycle +# instead of a fixed sleep (the extra reprompt/LLM round trip on top of the +# usual detect+wait+inject timing makes a fixed duration even more likely to +# race under a loaded/CI runner). Ceiling is generous; the marker or an early +# supervisor exit both end the wait promptly. +for _ in $(seq 1 40); do + if grep -qF "$LLM_TEXT" "$MARKER" 2>/dev/null; then + break + fi + if ! kill -0 "$SUP" 2>/dev/null; then + break + fi + sleep 0.5 +done kill "$SUP" 2>/dev/null; wait "$SUP" 2>/dev/null echo "== supervisor log =="; cat /tmp/ak_rp.log