(MOT-4202) fix(harness): reseed a turn when the finalize drain delivers a parked message - #577
Conversation
… message A notification or steer that parked during a turn's final step was drained to the transcript by the finalize drain but had no turn to react to it, so an autonomous run that ended its turn expecting the fire to wake it would stall. finalize_completed/finalize_failed now seed a fresh turn when the finalize drain delivered a model-visible row; custom-only rows never reseed and a cancelled turn drains but does not reseed. Gated on the drain actually delivering a row, so a redelivered finalize cannot double-seed and a racing external send is resolved by run_step's stale-turn guard.
…ed (E2E-003) A message parked during a completing terminal generation is always delivered by the turn loop's steering check, so the completed-finalize drain cannot be reached deterministically from the public boundary. The failed finalize has no steering check and shares the same drain-and- reseed, so E2E-003 steers a message mid-generation and then fails that generation with a scripted router error: the failed finalize drains the parked row and must reseed a fresh turn, pinned by generation 2 matching a new turn's :0 request over [user, empty assistant residue, parked follow-up] and by the follow-up keeping its durable e_q_ queue entry id. With the reseed disabled the scenario times out. Framework support: scripted generation failures (fails()/failure), declared per-turn terminal statuses enforced positionally by the floor, driver-derived expected trace count (harness-seeded turns chain into the originating send's trace), and latest-terminal-turn evidence binding for multi-turn direct scenarios.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 48 skipped (no docs/).
Four for four. Nicely done. |
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe turn loop now conditionally reseeds after finalization drains model-visible parked messages. The E2E harness adds serve-time steering, scripted failures, ordered terminal-status expectations, and scenario E2E-003 coverage for a failed turn followed by a reseeded completed turn. ChangesParked message reseed flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScriptedRouter
participant IIIClient
participant harness_send
participant turn_loop
ScriptedRouter->>IIIClient: execute on_serve steer
IIIClient->>harness_send: send parked follow-up
harness_send->>turn_loop: queue notification during finalization
turn_loop->>turn_loop: drain model-visible parked row
turn_loop->>turn_loop: reseed next turn
turn_loop-->>ScriptedRouter: run reseeded generation
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
andersonleal
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
harness/src/turn_loop.rs (1)
1073-1098: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve the wake signal after a partial drain.
If a model-visible row appends successfully but a later append or
delete_queuedfails, this returnsErr;drain_queued_best_effortconverts that tofalse, so no turn is reseeded even though input reached the transcript. Track successfully appended model-visible rows through the loop and return that outcome despite a subsequent best-effort error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/src/turn_loop.rs` around lines 1073 - 1098, The drain_queued flow currently loses the wake signal when a later append or delete fails after model-visible input was appended. Track successfully appended model-visible rows within drain_queued, and ensure drain_queued_best_effort preserves and returns a true outcome when any such row reached the transcript, even if a subsequent operation returns an error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harness/src/turn_loop.rs`:
- Line 1286: Make terminalization atomic with the final queue drain and reseed
decision in the turn-loop completion path around drain_queued_best_effort and
the failed-finalization path at harness/src/turn_loop.rs:1286-1286 and
harness/src/turn_loop.rs:1337-1337. Coordinate both paths with harness::send
using a shared session lock or equivalent CAS/recheck protocol so no message can
be accepted as queued after the final sweep without triggering a reseed;
preserve the existing woke behavior once the terminal transition and queue
decision are linearized.
In `@harness/tests/e2e/src/scripted_router.rs`:
- Around line 462-482: Update perform_serve_effect to wrap the client.trigger
call in the same outer tokio::time::timeout used by Client::call_with_timeout,
preserving DEFAULT_CALL_TIMEOUT_MS as the timeout duration. Handle timeout
errors through the existing Error::Handler path so an unconnected steer cannot
remain parked indefinitely.
---
Outside diff comments:
In `@harness/src/turn_loop.rs`:
- Around line 1073-1098: The drain_queued flow currently loses the wake signal
when a later append or delete fails after model-visible input was appended.
Track successfully appended model-visible rows within drain_queued, and ensure
drain_queued_best_effort preserves and returns a true outcome when any such row
reached the transcript, even if a subsequent operation returns an error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f23e6385-30a2-4c77-ba38-4c6117a674b3
📒 Files selected for processing (13)
harness/src/functions/send.rsharness/src/turn_loop.rsharness/tests/e2e/README.mdharness/tests/e2e/src/fixtures/loading.rsharness/tests/e2e/src/fixtures/tests.rsharness/tests/e2e/src/scenario/floor.rsharness/tests/e2e/src/scenario/phases/completion.rsharness/tests/e2e/src/scenario/phases/evidence.rsharness/tests/e2e/src/scenarios/dsl.rsharness/tests/e2e/src/scenarios/mod.rsharness/tests/e2e/src/scenarios/reseed_parked_message.rsharness/tests/e2e/src/scripted_router.rsharness/tests/e2e/src/types/script.rs
…al sweep Review follow-up (CodeRabbit). A send racing a finalize could enqueue after the finalize drain and recheck the turn record before the terminal write landed, concluding queued against a turn that would never drain again. A second drain sweep after the terminal put_turn pairs with try_enqueue's post-enqueue recheck: a recheck that still saw Running implies the row landed before the sweep; a recheck that sees the terminal record seeds its own turn. Applied to completed, failed, and cancelled finalize (the latter still drain-only). Also wrap the scripted router's steer trigger in the same outer timeout Client::call_with_timeout uses, so a connection that never establishes cannot park the chat handler.
1 participant
Problem
A message that parks while a turn's final step is in flight is drained into the transcript by
finalize_completed/finalize_failed, but no turn is ever seeded to react to it. An autonomous run that ends its turn expecting a notification to wake it strands forever — observed live 2026-07-22: a scan coordinator's child-completion notification landed in the finalize window and the session went permanently idle.Fix
drain_queued_best_effortnow reports whether it delivered a MODEL-VISIBLE row (custom-role status rows don't count — regenerating over an assistant-tailed context is a guaranteed provider prefill rejection). When it did, the finalizing turn reseeds a fresh turn viasend::seed_new, reusing the finalized turn's frozen options and registry generation — the same outcome an externalharness::sendproduces against a now-terminal session. Applies tofinalize_completedandfinalize_failed;finalize_cancelleddrains without reseeding (the user cancelled). The reseed runs after the terminalput_turn, since the turn slot is keyed per session.Coverage
finalize_reseed_gate_counts_only_model_visible_rowspins the reseed gate (custom-only queue must not wake a turn).reseed-parked-message): deterministic public-path regression test. The completed finalize can't be pinned externally — a park during a completing terminal generation is always delivered by the steering check first — so the scenario routes through the failed finalize (no steering check, same drain-and-reseed): generation 1 steers a message mid-generation (it parks durably) and then fails; the finalize drain delivers the parked row and the reseeded turn's:0request over[user, empty assistant residue, parked follow-up]is matched exactly, with the follow-up keeping its durablee_q_queue entry id. Verified both directions: passes with the fix (~3.3s), times out with the reseed disabled.E2E-001/002/003 pass,
harness-integrationunit tests and clippy-D warningsclean, all 5 fixtures validate.Summary by CodeRabbit
New Features
Bug Fixes
Documentation