test(pty): deflake agent_focus auto-review receipt test

[Hmbown]

Summary

macOS CI flake in agent_focus_pty::auto_review_gates_a_workers_call_and_the_receipt_shows_in_focus. The nextest FAIL block is:

Error: ← then Enter did not focus the worker
== frame 42x150 cursor=(40, 2) ==
  ▾ Subagents 1
  ✓ gate-worker  completed  ...
  ...
  ▌✓ done │ 2 turns · 5 steps │ ...  ← for agents · ↓ to manage
  ❯ Write a task or use /.

Seen on jobs 95242220506 (PR #5448), 95242912374 (PR #5438 run 31979001550), and reported on 95245484898 (PR #5445). Same dump on independent PRs (including docs-only); reruns pass.

Root cause

The sibling focus_a_worker_send_a_follow_up_and_return_to_main keeps workers running (40s mock hold) and does not flake. This test let the child's wrap-up return immediately, then slept 200ms between ← and Enter.

By the time those keys were sent the worker was already completed. Top placement collapses settled workers off the live strip (project_visible / agent_row_is_strip_settled). ← can still switch the rail to the Agents panel, but the 200ms gap is a window for completion redraws / clamp_viewport / collapse_strip to drop rail keyboard focus. Enter then hits the empty composer (no-op). After the CI-scaled 5s wait the dump is still the main conversation with ← for agents advertised.

wait_for_text("for agents") is also true from the moment the worker exists, so it does not mean the parent turn or the rail have settled.

This is a test race, not a product bug in Auto-Review. Focusing a completed worker via the Agents register remains a supported path (finished_agent_row_opens_its_transcript_and_alt_v_reaches_details); this journey just needs a live row, the same way the sibling does.

Fix

• Hold the child's wrap-up (not the bash call) for 40s so the guardian receipt exists and the worker stays running while we focus.
• Wait for a settled live row (✓ done, empty composer, gate-worker without completed) instead of a fixed sleep.
• Send ←+Enter back-to-back so Enter lands on the Agents panel.
• Assert the receipt as a condition (Auto-Review allowed 'bash' and model guardian) on the focused frame.

Testing

• cargo fmt --all -- --check
• cargo clippy -p codewhale-tui --test pty -- -D warnings
• cargo clippy --workspace --all-targets --all-features --locked
• cargo test --workspace --all-features --locked

Verification

RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --test pty -- auto_review_gates_a_workers_call_and_the_receipt_shows_in_focus
# 15-iteration bash for-loop: pass=15 fail=0
# individual runtimes ~1.2–3.5s after compile (loop 1 was 9.34s under a cargo lock)

RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --test pty -- agent_focus_pty
# 2 passed (focus_a_worker_send_a_follow_up_and_return_to_main + auto_review_...)

Not verified: macOS GitHub Actions under full-workspace nextest load; workspace clippy/test gate.

Refs #5056 #5403
No-Issue: CI flake fix tracked under #5056

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @Hmbown's task in 1m 53s —— View job

---

Reviewing PR #5457

• Gather context (PR description, diff, repo guidance)
• Read changed file crates/tui/tests/pty/agent_focus_pty.rs
• Review for correctness bugs, race conditions, flake risk
• Review for provider/model/route safety (N/A — test-only PTY change)
• Review for reuse/simplification
• Review test coverage
• Post final review

---

Review

Test-only change (crates/tui/tests/pty/agent_focus_pty.rs), no product code touched. The fix targets a real race and the reasoning in the PR body checks out against the code.

Correctness

• The core fix is sound. focus_listed_worker (lines 284–308) replaces the fixed 200ms sleep between ← and Enter with harness.wait_for(|frame| text.contains(worker) && !text.contains("completed"), ...) (lines 289–295) before sending the chord as one write (chord.extend(keys::key::enter()), lines 296–298). That directly addresses the described race: sending both keys back-to-back removes the window where a completion redraw can drop rail focus between the two key events.
• child_hold: Duration::from_secs(40) (GateResponder, line 434/544) now holds the wrap-up response instead of the bash call, so the guardian receipt is written (via the allowed bash call) while the worker row is still running. This matches the sibling test's already-passing pattern (ProbeResponder.child_hold, line 148/322) — good, mirrors a proven-stable approach rather than inventing a new one.
• Minor robustness nit: !text.contains("completed") at line 292 checks the whole frame's text, not just the gate-worker row. With a single worker in this test that's safe today, but it's a global substring check rather than a row-scoped one — if this helper is reused later for a multi-worker journey, "completed" appearing anywhere else on screen (e.g. a second worker's row) would block the wait incorrectly. Not a bug in the current test, just something to watch if focus_listed_worker gets reused.
• harness.wait_for_text("✓ done", ...) (line 288) is worth double-checking: the flake dump in the PR description shows "✓ done" present in the failing frame too (it's the parent-turn footer status, not the worker-row state). So this particular wait doesn't by itself distinguish good/bad timing — the actual fix is the subsequent row-state predicate. Not incorrect, just less load-bearing than it might look; the docstring above the function could make this explicit so a future reader doesn't assume "✓ done" alone is what prevents the race.

Reuse / duplication

• focus_a_worker_send_a_follow_up_and_return_to_main (lines 349–356) keeps the old inline ← + sleep(200ms) + Enter sequence rather than adopting focus_listed_worker. The PR body explains why (that test holds the worker running for its full duration, so it doesn't hit the race) — reasonable, but it does leave two slightly different focus-key sequences in the same file. Could be worth a short comment at the sibling's call site noting it intentionally still uses the old inline path (and why), so it doesn't read as an oversight next time someone touches this file.

Tests

• No new test cases added — this is purely deflaking an existing one, which matches the PR's stated scope. The verification notes (15/15 local loop passes, cargo fmt and targeted clippy clean) are reasonable evidence for a timing fix; the outstanding items are full-workspace clippy/test and macOS CI, which are appropriately left unchecked in the PR checklist pending CI.

Provider/model/route safety

Not applicable — no model/provider selection logic in this diff.

Summary

Solid, well-reasoned flake fix with a clear root-cause explanation matching the code changes. Only minor, non-blocking suggestions above (row-scoped "completed" check if reused, and clarifying which wait actually prevents the race).