From e376edc559b9be789258a76b809e23875f4be094 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 23 Sep 2026 21:04:45 +0700 Subject: [PATCH] Run on nagoya, with the reactor passed in tokio was the last runtime this crate tied its consumers to: agency-proxy could not leave tokio while every provider CLI was spawned through tokio::process. Processes now come from nagoya::process, channels from futures, timers from nagoya, and every entry point that spawns a CLI takes the reactor Handle it registers on. Nothing in the crate creates a reactor or keeps one in a static; the caller owns it, and each test starts its own. That is a public API change, so 0.5.0: run, stream, interrupt, Probe::run, Probe::run_bin, AuthStatus::check, AuthStatus::check_bin, Agent::account_usage and Agent::discover_models take `&Handle`. `nagoya` is re-exported so a caller does not need a second copy of it. Two behaviours tokio's scheduler had been hiding are fixed on the way. A futures oneshot receiver counts as finished once its sender drops, which is exactly how cancel is signalled, so the cancel and close arms are fused or cancelling would hang until the agent exited. And a closed control channel no longer spins the Codex and Grok drivers, which futures has no budget to interrupt. A panic in a spawned driver is still Error::Interrupted, caught inside the task because nagoya rethrows it in whoever awaits. --- CHANGELOG.md | 37 + Cargo.toml | 16 +- README.md | 41 +- docs/host-integration.md | 6 +- src/account.rs | 34 +- src/auth.rs | 37 +- src/error.rs | 10 +- src/lib.rs | 26 +- src/model.rs | 28 +- src/probe.rs | 31 +- src/proc.rs | 6 +- src/run.rs | 539 +++++--- tests/live.rs | 2671 +++++++++++++++++++++----------------- tests/process.rs | 429 +++--- 14 files changed, 2227 insertions(+), 1684 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 113c2d1..c71135b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,43 @@ appear in a patch rather than inflating the version toward 1.0 on a crate still shape. **Where that happens the entry says so at the top**, because a version number that under-signals is only acceptable if the changelog over-signals to compensate. +## 0.5.0 + +The crate now runs on nagoya instead of tokio, and every entry point that spawns a CLI +takes the I/O reactor it should use. + +### Breaking + +- **Spawning entry points take a `&nagoya::reactor::Handle`.** The caller starts a + `nagoya::reactor::Reactor`, keeps it alive while its runs are in flight, and passes + `&reactor.handle()`. The crate never starts a reactor of its own and holds no global + one. Changed signatures: + - `run(request: &Request, reactor: &Handle) -> Result` + - `stream(request: &Request, reactor: &Handle) -> Result` + - `interrupt(request: &Request, reactor: &Handle) -> Result` + - `Probe::run(agent: Agent, reactor: &Handle) -> Result` + - `Probe::run_bin(agent: Agent, bin: &str, reactor: &Handle) -> Result` + - `AuthStatus::check(agent: Agent, reactor: &Handle) -> Result` + - `AuthStatus::check_bin(agent: Agent, bin: &str, reactor: &Handle) -> Result` + - `Agent::account_usage(self, reactor: &Handle) -> Result` + - `Agent::discover_models(&self, reactor: &Handle) -> Result>` +- `nagoya` is re-exported as `agent_abstraction::nagoya`, so a caller can name + `Reactor` and `Handle` without a second copy of the dependency. + +### Changed + +- **tokio is gone**, from dependencies and dev-dependencies. Child processes come from + `nagoya::process`, the driver and stderr reader run on nagoya's shared pool, and the + channels, `select_biased!` and I/O extension traits come from `futures`. Every future + this crate returns still works under any executor, tokio included. +- **`stream` no longer needs an ambient runtime.** It used to return + `Error::NoRuntime` when called outside a tokio runtime; nagoya's pool starts on first + use and the reactor is passed in, so that variant is never returned now. It stays in the enum so existing matches + compile. +- **A closed control channel no longer spins the Codex and Grok drivers.** Under tokio a + detached interactive run polled its closed channel until tokio's cooperative budget + forced a yield; the arm is now skipped once every sender is gone. + ## 0.4.20 ### Added diff --git a/Cargo.toml b/Cargo.toml index c8e647c..5a4417c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "agent-abstraction" -version = "0.4.21" +version = "0.5.0" edition = "2024" # The floor edition 2024 requires, and where the strictest dependencies (uuid, # getrandom) sit. Derived from the dependency graph rather than compile-tested. @@ -32,10 +32,15 @@ name = "agent_abstraction" path = "src/lib.rs" [dependencies] -# `process` gives an async child with piped stdio; `io-util` the line reader that -# turns a JSONL stream into events; `time` the run timeout; `sync` the event -# channel. No `full`: the consumer picks its own runtime features. -tokio = { version = "1", features = ["process", "io-util", "sync", "time", "rt", "macros"] } +# The runtime: `spawn` for the driver task, `sleep` and `timeout` for the run +# deadline, and `process` for an async child with piped stdio. `process` needs +# the reactor, which is why both are named. The returned futures and pipes work +# under any executor, so a consumer on another runtime can still await a `Run`. +nagoya = { version = "^0.1", features = ["reactor", "process"] } +# What nagoya deliberately does not carry: the oneshot and mpsc channels, the +# `select_biased!` that stands in for tokio's biased `select!`, and the +# `AsyncRead`/`AsyncBufRead` extension traits the line reader is built on. +futures = "^0.3" serde = { version = "1", features = ["derive"] } serde_json = "1" thiserror = "2" @@ -53,7 +58,6 @@ uuid = { version = "1", features = ["v4", "serde"] } libc = "0.2" [dev-dependencies] -tokio = { version = "1", features = ["rt-multi-thread", "macros", "time"] } # The live tests mint UUIDs to prove a caller-assigned session id round-trips. uuid = { version = "1", features = ["v4"] } diff --git a/README.md b/README.md index c6a8d22..bb170f6 100644 --- a/README.md +++ b/README.md @@ -20,12 +20,18 @@ It is a **library, not a CLI**. Your program links it and spawns the agent direc nothing marshals a request through a command line and back out of stdout twice. ```rust +use agent_abstraction::nagoya::reactor::Reactor; use agent_abstraction::{Agent, Permission, Request, run}; +// The I/O driver for the child's pipes. Yours to start and keep alive while +// runs are in flight; the crate never starts one of its own. +let reactor = Reactor::start()?; + let outcome = run( &Request::new(Agent::Claude, "Reply with the single word: pong") .model("haiku") .permission(Permission::ReadOnly), + &reactor.handle(), ) .await?; @@ -33,6 +39,10 @@ println!("{}", outcome.text); // "pong" println!("{:?}", outcome.usage.cost_usd); ``` +Every entry point that spawns a CLI (`run`, `stream`, `interrupt`, `Probe::run`, +`AuthStatus::check`, `Agent::account_usage`, `Agent::discover_models`) takes that +`&reactor.handle()`. The examples below assume a `reactor` like this one is in scope. + ## What each agent can actually do Verified live, against `claude 2.1.205`, `codex-cli 0.146.0` and `GitHub Copilot CLI 1.0.78` @@ -60,7 +70,7 @@ Both, depending on the agent. Verified by round-trip, not from `--help`: // Claude and Copilot: the id is yours to pick, so it can match a thread id // your app already has, with no mapping table in between. let mine = uuid::Uuid::new_v4().to_string(); -let outcome = run(&Request::new(Agent::Claude, "hi").session_id(&mine)).await?; +let outcome = run(&Request::new(Agent::Claude, "hi").session_id(&mine), &reactor.handle()).await?; assert_eq!(outcome.session.as_deref(), Some(mine.as_str())); ``` @@ -83,7 +93,7 @@ conversation it meant to branch. ## Streaming ```rust -let mut running = stream(&Request::new(Agent::Claude, "audit this repo"))?; +let mut running = stream(&Request::new(Agent::Claude, "audit this repo"), &reactor.handle())?; while let Some(event) = running.recv().await { match event { Event::Text(text) => print!("{text}"), @@ -123,7 +133,7 @@ let turn = Request::new(Agent::Claude, "what did I ask you to remember?") .session(&store, ".", "thread-42", /* fork */ false)?; assert_eq!(turn.session_phase(), Some(Phase::Continue)); -let outcome = run(&turn).await?; +let outcome = run(&turn, &reactor.handle()).await?; ``` Records live at `//.json`, partitioned by project so the same name @@ -206,7 +216,7 @@ established here; the entry says which. Where a CLI can be asked directly, prefer that: ```rust -let models = Agent::Codex.discover_models().await?; // reflects the installed binary +let models = Agent::Codex.discover_models(&reactor.handle()).await?; // reflects the installed binary ``` `discover_models` returns `Error::Unsupported` on Claude and Copilot rather than silently @@ -241,7 +251,8 @@ back parsed instead of guessing at formatting the model never promised: let outcome = run(&Request::new(Agent::Codex, "Alice is 30 years old.") .schema(r#"{"type":"object", "properties":{"name":{"type":"string"},"age":{"type":"integer"}}, - "required":["name","age"],"additionalProperties":false}"#)) + "required":["name","age"],"additionalProperties":false}"#), + &reactor.handle()) .await?; assert_eq!(outcome.structured.unwrap()["name"], "Alice"); @@ -265,7 +276,7 @@ cancelling a request should stop the work, not leave an agent running invisibly, quota and writing files with nobody watching. ```rust -let running = stream(&request)?; +let running = stream(&request, &reactor.handle())?; drop(running); // agent and its children are killed running.cancel().await?; // cooperative: returns only once the tree has exited running.detach(); // opt out: keep running unsupervised @@ -294,7 +305,7 @@ a run already under way: ```rust let request = Request::new(Agent::Claude, prompt).interactive(); -let mut run = stream(&request)?; +let mut run = stream(&request, &reactor.handle())?; // Keep input independent from the task continuously draining run.recv(). let control = run.control(); @@ -353,7 +364,7 @@ use agent_abstraction::{Agent, Command, Compaction, Event, Request, stream}; let request = Request::command(Agent::Claude, &Command::Compact { instructions: None }) .resume(&session_id); -let mut run = stream(&request)?; +let mut run = stream(&request, &reactor.handle())?; while let Some(event) = run.recv().await { if let Event::Compaction(Compaction::Finished { ok, error }) = event { // `ok: false` with a reason is an answer, not an error. @@ -402,7 +413,7 @@ let request = Request::new(Agent::Claude, prompt) .permission(Permission::Edit) .approvals(); -let mut run = stream(&request)?; +let mut run = stream(&request, &reactor.handle())?; while let Some(event) = run.recv().await { if let Event::ApprovalRequest(approval) = event { // approval.tool is "Bash"; approval.input carries the actual command @@ -484,7 +495,7 @@ Without spending a request: ```rust for agent in Agent::ALL { - let status = AuthStatus::check(agent).await?; + let status = AuthStatus::check(agent, &reactor.handle()).await?; println!("{agent}: {}", status.summary()); } ``` @@ -613,7 +624,7 @@ These are `Error::AgentError`, carrying the agent's own wording and the provider where one was reported: ```rust -match run(&request).await { +match run(&request, &reactor.handle()).await { Err(Error::AgentError { status: Some(404), message, .. }) => { // Typically a model the account cannot reach. `message` is the agent's wording. eprintln!("{message}"); @@ -634,14 +645,14 @@ passed through untouched. ## Usage and quota -Two questions with two answers. `Outcome::usage` measures the run; `Agent::account_usage()` +Two questions with two answers. `Outcome::usage` measures the run; `Agent::account_usage` measures the plan behind it. Everything is a value, never a formatted string or a rendered bar, so a host presents it however it likes. ### Per run, and per session ```rust -let outcome = run(&request).await?; +let outcome = run(&request, &reactor.handle()).await?; let used = outcome.usage.context_used(); // Option, 0.0 to 1.0 ``` @@ -710,7 +721,7 @@ until the turn completes, so it never emits this. ```rust if agent.reports_account_usage() { - let account = agent.account_usage().await?; + let account = agent.account_usage(&reactor.handle()).await?; for window in &account.windows { // window.used_percent, window.window_minutes, window.resets_at } @@ -770,7 +781,7 @@ A Rust port of [nickderobertis/oneharness](https://github.com/nickderobertis/one two JSON round-trips to ask a question. - **The shell scripts are gone**, 39 of them, mostly CI gates and per-harness e2e drivers. - **Five harnesses are gone** (OpenCode, Goose, Qwen, Crush, Cursor). -- **Async throughout.** oneharness runs blocking; this streams over tokio, which is what a +- **Async throughout.** oneharness runs blocking; this streams over nagoya, which is what a Tauri front end needs to render a run as it happens. Some findings did not survive re-verification against the current CLIs. oneharness models diff --git a/docs/host-integration.md b/docs/host-integration.md index 87e4a6a..4b2f782 100644 --- a/docs/host-integration.md +++ b/docs/host-integration.md @@ -14,7 +14,7 @@ A user who types a correction mid-turn must not have to wait for the turn to end ```rust let request = Request::new(Agent::Claude, prompt).interactive(); -let mut run = stream(&request)?; +let mut run = stream(&request, &reactor.handle())?; // Later, from the UI, the moment the user hits enter: run.send("actually, skip the tests and just fix the parser").await?; @@ -70,7 +70,7 @@ let request = Request::new(Agent::Claude, prompt) .permission(Permission::Edit) // not ReadOnly, see below .approvals(); -let mut run = stream(&request)?; +let mut run = stream(&request, &reactor.handle())?; while let Some(event) = run.recv().await { if let Event::ApprovalRequest(approval) = event { let decision = if user_approves(&approval) { @@ -119,7 +119,7 @@ let request = Request::new(Agent::Claude, prompt) .session(&store, &project, "chat")? // so the conversation continues across turns .approvals(); // implies .interactive() -let mut run = stream(&request)?; +let mut run = stream(&request, &reactor.handle())?; while let Some(event) = run.recv().await { match event { Event::Text(chunk) => ui.append_assistant(&chunk), diff --git a/src/account.rs b/src/account.rs index 89b1c3f..91c9c77 100644 --- a/src/account.rs +++ b/src/account.rs @@ -36,9 +36,10 @@ use std::process::Stdio; use std::time::Duration; +use futures::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use futures::stream::StreamExt as _; use serde::{Deserialize, Serialize}; use serde_json::Value; -use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use crate::agent::Agent; use crate::error::{Error, Result}; @@ -144,6 +145,9 @@ impl Agent { /// Ask the agent what the account has spent and what remains. /// + /// `reactor` drives the child's pipes; the caller keeps its + /// [`nagoya::reactor::Reactor`] alive until this returns. + /// /// # Errors /// [`Error::Unsupported`] where the agent has no headless way to answer, /// which today is Claude and Copilot; check @@ -152,9 +156,9 @@ impl Agent { /// cannot be run, [`Error::Timeout`] if it does not reply, /// [`Error::AgentError`] if it replies with a refusal, and /// [`Error::Parse`] if the reply is not the expected shape. - pub async fn account_usage(self) -> Result { + pub async fn account_usage(self, reactor: &nagoya::reactor::Handle) -> Result { match self { - Agent::Codex => codex_account_usage(self.bin()).await, + Agent::Codex => codex_account_usage(self.bin(), reactor).await, // Deliberately an error rather than a half-answer assembled from a // past run's rate-limit event: that would be neither current nor // account-wide, and would read as though it were both. @@ -174,15 +178,15 @@ impl Agent { /// names the method, rather than as silence. /// /// Verified against codex-cli 0.145.0 on 2026-07-29. -async fn codex_account_usage(bin: &str) -> Result { - let mut child = tokio::process::Command::new(bin) +async fn codex_account_usage(bin: &str, reactor: &nagoya::reactor::Handle) -> Result { + let mut child = nagoya::process::Command::new(bin) .arg("app-server") .stdin(Stdio::piped()) .stdout(Stdio::piped()) // Silenced rather than captured: the server logs progress here and none // of it belongs in an error about usage. .stderr(Stdio::null()) - .spawn() + .spawn(reactor) .map_err(|source| { if source.kind() == std::io::ErrorKind::NotFound { Error::NotInstalled { @@ -199,7 +203,7 @@ async fn codex_account_usage(bin: &str) -> Result { })?; let exchange = codex_exchange(&mut child); - let result = match tokio::time::timeout(QUERY_TIMEOUT, exchange).await { + let result = match nagoya::timeout(QUERY_TIMEOUT, exchange).await { Ok(result) => result, Err(_) => Err(Error::Timeout { bin: bin.to_string(), @@ -214,7 +218,7 @@ async fn codex_account_usage(bin: &str) -> Result { } /// Drive the three requests and collect their replies. -async fn codex_exchange(child: &mut tokio::process::Child) -> Result { +async fn codex_exchange(child: &mut nagoya::process::Child) -> Result { const ACCOUNT: i64 = 2; const LIMITS: i64 = 3; const USAGE: i64 = 4; @@ -266,7 +270,9 @@ async fn codex_exchange(child: &mut tokio::process::Child) -> Result 0 { // A stream that ends before every reply arrives leaves whatever was // collected in place rather than discarding it. - let Ok(Some(line)) = lines.next_line().await else { + // futures' `Lines` is a stream, so tokio's `Ok(Some(line))` from + // `next_line` is `Some(Ok(line))` here; a read error still ends it. + let Some(Ok(line)) = lines.next().await else { break; }; if line.len() > MAX_REPLY_BYTES { @@ -470,12 +476,16 @@ mod tests { /// The capability is answerable without spawning anything, so a host can /// decide whether to build the panel at all. - #[tokio::test] - async fn agents_that_cannot_report_say_so_without_being_asked_twice() { + #[test] + fn agents_that_cannot_report_say_so_without_being_asked_twice() { + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); for agent in [Agent::Claude, Agent::Copilot] { assert!(!agent.reports_account_usage(), "{agent}"); assert!( - matches!(agent.account_usage().await, Err(Error::Unsupported { .. })), + matches!( + nagoya::block_on(agent.account_usage(&reactor.handle())), + Err(Error::Unsupported { .. }) + ), "{agent} should refuse rather than assemble a partial answer" ); } diff --git a/src/auth.rs b/src/auth.rs index e2eeca2..aecb143 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -7,9 +7,10 @@ //! //! ```no_run //! # use agent_abstraction::{Agent, AuthStatus}; -//! # async fn example() -> agent_abstraction::Result<()> { +//! # use agent_abstraction::nagoya::reactor::Handle; +//! # async fn example(reactor: &Handle) -> agent_abstraction::Result<()> { //! for agent in Agent::ALL { -//! let status = AuthStatus::check(agent).await?; +//! let status = AuthStatus::check(agent, reactor).await?; //! println!("{agent}: {}", status.summary()); //! } //! # Ok(()) @@ -66,8 +67,12 @@ impl AuthStatus { /// [`Error::NotInstalled`] if the binary is missing, [`Error::Spawn`] if it /// cannot be run. A CLI that answers "logged out" is a successful check, /// not an error. - pub async fn check(agent: Agent) -> Result { - AuthStatus::check_bin(agent, agent.bin()).await + /// + /// # Reactor + /// `reactor` drives the status command's pipes; the caller keeps its + /// [`nagoya::reactor::Reactor`] alive until this returns. + pub async fn check(agent: Agent, reactor: &nagoya::reactor::Handle) -> Result { + AuthStatus::check_bin(agent, agent.bin(), reactor).await } /// Ask a specific binary, for a caller overriding the path with @@ -76,14 +81,18 @@ impl AuthStatus { /// # Errors /// [`Error::NotInstalled`] if the binary is missing, [`Error::Spawn`] if it /// cannot be run. - pub async fn check_bin(agent: Agent, bin: &str) -> Result { + pub async fn check_bin( + agent: Agent, + bin: &str, + reactor: &nagoya::reactor::Handle, + ) -> Result { let Some(args) = agent.auth_status_argv() else { return Ok(AuthStatus::uncheckable(agent)); }; - let output = tokio::process::Command::new(bin) + let output = nagoya::process::Command::new(bin) .args(args) - .output() + .output(reactor) .await .map_err(|source| { if source.kind() == std::io::ErrorKind::NotFound { @@ -320,11 +329,15 @@ mod tests { /// Copilot exposes no status command, and saying "logged out" for an agent /// that cannot be asked would send someone to fix a working setup. - #[tokio::test] - async fn copilot_reports_that_it_cannot_be_checked() { - let status = AuthStatus::check_bin(Agent::Copilot, "copilot") - .await - .expect("an uncheckable agent is not an error"); + #[test] + fn copilot_reports_that_it_cannot_be_checked() { + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + let status = nagoya::block_on(AuthStatus::check_bin( + Agent::Copilot, + "copilot", + &reactor.handle(), + )) + .expect("an uncheckable agent is not an error"); assert_eq!(status.state, AuthState::Unknown); assert!(!status.needs_login()); assert!( diff --git a/src/error.rs b/src/error.rs index 56f8e05..3057a64 100644 --- a/src/error.rs +++ b/src/error.rs @@ -243,11 +243,13 @@ pub enum Error { limit: usize, }, - /// [`crate::stream`] was called outside a Tokio runtime. + /// No longer returned. Kept so existing matches on it still compile. /// - /// Spawning the driver task needs a runtime context. Reporting this rather - /// than letting `tokio::spawn` panic keeps the fallible signature honest. - #[error("no Tokio runtime is running; call this from within one")] + /// [`crate::stream`] used to need an ambient Tokio runtime to spawn its + /// driver task, and reported its absence here. The driver now runs on + /// nagoya's shared pool, which exists without being entered, so there is no + /// missing context left to report. + #[error("no async runtime is running; call this from within one")] NoRuntime, /// The task driving the run panicked or was cancelled, so there is no diff --git a/src/lib.rs b/src/lib.rs index 7590804..a264833 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,14 +7,23 @@ //! //! # Running a prompt //! +//! Every entry point that spawns a CLI takes a handle to a +//! [`nagoya::reactor::Reactor`], the I/O driver the child's pipes are +//! registered with. The caller starts it, keeps it alive for as long as any run +//! it served is in flight, and passes `&reactor.handle()`. This crate never +//! starts one itself. +//! //! ```no_run +//! use agent_abstraction::nagoya::reactor::Reactor; //! use agent_abstraction::{Agent, Permission, Request, run}; //! //! # async fn example() -> agent_abstraction::Result<()> { +//! let reactor = Reactor::start().expect("an I/O reactor"); //! let outcome = run( //! &Request::new(Agent::Claude, "Reply with the single word: pong") //! .model("haiku") //! .permission(Permission::ReadOnly), +//! &reactor.handle(), //! ) //! .await?; //! @@ -26,10 +35,11 @@ //! # Watching one as it works //! //! ```no_run +//! use agent_abstraction::nagoya::reactor::Handle; //! use agent_abstraction::{Agent, Event, Request, stream}; //! -//! # async fn example() -> agent_abstraction::Result<()> { -//! let mut running = stream(&Request::new(Agent::Claude, "audit this repo"))?; +//! # async fn example(reactor: &Handle) -> agent_abstraction::Result<()> { +//! let mut running = stream(&Request::new(Agent::Claude, "audit this repo"), reactor)?; //! while let Some(event) = running.recv().await { //! match event { //! Event::Text(text) => print!("{text}"), @@ -48,19 +58,20 @@ //! whatever handle the agent understands: //! //! ```no_run +//! use agent_abstraction::nagoya::reactor::Handle; //! use agent_abstraction::{Agent, Request, SessionStore, run}; //! -//! # async fn example() -> agent_abstraction::Result<()> { +//! # async fn example(reactor: &Handle) -> agent_abstraction::Result<()> { //! let store = SessionStore::open("/var/lib/myapp/sessions"); //! //! // First turn creates the session; later turns continue it. //! let first = Request::new(Agent::Claude, "remember the number 7") //! .session(&store, ".", "thread-42", false)?; -//! run(&first).await?; +//! run(&first, reactor).await?; //! //! let second = Request::new(Agent::Claude, "what number did I say?") //! .session(&store, ".", "thread-42", false)?; -//! println!("{}", run(&second).await?.text); +//! println!("{}", run(&second, reactor).await?.text); //! # Ok(()) //! # } //! ``` @@ -116,3 +127,8 @@ pub use probe::{Probe, Version, VersionStatus}; pub use request::Request; pub use run::{Run, RunControl, interrupt, run, stream}; pub use session::{Phase, SessionRecord, SessionStore}; + +/// The runtime this crate is built on, re-exported so a caller can start the +/// [`nagoya::reactor::Reactor`] every spawning entry point takes a handle to +/// without naming a second, possibly mismatched, copy of the dependency. +pub use nagoya; diff --git a/src/model.rs b/src/model.rs index 66071ed..8ee175a 100644 --- a/src/model.rs +++ b/src/model.rs @@ -191,6 +191,9 @@ impl Agent { /// Worth preferring wherever it works: it reflects the binary actually /// present instead of the one this crate was written against. /// + /// `reactor` drives the child's pipes; the caller keeps its + /// [`nagoya::reactor::Reactor`] alive until this returns. + /// /// # Errors /// [`Error::Unsupported`] on an agent with no headless way to answer, which /// today is Claude and Copilot. That is deliberately an error rather than a @@ -199,10 +202,10 @@ impl Agent { /// answers a question they did not ask. [`Error::NotInstalled`] if the /// binary is missing, [`Error::Spawn`] if it cannot be run, and /// [`Error::Parse`] if its output is not the expected shape. - pub async fn discover_models(&self) -> Result> { + pub async fn discover_models(&self, reactor: &nagoya::reactor::Handle) -> Result> { match self { - Agent::Codex => discover_codex(self.bin()).await, - Agent::Grok => discover_grok(self.bin()).await, + Agent::Codex => discover_codex(self.bin(), reactor).await, + Agent::Grok => discover_grok(self.bin(), reactor).await, // Neither can be asked without a terminal, verified against // Copilot CLI 1.0.75 and claude 2.1.212. Copilot has no `models` // subcommand, rejects an unknown `--model` without listing the valid @@ -542,10 +545,10 @@ fn grok_models() -> Vec { ] } -async fn discover_grok(bin: &str) -> Result> { - let output = tokio::process::Command::new(bin) +async fn discover_grok(bin: &str, reactor: &nagoya::reactor::Handle) -> Result> { + let output = nagoya::process::Command::new(bin) .arg("models") - .output() + .output(reactor) .await .map_err(|source| { if source.kind() == std::io::ErrorKind::NotFound { @@ -603,10 +606,10 @@ fn parse_grok_models(stdout: &str) -> Result> { /// `codex debug models` prints one JSON document carrying every model plus each /// one's full system prompt, so the reply runs to hundreds of kilobytes. Only /// the descriptive fields are kept. -async fn discover_codex(bin: &str) -> Result> { - let output = tokio::process::Command::new(bin) +async fn discover_codex(bin: &str, reactor: &nagoya::reactor::Handle) -> Result> { + let output = nagoya::process::Command::new(bin) .args(["debug", "models"]) - .output() + .output(reactor) .await .map_err(|source| { if source.kind() == std::io::ErrorKind::NotFound { @@ -789,12 +792,13 @@ mod tests { /// Discovery must not quietly answer with the compiled-in list: a caller /// asking for it is asking for freshness, and a silent fallback answers a /// different question. - #[tokio::test] - async fn agents_that_cannot_be_asked_say_so() { + #[test] + fn agents_that_cannot_be_asked_say_so() { + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); for agent in [Agent::Claude, Agent::Copilot] { assert!( matches!( - agent.discover_models().await, + nagoya::block_on(agent.discover_models(&reactor.handle())), Err(Error::Unsupported { .. }) ), "{agent} should report that it cannot enumerate models" diff --git a/src/probe.rs b/src/probe.rs index 8336357..0afbd9e 100644 --- a/src/probe.rs +++ b/src/probe.rs @@ -132,11 +132,14 @@ pub struct Probe { impl Probe { /// Ask `agent`'s default binary for its version. /// + /// `reactor` drives the child's pipes; the caller keeps its + /// [`nagoya::reactor::Reactor`] alive until this returns. + /// /// # Errors /// [`Error::NotInstalled`] if the binary is missing, [`Error::Spawn`] if it /// cannot be run. - pub async fn run(agent: Agent) -> Result { - Probe::run_bin(agent, agent.bin()).await + pub async fn run(agent: Agent, reactor: &nagoya::reactor::Handle) -> Result { + Probe::run_bin(agent, agent.bin(), reactor).await } /// Ask a specific binary for its version, for a caller that overrides the @@ -145,10 +148,14 @@ impl Probe { /// # Errors /// [`Error::NotInstalled`] if the binary is missing, [`Error::Spawn`] if it /// cannot be run. - pub async fn run_bin(agent: Agent, bin: &str) -> Result { - let output = tokio::process::Command::new(bin) + pub async fn run_bin( + agent: Agent, + bin: &str, + reactor: &nagoya::reactor::Handle, + ) -> Result { + let output = nagoya::process::Command::new(bin) .arg("--version") - .output() + .output(reactor) .await .map_err(|source| { if source.kind() == std::io::ErrorKind::NotFound { @@ -298,11 +305,15 @@ mod tests { } } - #[tokio::test] - async fn probing_a_missing_binary_says_how_to_install_it() { - let err = Probe::run_bin(Agent::Claude, "agent-abstraction-no-such-binary") - .await - .unwrap_err(); + #[test] + fn probing_a_missing_binary_says_how_to_install_it() { + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + let err = nagoya::block_on(Probe::run_bin( + Agent::Claude, + "agent-abstraction-no-such-binary", + &reactor.handle(), + )) + .unwrap_err(); assert!(matches!(err, Error::NotInstalled { .. }), "{err:?}"); } } diff --git a/src/proc.rs b/src/proc.rs index 576a25f..cddc780 100644 --- a/src/proc.rs +++ b/src/proc.rs @@ -36,7 +36,7 @@ /// failure. Call this **before** reaping the child, because reaping clears the /// pid this needs to address the group. #[cfg(unix)] -pub(crate) fn kill_process_group(child: &tokio::process::Child) { +pub(crate) fn kill_process_group(child: &nagoya::process::Child) { let Some(pid) = child.id() else { // Already reaped, so there is no pid left to address. Signalling now // would risk hitting a pid the OS has since recycled. @@ -46,7 +46,7 @@ pub(crate) fn kill_process_group(child: &tokio::process::Child) { } /// Signal a group by its leader pid, for a caller holding the pid rather than -/// the [`tokio::process::Child`]. +/// the [`nagoya::process::Child`]. /// /// [`crate::Run`]'s `Drop` needs this. `Drop` cannot await, so its only other /// option is to abort the driver task and rely on the runtime polling that task @@ -80,7 +80,7 @@ pub(crate) fn kill_group_by_pid(pid: u32) { /// No-op: see the module docs. Only the direct child is killed on Windows. #[cfg(not(unix))] -pub(crate) fn kill_process_group(_child: &tokio::process::Child) {} +pub(crate) fn kill_process_group(_child: &nagoya::process::Child) {} /// No-op counterpart for non-unix. #[cfg(not(unix))] diff --git a/src/run.rs b/src/run.rs index 5bdd411..48d1caa 100644 --- a/src/run.rs +++ b/src/run.rs @@ -13,9 +13,12 @@ use std::collections::{HashMap, VecDeque}; use std::process::Stdio; use std::time::Duration; -use tokio::io::{AsyncReadExt, AsyncWriteExt, BufReader}; -use tokio::process::{Child, Command}; -use tokio::sync::mpsc; +use futures::channel::{mpsc, oneshot}; +use futures::future::FutureExt as _; +use futures::io::{AsyncReadExt, AsyncWriteExt, BufReader}; +use futures::sink::SinkExt as _; +use futures::stream::StreamExt as _; +use nagoya::process::{Child, ChildStdin, ChildStdout, Command}; use crate::agent::{Continue, EnvPolicy}; use crate::error::{Error, Result}; @@ -32,7 +35,7 @@ use crate::request::Request; /// discards the remainder of that line. Returns `None` at end of input. async fn read_bounded_line(reader: &mut R, buf: &mut String) -> std::io::Result> where - R: tokio::io::AsyncBufRead + Unpin, + R: futures::io::AsyncBufRead + Unpin, { buf.clear(); let mut bytes = Vec::new(); @@ -69,14 +72,46 @@ where /// /// The decision forwarder holds the child's stdin, so leaving it running past /// the run would keep a pipe open to a process that is gone. -struct AbortOnDrop(tokio::task::JoinHandle<()>); +struct AbortOnDrop(Option>); impl Drop for AbortOnDrop { fn drop(&mut self) { - self.0.abort(); + // nagoya's `cancel` consumes the handle where tokio's `abort` borrowed + // it, hence the `Option`. Dropping the handle instead would detach. + if let Some(task) = self.0.take() { + task.cancel(); + } } } +/// A spawned task whose panic comes back as a value rather than a rethrow. +/// +/// tokio reported a task's panic as a `JoinError` on its handle; nagoya +/// rethrows it in whoever awaits the handle. Catching inside the task keeps +/// the old contract: a panicking driver becomes [`Error::Interrupted`] from +/// [`Run::finish`], and a panicking stderr reader an empty capture, rather than +/// a panic in the caller. The outer `Option` is nagoya's own: `None` means the +/// task was cancelled before it produced anything. +type Caught = nagoya::JoinHandle>; + +/// Spawn on nagoya's shared pool with the task's panic caught. See [`Caught`]. +fn spawn_caught(future: F) -> Caught +where + F: Future + Send + 'static, + F::Output: Send + 'static, +{ + nagoya::spawn(std::panic::AssertUnwindSafe(future).catch_unwind()) +} + +/// The event channel, holding at most [`EVENT_BUFFER`] events. +/// +/// A futures bounded channel admits `buffer` messages plus one per sender, +/// where tokio's admitted exactly `buffer`. The driver is the only sender, so +/// asking for one fewer keeps the queue depth tokio had. +fn event_channel() -> (mpsc::Sender, mpsc::Receiver) { + mpsc::channel(EVENT_BUFFER - 1) +} + /// How many decisions may queue on the way back to the agent. /// /// Small on purpose: the agent asks one question at a time and waits, so a deep @@ -96,7 +131,7 @@ const EVENT_BUFFER: usize = 256; enum Control { Message { body: String, - receipt: tokio::sync::oneshot::Sender>, + receipt: oneshot::Sender>, }, Approval { id: String, @@ -138,11 +173,11 @@ pub struct Run { reaped: std::sync::Arc, /// Dropping or firing this asks the driver to tear down in order. Held as /// an `Option` so `detach` can discard it without signalling. - cancel: Option>, + cancel: Option>, /// `None` only after [`Run::finish`], [`Run::cancel`] or [`Run::detach`] /// has taken ownership, which is what stops `Drop` from aborting a run that /// was already settled deliberately. - task: Option>>, + task: Option>>, argv: Vec, } @@ -193,15 +228,25 @@ impl RunControl { let cancelled = || Error::Cancelled { bin: self.bin.clone(), }; - let (receipt, delivered) = tokio::sync::oneshot::channel(); + let (receipt, delivered) = oneshot::channel(); + // futures' `send` needs `&mut Sender` where tokio's took `&self`, so + // each call sends through its own clone. The future still resolves + // only once the queue is back within its bound, which is the + // backpressure tokio gave; the difference is that the message is + // already queued while it waits, so dropping this future no longer + // withdraws it. channel + .clone() .send(Control::Message { body: message.to_string(), receipt, }) .await .map_err(|_| cancelled())?; - match tokio::time::timeout(timeout, delivered).await { + // A dropped receipt sender is `Canceled` here where tokio said + // `RecvError`; both mean the run settled first, and both map to + // `Error::Cancelled` below. + match nagoya::timeout(timeout, delivered).await { Ok(receipt) => receipt.map_err(|_| cancelled())?, Err(_) => Err(Error::ControlTimeout { bin: self.bin.clone(), @@ -223,7 +268,9 @@ impl RunControl { what: "answering an approval on a run that did not request them", }); }; + // A clone per call for the reason given in `send_with_timeout`. channel + .clone() .send(Control::Approval { id: id.to_string(), decision: decision.clone(), @@ -238,7 +285,7 @@ impl RunControl { impl Run { /// The next event, or `None` once the agent has finished producing them. pub async fn recv(&mut self) -> Option { - self.events.recv().await + self.events.next().await } /// Clone the route used for follow-up messages and approval decisions. @@ -331,26 +378,16 @@ impl Run { pub async fn finish(mut self) -> Result { // The driver owns teardown from here; `Drop` must not also fire. self.pid = None; - while self.events.recv().await.is_some() {} + while self.events.next().await.is_some() {} // Taking the handle disarms the `Drop` guard: this run is settling // normally, not being abandoned. let Some(task) = self.task.take() else { unreachable!("the handle is only taken by a consuming method") }; - match task.await { - Ok(result) => result, - // The driver task panicked or was cancelled. The process itself - // started fine, so this is not a spawn failure and must not claim - // to be one. - Err(join) => Err(Error::Interrupted { - bin: self.argv.first().cloned().unwrap_or_default(), - detail: if join.is_panic() { - "the driver task panicked".into() - } else { - "the driver task was cancelled".into() - }, - }), - } + // The driver task panicked or was cancelled. The process itself + // started fine, so this is not a spawn failure and must not claim to + // be one. + joined(task.await, &self.argv) } /// Stop the run and wait until the agent is actually gone. @@ -376,17 +413,7 @@ impl Run { let Some(task) = self.task.take() else { unreachable!("the handle is only taken by a consuming method") }; - match task.await { - Ok(result) => result, - Err(join) => Err(Error::Interrupted { - bin: self.argv.first().cloned().unwrap_or_default(), - detail: if join.is_panic() { - "the driver task panicked".into() - } else { - "the driver task was cancelled".into() - }, - }), - } + joined(task.await, &self.argv) } /// Let the run continue after this handle goes away. @@ -403,11 +430,32 @@ impl Run { if let Some(cancel) = self.cancel.take() { std::mem::forget(cancel); } - // Dropping the handle without aborting is what detaches a tokio task. + // Dropping a nagoya handle without cancelling it detaches the task, + // exactly as dropping a tokio handle without aborting it did. drop(self.task.take()); } } +/// Read a driver task's result off its [`Caught`] handle. +/// +/// `None` is nagoya cancelling the task before it finished, and `Some(Err)` a +/// panic caught inside it: the two cases tokio's `JoinError` distinguished +/// with `is_panic`, reported with the same wording. +fn joined( + result: Option>>, + argv: &[String], +) -> Result { + let detail = match result { + Some(Ok(outcome)) => return outcome, + Some(Err(_panic)) => "the driver task panicked", + None => "the driver task was cancelled", + }; + Err(Error::Interrupted { + bin: argv.first().cloned().unwrap_or_default(), + detail: detail.into(), + }) +} + impl Drop for Run { fn drop(&mut self) { // Abandoned rather than finished, cancelled or detached. @@ -425,7 +473,7 @@ impl Drop for Run { } drop(self.cancel.take()); if let Some(task) = self.task.take() { - task.abort(); + task.cancel(); } } } @@ -458,14 +506,16 @@ fn redact(argv: &[crate::agent::Arg]) -> Vec { /// [`Error::Unsupported`] for a request that asked for approvals: this entry /// point discards events, so an approval request would reach nobody and the run /// would sit blocked until its timeout. Use [`stream`] instead. -pub async fn run(request: &Request) -> Result { +/// +/// `reactor` drives the child's pipes and exit; see [`stream`]. +pub async fn run(request: &Request, reactor: &nagoya::reactor::Handle) -> Result { if request.plan().approvals { return Err(Error::Unsupported { agent: request.agent, what: "approvals on a run whose events are discarded; use `stream`", }); } - stream(request)?.finish().await + stream(request, reactor)?.finish().await } /// Interrupt an orphaned active Codex turn without starting a replacement. @@ -478,7 +528,7 @@ pub async fn run(request: &Request) -> Result { /// # Errors /// Returns [`Error::Unsupported`] for another provider or a request that does /// not resume a session, and the ordinary spawn/protocol errors otherwise. -pub async fn interrupt(request: &Request) -> Result { +pub async fn interrupt(request: &Request, reactor: &nagoya::reactor::Handle) -> Result { if !matches!(request.agent, crate::Agent::Codex | crate::Agent::Grok) { return Err(Error::Unsupported { agent: request.agent, @@ -494,7 +544,7 @@ pub async fn interrupt(request: &Request) -> Result { let mut request = request.clone(); request.duplex = true; request.operation = crate::request::Operation::Interrupt; - let outcome = run(&request).await?; + let outcome = run(&request, reactor).await?; Ok(matches!(outcome.stop, Stop::Other(ref reason) if reason == "interrupted")) } @@ -502,6 +552,12 @@ pub async fn interrupt(request: &Request) -> Result { /// /// Returns as soon as the child is spawned; the work proceeds on a task. /// +/// `reactor` is the I/O driver the child's pipes and exit are registered +/// with. The caller owns it and must keep the [`nagoya::reactor::Reactor`] +/// behind it alive until the returned [`Run`] has settled: a dropped reactor +/// stops the thread that wakes these reads, and the run would stall. This +/// crate never starts one of its own. +/// /// # Errors /// [`Error::NotInstalled`] if the binary is missing, [`Error::Unsupported`] if /// the agent cannot honour the request, or [`Error::Spawn`] on an OS failure. @@ -509,11 +565,11 @@ pub async fn interrupt(request: &Request) -> Result { clippy::too_many_lines, reason = "one spawn boundary keeps command posture, pipes, process group, and driver selection together" )] -pub fn stream(request: &Request) -> Result { - // `tokio::spawn` panics outside a runtime. A fallible signature must not - // hide that, so the context is checked and reported as an ordinary error. - let runtime = tokio::runtime::Handle::try_current().map_err(|_| Error::NoRuntime)?; - +pub fn stream(request: &Request, reactor: &nagoya::reactor::Handle) -> Result { + // No runtime context to check: the driver goes to nagoya's shared pool, + // which starts on first use, and I/O goes through the caller's `reactor`, + // so this cannot fail the way `tokio::spawn` outside a runtime did, and + // `Error::NoRuntime` is no longer returned. let mut request = request.clone(); let session_lease = if let Some(binding) = &request.binding { let lease = binding.store.lease(&binding.project, &binding.name)?; @@ -610,8 +666,8 @@ pub fn stream(request: &Request) -> Result { // together. Killing only the CLI leaves the commands *it* spawned running: // a build, a test run, a server, still holding files and credentials after // the run is supposedly over. - // 0 means "make this child its own group leader". `tokio::process::Command` - // exposes this directly on unix. + // 0 means "make this child its own group leader". `nagoya::process::Command` + // exposes this directly on unix, as tokio's did. #[cfg(unix)] command.process_group(0); @@ -622,7 +678,7 @@ pub fn stream(request: &Request) -> Result { persist_session(request, &token)?; } - let child = command.spawn().map_err(|source| { + let child = command.spawn(reactor).map_err(|source| { // A missing binary is the common case and deserves an actionable error // with an install hint. Reading it off the spawn avoids resolving PATH // twice, and with it the window where the resolved path is replaced @@ -643,20 +699,24 @@ pub fn stream(request: &Request) -> Result { let request_agent = request.agent; let pid = child.id(); - let (tx, rx) = mpsc::channel(EVENT_BUFFER); + let (tx, rx) = event_channel(); // Only created for an approvals run, so `respond` can tell "no channel" from // "channel closed" and refuse the first rather than hanging on it. let (decisions_tx, decisions_rx) = if grok_acp || plan.duplex || plan.approvals { - let (tx, rx) = mpsc::channel::(APPROVAL_BUFFER); + // Less one for the sender `RunControl` holds, as in `event_channel`. + // Each send goes through a fresh clone (see `send_with_timeout`), and + // each clone adds one slot, so concurrent senders can together queue + // past this, one message each, before any of them resolves. + let (tx, rx) = mpsc::channel::(APPROVAL_BUFFER - 1); (Some(tx), Some(rx)) } else { (None, None) }; - let (cancel_tx, cancel_rx) = tokio::sync::oneshot::channel(); + let (cancel_tx, cancel_rx) = oneshot::channel(); let reaped = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); let reaped_for_task = std::sync::Arc::clone(&reaped); let request = request.clone(); - let task = runtime.spawn(async move { + let task = spawn_caught(async move { // Held across the whole read-run-commit cycle. It deliberately lives in // the driver task rather than `Run`, so `detach` keeps the session // reserved until the background agent actually exits. @@ -769,8 +829,8 @@ impl Drop for ChildGuard { async fn drive( child: Child, request: Request, - events: mpsc::Sender, - cancel: tokio::sync::oneshot::Receiver<()>, + mut events: mpsc::Sender, + cancel: oneshot::Receiver<()>, reaped: std::sync::Arc, decisions: Option>, ) -> Result { @@ -809,20 +869,27 @@ async fn drive( // Forwarding runs on its own task so a decision can be written while // stdout is being read. It ends on whichever comes first: the channel // closing, or the turn settling. - let (close_tx, mut close_rx) = tokio::sync::oneshot::channel::<()>(); + let (close_tx, close_rx) = oneshot::channel::<()>(); + // Fused so a dropped sender still wins its arm. A bare futures + // `oneshot::Receiver` reports itself terminated once its sender is + // gone, and `select!` skips a terminated arm, where tokio's + // `&mut close_rx` resolved with an error and broke the loop. + let mut close_rx = close_rx.fuse(); close_stdin = Some(close_tx); let decision_bin = bin.clone(); decision_task = decisions.map(|mut rx| { - tokio::spawn(async move { + nagoya::spawn(async move { loop { - tokio::select! { - biased; + // `select_biased!` is tokio's `biased;`: arms are polled + // in the order written. Plain `futures::select!` would + // shuffle them and lose the priority below. + futures::select_biased! { // Once the terminal record has arrived there is no // turn left to receive another message. Prioritizing // closure makes a simultaneous late send fail instead // of being reported as delivered to a finished turn. - _ = &mut close_rx => break, - reply = rx.recv() => { + _ = close_rx => break, + reply = rx.next() => { let Some(control) = reply else { break }; match control { Control::Message { body, receipt } => { @@ -877,10 +944,10 @@ async fn drive( // while stdout still has room. // Aborted on every exit path from here, so a forwarder never survives the // run it belongs to. - let _decision_guard = decision_task.map(AbortOnDrop); + let _decision_guard = decision_task.map(|task| AbortOnDrop(Some(task))); let stderr = child.child.stderr.take(); - let stderr_task = tokio::spawn(async move { + let stderr_task = spawn_caught(async move { let mut buf = String::new(); if let Some(handle) = stderr { let mut reader = BufReader::new(handle); @@ -954,17 +1021,20 @@ async fn drive( // rather than duplicating the whole select. let deadline = async { match request.timeout { - Some(limit) => tokio::time::sleep(limit).await, + Some(limit) => nagoya::sleep(limit).await, None => std::future::pending().await, } }; - let status = tokio::select! { + // `select_biased!` is tokio's `biased;`. The three futures are passed as + // expressions, not by name, so the macro owns and drops them before an + // arm's body runs, as tokio's `select!` did; that is what lets the bodies + // below borrow `child` and `parser` again. + let status = futures::select_biased! { // Biased so a finished run is reported as finished even if a deadline // or cancellation lands in the same tick. - biased; - result = work => result, - () = deadline => { + result = work.fuse() => result, + () = deadline.fuse() => { // Order matters: signal the group *before* reaping. Reaping clears // the child's pid, and the group kill needs that pid to target the // group, so the other order silently leaves grandchildren running. @@ -977,7 +1047,10 @@ async fn drive( }) .inspect_err(|_| drop(partial)); } - _ = cancel => { + // Fused because a bare futures `oneshot::Receiver` counts as terminated + // once its sender is dropped, and dropping the sender is exactly how + // `Run::cancel` and `Run::drop` signal; `select!` would skip the arm. + _ = cancel.fuse() => { // Cooperative teardown: the caller is waiting on this, so the tree // is signalled, reaped and joined before returning. shut_down(&mut child, stderr_task).await; @@ -996,7 +1069,10 @@ async fn drive( reaped.store(true, std::sync::atomic::Ordering::SeqCst); drop(events); - let stderr = stderr_task.await.unwrap_or_default(); + let stderr: String = stderr_task + .await + .and_then(std::thread::Result::ok) + .unwrap_or_default(); let saw_structured = parser.saw_structured_record(); let saw_terminal = parser.saw_terminal_record(); let terminal = parser.finish(); @@ -1133,8 +1209,8 @@ async fn drive( async fn drive_grok_acp( child: Child, request: Request, - events: mpsc::Sender, - cancel: tokio::sync::oneshot::Receiver<()>, + mut events: mpsc::Sender, + cancel: oneshot::Receiver<()>, reaped: std::sync::Arc, controls: Option>, ) -> Result { @@ -1161,7 +1237,7 @@ async fn drive_grok_acp( }; let stderr = child.child.stderr.take(); - let stderr_task = tokio::spawn(async move { + let stderr_task = spawn_caught(async move { let mut buf = String::new(); if let Some(handle) = stderr { let mut reader = BufReader::new(handle); @@ -1197,20 +1273,28 @@ async fn drive_grok_acp( let mut persist_result: Result<()> = Ok(()); let deadline = async { match request.timeout { - Some(limit) => tokio::time::sleep(limit).await, + Some(limit) => nagoya::sleep(limit).await, None => std::future::pending().await, } - }; - tokio::pin!(deadline); - tokio::pin!(cancel); + } + .fuse(); + futures::pin_mut!(deadline); + // Fused so a dropped sender still wins its arm; see `drive`. + let mut cancel = cancel.fuse(); while !protocol.finished { - tokio::select! { - biased; - control = controls.recv() => { - let Some(control) = control else { - continue; - }; + // `select_biased!` is tokio's `biased;`; see `drive`. `deadline` and + // `cancel` are named, so they persist across iterations as the pinned + // futures did; the read is an expression, rebuilt each pass and dropped + // before an arm's body runs, as under tokio. + futures::select_biased! { + // `select_next_some` rather than `next`: once every sender is gone + // the arm is skipped instead of yielding `None` on every pass. + // tokio's arm did yield `None`, and the `continue` it took then + // spun this loop until tokio's cooperative budget forced a yield; + // futures has no such budget, so a biased arm that is always ready + // would starve the reader outright. + control = controls.select_next_some() => { pending.push_back(control); flush_grok_controls( &mut protocol, @@ -1223,7 +1307,7 @@ async fn drive_grok_acp( bin: bin.clone(), source })?; } - record = read_bounded_line(&mut reader, &mut line) => { + record = read_bounded_line(&mut reader, &mut line).fuse() => { if record.map_err(|source| Error::Spawn { bin: bin.clone(), source })?.is_some() { append_capped(&mut raw, &line); if let Ok(value) = serde_json::from_str::(&line) { @@ -1266,7 +1350,7 @@ async fn drive_grok_acp( protocol.finished = true; } } - () = &mut deadline => { + () = deadline => { let partial = protocol.terminal.text.clone(); interrupt_grok_turn(&mut protocol, &mut stdin, &mut reader, &mut line, &mut raw) .await; @@ -1278,7 +1362,7 @@ async fn drive_grok_acp( partial, }); } - _ = &mut cancel => { + _ = cancel => { interrupt_grok_turn(&mut protocol, &mut stdin, &mut reader, &mut line, &mut raw) .await; shut_down(&mut child, stderr_task).await; @@ -1289,7 +1373,7 @@ async fn drive_grok_acp( } drop(stdin); - if tokio::time::timeout(std::time::Duration::from_secs(2), child.child.wait()) + if nagoya::timeout(std::time::Duration::from_secs(2), child.child.wait()) .await .is_err() { @@ -1299,7 +1383,10 @@ async fn drive_grok_acp( child.armed = false; reaped.store(true, std::sync::atomic::Ordering::SeqCst); drop(events); - let stderr = stderr_task.await.unwrap_or_default(); + let stderr: String = stderr_task + .await + .and_then(std::thread::Result::ok) + .unwrap_or_default(); persist_result?; if let Some(detail) = protocol.failure { @@ -1343,8 +1430,8 @@ async fn drive_grok_acp( async fn interrupt_grok_turn( protocol: &mut crate::grok_acp::Protocol, - stdin: &mut tokio::process::ChildStdin, - reader: &mut BufReader, + stdin: &mut ChildStdin, + reader: &mut BufReader, line: &mut String, raw: &mut String, ) { @@ -1365,14 +1452,14 @@ async fn interrupt_grok_turn( } } }; - let _ = tokio::time::timeout(std::time::Duration::from_secs(2), settle).await; + let _ = nagoya::timeout(std::time::Duration::from_secs(2), settle).await; } async fn flush_grok_controls( protocol: &mut crate::grok_acp::Protocol, pending: &mut VecDeque, - steer_receipts: &mut HashMap>>, - stdin: &mut tokio::process::ChildStdin, + steer_receipts: &mut HashMap>>, + stdin: &mut ChildStdin, bin: &str, ) -> Result<()> { let mut waiting = VecDeque::new(); @@ -1411,7 +1498,7 @@ async fn flush_grok_controls( } fn settle_grok_steers( - receipts: &mut HashMap>>, + receipts: &mut HashMap>>, responses: Vec, bin: &str, ) { @@ -1444,8 +1531,8 @@ fn settle_grok_steers( async fn drive_codex_app_server( child: Child, request: Request, - events: mpsc::Sender, - cancel: tokio::sync::oneshot::Receiver<()>, + mut events: mpsc::Sender, + cancel: oneshot::Receiver<()>, reaped: std::sync::Arc, controls: Option>, ) -> Result { @@ -1472,7 +1559,7 @@ async fn drive_codex_app_server( }; let stderr = child.child.stderr.take(); - let stderr_task = tokio::spawn(async move { + let stderr_task = spawn_caught(async move { let mut buf = String::new(); if let Some(handle) = stderr { let mut reader = BufReader::new(handle); @@ -1508,24 +1595,27 @@ async fn drive_codex_app_server( let mut persist_result: Result<()> = Ok(()); let deadline = async { match request.timeout { - Some(limit) => tokio::time::sleep(limit).await, + Some(limit) => nagoya::sleep(limit).await, None => std::future::pending().await, } - }; - tokio::pin!(deadline); - tokio::pin!(cancel); + } + .fuse(); + futures::pin_mut!(deadline); + // Fused so a dropped sender still wins its arm; see `drive`. + let mut cancel = cancel.fuse(); while !protocol.finished { - tokio::select! { - biased; + // `select_biased!` is tokio's `biased;`; see `drive_grok_acp` for why + // the arms are shaped the way they are. + futures::select_biased! { // User steering and approval answers outrank the agent's output. // app-server can keep stdout continuously ready with reasoning and // text deltas; reading it first in a biased select could starve a // correction precisely while Codex was busiest. - control = controls.recv() => { - let Some(control) = control else { - continue; - }; + // + // `select_next_some` skips a closed channel rather than spinning on + // its `None`, as `drive_grok_acp` explains. + control = controls.select_next_some() => { pending.push_back(control); flush_codex_controls( &mut protocol, @@ -1538,7 +1628,7 @@ async fn drive_codex_app_server( bin: bin.clone(), source })?; } - record = read_bounded_line(&mut reader, &mut line) => { + record = read_bounded_line(&mut reader, &mut line).fuse() => { if record.map_err(|source| Error::Spawn { bin: bin.clone(), source })?.is_some() { append_capped(&mut raw, &line); if let Ok(value) = serde_json::from_str::(&line) { @@ -1581,7 +1671,7 @@ async fn drive_codex_app_server( protocol.finished = true; } } - () = &mut deadline => { + () = deadline => { let partial = protocol.terminal.text.clone(); interrupt_codex_turn(&mut protocol, &mut stdin, &mut reader, &mut line, &mut raw) .await; @@ -1593,7 +1683,7 @@ async fn drive_codex_app_server( partial, }); } - _ = &mut cancel => { + _ = cancel => { interrupt_codex_turn(&mut protocol, &mut stdin, &mut reader, &mut line, &mut raw) .await; shut_down(&mut child, stderr_task).await; @@ -1607,7 +1697,7 @@ async fn drive_codex_app_server( // stop cleanly; the short fallback prevents a completed turn from hanging // because a future CLI release keeps serving after its input closes. drop(stdin); - if tokio::time::timeout(std::time::Duration::from_secs(2), child.child.wait()) + if nagoya::timeout(std::time::Duration::from_secs(2), child.child.wait()) .await .is_err() { @@ -1617,7 +1707,10 @@ async fn drive_codex_app_server( child.armed = false; reaped.store(true, std::sync::atomic::Ordering::SeqCst); drop(events); - let stderr = stderr_task.await.unwrap_or_default(); + let stderr: String = stderr_task + .await + .and_then(std::thread::Result::ok) + .unwrap_or_default(); persist_result?; if let Some(detail) = protocol.failure { @@ -1666,8 +1759,8 @@ async fn drive_codex_app_server( /// `turn/interrupt` it needs to make this thread resumable again. async fn interrupt_codex_turn( protocol: &mut crate::codex_app_server::Protocol, - stdin: &mut tokio::process::ChildStdin, - reader: &mut BufReader, + stdin: &mut ChildStdin, + reader: &mut BufReader, line: &mut String, raw: &mut String, ) { @@ -1696,7 +1789,7 @@ async fn interrupt_codex_turn( } } }; - let _ = tokio::time::timeout(std::time::Duration::from_secs(2), settle).await; + let _ = nagoya::timeout(std::time::Duration::from_secs(2), settle).await; } /// Write every control whose protocol ids are available, preserving earlier @@ -1704,8 +1797,8 @@ async fn interrupt_codex_turn( async fn flush_codex_controls( protocol: &mut crate::codex_app_server::Protocol, pending: &mut VecDeque, - steer_receipts: &mut HashMap>>, - stdin: &mut tokio::process::ChildStdin, + steer_receipts: &mut HashMap>>, + stdin: &mut ChildStdin, bin: &str, ) -> Result<()> { let mut waiting = VecDeque::new(); @@ -1748,7 +1841,7 @@ async fn flush_codex_controls( /// turn completes is deliberate: the caller receives `Error::Cancelled` and /// can put the message into a fresh resumed turn. fn settle_codex_steers( - receipts: &mut HashMap>>, + receipts: &mut HashMap>>, responses: Vec, bin: &str, ) { @@ -1773,14 +1866,17 @@ fn settle_codex_steers( /// /// The orderly teardown both cancellation and timeout share. Returns whatever /// stderr had been captured, so a caller can still report why a run was stopped. -async fn shut_down(child: &mut ChildGuard, stderr_task: tokio::task::JoinHandle) -> String { +async fn shut_down(child: &mut ChildGuard, stderr_task: Caught) -> String { kill_process_group(&child.child); // Reap, so the caller is not left with a zombie once this returns. let _ = child.child.kill().await; child.armed = false; // The pipes are closed now that the child is gone, so this finishes // promptly rather than hanging the cancellation. - stderr_task.await.unwrap_or_default() + stderr_task + .await + .and_then(std::thread::Result::ok) + .unwrap_or_default() } /// Turn a failure into the most specific error available, agent included so an @@ -2757,18 +2853,33 @@ mod tests { assert!(safe.contains(&"resume".to_string())); } - /// `stream` is synchronous but spawns a task. Outside a runtime that would - /// panic, which a `Result`-returning function must not do. + /// `stream` is synchronous but spawns a task. Under tokio that needed an + /// ambient runtime and reported `Error::NoRuntime` without one. nagoya's + /// shared pool needs no context and the reactor is passed in, so outside + /// any executor the call gets as far as the spawn and reports what is + /// actually wrong. #[test] - fn stream_outside_a_runtime_errors_instead_of_panicking() { - let err = stream(&crate::Request::new(Agent::Claude, "hi")).unwrap_err(); - assert!(matches!(err, Error::NoRuntime), "got {err:?}"); + fn stream_needs_no_ambient_runtime() { + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + let request = Request::new(Agent::Claude, "hi").bin("definitely-not-a-real-binary-xyz"); + let err = stream(&request, &reactor.handle()).unwrap_err(); + assert!(matches!(err, Error::NotInstalled { .. }), "got {err:?}"); } - #[tokio::test] - async fn a_missing_binary_names_the_install_command() { + /// The handles a host moves between tasks and threads must stay `Send` and + /// `Sync` whatever the channel and task types underneath them are. + #[test] + fn run_handles_are_send_and_sync() { + fn assert_send_sync() {} + assert_send_sync::(); + assert_send_sync::(); + } + + #[test] + fn a_missing_binary_names_the_install_command() { + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); let request = Request::new(Agent::Claude, "hi").bin("definitely-not-a-real-binary-xyz"); - let err = run(&request).await.unwrap_err(); + let err = nagoya::block_on(run(&request, &reactor.handle())).unwrap_err(); let Error::NotInstalled { hint, agent, .. } = err else { panic!("expected NotInstalled, got {err:?}") }; @@ -2780,96 +2891,102 @@ mod tests { /// behind more events than fit in the bounded channel. Input must wait on a /// handle independent from the mutable event receiver, or the producer and /// consumer block each other permanently. - #[tokio::test] - async fn a_control_receipt_can_wait_behind_a_full_event_buffer() { - let (events_tx, events_rx) = mpsc::channel(EVENT_BUFFER); - let (controls_tx, mut controls_rx) = mpsc::channel(1); - let (cancel_tx, cancel_rx) = tokio::sync::oneshot::channel(); - let task = tokio::spawn(async move { - let _cancel_rx = cancel_rx; - let Some(Control::Message { receipt, .. }) = controls_rx.recv().await else { - panic!("follow-up message") + #[test] + fn a_control_receipt_can_wait_behind_a_full_event_buffer() { + nagoya::block_on(async { + // The production channel, so "full" means the depth a real run has: + // one more event than it holds is sent below. + let (mut events_tx, events_rx) = event_channel(); + let (controls_tx, mut controls_rx) = mpsc::channel(1); + let (cancel_tx, cancel_rx) = oneshot::channel(); + let task = spawn_caught(async move { + let _cancel_rx = cancel_rx; + let Some(Control::Message { receipt, .. }) = controls_rx.next().await else { + panic!("follow-up message") + }; + for index in 0..=EVENT_BUFFER { + events_tx + .send(Event::Thinking(index.to_string())) + .await + .expect("the host keeps draining events"); + } + let _ = receipt.send(Ok(())); + Ok(Outcome { + agent: Agent::Codex, + session: None, + text: String::new(), + usage: crate::Usage::default(), + stop: Stop::Completed, + rate_limit: None, + exit_code: 0, + stderr: String::new(), + unparsed: 0, + first_unparsed: None, + structured: None, + }) + }); + let reaped = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); + let mut run = Run { + events: events_rx, + control: RunControl { + agent: Agent::Codex, + to_agent: Some(controls_tx), + bin: "codex".into(), + }, + typed: Vec::new(), + pid: None, + reaped, + cancel: Some(cancel_tx), + task: Some(task), + argv: vec!["codex".into()], }; - for index in 0..=EVENT_BUFFER { - events_tx - .send(Event::Thinking(index.to_string())) - .await - .expect("the host keeps draining events"); - } - let _ = receipt.send(Ok(())); - Ok(Outcome { - agent: Agent::Codex, - session: None, - text: String::new(), - usage: crate::Usage::default(), - stop: Stop::Completed, - rate_limit: None, - exit_code: 0, - stderr: String::new(), - unparsed: 0, - first_unparsed: None, - structured: None, - }) - }); - let reaped = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(true)); - let mut run = Run { - events: events_rx, - control: RunControl { - agent: Agent::Codex, - to_agent: Some(controls_tx), - bin: "codex".into(), - }, - typed: Vec::new(), - pid: None, - reaped, - cancel: Some(cancel_tx), - task: Some(task), - argv: vec!["codex".into()], - }; - let control = run.control(); - let delivery = tokio::spawn(async move { control.send("change course").await }); - let mut seen = 0; - while run.recv().await.is_some() { - seen += 1; - } + let control = run.control(); + let delivery = nagoya::spawn(async move { control.send("change course").await }); + let mut seen = 0; + while run.recv().await.is_some() { + seen += 1; + } - assert_eq!(seen, EVENT_BUFFER + 1); - delivery - .await - .expect("delivery task") - .expect("transport receipt"); - run.finish().await.expect("run outcome"); + assert_eq!(seen, EVENT_BUFFER + 1); + delivery + .await + .expect("delivery task") + .expect("transport receipt"); + run.finish().await.expect("run outcome"); + }); } /// A live app-server can keep its pipes open after it stops processing /// requests. Input delivery needs a deadline so the host can reap that run /// and retry the visible message on the resumed session. - #[tokio::test] - async fn a_control_receipt_that_never_arrives_times_out() { - let (controls_tx, mut controls_rx) = mpsc::channel(1); - let control = RunControl { - agent: Agent::Codex, - to_agent: Some(controls_tx), - bin: "codex".into(), - }; - let receiver = tokio::spawn(async move { - let Some(Control::Message { receipt, .. }) = controls_rx.recv().await else { - panic!("follow-up message") + #[test] + fn a_control_receipt_that_never_arrives_times_out() { + nagoya::block_on(async { + let (controls_tx, mut controls_rx) = mpsc::channel(1); + let control = RunControl { + agent: Agent::Codex, + to_agent: Some(controls_tx), + bin: "codex".into(), }; - tokio::time::sleep(Duration::from_secs(1)).await; - drop(receipt); - }); + let receiver = nagoya::spawn(async move { + let Some(Control::Message { receipt, .. }) = controls_rx.next().await else { + panic!("follow-up message") + }; + nagoya::sleep(Duration::from_secs(1)).await; + drop(receipt); + }); - let err = control - .send_with_timeout("are you there?", Duration::from_millis(10)) - .await - .expect_err("the missing receipt must not wait forever"); - assert!( - matches!(err, Error::ControlTimeout { .. }), - "expected ControlTimeout, got {err:?}" - ); - receiver.abort(); + let err = control + .send_with_timeout("are you there?", Duration::from_millis(10)) + .await + .expect_err("the missing receipt must not wait forever"); + assert!( + matches!(err, Error::ControlTimeout { .. }), + "expected ControlTimeout, got {err:?}" + ); + receiver.cancel(); + }); } #[test] diff --git a/tests/live.rs b/tests/live.rs index 3017bd6..1d37216 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -45,20 +45,27 @@ fn ping(agent: Agent) -> Request { } } -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn claude_answers_and_reports_usage() { - if !available(Agent::Claude) { - return; - } - let outcome = run(&ping(Agent::Claude)).await.expect("claude run failed"); - - assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); - assert_eq!(outcome.text.trim().to_lowercase(), "pong"); - assert!(outcome.session.is_some(), "claude must report a session id"); - // Claude prices its own runs, so both tokens and cost should be present. - assert!(outcome.usage.output_tokens.is_some(), "{:?}", outcome.usage); - assert!(outcome.usage.cost_usd.is_some(), "{:?}", outcome.usage); +fn claude_answers_and_reports_usage() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let outcome = run(&ping(Agent::Claude), &reactor.handle()) + .await + .expect("claude run failed"); + + assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); + assert_eq!(outcome.text.trim().to_lowercase(), "pong"); + assert!(outcome.session.is_some(), "claude must report a session id"); + // Claude prices its own runs, so both tokens and cost should be present. + assert!(outcome.usage.output_tokens.is_some(), "{:?}", outcome.usage); + assert!(outcome.usage.cost_usd.is_some(), "{:?}", outcome.usage); + }); } /// The failure this crate most has to get right: claude exits **0** for an @@ -67,60 +74,70 @@ async fn claude_answers_and_reports_usage() { /// with the selected model" as the model's reply. /// /// Cheap: the turn is refused before any tokens are spent. -#[tokio::test] +#[test] #[ignore = "spawns a real agent"] -async fn an_unknown_model_is_an_error_not_an_answer() { - if !available(Agent::Claude) { - return; - } - let request = ping(Agent::Claude).model("bogus-model-xyz"); - let err = run(&request) - .await - .expect_err("an unknown model must not come back as an answer"); - - let agent_abstraction::Error::AgentError { - status, message, .. - } = &err - else { - panic!("expected AgentError, got {err:?}") - }; - // Not asserted as exactly 404: the status is passed through from the - // provider, and the point is that whatever it sends survives. - assert!( - status.is_some(), - "the provider status should survive: {err:?}" - ); - assert!( - message.to_lowercase().contains("model"), - "the agent's own wording should survive: {message}" - ); +fn an_unknown_model_is_an_error_not_an_answer() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let request = ping(Agent::Claude).model("bogus-model-xyz"); + let err = run(&request, &reactor.handle()) + .await + .expect_err("an unknown model must not come back as an answer"); + + let agent_abstraction::Error::AgentError { + status, message, .. + } = &err + else { + panic!("expected AgentError, got {err:?}") + }; + // Not asserted as exactly 404: the status is passed through from the + // provider, and the point is that whatever it sends survives. + assert!( + status.is_some(), + "the provider status should survive: {err:?}" + ); + assert!( + message.to_lowercase().contains("model"), + "the agent's own wording should survive: {message}" + ); + }); } /// Codex fails the same way and adds a wrinkle: it forwards the upstream error /// body as a JSON string, so without unwrapping, the caller is handed JSON /// instead of a sentence. -#[tokio::test] +#[test] #[ignore = "spawns a real agent"] -async fn codex_reports_an_unknown_model_as_a_sentence_not_json() { - if !available(Agent::Codex) { - return; - } - let request = ping(Agent::Codex).model("bogus-model-xyz"); - let err = run(&request) - .await - .expect_err("an unknown model must not come back as an answer"); +fn codex_reports_an_unknown_model_as_a_sentence_not_json() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let request = ping(Agent::Codex).model("bogus-model-xyz"); + let err = run(&request, &reactor.handle()) + .await + .expect_err("an unknown model must not come back as an answer"); - let agent_abstraction::Error::AgentError { message, .. } = &err else { - panic!("expected AgentError, got {err:?}") - }; - assert!( - !message.trim_start().starts_with('{'), - "the upstream envelope should have been unwrapped: {message}" - ); - assert!( - message.to_lowercase().contains("model"), - "the agent's own wording should survive: {message}" - ); + let agent_abstraction::Error::AgentError { message, .. } = &err else { + panic!("expected AgentError, got {err:?}") + }; + assert!( + !message.trim_start().starts_with('{'), + "the upstream envelope should have been unwrapped: {message}" + ); + assert!( + message.to_lowercase().contains("model"), + "the agent's own wording should survive: {message}" + ); + }); } /// The compiled-in catalogue is a snapshot and the CLI is the authority. Where @@ -128,52 +145,62 @@ async fn codex_reports_an_unknown_model_as_a_sentence_not_json() { /// user picking a model that no longer exists. /// /// Cheap: `codex debug models` spends no tokens. -#[tokio::test] -async fn the_codex_catalogue_still_matches_what_codex_reports() { - if !available(Agent::Codex) { - return; - } - let discovered = Agent::Codex - .discover_models() - .await - .expect("codex should list its own models"); - let compiled = Agent::Codex.models(); - - let ids = |models: &[agent_abstraction::Model]| -> Vec { - models.iter().map(|m| m.id.to_string()).collect() - }; - assert_eq!( - ids(&discovered), - ids(&compiled), - "codex's model list has moved; update `codex_models` in src/model.rs" - ); - assert!( - !discovered.iter().any(|m| m.id == "codex-auto-review"), - "a model codex marks hidden must not reach a picker" - ); - assert!( - !discovered.iter().any(|m| m.id == "gpt-5.3-codex-spark"), - "a visible model codex marks unsupported in the API must not reach a picker" - ); +#[test] +fn the_codex_catalogue_still_matches_what_codex_reports() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let discovered = Agent::Codex + .discover_models(&reactor.handle()) + .await + .expect("codex should list its own models"); + let compiled = Agent::Codex.models(); + + let ids = |models: &[agent_abstraction::Model]| -> Vec { + models.iter().map(|m| m.id.to_string()).collect() + }; + assert_eq!( + ids(&discovered), + ids(&compiled), + "codex's model list has moved; update `codex_models` in src/model.rs" + ); + assert!( + !discovered.iter().any(|m| m.id == "codex-auto-review"), + "a model codex marks hidden must not reach a picker" + ); + assert!( + !discovered.iter().any(|m| m.id == "gpt-5.3-codex-spark"), + "a visible model codex marks unsupported in the API must not reach a picker" + ); + }); } /// Both of these have an interactive picker and no headless listing, so asking /// must fail rather than quietly hand back the compiled list. -#[tokio::test] -async fn agents_without_a_headless_listing_refuse_to_guess() { - for agent in [Agent::Claude, Agent::Copilot] { - if !available(agent) { - continue; +#[test] +fn agents_without_a_headless_listing_refuse_to_guess() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in [Agent::Claude, Agent::Copilot] { + if !available(agent) { + continue; + } + let err = agent + .discover_models(&reactor.handle()) + .await + .expect_err("should not invent a model list"); + assert!( + matches!(err, agent_abstraction::Error::Unsupported { .. }), + "{agent} should report it cannot enumerate models, got {err:?}" + ); } - let err = agent - .discover_models() - .await - .expect_err("should not invent a model list"); - assert!( - matches!(err, agent_abstraction::Error::Unsupported { .. }), - "{agent} should report it cannot enumerate models, got {err:?}" - ); - } + }); } /// The catalogue advertises effort levels; this proves each CLI actually takes @@ -185,21 +212,26 @@ async fn agents_without_a_headless_listing_refuse_to_guess() { /// rejects the flag outright; that is asserted separately below. /// /// Each agent runs at the lowest level it documents, which is also the cheapest. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn effort_reaches_the_agents_that_take_it() { - for agent in [Agent::Claude, Agent::Codex] { - if !available(agent) { - continue; +fn effort_reaches_the_agents_that_take_it() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in [Agent::Claude, Agent::Codex] { + if !available(agent) { + continue; + } + let outcome = run(&ping(agent).effort("low"), &reactor.handle()) + .await + .unwrap_or_else(|e| panic!("{agent} rejected effort low: {e}")); + assert!( + outcome.is_ok(), + "{agent} did not finish cleanly at effort low: {outcome:?}" + ); } - let outcome = run(&ping(agent).effort("low")) - .await - .unwrap_or_else(|e| panic!("{agent} rejected effort low: {e}")); - assert!( - outcome.is_ok(), - "{agent} did not finish cleanly at effort low: {outcome:?}" - ); - } + }); } /// Effort support is not uniform across an agent even where `--help` documents @@ -212,37 +244,50 @@ async fn effort_reaches_the_agents_that_take_it() { /// Which is why the catalogue leaves `auto` with no levels. This asserts the /// behaviour that decision rests on, so a future Copilot release that starts /// accepting it shows up here. -#[tokio::test] +#[test] #[ignore = "spawns a real agent"] -async fn copilot_auto_still_refuses_an_effort() { - if !available(Agent::Copilot) { - return; - } - let err = run(&ping(Agent::Copilot).model("auto").effort("low")) +fn copilot_auto_still_refuses_an_effort() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Copilot) { + return; + } + let err = run( + &ping(Agent::Copilot).model("auto").effort("low"), + &reactor.handle(), + ) .await .expect_err("auto should still refuse an effort"); - assert!( - err.to_string().to_lowercase().contains("reasoning effort"), - "the CLI's own complaint should survive: {err}" - ); + assert!( + err.to_string().to_lowercase().contains("reasoning effort"), + "the CLI's own complaint should survive: {err}" + ); + }); } /// The levels are the provider's to define, so a bad one must surface rather /// than be swallowed. Cheap: refused before any tokens are spent. -#[tokio::test] +#[test] #[ignore = "spawns a real agent"] -async fn a_bogus_effort_is_reported_not_ignored() { - if !available(Agent::Codex) { - return; - } - let err = run(&ping(Agent::Codex).effort("bogus-level")) - .await - .expect_err("an invalid effort must not pass silently"); - let message = err.to_string().to_lowercase(); - assert!( - message.contains("effort") || message.contains("invalid"), - "the provider's complaint should survive: {err}" - ); +fn a_bogus_effort_is_reported_not_ignored() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let err = run(&ping(Agent::Codex).effort("bogus-level"), &reactor.handle()) + .await + .expect_err("an invalid effort must not pass silently"); + let message = err.to_string().to_lowercase(); + assert!( + message.contains("effort") || message.contains("invalid"), + "the provider's complaint should survive: {err}" + ); + }); } /// The false-termination regression, which only the real CLI can prove. @@ -255,130 +300,157 @@ async fn a_bogus_effort_is_reported_not_ignored() { /// /// The prompt asks for the trigger phrases deliberately. A pass is an ordinary /// successful outcome whose text contains them. -#[tokio::test] +#[test] #[ignore = "spawns a real agent"] -async fn an_answer_about_limits_and_login_is_not_a_failed_run() { - if !available(Agent::Claude) { - return; - } - let prompt = "In three sentences of plain prose, explain the difference between \ - a provider rate limit and an authentication failure. Use the exact \ - phrases \"rate limit\", \"usage limit\", \"not authenticated\" and \ - \"please run /login\" somewhere in your answer. Do not use a list."; - let request = Request::new(Agent::Claude, prompt) - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)); - let outcome = run(&request) - .await - .expect("an answer about limits and login must not fail the run"); - let text = outcome.text.to_lowercase(); - assert!( - text.contains("rate limit") || text.contains("usage limit"), - "the model did not use the trigger wording, so this proved nothing: {}", - outcome.text - ); - assert!( - !outcome.text.trim().is_empty(), - "the answer survived classification but arrived empty" - ); +fn an_answer_about_limits_and_login_is_not_a_failed_run() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let prompt = "In three sentences of plain prose, explain the difference between \ + a provider rate limit and an authentication failure. Use the exact \ + phrases \"rate limit\", \"usage limit\", \"not authenticated\" and \ + \"please run /login\" somewhere in your answer. Do not use a list."; + let request = Request::new(Agent::Claude, prompt) + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)); + let outcome = run(&request, &reactor.handle()) + .await + .expect("an answer about limits and login must not fail the run"); + let text = outcome.text.to_lowercase(); + assert!( + text.contains("rate limit") || text.contains("usage limit"), + "the model did not use the trigger wording, so this proved nothing: {}", + outcome.text + ); + assert!( + !outcome.text.trim().is_empty(), + "the answer survived classification but arrived empty" + ); + }); } /// Cheap and free of tokens: `codex app-server` answers from the account, not /// the model. Pins the shape a quota panel depends on. -#[tokio::test] -async fn codex_reports_account_usage_without_a_terminal() { - if !available(Agent::Codex) { - return; - } - assert!(Agent::Codex.reports_account_usage()); - let usage = Agent::Codex - .account_usage() - .await - .expect("codex should report account usage"); +#[test] +fn codex_reports_account_usage_without_a_terminal() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + assert!(Agent::Codex.reports_account_usage()); + let usage = Agent::Codex + .account_usage(&reactor.handle()) + .await + .expect("codex should report account usage"); - assert!( - usage.plan.is_some(), - "a plan name should survive: {usage:?}" - ); - let window = usage - .windows - .first() - .unwrap_or_else(|| panic!("at least one quota window: {usage:?}")); - assert!( - window.used_percent.is_some(), - "a percentage is the point of asking: {window:?}" - ); - assert!(window.window_minutes.is_some(), "{window:?}"); + assert!( + usage.plan.is_some(), + "a plan name should survive: {usage:?}" + ); + let window = usage + .windows + .first() + .unwrap_or_else(|| panic!("at least one quota window: {usage:?}")); + assert!( + window.used_percent.is_some(), + "a percentage is the point of asking: {window:?}" + ); + assert!(window.window_minutes.is_some(), "{window:?}"); + }); } /// Provider-level recovery without a model call. The caller supplies a Codex /// thread that currently has an orphaned `inProgress` turn; the test proves the /// one-shot control path reaches `turn/interrupt` without submitting a prompt. -#[tokio::test] +#[test] #[ignore = "requires AGENT_ABSTRACTION_INTERRUPT_SESSION naming an active Codex turn"] -async fn codex_interrupts_an_orphaned_active_turn() { - if !available(Agent::Codex) { - return; - } - let session = std::env::var("AGENT_ABSTRACTION_INTERRUPT_SESSION") - .expect("set AGENT_ABSTRACTION_INTERRUPT_SESSION to an active Codex thread id"); - let request = Request::new(Agent::Codex, "") - .resume(session) - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(20)); +fn codex_interrupts_an_orphaned_active_turn() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let session = std::env::var("AGENT_ABSTRACTION_INTERRUPT_SESSION") + .expect("set AGENT_ABSTRACTION_INTERRUPT_SESSION to an active Codex thread id"); + let request = Request::new(Agent::Codex, "") + .resume(session) + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(20)); - assert!( - interrupt(&request) - .await - .expect("Codex session interruption failed"), - "the supplied thread had no active turn" - ); + assert!( + interrupt(&request, &reactor.handle()) + .await + .expect("Codex session interruption failed"), + "the supplied thread had no active turn" + ); + }); } /// Long-lived idle threads used to exceed the bounded JSON line on resume and /// time out before the caller could learn that no recovery was needed. -#[tokio::test] +#[test] #[ignore = "requires AGENT_ABSTRACTION_IDLE_SESSION naming an idle Codex thread id"] -async fn codex_reads_a_long_idle_session_without_starting_a_turn() { - if !available(Agent::Codex) { - return; - } - let session = std::env::var("AGENT_ABSTRACTION_IDLE_SESSION") - .expect("set AGENT_ABSTRACTION_IDLE_SESSION to an idle Codex thread id"); - let request = Request::new(Agent::Codex, "") - .resume(session) - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(20)); +fn codex_reads_a_long_idle_session_without_starting_a_turn() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let session = std::env::var("AGENT_ABSTRACTION_IDLE_SESSION") + .expect("set AGENT_ABSTRACTION_IDLE_SESSION to an idle Codex thread id"); + let request = Request::new(Agent::Codex, "") + .resume(session) + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(20)); - assert!( - !interrupt(&request) - .await - .expect("Codex idle-session inspection failed"), - "the supplied thread unexpectedly had an active turn" - ); + assert!( + !interrupt(&request, &reactor.handle()) + .await + .expect("Codex idle-session inspection failed"), + "the supplied thread unexpectedly had an active turn" + ); + }); } /// Claude's context tracker, end to end: it is the one agent that reports the /// window alongside the tokens, so a share of it is knowable. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn claude_reports_enough_to_track_context() { - if !available(Agent::Claude) { - return; - } - let outcome = run(&ping(Agent::Claude)).await.expect("claude run failed"); - let usage = &outcome.usage; +fn claude_reports_enough_to_track_context() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let outcome = run(&ping(Agent::Claude), &reactor.handle()) + .await + .expect("claude run failed"); + let usage = &outcome.usage; - let context = usage.context_tokens.expect("context tokens"); - let window = usage.context_window.expect("context window"); - assert!(context > 0 && window > 0, "{usage:?}"); - assert!( - context <= window, - "context cannot exceed the window: {usage:?}" - ); - let share = usage.context_used().expect("both halves present"); - assert!((0.0..=1.0).contains(&share), "share out of range: {share}"); - assert!(usage.max_output_tokens.is_some(), "{usage:?}"); + let context = usage.context_tokens.expect("context tokens"); + let window = usage.context_window.expect("context window"); + assert!(context > 0 && window > 0, "{usage:?}"); + assert!( + context <= window, + "context cannot exceed the window: {usage:?}" + ); + let share = usage.context_used().expect("both halves present"); + assert!((0.0..=1.0).contains(&share), "share out of range: {share}"); + assert!(usage.max_output_tokens.is_some(), "{usage:?}"); + }); } /// The accounting difference this release exists to fix. Codex sends the whole @@ -388,126 +460,150 @@ async fn claude_reports_enough_to_track_context() { /// the 200k of the Haiku helper that claude lists first in its per-model /// usage. This is the regression test for sessions presenting as capped at /// 200k. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_1m_alias_reports_the_widened_window() { - if !available(Agent::Claude) { - return; - } - let request = Request::new(Agent::Claude, PING) - .model("sonnet[1m]") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)); - let outcome = run(&request).await.expect("sonnet[1m] run failed"); +fn a_1m_alias_reports_the_widened_window() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let request = Request::new(Agent::Claude, PING) + .model("sonnet[1m]") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)); + let outcome = run(&request, &reactor.handle()) + .await + .expect("sonnet[1m] run failed"); - assert_eq!( - outcome.usage.context_window, - Some(1_000_000), - "the widened window should be reported: {:?}", - outcome.usage - ); - let share = outcome.usage.context_used().expect("both halves present"); - assert!( - share < 0.1, - "a fresh session should be far from full: {share}" - ); + assert_eq!( + outcome.usage.context_window, + Some(1_000_000), + "the widened window should be reported: {:?}", + outcome.usage + ); + let share = outcome.usage.context_used().expect("both halves present"); + assert!( + share < 0.1, + "a fresh session should be far from full: {share}" + ); + }); } -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_token_parts_reconcile_with_the_total_it_reported() { - if !available(Agent::Codex) { - return; - } - let outcome = run(&ping(Agent::Codex)).await.expect("codex run failed"); - let usage = &outcome.usage; - - let context = usage.context_tokens.expect("context tokens"); - let uncached = usage.input_tokens.expect("input tokens"); - let cached = usage.cache_read_tokens.unwrap_or(0); - assert_eq!( - uncached + cached, - context, - "normalized input plus cache should equal the prompt codex reported: {usage:?}" - ); - assert!( - usage.reasoning_tokens.is_some(), - "codex separates reasoning tokens: {usage:?}" - ); +fn codex_token_parts_reconcile_with_the_total_it_reported() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let outcome = run(&ping(Agent::Codex), &reactor.handle()) + .await + .expect("codex run failed"); + let usage = &outcome.usage; + + let context = usage.context_tokens.expect("context tokens"); + let uncached = usage.input_tokens.expect("input tokens"); + let cached = usage.cache_read_tokens.unwrap_or(0); + assert_eq!( + uncached + cached, + context, + "normalized input plus cache should equal the prompt codex reported: {usage:?}" + ); + assert!( + usage.reasoning_tokens.is_some(), + "codex separates reasoning tokens: {usage:?}" + ); + }); } /// Copilot reports spend in AI credits, the unit that replaced premium /// requests, on its own event rather than only at the end. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn copilot_reports_its_credit_spend() { - if !available(Agent::Copilot) { - return; - } - let outcome = run(&ping(Agent::Copilot)) - .await - .expect("copilot run failed"); - assert!( - outcome.usage.ai_credits_nano.is_some(), - "the checkpoint event should reach the outcome: {:?}", - outcome.usage - ); - assert!(outcome.usage.duration_ms.is_some(), "{:?}", outcome.usage); +fn copilot_reports_its_credit_spend() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Copilot) { + return; + } + let outcome = run(&ping(Agent::Copilot), &reactor.handle()) + .await + .expect("copilot run failed"); + assert!( + outcome.usage.ai_credits_nano.is_some(), + "the checkpoint event should reach the outcome: {:?}", + outcome.usage + ); + assert!(outcome.usage.duration_ms.is_some(), "{:?}", outcome.usage); + }); } /// The whole human-in-the-loop round trip against the real CLI: a gated tool /// call arrives as an event, the decision goes back mid-turn, and the denial is /// honoured. The file is the proof: if the answer had not reached claude, it /// would exist. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_denied_approval_stops_the_tool_from_running() { - if !available(Agent::Claude) { - return; - } - let dir = - std::env::temp_dir().join(format!("agent-abstraction-approval-{}", std::process::id())); - std::fs::create_dir_all(&dir).expect("temp dir"); - let target = dir.join("must-not-exist.txt"); - let _ = std::fs::remove_file(&target); - - let request = Request::new( - Agent::Claude, - "Use the Bash tool to run exactly: touch must-not-exist.txt", - ) - .model("haiku") - .cwd(&dir) - // Not `ReadOnly`: that removes Bash outright, so there would be nothing - // to be asked about. Here the human is the gate instead. - .permission(Permission::Edit) - .approvals() - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("stream should start"); - let mut asked = Vec::new(); - while let Some(event) = run.recv().await { - if let Event::ApprovalRequest(approval) = event { - asked.push(approval.tool.clone()); - run.respond(&approval.id, &agent_abstraction::Decision::deny()) - .await - .expect("the decision should reach claude"); +fn a_denied_approval_stops_the_tool_from_running() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; } - } - let outcome = run.finish().await.expect("a denial is not a failed run"); + let dir = + std::env::temp_dir().join(format!("agent-abstraction-approval-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("temp dir"); + let target = dir.join("must-not-exist.txt"); + let _ = std::fs::remove_file(&target); + + let request = Request::new( + Agent::Claude, + "Use the Bash tool to run exactly: touch must-not-exist.txt", + ) + .model("haiku") + .cwd(&dir) + // Not `ReadOnly`: that removes Bash outright, so there would be nothing + // to be asked about. Here the human is the gate instead. + .permission(Permission::Edit) + .approvals() + .timeout(Duration::from_secs(180)); - assert!( - asked.iter().any(|tool| tool == "Bash"), - "claude should have asked before running a mutating command, asked: {asked:?}" - ); - assert!( - !target.exists(), - "the denial was not honoured: the file was created anyway" - ); - assert!( - outcome.is_ok(), - "the turn should still complete: {outcome:?}" - ); - let _ = std::fs::remove_dir_all(&dir); + let mut run = stream(&request, &reactor.handle()).expect("stream should start"); + let mut asked = Vec::new(); + while let Some(event) = run.recv().await { + if let Event::ApprovalRequest(approval) = event { + asked.push(approval.tool.clone()); + run.respond(&approval.id, &agent_abstraction::Decision::deny()) + .await + .expect("the decision should reach claude"); + } + } + let outcome = run.finish().await.expect("a denial is not a failed run"); + + assert!( + asked.iter().any(|tool| tool == "Bash"), + "claude should have asked before running a mutating command, asked: {asked:?}" + ); + assert!( + !target.exists(), + "the denial was not honoured: the file was created anyway" + ); + assert!( + outcome.is_ok(), + "the turn should still complete: {outcome:?}" + ); + let _ = std::fs::remove_dir_all(&dir); + }); } /// The point of `interactive`: a correction typed mid-turn actually redirects @@ -516,529 +612,600 @@ async fn a_denied_approval_stops_the_tool_from_running() { /// /// The timing is generous: the message only has to arrive before the third /// command, and each is an eight second sleep. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_message_sent_mid_turn_redirects_the_agent() { - if !available(Agent::Claude) { - return; - } - let dir = std::env::temp_dir().join(format!("agent-abstraction-duplex-{}", std::process::id())); - std::fs::create_dir_all(&dir).expect("temp dir"); - for name in ["step-b.txt", "step-c.txt"] { - let _ = std::fs::remove_file(dir.join(name)); - } +fn a_message_sent_mid_turn_redirects_the_agent() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let dir = + std::env::temp_dir().join(format!("agent-abstraction-duplex-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("temp dir"); + for name in ["step-b.txt", "step-c.txt"] { + let _ = std::fs::remove_file(dir.join(name)); + } - let request = Request::new( - Agent::Claude, - "Using the Bash tool, run these three commands ONE AT A TIME, in order: \ - `sleep 8`, then `touch step-b.txt`, then `touch step-c.txt`.", - ) - .model("haiku") - .cwd(&dir) - .permission(Permission::Bypass) - .interactive() - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("stream should start"); - let mut sent = false; - while let Some(event) = run.recv().await { - // Send once the agent is demonstrably working, so the message lands - // mid-turn rather than before the turn begins. - if !sent && matches!(event, Event::ToolCall { .. }) { - sent = true; - run.send("STOP. Do not run any more commands. Reply with only: ABORTED") - .await - .expect("the message should reach claude"); + let request = Request::new( + Agent::Claude, + "Using the Bash tool, run these three commands ONE AT A TIME, in order: \ + `sleep 8`, then `touch step-b.txt`, then `touch step-c.txt`.", + ) + .model("haiku") + .cwd(&dir) + .permission(Permission::Bypass) + .interactive() + .timeout(Duration::from_secs(180)); + + let mut run = stream(&request, &reactor.handle()).expect("stream should start"); + let mut sent = false; + while let Some(event) = run.recv().await { + // Send once the agent is demonstrably working, so the message lands + // mid-turn rather than before the turn begins. + if !sent && matches!(event, Event::ToolCall { .. }) { + sent = true; + run.send("STOP. Do not run any more commands. Reply with only: ABORTED") + .await + .expect("the message should reach claude"); + } } - } - let outcome = run - .finish() - .await - .expect("an interrupted turn still completes"); + let outcome = run + .finish() + .await + .expect("an interrupted turn still completes"); - assert!( - sent, - "the agent never called a tool, so nothing was interrupted" - ); - assert!( - !dir.join("step-c.txt").exists(), - "the third command ran, so the message did not redirect the agent: {outcome:?}" - ); - let _ = std::fs::remove_dir_all(&dir); + assert!( + sent, + "the agent never called a tool, so nothing was interrupted" + ); + assert!( + !dir.join("step-c.txt").exists(), + "the third command ran, so the message did not redirect the agent: {outcome:?}" + ); + let _ = std::fs::remove_dir_all(&dir); + }); } /// Codex uses app-server for an interactive turn. This proves the full live /// path rather than only its JSON shapes: a running tool call is steered, text /// arrives as deltas, and the app-server usage record carries the context /// window `AgencyZero` needs for its meter. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_app_server_accepts_a_live_steer() { - if !available(Agent::Codex) { - return; - } - let dir = std::env::temp_dir().join(format!( - "agent-abstraction-codex-duplex-{}", - std::process::id() - )); - std::fs::create_dir_all(&dir).expect("temp dir"); - let target = dir.join("must-not-exist.txt"); - let _ = std::fs::remove_file(&target); - - let request = Request::new( - Agent::Codex, - "Run `sleep 5` as one shell command. Wait for it to finish. Then, as a separate \ - command, run `touch must-not-exist.txt`. Do not combine the commands.", - ) - .cwd(&dir) - .permission(Permission::Bypass) - .interactive() - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("app-server should start"); - assert_eq!(run.argv(), ["codex", "app-server", "--stdio"]); - let mut sent = false; - let mut text_events = 0; - while let Some(event) = run.recv().await { - if matches!(event, Event::Text(_)) { - text_events += 1; +fn codex_app_server_accepts_a_live_steer() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; } - if !sent && matches!(event, Event::ToolCall { .. }) { - sent = true; - run.send("STOP. Do not run another command. Reply with only: ABORTED") - .await - .expect("turn/steer should accept the message"); + let dir = std::env::temp_dir().join(format!( + "agent-abstraction-codex-duplex-{}", + std::process::id() + )); + std::fs::create_dir_all(&dir).expect("temp dir"); + let target = dir.join("must-not-exist.txt"); + let _ = std::fs::remove_file(&target); + + let request = Request::new( + Agent::Codex, + "Run `sleep 5` as one shell command. Wait for it to finish. Then, as a separate \ + command, run `touch must-not-exist.txt`. Do not combine the commands.", + ) + .cwd(&dir) + .permission(Permission::Bypass) + .interactive() + .timeout(Duration::from_secs(180)); + + let mut run = stream(&request, &reactor.handle()).expect("app-server should start"); + assert_eq!(run.argv(), ["codex", "app-server", "--stdio"]); + let mut sent = false; + let mut text_events = 0; + while let Some(event) = run.recv().await { + if matches!(event, Event::Text(_)) { + text_events += 1; + } + if !sent && matches!(event, Event::ToolCall { .. }) { + sent = true; + run.send("STOP. Do not run another command. Reply with only: ABORTED") + .await + .expect("turn/steer should accept the message"); + } } - } - let outcome = run.finish().await.expect("steered turn should complete"); - assert!(sent, "Codex never called the first tool"); - assert!(text_events > 0, "assistant deltas were not streamed"); - assert!( - outcome.usage.context_window.is_some(), - "app-server did not report its context window: {:?}", - outcome.usage - ); - assert!( - !target.exists(), - "the second command ran, so the steer did not redirect Codex" - ); - let _ = std::fs::remove_dir_all(&dir); + let outcome = run.finish().await.expect("steered turn should complete"); + assert!(sent, "Codex never called the first tool"); + assert!(text_events > 0, "assistant deltas were not streamed"); + assert!( + outcome.usage.context_window.is_some(), + "app-server did not report its context window: {:?}", + outcome.usage + ); + assert!( + !target.exists(), + "the second command ran, so the steer did not redirect Codex" + ); + let _ = std::fs::remove_dir_all(&dir); + }); } /// A single declared cwd is the common project shape in `AgencyZero`. It must be /// writable without adding the same directory a second time as an extra root. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_app_server_can_write_inside_its_cwd() { - if !available(Agent::Codex) { - return; - } - let dir = std::env::temp_dir().join(format!( - "agent-abstraction-codex-writable-cwd-{}", - std::process::id() - )); - std::fs::create_dir_all(&dir).expect("temp dir"); - let target = dir.join("written-inside-cwd.txt"); - let _ = std::fs::remove_file(&target); - - let request = Request::new( - Agent::Codex, - "Create the file written-inside-cwd.txt in the current working directory. \ - Its exact contents must be: writable", - ) - .cwd(&dir) - .permission(Permission::Auto) - .interactive() - .timeout(Duration::from_secs(180)); - - let outcome = run(&request) - .await - .expect("Codex should write inside its declared cwd"); - assert!(outcome.is_ok(), "the turn did not complete: {outcome:?}"); - assert_eq!( - std::fs::read_to_string(&target).expect("Codex did not create the file"), - "writable" - ); - let _ = std::fs::remove_dir_all(&dir); +fn codex_app_server_can_write_inside_its_cwd() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let dir = std::env::temp_dir().join(format!( + "agent-abstraction-codex-writable-cwd-{}", + std::process::id() + )); + std::fs::create_dir_all(&dir).expect("temp dir"); + let target = dir.join("written-inside-cwd.txt"); + let _ = std::fs::remove_file(&target); + + let request = Request::new( + Agent::Codex, + "Create the file written-inside-cwd.txt in the current working directory. \ + Its exact contents must be: writable", + ) + .cwd(&dir) + .permission(Permission::Auto) + .interactive() + .timeout(Duration::from_secs(180)); + + let outcome = run(&request, &reactor.handle()) + .await + .expect("Codex should write inside its declared cwd"); + assert!(outcome.is_ok(), "the turn did not complete: {outcome:?}"); + assert_eq!( + std::fs::read_to_string(&target).expect("Codex did not create the file"), + "writable" + ); + let _ = std::fs::remove_dir_all(&dir); + }); } /// Auto is the workspace-rooted posture that may also use the network without /// asking the host. `AgencyZero` relies on this for ordinary GitHub reads and /// pushes; an approval request here can strand the whole turn before its card /// reaches the frontend. -#[tokio::test] +#[test] #[ignore = "spawns a real agent, uses the network, and consumes quota"] -async fn codex_auto_uses_github_without_an_approval_request() { - if !available(Agent::Codex) { - return; - } - let dir = std::env::temp_dir().join(format!( - "agent-abstraction-codex-auto-network-{}", - std::process::id() - )); - std::fs::create_dir_all(&dir).expect("temp dir"); - - let request = Request::new( - Agent::Codex, - "Run exactly this read-only command: git ls-remote https://github.com/pathscale/agencyzero.git HEAD. Then reply done.", - ) - .cwd(&dir) - .permission(Permission::Auto) - .interactive() - .approvals() - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("app-server should start"); - let mut asked = false; - while let Some(event) = run.recv().await { - if matches!(event, Event::ApprovalRequest(_)) { - asked = true; +fn codex_auto_uses_github_without_an_approval_request() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; } - } - let outcome = run.finish().await.expect("the GitHub read should complete"); + let dir = std::env::temp_dir().join(format!( + "agent-abstraction-codex-auto-network-{}", + std::process::id() + )); + std::fs::create_dir_all(&dir).expect("temp dir"); + + let request = Request::new( + Agent::Codex, + "Run exactly this read-only command: git ls-remote https://github.com/pathscale/agencyzero.git HEAD. Then reply done.", + ) + .cwd(&dir) + .permission(Permission::Auto) + .interactive() + .approvals() + .timeout(Duration::from_secs(180)); - assert!( - !asked, - "Codex asked the host to approve an Auto network read" - ); - assert!(outcome.is_ok(), "the turn did not complete: {outcome:?}"); - let _ = std::fs::remove_dir_all(&dir); + let mut run = stream(&request, &reactor.handle()).expect("app-server should start"); + let mut asked = false; + while let Some(event) = run.recv().await { + if matches!(event, Event::ApprovalRequest(_)) { + asked = true; + } + } + let outcome = run.finish().await.expect("the GitHub read should complete"); + + assert!( + !asked, + "Codex asked the host to approve an Auto network read" + ); + assert!(outcome.is_ok(), "the turn did not complete: {outcome:?}"); + let _ = std::fs::remove_dir_all(&dir); + }); } /// A Codex sandbox escape is a server request the host can deny mid-turn. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_app_server_routes_approvals() { - if !available(Agent::Codex) { - return; - } - let dir = std::env::temp_dir().join(format!( - "agent-abstraction-codex-approval-{}", - std::process::id() - )); - std::fs::create_dir_all(&dir).expect("temp dir"); - let outside = std::env::temp_dir().join(format!( - "agent-abstraction-codex-outside-{}", - std::process::id() - )); - let _ = std::fs::remove_file(&outside); - - let request = Request::new( - Agent::Codex, - format!( - "Use a shell command to create exactly this file: {}", - outside.display() - ), - ) - .cwd(&dir) - .permission(Permission::Auto) - .approvals() - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("app-server should start"); - let mut asked = false; - while let Some(event) = run.recv().await { - if let Event::ApprovalRequest(approval) = event { - asked = true; - run.respond(&approval.id, &agent_abstraction::Decision::deny()) - .await - .expect("the denial should reach app-server"); +fn codex_app_server_routes_approvals() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; } - } - let outcome = run - .finish() - .await - .expect("a denial should not fail the turn"); - assert!(asked, "Codex did not ask for the out-of-root write"); - assert!(!outside.exists(), "the denied write still created the file"); - assert!(outcome.is_ok(), "the turn did not recover: {outcome:?}"); - let _ = std::fs::remove_dir_all(&dir); + let dir = std::env::temp_dir().join(format!( + "agent-abstraction-codex-approval-{}", + std::process::id() + )); + std::fs::create_dir_all(&dir).expect("temp dir"); + let outside = std::env::temp_dir().join(format!( + "agent-abstraction-codex-outside-{}", + std::process::id() + )); + let _ = std::fs::remove_file(&outside); + + let request = Request::new( + Agent::Codex, + format!( + "Use a shell command to create exactly this file: {}", + outside.display() + ), + ) + .cwd(&dir) + .permission(Permission::Auto) + .approvals() + .timeout(Duration::from_secs(180)); + + let mut run = stream(&request, &reactor.handle()).expect("app-server should start"); + let mut asked = false; + while let Some(event) = run.recv().await { + if let Event::ApprovalRequest(approval) = event { + asked = true; + run.respond(&approval.id, &agent_abstraction::Decision::deny()) + .await + .expect("the denial should reach app-server"); + } + } + let outcome = run + .finish() + .await + .expect("a denial should not fail the turn"); + assert!(asked, "Codex did not ask for the out-of-root write"); + assert!(!outside.exists(), "the denied write still created the file"); + assert!(outcome.is_ok(), "the turn did not recover: {outcome:?}"); + let _ = std::fs::remove_dir_all(&dir); + }); } /// The allow half of the app-server approval round trip. `AgencyZero` can /// remember a scoped decision and answer immediately, so an accepted request /// must resume the same turn rather than leaving it waiting forever. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_app_server_resumes_after_an_allowed_approval() { - if !available(Agent::Codex) { - return; - } - let dir = std::env::temp_dir().join(format!( - "agent-abstraction-codex-allow-{}", - std::process::id() - )); - std::fs::create_dir_all(&dir).expect("temp dir"); - let outside = std::path::PathBuf::from(std::env::var_os("HOME").expect("home dir")) - .join(format!(".codex-allowed-outside-{}", std::process::id())); - let _ = std::fs::remove_file(&outside); - - let request = Request::new( - Agent::Codex, - format!( - "Use a shell command to create exactly this file, then reply done: {}", - outside.display() - ), - ) - .cwd(&dir) - .permission(Permission::Auto) - .approvals() - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("app-server should start"); - let mut asked = false; - while let Some(event) = run.recv().await { - if let Event::ApprovalRequest(approval) = event { - asked = true; - run.respond(&approval.id, &agent_abstraction::Decision::Allow) - .await - .expect("the approval should reach app-server"); +fn codex_app_server_resumes_after_an_allowed_approval() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; } - } - let outcome = run - .finish() - .await - .expect("an approval should resume the turn"); - assert!(asked, "Codex did not ask for the out-of-root write"); - assert!(outside.exists(), "the approved write did not run"); - assert!(outcome.is_ok(), "the turn did not complete: {outcome:?}"); - let _ = std::fs::remove_file(&outside); - let _ = std::fs::remove_dir_all(&dir); + let dir = std::env::temp_dir().join(format!( + "agent-abstraction-codex-allow-{}", + std::process::id() + )); + std::fs::create_dir_all(&dir).expect("temp dir"); + let outside = std::path::PathBuf::from(std::env::var_os("HOME").expect("home dir")) + .join(format!(".codex-allowed-outside-{}", std::process::id())); + let _ = std::fs::remove_file(&outside); + + let request = Request::new( + Agent::Codex, + format!( + "Use a shell command to create exactly this file, then reply done: {}", + outside.display() + ), + ) + .cwd(&dir) + .permission(Permission::Auto) + .approvals() + .timeout(Duration::from_secs(180)); + + let mut run = stream(&request, &reactor.handle()).expect("app-server should start"); + let mut asked = false; + while let Some(event) = run.recv().await { + if let Event::ApprovalRequest(approval) = event { + asked = true; + run.respond(&approval.id, &agent_abstraction::Decision::Allow) + .await + .expect("the approval should reach app-server"); + } + } + let outcome = run + .finish() + .await + .expect("an approval should resume the turn"); + assert!(asked, "Codex did not ask for the out-of-root write"); + assert!(outside.exists(), "the approved write did not run"); + assert!(outcome.is_ok(), "the turn did not complete: {outcome:?}"); + let _ = std::fs::remove_file(&outside); + let _ = std::fs::remove_dir_all(&dir); + }); } /// A live counter has to agree with the number that replaces it when the run /// ends, or a UI would show a total that jumps at the last moment. Drives a /// multi-step task so several model calls report, then checks the accumulated /// figures against the terminal record. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn live_usage_events_agree_with_the_final_outcome() { - if !available(Agent::Claude) { - return; - } - let request = Request::new( - Agent::Claude, - "Using the Bash tool, run these one at a time: `echo one`, `echo two`, \ - `echo three`. Then say done.", - ) - .model("haiku") - .permission(Permission::Bypass) - .timeout(Duration::from_secs(180)); - - let mut run = stream(&request).expect("stream should start"); - let mut live = agent_abstraction::Usage::default(); - let mut snapshots = 0; - while let Some(event) = run.recv().await { - if let Event::Usage(usage) = event { - snapshots += 1; - assert_eq!( - usage.output_tokens, None, - "a mid-turn output count is understated and must be withheld" - ); - live.accumulate(&usage); +fn live_usage_events_agree_with_the_final_outcome() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; } - } - let outcome = run.finish().await.expect("run failed"); + let request = Request::new( + Agent::Claude, + "Using the Bash tool, run these one at a time: `echo one`, `echo two`, \ + `echo three`. Then say done.", + ) + .model("haiku") + .permission(Permission::Bypass) + .timeout(Duration::from_secs(180)); - assert!( - snapshots > 1, - "a multi-step turn should report more than one model call, got {snapshots}" - ); - assert_eq!( - live.input_tokens, outcome.usage.input_tokens, - "accumulated input should match the terminal record" - ); - assert_eq!( - live.context_tokens, outcome.usage.context_tokens, - "the last snapshot's context should be the final context" - ); + let mut run = stream(&request, &reactor.handle()).expect("stream should start"); + let mut live = agent_abstraction::Usage::default(); + let mut snapshots = 0; + while let Some(event) = run.recv().await { + if let Event::Usage(usage) = event { + snapshots += 1; + assert_eq!( + usage.output_tokens, None, + "a mid-turn output count is understated and must be withheld" + ); + live.accumulate(&usage); + } + } + let outcome = run.finish().await.expect("run failed"); + + assert!( + snapshots > 1, + "a multi-step turn should report more than one model call, got {snapshots}" + ); + assert_eq!( + live.input_tokens, outcome.usage.input_tokens, + "accumulated input should match the terminal record" + ); + assert_eq!( + live.context_tokens, outcome.usage.context_tokens, + "the last snapshot's context should be the final context" + ); + }); } /// Sending is refused where it cannot work, rather than silently doing nothing. -#[tokio::test] -async fn a_follow_up_is_refused_where_it_cannot_be_delivered() { - assert!(matches!( - Request::new(Agent::Copilot, PING).interactive().argv(), - Err(agent_abstraction::Error::Unsupported { .. }) - )); - assert!( - Request::new(Agent::Codex, PING) - .interactive() - .argv() - .is_ok() - ); - // A run that never opened the channel has nowhere to put a message. This - // half needs the binary, since it has to actually spawn; the argv checks - // above do not. - if !available(Agent::Claude) { - return; - } - let plain = stream(&Request::new(Agent::Claude, PING)).expect("stream"); - assert!( - matches!( - plain.send("late").await, +#[test] +fn a_follow_up_is_refused_where_it_cannot_be_delivered() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + assert!(matches!( + Request::new(Agent::Copilot, PING).interactive().argv(), Err(agent_abstraction::Error::Unsupported { .. }) - ), - "a non-interactive run should refuse a follow-up" - ); - let _ = plain.cancel().await; + )); + assert!( + Request::new(Agent::Codex, PING) + .interactive() + .argv() + .is_ok() + ); + // A run that never opened the channel has nowhere to put a message. This + // half needs the binary, since it has to actually spawn; the argv checks + // above do not. + if !available(Agent::Claude) { + return; + } + let plain = stream(&Request::new(Agent::Claude, PING), &reactor.handle()).expect("stream"); + assert!( + matches!( + plain.send("late").await, + Err(agent_abstraction::Error::Unsupported { .. }) + ), + "a non-interactive run should refuse a follow-up" + ); + let _ = plain.cancel().await; + }); } /// Both refusals, checked without spawning: Copilot cannot ask, and `run` /// cannot carry the question to anyone. -#[tokio::test] -async fn approvals_are_refused_where_they_cannot_work() { - let unsupported = Request::new(Agent::Copilot, PING) - .permission(Permission::Edit) - .approvals(); - assert!(matches!( - unsupported.argv(), - Err(agent_abstraction::Error::Unsupported { .. }) - )); - assert!( - Request::new(Agent::Codex, PING) +#[test] +fn approvals_are_refused_where_they_cannot_work() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let unsupported = Request::new(Agent::Copilot, PING) .permission(Permission::Edit) - .approvals() - .argv() - .is_ok() - ); - let discarded = Request::new(Agent::Codex, PING) - .permission(Permission::Edit) - .approvals(); - - // Read-only removes the tools that would be asked about, so it is refused - // rather than silently never asking. - assert!( - matches!( - Request::new(Agent::Claude, PING) - .permission(Permission::ReadOnly) - .approvals() - .argv(), - Err(agent_abstraction::Error::Unsupported { .. }) - ), - "approvals under read-only should be refused" - ); - assert!( - matches!( - run(&discarded).await, + .approvals(); + assert!(matches!( + unsupported.argv(), Err(agent_abstraction::Error::Unsupported { .. }) - ), - "`run` discards events, so nobody could answer" - ); + )); + assert!( + Request::new(Agent::Codex, PING) + .permission(Permission::Edit) + .approvals() + .argv() + .is_ok() + ); + let discarded = Request::new(Agent::Codex, PING) + .permission(Permission::Edit) + .approvals(); + + // Read-only removes the tools that would be asked about, so it is refused + // rather than silently never asking. + assert!( + matches!( + Request::new(Agent::Claude, PING) + .permission(Permission::ReadOnly) + .approvals() + .argv(), + Err(agent_abstraction::Error::Unsupported { .. }) + ), + "approvals under read-only should be refused" + ); + assert!( + matches!( + run(&discarded, &reactor.handle()).await, + Err(agent_abstraction::Error::Unsupported { .. }) + ), + "`run` discards events, so nobody could answer" + ); + }); } -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_answers_and_reports_usage() { - if !available(Agent::Codex) { - return; - } - let outcome = run(&ping(Agent::Codex)).await.expect("codex run failed"); +fn codex_answers_and_reports_usage() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let outcome = run(&ping(Agent::Codex), &reactor.handle()) + .await + .expect("codex run failed"); - assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); - assert!( - outcome.text.trim().to_lowercase().contains("pong"), - "got {:?}", - outcome.text - ); - assert!(outcome.session.is_some(), "codex must report a thread id"); - assert!(outcome.usage.input_tokens.is_some(), "{:?}", outcome.usage); + assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); + assert!( + outcome.text.trim().to_lowercase().contains("pong"), + "got {:?}", + outcome.text + ); + assert!(outcome.session.is_some(), "codex must report a thread id"); + assert!(outcome.usage.input_tokens.is_some(), "{:?}", outcome.usage); + }); } -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn copilot_answers_and_reports_a_session() { - if !available(Agent::Copilot) { - return; - } - let outcome = run(&ping(Agent::Copilot)) - .await - .expect("copilot run failed"); +fn copilot_answers_and_reports_a_session() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Copilot) { + return; + } + let outcome = run(&ping(Agent::Copilot), &reactor.handle()) + .await + .expect("copilot run failed"); - assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); - assert!( - outcome.text.trim().to_lowercase().contains("pong"), - "got {:?}", - outcome.text - ); - assert!( - outcome.session.is_some(), - "copilot must report a session id" - ); + assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); + assert!( + outcome.text.trim().to_lowercase().contains("pong"), + "got {:?}", + outcome.text + ); + assert!( + outcome.session.is_some(), + "copilot must report a session id" + ); + }); } /// A read-only `codex exec` can safely inspect a non-repository scratch /// directory. Running the rest of the suite from the repo would never catch a /// regression here, because the repo already satisfies Codex's guard. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_runs_outside_a_git_repository() { - if !available(Agent::Codex) { - return; - } - let scratch = std::env::temp_dir().join(format!("aa-nogit-{}", std::process::id())); - std::fs::create_dir_all(&scratch).unwrap(); - assert!( - !scratch.join(".git").exists(), - "the point of this test is that it is not a repo" - ); +fn codex_runs_outside_a_git_repository() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let scratch = std::env::temp_dir().join(format!("aa-nogit-{}", std::process::id())); + std::fs::create_dir_all(&scratch).unwrap(); + assert!( + !scratch.join(".git").exists(), + "the point of this test is that it is not a repo" + ); - let outcome = run(&ping(Agent::Codex) - .cwd(&scratch) - .permission(Permission::ReadOnly)) - .await - .expect("codex refused to run outside a git repo"); + let outcome = run( + &ping(Agent::Codex) + .cwd(&scratch) + .permission(Permission::ReadOnly), + &reactor.handle(), + ) + .await + .expect("codex refused to run outside a git repo"); - assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); - assert!( - outcome.text.trim().to_lowercase().contains("pong"), - "got {:?}", - outcome.text - ); + assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); + assert!( + outcome.text.trim().to_lowercase().contains("pong"), + "got {:?}", + outcome.text + ); - std::fs::remove_dir_all(&scratch).ok(); + std::fs::remove_dir_all(&scratch).ok(); + }); } /// Claude and Copilot let the caller *assign* the session id up front. That is /// only worth relying on if the agent actually honours the id we hand it, so /// this asserts the round trip rather than trusting `--help`: the id we chose /// must come back unchanged, and must then be resumable. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_caller_assigned_session_id_is_honoured_and_resumable() { - for agent in [Agent::Claude, Agent::Copilot] { - if !available(agent) { - continue; - } - // Both CLIs require a valid UUID. - let chosen = uuid::Uuid::new_v4().to_string(); +fn a_caller_assigned_session_id_is_honoured_and_resumable() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in [Agent::Claude, Agent::Copilot] { + if !available(agent) { + continue; + } + // Both CLIs require a valid UUID. + let chosen = uuid::Uuid::new_v4().to_string(); - let first = run(&ping(agent).session_id(&chosen)) - .await - .unwrap_or_else(|e| panic!("{agent} rejected an assigned id: {e}")); - assert_eq!( - first.session.as_deref(), - Some(chosen.as_str()), - "{agent} did not honour the id it was given" - ); + let first = run(&ping(agent).session_id(&chosen), &reactor.handle()) + .await + .unwrap_or_else(|e| panic!("{agent} rejected an assigned id: {e}")); + assert_eq!( + first.session.as_deref(), + Some(chosen.as_str()), + "{agent} did not honour the id it was given" + ); - // The id is only useful if it also resumes the same conversation. - let second = run(&ping(agent).resume(&chosen)) - .await - .unwrap_or_else(|e| panic!("{agent} could not resume the assigned id: {e}")); - assert_eq!( - second.session.as_deref(), - Some(chosen.as_str()), - "{agent} moved to a different session on resume" - ); - } + // The id is only useful if it also resumes the same conversation. + let second = run(&ping(agent).resume(&chosen), &reactor.handle()) + .await + .unwrap_or_else(|e| panic!("{agent} could not resume the assigned id: {e}")); + assert_eq!( + second.session.as_deref(), + Some(chosen.as_str()), + "{agent} moved to a different session on resume" + ); + } + }); } /// Codex cannot be told an id: `codex exec` has no `--session-id`, so the only /// way to learn its `thread_id` is to read it back. Asking for an assigned one /// must fail loudly rather than silently starting an unrelated conversation. -#[tokio::test] -async fn codex_refuses_an_assigned_session_id() { +#[test] +fn codex_refuses_an_assigned_session_id() { let err = Request::new(Agent::Codex, "hi") .session_id("11111111-2222-3333-4444-555555555555") .argv() @@ -1053,64 +1220,81 @@ async fn codex_refuses_an_assigned_session_id() { /// record of the stream and arrives *before* the model replies. A host can /// therefore persist the binding as soon as the stream opens rather than /// waiting for the turn to finish. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_reveals_its_thread_id_before_it_answers() { - if !available(Agent::Codex) { - return; - } - let mut running = stream(&ping(Agent::Codex).format(Format::Stream)).expect("spawn failed"); - - let mut first_event = None; - let mut text_seen_before_start = false; - while let Some(event) = running.recv().await { - match (&first_event, &event) { - (None, Event::Started { session, .. }) => { - assert!(!session.is_empty()); - first_event = Some(session.clone()); +fn codex_reveals_its_thread_id_before_it_answers() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let mut running = stream( + &ping(Agent::Codex).format(Format::Stream), + &reactor.handle(), + ) + .expect("spawn failed"); + + let mut first_event = None; + let mut text_seen_before_start = false; + while let Some(event) = running.recv().await { + match (&first_event, &event) { + (None, Event::Started { session, .. }) => { + assert!(!session.is_empty()); + first_event = Some(session.clone()); + } + (None, Event::Text(_)) => text_seen_before_start = true, + _ => {} } - (None, Event::Text(_)) => text_seen_before_start = true, - _ => {} } - } - let outcome = running.finish().await.expect("run failed"); + let outcome = running.finish().await.expect("run failed"); - assert!( - first_event.is_some(), - "codex never announced a thread id on the stream" - ); - assert!( - !text_seen_before_start, - "the id must arrive before any answer text, so a binding can be stored early" - ); - assert_eq!(outcome.session, first_event); + assert!( + first_event.is_some(), + "codex never announced a thread id on the stream" + ); + assert!( + !text_seen_before_start, + "the id must arrive before any answer text, so a binding can be stored early" + ); + assert_eq!(outcome.session, first_event); + }); } /// `EnvPolicy::Minimal` only earns its place if a run under it still works. /// An isolation setting that silently breaks authentication is worse than none, /// because the failure surfaces as "not logged in" rather than as a config /// mistake. This is the test that keeps the per-agent list honest. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn every_agent_still_works_under_a_minimal_environment() { - for agent in Agent::ALL { - if !available(agent) { - continue; - } - let outcome = run(&ping(agent).env_policy(EnvPolicy::Minimal)) +fn every_agent_still_works_under_a_minimal_environment() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in Agent::ALL { + if !available(agent) { + continue; + } + let outcome = run( + &ping(agent).env_policy(EnvPolicy::Minimal), + &reactor.handle(), + ) .await .unwrap_or_else(|e| { panic!( "{agent} could not authenticate under EnvPolicy::Minimal, \ - so its essential_env list is incomplete: {e}" + so its essential_env list is incomplete: {e}" ) }); - assert!( - outcome.text.trim().to_lowercase().contains("pong"), - "{agent} answered {:?}", - outcome.text - ); - } + assert!( + outcome.text.trim().to_lowercase().contains("pong"), + "{agent} answered {:?}", + outcome.text + ); + } + }); } /// Codex cannot be *told* a session id, so continuity depends entirely on @@ -1120,274 +1304,325 @@ async fn every_agent_still_works_under_a_minimal_environment() { /// `$CODEX_HOME/sessions/`: turn one states a fact, the id is captured and /// persisted, and a second run in a separate process resumes from the store and /// recalls it. If Codex ever stopped emitting `thread.started`, this fails. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn codex_resumes_from_a_captured_thread_id_without_scraping() { - if !available(Agent::Codex) { - return; - } - let dir = std::env::temp_dir().join(format!("aa-codex-resume-{}", std::process::id())); - let store = SessionStore::open(&dir); - let project = std::env::current_dir().unwrap(); - let name = "codex-memory"; +fn codex_resumes_from_a_captured_thread_id_without_scraping() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let dir = std::env::temp_dir().join(format!("aa-codex-resume-{}", std::process::id())); + let store = SessionStore::open(&dir); + let project = std::env::current_dir().unwrap(); + let name = "codex-memory"; - let first = Request::new(Agent::Codex, "Remember the number 5619. Reply OK.") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)) - .session(&store, &project, name, false) - .expect("planning the first turn failed"); - let first = run(&first).await.expect("first turn failed"); - let thread = first - .session - .clone() - .expect("codex must report a thread id on the stream"); - - // The binding must be on disk, since that is the only place the id exists - // for us: nothing reads Codex's own session directory. - let stored = store - .get(&project, name) - .expect("store read failed") - .expect("no binding was persisted"); - assert_eq!(stored.token, thread); - assert_eq!(stored.agent, Agent::Codex); - - let second = Request::new(Agent::Codex, "What number did I ask you to remember?") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)) - .session(&store, &project, name, false) - .expect("planning the second turn failed"); - assert_eq!( - second.session_phase(), - Some(agent_abstraction::Phase::Continue), - "the second turn must continue the stored thread" - ); - let second = run(&second).await.expect("second turn failed"); + let first = Request::new(Agent::Codex, "Remember the number 5619. Reply OK.") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + .session(&store, &project, name, false) + .expect("planning the first turn failed"); + let first = run(&first, &reactor.handle()) + .await + .expect("first turn failed"); + let thread = first + .session + .clone() + .expect("codex must report a thread id on the stream"); + + // The binding must be on disk, since that is the only place the id exists + // for us: nothing reads Codex's own session directory. + let stored = store + .get(&project, name) + .expect("store read failed") + .expect("no binding was persisted"); + assert_eq!(stored.token, thread); + assert_eq!(stored.agent, Agent::Codex); + + let second = Request::new(Agent::Codex, "What number did I ask you to remember?") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + .session(&store, &project, name, false) + .expect("planning the second turn failed"); + assert_eq!( + second.session_phase(), + Some(agent_abstraction::Phase::Continue), + "the second turn must continue the stored thread" + ); + let second = run(&second, &reactor.handle()) + .await + .expect("second turn failed"); - assert!( - second.text.contains("5619"), - "codex lost its context on resume: {:?}", - second.text - ); + assert!( + second.text.contains("5619"), + "codex lost its context on resume: {:?}", + second.text + ); - std::fs::remove_dir_all(&dir).ok(); + std::fs::remove_dir_all(&dir).ok(); + }); } /// The streaming path must deliver events *before* the run settles, and the /// terminal answer must still be authoritative afterwards. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn streaming_delivers_events_then_an_outcome() { - if !available(Agent::Claude) { - return; - } - let request = ping(Agent::Claude).format(Format::Stream); - let mut running = stream(&request).expect("spawn failed"); - - let mut started = false; - let mut text = String::new(); - while let Some(event) = running.recv().await { - match event { - Event::Started { session, .. } => { - assert!(!session.is_empty()); - started = true; +fn streaming_delivers_events_then_an_outcome() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let request = ping(Agent::Claude).format(Format::Stream); + let mut running = stream(&request, &reactor.handle()).expect("spawn failed"); + + let mut started = false; + let mut text = String::new(); + while let Some(event) = running.recv().await { + match event { + Event::Started { session, .. } => { + assert!(!session.is_empty()); + started = true; + } + Event::Text(chunk) => text.push_str(&chunk), + _ => {} } - Event::Text(chunk) => text.push_str(&chunk), - _ => {} } - } - let outcome = running.finish().await.expect("run failed"); + let outcome = running.finish().await.expect("run failed"); - assert!(started, "the stream must announce the session"); - assert!(text.to_lowercase().contains("pong"), "streamed {text:?}"); - assert_eq!(outcome.text.trim().to_lowercase(), "pong"); + assert!(started, "the stream must announce the session"); + assert!(text.to_lowercase().contains("pong"), "streamed {text:?}"); + assert_eq!(outcome.text.trim().to_lowercase(), "pong"); + }); } /// The point of the whole session layer: a second turn on the same name must /// see what the first turn was told, without the caller handling any id. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_named_session_carries_context_across_turns() { - if !available(Agent::Claude) { - return; - } - let dir = std::env::temp_dir().join(format!("aa-live-{}", std::process::id())); - let store = SessionStore::open(&dir); - let project = std::env::current_dir().unwrap(); - let name = "live-memory"; +fn a_named_session_carries_context_across_turns() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let dir = std::env::temp_dir().join(format!("aa-live-{}", std::process::id())); + let store = SessionStore::open(&dir); + let project = std::env::current_dir().unwrap(); + let name = "live-memory"; - let first = Request::new(Agent::Claude, "Remember the number 4271. Reply OK.") - .model("haiku") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)) - .session(&store, &project, name, false) - .expect("planning the first turn failed"); - assert_eq!( - first.session_phase(), - Some(agent_abstraction::Phase::Create) - ); - let first = run(&first).await.expect("first turn failed"); - let session = first.session.clone().expect("no session id captured"); + let first = Request::new(Agent::Claude, "Remember the number 4271. Reply OK.") + .model("haiku") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + .session(&store, &project, name, false) + .expect("planning the first turn failed"); + assert_eq!( + first.session_phase(), + Some(agent_abstraction::Phase::Create) + ); + let first = run(&first, &reactor.handle()) + .await + .expect("first turn failed"); + let session = first.session.clone().expect("no session id captured"); - let second = Request::new(Agent::Claude, "What number did I ask you to remember?") - .model("haiku") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)) - .session(&store, &project, name, false) - .expect("planning the second turn failed"); - assert_eq!( - second.session_phase(), - Some(agent_abstraction::Phase::Continue), - "the second turn must continue, not create" - ); - let second = run(&second).await.expect("second turn failed"); + let second = Request::new(Agent::Claude, "What number did I ask you to remember?") + .model("haiku") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + .session(&store, &project, name, false) + .expect("planning the second turn failed"); + assert_eq!( + second.session_phase(), + Some(agent_abstraction::Phase::Continue), + "the second turn must continue, not create" + ); + let second = run(&second, &reactor.handle()) + .await + .expect("second turn failed"); - assert!( - second.text.contains("4271"), - "the resumed turn lost its context: {:?}", - second.text - ); - assert_eq!( - second.session.as_deref(), - Some(session.as_str()), - "a linear resume must stay on the same session" - ); + assert!( + second.text.contains("4271"), + "the resumed turn lost its context: {:?}", + second.text + ); + assert_eq!( + second.session.as_deref(), + Some(session.as_str()), + "a linear resume must stay on the same session" + ); - std::fs::remove_dir_all(&dir).ok(); + std::fs::remove_dir_all(&dir).ok(); + }); } /// Forking must branch: the new turn sees the parent's context but lands on a /// different session id, leaving the original resumable. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn forking_branches_to_a_new_session() { - if !available(Agent::Claude) { - return; - } - let dir = std::env::temp_dir().join(format!("aa-fork-{}", std::process::id())); - let store = SessionStore::open(&dir); - let project = std::env::current_dir().unwrap(); - let name = "live-fork"; +fn forking_branches_to_a_new_session() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; + } + let dir = std::env::temp_dir().join(format!("aa-fork-{}", std::process::id())); + let store = SessionStore::open(&dir); + let project = std::env::current_dir().unwrap(); + let name = "live-fork"; - let first = Request::new(Agent::Claude, "Remember the number 8813. Reply OK.") - .model("haiku") - .timeout(Duration::from_secs(180)) - .session(&store, &project, name, false) - .unwrap(); - let parent = run(&first).await.expect("first turn failed"); - let parent_id = parent.session.expect("no session id"); + let first = Request::new(Agent::Claude, "Remember the number 8813. Reply OK.") + .model("haiku") + .timeout(Duration::from_secs(180)) + .session(&store, &project, name, false) + .unwrap(); + let parent = run(&first, &reactor.handle()) + .await + .expect("first turn failed"); + let parent_id = parent.session.expect("no session id"); - let forked = Request::new(Agent::Claude, "What number did I ask you to remember?") - .model("haiku") - .timeout(Duration::from_secs(180)) - .session(&store, &project, name, true) - .unwrap(); - assert_eq!(forked.session_phase(), Some(agent_abstraction::Phase::Fork)); - let forked = run(&forked).await.expect("forked turn failed"); + let forked = Request::new(Agent::Claude, "What number did I ask you to remember?") + .model("haiku") + .timeout(Duration::from_secs(180)) + .session(&store, &project, name, true) + .unwrap(); + assert_eq!(forked.session_phase(), Some(agent_abstraction::Phase::Fork)); + let forked = run(&forked, &reactor.handle()) + .await + .expect("forked turn failed"); - assert!( - forked.text.contains("8813"), - "the fork lost the parent's context: {:?}", - forked.text - ); - assert_ne!( - forked.session.as_deref(), - Some(parent_id.as_str()), - "a fork must land on a new session id, not append to the parent" - ); + assert!( + forked.text.contains("8813"), + "the fork lost the parent's context: {:?}", + forked.text + ); + assert_ne!( + forked.session.as_deref(), + Some(parent_id.as_str()), + "a fork must land on a new session id, not append to the parent" + ); - std::fs::remove_dir_all(&dir).ok(); + std::fs::remove_dir_all(&dir).ok(); + }); } /// A missing binary must be an actionable error, not a spawn failure. -#[tokio::test] -async fn a_missing_agent_reports_how_to_install_it() { - let request = Request::new(Agent::Codex, "hi").bin("agent-abstraction-no-such-binary"); - let err = run(&request).await.unwrap_err(); - // Assert on the structured field rather than the rendered message: the - // wording is free to change, the contract that an install hint is carried - // at all is not. - let agent_abstraction::Error::NotInstalled { agent, hint, .. } = &err else { - panic!("expected NotInstalled, got {err:?}") - }; - assert_eq!(*agent, Agent::Codex); - assert!( - !hint.is_empty(), - "a missing agent must say how to install it" - ); +#[test] +fn a_missing_agent_reports_how_to_install_it() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let request = Request::new(Agent::Codex, "hi").bin("agent-abstraction-no-such-binary"); + let err = run(&request, &reactor.handle()).await.unwrap_err(); + // Assert on the structured field rather than the rendered message: the + // wording is free to change, the contract that an install hint is carried + // at all is not. + let agent_abstraction::Error::NotInstalled { agent, hint, .. } = &err else { + panic!("expected NotInstalled, got {err:?}") + }; + assert_eq!(*agent, Agent::Codex); + assert!( + !hint.is_empty(), + "a missing agent must say how to install it" + ); + }); } /// Probing costs no quota, just `--version`, so unlike the rest of this file it /// runs by default. It is also the test that catches flag drift *before* a run /// fails on it: if an agent updates underneath us, this goes red and names the /// version rather than leaving someone to decode an unexpected-argument error. -#[tokio::test] -async fn installed_agents_match_the_versions_the_flags_were_verified_against() { - let mut checked = 0; - for agent in Agent::ALL { - if !available(agent) { - continue; - } - let probe = Probe::run(agent).await.expect("probe failed"); - checked += 1; +#[test] +fn installed_agents_match_the_versions_the_flags_were_verified_against() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let mut checked = 0; + for agent in Agent::ALL { + if !available(agent) { + continue; + } + let probe = Probe::run(agent, &reactor.handle()) + .await + .expect("probe failed"); + checked += 1; - assert!( - probe.version.is_some(), - "{agent} reported {:?}, which carries no readable version", - probe.reported - ); - assert_eq!( - probe.status, - VersionStatus::Verified, - "{}", - probe.advisory().unwrap_or_default() - ); - } - eprintln!("probed {checked} installed agents"); + assert!( + probe.version.is_some(), + "{agent} reported {:?}, which carries no readable version", + probe.reported + ); + assert_eq!( + probe.status, + VersionStatus::Verified, + "{}", + probe.advisory().unwrap_or_default() + ); + } + eprintln!("probed {checked} installed agents"); + }); } /// Checking login costs no quota, so like the version probe this runs by /// default. It is the test that keeps the status parsing honest: both CLIs /// answer in their own shape, and a parser that silently stopped recognizing /// `Logged in using ChatGPT` would report a working setup as unknown. -#[tokio::test] -async fn installed_agents_report_their_login_state() { - for agent in Agent::ALL { - if !available(agent) { - continue; - } - let status = AuthStatus::check(agent).await.expect("check failed"); - - if agent.auth_status_argv().is_some() { - // Claude and Codex answer, so the result must be a real yes or no. - // Unknown here means the parsing no longer matches the CLI. - assert_ne!( - status.state, - AuthState::Unknown, - "{agent} answered {:?}, which this crate no longer recognizes", - status.detail - ); - assert!( - status.is_logged_in(), - "{agent} is not logged in: {}", - status.summary() - ); - } else { - // Copilot cannot be asked, and must say so rather than claiming a - // logout that would send someone to fix a working setup. - assert_eq!(status.state, AuthState::Unknown); - assert!(!status.needs_login()); +#[test] +fn installed_agents_report_their_login_state() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in Agent::ALL { + if !available(agent) { + continue; + } + let status = AuthStatus::check(agent, &reactor.handle()) + .await + .expect("check failed"); + + if agent.auth_status_argv().is_some() { + // Claude and Codex answer, so the result must be a real yes or no. + // Unknown here means the parsing no longer matches the CLI. + assert_ne!( + status.state, + AuthState::Unknown, + "{agent} answered {:?}, which this crate no longer recognizes", + status.detail + ); + assert!( + status.is_logged_in(), + "{agent} is not logged in: {}", + status.summary() + ); + } else { + // Copilot cannot be asked, and must say so rather than claiming a + // logout that would send someone to fix a working setup. + assert_eq!(status.state, AuthState::Unknown); + assert!(!status.needs_login()); + } + eprintln!("{agent}: {}", status.summary()); } - eprintln!("{agent}: {}", status.summary()); - } + }); } /// Structured output is the capability a consumer needs to get findings back as /// data rather than prose to re-parse. The two CLIs deliver it differently, /// Claude inline and Codex through a file this crate writes, so this proves the /// unified interface against both rather than against the flag mapping alone. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_schema_constrains_the_answer_to_data() { +fn a_schema_constrains_the_answer_to_data() { const SCHEMA: &str = r#"{ "type": "object", "properties": {"name": {"type": "string"}, "age": {"type": "integer"}}, @@ -1395,35 +1630,40 @@ async fn a_schema_constrains_the_answer_to_data() { "additionalProperties": false }"#; - for agent in [Agent::Claude, Agent::Codex] { - if !available(agent) { - continue; - } - let request = Request::new(agent, "Alice is 30 years old.") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)) - .schema(SCHEMA); - let request = match agent { - Agent::Claude => request.model("haiku"), - _ => request, - }; - - let outcome = run(&request) - .await - .unwrap_or_else(|e| panic!("{agent} schema run failed: {e}")); + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in [Agent::Claude, Agent::Codex] { + if !available(agent) { + continue; + } + let request = Request::new(agent, "Alice is 30 years old.") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + .schema(SCHEMA); + let request = match agent { + Agent::Claude => request.model("haiku"), + _ => request, + }; + + let outcome = run(&request, &reactor.handle()) + .await + .unwrap_or_else(|e| panic!("{agent} schema run failed: {e}")); - let value = outcome - .structured - .unwrap_or_else(|| panic!("{agent} returned no structured answer: {:?}", outcome.text)); - assert_eq!(value["name"], "Alice", "{agent}: {value}"); - assert_eq!(value["age"], 30, "{agent}: {value}"); - } + let value = outcome.structured.unwrap_or_else(|| { + panic!("{agent} returned no structured answer: {:?}", outcome.text) + }); + assert_eq!(value["name"], "Alice", "{agent}: {value}"); + assert_eq!(value["age"], 30, "{agent}: {value}"); + } + }); } /// Copilot has no schema support, so asking must fail before spawning rather /// than returning prose that a caller would try to parse as data. -#[tokio::test] -async fn copilot_refuses_a_schema_before_spawning() { +#[test] +fn copilot_refuses_a_schema_before_spawning() { let err = Request::new(Agent::Copilot, "hi") .schema(r#"{"type":"object"}"#) .argv() @@ -1435,31 +1675,39 @@ async fn copilot_refuses_a_schema_before_spawning() { } /// The schema file Codex reads must not outlive the run that needed it. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn a_codex_schema_file_is_cleaned_up() { - if !available(Agent::Codex) { - return; - } - let before = schema_files_in_temp(); - let outcome = run(&Request::new(Agent::Codex, "Alice is 30 years old.") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)) - // `additionalProperties: false` is mandatory for Codex: without it the - // provider rejects the schema with a 400 before the model runs. - .schema( - r#"{"type":"object","properties":{"name":{"type":"string"}}, +fn a_codex_schema_file_is_cleaned_up() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Codex) { + return; + } + let before = schema_files_in_temp(); + let outcome = run( + &Request::new(Agent::Codex, "Alice is 30 years old.") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)) + // `additionalProperties: false` is mandatory for Codex: without it the + // provider rejects the schema with a 400 before the model runs. + .schema( + r#"{"type":"object","properties":{"name":{"type":"string"}}, "required":["name"],"additionalProperties":false}"#, - )) - .await - .expect("run failed"); - - assert!(outcome.structured.is_some()); - assert_eq!( - schema_files_in_temp(), - before, - "a schema file was left behind in the temp directory" - ); + ), + &reactor.handle(), + ) + .await + .expect("run failed"); + + assert!(outcome.structured.is_some()); + assert_eq!( + schema_files_in_temp(), + before, + "a schema file was left behind in the temp directory" + ); + }); } /// Count this crate's schema files currently in the temp directory. @@ -1480,52 +1728,57 @@ fn schema_files_in_temp() -> usize { /// the agent is still working, not in one lump at the end. A run under the old /// default reported nothing until it finished, which for a long turn is /// indistinguishable from a hang. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn text_arrives_in_pieces_before_the_run_finishes() { - if !available(Agent::Claude) { - return; - } - // Long enough to produce several chunks rather than one short answer. - let request = Request::new( - Agent::Claude, - "Count from 1 to 10, one number per line, with a short comment on each.", - ) - .model("haiku") - .permission(Permission::ReadOnly) - .timeout(Duration::from_secs(180)); - - // No .format(): this asserts the *default* streams, which is the fix. - let mut running = stream(&request).expect("spawn failed"); - - let mut chunks: Vec = Vec::new(); - while let Some(event) = running.recv().await { - if let Event::Text(text) = event { - chunks.push(text); +fn text_arrives_in_pieces_before_the_run_finishes() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; } - } - let outcome = running.finish().await.expect("run failed"); + // Long enough to produce several chunks rather than one short answer. + let request = Request::new( + Agent::Claude, + "Count from 1 to 10, one number per line, with a short comment on each.", + ) + .model("haiku") + .permission(Permission::ReadOnly) + .timeout(Duration::from_secs(180)); - assert!( - chunks.len() > 1, - "text arrived as {} chunk(s), so nothing was actually streamed: {chunks:?}", - chunks.len() - ); - // Every chunk must be a real piece, not the whole answer repeated. - let joined: String = chunks.concat(); - assert!( - joined.len() <= outcome.text.len() + 64, - "chunks total {} bytes against a {} byte answer, which means the \ - finished message was emitted on top of the deltas", - joined.len(), - outcome.text.len() - ); - assert!(outcome.text.contains('1'), "answer: {:?}", outcome.text); - eprintln!( - "streamed {} chunks for a {} byte answer", - chunks.len(), - outcome.text.len() - ); + // No .format(): this asserts the *default* streams, which is the fix. + let mut running = stream(&request, &reactor.handle()).expect("spawn failed"); + + let mut chunks: Vec = Vec::new(); + while let Some(event) = running.recv().await { + if let Event::Text(text) = event { + chunks.push(text); + } + } + let outcome = running.finish().await.expect("run failed"); + + assert!( + chunks.len() > 1, + "text arrived as {} chunk(s), so nothing was actually streamed: {chunks:?}", + chunks.len() + ); + // Every chunk must be a real piece, not the whole answer repeated. + let joined: String = chunks.concat(); + assert!( + joined.len() <= outcome.text.len() + 64, + "chunks total {} bytes against a {} byte answer, which means the \ + finished message was emitted on top of the deltas", + joined.len(), + outcome.text.len() + ); + assert!(outcome.text.contains('1'), "answer: {:?}", outcome.text); + eprintln!( + "streamed {} chunks for a {} byte answer", + chunks.len(), + outcome.text.len() + ); + }); } /// `/compact` is a command the CLI runs, not text the model reads. @@ -1539,101 +1792,115 @@ async fn text_arrives_in_pieces_before_the_run_finishes() { /// Two turns first, because a conversation shorter than that is refused. That /// refusal is itself the other half of the contract: it arrives as a completed /// run carrying the reason, never as an error. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn claude_runs_compact_as_a_command_and_reports_its_phases() { - if !available(Agent::Claude) { - return; - } - let dir = std::env::temp_dir().join("agent-abstraction-compact-live"); - std::fs::create_dir_all(&dir).expect("scratch dir"); - - // A session with enough behind it to be worth summarising. - let mut session = None; - for prompt in [ - "Remember the number 41. Reply with just: ok", - "Remember the colour teal. Reply with just: ok", - ] { - let mut request = Request::new(Agent::Claude, prompt) - .model("haiku") - .cwd(&dir) - .timeout(Duration::from_secs(180)); - if let Some(id) = &session { - request = request.resume(id); +fn claude_runs_compact_as_a_command_and_reports_its_phases() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + if !available(Agent::Claude) { + return; } - let outcome = run(&request).await.expect("seeding turn failed"); - session = outcome.session.clone(); - } - let session = session.expect("claude must report a session id"); - - let request = Request::command( - Agent::Claude, - &agent_abstraction::Command::Compact { instructions: None }, - ) - .model("haiku") - .cwd(&dir) - .resume(&session) - .timeout(Duration::from_secs(300)); - - let mut stream = stream(&request).expect("compact run failed to start"); - let mut phases = Vec::new(); - let mut catalogue = None; - while let Some(event) = stream.recv().await { - match event { - Event::Compaction(phase) => phases.push(phase), - Event::Commands(commands) => catalogue = Some(commands), - _ => {} + let dir = std::env::temp_dir().join("agent-abstraction-compact-live"); + std::fs::create_dir_all(&dir).expect("scratch dir"); + + // A session with enough behind it to be worth summarising. + let mut session = None; + for prompt in [ + "Remember the number 41. Reply with just: ok", + "Remember the colour teal. Reply with just: ok", + ] { + let mut request = Request::new(Agent::Claude, prompt) + .model("haiku") + .cwd(&dir) + .timeout(Duration::from_secs(180)); + if let Some(id) = &session { + request = request.resume(id); + } + let outcome = run(&request, &reactor.handle()) + .await + .expect("seeding turn failed"); + session = outcome.session.clone(); } - } - let outcome = stream.finish().await.expect("compact run failed"); + let session = session.expect("claude must report a session id"); - assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); - assert!( - phases + let request = Request::command( + Agent::Claude, + &agent_abstraction::Command::Compact { instructions: None }, + ) + .model("haiku") + .cwd(&dir) + .resume(&session) + .timeout(Duration::from_secs(300)); + + let mut stream = stream(&request, &reactor.handle()).expect("compact run failed to start"); + let mut phases = Vec::new(); + let mut catalogue = None; + while let Some(event) = stream.recv().await { + match event { + Event::Compaction(phase) => phases.push(phase), + Event::Commands(commands) => catalogue = Some(commands), + _ => {} + } + } + let outcome = stream.finish().await.expect("compact run failed"); + + assert!(outcome.is_ok(), "unexpected stop: {outcome:?}"); + assert!( + phases + .iter() + .any(|phase| matches!(phase, agent_abstraction::Compaction::Started)), + "no compaction began, so the slash was read as prose: {phases:?}" + ); + let finished = phases .iter() - .any(|phase| matches!(phase, agent_abstraction::Compaction::Started)), - "no compaction began, so the slash was read as prose: {phases:?}" - ); - let finished = phases - .iter() - .find_map(|phase| match phase { - agent_abstraction::Compaction::Finished { ok, error } => Some((*ok, error.clone())), - _ => None, - }) - .expect("a compaction that starts must settle"); - assert!(finished.0, "compaction refused: {:?}", finished.1); - - // The catalogue rides the same run, and is the agent's own rather than ours. - let catalogue = catalogue.expect("claude publishes its commands at init"); - assert!( - catalogue.has("compact"), - "an agent that just compacted must list the command: {catalogue:?}" - ); - assert!( - !catalogue.utilities().is_empty(), - "utilities are the commands that are not skills: {catalogue:?}" - ); - eprintln!( - "compacted; {} commands, {} of them skills", - catalogue.all.len(), - catalogue.skills.len() - ); + .find_map(|phase| match phase { + agent_abstraction::Compaction::Finished { ok, error } => Some((*ok, error.clone())), + _ => None, + }) + .expect("a compaction that starts must settle"); + assert!(finished.0, "compaction refused: {:?}", finished.1); + + // The catalogue rides the same run, and is the agent's own rather than ours. + let catalogue = catalogue.expect("claude publishes its commands at init"); + assert!( + catalogue.has("compact"), + "an agent that just compacted must list the command: {catalogue:?}" + ); + assert!( + !catalogue.utilities().is_empty(), + "utilities are the commands that are not skills: {catalogue:?}" + ); + eprintln!( + "compacted; {} commands, {} of them skills", + catalogue.all.len(), + catalogue.skills.len() + ); + }); } /// The other agents have no command vocabulary, and the refusal is raised /// before anything spawns, so this costs nothing. -#[tokio::test] +#[test] #[ignore = "spawns a real agent and consumes quota"] -async fn only_claude_takes_a_slash_command() { - for agent in [Agent::Codex, Agent::Copilot] { - let request = Request::command( - agent, - &agent_abstraction::Command::Compact { instructions: None }, - ); - let error = run(&request).await.expect_err("must refuse"); - assert!( - matches!(error, agent_abstraction::Error::Unsupported { .. }), - "{agent} refused with the wrong error: {error:?}" - ); - } +fn only_claude_takes_a_slash_command() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + for agent in [Agent::Codex, Agent::Copilot] { + let request = Request::command( + agent, + &agent_abstraction::Command::Compact { instructions: None }, + ); + let error = run(&request, &reactor.handle()) + .await + .expect_err("must refuse"); + assert!( + matches!(error, agent_abstraction::Error::Unsupported { .. }), + "{agent} refused with the wrong error: {error:?}" + ); + } + }); } diff --git a/tests/process.rs b/tests/process.rs index adb1278..69df734 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -96,7 +96,7 @@ async fn grandchild_pid(dir: &Path) -> i32 { return pid; } } - tokio::time::sleep(Duration::from_millis(50)).await; + nagoya::sleep(Duration::from_millis(50)).await; } panic!("the fake agent never spawned its grandchild"); } @@ -110,12 +110,14 @@ async fn grandchild_pid(dir: &Path) -> i32 { /// the contract actually promises is "killed promptly", so that is what this /// waits for. async fn wait_until_dead(pid: i32, limit: Duration) -> bool { - let deadline = tokio::time::Instant::now() + limit; - while tokio::time::Instant::now() < deadline { + // A plain monotonic clock: tokio's `Instant` differed only in being + // pausable by its test harness, which none of these tests use. + let deadline = std::time::Instant::now() + limit; + while std::time::Instant::now() < deadline { if !alive(pid) { return true; } - tokio::time::sleep(Duration::from_millis(25)).await; + nagoya::sleep(Duration::from_millis(25)).await; } !alive(pid) } @@ -127,140 +129,177 @@ const TEARDOWN_GRACE: Duration = Duration::from_secs(10); /// The session lease covers the whole run, not just the atomic record write. /// A second host must fail before spawning and leave the sole binding intact; /// once the holder settles, the same name is immediately usable again. -#[tokio::test] -async fn one_named_session_admits_exactly_one_run() { - let dir = scratch("session-lease"); - let script = sleeping_agent(&dir); - let store = SessionStore::open(dir.join("sessions")); - let project = dir.join("project"); - let request = || { - Request::new(Agent::Claude, "hi") - .bin(script.to_str().unwrap()) - .session(&store, &project, "thread-42", false) - .unwrap() - }; - - let first = stream(&request()).expect("first run should claim the session"); - let binding = store.get(&project, "thread-42").unwrap().unwrap(); - - let conflict = stream(&request()).expect_err("second run must not share the session"); - assert!( - matches!(conflict, Error::SessionBusy { ref name, .. } if name == "thread-42"), - "got {conflict:?}" - ); - assert!(conflict.is_transient(), "the holder can settle and free it"); - assert_eq!( - store.get(&project, "thread-42").unwrap().unwrap(), - binding, - "a rejected concurrent run must not rewrite the binding" - ); - - let cancelled = first.cancel().await.unwrap_err(); - assert!(cancelled.is_cancelled()); - - let next = stream(&request()).expect("the settled holder must release the session"); - let cancelled = next.cancel().await.unwrap_err(); - assert!(cancelled.is_cancelled()); - std::fs::remove_dir_all(&dir).ok(); +#[test] +fn one_named_session_admits_exactly_one_run() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("session-lease"); + let script = sleeping_agent(&dir); + let store = SessionStore::open(dir.join("sessions")); + let project = dir.join("project"); + let request = || { + Request::new(Agent::Claude, "hi") + .bin(script.to_str().unwrap()) + .session(&store, &project, "thread-42", false) + .unwrap() + }; + + let first = + stream(&request(), &reactor.handle()).expect("first run should claim the session"); + let binding = store.get(&project, "thread-42").unwrap().unwrap(); + + let conflict = stream(&request(), &reactor.handle()) + .expect_err("second run must not share the session"); + assert!( + matches!(conflict, Error::SessionBusy { ref name, .. } if name == "thread-42"), + "got {conflict:?}" + ); + assert!(conflict.is_transient(), "the holder can settle and free it"); + assert_eq!( + store.get(&project, "thread-42").unwrap().unwrap(), + binding, + "a rejected concurrent run must not rewrite the binding" + ); + + let cancelled = first.cancel().await.unwrap_err(); + assert!(cancelled.is_cancelled()); + + let next = stream(&request(), &reactor.handle()) + .expect("the settled holder must release the session"); + let cancelled = next.cancel().await.unwrap_err(); + assert!(cancelled.is_cancelled()); + std::fs::remove_dir_all(&dir).ok(); + }); } /// The default that matters for a GUI: closing a window must stop the agent, /// not leave it running invisibly and spending quota. -#[tokio::test] -async fn dropping_a_run_kills_the_agent_and_its_children() { - let dir = scratch("drop"); - let script = fake_agent(&dir); - - let running = stream(&Request::new(Agent::Claude, "hi").bin(script.to_str().unwrap())) +#[test] +fn dropping_a_run_kills_the_agent_and_its_children() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("drop"); + let script = fake_agent(&dir); + + let running = stream( + &Request::new(Agent::Claude, "hi").bin(script.to_str().unwrap()), + &reactor.handle(), + ) .expect("spawn failed"); - let grandchild = grandchild_pid(&dir).await; - assert!(alive(grandchild), "the grandchild should be running"); - - drop(running); - - assert!( - wait_until_dead(grandchild, TEARDOWN_GRACE).await, - "dropping the run left a grandchild ({grandchild}) alive in state {:?}; \ - killing only the CLI orphans whatever it spawned", - process_state(grandchild) - ); - std::fs::remove_dir_all(&dir).ok(); + let grandchild = grandchild_pid(&dir).await; + assert!(alive(grandchild), "the grandchild should be running"); + + drop(running); + + assert!( + wait_until_dead(grandchild, TEARDOWN_GRACE).await, + "dropping the run left a grandchild ({grandchild}) alive in state {:?}; \ + killing only the CLI orphans whatever it spawned", + process_state(grandchild) + ); + std::fs::remove_dir_all(&dir).ok(); + }); } /// `cancel` is the deterministic form: when it returns, the tree is gone. -#[tokio::test] -async fn cancel_stops_the_whole_tree_before_returning() { - let dir = scratch("cancel"); - let script = fake_agent(&dir); - - let running = stream(&Request::new(Agent::Claude, "hi").bin(script.to_str().unwrap())) +#[test] +fn cancel_stops_the_whole_tree_before_returning() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("cancel"); + let script = fake_agent(&dir); + + let running = stream( + &Request::new(Agent::Claude, "hi").bin(script.to_str().unwrap()), + &reactor.handle(), + ) .expect("spawn failed"); - let grandchild = grandchild_pid(&dir).await; - - let err = running.cancel().await.unwrap_err(); - assert!(err.is_cancelled(), "cancel should report itself: {err:?}"); - - // Checked immediately, with no polling, unlike the drop test above. That - // asymmetry is the point: `cancel` awaits its own teardown, so if the tree - // is not already gone when it returns, the contract is broken. - assert!( - !alive(grandchild), - "cancel returned while a grandchild was still alive (state {:?}), so it \ - is not awaiting its own cleanup", - process_state(grandchild) - ); - std::fs::remove_dir_all(&dir).ok(); + let grandchild = grandchild_pid(&dir).await; + + let err = running.cancel().await.unwrap_err(); + assert!(err.is_cancelled(), "cancel should report itself: {err:?}"); + + // Checked immediately, with no polling, unlike the drop test above. That + // asymmetry is the point: `cancel` awaits its own teardown, so if the tree + // is not already gone when it returns, the contract is broken. + assert!( + !alive(grandchild), + "cancel returned while a grandchild was still alive (state {:?}), so it \ + is not awaiting its own cleanup", + process_state(grandchild) + ); + std::fs::remove_dir_all(&dir).ok(); + }); } /// A timeout must contain the tree too, not just the process it timed out. -#[tokio::test] -async fn a_timed_out_run_kills_its_children() { - let dir = scratch("timeout"); - let script = fake_agent(&dir); - - let request = Request::new(Agent::Claude, "hi") - .bin(script.to_str().unwrap()) - .timeout(Duration::from_secs(3)); - let running = stream(&request).expect("spawn failed"); - let grandchild = grandchild_pid(&dir).await; - - let err = running.finish().await.unwrap_err(); - assert!( - matches!(err, agent_abstraction::Error::Timeout { .. }), - "got {err:?}" - ); - assert!( - wait_until_dead(grandchild, TEARDOWN_GRACE).await, - "the timeout left a grandchild alive in state {:?}", - process_state(grandchild) - ); - std::fs::remove_dir_all(&dir).ok(); +#[test] +fn a_timed_out_run_kills_its_children() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("timeout"); + let script = fake_agent(&dir); + + let request = Request::new(Agent::Claude, "hi") + .bin(script.to_str().unwrap()) + .timeout(Duration::from_secs(3)); + let running = stream(&request, &reactor.handle()).expect("spawn failed"); + let grandchild = grandchild_pid(&dir).await; + + let err = running.finish().await.unwrap_err(); + assert!( + matches!(err, agent_abstraction::Error::Timeout { .. }), + "got {err:?}" + ); + assert!( + wait_until_dead(grandchild, TEARDOWN_GRACE).await, + "the timeout left a grandchild alive in state {:?}", + process_state(grandchild) + ); + std::fs::remove_dir_all(&dir).ok(); + }); } /// The opt-out still works: an explicitly detached run survives its handle. -#[tokio::test] -async fn detach_lets_a_run_outlive_its_handle() { - let dir = scratch("detach"); - let script = fake_agent(&dir); - - let running = stream(&Request::new(Agent::Claude, "hi").bin(script.to_str().unwrap())) +#[test] +fn detach_lets_a_run_outlive_its_handle() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("detach"); + let script = fake_agent(&dir); + + let running = stream( + &Request::new(Agent::Claude, "hi").bin(script.to_str().unwrap()), + &reactor.handle(), + ) .expect("spawn failed"); - let grandchild = grandchild_pid(&dir).await; - - running.detach(); - // A brief pause is right here rather than a poll: the assertion is that - // nothing kills it, so the test has to give something the chance to. - tokio::time::sleep(Duration::from_millis(500)).await; - - assert!( - alive(grandchild), - "detach must not kill the run; that is the whole point of it" - ); - // Do not leave it behind for the rest of the suite. - let _ = std::process::Command::new("kill") - .args(["-9", &grandchild.to_string()]) - .status(); - std::fs::remove_dir_all(&dir).ok(); + let grandchild = grandchild_pid(&dir).await; + + running.detach(); + // A brief pause is right here rather than a poll: the assertion is that + // nothing kills it, so the test has to give something the chance to. + nagoya::sleep(Duration::from_millis(500)).await; + + assert!( + alive(grandchild), + "detach must not kill the run; that is the whole point of it" + ); + // Do not leave it behind for the rest of the suite. + let _ = std::process::Command::new("kill") + .args(["-9", &grandchild.to_string()]) + .status(); + std::fs::remove_dir_all(&dir).ok(); + }); } /// Write a script that dumps its own environment, as a stand-in for an agent @@ -275,8 +314,8 @@ fn env_dumping_agent(dir: &Path) -> PathBuf { } /// Collect everything the fake agent printed. -async fn captured_env(request: &Request) -> String { - let mut running = stream(request).expect("spawn failed"); +async fn captured_env(request: &Request, reactor: &nagoya::reactor::Handle) -> String { + let mut running = stream(request, reactor).expect("spawn failed"); let mut seen = String::new(); while let Some(event) = running.recv().await { if let agent_abstraction::Event::Text(line) = event { @@ -293,77 +332,89 @@ async fn captured_env(request: &Request) -> String { /// Cargo injects a pile of `CARGO_*` variables into this test process, which /// stand in for the unrelated secrets a Tauri or server host would be holding. /// Under `Inherit` they reach the agent; under `Minimal` they must not. -#[tokio::test] -async fn a_minimal_environment_withholds_the_hosts_variables() { - let dir = scratch("env"); - let script = env_dumping_agent(&dir); - let base = || { - Request::new(Agent::Claude, "hi") - .bin(script.to_str().unwrap()) - .format(agent_abstraction::Format::Text) - }; - - let inherited = captured_env(&base().env_policy(EnvPolicy::Inherit)).await; - assert!( - inherited.contains("CARGO"), - "the control case is broken: Inherit should pass the host environment" - ); - - // No explicit policy: Minimal is the default, which is the property under - // test as much as the filtering itself. - let minimal = captured_env(&base()).await; - assert!( - !minimal.contains("CARGO"), - "host variables leaked under EnvPolicy::Minimal:\n{minimal}" - ); - // ...while still passing what the agent needs to work at all. - assert!(minimal.contains("PATH="), "PATH must survive:\n{minimal}"); - assert!(minimal.contains("HOME="), "HOME must survive:\n{minimal}"); - - // An explicit variable always wins over the policy. - let explicit = captured_env( - &base() - .env_policy(EnvPolicy::Minimal) - .env("AA_EXPLICIT", "kept"), - ) - .await; - assert!(explicit.contains("AA_EXPLICIT=kept"), "{explicit}"); - - std::fs::remove_dir_all(&dir).ok(); +#[test] +fn a_minimal_environment_withholds_the_hosts_variables() { + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("env"); + let script = env_dumping_agent(&dir); + let base = || { + Request::new(Agent::Claude, "hi") + .bin(script.to_str().unwrap()) + .format(agent_abstraction::Format::Text) + }; + + let inherited = + captured_env(&base().env_policy(EnvPolicy::Inherit), &reactor.handle()).await; + assert!( + inherited.contains("CARGO"), + "the control case is broken: Inherit should pass the host environment" + ); + + // No explicit policy: Minimal is the default, which is the property under + // test as much as the filtering itself. + let minimal = captured_env(&base(), &reactor.handle()).await; + assert!( + !minimal.contains("CARGO"), + "host variables leaked under EnvPolicy::Minimal:\n{minimal}" + ); + // ...while still passing what the agent needs to work at all. + assert!(minimal.contains("PATH="), "PATH must survive:\n{minimal}"); + assert!(minimal.contains("HOME="), "HOME must survive:\n{minimal}"); + + // An explicit variable always wins over the policy. + let explicit = captured_env( + &base() + .env_policy(EnvPolicy::Minimal) + .env("AA_EXPLICIT", "kept"), + &reactor.handle(), + ) + .await; + assert!(explicit.contains("AA_EXPLICIT=kept"), "{explicit}"); + + std::fs::remove_dir_all(&dir).ok(); + }); } /// A line with no newline must not be buffered without limit. `lines()` would /// accumulate the whole thing, so a stream that never emits `\n` could exhaust /// memory long before any total cap applied. -#[tokio::test] -async fn an_endless_line_does_not_exhaust_memory() { +#[test] +fn an_endless_line_does_not_exhaust_memory() { use std::os::unix::fs::PermissionsExt as _; - let dir = scratch("longline"); - let script = dir.join("flood.sh"); - // 64 MiB on a single line, no trailing newline until the very end. - std::fs::write( - &script, - "#!/bin/sh\nawk 'BEGIN{for(i=0;i<1000000;i++)printf \"%s\", \"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\"; print \"\"}'\n", - ) - .unwrap(); - std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o700)).unwrap(); + // Each test owns its reactor and keeps it alive to the end: a dropped + // reactor stops the thread that wakes the child's pipes. + let reactor = nagoya::reactor::Reactor::start().expect("reactor"); + nagoya::block_on(async { + let dir = scratch("longline"); + let script = dir.join("flood.sh"); + // 64 MiB on a single line, no trailing newline until the very end. + std::fs::write( + &script, + "#!/bin/sh\nawk 'BEGIN{for(i=0;i<1000000;i++)printf \"%s\", \"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\"; print \"\"}'\n", + ) + .unwrap(); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o700)).unwrap(); - let request = Request::new(Agent::Claude, "hi") - .bin(script.to_str().unwrap()) - .format(agent_abstraction::Format::Text) - .timeout(Duration::from_secs(60)); - let outcome = stream(&request) - .expect("spawn failed") - .finish() - .await - .expect("run failed"); - - // Whatever is kept must respect the cap rather than the 64 MiB produced. - assert!( - outcome.text.len() <= agent_abstraction::MAX_CAPTURE, - "kept {} bytes, over the cap", - outcome.text.len() - ); - std::fs::remove_dir_all(&dir).ok(); + let request = Request::new(Agent::Claude, "hi") + .bin(script.to_str().unwrap()) + .format(agent_abstraction::Format::Text) + .timeout(Duration::from_secs(60)); + let outcome = stream(&request, &reactor.handle()) + .expect("spawn failed") + .finish() + .await + .expect("run failed"); + + // Whatever is kept must respect the cap rather than the 64 MiB produced. + assert!( + outcome.text.len() <= agent_abstraction::MAX_CAPTURE, + "kept {} bytes, over the cap", + outcome.text.len() + ); + std::fs::remove_dir_all(&dir).ok(); + }); }