Skip to content

Requeue unapplied steers synchronously when a forwarder guard drops - #462

Merged
senamakel merged 5 commits into
mainfrom
embed-turn-steering
Oct 15, 2026
Merged

senamakel merged 5 commits into
mainfrom
embed-turn-steering

Conversation

@senamakel

@senamakel senamakel commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Summary

A host that feeds a running turn from a RunQueue through SteeringForwarderGuard could 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:

  • RunQueue now guards its lanes with a std::sync::Mutex. The lock is held for a single vector operation and never across an .await. The queue also gets synchronous push_now and drain_now.
  • The forwarder's poll loop drains a lane and delivers it while holding the shutdown gate. An item is therefore always either in the queue or in the handle, and once the gate is closed the loop takes nothing more.
  • Drop requeues 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 by steer_arriving_after_the_final_answer_is_applied_before_any_followup), so the loop itself is unchanged.

API Or Behavior Changes

  • Added: RunQueue::push_now(&self, QueueLane, T) and RunQueue::drain_now(&self, QueueLane) -> Vec<T>.
  • Behavior: residual steers are requeued before SteeringForwarderGuard::drop returns, 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's async methods keep their signatures.

Tests

  • New regression tests in run_queue/forwarder_tests.rs:
    • residual_steers_are_back_in_the_queue_as_soon_as_the_guard_drops fails on the old spawned requeue and passes with this change.
    • a_steer_queued_while_the_guard_closes_stays_in_the_queue.
  • cargo fmt --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace: everything passes except three providers::claude_code::driver tests. 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 and CHANGELOG.md are updated.

Summary by CodeRabbit

  • Reliability
    • Pending steering messages are returned to the queue when forwarding shuts down, so they remain available for collection.
  • New Features
    • Added synchronous queue operations for immediately adding and retrieving messages, alongside the existing async operations.
  • Documentation
    • Updated queue documentation to describe the synchronous operations and shutdown behavior.

senamakel and others added 4 commits October 15, 2026 10:36
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>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d7196a62-3e67-4fa1-9e1f-488e9c7bde0a
📥 Commits

Reviewing files that changed from the base of the PR and between 478fd49 and 7c1a718.

📒 Files selected for processing: 5
  • CHANGELOG.md
  • crates/tinyagents-harness/src/run_queue/README.md
  • 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

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Walkthrough

RunQueue now uses a standard mutex and provides synchronous push and drain methods. Forwarding drains and delivers under the shutdown gate. Guard drop synchronously requeues residual steers, with tests covering two shutdown states.

Changes: Run Queue and Forwarder Shutdown

Layer / File(s) Summary
Synchronous RunQueue operations
crates/tinyagents-harness/src/run_queue/mod.rs, crates/tinyagents-harness/src/run_queue/README.md
RunQueue now uses std::sync::Mutex for queue operations. It adds synchronous push_now and drain_now methods.
Forwarding under the shutdown gate
crates/tinyagents-harness/src/run_queue/forwarder.rs
forward_lane checks the shutdown gate, drains the lane, and delivers messages within the same lock scope. Its callers no longer await it.
Synchronous residual requeue
crates/tinyagents-harness/src/run_queue/forwarder.rs, crates/tinyagents-harness/src/run_queue/forwarder_tests.rs, crates/tinyagents-harness/src/run_queue/README.md, CHANGELOG.md
Guard drop synchronously requeues residual messages. Tests check that delivered-but-unapplied and still-queued steers remain available after guard drop.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 7c1a7

The change makes unapplied steers available in the run queue as soon as the forwarder guard drops, and it keeps a closing shutdown gate from stranding drained messages. No concrete merge-blocking risk was identified in the supplied context.

Security architecture risk: ⚪ Minimal · up to 7c1a7

The change makes unapplied messages available immediately after shutdown without expanding who can submit or consume them. The reviewed ownership and permission checks remain intact, and no material security regression was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed authority remains bounded by the queue and steering-handle references supplied by the host. The synchronous APIs do not discover other queues, select tenants, or acquire additional run authority. Hosts can already share queues, so queue ownership is not itself a tenant-isolation guarantee; deployment-wide exposure depends on host attachment choices.

Trust Boundaries and Controls

  • observed — The existing steering boundary is preserved: send addresses the root target, handle draining removes only commands matching that handle's run, and command policy is enforced at application. Explicit RunContext queue attachment remains necessary for direct queue consumption by the agent loop.

