Add REPL host-embedding: external cancellation + live capability events#19
Conversation
Fills the three host-embedding gaps a host (openhuman) needs to drive
`repl::session::ReplSession` from an async runtime, none of which could be
worked around downstream:
- External cancellation. New `ReplCancelFlag` (sticky `Arc<AtomicBool>`),
installed via `ReplSession::with_cancel_flag`. Enforced fail-closed at the
same two points as the deadline: the engine `on_progress` hook terminates a
running script at the next statement (mapped to `Cancelled` via a
`CANCELLED_TOKEN` sentinel), and the blocking bridge grows a watcher thread
that drops an in-flight (possibly hung) capability future promptly on
cancel. `eval_cell` also fails closed before starting a cell if already
cancelled, without consuming an iteration. Reuses the existing unit
`TinyAgentsError::Cancelled` (the plan's proposed `Cancelled(String)` would
have been a breaking change to ~15 existing call sites for no added value).
- Live capability-call events. New feature-gated `AgentEvent::ReplCall
{ session_id, record, phase }` (+ `ReplCallPhase`) streamed on the session
`EventSink` at capability-call start and completion, correlated by a
`call_id` threaded from the built-in through `record`. A host subscribes an
`EventListener` and forwards REPL progress mid-cell, instead of only reading
`ReplResult.calls` after the cell returns.
- Async-embedding docs. `eval_cell` rustdoc now states it blocks internally
(`futures::executor::block_on`) and must be driven via
`spawn_blocking`/dedicated thread, and deadlocks on a current-thread runtime.
Tests ship with the change (crate convention): cancel-before-eval,
cancel-mid-script-loop (`on_progress` path), cancel-during-hanging-capability
(bridge path), and start+completion events paired by call_id. `cargo fmt
--check`, `cargo clippy --all-targets --features repl -- -D warnings`,
`cargo test --features repl`, and plain `cargo test` all pass.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 46 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 Run ID: 📒 Files selected for processing (9)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 622c9b27eb
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .ok_or_else(|| raise(ctx, TinyAgentsError::ModelNotFound(model_name.clone())))?; | ||
| let request = build_model_request(&model_name, params); | ||
| let call_id = new_call_id(); | ||
| emit_call_started(ctx, &call_id, ReplCallKind::Model, &model_name); |
There was a problem hiding this comment.
When this model call is cancelled, times out, or the provider returns an error, the later bridge_block_on(...).map_err(...)? exits after this Started event but before record emits a terminal ReplCall. A host that maintains an active-call set from ReplCall events will leave the call active forever in exactly those failure/cancellation paths; emit a completed/failed REPL call event before raising so every started call is paired.
Useful? React with 👍 / 👎.
| // Stream a "started" event for every fan-out leg up front, so a live | ||
| // observer sees the whole batch dispatch before any leg completes. | ||
| let call_id = new_call_id(); | ||
| emit_call_started(ctx, &call_id, ReplCallKind::Model, &model_name); |
There was a problem hiding this comment.
This sends Started for every batch item during preparation, but buffered(concurrency) only polls up to max_concurrency futures at a time. With max_concurrency = 1 and the first model call hanging or getting cancelled, later items are reported as started even though they were never dispatched and may never run, so live observers overcount in-flight work; move the start emission into each async leg immediately before invoking the capability.
Useful? React with 👍 / 👎.
`with_cancel_flag` consumes `self`, which suits construction but not a long-lived session held behind a lock by a host session manager. Because a `ReplCancelFlag` is sticky (once cancelled, stays cancelled), a persistent session reused after a cancelled cell would otherwise refuse every subsequent cell. `set_cancel_flag(&mut self, flag)` installs a fresh flag in place and rebuilds the engine (the persistent variable namespace is untouched, so `let` bindings survive the swap), letting a host arm a new per-cell flag before each `eval_cell` and stay resumable after a cancel. Test covers the swap + namespace preservation.
Points `vendor/tinyagents` at the `feat/repl-host-embedding` head (df391c4): external `ReplCancelFlag` cancellation, live `ReplCall` events, and `set_cancel_flag`. Depends on tinyhumansai/tinyagents#19; re-pin to the merged commit once that lands.
Points `vendor/tinyagents` at the `feat/repl-host-embedding` head (df391c4): external `ReplCancelFlag` cancellation, live `ReplCall` events, and `set_cancel_flag`. Depends on tinyhumansai/tinyagents#19; re-pin to the merged commit once that lands.
…nyhumansai#18 started_at_ms) tinyhumansai/tinyagents#19 (REPL host-embedding: ReplCancelFlag cancellation, live ReplCall events, set_cancel_flag) is now merged to tinyagents main, which also carries tinyhumansai#18 (`AgentEvent::ToolCompleted.started_at_ms`) that openhuman's observability path now requires. Pin the submodule at tinyagents main (e72036d) so the crate provides both — superseding the earlier pin at the pre-merge tinyhumansai#19 branch head, which predated tinyhumansai#18 and left `observability.rs` referencing a field the crate lacked.
tinyhumansai/tinyagents#19 (REPL host-embedding: ReplCancelFlag cancellation, live ReplCall events, set_cancel_flag) is now merged to tinyagents main (released v1.6.0, e72036d), which also carries tinyhumansai#18's `ToolCompleted`/ `ModelCompleted` field additions (started_at_ms/duration_ms/output_bytes/error). Pin the submodule at tinyagents main so the crate provides the REPL surface the `rlm` domain needs. Because the `rlm` domain requires tinyagents ≥ tinyhumansai#19 (linearly after tinyhumansai#18), this also completes openhuman's partial 1.6.0 adoption in `tinyagents/observability.rs`: the `ToolCompleted` projector match ignores the new fields via `..`, and the test-only `ToolCompleted`/`ModelCompleted` constructions carry the new fields (`None`). The projector's `_ => {}` catch-all already handles the new `ReplCall`/`ModelOverrideSkipped` variants. Regenerated Cargo.lock (1.5.0→1.6.0).
Points `vendor/tinyagents` at the `feat/repl-host-embedding` head (df391c4): external `ReplCancelFlag` cancellation, live `ReplCall` events, and `set_cancel_flag`. Depends on tinyhumansai/tinyagents#19; re-pin to the merged commit once that lands.
Points `vendor/tinyagents` at the `feat/repl-host-embedding` head (df391c4): external `ReplCancelFlag` cancellation, live `ReplCall` events, and `set_cancel_flag`. Depends on tinyhumansai/tinyagents#19; re-pin to the merged commit once that lands.
1 participant
Summary
Fills the three host-embedding gaps a host needs to drive
repl::session::ReplSessionfrom an async runtime — none of which could beworked around downstream. Motivated by exposing the
.ragshREPL as afirst-class
rlmtool in a downstream host (openhuman); see that host's planfor the consuming design.
1. External cancellation (
ReplCancelFlag)A running cell could previously only be stopped by its wall-clock timeout. New
ReplCancelFlag(a sticky, cheaply-cloneableArc<AtomicBool>) is installedvia
ReplSession::with_cancel_flag, and enforced fail-closed at the same twopoints as the deadline:
on_progresshook terminates a running script at the nextstatement/operation (via a
CANCELLED_TOKENsentinel mapped toTinyAgentsError::Cancelled);bridge_block_on_raw) grows a watcher thread that pollsthe flag and drops an in-flight (possibly hung) capability future promptly.
eval_cellalso fails closed before starting a cell if the flag is alreadytripped, without consuming a
max_iterationsslot.2. Live capability-call events on the
EventSinkReplResult.callsis only readable after a cell returns, so a long fan-outlooks frozen. New feature-gated
AgentEvent::ReplCall { session_id, record, phase }(+ReplCallPhase::{Started, Completed}) streams eachmodel_query/tool_call/agent_query/emiton the sessionEventSinkas it startsand again as it completes, correlated by a
call_idthreaded from the built-inthrough
record. A host subscribes anEventListenerand forwards REPLprogress mid-cell. Gated behind
replso the default build neither pulls inRhai nor references
ReplCallRecord.3. Async-embedding documentation
eval_cell's rustdoc now documents that it blocks internally(
futures::executor::block_on) and must be driven viaspawn_blocking/ adedicated thread from an async host, and deadlocks on a current-thread runtime.
Tests (shipped with the change, per crate convention)
iteration; session reusable with a fresh flag;
on_progresspath);ReplCallevents observed and paired bycall_id,correlated to the session id.
Commands run
cargo fmt --check— cleancargo clippy --all-targets --features repl -- -D warnings— cleancargo test --features repl— greencargo test(default features) — greenNote on base
Branched off
357bcc8(the commit the downstream submodule currently pins), ascalled for by the integration plan;
mainhas since advanced, so a rebase maybe needed before merge.