Repository navigation
Requeue unapplied steers synchronously when a forwarder guard drops - #462
Conversation
Added a forwarder that drains the run queue and hands each entry to the executor, so queued runs are actually started instead of sitting idle. Tests cover the forwarding path and the module now exports the new component. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Update the forwarder's doc comments to describe the current design: the poll task drains the queue and delivers under the shutdown gate, so an item is always either in the queue or in the handle and never in flight. The requeue is synchronous, so hosts can read leftover steers as soon as the guard drops. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a README describing the run queue's purpose and how it schedules work within the harness. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document that SteeringForwarderGuard now drains and delivers under its shutdown gate and requeues residual steers synchronously in Drop, so a host can deterministically collect a turn's leftovers. Also note that RunQueue guards its lanes with a std::sync::Mutex and gains synchronous push_now and drain_now while its async methods stay unchanged. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Tiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Incomplete Review snapshot
Completeness: Incomplete Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Could not review: CHANGELOG.md, tinysweeper/description Before merge
How this fits togetherflowchart LR
n0["..._keeps_returned_items_ahead_of_newer_ones<br/>changed"]:::changed
n1["...next_model_boundary_as_additional_context"]:::impacted
n2["recorder"]:::impacted
n3["to_string"]:::impacted
n4["...runs_cleanup_once_and_stops_the_poll_loop"]:::impacted
n5["...ects_framed_messages_and_reports_delivery"]:::impacted
n0 -->|calls| n3
n0 -->|tests| n3
n1 -->|calls| n2
n1 -->|tests| n2
n4 -->|calls| n2
n4 -->|tests| n2
n4 -->|calls| n3
n4 -->|tests| n3
n5 -->|calls| n2
n5 -->|tests| n2
n5 -->|calls| n3
n5 -->|tests| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinyagents-harness/src/run_queue/forwarder.rs, crates/tinyagents-harness/src/run_queue/forwarder_tests.rs, crates/tinyagents-harness/src/run_queue/mod.rs, tinysweeper/description.
$0.0012 · 100,336 in / 11,289 out · 2,368 cached (2%) · ladder/vectors-oai3, glm-5.3-flash, deepseek-v4.1-flash · 604 embedded
critique: $0.0003 · 26,659 in / 647 out · 0 cached (0%) · glm-5.3-flash
tests: $0.0007 · 66,028 in / 7,019 out · 2,368 cached (4%) · glm-5.3-flash
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: CHANGELOG.md, tinysweeper/description.
$0.0009 · 64,957 in / 2,657 out · 0 cached (0%) · ladder/vectors-oai3, glm-5.3-flash, deepseek-v4.1-flash · 604 embedded
critique: $0.0005 · 29,164 in / 499 out · 0 cached (0%) · glm-5.3-flash
tests: $0.0002 · 17,990 in / 11 out · 0 cached (0%) · glm-5.3-flash
Summary
A host that feeds a running turn from a
RunQueuethroughSteeringForwarderGuardcould not reliably find out which steers the run never applied. When the guard dropped, it requeued residual (delivered-but-unapplied) steers on a spawned task. The poll loop could also be holding a drained batch when the gate closed, and it pushed that batch back later from its own task. So for a short window after a turn ended, a steer could be in neither the queue nor the handle, and a host reading the queue straight away could miss it.This change removes that window:
RunQueuenow guards its lanes with astd::sync::Mutex. The lock is held for a single vector operation and never across an.await. The queue also gets synchronouspush_nowanddrain_now.Droprequeues residual steers synchronously. Once the guard has dropped, every steer the run did not apply is back in the queue.The loop already continued past a natural finish when a steer was pending (
settle_natural_finish, covered bysteer_arriving_after_the_final_answer_is_applied_before_any_followup), so the loop itself is unchanged.API Or Behavior Changes
RunQueue::push_now(&self, QueueLane, T)andRunQueue::drain_now(&self, QueueLane) -> Vec<T>.SteeringForwarderGuard::dropreturns, not on a spawned task. The old "no tokio runtime at drop" path, which lost steers, is gone. Requeued items are still appended behind anything already queued.RunQueue'sasyncmethods keep their signatures.Tests
run_queue/forwarder_tests.rs:residual_steers_are_back_in_the_queue_as_soon_as_the_guard_dropsfails on the old spawned requeue and passes with this change.a_steer_queued_while_the_guard_closes_stays_in_the_queue.cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace: everything passes except threeproviders::claude_code::drivertests. Those fail in my local sandbox because it refuses to exec a script from the temp dir (Operation not permitted). They don't touch the run queue.Documentation
run_queue/README.md(public surface, operational constraints), the forwarder module docs andCHANGELOG.mdare updated.Summary by CodeRabbit