Resilience and Maintainability Implications

  • observed — Application and destruction both consume commands through the mutex-protected handle drain, so a command removed by one is unavailable to the other. Recovery covers commands still in the handle, not every command that failed to affect a transcript: policy rejection and Cancel discarding the remainder of an already-drained batch predate this PR.
Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: synchronously requeueing unapplied steers when the forwarder guard drops.
Docstring Coverage Passed Docstring coverage is 90.48% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 3 files. (2 skipped: 2 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

I’m a rabbit; queues now turn,
With steers returned before I’m done.
The gate keeps forwarding in its place,
Then drop puts leftovers back with grace.
I nibble clover, pleased to see
Each waiting steer where it should be.

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-15T05:11:21.098509Z b6b7f99 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tinysweeper

tinysweeper Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper completed its review; deterministic results follow.

State: Incomplete
Priority: none
Reviewed head: 7c1a718a31bd
Updated: 2026-10-15T07:33:29Z

Review snapshot

Change surface Files Review signal Count
Production 2 Active findings 0
Tests 1 Noted findings 0
Documentation 2 Resolved findings 0
Configuration 0 Pending checks/questions 2

Completeness: Incomplete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

Features

  • Modified — Synchronous requeue of residual steers on guard drop: Residual steers drained from the steering handle are pushed back onto the run queue before `Drop` returns, rather than on a detached task, so a host reading the queue after the turn ends deterministically finds every steer the run did not apply. The previous failure path that lost steers when no tokio runtime was available at drop is gone, since no runtime is needed any more. (crates/tinyagents-harness/src/run_queue/forwarder.rs#impl<T: QueuedMessage> Drop for SteeringForwarderGuard<T> {, crates/tinyagents-harness/src/run_queue/forwarder.rs#impl<T: QueuedMessage> SteeringForwarderGuard<T> {)
  • Modified — Gate held across drain and delivery in the forwarder poll loop: `forward_lane` is now synchronous and, when a gate is supplied, checks the gate, drains the lane, and delivers into the steering handle while holding it; a closed gate makes it return `false` without draining anything. This removes the previous drain-then-requeue-on-closure path and its debug log, leaving every item either in the queue or in the handle and never in flight. (crates/tinyagents-harness/src/run_queue/forwarder.rs#async fn forward_lane<T: QueuedMessage>(, crates/tinyagents-harness/src/run_queue/forwarder.rs#pub async fn forward_steers<T: QueuedMessage>(, crates/tinyagents-harness/src/run_queue/forwarder.rs#pub async fn forward_collects<T: QueuedMessage>()
  • Added — Synchronous RunQueue API: `push_now` and `drain_now`: Adds non-async forms of push and drain usable from `Drop` and other non-async callers without spawning a task; the existing `async` `push` and `drain` delegate to them, so callers of the async API see unchanged behaviour and signatures. (crates/tinyagents-harness/src/run_queue/mod.rs#impl<T> RunQueue<T> {, crates/tinyagents-harness/src/run_queue/mod.rs#pub use forwarder::{)
  • Internal refactor — RunQueue lanes locked with `std::sync::Mutex` and a poison-recovering lock helper: Replaces the `tokio::sync::Mutex` guarding all three lanes with a `std::sync::Mutex` held only for single vector operations and never across an `.await`, which is what makes the synchronous forms safe. A poisoned lock is recovered rather than propagated, on the stated grounds that a panic mid-operation cannot leave a lane half-updated. The README's operational-constraints section is rewritten to describe this lock discipline, so documented behaviour matches the implementation. (crates/tinyagents-harness/src/run_queue/mod.rs, crates/tinyagents-harness/src/run_queue/README.md#stops, pauses, and deferrals leave the queue untouched. Details:, crates/tinyagents-harness/src/run_queue/forwarder.rs#fn lock_gate(gate: &ClosedGate) -> std::sync::MutexGuard<'_, bool> {, crates/tinyagents-harness/src/run_queue/forwarder.rs#fn now_ms() -> u64 {)
  • Added — Requeue semantics documented in changelog and README: Records the new guarantee that unapplied steers are back in the run queue when a turn ends, and that `push_now`/`drain_now` exist for non-async callers, so host-facing expectations about deterministic collection of leftovers are stated explicitly. (CHANGELOG.md, crates/tinyagents-harness/src/run_queue/README.md)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Could not review: CHANGELOG.md, tinysweeper/description

Before merge

  • Complete the critique review for CHANGELOG.md.
  • Complete the description review for tinysweeper/description.

How this fits together

flowchart 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
Loading
Agent review details

critique

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: CHANGELOG.md
  • Positive: No findings were reported in the critique lane; that lane reviewed 0 files and records that `CHANGELOG.md` could not be reviewed, so the absence of findings is not an assurance about that file.
  • Lane summary: Reviewed 0 files; 0 findings. 1 file could not be reviewed: CHANGELOG.md.

security

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Positive: No findings were reported in the security lane; that lane states no changed file has attack surface and treated `CHANGELOG.md` as prose rather than security-reviewed code.
  • Lane summary: No changed file has any attack surface. 1 file was not security-reviewed: CHANGELOG.md (prose or tabular data).

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane reported no findings; its summary carries only the note that the code index is behind this pull request (indexed at `462395cfea44`), so retrieved context may be out of date.
  • Lane summary: _The code index is behind this pull request (indexed at `462395cfea44`), so retrieved context may be out of date._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane reports nothing sensitive found in what this pull request commits; the lane records no unanswered items.
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: tinysweeper/description
  • Positive: The description lane produced no usable response, so no reviewer commentary on the pull request description is available.
  • Lane summary: No reviewer produced a usable response.
Evidence and run details
  • Models: ladder/vectors-oai3, glm-5.3-flash, deepseek-v4.1-flash
  • Spend: $0.000898
  • Tokens: 64957 input · 2657 output · 0 cached · 604 embedding
Head State Pass summary
b6b7f994a54f incomplete 0 active finding(s), 0 resolved finding(s) (at 2026-10-15T05:21:53Z)
7c1a718a31bd incomplete 0 active finding(s), 0 resolved finding(s) (at 2026-10-15T07:33:29Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 15, 2026
@senamakel
senamakel merged commit 41a8fb3 into main Oct 15, 2026
11 checks passed

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant