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(); + }); }