From f8bca7f5f73561d91dbd0e55545cf96b4dde7942 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:14:24 +0530 Subject: [PATCH 01/10] feat(provider): run the chain the panel has been drawing since 0.21.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `config.fallback_specs()` has been read, rendered by `/provider` and then thrown away: `provider::build` took the head spec and nothing else. An operator with three providers in their file was shown a promise the product did not keep. `Vendor` is one enum over the four vendor types, so a chain of arbitrary length has the single concrete type a non-dyn-compatible `Provider` requires. It wraps and never constructs — the four constructors stay written exactly once, in `maker_for`, which is what the one-construction-site gates assert. `Chain` is flat rather than nested over `io_harness::Fallback`. The nested shape does not compile: a `Fallback` makes the RPITIT future's auto-trait inference depend on itself, and rustc reports a type cycle that boxing the recursive arm does not break. The decision to fall over is still io-harness's — `ProviderErrorKind::is_retryable`, the same predicate its own `Fallback` and its own in-run retry ask. `last_served()` answers `None` when the head served, which is a deliberate difference from `Fallback`. io-harness emits `EventKind::FellBackTo` for any `Some` (`run/step.rs:503`) while `Fallback::last_served` answers `Some` for its own primary too, so a chain built on that type would have told the operator "the provider fell over" on every step of every run. `--provider` replaces the whole chain rather than heading it: a run scoped to one vendor on the command line must not spend at another. --- src/exec.rs | 15 +- src/main.rs | 2 +- src/provider.rs | 466 +++++++++++++++++++++++++++++++++++++++++++--- tests/provider.rs | 200 ++++++++++++++++++++ 4 files changed, 652 insertions(+), 31 deletions(-) diff --git a/src/exec.rs b/src/exec.rs index 80ca05c..1ec5a9e 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -493,12 +493,18 @@ pub fn widening(sandbox: Option) -> Option<&'static str> { /// [`resume_main`] rather than written twice, because the sentence a run with no /// provider gets is the one thing both of them are certain to hit on a machine /// that has never run `io setup`. +/// +/// **`--provider` replaces the whole chain rather than heading it.** An operator +/// naming a provider on the command line has said which endpoint this run uses; +/// silently keeping the file's fallbacks underneath would let a run they scoped to +/// one vendor spend at another. The file's own chain is head-plus-fallbacks, which +/// is what [`provider::chain_of`] answers. pub fn spec_for( which: Option, config: &Config, model_override: Option<&str>, -) -> Result { - match (which, config.provider_spec().cloned()) { +) -> Result, String> { + match (which, config.provider_spec().is_some()) { (Some(which), _) => { let (key_var, model_var) = which.vars(); provider::spec_from( @@ -508,9 +514,10 @@ pub fn spec_for( .map(str::to_string) .or_else(|| std::env::var(model_var).ok()), ) + .map(|spec| vec![spec]) } - (None, Some(spec)) => Ok(spec), - (None, None) => Err( + (None, true) => Ok(provider::chain_of(config)), + (None, false) => Err( "no provider is configured; run `io setup`, or pass `--provider` with \ its credential in the environment" .into(), diff --git a/src/main.rs b/src/main.rs index 1a0d405..37f2902 100644 --- a/src/main.rs +++ b/src/main.rs @@ -490,7 +490,7 @@ async fn drive( // dyn-compatible, so what comes back from that call is not a provider: it is // this session, run from inside the arm that built one. provider::build( - spec, + provider::chain_of(&config), model_override, Interactive { screen, diff --git a/src/provider.rs b/src/provider.rs index 2fe76d4..ff0beb3 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -68,30 +68,392 @@ pub fn model_of(spec: &ProviderSpec) -> &str { } } -pub async fn build( - spec: ProviderSpec, - model_override: Option, - with: W, -) -> Result { - let model = |configured: String| model_override.unwrap_or(configured); +/// The whole chain a configuration names, head first. +/// +/// `provider_spec()` is the head and `fallback_specs()` is the tail — io-harness's +/// own split, and the same one [`crate::providers::chain`] draws in the panel. The +/// panel has shown this list since 0.21.0 while [`build`] ran only its head; asking +/// for it in one place is what keeps the chain that runs and the chain that is drawn +/// from being two different answers. +pub fn chain_of(config: &io_harness::Config) -> Vec { + config + .provider_spec() + .into_iter() + .chain(config.fallback_specs()) + .cloned() + .collect() +} + +/// One provider, whichever vendor built it. +/// +/// **A chain of arbitrary length needs one concrete type, and this is it.** +/// `Provider::complete` returns `impl Future` (RPITIT), so the trait is not +/// dyn-compatible and there is no `Box` to put in a `Vec` — +/// `io_harness::provider::Fallback`'s own module says as much. An enum is the +/// remaining shape: four arms, one type, and `Fallback` can nest over it. +/// +/// **It wraps and never constructs.** Every value inside one of these arms was +/// built in [`build`], which is still the only site in this crate that names a +/// vendor constructor — the rule `tests/provider.rs` enforces and which this +/// release does not widen. +/// +/// Every method delegates across all four arms, for the reason [`Watched`] gives +/// at length: the trait has eight defaulted methods, and a default that fired here +/// would change behaviour silently rather than fail. The async ones are written as +/// `async fn` rather than by returning the inner future, because four arms return +/// four distinct future types and only an `async` body can unify them. +pub enum Vendor { + OpenRouter(OpenRouter), + Anthropic(Anthropic), + OpenAi(OpenAi), + Compatible(Compatible), +} + +impl Provider for Vendor { + async fn complete(&self, request: CompletionRequest) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.complete(request).await, + Self::Anthropic(p) => p.complete(request).await, + Self::OpenAi(p) => p.complete(request).await, + Self::Compatible(p) => p.complete(request).await, + } + } + + async fn complete_streaming( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + ) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.complete_streaming(request, on_token).await, + Self::Anthropic(p) => p.complete_streaming(request, on_token).await, + Self::OpenAi(p) => p.complete_streaming(request, on_token).await, + Self::Compatible(p) => p.complete_streaming(request, on_token).await, + } + } + + async fn complete_streaming_calls( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + on_call: &(dyn Fn(usize, &ToolCall) + Send + Sync), + ) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.complete_streaming_calls(request, on_token, on_call).await, + Self::Anthropic(p) => p.complete_streaming_calls(request, on_token, on_call).await, + Self::OpenAi(p) => p.complete_streaming_calls(request, on_token, on_call).await, + Self::Compatible(p) => p.complete_streaming_calls(request, on_token, on_call).await, + } + } + + async fn models(&self) -> io_harness::Result> { + match self { + Self::OpenRouter(p) => p.models().await, + Self::Anthropic(p) => p.models().await, + Self::OpenAi(p) => p.models().await, + Self::Compatible(p) => p.models().await, + } + } + + async fn reachable(&self) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.reachable().await, + Self::Anthropic(p) => p.reachable().await, + Self::OpenAi(p) => p.reachable().await, + Self::Compatible(p) => p.reachable().await, + } + } + + fn model_hint(&self) -> Option<&str> { + match self { + Self::OpenRouter(p) => p.model_hint(), + Self::Anthropic(p) => p.model_hint(), + Self::OpenAi(p) => p.model_hint(), + Self::Compatible(p) => p.model_hint(), + } + } + + fn name(&self) -> &str { + match self { + Self::OpenRouter(p) => p.name(), + Self::Anthropic(p) => p.name(), + Self::OpenAi(p) => p.name(), + Self::Compatible(p) => p.name(), + } + } + + fn prompt_family(&self) -> PromptFamily { + match self { + Self::OpenRouter(p) => p.prompt_family(), + Self::Anthropic(p) => p.prompt_family(), + Self::OpenAi(p) => p.prompt_family(), + Self::Compatible(p) => p.prompt_family(), + } + } + + fn accepts_images(&self) -> bool { + match self { + Self::OpenRouter(p) => p.accepts_images(), + Self::Anthropic(p) => p.accepts_images(), + Self::OpenAi(p) => p.accepts_images(), + Self::Compatible(p) => p.accepts_images(), + } + } + + fn endpoint(&self) -> Option<&str> { + match self { + Self::OpenRouter(p) => p.endpoint(), + Self::Anthropic(p) => p.endpoint(), + Self::OpenAi(p) => p.endpoint(), + Self::Compatible(p) => p.endpoint(), + } + } + + fn endpoints(&self) -> Vec<&str> { + match self { + Self::OpenRouter(p) => p.endpoints(), + Self::Anthropic(p) => p.endpoints(), + Self::OpenAi(p) => p.endpoints(), + Self::Compatible(p) => p.endpoints(), + } + } + + fn last_served(&self) -> Option { + match self { + Self::OpenRouter(p) => p.last_served(), + Self::Anthropic(p) => p.last_served(), + Self::OpenAi(p) => p.last_served(), + Self::Compatible(p) => p.last_served(), + } + } +} + +/// A chain of providers, each answering when the one before it fails. +/// +/// **The decision to fall over is io-harness's**, not this crate's: +/// [`io_harness::ProviderErrorKind::is_retryable`] is the same predicate its own +/// `Fallback` and its own in-run retry both ask, and the reasoning is its own — a +/// failure about the request or about the caller's configuration will happen +/// identically at the next vendor, so falling over on one fails twice and spends +/// twice. io-cli holds no opinion here and must not grow one. +/// +/// **Flat rather than nested, and that is not a preference.** The obvious shape is +/// `Fallback>`, folded recursively so the tail is +/// itself one type. It does not compile: `Provider::complete` returns `impl Future` +/// (RPITIT), so a `Chain` whose secondary is a `Chain` makes the future's auto-trait +/// inference depend on itself, and rustc reports a type cycle that `Box::pin` on the +/// recursive arm does not break. A list and a loop have neither problem. +/// +/// **`last_served` answers `None` when the head served, and that is a deliberate +/// difference from `Fallback`.** io-harness emits `EventKind::FellBackTo` for any +/// `Some` (`run/step.rs:503`), while `Fallback::last_served` answers `Some` for its +/// *primary* too — so a chain built from that type would report "the provider fell +/// over" on every step of every run, including the ones where nothing did. Reporting +/// only a link that is not the head makes the event mean what its name says, and +/// makes a chain whose head is answering indistinguishable from no chain at all, +/// which is what an operator would expect. +/// +/// Generic over the link so the fall-through is assertable. The chain a +/// configuration builds is `Chain`; a test builds `Chain` over links +/// that fail on demand with a chosen `ProviderErrorKind`, which is the only way to +/// prove that a retryable failure falls through and an `Auth` failure does not +/// without a network and without spending anything. +pub struct Chain

{ + links: Vec

, + /// Which link answered last, as an index, or `usize::MAX` for nobody yet. + /// + /// An atomic rather than a lock for the reason io-harness gives for the same + /// field: a `MutexGuard` is not `Send` and `complete`'s future has to be. + served: std::sync::atomic::AtomicUsize, +} + +/// Nobody has answered yet. +const UNSERVED: usize = usize::MAX; + +impl Chain

{ + /// A chain over `links`, head first. `None` when there are no links at all. + pub fn of(links: Vec

) -> Option { + if links.is_empty() { + return None; + } + Some(Self { + links, + served: std::sync::atomic::AtomicUsize::new(UNSERVED), + }) + } + + /// The head, which is the provider every question is asked of first. + fn head(&self) -> &P { + &self.links[0] + } + + fn note(&self, who: usize) { + self.served.store(who, std::sync::atomic::Ordering::Relaxed); + } + + /// Whether a different vendor is worth trying, asked of io-harness. + fn worth_another(error: &io_harness::Error) -> bool { + matches!(error, io_harness::Error::Provider { kind, .. } if kind.is_retryable()) + } +} + +impl Provider for Chain

{ + async fn complete(&self, request: CompletionRequest) -> io_harness::Result { + let mut last = None; + for (index, link) in self.links.iter().enumerate() { + match link.complete(request.clone()).await { + Ok(response) => { + self.note(index); + return Ok(response); + } + Err(error) if Self::worth_another(&error) => last = Some(error), + // Not worth another vendor: a bad credential or a malformed + // request fails the same way everywhere, and trying the next link + // would spend somebody else's budget to be told so again. + Err(error) => return Err(error), + } + } + Err(last.expect("a chain has at least one link, so the loop ran at least once")) + } + + async fn complete_streaming( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + ) -> io_harness::Result { + let mut last = None; + for (index, link) in self.links.iter().enumerate() { + match link.complete_streaming(request.clone(), on_token).await { + Ok(response) => { + self.note(index); + return Ok(response); + } + Err(error) if Self::worth_another(&error) => last = Some(error), + Err(error) => return Err(error), + } + } + Err(last.expect("a chain has at least one link, so the loop ran at least once")) + } + + async fn complete_streaming_calls( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + on_call: &(dyn Fn(usize, &ToolCall) + Send + Sync), + ) -> io_harness::Result { + let mut last = None; + for (index, link) in self.links.iter().enumerate() { + match link + .complete_streaming_calls(request.clone(), on_token, on_call) + .await + { + Ok(response) => { + self.note(index); + return Ok(response); + } + Err(error) if Self::worth_another(&error) => last = Some(error), + Err(error) => return Err(error), + } + } + Err(last.expect("a chain has at least one link, so the loop ran at least once")) + } + + async fn models(&self) -> io_harness::Result> { + self.head().models().await + } + + async fn reachable(&self) -> io_harness::Result { + self.head().reachable().await + } + + fn model_hint(&self) -> Option<&str> { + self.head().model_hint() + } + + fn name(&self) -> &str { + self.head().name() + } + + fn prompt_family(&self) -> PromptFamily { + self.head().prompt_family() + } + + /// Every link, or none of them — io-harness's own rule for the same question. + /// + /// Reporting the head's answer would let an image reach a link that cannot read + /// it on the one call that matters, the fall-through. The link would refuse it + /// anyway, so the conjunction only changes *when* the operator finds out: before + /// the run rather than midway through a failure. + fn accepts_images(&self) -> bool { + self.links.iter().all(Provider::accepts_images) + } + + fn endpoint(&self) -> Option<&str> { + self.head().endpoint() + } + + /// **Every link's hosts, and this is the reason `endpoints` exists at all.** + /// io-harness's egress policy is deny-by-default and a run authorizes its + /// provider's hosts before its first step, so a chain that reported only the + /// head's host would make a fall-through a way to reach a host the policy never + /// saw. + fn endpoints(&self) -> Vec<&str> { + self.links.iter().flat_map(Provider::endpoints).collect() + } + + /// The link that answered, unless it was the head. + /// + /// See the type's own documentation: io-harness emits `EventKind::FellBackTo` + /// for any `Some`, so answering `Some` for the head would report a fall-through + /// on every step of every run that configured a chain. + fn last_served(&self) -> Option { + match self.served.load(std::sync::atomic::Ordering::Relaxed) { + UNSERVED | 0 => None, + index => self.links.get(index).map(|link| { + link.last_served() + .unwrap_or_else(|| link.name().to_string()) + }), + } + } +} + +/// One link's maker: a closure that builds this vendor for a named model. +/// +/// Boxed because the four arms produce four closure types and the chain holds a +/// list of them. The credential is captured here and nowhere else, which is what +/// keeps it out of every struct in this crate. +type Maker = Box Result + Send + Sync>; + +/// The maker for one spec, and the model that spec names. +fn maker_for(spec: ProviderSpec) -> Result<(Maker, String), String> { match spec { - ProviderSpec::OpenRouter { model: m, api_key } => { + ProviderSpec::OpenRouter { model, api_key } => { let key = key_for(api_key, "OPENROUTER_API_KEY")?; - let make = move |name: &str| Ok(OpenRouter::new(key.clone(), name)); - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| { + Ok(Vendor::OpenRouter(OpenRouter::new(key.clone(), name))) + }), + model, + )) } - ProviderSpec::Anthropic { model: m, api_key } => { + ProviderSpec::Anthropic { model, api_key } => { let key = key_for(api_key, "ANTHROPIC_API_KEY")?; - let make = move |name: &str| Ok(Anthropic::new(key.clone(), name)); - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| { + Ok(Vendor::Anthropic(Anthropic::new(key.clone(), name))) + }), + model, + )) } - ProviderSpec::OpenAi { model: m, api_key } => { + ProviderSpec::OpenAi { model, api_key } => { let key = key_for(api_key, "OPENAI_API_KEY")?; - let make = move |name: &str| Ok(OpenAi::new(key.clone(), name)); - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| Ok(Vendor::OpenAi(OpenAi::new(key.clone(), name)))), + model, + )) } ProviderSpec::Compatible { - model: m, + model, preset, base_url, api_key, @@ -103,16 +465,25 @@ pub async fn build( return Err("this provider names neither a preset nor a base URL".into()); } let auth = auth.unwrap_or(Auth::Bearer); - let make = move |name: &str| match (&preset, &base_url) { - (Some(preset), _) => { - Compatible::preset(preset, key.clone(), name).map_err(|error| error.to_string()) - } - (None, Some(base)) => Ok(Compatible::new(base.clone(), auth, key.clone(), name)), - // Refused above, before anything was built, so this arm exists - // only to make the match total. - (None, None) => Err("this provider names neither a preset nor a base URL".into()), - }; - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| match (&preset, &base_url) { + (Some(preset), _) => Compatible::preset(preset, key.clone(), name) + .map(Vendor::Compatible) + .map_err(|error| error.to_string()), + (None, Some(base)) => Ok(Vendor::Compatible(Compatible::new( + base.clone(), + auth, + key.clone(), + name, + ))), + // Refused above, before anything was built, so this arm exists + // only to make the match total. + (None, None) => { + Err("this provider names neither a preset nor a base URL".into()) + } + }), + model, + )) } // `ProviderSpec` is `#[non_exhaustive]`: a provider the harness gains and // this release has not seen is refused by name rather than driven wrongly. @@ -122,6 +493,49 @@ pub async fn build( } } +/// Build the chain a configuration names and hand it to `with`. +/// +/// `specs` is head first — [`chain_of`] is how a caller gets one. A single-element +/// list builds exactly one provider and no `Fallback` at all, which is what keeps +/// an operator who has configured one provider on precisely the code path they +/// were on before this release. +/// +/// `model_override` is `-m/--model`: it replaces the model the configuration names +/// for the **head only**, which is why it is applied to the extracted model rather +/// than by rewriting the spec. A fallback link keeps the model its own entry names — +/// naming one model for a whole chain would ask a second vendor for a model id only +/// the first one serves. +pub async fn build( + specs: Vec, + model_override: Option, + with: W, +) -> Result { + let mut makers = Vec::new(); + let mut models = Vec::new(); + for spec in specs { + let (maker, model) = maker_for(spec)?; + makers.push(maker); + models.push(model); + } + let Some(head) = models.first().cloned() else { + return Err("no provider is configured; run `io setup`".into()); + }; + let head = model_override.unwrap_or(head); + // The tail's models are fixed at build time; only the head's follows the + // maker's argument, so a `/model` switch changes what is asked first and + // leaves every fallback answering as its own entry says it should. + let tail: Vec = models.split_off(1); + let make = move |name: &str| { + let mut links = Vec::with_capacity(makers.len()); + links.push(makers[0](name)?); + for (maker, model) in makers[1..].iter().zip(tail.iter()) { + links.push(maker(model)?); + } + Chain::of(links).ok_or_else(|| "no provider is configured; run `io setup`".to_string()) + }; + Ok(with.call(make, head).await) +} + /// A provider that keeps a copy of every request on the way past. /// /// **The only way io-cli can say what is in the context window.** io-harness diff --git a/tests/provider.rs b/tests/provider.rs index 8a26667..6dd5f66 100644 --- a/tests/provider.rs +++ b/tests/provider.rs @@ -249,3 +249,203 @@ fn f10_the_reviewer_judges_and_never_drives() { "the reviewer is built for the contract's criterion and for nothing else", ); } + +// --------------------------------------------------------------------------- +// F5 and F6 — the chain that runs is the chain the panel draws. + +use io_cli::provider::Chain; +use io_harness::{CompletionRequest, CompletionResponse, Provider, ProviderErrorKind}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; + +/// A link that answers, or fails in a chosen way, and counts what it was asked. +/// +/// The counter lives outside the link because [`Chain::of`] takes ownership of its +/// links. It is what makes F6 assertable at all: "the second provider is never +/// called" is a statement about a call that did not happen, and no message can tell +/// that apart from a call that happened and was discarded. +struct Fake { + label: String, + fail: Option, + calls: Arc, +} + +/// A counter to read afterwards, and the link that increments it. +fn link(label: &str, fail: Option) -> (Fake, Arc) { + let calls = Arc::new(AtomicUsize::new(0)); + ( + Fake { + label: label.into(), + fail, + calls: Arc::clone(&calls), + }, + calls, + ) +} + +fn asked(calls: &Arc) -> usize { + calls.load(Ordering::Relaxed) +} + +impl Provider for Fake { + async fn complete( + &self, + _request: CompletionRequest, + ) -> io_harness::Result { + self.calls.fetch_add(1, Ordering::Relaxed); + match self.fail { + Some(kind) => Err(io_harness::Error::Provider { + kind, + status: None, + retry_after: None, + message: format!("{} was asked and refused", self.label), + }), + None => Ok(CompletionResponse { + text: Some(self.label.clone()), + ..Default::default() + }), + } + } + + fn name(&self) -> &str { + &self.label + } + + fn endpoints(&self) -> Vec<&str> { + vec![&self.label] + } +} + +fn asking() -> CompletionRequest { + CompletionRequest { + system: "s".into(), + user: "u".into(), + ..Default::default() + } +} + +#[tokio::test] +async fn f5_a_retryable_failure_falls_through_to_the_next_link() { + let (first, first_calls) = link("first", Some(ProviderErrorKind::Server)); + let (second, second_calls) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("two links are a chain"); + + let answer = chain.complete(asking()).await.expect("the second answers"); + + assert_eq!( + answer.text.as_deref(), + Some("second"), + "a head that failed in a way another vendor might survive must not end the turn", + ); + assert_eq!( + chain.last_served().as_deref(), + Some("second"), + "the link that answered is what io-harness records and what the status line moves to", + ); + assert_eq!((asked(&first_calls), asked(&second_calls)), (1, 1)); +} + +#[tokio::test] +async fn f5_the_head_is_asked_first_and_a_link_below_it_is_not_asked_at_all() { + let (first, first_calls) = link("first", None); + let (second, second_calls) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + + let answer = chain.complete(asking()).await.expect("the head answers"); + + assert_eq!( + answer.text.as_deref(), + Some("first"), + "the operator's first choice is the provider a question is put to", + ); + assert_eq!( + (asked(&first_calls), asked(&second_calls)), + (1, 0), + "a link below a head that answered is not asked, so it is not billed", + ); +} + +#[tokio::test] +async fn f5_a_head_that_answered_reports_no_fallover() { + // **The whole reason this crate does not build on `io_harness::Fallback`.** + // io-harness emits `EventKind::FellBackTo` for any `Some` from `last_served` + // (`run/step.rs:503`), and `Fallback::last_served` answers `Some` for its own + // primary — so a chain built from that type would tell the operator "the + // provider fell over" on every step of every run. + let (first, _) = link("first", None); + let (second, _) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + chain.complete(asking()).await.expect("the head answers"); + + assert_eq!( + chain.last_served(), + None, + "nothing fell over, so nothing may be reported as having fallen over", + ); +} + +#[tokio::test] +async fn f5_one_provider_is_a_chain_that_behaves_exactly_as_no_chain_did() { + let (only, _) = link("only", None); + let chain = Chain::of(vec![only]).expect("one link is a chain"); + + let answer = chain + .complete(asking()) + .await + .expect("the only link answers"); + + assert_eq!(answer.text.as_deref(), Some("only")); + assert_eq!( + chain.last_served(), + None, + "an operator who configured one provider must stay on the path they were on", + ); +} + +#[tokio::test] +async fn f6_a_bad_credential_on_the_head_does_not_spend_the_link_below_it() { + let (first, first_calls) = link("first", Some(ProviderErrorKind::Auth)); + let (second, second_calls) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + + let refused = chain.complete(asking()).await; + + assert!( + refused.is_err(), + "a wrong key is not a failure another vendor can survive", + ); + assert_eq!( + (asked(&first_calls), asked(&second_calls)), + (1, 0), + "the second link is never called, so a typo in a key cannot start spending elsewhere", + ); + assert_eq!( + chain.last_served(), + None, + "nobody answered, so no provider may be named as having done so", + ); +} + +#[test] +fn f5_every_link_s_host_is_authorized_and_not_only_the_head_s() { + // io-harness's egress policy is deny-by-default and authorizes the provider's + // hosts before the first step. A chain reporting only its head's host would + // make a fall-through a way to reach a host the policy never saw. + let (first, _) = link("first", None); + let (second, _) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + + assert_eq!(chain.endpoints(), vec!["first", "second"]); +} + +#[test] +fn f5_an_image_is_refused_unless_every_link_accepts_one() { + // The conjunction is io-harness's own rule for the same question, and the + // reason is that the fall-through is the one call where it matters. + let (first, _) = link("first", None); + let (second, _) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + // `Fake` takes the trait's default, which is `false`, so the conjunction is + // asserted rather than a default being read back. + assert!(!chain.accepts_images()); +} From 465543b95cacf14e2bd496a733ce4b7281e1b1bb Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:25:04 +0530 Subject: [PATCH 02/10] feat(effort): say how much reasoning a turn buys, and re-file /profile to make room MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TaskContract::with_effort` has been public and unused since io-harness gained it. `/effort low|medium|high` puts a level on every later turn, `/effort off` returns to sending no reasoning field at all, and a bare `/effort` reports. The absent case is not a fourth level. `TaskContract::effort` is an `Option` and `None` sends the pre-0.31.0 request body byte for byte, so `contract::buying` leaves the contract Debug-identical when nothing was asked for — asserted through the same instrument that holds a configured-nothing session to the contract it built before. `Turn` stood at ten of ten, and `src/commands.rs` pre-committed the answer for the release that met the bound: re-file what is in the wrong group, do not widen it. `/profile` was misfiled by that group's own rule — it changes the overlay every later turn is built from rather than acting on the work the turn just finished — so it moves to `Session` and `/effort` takes the slot. The level is a posture and not a one-shot: it is read by each turn rather than taken, so it survives to the next one, and it survives `Status::forget_run` beside `policy` and `planning`. It is drawn by both renderers, because a field added to `Status::fields` alone is green in a unit test and nowhere the binary draws. The decisions live in `contract::buying`, `app::reasoning_of` and `app::reasoning_said` rather than in the driver, because nothing under `tests/` links `src/main.rs` and a conditional written there could be neither asserted nor sabotaged. --- src/app.rs | 43 +++++++++++++++ src/commands.rs | 70 ++++++++++++++++++++++++- src/contract.rs | 27 ++++++++++ src/main.rs | 43 +++++++++++++++ src/status.rs | 32 ++++++++++++ tests/commands.rs | 130 ++++++++++++++++++++++++++++++++++++++++++++-- tests/contract.rs | 48 +++++++++++++++++ tests/status.rs | 53 +++++++++++++++++++ 8 files changed, 441 insertions(+), 5 deletions(-) diff --git a/src/app.rs b/src/app.rs index ada46be..81941cd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2367,6 +2367,49 @@ fn gate_said( .or_else(|| (!last.detail.trim().is_empty()).then(|| last.detail.clone())) } +/// What `/effort` changes the session's level to, or `None` to change nothing. +/// +/// The outer `Option` is the question and the inner one is the answer, which reads +/// awkwardly and is the honest shape: `Some(None)` is `/effort off` — a level was +/// set, and the level set is the absence of one — while `None` is a bare `/effort`, +/// which is a question and must leave the session as it found it. +/// +/// Here rather than in the driver because nothing under `tests/` links +/// `src/main.rs`: an assignment written there could be neither asserted nor +/// sabotaged, and this release's F1 turns on exactly the difference between a level +/// that survives the turn and one that does not. +#[must_use] +pub fn reasoning_of(said: crate::commands::Reasoning) -> Option> { + match said { + crate::commands::Reasoning::Buy(level) => Some(Some(level)), + crate::commands::Reasoning::Off => Some(None), + crate::commands::Reasoning::Report => None, + } +} + +/// The line `/effort` commits into the scrollback. +/// +/// `now` is the level in force **after** the command has been applied, so one +/// sentence covers setting and reporting: what an operator wants to read back is +/// the state they are now in, not the instruction they gave. +/// +/// The absent case says what it means rather than naming a level. "No reasoning +/// field" is the fact — io-harness sends the pre-0.31.0 request body — and calling +/// it "off" on screen would suggest a fourth setting between `low` and nothing. +#[must_use] +pub fn reasoning_said(said: crate::commands::Reasoning, now: Option) -> String { + let level = match now { + Some(level) => format!("{level} reasoning"), + None => "no reasoning field, which is what this product sent before 0.26.0".to_string(), + }; + match said { + crate::commands::Reasoning::Report => { + format!("every turn asks for {level}") + } + _ => format!("every turn from here asks for {level}"), + } +} + /// The one line a turn's gate commits into the scrollback, with its tone. /// /// `None` for a turn nothing gated, which is the ordinary case and not a thing to diff --git a/src/commands.rs b/src/commands.rs index febb302..d4c4a88 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -239,6 +239,10 @@ pub const COMMANDS: &[(&str, &str)] = &[ "/profile", "switch to a named profile from the configuration, for this session", ), + ( + "/effort", + "how much reasoning the next turn buys: low, medium, high, or off", + ), ( "/contain", "run turns contained, so the agent can fan out: on, off, or ask", @@ -356,9 +360,25 @@ impl Group { /// compile error in a file nobody was editing, rather than a test that says /// which command has no home. pub const GROUPS: &[(Group, &[&str])] = &[ + // **`/profile` moved here in 0.26.0, and it is a correction rather than a way + // of making room** — the third time this sentence has been written, after + // 0.19.0's `/mcp` and `/provider` and 0.22.0's `/image` and `/copy`. `Turn` + // means a command that acts on the work the turn just finished, which is the + // argument the note above that group makes at length. `/profile` acts on + // nothing that has happened: it changes which configuration overlay every + // *later* turn is built from, which is a property of the session. It was filed + // under `Turn` because switching one feels like something you do between turns, + // and where a command sits is decided by what it acts on rather than by when it + // is typed. + // + // That this is also what makes room for `/effort` is the order the bound was + // meant to force: `src/commands.rs` pre-committed the answer for the release + // that met it — re-file what is in the wrong group, do not widen the bound — + // and a correction that pays for itself is the correction working, not a + // loophole in it. ( Group::Session, - &["/clear", "/resume", "/fork", "/setup", "/exit"], + &["/clear", "/resume", "/fork", "/profile", "/setup", "/exit"], ), // **`/image`, `/copy` and `/copy diff` moved here in 0.22.0, and it is a // correction rather than a way of making room** — the same sentence 0.19.0 @@ -379,9 +399,16 @@ pub const GROUPS: &[(Group, &[&str])] = &[ Group::Turn, &[ "/model", + // **`/effort` takes the slot `/profile` left**, and it belongs here on + // this group's own rule: it decides how much reasoning the next turn + // buys, which is a property of the turn and of nothing else. It asks + // the store nothing, which is what `Inspect` means, and it writes no + // configuration file, which is what `Configure` means — the level is + // this session's and dies with it, for the reason the release contract + // records under `open_questions`. + "/effort", "/contain", "/plan", - "/profile", "/steer", "/compact", "/image", @@ -886,6 +913,24 @@ pub fn skills( ) } +/// What `/effort` was asked to do. +/// +/// **Three words and not four, because the absent case is not a fourth level.** +/// `TaskContract::effort` is an `Option` and `None` sends no reasoning +/// field at all — the request body this product sent before 0.26.0, byte for byte. +/// So "off" is a real answer an operator can want and is not the same as "low", +/// which buys a thinking budget of 1,024 tokens on Anthropic and sets +/// `reasoning_effort` on the OpenAI wire. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Reasoning { + /// Buy this much, for this turn and every turn after it. + Buy(io_harness::Effort), + /// Send no reasoning field at all. + Off, + /// Say what is in force, and change nothing. + Report, +} + /// What the driver should do about a slash command. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Action { @@ -1096,6 +1141,11 @@ pub enum Action { Provider, /// List the named profiles, and switch to one for this session. Profile, + /// Say how much reasoning the next turn should buy, or report what is in force. + /// + /// The word is spelled the way `/contain on` is: an argument on the existing + /// command rather than a second command, and a bare `/effort` is a question. + Effort(Reasoning), /// List the capability bundles a `[[plugin]]` entry declared: what each one /// contributed, and what each dropped bundle failed on. /// @@ -1731,6 +1781,22 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { "skills" => Action::Skills, "provider" | "providers" => Action::Provider, "profile" | "profiles" => Action::Profile, + // `Effort` carries `FromStr`, so the three levels are spelled io-harness's + // way rather than this crate's — a second spelling of `low` would be a + // second vocabulary for one wire field. "off" is handled beside it because + // the harness has no word for the absent case; it is an `Option`, not a + // fourth variant. + "effort" | "reasoning" => match input.split_whitespace().nth(1) { + Some("off" | "none") => Action::Effort(Reasoning::Off), + Some(word) => match word.parse::() { + Ok(level) => Action::Effort(Reasoning::Buy(level)), + // An unrecognised word reports rather than guesses. Choosing the + // nearest level for `/effort hgih` would spend a turn's reasoning + // budget on a typo. + Err(_) => Action::Effort(Reasoning::Report), + }, + None => Action::Effort(Reasoning::Report), + }, // `/plugins` is admitted for the same reason `/servers` and `/providers` // are: the thing being listed is plural, so the plural is what a hand // reaches for, and refusing it teaches nothing. diff --git a/src/contract.rs b/src/contract.rs index e095764..b20a996 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -311,6 +311,33 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta resolve_skills(contract) } +/// Apply what `/effort` said to the contract of a turn that is about to run. +/// +/// **A function here rather than a seventh parameter of [`session`], and the +/// argument is the one `src/main.rs` already makes for `/compact`'s fold.** Three +/// of that builder's callers make a contract nothing runs — the startup reading +/// and the two reporting pages — so a parameter would be three `None`s of noise +/// and a signature break, against one call on the contract that is actually a +/// turn. +/// +/// **A function and not a line in the driver**, because nothing under `tests/` +/// links `src/main.rs`: a conditional written there could not be asserted or +/// sabotaged. The decision is here, the driver holds one call. +/// +/// **`None` must produce the contract unchanged, byte for byte.** +/// `TaskContract::effort` is an `Option` and its absence sends no +/// reasoning field at all — `openai_wire.rs:1443` and `anthropic.rs:1529` are the +/// pre-0.31.0 bodies — so an unconditional `with_effort` carrying a default would +/// change what every operator's turn asks for, silently, while passing every test +/// that only looks at the level it set. +#[must_use] +pub fn buying(contract: TaskContract, effort: Option) -> TaskContract { + match effort { + Some(effort) => contract.with_effort(effort), + None => contract, + } +} + /// The criterion this configuration resolves to, with its reviewer already built. /// /// `None` covers every case in which the run must not be gated: no section, a diff --git a/src/main.rs b/src/main.rs index 37f2902..4950eb9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -960,6 +960,16 @@ async fn loop_over Result>( let mut planning = false; // One `/compact` typed at an idle prompt, spent by the next turn that starts. let mut fold_next = false; + // **How much reasoning every later turn buys, and it is not spent by being + // used.** The sibling of `fold_next` and its opposite in the one way that + // matters: a fold is a one-shot and a level is a posture, so this is read by + // each turn and cleared by nothing but `/effort off`. `None` is the absence of + // the reasoning field rather than a fourth level — see `contract::buying`. + // + // Session state and not configuration: there is no `[app.io-cli] effort` key, + // and it deliberately does not live on `Capabilities`, which is rebuilt from + // the file on every turn and would wipe the level each time. + let mut effort: Option = None; // **The file, held so it can be read again — which through 0.17.0 it never // was.** io-harness composes the instruction files inside `Config::discover` // and stores the result privately; there is no `Config::reload`, so a @@ -3290,6 +3300,23 @@ async fn loop_over Result>( // the completion above and the turn below build it — so what may // be attached and what the agent may read are the same set by // construction rather than by two agreeing lists. + // **A posture and not a one-shot**, which is the whole of what the + // three words mean here: a level set now is what every later turn + // buys until `/effort off`. The sentence is built by + // `io_cli::app::reasoning_said` so it can be asserted; this arm + // holds the assignment and nothing else. + Action::Effort(said) => { + if let Some(level) = io_cli::app::reasoning_of(said) { + effort = level; + // The status line carries the same value, not a second + // opinion about it. Set beside the assignment so the two + // cannot drift — the 0.25.0 defect where one wave gave + // `App` a branch field and another gave `Status` one, and + // the line drew the half nothing wrote. + app.status.effort = level; + } + app.record(Tone::Muted, io_cli::app::reasoning_said(said, effort)); + } Action::Contain(want) => match (&containment, want) { // Nothing to switch. Said as the configuration gap it is, // with the key that closes it, rather than as a refusal — @@ -3785,6 +3812,13 @@ async fn loop_over Result>( // Taken rather than read: one request, one turn. A queue // of three prompts must not fold three times. std::mem::take(&mut fold_next), + // **Read rather than taken, which is the whole difference + // between a posture and a one-shot.** `/compact` folds the + // turn it was typed before and nothing after it; `/effort` + // says how hard to think until it is told otherwise, so a + // `mem::take` here would make every level last exactly one + // turn — the defect F1's sabotage names. + effort, text, // **This turn's own clock, not the session's.** What a // reader wants of the row above the prompt is how long the @@ -4091,6 +4125,11 @@ async fn turn( // with `mem::take`, so a contract reused for every turn would not fold every // turn; io-cli builds a fresh one anyway. fold: bool, + // How much reasoning this turn buys — what `/effort` last said, or `None` for a + // session that has never said it. `None` is the absence of the field rather + // than a fourth level, so a session that never types `/effort` sends the + // request body this crate sent before 0.26.0. + effort: Option, text: String, started: Instant, ) -> Result { @@ -4202,6 +4241,10 @@ async fn turn( } else { contract }; + // **What `/effort` last said, applied here for the reason above.** The decision + // is `contract::buying`'s, not this file's — nothing under `tests/` links this + // binary, so a conditional written here could not be asserted or sabotaged. + let contract = io_cli::contract::buying(contract, effort); // Set while a fold has been asked for and no `Compacted` event has arrived. // A one-shot: io-harness spends the request whether or not it folds, so what // this guards is the report and never a retry. diff --git a/src/status.rs b/src/status.rs index d52e362..24975a4 100644 --- a/src/status.rs +++ b/src/status.rs @@ -353,6 +353,19 @@ pub struct Status { /// `false` renders as nothing at all, on the rule this line already holds: /// a session that has not asked to plan is not a session planning zero times. pub planning: bool, + /// How much reasoning every turn is buying, from `/effort`. + /// + /// A standing choice like [`Self::planning`], and it survives + /// [`Status::forget_run`] for the same reason: it holds until the operator + /// says otherwise, and the turn it was set on is over by the time anybody + /// reads it. + /// + /// `None` renders as nothing at all, on the rule this line already holds — a + /// session that has never said `/effort` is not a session buying zero + /// reasoning, it is one sending no reasoning field. Rendering a word for the + /// default would also put a field on every operator's status line for a + /// release that changed nothing about their turns. + pub effort: Option, /// Where this turn's verification gate stands, in one word. /// /// **A word and never a mark**, for the reason [`WORDS`] is a list of words @@ -559,6 +572,7 @@ impl Status { Self { model: model.into(), provider: None, + effort: None, steps: None, unknown: 0, policy: None, @@ -1207,6 +1221,13 @@ impl Status { if self.planning { fields.push(Field::new("planning".to_string(), Tone::Normal)); } + // Beside `planning` because it is the same class of fact: a standing choice + // the operator made that holds until they unmake it, and one that changes + // what every later turn costs. `Normal` for that reason too — it is not a + // footnote about the turn that just ended. + if let Some(effort) = self.effort { + fields.push(Field::new(format!("effort {effort}"), Tone::Normal)); + } // **Immediately right of the planning phase and left of every counter, // which is a decision about a narrow terminal and not a grouping.** The // rule this row already states is that a standing mode which stops the @@ -1544,6 +1565,17 @@ impl Status { } allowed.push(Span::styled("planning", muted)); } + // Beside `planning` here as well as on `Status::line`, and out of the same + // field, for the reason that paragraph gives at length: the binary draws + // this footer at every ordinary prompt, so a standing choice added to + // `line` alone is a choice no operator ever sees. 0.12.0's planning field + // is the precedent and this release is not going to repeat it. + if let Some(effort) = self.effort { + if !allowed.is_empty() { + allowed.push(Span::styled(separator, muted)); + } + allowed.push(Span::styled(format!("effort {effort}"), muted)); + } // **`counts` and NOT the right-hand group, and this release measured why.** // The branch first went in beside the posture, on the argument that it // describes the circumstances the agent works in rather than counting diff --git a/tests/commands.rs b/tests/commands.rs index 278ef2d..2e8f73d 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -82,6 +82,7 @@ fn the_commands_are_the_commands() { // whose subject is a tool that is not io. "/import", "/profile", + "/effort", "/contain", // 0.12.0 — the planning phase stopped being something // `[app.io-cli.containment]` switched on by accident, so it needs a @@ -308,12 +309,135 @@ fn f12_resume_says_it_answers_a_parked_run_rather_than_merely_reopening_a_sessio // release growing a command and losing another. assert_eq!( COMMANDS.len(), - 32, - "0.25.0 adds exactly one command, `/commit`, on top of the thirty-one \ - 0.24.0 shipped; a thirty-third here means one arrived unrecorded", + 33, + "0.26.0 adds exactly one command, `/effort`, on top of the thirty-two \ + 0.25.0 shipped; a thirty-fourth here means one arrived unrecorded", ); } +/// **F1 — `/effort` is a posture, and a bare `/effort` is a question.** +/// +/// The three words the command understands, plus the two ways of asking rather +/// than setting. `Reasoning::Report` is what a question resolves to and it is what +/// makes the sentence below safe to print: the driver only assigns when +/// [`io_cli::app::reasoning_of`] answers `Some`. +/// +/// Sabotage: resolve an unrecognised word to the nearest level — under which this +/// fails on `/effort hgih`, and it fails by spending a turn's reasoning budget on +/// a typo. +#[test] +fn f1_effort_parses_three_levels_the_absence_and_the_question() { + use io_cli::commands::Reasoning; + let parse = |text: &str| commands::parse(text, &defaults(), &DARK); + + assert_eq!( + parse("effort low"), + Action::Effort(Reasoning::Buy(io_harness::Effort::Low)), + ); + assert_eq!( + parse("effort medium"), + Action::Effort(Reasoning::Buy(io_harness::Effort::Medium)), + ); + assert_eq!( + parse("effort high"), + Action::Effort(Reasoning::Buy(io_harness::Effort::High)), + ); + assert_eq!(parse("effort off"), Action::Effort(Reasoning::Off)); + assert_eq!(parse("effort"), Action::Effort(Reasoning::Report)); + assert_eq!( + parse("effort hgih"), + Action::Effort(Reasoning::Report), + "an unrecognised word reports rather than guessing at a level", + ); +} + +/// **F1 — a question changes nothing, and `off` is a change.** +/// +/// The outer `Option` [`io_cli::app::reasoning_of`] returns is the difference +/// between the two, and it is the difference the driver's assignment turns on. A +/// `Report` that answered `Some(None)` would make every bare `/effort` silently +/// clear the level the operator set — a question with a side effect. +#[test] +fn f1_asking_what_the_effort_is_does_not_change_it() { + use io_cli::app::reasoning_of; + use io_cli::commands::Reasoning; + + assert_eq!(reasoning_of(Reasoning::Report), None); + assert_eq!(reasoning_of(Reasoning::Off), Some(None)); + assert_eq!( + reasoning_of(Reasoning::Buy(io_harness::Effort::High)), + Some(Some(io_harness::Effort::High)), + ); +} + +/// **F1 — the sentence says the state the session is now in.** +/// +/// Setting and reporting share one sentence because what an operator wants read +/// back is where they now are, and the absent case names the fact rather than a +/// level: "no reasoning field" is what goes on the wire, and calling it "off" +/// would suggest a setting between `low` and nothing. +#[test] +fn f1_the_effort_line_names_the_level_now_in_force() { + use io_cli::app::reasoning_said; + use io_cli::commands::Reasoning; + + let set = reasoning_said( + Reasoning::Buy(io_harness::Effort::High), + Some(io_harness::Effort::High), + ); + assert!(set.contains("high"), "{set}"); + assert!( + set.contains("from here"), + "a level set holds for later turns: {set}" + ); + + let asked = reasoning_said(Reasoning::Report, Some(io_harness::Effort::Low)); + assert!(asked.contains("low"), "{asked}"); + assert!( + !asked.contains("from here"), + "a question reports what is already true rather than announcing a change: {asked}", + ); + + let none = reasoning_said(Reasoning::Off, None); + assert!( + none.contains("no reasoning field"), + "the absent case is the absence of the field, not a fourth level: {none}", + ); +} + +/// The group bound was met and paid for by a re-file, not by widening it. +/// +/// `src/commands.rs` pre-committed this answer when `Configure` reached nine and +/// again when `/commit` took `Turn` to ten: re-file what is in the wrong group, do +/// not widen the bound. This release is the one that had to keep it, so the keeping +/// is asserted rather than described — `f13_no_group_is_longer_than_ten` would pass +/// just as well if a later release had moved the bound to eleven. +#[test] +fn f2_effort_is_a_turn_command_and_profile_moved_to_the_session() { + assert_eq!( + commands::group_of("/effort"), + Some(commands::Group::Turn), + "`/effort` decides what the next turn buys, which is what `Turn` means", + ); + assert_eq!( + commands::group_of("/profile"), + Some(commands::Group::Session), + "`/profile` changes the overlay every later turn is built from, which is \ + not the work the turn just finished — it was misfiled, and moving it is \ + what made room rather than widening the bound", + ); + // The bound itself, restated here so this test fails as one story: a release + // that added `/effort` without the re-file would leave `Turn` at eleven. + for (group, names) in commands::GROUPS { + assert!( + names.len() <= 10, + "the {group:?} group holds {}; the bound is ten and this release \ + does not move it", + names.len(), + ); + } +} + #[test] fn an_unknown_command_says_what_does_exist() { let Action::Print(lines) = commands::parse("models", &defaults(), &DARK) else { diff --git a/tests/contract.rs b/tests/contract.rs index 2613b43..462be92 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -284,6 +284,54 @@ fn f2_nothing_configured_is_the_contract_the_session_built_before() { // reads it off the built contract, which is where it can actually be wrong. } +/// **F1 — a session that never says `/effort` sends no reasoning field.** +/// +/// The absent case is not a fourth level. `TaskContract::effort` is an +/// `Option`, and `None` sends the pre-0.31.0 request body byte for byte — +/// `openai_wire.rs:1443` and `anthropic.rs:1529`. So the whole of what must be +/// proven here is that [`io_cli::contract::buying`] leaves the contract *identical* +/// when nothing was asked for, which Debug equality states exactly. +/// +/// Asserted through the same instrument as +/// [`f2_nothing_configured_is_the_contract_the_session_built_before`] and for the +/// same reason: a field set by accident is invisible to a test that only looks at +/// the field it meant to set. +/// +/// Sabotage: give `buying` a default of `Effort::Medium` for the `None` case — +/// under which only this test fails, and it fails by buying reasoning on every +/// turn of every operator who never asked for any. +#[test] +fn f1_no_effort_asked_for_leaves_the_contract_byte_for_byte() { + let root = std::path::PathBuf::from("/nowhere"); + let contract = TaskContract::workspace("a turn", root); + let before = format!("{contract:?}"); + + let after = io_cli::contract::buying(contract, None); + + assert_eq!(format!("{after:?}"), before); + assert_eq!(after.effort, None); +} + +/// **F1 — a level that was asked for is on the contract, and it is the one asked +/// for.** +/// +/// All three levels rather than one, because the mapping is a place a swap is +/// invisible: `Effort` is `Ord`, so `Low` and `High` transposed would still be a +/// valid contract and would still pass a test that only checked `is_some()`. +#[test] +fn f1_the_level_asked_for_is_the_level_the_turn_buys() { + let root = std::path::PathBuf::from("/nowhere"); + for level in [ + io_harness::Effort::Low, + io_harness::Effort::Medium, + io_harness::Effort::High, + ] { + let contract = + io_cli::contract::buying(TaskContract::workspace("a turn", root.clone()), Some(level)); + assert_eq!(contract.effort, Some(level)); + } +} + /// **F1 — the prompt is on the contract itself, so both arms carry it.** /// /// The difference the test above allows, asserted as the one it is: a contract diff --git a/tests/status.rs b/tests/status.rs index 79e4e8a..5e12cd8 100644 --- a/tests/status.rs +++ b/tests/status.rs @@ -1253,6 +1253,59 @@ fn both_renderers(status: &Status) -> [(&'static str, String); 2] { ] } +/// **F1 — the effort level is on both renderers, and absent from both when unset.** +/// +/// Both, because a field added to `Status::fields` alone is green on +/// `Status::line` and nowhere the binary draws: `Status::render` takes the footer +/// on any terminal seven rows or taller. That is 0.12.0's planning field and +/// 0.8.0's spend field, and [`both_renderers`] exists because of them. +/// +/// The absent half is asserted as an absence rather than assumed. A default that +/// rendered `effort medium` would put a field on every operator's status line for +/// a release that changed nothing about their turns. +/// +/// Sabotage: draw the level on `Status::line` only — under which this fails on the +/// footer arm alone, which is exactly the shape of the two defects above. +#[test] +fn f1_the_effort_level_is_on_both_renderers_and_absent_until_it_is_set() { + let mut status = Status::new("a-model"); + + for (which, drawn) in both_renderers(&status) { + assert!( + !drawn.contains("effort"), + "{which} names an effort level in a session that has never set one: {drawn}", + ); + } + + status.effort = Some(io_harness::Effort::High); + for (which, drawn) in both_renderers(&status) { + assert!( + drawn.contains("effort high"), + "{which} does not carry the level every turn is buying: {drawn}", + ); + } +} + +/// **F1 — the level survives the turn it was set on.** +/// +/// `Status::forget_run` clears everything that was true of one run. A standing +/// choice is not one of those, and it is cleared beside them exactly as often as +/// somebody adds a field to that function without asking which kind it is — +/// `policy`, `budgets` and `planning` are all there for the same reason. +#[test] +fn f1_forgetting_a_run_does_not_forget_the_effort_level() { + let mut status = Status::new("a-model"); + status.effort = Some(io_harness::Effort::Low); + + status.forget_run(); + + assert_eq!( + status.effort, + Some(io_harness::Effort::Low), + "a level holds until `/effort` says otherwise, not until a run ends", + ); +} + /// A contract carrying all three budgets, the way an operator's `[run]` table /// leaves one. fn budgeted() -> TaskContract { From b08e9e2c4a28d654a30f85d4d0d1c841409b548c Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:31:50 +0530 Subject: [PATCH 03/10] feat(routing): change models on rules the operator wrote, and say where they do not fire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[app.io-cli.routing]` carries two rules — escalate to a stronger model after a number of consecutive failed gate attempts, downshift to a cheaper one while the run has written little. The decision stays io-harness's: `Routing::model_for` is its own pure function and the only implementation of it, and this crate never re-derives the rules or their precedence. The section is io-cli's own `Deserialize` type rather than `io_harness::Routing` directly, which is the one place it differs from `[app.io-cli.containment]`. `Routing` derives no serde at all and is `#[non_exhaustive]`, so it can neither be deserialized into nor written as a literal; the three builders are the way in and they keep working when the harness grows a fourth field. `require_primary` is not offered. It gates on `Provider::reachable`, a defaulted trait method returning `Ok(true)` that no provider in io-harness 0.69 overrides, so a key for it would be advertised, accepted and permanently inert. And routing does not reach a contained turn. io-harness applies it only in the flat workspace loop; the tree loop takes each agent's model from its own roster entry. So a session with containment configured is told, once, at startup — and a session without it is told nothing, because warning every operator with a routing section about a limitation most of them cannot hit is worse than warning none. Alongside it, two smaller repairs to what a turn says about itself. `[app.io-cli] conversational` is the first way to say that a question which is only a question should still open a run; absent, the decision stays io-harness's own, which is what it has always been. And `TurnResult::kind` is read at last: a turn answered in one completion, with no steps and no tools, has always arrived as silence, because every line this product draws about a turn comes from events such a turn never emits. --- src/app.rs | 29 +++++ src/configure.rs | 19 ++++ src/contract.rs | 32 ++++++ src/lib.rs | 1 + src/main.rs | 32 ++++++ src/routing.rs | 266 ++++++++++++++++++++++++++++++++++++++++++++++ src/settings.rs | 43 ++++++++ tests/contract.rs | 122 +++++++++++++++++++++ tests/docs.rs | 15 +++ tests/routing.rs | 249 +++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 808 insertions(+) create mode 100644 src/routing.rs create mode 100644 tests/routing.rs diff --git a/src/app.rs b/src/app.rs index 81941cd..6dd4783 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2367,6 +2367,35 @@ fn gate_said( .or_else(|| (!last.detail.trim().is_empty()).then(|| last.detail.clone())) } +/// The line a turn that was answered rather than run commits, if it was one. +/// +/// **io-harness has answered questions conversationally for longer than this +/// interface has existed, and this crate has never read the field that says so.** +/// `session.rs:1125-1127` turns classification on whenever the contract carries +/// `Verification::None`, which `TaskContract::workspace` does — so a greeting has +/// always come back after one completion with **no steps row, no gate attempt, no +/// checkpoint, no snapshot and no tool loop** (`run/step.rs:312-320`). What reached +/// the operator was silence, because every line this product draws about a turn is +/// drawn from events a conversational turn does not emit. +/// +/// `None` for a run, which must stay byte-identical to what it was: an ordinary +/// turn already accounts for itself and a second sentence saying it was a run +/// would appear under every turn anybody ever takes. +/// +/// **A kind this build does not know reports as a run.** `TurnKind` is +/// `#[non_exhaustive]` (`session.rs:1434`), and the conservative arm is the one +/// that says nothing: claiming a turn was answered when it may have used tools is +/// the one error here that would mislead about what happened to somebody's files. +#[must_use] +pub fn answered_said(kind: &io_harness::TurnKind) -> Option { + match kind { + io_harness::TurnKind::Reply => Some( + "answered without opening a run — one completion, no steps and no tools".to_string(), + ), + _ => None, + } +} + /// What `/effort` changes the session's level to, or `None` to change nothing. /// /// The outer `Option` is the question and the inner one is the answer, which reads diff --git a/src/configure.rs b/src/configure.rs index af5bd4e..6ed1470 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -178,6 +178,25 @@ pub const CATALOGUE: &[&str] = &[ "app.io-cli.gates.rubric", "app.io-cli.gates.reviewer", "app.io-cli.gates.allow_self_review", + // Whether a question that is only a question opens a run. One key rather than + // a section, and listed because its default is io-harness's rather than + // io-cli's: an operator who wants every prompt to be a run has no other way to + // discover that the answer is currently no. + "app.io-cli.conversational", + // When a run changes model, and to which. Every key of both rules for the + // reason the gate's eight are all here: a threshold without its model is half + // a rule, and an operator meeting one half on this surface would write a + // section that refuses to parse. `require_primary` is deliberately absent — + // see `crate::routing` for the provider method that never answers. + // + // **This surface is also where the containment disclosure is owed.** The rules + // reach the contract on every arm and fire only on the flat one, so an + // operator editing them with `[app.io-cli.containment]` in the same file is + // told here as well as at startup — see `routing::inert_under_containment`. + "app.io-cli.routing.escalate_after.failures", + "app.io-cli.routing.escalate_after.model", + "app.io-cli.routing.downshift_under.bytes", + "app.io-cli.routing.downshift_under.model", ]; /// The `/config` row that re-reads the price catalogue. diff --git a/src/contract.rs b/src/contract.rs index b20a996..288afc7 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -305,6 +305,38 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta } None => contract, }; + // **The operator's own answer, and it outranks the gate's.** The block above + // sets classification on because a criterion would otherwise turn it off; this + // is the only place an operator can say what they actually want, so it is + // applied last and wins. `None` changes nothing at all — not + // `with_conversational_turns(true)`, which would be io-cli deciding for the + // ungated majority a question io-harness already answers for them, and would + // put a field on every contract that was not there before. + let contract = match crate::settings::stored(config) + .0 + .and_then(|s| s.conversational) + { + Some(want) => contract.with_conversational_turns(want), + None => contract, + }; + // **`[app.io-cli.routing]`, in `configured` and not in `session`, because + // `io exec` is where it matters most.** An unattended run is the one that + // cannot notice a model failing its gate four times and reach for a better + // one, and the headless arm uses io-harness's flat loop — the only loop that + // consults the rules at all (`run/step.rs:1097`). + // + // A section that names no rule leaves the contract's routing unset rather than + // putting a default `Routing` on it; `routing::routing` owns that decision and + // says why at length. + let contract = match crate::settings::stored(config) + .0 + .and_then(|s| s.routing) + .as_ref() + .and_then(crate::routing::routing) + { + Some(routing) => contract.with_routing(routing), + None => contract, + }; // `[run] skills` has had its say, and `io exec` reads no other key that can // name one — so for the headless arm this is already the point after every // key. [`session`] calls this again once `[app.io-cli]` has had its own. diff --git a/src/lib.rs b/src/lib.rs index 055dc12..7c16be9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -90,6 +90,7 @@ pub mod repo; pub mod resume; pub mod reviewer; pub mod rewind; +pub mod routing; pub mod servers; pub mod sessions; pub mod settings; diff --git a/src/main.rs b/src/main.rs index 4950eb9..344c83b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -989,6 +989,23 @@ async fn loop_over Result>( // it can carry has already been disclosed by `drive`, which is where that // read is documented as *the read that discloses*. let (settings_in_force, _) = settings::stored(&config); + // **The one thing a routing section cannot say about itself**, said before the + // configuration is handed on so it reads from the settings actually in force + // rather than from a copy of them. io-harness consults the rules only in its + // flat loop (`run/step.rs:1097`), so a contained turn parses them, carries them + // and never fires them. + // + // Conditional on the session actually being contained, and that is F4's whole + // point: warning every operator who wrote a routing section would tell the + // majority — who have no containment at all — about a limitation that does not + // apply to them. `routing::inert_under_containment` owns the condition; this is + // its one caller. + let inert_routing = settings_in_force + .as_ref() + .and_then(|stored| stored.routing.as_ref()) + .and_then(|routing| { + io_cli::routing::inert_under_containment(routing, containment.is_some()) + }); let mut configuration = io_cli::reload::Configuration::new( session.root().to_path_buf(), config.clone(), @@ -998,6 +1015,10 @@ async fn loop_over Result>( let notice = settings::contained_notice(caps, app.theme.glyphs.dash); app.say(Tone::Muted, notice); } + // Beside the containment notice, because it is a qualification of it. + if let Some(notice) = inert_routing { + app.say(Tone::Warning, notice); + } // **The session no longer keeps a clock, because nothing shows one.** The // clock on screen belongs to the turn — it starts at zero when one starts and // stops where it stopped — so the reading a session-long `Instant` gave was @@ -4854,6 +4875,17 @@ async fn turn( ), ); } + // **A turn that was answered rather than run, said out loud at last.** + // io-harness has classified these since before this interface existed + // and io-cli has never read `TurnResult::kind`, so the commonest turn + // there is — a question that is only a question — arrived as silence: + // every line this product draws about a turn comes from events a + // conversational turn does not emit. The sentence is + // `app::answered_said`'s, which also owns the decision that an unknown + // kind reports as a run. + if let Some(said) = io_cli::app::answered_said(&result.kind) { + app.record(Tone::Muted, said); + } } } app.status.elapsed = started.elapsed(); diff --git a/src/routing.rs b/src/routing.rs new file mode 100644 index 0000000..eccc3d1 --- /dev/null +++ b/src/routing.rs @@ -0,0 +1,266 @@ +//! Which model a run asks, once the run has told you something about itself. +//! +//! io-harness has carried the decision since 0.55.0: a `TaskContract` holds one +//! [`io_harness::Routing`], the flat run loop asks it after every step, and the +//! answer replaces the model on the next request. Until this release io-cli +//! supplied none, so every contract it built carried no routing and every run +//! asked one model from the first token to the last. +//! +//! This module is where the operator's answer lives, and it is deliberately thin: +//! [`Settings`] is the `[app.io-cli.routing]` section as written, [`routing`] +//! turns it into the dependency's own value, and the other two functions are what +//! a surface says about it. **No rule is evaluated here.** io-harness owns +//! `Routing::model_for` (`contract.rs:1811`), it is the only code that knows the +//! consecutive-failure count and the byte total, and a second implementation in +//! this crate would be a second answer that drifts from the one the run uses. +//! +//! # Why this section is not deserialized straight into the harness's type +//! +//! `[app.io-cli.containment]` is `Option` in +//! [`crate::settings::CliSettings`] because that type is `Serialize` and +//! `Deserialize` for exactly that purpose. [`io_harness::Routing`] is not: it +//! derives `Debug, Clone, Default, PartialEq, Eq` and nothing else +//! (`contract.rs:1745`), and it is `#[non_exhaustive]`, so this crate can neither +//! deserialize into it nor write a struct literal for it. Both halves of that +//! matter — a `serde` derive added upstream later would still leave the literal +//! refused, because `#[non_exhaustive]` is a promise about fields yet to exist. +//! +//! So io-cli defines its own `Deserialize` shapes below and builds through the +//! three builder methods, which are the supported way in and which keep working +//! when the harness grows a fourth field. +//! +//! # What an operator writes +//! +//! ```toml +//! [app.io-cli.routing.escalate_after] +//! failures = 3 +//! model = "a-stronger-model" +//! +//! [app.io-cli.routing.downshift_under] +//! bytes = 2000 +//! model = "a-cheaper-model" +//! ``` +//! +//! Each rule is a sub-table rather than a pair of flat keys +//! (`escalate_after_failures`, `escalate_after_model`) because a rule is two +//! numbers that only mean anything together: a threshold with no model and a +//! model with no threshold are both half a rule, and a sub-table makes the pair +//! the unit TOML itself enforces. Neither field carries a `#[serde(default)]`, so +//! half a rule is a parse failure the operator hears about with the key named, +//! rather than a threshold silently defaulting to zero — which would escalate on +//! the very first gate attempt of every turn. +//! +//! # `require_primary` is not offered, and that is a decision +//! +//! [`io_harness::Routing`] has a third field: `require_primary`, which refuses to +//! start when the primary provider says it is unreachable. It is not exposed +//! here. It gates on `Provider::reachable`, a **defaulted** trait method whose +//! body is `async { Ok(true) }` (`provider/mod.rs:1745`), and **no provider in +//! io-harness 0.69 overrides it** — the only other mention of the name in that +//! crate is the doc example demonstrating how one *could*. A key for it would +//! therefore be advertised on a surface, accepted from a file, and permanently +//! inert: an operator would set it, believe an unattended overnight job now +//! refuses to start against a dead endpoint, and get exactly the behaviour they +//! had before. That is worse than the setting not existing. It goes in when a +//! provider answers the question. +//! +//! # Routing does not reach a contained turn +//! +//! `apply_routing` has exactly one call site in io-harness: `run/step.rs:1097`, +//! inside the flat workspace loop. The contained loop takes each agent's model +//! from that agent's own `AgentDef` (`run/tree.rs:638`), and the root is entered +//! with its identity passed as `None` (`run.rs:3959`), which every provider reads +//! as "the model you were built with". So for an operator who has configured +//! `[app.io-cli.containment]`, a routing section parses, reaches the contract, +//! and never fires. +//! +//! Nothing in this crate can fix that — the loop that would have to consult the +//! rules is the dependency's — so what this module owes the operator is the +//! disclosure, and [`inert_under_containment`] is it. It is deliberately narrow: +//! see that function for why warning every operator with a routing section would +//! be worse than warning none. + +use serde::{Deserialize, Serialize}; + +/// The `[app.io-cli.routing]` section exactly as an operator wrote it. +/// +/// Two optional rules and nothing else. There is no `enabled` key and no +/// `require_primary` key: a section that names a rule is a section that wants it, +/// and the third field of [`io_harness::Routing`] is inert in io-harness 0.69 for +/// the reasons in the module documentation. +/// +/// Not `deny_unknown_fields`, unlike [`crate::gates::Settings`], and the +/// asymmetry is worth stating. That section's failure mode is a typo landing on +/// the default and silently producing no gate at all, which is why it refuses +/// names it does not know. Here the two names are the *tables*, and a mistyped +/// table is a table nobody reads — the section then names no rule, [`routing`] +/// answers `None`, and the run behaves exactly as it did before the operator +/// touched the file. Wrong, but not a run that reports success on work nobody +/// checked. Inside each table the keys are required, so a typo *there* is a parse +/// error with the key named, which is the loud half already. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Settings { + /// Move up to a stronger model after this many consecutive failed gates. + #[serde(default)] + pub escalate_after: Option, + /// Stay on a cheaper model while the run has written little. + #[serde(default)] + pub downshift_under: Option, +} + +/// The upward rule: `[app.io-cli.routing.escalate_after]`. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Escalation { + /// How many *consecutive* failed gate attempts trigger the change. + /// + /// Consecutive rather than cumulative, and that is io-harness's counting + /// rather than a choice made here (`contract.rs:1748`): a run that fails, + /// recovers, and fails again much later is a run doing hard work, not a run + /// that needs a bigger model. + pub failures: u32, + /// The model asked from then on. + pub model: String, +} + +/// The downward rule: `[app.io-cli.routing.downshift_under]`. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Downshift { + /// The byte total below which the cheaper model is asked. + /// + /// Measured on what the run has already written to disk, not on what it + /// planned to write — again io-harness's definition (`contract.rs:1759`). + pub bytes: u64, + /// The model asked while the run is under that total. + pub model: String, +} + +/// The section as the dependency's own type, ready for the contract, or `None`. +/// +/// **`None` for a section that names neither rule, including a section that is +/// present and empty, and that is not tidiness.** `io_harness::Routing::default()` +/// is a perfectly constructible value whose `model_for` always answers `None`, so +/// putting one on the contract looks harmless. It is not the same thing as +/// leaving the contract's routing unset: it is a value where there was absence, +/// it is what the run records and what a later reader of that run sees, and it +/// says the operator asked for routing when they asked for nothing. An operator +/// who writes `[app.io-cli.routing]` and no rule under it gets the contract they +/// would have had without the line. +/// +/// Built through the builders rather than by literal because +/// [`io_harness::Routing`] is `#[non_exhaustive]` — see the module +/// documentation. The builders are `#[must_use]` and consuming, so the chain +/// below is the whole construction. +pub fn routing(settings: &Settings) -> Option { + let mut routing = io_harness::Routing::new(); + let mut asked = false; + if let Some(escalation) = &settings.escalate_after { + routing = routing.escalate_after(escalation.failures, escalation.model.clone()); + asked = true; + } + if let Some(downshift) = &settings.downshift_under { + routing = routing.downshift_under(downshift.bytes, downshift.model.clone()); + asked = true; + } + asked.then_some(routing) +} + +/// What the rules do, in one sentence, for `/config` and the startup notices. +/// +/// `None` when no rule is named, so a caller writes nothing rather than a line +/// saying nothing happens. +/// +/// Escalation is stated first because it is the rule that overrides the other, +/// and the two sentences an operator needs after that are both io-harness's +/// documented behaviour rather than io-cli's: escalation wins over downshifting +/// when both conditions hold (`contract.rs:1808`, and the ordering of the two +/// checks in `model_for` at `contract.rs:1812`), and it happens once and does not +/// come back down (`contract.rs:1753`) — because a run oscillating between two +/// models mid-flight is a behaviour nobody asked for. Both clauses are stated +/// only when they can bite: a section with no escalation rule cannot escalate, +/// and one with only an escalation rule has nothing for it to win over. +/// +/// ASCII throughout, as [`crate::gates::Refusal`]'s sentences are: this renders +/// on the plain renderer, under `NO_COLOR`, and through the ASCII glyph set, and +/// a notice that arrives as a replacement character is a notice nobody reads. +pub fn describe(settings: &Settings) -> Option { + let mut sentence = String::from("routing is in force: this run asks "); + match (&settings.escalate_after, &settings.downshift_under) { + (None, None) => return None, + (Some(escalation), None) => { + sentence.push_str(&escalation_clause(escalation)); + sentence.push_str(". The change happens once and does not come back down."); + } + (None, Some(downshift)) => { + sentence.push_str(&downshift_clause(downshift)); + sentence.push('.'); + } + (Some(escalation), Some(downshift)) => { + sentence.push_str(&escalation_clause(escalation)); + sentence.push_str(", and "); + sentence.push_str(&downshift_clause(downshift)); + sentence.push_str( + ". Escalating wins over downshifting when both apply, and it happens once and \ + does not come back down.", + ); + } + } + Some(sentence) +} + +/// The upward rule as a clause, with the count read in English. +/// +/// One attempt is singular. A notice that says "after 1 consecutive failed gate +/// attempts" is a notice that reads as unfinished, and the surfaces this appears +/// on are the ones an operator reads once and trusts. +fn escalation_clause(escalation: &Escalation) -> String { + format!( + "{} after {} consecutive failed gate attempt{}", + escalation.model, + escalation.failures, + if escalation.failures == 1 { "" } else { "s" }, + ) +} + +/// The downward rule as a clause. +fn downshift_clause(downshift: &Downshift) -> String { + format!( + "{} while it has written fewer than {} bytes", + downshift.model, downshift.bytes, + ) +} + +/// The one thing a contained operator has to be told, or `None`. +/// +/// Returned only when both halves are true: the session runs contained **and** +/// the section names at least one rule. Either alone is silence. +/// +/// **The narrowness is the whole function, and it has a name.** Returning the +/// sentence whenever a routing section exists would warn every operator who +/// configured routing — the great majority of whom have no +/// `[app.io-cli.containment]` at all, whose runs go through the flat loop, and +/// whose rules therefore work exactly as written — about a limitation that does +/// not apply to them. A caveat attached to a working feature is how an operator +/// learns to stop reading the notices. And the mirror mistake is as bad the other +/// way: a contained session with no rules configured has nothing to disclose, and +/// a line about routing there is a line about a feature the operator never asked +/// for. +/// +/// `contained` is passed in rather than read here, so this stays a pure decision +/// over its arguments and a test needs no configuration on disk — the same reason +/// [`crate::gates::Settings::criterion`] takes its model as an argument. The +/// caller knows whether *this* session is contained, which is not the same +/// question as whether caps are configured: `/contain off` is a real switch, and +/// a turn run with it takes the flat loop and routes normally. The sentence says +/// so, because the operator who reads it has a way out. +pub fn inert_under_containment(settings: &Settings, contained: bool) -> Option { + if !contained || (settings.escalate_after.is_none() && settings.downshift_under.is_none()) { + return None; + } + Some( + "these routing rules will not fire while [app.io-cli.containment] is configured: \ + io-harness applies routing in its flat workspace loop only, and a contained turn takes \ + each agent's model from that agent's own definition. A turn run with /contain off routes \ + normally." + .to_string(), + ) +} diff --git a/src/settings.rs b/src/settings.rs index 8c6f9fa..e015633 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -166,6 +166,38 @@ pub struct CliSettings { /// addition rather than a break. #[serde(default, skip_serializing_if = "Option::is_none")] pub gates: Option, + /// Whether a question that is only a question may be answered without opening + /// a run. + /// + /// **The behaviour is io-harness's and it has been on all along.** + /// `session.rs:1125-1127` reads + /// `contract.conversational.unwrap_or(matches!(contract.verify, + /// Verification::None))`, and `TaskContract::workspace` starts at + /// `Verification::None` — so every ungated operator has had greetings answered + /// in one completion, with no steps row, no gate attempt and no snapshot, since + /// before this interface existed. 0.24.0 turned it on for gated operators too, + /// because attaching a criterion would otherwise have switched it off. + /// + /// What has never existed is a way to say *no*. This key is that, and it is + /// `false`-only in spirit: `true` is what an unset file already does in every + /// case io-cli produces. An operator who wants every prompt to open a run — + /// because a run is what their hooks, their gate or their trace expect — has + /// had no way to ask for one. + /// + /// Absent leaves the decision exactly where it was, which is what + /// `tests/contract.rs`'s field-for-field gate requires. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub conversational: Option, + /// `[app.io-cli.routing]` — when a run should change models, and to which. + /// + /// io-cli's own type rather than `io_harness::Routing`, which is the one place + /// this section differs from `[app.io-cli.containment]`: `Containment` + /// deserializes straight into the harness's type, and `Routing` derives no + /// serde at all and is `#[non_exhaustive]` besides (`contract.rs:1745`). So the + /// shape an operator writes is [`crate::routing::Settings`] and the conversion + /// goes through the harness's three builders. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub routing: Option, } /// Where prices come from, and what the last read was. @@ -536,6 +568,17 @@ pub fn render( // there is no default worth writing: "no gate" is the honest // answer for an operator who has not answered yet. gates: None, + // Left out for the strongest version of the same reason: absent + // is what io-harness already does, and it is what almost every + // operator wants. Writing `conversational = true` into a fresh + // file would state a default as though it were a choice, and + // writing `false` would make a new install open a run to answer + // "hello". + conversational: None, + // Left out for the caps' reason: routing names models, and the + // wizard has asked about exactly one. A rule pointing at a model + // the operator never chose is worse than no rule. + routing: None, }, }, }; diff --git a/tests/contract.rs b/tests/contract.rs index 462be92..062a007 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -284,6 +284,128 @@ fn f2_nothing_configured_is_the_contract_the_session_built_before() { // reads it off the built contract, which is where it can actually be wrong. } +/// **F8 — `conversational = false` makes a greeting a run, and absent changes +/// nothing.** +/// +/// Both files, because an assertion on one of them cannot tell a wired key from a +/// default. io-harness decides this for io-cli today — +/// `contract.conversational.unwrap_or(matches!(contract.verify, +/// Verification::None))` at `session.rs:1125-1127` — so with the key absent the +/// contract must carry `None` and leave that decision where it is, and with the key +/// present it must carry exactly what was written. +/// +/// Sabotage: pass the key's value only when it is `true` — under which this fails +/// on the `false` file, which is the only file anybody would write the key into. +#[test] +fn f8_the_conversational_key_reaches_the_contract_in_both_directions() { + let _guard = env_lock(); + let _home = HomeFixture::new(); + let (answerer, _questions) = io_cli::intent::channel(); + let responder: Arc = Arc::new(answerer); + + let built = |body: &str| { + let (dir, config) = discovered(&[("io.toml", body)]); + let root = dir.path().to_path_buf(); + let contract = session( + "say hello", + root, + &config, + &Capabilities::default(), + responder.clone(), + None, + ); + (dir, contract.conversational) + }; + + let (_empty, absent) = built(""); + assert_eq!( + absent, None, + "with no key, the decision stays io-harness's — it is on for an ungated \ + contract and io-cli must not restate that as a choice of its own", + ); + + let (_off, refused) = built("[app.io-cli]\nconversational = false\n"); + assert_eq!( + refused, + Some(false), + "an operator who wants every prompt to open a run has asked for one", + ); + + let (_on, wanted) = built("[app.io-cli]\nconversational = true\n"); + assert_eq!(wanted, Some(true)); +} + +/// **F3 — a routing section reaches the contract, and no section leaves it alone.** +/// +/// The rules themselves are asserted in `tests/routing.rs` against +/// `io_harness::Routing::model_for`, which is io-harness's own pure function and +/// the only implementation of the decision. What belongs here is the seam: that +/// what the operator wrote arrives on the contract, and that a file which names no +/// rule does not put a default `Routing` where there was nothing. +#[test] +fn f3_a_routing_section_reaches_the_contract_and_an_absent_one_leaves_it_unset() { + let _guard = env_lock(); + let _home = HomeFixture::new(); + let (answerer, _questions) = io_cli::intent::channel(); + let responder: Arc = Arc::new(answerer); + + let built = |body: &str| { + let (dir, config) = discovered(&[("io.toml", body)]); + let root = dir.path().to_path_buf(); + let contract = session( + "bring the docs up to date", + root, + &config, + &Capabilities::default(), + responder.clone(), + None, + ); + (dir, contract.routing) + }; + + let (_none, unset) = built(""); + assert_eq!(unset, None, "a file with no rules asks for no routing"); + + let (_empty, still_unset) = built("[app.io-cli.routing]\n"); + assert_eq!( + still_unset, None, + "a present but empty section names no rule, and a default `Routing` is a \ + value where there was absence", + ); + + let (_both, routed) = built( + "[app.io-cli.routing.escalate_after]\nfailures = 3\nmodel = \"stronger\"\n\ + [app.io-cli.routing.downshift_under]\nbytes = 2000\nmodel = \"cheaper\"\n", + ); + let routed = routed.expect("a section naming both rules routes"); + assert_eq!(routed.escalate_after, Some((3, "stronger".to_string()))); + assert_eq!(routed.downshift_under, Some((2000, "cheaper".to_string()))); +} + +/// **F7 — a turn that was answered is reported as answered, and a run is not.** +/// +/// The `Run` half is the one that must not move: every existing report about an +/// ordinary turn stays byte-identical, so this arm answers `None` and the driver +/// records nothing. +/// +/// Sabotage: report `Reply` for any run whose step count is zero — under which this +/// still passes and F7's live arm fails, because a turn cancelled before its first +/// step also has zero steps and was not an answer. That is why the kind is read +/// rather than inferred. +#[test] +fn f7_only_a_reply_is_reported_as_having_been_answered() { + assert!( + io_cli::app::answered_said(&io_harness::TurnKind::Reply) + .is_some_and(|said| said.contains("without opening a run")), + "a question that was only a question has to say so, or it arrives as silence", + ); + assert_eq!( + io_cli::app::answered_said(&io_harness::TurnKind::Run), + None, + "an ordinary turn already accounts for itself", + ); +} + /// **F1 — a session that never says `/effort` sends no reasoning field.** /// /// The absent case is not a fourth level. `TaskContract::effort` is an diff --git a/tests/docs.rs b/tests/docs.rs index 9abf306..32b7995 100644 --- a/tests/docs.rs +++ b/tests/docs.rs @@ -261,6 +261,21 @@ fn the_readme_documents_every_key_of_the_io_cli_section() { reviewer: Some("a-reviewing-model".into()), allow_self_review: Some(false), }), + conversational: Some(false), + // Written out field by field for `gates`' reason, and with the same + // consequence: `[app.io-cli.routing]`'s two rules are sub-tables whose + // keys are explained in README prose rather than in rows of their own, so + // a key added to `routing::Settings` has to break something to be noticed. + routing: Some(io_cli::routing::Settings { + escalate_after: Some(io_cli::routing::Escalation { + failures: 3, + model: "a-stronger-model".into(), + }), + downshift_under: Some(io_cli::routing::Downshift { + bytes: 2_000, + model: "a-cheaper-model".into(), + }), + }), }; let value = serde_json::to_value(&every).expect("[app.io-cli] serializes"); let keys = value.as_object().expect("a table"); diff --git a/tests/routing.rs b/tests/routing.rs new file mode 100644 index 0000000..d7fd8d9 --- /dev/null +++ b/tests/routing.rs @@ -0,0 +1,249 @@ +//! F3 and F4 — what `[app.io-cli.routing]` puts on the contract, and what a +//! contained session is told about it. +//! +//! F3 is the section reaching an `io_harness::Routing`: the keys an operator +//! types, the two rules they name, and the run's behaviour under them. F4 is the +//! disclosure that those rules do not fire for a contained turn. +//! +//! Two things here are asserted the way the implementation would be wrong rather +//! than the way it is right. +//! +//! **The behaviour is asserted through `io_harness::Routing::model_for` and never +//! re-implemented.** The precedence rule — escalating beats downshifting when +//! both conditions hold — belongs to io-harness, and a test that computed the +//! expected model from io-cli's own reading of the rules would pass against an +//! io-cli that had quietly disagreed with the harness about which model a run +//! asks. Asking the harness's own method is the only assertion that catches that. +//! +//! **The empty cases are asserted separately from the absent ones.** A section +//! that is present and names no rule and a section that was never written must +//! both leave the contract's routing unset; building a `Routing::default()` for +//! the first is the mistake that looks harmless, so it gets its own test. + +use io_cli::routing::{self, Downshift, Escalation, Settings}; + +/// The models the fixtures route between. Names, not real models: nothing here +/// connects to anything, and a name that looks like a real one invites a reader +/// to think it matters which. +const STRONGER: &str = "vendor/stronger-model"; +const CHEAPER: &str = "vendor/cheaper-model"; + +/// A section naming both rules. +fn both() -> Settings { + Settings { + escalate_after: Some(Escalation { + failures: 3, + model: STRONGER.to_string(), + }), + downshift_under: Some(Downshift { + bytes: 2_000, + model: CHEAPER.to_string(), + }), + } +} + +// --------------------------------------------------------------------------- +// F3 — the section becomes a Routing +// --------------------------------------------------------------------------- + +#[test] +fn a_section_naming_both_rules_carries_both_exactly_as_written() { + let routing = routing::routing(&both()).expect("two rules are a routing"); + assert_eq!(routing.escalate_after, Some((3, STRONGER.to_string()))); + assert_eq!(routing.downshift_under, Some((2_000, CHEAPER.to_string()))); +} + +#[test] +fn a_section_that_names_no_rule_puts_no_routing_on_the_contract() { + assert_eq!(routing::routing(&Settings::default()), None); +} + +#[test] +fn a_present_but_empty_section_puts_no_routing_on_the_contract() { + let empty: Settings = toml::from_str("").expect("an empty routing section parses"); + assert_eq!(empty, Settings::default()); + assert_eq!( + routing::routing(&empty), + None, + "an empty Routing is a value where there was none, and it changes the contract" + ); +} + +#[test] +fn a_section_naming_only_escalation_carries_only_escalation() { + let settings = Settings { + downshift_under: None, + ..both() + }; + let routing = routing::routing(&settings).expect("one rule is a routing"); + assert_eq!(routing.escalate_after, Some((3, STRONGER.to_string()))); + assert_eq!(routing.downshift_under, None); +} + +#[test] +fn a_section_naming_only_downshift_carries_only_downshift() { + let settings = Settings { + escalate_after: None, + ..both() + }; + let routing = routing::routing(&settings).expect("one rule is a routing"); + assert_eq!(routing.escalate_after, None); + assert_eq!(routing.downshift_under, Some((2_000, CHEAPER.to_string()))); +} + +// --------------------------------------------------------------------------- +// F3 — what the harness then does with it +// --------------------------------------------------------------------------- + +#[test] +fn under_the_failure_threshold_a_small_run_is_asked_of_the_cheaper_model() { + let routing = routing::routing(&both()).expect("two rules are a routing"); + assert_eq!(routing.model_for(0, 0), Some(CHEAPER)); + assert_eq!(routing.model_for(2, 1_999), Some(CHEAPER)); +} + +#[test] +fn escalating_beats_downshifting_at_and_above_the_threshold() { + let routing = routing::routing(&both()).expect("two rules are a routing"); + // Both conditions hold: three consecutive failures, and fewer bytes written + // than the downshift bound. io-harness answers with the stronger model. + assert_eq!(routing.model_for(3, 0), Some(STRONGER)); + assert_eq!(routing.model_for(9, 1_999), Some(STRONGER)); +} + +#[test] +fn neither_condition_met_leaves_the_requests_model_alone() { + let routing = routing::routing(&both()).expect("two rules are a routing"); + assert_eq!(routing.model_for(2, 2_000), None); + assert_eq!(routing.model_for(0, 10_000), None); +} + +// --------------------------------------------------------------------------- +// F4 — the containment disclosure +// --------------------------------------------------------------------------- + +#[test] +fn a_contained_session_with_rules_is_told_they_will_not_fire() { + let notice = routing::inert_under_containment(&both(), true) + .expect("contained plus rules is the one case that discloses"); + assert!( + notice.contains("[app.io-cli.containment]"), + "the notice must name the section that caused it: {notice}" + ); + assert!( + notice.contains("/contain off"), + "the notice must name the way out: {notice}" + ); +} + +#[test] +fn an_uncontained_session_with_rules_is_told_nothing() { + assert_eq!( + routing::inert_under_containment(&both(), false), + None, + "the majority of operators have no containment, and their rules work" + ); +} + +#[test] +fn a_contained_session_with_no_rules_is_told_nothing() { + assert_eq!( + routing::inert_under_containment(&Settings::default(), true), + None + ); +} + +#[test] +fn an_uncontained_session_with_no_rules_is_told_nothing() { + assert_eq!( + routing::inert_under_containment(&Settings::default(), false), + None + ); +} + +// --------------------------------------------------------------------------- +// F3 — the sentence a surface draws +// --------------------------------------------------------------------------- + +#[test] +fn the_description_states_escalation_before_downshift_and_names_both_rules() { + let sentence = routing::describe(&both()).expect("two rules describe as something"); + let escalation = sentence + .find(STRONGER) + .expect("the stronger model is named"); + let downshift = sentence.find(CHEAPER).expect("the cheaper model is named"); + assert!( + escalation < downshift, + "escalation is the rule that overrides the other and is stated first: {sentence}" + ); + assert!(sentence.contains("3 consecutive failed gate attempts")); + assert!(sentence.contains("2000 bytes")); + assert!( + sentence.contains("wins over downshifting"), + "io-harness's precedence rule has to be in the sentence: {sentence}" + ); + assert!( + sentence.contains("happens once"), + "escalation not coming back down is the other half: {sentence}" + ); +} + +#[test] +fn a_single_failure_threshold_is_described_in_the_singular() { + let settings = Settings { + escalate_after: Some(Escalation { + failures: 1, + model: STRONGER.to_string(), + }), + downshift_under: None, + }; + let sentence = routing::describe(&settings).expect("one rule describes as something"); + assert!( + sentence.contains("1 consecutive failed gate attempt"), + "the count is stated: {sentence}" + ); + assert!( + !sentence.contains("attempts"), + "one attempt is singular: {sentence}" + ); +} + +#[test] +fn a_section_with_no_rules_describes_as_nothing() { + assert_eq!(routing::describe(&Settings::default()), None); +} + +// --------------------------------------------------------------------------- +// F3 — the keys an operator actually types +// --------------------------------------------------------------------------- + +/// The spellings are pinned by a test because they are the operator's interface. +/// A field renamed in `src/routing.rs` compiles, passes every other test in this +/// file, and silently stops reading a file somebody already wrote. +#[test] +fn the_section_deserializes_from_the_toml_an_operator_writes() { + let settings: Settings = toml::from_str( + r#" +[escalate_after] +failures = 3 +model = "vendor/stronger-model" + +[downshift_under] +bytes = 2000 +model = "vendor/cheaper-model" +"#, + ) + .expect("the documented section parses"); + assert_eq!(settings, both()); +} + +#[test] +fn a_rule_missing_half_of_itself_is_refused_rather_than_defaulted() { + // A threshold with no model is half a rule. Defaulted, it would be a rule + // routing to the empty string; refused, the operator hears the key name. + let refused = toml::from_str::("[escalate_after]\nfailures = 3\n"); + assert!( + refused.is_err(), + "half a rule must not deserialize: {refused:?}" + ); +} From da0a3dcae529943663996a23cea4e918d10ea3ff Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:37:00 +0530 Subject: [PATCH 04/10] test: gate the two decisions that live in the driver, and the chain's assembly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of this release's criteria name a sabotage with no site a test could reach. Two are in `src/main.rs`, which nothing under `tests/` links: whether the effort level is read by each turn or taken from the session — the difference between a posture and a one-shot, and the whole of what `/effort` promises — and whether the driver asks `TurnResult::kind` or infers an answered turn from a step count, which would call a turn cancelled before its first step a conversation. Both are gated by reading the driver as text, which is a weak instrument and the only one available; four gates of the same kind already exist for the same reason. The third was in `provider::build`. Every existing test drives `Chain` directly, and a `build` that folded the chain from the fallback specs alone would produce a perfectly working chain of the operator's second choice. `Probe` implements `WithProvider` so the assembly itself can be asserted: which model the chain asks first, and that every link's host reaches the egress policy. All three were confirmed by executing the sabotage each names. --- tests/contract.rs | 59 ++++++++++++++++++++++++++++++++++ tests/provider.rs | 80 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/tests/contract.rs b/tests/contract.rs index 062a007..0416b72 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -284,6 +284,65 @@ fn f2_nothing_configured_is_the_contract_the_session_built_before() { // reads it off the built contract, which is where it can actually be wrong. } +/// **F1 — the effort level is read by each turn, never taken from the session.** +/// +/// A driver-text gate, and it is here because there is no other instrument. +/// Nothing under `tests/` links `src/main.rs`, so the difference between a posture +/// and a one-shot — which is the whole of what `/effort` promises — lives in a +/// binding this suite cannot call. The sibling `fold_next` is deliberately a +/// `std::mem::take` two lines above it, so the wrong shape is not merely +/// imaginable here: it is written out, correctly, immediately adjacent. +/// +/// Weak, and the only one available. Four gates of this kind already exist in this +/// file and in `tests/steer.rs` for the same reason. +/// +/// Sabotage: change the argument to `std::mem::take(&mut effort)` — under which +/// only this test fails, and it fails by making every level last exactly one turn +/// while every other assertion about `/effort` stays green. +#[test] +fn f1_the_driver_reads_the_effort_level_rather_than_taking_it() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver).expect("the driver"); + // Line endings normalised: a Windows checkout has `\r\n`, and slicing on a + // bare `\n` has now shipped a defect in a test twice — 0.19.0 and 0.23.0. + let text = text.replace("\r\n", "\n"); + + assert!( + !text.contains("std::mem::take(&mut effort)"), + "the effort level is a posture: taking it would spend the level on one \ + turn, which is what `fold_next` beside it is supposed to be alone in doing", + ); + assert!( + text.contains("io_cli::contract::buying(contract, effort)"), + "the level reaches the turn through `contract::buying`, which is where \ + the decision is so that it can be asserted at all", + ); +} + +/// **F7 — the driver asks the turn's own kind and never infers it.** +/// +/// The second driver-text gate, and the criterion's named sabotage is exactly what +/// it forbids: a run cancelled before its first step also has zero steps and was +/// not an answer, so a report derived from a step count would call an interrupted +/// turn a conversation. +/// +/// Sabotage: report on `result.outcome`'s step count instead — under which only +/// this test fails, because `app::answered_said` would still be correct and simply +/// never called. +#[test] +fn f7_the_driver_reads_the_turn_kind_rather_than_counting_steps() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver) + .expect("the driver") + .replace("\r\n", "\n"); + + assert!( + text.contains("io_cli::app::answered_said(&result.kind)"), + "whether a turn was answered is a fact io-harness reports on \ + `TurnResult::kind`, not something to infer from what the run did not do", + ); +} + /// **F8 — `conversational = false` makes a greeting a run, and absent changes /// nothing.** /// diff --git a/tests/provider.rs b/tests/provider.rs index 6dd5f66..4c7275f 100644 --- a/tests/provider.rs +++ b/tests/provider.rs @@ -426,6 +426,86 @@ async fn f6_a_bad_credential_on_the_head_does_not_spend_the_link_below_it() { ); } +/// What a caller learns about the chain `provider::build` handed it. +/// +/// The only way to assert `build` itself. Everything above tests [`Chain`] +/// directly, which cannot catch a `build` that assembles the wrong links — and +/// "assembles the wrong links" is precisely F5's named sabotage, since dropping +/// the head still produces a perfectly working chain of the operator's second +/// choice. +struct Probe; + +impl io_cli::provider::WithProvider for Probe { + /// The model the chain will ask first, and every host it may reach. + type Out = (String, usize); + + async fn call( + self, + make: impl Fn(&str) -> Result, + model: String, + ) -> Self::Out { + let provider = make(&model).expect("the chain builds from two valid specs"); + ( + provider.model_hint().unwrap_or_default().to_string(), + provider.endpoints().len(), + ) + } +} + +#[tokio::test] +async fn f5_build_asks_the_operators_first_choice_first() { + // Two vendors rather than two of one, so the links are distinguishable by + // something other than the model name — and the credential is in the spec so + // no test has to touch the process environment to run. + let specs = vec![ + io_harness::ProviderSpec::OpenRouter { + model: "head-model".into(), + api_key: Some("k".into()), + }, + io_harness::ProviderSpec::Anthropic { + model: "tail-model".into(), + api_key: Some("k".into()), + }, + ]; + + let (asked_first, hosts) = io_cli::provider::build(specs, None, Probe) + .await + .expect("a chain of two"); + + assert_eq!( + asked_first, "head-model", + "the head of the chain is the provider the operator wrote first; a chain \ + folded from the tail alone answers every request from their second choice", + ); + assert_eq!( + hosts, 2, + "every link's host reaches the egress policy, or a fall-through is a way \ + to reach a host the policy never saw", + ); +} + +#[tokio::test] +async fn f5_a_model_override_replaces_the_heads_model_and_not_the_tails() { + // `-m/--model` names one model, and a chain has several. Applying it to every + // link would ask a second vendor for a model id only the first one serves. + let specs = vec![ + io_harness::ProviderSpec::OpenRouter { + model: "head-model".into(), + api_key: Some("k".into()), + }, + io_harness::ProviderSpec::Anthropic { + model: "tail-model".into(), + api_key: Some("k".into()), + }, + ]; + + let (asked_first, _) = io_cli::provider::build(specs, Some("chosen".into()), Probe) + .await + .expect("a chain of two"); + + assert_eq!(asked_first, "chosen"); +} + #[test] fn f5_every_link_s_host_is_authorized_and_not_only_the_head_s() { // io-harness's egress policy is deny-by-default and authorizes the provider's From 08fbabd4741fda9b6233f7a76475957c85259fc5 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:46:39 +0530 Subject: [PATCH 05/10] test(dependencies): make the agent-loop gate see the loop this release adds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate refused `src/provider.rs`, and it was right to. It also could not have seen the loop that matters: its three needles were anchored at four spaces, so they caught a loop written at the top level of a function and were blind to one written inside an `impl` — which is where an agent loop would actually be written. The release that introduces this crate's first loop around provider calls is the release that has to fix that, so the needles are unanchored and every loop in a provider-calling file is now seen. Unanchoring immediately refused `src/verify.rs` for three doc comments containing the word "while". A gate that reads prose forbids a file from explaining itself — 0.16.0, 0.19.0, and twice in one wave in 0.25.0 — and the lesson recorded there is that the stripping belongs in the sweep and not only in the exception. It is in the sweep. The chain is exempted by path and held to being a boundary rather than a hole: its loops may iterate nothing but `self.links`, the decision to try another vendor must be io-harness's own `is_retryable`, it still may not read a response, and it must still forward every completion method the trait declares. The count moved rather than being weakened. Subtracting the module's calls one by one is how this held through 0.17.0, and it stops being honest once the file holds a delegating enum and a chain: the subtraction would grow until it cancelled everything, which is a gate going vacuous without going red. So the count is asserted everywhere else — still exactly one, the wizard's handshake — and inside the module every call must be on a decorator's inner provider, a vendor arm or a chain link, counted on squashed text because rustfmt decides where a long call breaks. Both halves were sabotaged: a call on an unnamed receiver, and a predicate of this crate's own instead of the harness's. Each was refused. --- src/provider.rs | 30 ++++++---- tests/dependencies.rs | 135 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 140 insertions(+), 25 deletions(-) diff --git a/src/provider.rs b/src/provider.rs index ff0beb3..41acc92 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -510,13 +510,15 @@ pub async fn build( model_override: Option, with: W, ) -> Result { - let mut makers = Vec::new(); - let mut models = Vec::new(); - for spec in specs { - let (maker, model) = maker_for(spec)?; - makers.push(maker); - models.push(model); - } + // **Iterators and not a `for`, which is a rule rather than a taste.** + // `tests/dependencies.rs` refuses a loop in any file that calls a provider, + // because a loop beside a provider call is the shape of a second agent loop. + // The one loop this module is permitted is the chain's own over its links, and + // that gate now names it by path and holds it to iterating nothing else — so + // turning configuration into closures, which is what this does, has to be + // written without one. + let built: Result, String> = specs.into_iter().map(maker_for).collect(); + let (makers, mut models): (Vec, Vec) = built?.into_iter().unzip(); let Some(head) = models.first().cloned() else { return Err("no provider is configured; run `io setup`".into()); }; @@ -526,12 +528,14 @@ pub async fn build( // leaves every fallback answering as its own entry says it should. let tail: Vec = models.split_off(1); let make = move |name: &str| { - let mut links = Vec::with_capacity(makers.len()); - links.push(makers[0](name)?); - for (maker, model) in makers[1..].iter().zip(tail.iter()) { - links.push(maker(model)?); - } - Chain::of(links).ok_or_else(|| "no provider is configured; run `io setup`".to_string()) + let head = std::iter::once(makers[0](name)?); + let rest: Vec = makers[1..] + .iter() + .zip(tail.iter()) + .map(|(maker, model)| maker(model)) + .collect::>()?; + Chain::of(head.chain(rest).collect()) + .ok_or_else(|| "no provider is configured; run `io setup`".to_string()) }; Ok(with.call(make, head).await) } diff --git a/tests/dependencies.rs b/tests/dependencies.rs index 1e8aaca..a3b7d17 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -359,6 +359,39 @@ fn f7_no_source_file_loops_over_provider_responses() { ); } + // **The 0.26.0 exemption, held to being a boundary.** `src/provider.rs` is the + // one file allowed a loop beside a provider call, because the chain asks each + // configured vendor in turn when the one before it failed. Two properties keep + // that from being an agent loop, and both are asserted rather than intended. + // + // First: every loop in the file iterates the chain's own links. A loop over + // anything else — steps, messages, attempts at the same vendor — is the shape + // this gate exists to refuse, and it would be refused here by name. + let forwarding_code = code_of(&forwarding); + for line in forwarding_code.lines().filter(|line| { + let trimmed = line.trim_start(); + trimmed.starts_with("for ") + || trimmed.starts_with("loop {") + || trimmed.starts_with("while ") + }) { + assert!( + line.contains("self.links.iter()"), + "src/provider.rs loops over something that is not the chain's links: \ + `{}`. The exemption covers asking the next vendor and nothing else.", + line.trim(), + ); + } + + // Second: the decision to try another vendor is io-harness's own predicate. + // A condition written here would be this crate forming an opinion about which + // failures are worth retrying — which is the judgement that turns a fallover + // into a loop with a policy. + assert!( + forwarding.contains("kind.is_retryable()"), + "the chain must fall through on `ProviderErrorKind::is_retryable`, which is \ + what io-harness's own `Fallback` and its own in-run retry both ask", + ); + let mut calls = Vec::new(); for (path, text) in sources() { // The forwards are subtracted rather than the file being skipped, so the @@ -377,22 +410,100 @@ fn f7_no_source_file_loops_over_provider_responses() { // A provider call and a loop in the same file is the shape of a second // agent loop. Asserted per file rather than per function on purpose: it is // coarse, and being coarse is what makes it hard to creep past. - for keyword in ["\n loop {", "\n while ", "\n for "] { - assert!( - !text.contains(keyword), - "{} both calls a provider and contains a top-level `{}` — \ - which is what an agent loop looks like", - path.display(), - keyword.trim(), - ); + // + // **0.26.0 amends this gate rather than relaxing it, and the amendment + // makes it stricter in the place it was weakest.** Until this release the + // three needles were anchored at four spaces, so they saw a loop written + // at the top level of a function and were blind to one written inside an + // `impl` — where an agent loop would actually be written. The release that + // introduced this crate's first loop around provider calls is the release + // that has to notice that, so the needles are now unanchored and every + // loop in a provider-calling file is seen. + // + // `src/provider.rs`'s chain is exempted BY PATH, and the exemption is a + // boundary rather than a hole: the assertions below it require that the + // only thing it loops over is its own list of links, and that the decision + // to try the next one is io-harness's `is_retryable` and not a predicate + // of this crate's. Iterating vendors on a failure the dependency has + // classified is not an agent loop — it never looks at a successful + // response, which the needles above already require. + // + // The shape is 0.7.0's amendment of the spawn ban and 0.17.0's of this + // gate, for the same reason both times. + // + // **Comments are stripped before the sweep looks**, which is the other + // half of unanchoring the needles. `while` and `for` are ordinary English + // words, and the first run of the wider gate refused `src/verify.rs` for + // three doc comments containing the word "while" — a gate that reads prose + // forbids a file from explaining itself, which this repository has now + // paid for in 0.16.0, 0.19.0 and twice in one wave in 0.25.0. The rule + // learned there is that the stripping belongs in the SWEEP and not only in + // the exception, so it is here. + let code = code_of(&text); + if path != delegating { + for keyword in ["loop {", "while ", "for "] { + assert!( + !code.contains(keyword), + "{} both calls a provider and contains a `{}` — \ + which is what an agent loop looks like", + path.display(), + keyword.trim(), + ); + } } } - let total: usize = calls.iter().map(|(_, count)| count).sum(); + // **The count is asserted over every file EXCEPT the delegating one, and that + // is 0.26.0's amendment.** Subtracting the provider module's calls one by one + // was how this held through 0.17.0, and it stopped being honest the moment the + // module gained a chain: `Vendor` delegates across four arms and `Chain` asks + // each link in turn, so the subtraction would have had to grow until it + // cancelled everything the file contains — a gate that goes vacuous without + // going red, which is the failure this suite has recorded twice. + // + // So the file is taken out of the count and held instead to the four + // properties above, each of which is specific and each of which fails loudly: + // its loops iterate only the chain's links, the decision to try another vendor + // is io-harness's own predicate, it never reads a response, and it forwards + // every completion method the trait declares. Those say what "no second agent + // loop" actually means here, where a number no longer can. + let elsewhere: usize = calls + .iter() + .filter(|(path, _)| path != &delegating) + .map(|(_, count)| count) + .sum(); + assert_eq!( + elsewhere, 1, + "outside src/provider.rs, io-cli should call a provider exactly once — the \ + wizard's verification handshake. Found {calls:?}", + ); + + // And the exempted file is still held to naming what it calls. Every call it + // makes is on a link of the chain, on the inner provider of a decorator, or on + // the arm of the vendor enum — never on a provider this module built in order + // to ask it something, which is what the count used to prevent. + // + // Counted on whitespace-squashed text rather than line by line, for the reason + // the forward count above already gives: rustfmt decides where a long call + // breaks, and an assertion about where a newline sits is an assertion about + // formatting. `link.complete_streaming_calls(..)` is split across three lines + // by rustfmt and was refused by the first, line-based version of this check. + let provider_module = code_of(&std::fs::read_to_string(&delegating).expect("the module")); + let squashed_module: String = provider_module + .chars() + .filter(|c| !c.is_whitespace()) + .collect(); + let made = squashed_module.matches(".complete").count(); + let named: usize = ["self.inner.complete", "p.complete", "link.complete"] + .iter() + .map(|receiver| squashed_module.matches(receiver).count()) + .sum(); assert_eq!( - total, 1, - "io-cli should call a provider exactly once, for the wizard's verification \ - handshake. Found {calls:?}", + made, named, + "src/provider.rs makes {made} provider calls and only {named} of them are on \ + a decorator's inner provider, a vendor arm or a chain link — the rest are \ + this module asking a provider something of its own, which is what the count \ + outside this file exists to prevent", ); } From 1808d60e6b6df48cbb5fa308e519bde1f5481f9e Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:50:34 +0530 Subject: [PATCH 06/10] docs(0.26.0): the four surfaces, and the two live arms no fixture can settle The README gains a section on which model a run asks, with the containment disclosure as its own paragraph rather than a footnote: the rules parse, are listed by /config, reach the contract and never fire under containment, and io-cli says so at start only when both are configured. `/effort` and the moved `/profile` are in the commands table and the notes; the provider paragraph now says the chain runs, which failures fall through and which do not. `docs/config.example.toml` gains the routing sub-tables and `conversational`, commented at the density the gates block already uses, and both `tests/docs.rs` and `tests/configure.rs` hold the example and the README to the catalogue. Two live arms, for the two claims a fixture cannot reach. A turn carrying a reasoning level has to complete against a real endpoint, because a field the vendor does not accept is an HTTP 400 rather than a failing comparison. And a real fall-through needs a real transport failure: the head is a `compatible` endpoint pointed at a port nothing is listening on, which spends no credential because the connection never opens, with the real provider underneath it. --- CHANGELOG.md | 70 +++++++++++++- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 152 +++++++++++++++++++++++++++++- docs/config.example.toml | 66 ++++++++++++- tests/live.rs | 195 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 481 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15aea29..3faf36f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,74 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.26.0] - 2026-08-28 + +A turn asks for the model, the vendor and the amount of thinking the work +actually needs. Three of the four things in this release were already sitting in +io-harness, reachable and unasked for; the fourth was a list this interface had +been drawing for five releases without ever running it. + +**`/effort low`, `/effort medium`, `/effort high`, `/effort off`.** How much +reasoning a turn buys, said once and held: the level applies to that turn and to +every turn after it until you change it, it shows on the status line as `effort +high`, and a bare `/effort` reports what is in force and changes nothing. +io-harness owns the translation to each vendor's spelling — `reasoning_effort` on +the OpenAI wire, `reasoning: { effort }` on OpenRouter, a converted thinking +budget on Anthropic — and io-cli names a level and nothing else. `off` is not a +fourth level below `low`: it goes back to sending no reasoning field at all, which +is what every release before this one sent, and the line it commits says that +rather than naming a level. The level is the session's and is written to no file. + +**`/profile` moves from *this turn* to *the session*, and it is a correction +rather than a reshuffle** — the third time after 0.19.0's `/mcp` and `/provider` +and 0.22.0's `/image` and `/copy`. *this turn* means a command acting on the work +the turn just finished; `/profile` changes which configuration overlay every later +turn is built from, which is a property of the session. It is where `/help` lists +it now, which is the only visible trace of the move. That it also made room for +`/effort` is the order the bound was meant to force: the group stood at ten of +ten, and the answer written down in advance was to re-file what is in the wrong +group rather than widen the bound. + +**`[app.io-cli.routing]` changes the model mid-run.** `escalate_after` moves up to +a stronger model after that many consecutive failed verification-gate attempts; +`downshift_under` asks a cheaper one while the run has written fewer than that +many bytes to disk. Escalation happens once and does not come back down, and it +wins over downshifting where both apply — io-harness's rules, and it evaluates +them, so io-cli holds no counter of its own. + +**Routing does not reach a contained turn, and this release says so rather than +letting it be discovered.** io-harness applies routing in its flat workspace loop +only; a turn run under `[app.io-cli.containment]` takes each agent's model from +that agent's own roster entry and never consults the rules. For an operator with +containment configured the section parses, is listed by `/config`, reaches the +contract and never fires. A session with both is told at start, and a session +without containment is told nothing, because a caveat attached to a working +feature is how somebody learns to stop reading the notices. `io exec` uses the +flat loop, so routing works there, and so does a turn taken with `/contain off`. +There is no `require_primary` key: io-harness has the field, it gates on +`Provider::reachable`, which is defaulted to yes and which no provider in +io-harness 0.69 overrides, so a key for it would be permanently inert. + +**The `[[provider]]` chain is finally what runs, and this changes behaviour for a +file that already has more than one entry.** `/provider` has drawn the whole chain +since 0.21.0 while the product only ever asked its head. From this release the +next entry answers when the first fails in a way another vendor might survive — a +transport error, a timeout, a rate limit, a 5xx — with the fall-through committed +to the scrollback and the status line's provider field moving to whoever actually +answered. **A failure that will fail identically everywhere does not fall +through**, a bad API key above all, so a wrong credential on the primary cannot +start spending at the secondary. The predicate is io-harness's own. `--provider` +on `io exec` replaces the whole chain rather than heading it, because a run scoped +to one vendor on the command line must not spend at another. + +**A question that is only a question says so.** io-harness has answered such a +turn in one completion — no steps, no tools, no gate — for longer than this +interface has existed, and io-cli drew every line it draws about a turn from +events that turn does not emit, so what reached you was silence. It now commits +`answered without opening a run`. `conversational = false` in `[app.io-cli]` turns +the classification off so every prompt opens a full run; absent leaves the +behaviour exactly as it was. + ## [0.25.0] - 2026-08-28 The work a turn does ends as something somebody can review. io-harness has @@ -2113,6 +2181,6 @@ client, tool, sandbox, policy engine or session store of its own. - There is no crates.io publish and `cargo install` is not an install path. - No test in this release asserts on wall-clock time. -[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.23.0...HEAD +[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.26.0...HEAD [0.1.1]: https://github.com/initorigin/io-cli/releases/tag/v0.1.1 [0.1.0]: https://github.com/initorigin/io-cli/releases/tag/v0.1.0 diff --git a/Cargo.lock b/Cargo.lock index 12e7027..2fde11f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.25.0" +version = "0.26.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index b407a14..60e5bf1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.25.0" +version = "0.26.0" edition = "2021" # io-harness declares 1.95, set by `libsqlite3-sys` calling the std `cfg_select!` # macro from its build script. Nothing here can be lower than what the harness diff --git a/README.md b/README.md index 717f808..3ab390d 100644 --- a/README.md +++ b/README.md @@ -334,6 +334,52 @@ same session — io re-reads the configuration for every turn, so an edit you ma in your own editor counts too, and a file that stops parsing leaves the last good one in force rather than ending the session. +### Answered without opening a run + +**A prompt that is only a question is answered in one completion**, with no steps, +no tools, no checkpoint and no verification gate. io-harness has classified turns +that way for longer than this interface has existed — a contract carrying no +criterion turns the classification on, and the contract io-cli builds carries none +until you write a gate — so a greeting has always come back from a single call. +What io-cli never did was *say* so. Every line this product draws about a turn is +drawn from events that a turn like this does not emit, so what reached you was +silence, and a fast answer with nothing above it reads as something having gone +wrong. From 0.26.0 such a turn commits one line: `answered without opening a run — +one completion, no steps and no tools`. + +**`conversational` in `[app.io-cli]` is where you overrule it.** `false` opens a +full run for every prompt; absent leaves the behaviour exactly as it is, which is +why there is nothing to write here for the ordinary case. The key earns its place +on the other side of the gate: attaching a criterion is what io-harness reads as +"this turn is not a conversation", so a repository with a gate would otherwise +have every idle question turned into an agent run that then runs the test suite +after each of its steps. io-cli sets classification back on wherever a criterion +was attached, and this key is how you say you wanted the runs. + +### How much reasoning a turn buys + +**`/effort low`, `/effort medium`, `/effort high`, `/effort off`.** The level is a +posture and not a one-shot: it holds for the turn you set it on and for every turn +after it until you change it, it sits on the status line as `effort high`, and a +bare `/effort` reports what is in force and changes nothing. + +What reaches the wire is the vendor's own spelling, and io-harness owns the +translation: `reasoning_effort` on the OpenAI wire, `reasoning: { effort }` on +OpenRouter, and a converted thinking budget on Anthropic — 1024, 4096 and 16384 +tokens for `low`, `medium` and `high`. io-cli names a level and nothing else, +because a per-vendor number chosen here would be a second opinion about somebody +else's request body. + +**`off` is not a fourth level below `low`.** It goes back to sending no reasoning +field at all, which is what every release before 0.26.0 sent, and the line it +commits says that rather than naming a level — calling it "off" on screen would +suggest a setting between `low` and nothing. + +The level is this session's and is written to no file. There is no `[app.io-cli]` +key for it: how much thinking to buy is a thing you change while you work — a +cheap question, then a hard one — and a value on disk would be one more setting +that is in force on a session you have forgotten setting it for. + ## Keys @@ -452,6 +498,7 @@ above a row that ranked there for reasons having nothing to do with it. | `/clear` | start a new conversation; this one stays in /resume | | `/resume` | reopen an earlier session and answer whatever its last run is waiting on | | `/fork` | continue from an earlier turn of this conversation | +| `/profile` | switch to a named profile from the configuration, for this session | | `/setup` | run the first-run wizard again | | `/exit` | leave | @@ -460,9 +507,9 @@ above a row that ranked there for reasons having nothing to do with it. | Command | Does | | --- | --- | | `/model` | change the model the next turn is sent to | +| `/effort` | how much reasoning the next turn buys: low, medium, high, or off | | `/contain` | run turns contained, so the agent can fan out: on, off, or ask | | `/plan` | make turns propose a plan before they work: on, off, or ask | -| `/profile` | switch to a named profile from the configuration, for this session | | `/steer` | send what is queued into the turn that is already running | | `/compact` | fold this conversation into a summary, at the next step | | `/image` | draw an attached image again: /image 1 | @@ -497,6 +544,27 @@ above a row that ranked there for reasons having nothing to do with it. | `/gates` | the check a turn must pass before it is done: a command, a file, or a rubric | | `/import` | bring instructions, MCP servers, skills and a model across from another agent | +`/effort` is new in 0.26.0 and sits under **this turn** beside `/model`, because +the two are the same question asked twice: which model the work goes to, and how +much thinking it is worth buying from it. It is a posture rather than a one-shot — +the level it sets holds for that turn and for every turn after it until you change +it — and a bare `/effort` reports the level in force and changes nothing. +`/effort off` is not a fourth level below `low`: it goes back to sending no +reasoning field at all, which is what every release before this one sent. See [How +much reasoning a turn buys](#how-much-reasoning-a-turn-buys). + +`/profile` sits under **the session** from 0.26.0, and it is a correction rather +than a reshuffle — the third time this sentence has been written, after 0.19.0's +`/mcp` and `/provider` and 0.22.0's `/image` and `/copy`. **this turn** means a +command acting on the work the turn just finished, and `/profile` acts on nothing +that has happened: it changes which configuration overlay every *later* turn is +built from, which is a property of the session. It was filed under **this turn** +because switching one feels like something you do between turns, and where a +command sits is decided by what it acts on rather than by when it is typed. That +it also made room for `/effort` is the order the bound was meant to force: the +group stood at ten of ten, and the answer had been written down in advance — +re-file what is in the wrong group, do not widen the bound. + `/commit` is new in 0.25.0 and sits under **this turn** beside `/copy` and `/copy diff` because it is the third thing you do with work that has just finished: one puts the answer somewhere, one puts the patch somewhere, and this one makes the @@ -1217,6 +1285,65 @@ back with. `io exec` gains exit `6` for this — the agent finished and the work does not hold up. See [Exit status](#exit-status). +## Which model a run asks + +A gate says what "done" means. This says what to do when the model that has to +reach it keeps missing, and what to do when the work turns out not to have needed +the model you started with. `[app.io-cli.routing]` holds two optional rules, each +written as a sub-table because a rule is a threshold and a model that only mean +anything together — a threshold with no model and a model with no threshold are +both half a rule, and a sub-table makes the pair the unit the file itself +enforces: + +```toml +[app.io-cli.routing.escalate_after] +failures = 3 +model = "a-stronger-model" + +[app.io-cli.routing.downshift_under] +bytes = 2000 +model = "a-cheaper-model" +``` + +**`escalate_after` moves up after that many consecutive failed gate attempts** — +the gates above, counted consecutively rather than cumulatively, because a run +that fails, recovers, and fails again much later is a run doing hard work rather +than one that needs a bigger model. **`downshift_under` asks the cheaper model +while the run has written fewer than that many bytes to disk**, measured on what +was actually written rather than on what was planned, so it is a fact about the +run rather than a forecast of it. Neither key defaults: half a rule is a parse +error naming the key you left out, rather than a threshold quietly reading as +zero and escalating on the first gate attempt of every turn. + +Escalation happens **once** and does not come back down, and escalation **wins** +over downshifting where both apply. Both of those are io-harness's rules rather +than io-cli's: it owns the consecutive-failure count, the byte total and the +decision, taken after every step of the run. io-cli evaluates none of it, because +a second implementation here would be a second answer that drifts from the one the +run actually used. + +**Routing does not reach a contained turn, and that is the first thing to know +about it.** io-harness applies routing in its flat workspace loop only; a turn run +under `[app.io-cli.containment]` takes each agent's model from that agent's own +roster entry and never consults the rules. So for an operator who has configured +containment, the section parses, is listed by `/config`, reaches the contract — +and never fires. A session that has both says so at start, and a session with +containment off says nothing, because a caveat attached to a feature that is +working is how an operator learns to stop reading the notices. A turn taken with +`/contain off` routes normally, and `io exec` uses the flat loop, so routing works +there. Nothing in io-cli can close this: the loop that would have to consult the +rules is the dependency's, and what this interface owes you meanwhile is the +disclosure. + +**There is no `require_primary` key**, and its absence is a decision rather than +an omission. io-harness's own `Routing` carries the field, and it gates on +`Provider::reachable` — a defaulted trait method whose body answers yes, and which +no provider in io-harness 0.69 overrides. A key for it would be offered on a +surface, accepted from a file, and permanently inert: you would set it, believe an +unattended overnight run now refuses to start against a dead endpoint, and get +exactly the behaviour you had before. It goes in when a provider answers the +question. + ## Git A gate says the work holds up. This says what becomes of it. @@ -1711,6 +1838,25 @@ Reordering moves an entry with its own comments and its own keys rather than rebuilding the array, because a chain rebuilt from io-cli's model would silently drop whatever io-cli does not model. +**From 0.26.0 the chain is what runs.** Since 0.21.0 that panel has drawn the +whole order while the product only ever asked its head, so a second +`[[provider]]` entry was a row on a screen and nothing else. It answers now: when +the first provider fails in a way another vendor might survive — a transport +error, a timeout, a rate limit, a 5xx — the next entry is asked, the fall-through +is committed to the scrollback as it happens, and the status line's provider field +moves to whoever actually answered. **A failure that will fail identically +everywhere does not fall through**, a bad API key above all, so a wrong credential +on the primary can never start spending at the secondary. That predicate is +io-harness's own — the same one its `Fallback` and its in-run retry ask — and +io-cli holds no opinion here and must not grow one. **This is a behaviour change +for a file that already has more than one entry**: the second one starts being +used. + +`--provider` on `io exec` and `io resume` replaces the whole chain rather than +heading it. Naming a provider on the command line is saying which endpoint this +run uses, and keeping the file's fallbacks underneath it would let a run you +scoped to one vendor spend at another. + **Both of those panels could only list until 0.21.0.** The writers were there and tested and called from nothing, while three places in this documentation said they "add, edit, disable and remove". They genuinely write now, and the word *disable* @@ -1730,7 +1876,7 @@ bundles](#capability-bundles). **`/profile`** switches to a named `[profile.]` for the session, and `--profile ` picks one for a single run without writing anything. -Eight keys live there, and seven tables: +Nine keys live there, and eight tables: | Key | Is | | --- | --- | @@ -1742,12 +1888,14 @@ Eight keys live there, and seven tables: | `max_parallel_reads` | how many read-only tool calls one turn may run at once. Absent, it is io-harness's own 10; `0` is clamped to 1 rather than meaning none. A `TaskContract` field with no io-harness configuration key of its own, which is why it is named here. | | `spawn_background_after_secs` | how long a spawned child may run before it is backgrounded. Absent, a child is waited for however long it takes. | | `detached_spawns` | whether a spawn may detach at all. Absent, it may. `false` buys a trace with every child's whole life in it, which a detached child gives up. | +| `conversational` | whether a prompt that is only a question may be answered in one completion, with no steps and no tools. Absent leaves io-harness's own classification where it is, which is what every release before 0.26.0 did; `false` opens a full run for every prompt. See [Answered without opening a run](#answered-without-opening-a-run). | | `[app.io-cli.keys]` | the session's keys, by action name. See [Moving a key](#moving-a-key). | | `[app.io-cli.containment]` | the caps a fan-out runs under. Absent, a session cannot decompose anything. See [The fleet](#the-fleet). | | `[[app.io-cli.mcp]]` | MCP servers for the turn, in io-harness's own shape. Merged with the top-level `[[mcp]]`, and wins a collision of ids. | | `[[app.io-cli.lsp]]` | language servers for this workspace. Merged with the top-level `[[lsp]]`, and wins a collision of ids. | | `[app.io-cli.browser]` | a browser the agent may drive. Never downloaded — it is one you already have. | | `[app.io-cli.gates]` | what "done" means for this repository: one of `command` (with `expect_exit`), `file` (with `contains`), or `rubric` (with `reviewer`, and `allow_self_review` if the judge may be the model that did the work), plus `retries`, which defaults to 1 and is report-only at 0. Naming none of the three, or more than one, is refused rather than resolved by precedence. See [Verification gates](#verification-gates). | +| `[app.io-cli.routing]` | when a run should change models, and to which: `escalate_after` with `failures` and `model`, `downshift_under` with `bytes` and `model`, each a sub-table and both optional. Absent, a run asks one model from the first token to the last. **The rules do not fire under `[app.io-cli.containment]`**, which the session says at start when both are configured. See [Which model a run asks](#which-model-a-run-asks). | | `[app.io-cli.prices]` | where the rates in `[prices]` came from: `source_url` names a catalogue to read instead of io-harness's default, and `source` and `models` record what the last read was and how many models it priced. The last two are written by a fetch rather than by hand. See [Where a price comes from](#where-a-price-comes-from). | Because the section is unvalidated by design, an unrecognised *value* reads as the diff --git a/docs/config.example.toml b/docs/config.example.toml index 1c3d61b..2a50336 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -388,7 +388,7 @@ net = "deny" # --- io-cli's own section --------------------------------------------------- # -# Eight keys and seven tables. io-harness does not validate any of it, so an +# Nine keys and eight tables. io-harness does not validate any of it, so an # unrecognised VALUE reads as the default rather than stopping a session. A # table io-harness cannot parse at all is different: the session starts on the # defaults and says so, carrying the harness's own message. @@ -425,6 +425,24 @@ diff = "unified" # says which way round it is. plain = false # +# Whether a prompt that is only a question may be answered in one completion. +# +# io-harness has classified turns since long before io-cli asked it to: a contract +# carrying no verification criterion turns classification on, and a question then +# comes back after ONE completion — no steps, no tools, no checkpoint and no gate. +# Absent leaves that where it is, which is what every release before 0.26.0 did, +# so there is nothing to write here for the ordinary case. +# +# false OPENS A FULL RUN FOR EVERY PROMPT, which is what you want in a session +# meant to work rather than to answer. The key earns its place on the other side +# of a gate: attaching a criterion is what io-harness reads as "this turn is not a +# conversation", so a repository with `[app.io-cli.gates]` would otherwise have +# every idle question become an agent run that then runs the gate after each of +# its steps. io-cli sets classification back on wherever it attached a criterion, +# and this key is how you say you wanted the runs after all. +# +# conversational = false +# # How many steps one turn may take. # # DEPRECATED IN 0.14.0 AND REMOVED IN 0.16.0. It existed because the flat turn @@ -687,3 +705,49 @@ fleet = "ctrl+f" # rubric = "every public item changed in this turn has a doc comment saying why" # reviewer = "anthropic/claude-sonnet-4.5" # allow_self_review = false + +# --- which model a run asks (0.26.0) ---------------------------------------- +# +# Two optional rules. Each is a SUB-TABLE rather than a pair of flat keys because +# a rule is a threshold and a model that only mean anything together: a threshold +# with no model and a model with no threshold are both half a rule. Neither key +# defaults, so half a rule is a parse error naming the key you left out, rather +# than a threshold quietly reading as zero — which would escalate on the very +# first gate attempt of every turn. +# +# `escalate_after` moves UP after that many CONSECUTIVE failed gate attempts — +# the gates above. Consecutive rather than cumulative: a run that fails, recovers +# and fails again much later is doing hard work, not asking for a bigger model. +# +# `downshift_under` asks the cheaper model while the run has written fewer than +# that many bytes to disk, measured on what was WRITTEN rather than on what was +# planned — a fact about the run rather than a forecast of it. +# +# ESCALATION HAPPENS ONCE AND DOES NOT COME BACK DOWN, AND IT WINS OVER +# DOWNSHIFTING WHERE BOTH APPLY. Both are io-harness's own rules: it owns the +# failure count, the byte total and the decision, taken after every step. io-cli +# evaluates none of it, because a second implementation here would be a second +# answer that drifts from the one the run actually used. +# +# THESE RULES DO NOT FIRE UNDER [app.io-cli.containment]. io-harness applies +# routing in its flat workspace loop only, and a contained turn takes each agent's +# model from that agent's own definition without ever consulting them. The section +# still parses, still shows in /config and still reaches the contract — it simply +# never fires, and a session configured for both says so at start. `io exec` uses +# the flat loop, and a turn taken with /contain off routes normally. +# +# THERE IS NO `require_primary` KEY, AND THAT IS A DECISION. io-harness's own +# Routing carries the field; it gates on Provider::reachable, a defaulted trait +# method whose body answers yes and which no provider in io-harness 0.69 +# overrides. A key for it would be accepted from this file, believed, and +# permanently inert — you would set it, expect an unattended run to refuse to +# start against a dead endpoint, and get exactly the behaviour you had before. It +# goes in when a provider answers the question. +# +# [app.io-cli.routing.escalate_after] +# failures = 3 +# model = "a-stronger-model" +# +# [app.io-cli.routing.downshift_under] +# bytes = 2000 +# model = "a-cheaper-model" diff --git a/tests/live.rs b/tests/live.rs index e7f6639..e02a5a9 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -3200,3 +3200,198 @@ async fn live_f1_f2_an_asking_posture_refuses_git_and_the_allowance_lifts_it() { `App::note_git` explains and what `/commit allow` lifts; got {refusals:?}", ); } + +/// **F1 live — a reasoning level goes out on a real wire and the endpoint takes +/// it.** +/// +/// The half of F1 no fixture can reach. `contract::buying` puts an `Effort` on the +/// contract and `tests/contract.rs` asserts that; what only a real request can +/// settle is that io-harness translates it into a field the vendor accepts, and +/// that a turn carrying one still finishes. A wrong spelling would be a 400 from +/// the endpoint rather than a failing assertion here, which is exactly why this arm +/// exists. +/// +/// **The assertion is that the turn finishes, and that is the whole of it.** A +/// reasoning field the endpoint does not recognise is an HTTP 400 from the vendor, +/// not a failing comparison — so a turn that completes against a real endpoint +/// with `contract.effort` set is the evidence, and there is nothing further to +/// read back. io-cli's own `context::Request` deliberately does not carry the +/// field: `/context` reports what is in the window, and a reasoning level is not +/// context. +/// +/// Nothing is asserted about the answer's words, for the reason every arm in this +/// file gives: identical code and the same model produce different prose. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f1_a_reasoning_level_reaches_the_wire_and_the_turn_still_finishes() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + std::fs::write(root.join("notes.md"), "one line\n").expect("the fixture file"); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let provider = io_harness::OpenRouter::new(&key, model()); + let policy = workspace_policy(); + + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::buying( + io_cli::contract::session( + "What is in notes.md? Answer in one sentence.", + root.to_path_buf(), + &no_configuration(), + &io_cli::contract::Capabilities::default(), + Arc::new(answerer), + None, + ), + Some(io_harness::Effort::Low), + ); + + assert_eq!( + contract.effort, + Some(io_harness::Effort::Low), + "the contract has to carry the level before the wire can", + ); + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &policy, + &DenyAll, + &io_harness::Ignore, + ) + .await + .expect("a turn carrying a reasoning level still finishes"); + + println!( + "live 0.26.0 F1: outcome {:?} kind {:?}", + result.outcome, result.kind + ); + + assert!( + result + .reply + .as_deref() + .is_some_and(|text| !text.trim().is_empty()), + "a turn carrying a reasoning level produced no answer, which is what a \ + field the endpoint refuses looks like from here: {:?}", + result.outcome, + ); +} + +/// **F5 live — a primary that cannot be reached is fallen through, against a real +/// endpoint underneath.** +/// +/// The other half no fixture settles. `tests/provider.rs` proves the chain's logic +/// with a fake link that fails on demand; what it cannot prove is that a *real* +/// transport failure is classified as retryable by io-harness and therefore +/// reaches the next link at all. The head here is a `Compatible` pointed at a port +/// nothing is listening on, which is the cheapest genuine `ProviderErrorKind:: +/// Transport` available — no credential is spent on it, because the connection +/// never opens. +/// +/// The tail is the real provider, so a pass means an answer that actually came +/// from the second link, and `last_served` names it. That name is also what +/// io-harness turns into `EventKind::FellBackTo` (`run/step.rs:503`), which is the +/// scrollback arm this release makes reachable for the first time. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f5_an_unreachable_primary_falls_through_to_the_provider_underneath() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let policy = workspace_policy(); + + // A base URL on the loopback with nothing behind it. The failure is a refused + // connection, which is `Transport` — retryable, and therefore worth another + // vendor by io-harness's own predicate. + let specs = vec![ + ProviderSpec::Compatible { + model: model(), + preset: None, + base_url: Some("http://127.0.0.1:9/v1".into()), + api_key: Some("unused".into()), + auth: None, + name: None, + reference_prices: false, + }, + ProviderSpec::OpenRouter { + model: model(), + api_key: Some(key), + }, + ]; + + struct Falling<'a> { + session: &'a mut Session, + store: &'a Store, + policy: &'a Policy, + root: std::path::PathBuf, + } + + impl io_cli::provider::WithProvider for Falling<'_> { + type Out = (String, Option); + + async fn call( + self, + make: impl Fn(&str) -> Result, + model: String, + ) -> Self::Out { + let provider = make(&model).expect("the chain builds"); + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::session( + "Reply with the single word: ready.", + self.root.clone(), + &no_configuration(), + &io_cli::contract::Capabilities::default(), + Arc::new(answerer), + None, + ); + let result = self + .session + .turn_bounded_observed( + &contract, + &provider, + self.store, + self.policy, + &DenyAll, + &io_harness::Ignore, + ) + .await + .expect("the second link answers"); + ( + result.reply.clone().unwrap_or_default(), + provider.last_served(), + ) + } + } + + let (reply, served) = io_cli::provider::build( + specs, + None, + Falling { + session: &mut session, + store: &store, + policy: &policy, + root: root.to_path_buf(), + }, + ) + .await + .expect("a chain of two builds"); + + println!("live 0.26.0 F5: served {served:?} reply {reply:?}"); + + assert!( + !reply.trim().is_empty(), + "the turn produced no answer, so nothing fell through to anything", + ); + assert!( + served.is_some(), + "a fall-through happened and `last_served` must name the link that \ + answered — it is what io-harness turns into `EventKind::FellBackTo`", + ); +} From 0066d38b48497c3f1750571332e9dd2abe98953b Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:53:48 +0530 Subject: [PATCH 07/10] fix(routing): give the disclosure its callers, and describe one at all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tracing callers before the review reported found this release about to ship `routing::describe` with none. `tests/routing.rs` covered it thoroughly — three properties, the singular/plural, the empty case — and no test can see that nothing calls a function. That is what 0.20.0 shipped seven of behind 1,077 passing tests, and what this release's own risks section calls this codebase's proven blind spot. The disclosure had one call site where it needed three. An operator meets this state at session start, when they open `/config` to edit the rules, and when they type `/contain on` — and the last is the worst of the three, because they begin uncontained, are told nothing because nothing applied, and then move into the one mode where their rules do not fire without a word. `/config` was also promised the sentence by a comment in `src/configure.rs` and by F4 itself, and never printed it. All three now key on `contained` rather than on `containment.is_some()`. The two are equal at startup and stop being equal the moment `/contain off` is typed, which is precisely when the rules start working again — a warning keyed on the configuration would go on being shown to an operator it no longer applies to. A driver-text gate holds all four call sites, since nothing under `tests/` links the binary. --- src/main.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++--- tests/contract.rs | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 344c83b..fe2dcad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1000,12 +1000,14 @@ async fn loop_over Result>( // majority — who have no containment at all — about a limitation that does not // apply to them. `routing::inert_under_containment` owns the condition; this is // its one caller. + // `contained` rather than `containment.is_some()`, which are equal here and stop + // being equal the moment the operator types `/contain off`. Keying the sentence + // on what the session is actually doing is what lets the same call answer at + // `/config` and after a `/contain` switch. let inert_routing = settings_in_force .as_ref() .and_then(|stored| stored.routing.as_ref()) - .and_then(|routing| { - io_cli::routing::inert_under_containment(routing, containment.is_some()) - }); + .and_then(|routing| io_cli::routing::inert_under_containment(routing, contained)); let mut configuration = io_cli::reload::Configuration::new( session.root().to_path_buf(), config.clone(), @@ -3047,6 +3049,30 @@ async fn loop_over Result>( } } Action::Config(None) => { + // **What the routing rules say, and where they will not fire**, + // said on the surface that edits them. `/config` is where an + // operator meets `[app.io-cli.routing]`, and a section they can + // change here without being told it is inert for their session + // is the defect this release exists partly to avoid. Read from + // the configuration in force rather than the startup snapshot, + // so an edit made this session is what is described. + if let Some(routing) = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.routing) + { + if let Some(said) = io_cli::routing::describe(&routing) { + app.record(Tone::Muted, said); + } + // `contained` and not `containment.is_some()`: `/contain off` + // puts the session back on the flat loop, where the rules do + // fire, so a warning keyed on the configuration would go on + // being shown to an operator it no longer applies to. + if let Some(notice) = + io_cli::routing::inert_under_containment(&routing, contained) + { + app.record(Tone::Warning, notice); + } + } let settings = io_cli::configure::settings(&config); let mut paths: Vec = settings.iter().map(|s| s.path.clone()).collect(); let mut rows = io_cli::configure::rows(&settings); @@ -3369,6 +3395,22 @@ async fn loop_over Result>( contained = true; let notice = settings::contained_notice(caps, app.theme.glyphs.dash); app.record(Tone::Muted, notice); + // **Turning containment on is entering the state the routing + // disclosure warns about**, so it is said here as well as at + // start. An operator who begins uncontained, reads nothing + // about routing because nothing applied, and then types + // `/contain on` has silently moved into the one mode where + // their rules do not fire. + if let Some(said) = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.routing) + .as_ref() + .and_then(|routing| { + io_cli::routing::inert_under_containment(routing, contained) + }) + { + app.record(Tone::Warning, said); + } } (Some(_), Some(false)) => { contained = false; diff --git a/tests/contract.rs b/tests/contract.rs index 0416b72..96f07cb 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -319,6 +319,42 @@ fn f1_the_driver_reads_the_effort_level_rather_than_taking_it() { ); } +/// **F4 — the routing disclosure has callers, and `describe` has one at all.** +/// +/// The third driver-text gate, and it exists because tracing callers found this +/// release about to ship `routing::describe` with none — a public function +/// reachable from no keystroke, no subcommand and no event arm, which is exactly +/// what 0.20.0 shipped seven of behind 1,077 passing tests and what this release's +/// own `risks` calls this codebase's proven blind spot. `tests/routing.rs` covered +/// the function thoroughly and could not see that nothing called it. +/// +/// Three call sites, because there are three moments an operator can be in this +/// state: at session start, when they open the surface that edits the rules, and +/// when they type `/contain on` and enter the state mid-session. The last was +/// missing too. +#[test] +fn f4_the_routing_disclosure_is_reachable_from_the_driver() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver) + .expect("the driver") + .replace("\r\n", "\n"); + + assert!( + text.contains("io_cli::routing::describe("), + "`routing::describe` says what the rules are and nothing calls it — a \ + function no operator can reach is not a surface, it is dead code with \ + tests", + ); + assert_eq!( + text.matches("io_cli::routing::inert_under_containment(") + .count(), + 3, + "the disclosure belongs at session start, on `/config` where the rules are \ + edited, and on `/contain on`, which is the keystroke that puts an operator \ + into the state it warns about", + ); +} + /// **F7 — the driver asks the turn's own kind and never infers it.** /// /// The second driver-text gate, and the criterion's named sabotage is exactly what From 483f68771bf37a0e8613e647dfd8789861e3e262 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 12:56:37 +0530 Subject: [PATCH 08/10] test(narrow): the effort field is a plain word in both glyph sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asserted at two widths, which is 0.25.0's lesson rather than a flourish: at eighty columns a crowded row may not fit a field either way, and a gate that asserts only there is equally true of a status line that never drew the field at all — that release shipped one and found it by sabotage. The wide arm proves it is drawn; the narrow arm proves the row still fits. The absent case is asserted as an absence on both renderers, because a default that drew `effort medium` would put a field on every operator's status line for a release that changed nothing about their turns. Sabotaged by drawing the level with an em dash: the ASCII arm fails, which is what a terminal that cannot render it actually pays for. --- tests/narrow.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/narrow.rs b/tests/narrow.rs index 4b85eb3..e1e0c06 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -1711,6 +1711,74 @@ fn f11_the_branch_field_is_a_plain_name_and_costs_a_stated_number_of_columns() { } } +/// **0.26.0 N2 — the effort level is drawn in both glyph sets and costs no glyph +/// it does not have.** +/// +/// The field is a word and a level: `effort low`, `effort medium`, `effort high`. +/// It carries no marker, no bullet and no arrow, so it is the same characters in +/// both sets — which is the property to assert rather than assume, because every +/// other standing field on this row was at some point a glyph the ASCII set did +/// not have. `Status::line` hardcoded a `•` for four releases for exactly that +/// reason. +/// +/// **Two widths, which is 0.25.0's lesson and not a flourish.** At eighty columns +/// with a crowded row a field may not fit either way, and a gate that asserts only +/// there is equally true of a status line that never drew the field at all — that +/// release shipped one and found it by sabotage. So the wide arm proves it is +/// drawn, and the narrow arm proves the row still fits when it is. +/// +/// Sabotage: draw the level with a glyph — under which the ASCII arm fails on the +/// `is_ascii` assertion, which is the one a monochrome or ASCII terminal actually +/// pays for. +#[test] +fn n2_the_effort_field_is_a_plain_word_in_both_glyph_sets() { + use io_cli::status::Status; + + for theme in themes() { + let set = theme.glyphs.name; + let mut status = Status::new("io/model"); + + // Absent until it is set, asserted on both renderers — a default that + // drew `effort medium` would put a field on every operator's status line + // for a release that changed nothing about their turns. + let quiet = text_of(&[status.line(WIDTH, &theme)]).remove(0); + assert!( + !quiet.contains("effort"), + "{set}: a session that has never said `/effort` is not a session \ + buying zero reasoning: {quiet:?}", + ); + + for level in [ + io_harness::Effort::Low, + io_harness::Effort::Medium, + io_harness::Effort::High, + ] { + status.effort = Some(level); + let drawn = format!("effort {level}"); + assert!( + drawn.is_ascii(), + "{set}: the field needs a glyph it does not have: {drawn:?}", + ); + + // Wide, where the row has room: the field is there and whole. + let wide = text_of(&[status.line(200, &theme)]).remove(0); + assert!( + wide.contains(&drawn), + "{set}: the level every turn is buying is not on the line: {wide:?}", + ); + + // And at eighty, where the row must still fit whatever it decided to + // keep. The field may yield here — it is a narrowing decision, not a + // promise — but the line may never overrun. + let narrow = text_of(&[status.line(WIDTH, &theme)]).remove(0); + assert!( + narrow.chars().count() <= WIDTH as usize, + "{set}: the status line overran eighty columns: {narrow:?}", + ); + } + } +} + /// **0.25.0 F11 — the branch reaches the footer, and yields before the standing /// facts when the row cannot hold it.** /// From f9a2603400af57f36a56021d73b2580ee6862de7 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 13:18:24 +0530 Subject: [PATCH 09/10] fix: the adversarial review's findings, three of them high MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewers, split by area, against a suite that was fully green. Both found the first one independently, which is the strongest signal this gate gives. **A turn refused at the token ceiling was reported as answered.** `TurnKind::Reply` carries a second meaning io-harness documents on the variant itself: a completion that crossed `max_tokens` and was refused rather than served is also a `Reply`, because no run was opened either way. Reading the kind alone told an operator with `[run] max_tokens` set that their refused question had been answered — no answer on screen, no mention of the budget. The guard is io-harness's own, which emits its `Answered` event only for a `Reply` whose outcome is `Finished`. **The resumed run never bought the reasoning the session asked for.** `contract::buying`'s own note counted three `contract::session` callers that build a contract nothing runs; there are five sites, and `resume_pending` drives real completions. `/effort high` applied to every turn except the half of the work an operator came back to finish, while the status line went on saying `effort high`. The gate that should have caught it asked `contains` rather than a count, so one call site satisfied it forever; it counts now. **Typing `/effort high` took the posture off an eighty-column footer.** The level went into the group `row` fits all or nothing, so it displaced the policy and the containment word together — the failure `branch` was moved out of that group for one release ago. It is in the counters now, pushed last, where narrowing takes one field at a time. The startup disclosure used `say`, a one-slot footer notice: it overwrote the containment line it was meant to qualify and died on the first keystroke. Half a routing rule failed to deserialize `[app.io-cli]` **entirely**, which silently reverted the theme, the keys, the ceilings, the capabilities and the verification gate — and `/config` writes one key at a time, so every path through that surface to a routing rule passed through it. Both keys in each rule are optional now and the half rule is refused by name, the shape `gates::Settings::criterion` already uses. The same refusal covers the three values that are writable and disastrous: `failures = 0`, which io-harness reads as "escalate before anything has failed"; `bytes = 0`, which can never be true; and a model written as the empty string. `/effort ` reported instead of refusing, so an operator on `high` trying to spend less was told "every turn asks for high reasoning" and went on paying for it. `/effort OFF` did the same, because the literal match sat in front of a `FromStr` that lowercases for itself. Two tests were the defect rather than the code: the image conjunction asserted `!all` over two links that both took the trait's default of `false`, which every possible implementation satisfies; and the catalogue gate matched a key's last segment, which for two routing keys is `model` and occurs dozens of times in the file it checks. --- CHANGELOG.md | 14 ++- README.md | 12 ++- docs/config.example.toml | 11 ++- src/app.rs | 46 +++++++-- src/commands.rs | 34 +++++-- src/contract.rs | 10 +- src/main.rs | 56 ++++++++++- src/routing.rs | 204 +++++++++++++++++++++++++++++++++++---- src/status.rs | 32 +++--- tests/commands.rs | 56 +++++++++-- tests/configure.rs | 17 ++++ tests/contract.rs | 71 +++++++++++++- tests/docs.rs | 8 +- tests/narrow.rs | 82 ++++++++++++++++ tests/provider.rs | 58 +++++++++-- tests/routing.rs | 179 ++++++++++++++++++++++++++++++---- 16 files changed, 788 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3faf36f..8e526b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,13 +41,23 @@ many bytes to disk. Escalation happens once and does not come back down, and it wins over downshifting where both apply — io-harness's rules, and it evaluates them, so io-cli holds no counter of its own. +**A rule that could only misfire is refused by name.** Half a rule, `failures = 0` +— which io-harness reads as "escalate before anything has failed", pinning every +run to the escalation model from its first request — `bytes = 0`, which can never +be true, and a model written as the empty string are each refused with the key +named, and the run goes unrouted rather than obeying them. The two keys inside +each rule are optional for the same reason: making them required meant a half +rule failed to deserialize `[app.io-cli]` entirely, which silently took the +theme, the keys, the ceilings, the capabilities and the verification gate with +it. + **Routing does not reach a contained turn, and this release says so rather than letting it be discovered.** io-harness applies routing in its flat workspace loop only; a turn run under `[app.io-cli.containment]` takes each agent's model from that agent's own roster entry and never consults the rules. For an operator with containment configured the section parses, is listed by `/config`, reaches the -contract and never fires. A session with both is told at start, and a session -without containment is told nothing, because a caveat attached to a working +contract and never fires. A session with both is told at start, on `/config`, and +when `/contain on` is typed, and a session without containment is told nothing, because a caveat attached to a working feature is how somebody learns to stop reading the notices. `io exec` uses the flat loop, so routing works there, and so does a turn taken with `/contain off`. There is no `require_primary` key: io-harness has the field, it gates on diff --git a/README.md b/README.md index 3ab390d..8473a02 100644 --- a/README.md +++ b/README.md @@ -1327,9 +1327,13 @@ about it.** io-harness applies routing in its flat workspace loop only; a turn r under `[app.io-cli.containment]` takes each agent's model from that agent's own roster entry and never consults the rules. So for an operator who has configured containment, the section parses, is listed by `/config`, reaches the contract — -and never fires. A session that has both says so at start, and a session with -containment off says nothing, because a caveat attached to a feature that is -working is how an operator learns to stop reading the notices. A turn taken with +and never fires. A session that has both is told so at three moments: at start, when `/config` +is opened on the keys themselves, and when `/contain on` is typed — which is the +one that matters most, because that operator began uncontained, was told nothing +because nothing applied, and has just moved into the mode where their rules do +not fire. A session with containment off is told nothing, because a caveat +attached to a feature that is working is how an operator learns to stop reading +the notices. A turn taken with `/contain off` routes normally, and `io exec` uses the flat loop, so routing works there. Nothing in io-cli can close this: the loop that would have to consult the rules is the dependency's, and what this interface owes you meanwhile is the @@ -1895,7 +1899,7 @@ Nine keys live there, and eight tables: | `[[app.io-cli.lsp]]` | language servers for this workspace. Merged with the top-level `[[lsp]]`, and wins a collision of ids. | | `[app.io-cli.browser]` | a browser the agent may drive. Never downloaded — it is one you already have. | | `[app.io-cli.gates]` | what "done" means for this repository: one of `command` (with `expect_exit`), `file` (with `contains`), or `rubric` (with `reviewer`, and `allow_self_review` if the judge may be the model that did the work), plus `retries`, which defaults to 1 and is report-only at 0. Naming none of the three, or more than one, is refused rather than resolved by precedence. See [Verification gates](#verification-gates). | -| `[app.io-cli.routing]` | when a run should change models, and to which: `escalate_after` with `failures` and `model`, `downshift_under` with `bytes` and `model`, each a sub-table and both optional. Absent, a run asks one model from the first token to the last. **The rules do not fire under `[app.io-cli.containment]`**, which the session says at start when both are configured. See [Which model a run asks](#which-model-a-run-asks). | +| `[app.io-cli.routing]` | when a run should change models, and to which: `escalate_after` with `failures` and `model`, `downshift_under` with `bytes` and `model`, each a sub-table and both optional. Absent, a run asks one model from the first token to the last. **The rules do not fire under `[app.io-cli.containment]`**, which the session says at start, on `/config`, and when `/contain on` is typed. A rule that cannot be obeyed — half a rule, a threshold of zero, or an empty model — is refused by name and leaves the run unrouted. See [Which model a run asks](#which-model-a-run-asks). | | `[app.io-cli.prices]` | where the rates in `[prices]` came from: `source_url` names a catalogue to read instead of io-harness's default, and `source` and `models` record what the last read was and how many models it priced. The last two are written by a fetch rather than by hand. See [Where a price comes from](#where-a-price-comes-from). | Because the section is unvalidated by design, an unrecognised *value* reads as the diff --git a/docs/config.example.toml b/docs/config.example.toml index 2a50336..b4197ea 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -733,8 +733,15 @@ fleet = "ctrl+f" # routing in its flat workspace loop only, and a contained turn takes each agent's # model from that agent's own definition without ever consulting them. The section # still parses, still shows in /config and still reaches the contract — it simply -# never fires, and a session configured for both says so at start. `io exec` uses -# the flat loop, and a turn taken with /contain off routes normally. +# never fires, and a session configured for both is told so at start, on /config, +# and when /contain on is typed. `io exec` uses the flat loop, and a turn taken +# with /contain off routes normally. +# +# A RULE THAT COULD ONLY MISFIRE IS REFUSED BY NAME rather than obeyed: half a +# rule, `failures = 0` (which io-harness reads as "escalate before anything has +# failed", pinning every run to the escalation model from its first request), +# `bytes = 0` (which can never be true), and a model written as the empty string. +# The refusal names the key and says the turn is not routed. # # THERE IS NO `require_primary` KEY, AND THAT IS A DECISION. io-harness's own # Routing carries the field; it gates on Provider::reachable, a defaulted trait diff --git a/src/app.rs b/src/app.rs index 6dd4783..7a2217d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2386,10 +2386,28 @@ fn gate_said( /// `#[non_exhaustive]` (`session.rs:1434`), and the conservative arm is the one /// that says nothing: claiming a turn was answered when it may have used tools is /// the one error here that would mislead about what happened to somebody's files. +/// +/// **`TurnKind::Reply` alone is not "answered", and reading it alone shipped a +/// lie.** io-harness documents the second meaning in the variant itself +/// (`session.rs:1440-1443`): a turn whose one completion *crossed the token +/// ceiling and was refused rather than served* is also a `Reply`, because no run +/// was opened either way and "`outcome` is what says how it ended". So an operator +/// with `[run] max_tokens` set, whose question is refused at the budget, would have +/// been told the turn was answered — with no answer anywhere on screen. +/// +/// The guard is io-harness's own, copied deliberately rather than invented: +/// `session.rs:1202` emits `EventKind::Answered` only +/// `if kind == TurnKind::Reply && matches!(result.outcome, RunOutcome::Finished { .. })`, +/// with the comment that announcing it otherwise "would be reporting a message +/// nobody wrote". That is exactly this function's job, so it asks exactly that +/// question. #[must_use] -pub fn answered_said(kind: &io_harness::TurnKind) -> Option { - match kind { - io_harness::TurnKind::Reply => Some( +pub fn answered_said( + kind: &io_harness::TurnKind, + outcome: &io_harness::RunOutcome, +) -> Option { + match (kind, outcome) { + (io_harness::TurnKind::Reply, io_harness::RunOutcome::Finished { .. }) => Some( "answered without opening a run — one completion, no steps and no tools".to_string(), ), _ => None, @@ -2408,11 +2426,14 @@ pub fn answered_said(kind: &io_harness::TurnKind) -> Option { /// sabotaged, and this release's F1 turns on exactly the difference between a level /// that survives the turn and one that does not. #[must_use] -pub fn reasoning_of(said: crate::commands::Reasoning) -> Option> { +pub fn reasoning_of(said: &crate::commands::Reasoning) -> Option> { match said { - crate::commands::Reasoning::Buy(level) => Some(Some(level)), + crate::commands::Reasoning::Buy(level) => Some(Some(*level)), crate::commands::Reasoning::Off => Some(None), - crate::commands::Reasoning::Report => None, + // A question and a rejected word both leave the level exactly where it is. + // They say different things, which is [`reasoning_said`]'s job, not this + // one's. + crate::commands::Reasoning::Report | crate::commands::Reasoning::Unknown(_) => None, } } @@ -2426,12 +2447,23 @@ pub fn reasoning_of(said: crate::commands::Reasoning) -> Option) -> String { +pub fn reasoning_said( + said: &crate::commands::Reasoning, + now: Option, +) -> String { let level = match now { Some(level) => format!("{level} reasoning"), None => "no reasoning field, which is what this product sent before 0.26.0".to_string(), }; match said { + // **A rejected word says so first**, and says the level it did not change + // second. Reporting alone read as an answer: an operator on `high` who + // typed `/effort lwo` to spend less was told "every turn asks for high + // reasoning" and went on paying for it, with the typo invisible. + crate::commands::Reasoning::Unknown(word) => format!( + "`{word}` is not a reasoning level — say low, medium, high, or off. \ + Every turn still asks for {level}" + ), crate::commands::Reasoning::Report => { format!("every turn asks for {level}") } diff --git a/src/commands.rs b/src/commands.rs index d4c4a88..c460680 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -921,7 +921,9 @@ pub fn skills( /// So "off" is a real answer an operator can want and is not the same as "low", /// which buys a thinking budget of 1,024 tokens on Anthropic and sets /// `reasoning_effort` on the OpenAI wire. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +/// Not `Copy` since the rejected word is carried, which is why every reader takes +/// it by reference. +#[derive(Debug, Clone, PartialEq, Eq)] pub enum Reasoning { /// Buy this much, for this turn and every turn after it. Buy(io_harness::Effort), @@ -929,6 +931,15 @@ pub enum Reasoning { Off, /// Say what is in force, and change nothing. Report, + /// A word that is not a level, kept so it can be quoted back. + /// + /// **A fourth variant rather than falling back to [`Self::Report`], and the + /// difference is money.** Collapsing the two made `/effort lwo` — an operator + /// on `high` trying to cut what a turn costs — answer "every turn asks for high + /// reasoning", which reads as a state report rather than as a rejection: the + /// typo is invisible and the expensive level goes on being bought. The only + /// tell was a one-word difference between two sentences nobody was comparing. + Unknown(String), } /// What the driver should do about a slash command. @@ -1787,13 +1798,24 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { // the harness has no word for the absent case; it is an `Option`, not a // fourth variant. "effort" | "reasoning" => match input.split_whitespace().nth(1) { - Some("off" | "none") => Action::Effort(Reasoning::Off), + // Lowercased before the match, because `Effort::FromStr` trims and + // lowercases and this arm sits in front of it: matching `off` + // literally made `/effort OFF` fall through to a parse that fails, + // land on a report, and leave the level exactly where it was — while + // `/effort HIGH` worked, since the harness lowercases for itself. + // Two spellings of the same word behaving differently is the kind of + // asymmetry nobody reports and everybody trips on once. + Some(word) if matches!(word.to_ascii_lowercase().as_str(), "off" | "none") => { + Action::Effort(Reasoning::Off) + } Some(word) => match word.parse::() { Ok(level) => Action::Effort(Reasoning::Buy(level)), - // An unrecognised word reports rather than guesses. Choosing the - // nearest level for `/effort hgih` would spend a turn's reasoning - // budget on a typo. - Err(_) => Action::Effort(Reasoning::Report), + // An unrecognised word is refused by name and never guessed at. + // Choosing the nearest level for `/effort hgih` would spend a + // turn's reasoning budget on a typo; reporting instead — which is + // what this did — hid the typo behind a sentence that reads like + // an answer. + Err(_) => Action::Effort(Reasoning::Unknown(word.to_string())), }, None => Action::Effort(Reasoning::Report), }, diff --git a/src/contract.rs b/src/contract.rs index 288afc7..bf8230a 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -328,11 +328,17 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta // A section that names no rule leaves the contract's routing unset rather than // putting a default `Routing` on it; `routing::routing` owns that decision and // says why at length. + // + // **A refused section leaves the turn unrouted rather than killing it**, which + // is the same answer the gate block above gives an unusable criterion: the + // honest response to a rule that cannot be obeyed is a run with no routing plus + // a notice, never a turn that will not start. `routing::notice` is what puts + // that on screen, and it is the only place this product says why a section that + // is plainly in the operator's file is not doing anything. let contract = match crate::settings::stored(config) .0 .and_then(|s| s.routing) - .as_ref() - .and_then(crate::routing::routing) + .and_then(|routing| crate::routing::routing(&routing).ok().flatten()) { Some(routing) => contract.with_routing(routing), None => contract, diff --git a/src/main.rs b/src/main.rs index fe2dcad..1bb4f93 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1008,6 +1008,10 @@ async fn loop_over Result>( .as_ref() .and_then(|stored| stored.routing.as_ref()) .and_then(|routing| io_cli::routing::inert_under_containment(routing, contained)); + let refused_routing = settings_in_force + .as_ref() + .and_then(|stored| stored.routing.as_ref()) + .and_then(io_cli::routing::notice); let mut configuration = io_cli::reload::Configuration::new( session.root().to_path_buf(), config.clone(), @@ -1017,9 +1021,23 @@ async fn loop_over Result>( let notice = settings::contained_notice(caps, app.theme.glyphs.dash); app.say(Tone::Muted, notice); } - // Beside the containment notice, because it is a qualification of it. + // Beside the containment notice, because it is a qualification of it — and + // **`record` and not `say`**, which is the difference between a sentence that + // stays and one nobody reads. `App::say` writes `status.notice`, a single slot + // that `App::key` clears on the first keystroke, so a `say` here would first + // overwrite the containment notice immediately above (the two conditions are + // true in exactly the same case, since the disclosure only fires when + // contained) and would then be wiped by the operator's first character. This + // file already states that rule where the migration report is committed, and + // the `/config` and `/contain on` sites both got it right. if let Some(notice) = inert_routing { - app.say(Tone::Warning, notice); + app.record(Tone::Warning, notice); + } + // And a section that cannot be obeyed at all, said once at the start for the + // reason every other startup notice is: an operator meets this one without + // having asked a question. + if let Some(why) = refused_routing { + app.record(Tone::Refused, why); } // **The session no longer keeps a clock, because nothing shows one.** The // clock on screen belongs to the turn — it starts at zero when one starts and @@ -1275,6 +1293,7 @@ async fn loop_over Result>( .flatten(), &capabilities, &seen, + effort, run_id, pending, ) @@ -3063,6 +3082,14 @@ async fn loop_over Result>( if let Some(said) = io_cli::routing::describe(&routing) { app.record(Tone::Muted, said); } + // **Why the section is not routing anything, if it is + // not.** The pair of `contract::gate_notice`: this + // surface lists the four routing keys, so a section + // that is plainly in the operator's file and doing + // nothing has to say why here or nowhere. + if let Some(why) = io_cli::routing::notice(&routing) { + app.record(Tone::Refused, why); + } // `contained` and not `containment.is_some()`: `/contain off` // puts the session back on the flat loop, where the rules do // fire, so a warning keyed on the configuration would go on @@ -3353,7 +3380,7 @@ async fn loop_over Result>( // `io_cli::app::reasoning_said` so it can be asserted; this arm // holds the assignment and nothing else. Action::Effort(said) => { - if let Some(level) = io_cli::app::reasoning_of(said) { + if let Some(level) = io_cli::app::reasoning_of(&said) { effort = level; // The status line carries the same value, not a second // opinion about it. Set beside the assignment so the two @@ -3362,7 +3389,13 @@ async fn loop_over Result>( // the line drew the half nothing wrote. app.status.effort = level; } - app.record(Tone::Muted, io_cli::app::reasoning_said(said, effort)); + // A word that is not a level is refused rather than + // reported, so the tone is the refusal's. + let tone = match said { + io_cli::commands::Reasoning::Unknown(_) => Tone::Refused, + _ => Tone::Muted, + }; + app.record(tone, io_cli::app::reasoning_said(&said, effort)); } Action::Contain(want) => match (&containment, want) { // Nothing to switch. Said as the configuration gap it is, @@ -4925,7 +4958,7 @@ async fn turn( // conversational turn does not emit. The sentence is // `app::answered_said`'s, which also owns the decision that an unknown // kind reports as a run. - if let Some(said) = io_cli::app::answered_said(&result.kind) { + if let Some(said) = io_cli::app::answered_said(&result.kind, &result.outcome) { app.record(Tone::Muted, said); } } @@ -6236,6 +6269,10 @@ async fn resume_pending( // through a new door. `tests/context_share.rs` counts the two against each // other so this cannot be forgotten on a path added later. seen: &io_cli::context::Seen, + // What `/effort` last said. A resumed run is a turn like any other and buys the + // same reasoning — see `contract::buying` at the bottom of this function for why + // this parameter exists at all. + effort: Option, run_id: i64, pending: io_cli::resume::Pending, ) -> Result<(), String> { @@ -6323,6 +6360,15 @@ async fn resume_pending( // planning phase back on for the continuation. None, ); + // **The resumed run buys the same reasoning the session is buying**, and this + // is the second of the two sites that run a turn rather than read one. + // `contract::buying`'s own note counts three callers that build a contract + // nothing runs — the startup reading and the two reporting pages — and it + // undercounted: this is a fifth `contract::session` call site and it drives + // real completions. Without it, `/effort high` applied to every turn except + // the half of the work an operator came back to `/resume` and finish, while + // the status line went on saying `effort high`. + let continuing = io_cli::contract::buying(continuing, effort); let (observer, mut events) = bridge::channel(); let canceller = observer.canceller(); let (approver, mut asks) = approval::channel(); diff --git a/src/routing.rs b/src/routing.rs index eccc3d1..82f50b5 100644 --- a/src/routing.rs +++ b/src/routing.rs @@ -45,10 +45,23 @@ //! (`escalate_after_failures`, `escalate_after_model`) because a rule is two //! numbers that only mean anything together: a threshold with no model and a //! model with no threshold are both half a rule, and a sub-table makes the pair -//! the unit TOML itself enforces. Neither field carries a `#[serde(default)]`, so -//! half a rule is a parse failure the operator hears about with the key named, -//! rather than a threshold silently defaulting to zero — which would escalate on -//! the very first gate attempt of every turn. +//! the unit TOML itself enforces. +//! +//! **Both fields are nonetheless optional, and a half rule is refused rather than +//! rejected.** Making them required looked like the stricter choice and was the +//! more damaging one: a required field is a *deserialization* failure, so a +//! threshold with no model did not fail the rule, it failed `CliSettings` — and +//! `crate::settings::stored` then answered `None` for the whole `[app.io-cli]` +//! section, taking the theme, the keys, the ceilings, the capabilities and the +//! **verification gate** with it, silently. [`routing`] refuses the half rule by +//! name instead, which is the shape [`crate::gates::Settings::criterion`] already +//! uses, and [`notice`] is what says so on screen. +//! +//! The same function refuses the three values that are writable and disastrous: +//! `failures = 0`, which io-harness reads as "escalate before anything has +//! failed" and which therefore pins every run to the escalation model from its +//! first request; `bytes = 0`, which can never be true; and a model named as the +//! empty string, which would send every request of the run with no model id. //! //! # `require_primary` is not offered, and that is a decision //! @@ -96,8 +109,10 @@ use serde::{Deserialize, Serialize}; /// table is a table nobody reads — the section then names no rule, [`routing`] /// answers `None`, and the run behaves exactly as it did before the operator /// touched the file. Wrong, but not a run that reports success on work nobody -/// checked. Inside each table the keys are required, so a typo *there* is a parse -/// error with the key named, which is the loud half already. +/// checked. Inside each table a typo is likewise a key nobody reads, and the half +/// rule it leaves behind is refused by [`routing`] by name rather than by serde — +/// see the module documentation for why the stricter-looking choice, making those +/// keys required, was the more damaging one. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct Settings { /// Move up to a stronger model after this many consecutive failed gates. @@ -117,9 +132,23 @@ pub struct Escalation { /// rather than a choice made here (`contract.rs:1748`): a run that fails, /// recovers, and fails again much later is a run doing hard work, not a run /// that needs a bigger model. - pub failures: u32, + /// + /// **Optional, and that is a correction rather than a loosening.** It was + /// required, and a required field is a *deserialization* failure — so half a + /// rule did not fail the rule, it failed `CliSettings` entirely, and + /// `crate::settings::stored` then answered `None` for the whole `[app.io-cli]` + /// section. An operator who wrote a threshold and forgot the model silently + /// lost their theme, their keys, their ceilings, their capabilities and — worst + /// of all — their **verification gate**, because `contract::criterion_for` + /// gives up on the same `None`. A gate that stops gating without saying so is + /// the most expensive failure this crate has. Half a rule now parses and is + /// refused by name at [`routing`], which is the shape + /// [`crate::gates::Settings::criterion`] already uses. + #[serde(default)] + pub failures: Option, /// The model asked from then on. - pub model: String, + #[serde(default)] + pub model: Option, } /// The downward rule: `[app.io-cli.routing.downshift_under]`. @@ -129,9 +158,84 @@ pub struct Downshift { /// /// Measured on what the run has already written to disk, not on what it /// planned to write — again io-harness's definition (`contract.rs:1759`). - pub bytes: u64, + /// + /// Optional for the reason [`Escalation::failures`] gives at length. + #[serde(default)] + pub bytes: Option, /// The model asked while the run is under that total. - pub model: String, + #[serde(default)] + pub model: Option, +} + +/// A routing section that cannot be obeyed, and why. +/// +/// The sibling of [`crate::gates::Refusal`] and it exists for the same reason: a +/// mistake TOML itself cannot express has to be caught where the operator can be +/// told about it, rather than reaching io-harness and changing every request of +/// every run. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Refusal { + /// A threshold with no model, or a model with no threshold. + HalfARule { + /// `escalate_after` or `downshift_under`. + rule: &'static str, + /// The key that is missing. + missing: &'static str, + }, + /// `failures = 0`, which escalates before anything has failed. + /// + /// io-harness compares `consecutive_gate_failures >= failures` + /// (`contract.rs:1813`), so zero is true at the first request of every run — + /// the stronger model is used unconditionally, from the start, and + /// `downshift_under` is never reached because escalation is checked first. An + /// operator writing it means "escalate readily" and gets "never use the model + /// I configured". + EscalatesBeforeAnythingFailed, + /// `bytes = 0`, which can never be true. + /// + /// `written < 0` is false for every run, so the rule is inert. Unlike the + /// above it costs nothing, and it is still refused rather than ignored: a rule + /// that silently does nothing is what the operator will not find when they + /// wonder why the cheap model never appears. + NeverDownshifts, + /// A model named as the empty string. + /// + /// `apply_routing` sets `request.model = Some("")` and every request of the run + /// goes to the vendor with an empty model id. + NoModel { + /// Which rule named it. + rule: &'static str, + }, +} + +impl std::fmt::Display for Refusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::HalfARule { rule, missing } => write!( + f, + "[app.io-cli.routing.{rule}] names no `{missing}`, and a threshold \ + without a model is half a rule — this turn is not routed" + ), + Self::EscalatesBeforeAnythingFailed => write!( + f, + "[app.io-cli.routing.escalate_after] has `failures = 0`, which is \ + true before anything has failed — every run would start on the \ + escalation model and never reach the downshift. This turn is not \ + routed." + ), + Self::NeverDownshifts => write!( + f, + "[app.io-cli.routing.downshift_under] has `bytes = 0`, and a run \ + that has written fewer than zero bytes does not exist — the rule \ + could never fire. This turn is not routed." + ), + Self::NoModel { rule } => write!( + f, + "[app.io-cli.routing.{rule}] names an empty model, which would send \ + every request of the run with no model id — this turn is not routed" + ), + } + } } /// The section as the dependency's own type, ready for the contract, or `None`. @@ -150,18 +254,67 @@ pub struct Downshift { /// [`io_harness::Routing`] is `#[non_exhaustive]` — see the module /// documentation. The builders are `#[must_use]` and consuming, so the chain /// below is the whole construction. -pub fn routing(settings: &Settings) -> Option { +/// **The two thresholds are checked rather than trusted**, because io-harness +/// obeys them literally and each has a value that is both writable and disastrous. +/// `failures = 0` satisfies `consecutive_gate_failures >= 0` at the first request +/// of every run, so the escalation model is used unconditionally and the downshift +/// — checked second — never runs at all. `bytes = 0` can never be true. An empty +/// model sends every request with no model id. None of the three is a shape TOML +/// can refuse, and all three are one keystroke from a plausible file. +pub fn routing(settings: &Settings) -> Result, Refusal> { let mut routing = io_harness::Routing::new(); let mut asked = false; if let Some(escalation) = &settings.escalate_after { - routing = routing.escalate_after(escalation.failures, escalation.model.clone()); + let failures = escalation.failures.ok_or(Refusal::HalfARule { + rule: "escalate_after", + missing: "failures", + })?; + let model = escalation.model.as_deref().ok_or(Refusal::HalfARule { + rule: "escalate_after", + missing: "model", + })?; + if failures == 0 { + return Err(Refusal::EscalatesBeforeAnythingFailed); + } + if model.trim().is_empty() { + return Err(Refusal::NoModel { + rule: "escalate_after", + }); + } + routing = routing.escalate_after(failures, model.to_string()); asked = true; } if let Some(downshift) = &settings.downshift_under { - routing = routing.downshift_under(downshift.bytes, downshift.model.clone()); + let bytes = downshift.bytes.ok_or(Refusal::HalfARule { + rule: "downshift_under", + missing: "bytes", + })?; + let model = downshift.model.as_deref().ok_or(Refusal::HalfARule { + rule: "downshift_under", + missing: "model", + })?; + if bytes == 0 { + return Err(Refusal::NeverDownshifts); + } + if model.trim().is_empty() { + return Err(Refusal::NoModel { + rule: "downshift_under", + }); + } + routing = routing.downshift_under(bytes, model.to_string()); asked = true; } - asked.then_some(routing) + Ok(asked.then_some(routing)) +} + +/// Why the configured routing is not routing this run, if it is not. +/// +/// The sibling of [`crate::contract::gate_notice`], and it exists for the reason +/// that one does: a refusal is not a `Routing`, so the function that answers with a +/// value cannot also be the one that explains itself. +#[must_use] +pub fn notice(settings: &Settings) -> Option { + routing(settings).err().map(|refusal| refusal.to_string()) } /// What the rules do, in one sentence, for `/config` and the startup notices. @@ -183,7 +336,14 @@ pub fn routing(settings: &Settings) -> Option { /// on the plain renderer, under `NO_COLOR`, and through the ASCII glyph set, and /// a notice that arrives as a replacement character is a notice nobody reads. pub fn describe(settings: &Settings) -> Option { - let mut sentence = String::from("routing is in force: this run asks "); + // **"asks for" and not "is in force", because this sentence is printed beside + // one that may contradict it.** `/config` records this and then, for a + // contained session, `inert_under_containment`'s warning that the rules will + // not fire — so a prefix asserting the rules are active made two adjacent + // lines disagree. What this function knows is what the operator *wrote*; + // whether it fires is the other function's subject. `/config` is also typed at + // an idle prompt, so "this run" named nothing. + let mut sentence = String::from("the routing rules ask for "); match (&settings.escalate_after, &settings.downshift_under) { (None, None) => return None, (Some(escalation), None) => { @@ -213,11 +373,16 @@ pub fn describe(settings: &Settings) -> Option { /// attempts" is a notice that reads as unfinished, and the surfaces this appears /// on are the ones an operator reads once and trusts. fn escalation_clause(escalation: &Escalation) -> String { + // A half rule is refused by `routing` before any caller reaches a sentence, so + // the fallbacks below describe a rule that cannot reach the contract. They are + // written rather than unwrapped because a surface that panics on a + // configuration file is worse than one that says "unset". + let failures = escalation.failures.unwrap_or_default(); format!( "{} after {} consecutive failed gate attempt{}", - escalation.model, - escalation.failures, - if escalation.failures == 1 { "" } else { "s" }, + escalation.model.as_deref().unwrap_or("an unnamed model"), + failures, + if failures == 1 { "" } else { "s" }, ) } @@ -225,7 +390,8 @@ fn escalation_clause(escalation: &Escalation) -> String { fn downshift_clause(downshift: &Downshift) -> String { format!( "{} while it has written fewer than {} bytes", - downshift.model, downshift.bytes, + downshift.model.as_deref().unwrap_or("an unnamed model"), + downshift.bytes.unwrap_or_default(), ) } diff --git a/src/status.rs b/src/status.rs index 24975a4..c4beab8 100644 --- a/src/status.rs +++ b/src/status.rs @@ -1484,6 +1484,15 @@ impl Status { // operator ever saw. 0.12.0's planning field, again, and 0.8.0's spend // field before it. counts.extend(self.cost_field()); + // **The effort level is a count-side field, not a standing one, and the + // reason is arithmetic rather than taxonomy** — see the note beside + // `planning` below for what putting it there cost. Pushed last, so + // narrowing takes it before it takes any number: of everything on this row, + // a level the operator has just set and can read back by typing `/effort` + // is the one they can most afford to lose to a narrow terminal. + if let Some(effort) = self.effort { + counts.push(format!("effort {effort}")); + } // **Here as well as on `Status::line`, from the same method, and that is // deliberate rather than tidy.** This is the row the binary draws at an // ordinary prompt — `Status::render` takes the footer on any terminal @@ -1565,17 +1574,18 @@ impl Status { } allowed.push(Span::styled("planning", muted)); } - // Beside `planning` here as well as on `Status::line`, and out of the same - // field, for the reason that paragraph gives at length: the binary draws - // this footer at every ordinary prompt, so a standing choice added to - // `line` alone is a choice no operator ever sees. 0.12.0's planning field - // is the precedent and this release is not going to repeat it. - if let Some(effort) = self.effort { - if !allowed.is_empty() { - allowed.push(Span::styled(separator, muted)); - } - allowed.push(Span::styled(format!("effort {effort}"), muted)); - } + // **The effort level is NOT here, and 0.25.0 already paid for the lesson.** + // The obvious home for a standing choice is beside `planning`, on the + // argument that it describes the circumstances a turn works in rather than + // counting anything — which reads well and is wrong, because of how the two + // groups yield. `row` fits this right-hand group all or nothing, so adding + // `effort high` to it takes the group past what eighty columns can hold and + // the operator loses the posture and the containment word **together**, by + // typing `/effort high`. That is the failure + // `f4_a_full_counts_row_drops_a_counter_and_not_the_planning_phase` exists + // to prevent, and it is why `branch` was moved out of this group one + // release ago. The level goes into `counts` below, where narrowing takes + // one field at a time. // **`counts` and NOT the right-hand group, and this release measured why.** // The branch first went in beside the posture, on the argument that it // describes the circumstances the agent works in rather than counting diff --git a/tests/commands.rs b/tests/commands.rs index 2e8f73d..2fe1683 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -346,8 +346,48 @@ fn f1_effort_parses_three_levels_the_absence_and_the_question() { assert_eq!(parse("effort"), Action::Effort(Reasoning::Report)); assert_eq!( parse("effort hgih"), - Action::Effort(Reasoning::Report), - "an unrecognised word reports rather than guessing at a level", + Action::Effort(Reasoning::Unknown("hgih".into())), + "an unrecognised word is refused by name rather than guessed at — and \ + never quietly reported, which read as an answer and left an operator \ + paying for the level they were trying to leave", + ); + // **Both spellings of `off`, because only one of them used to work.** + // `Effort::FromStr` lowercases for itself, so `/effort HIGH` was fine while + // `/effort OFF` fell past the literal match, failed to parse, and reported — + // leaving the level exactly where it was. Two spellings of one word behaving + // differently is the asymmetry nobody reports and everybody hits once. + assert_eq!(parse("effort OFF"), Action::Effort(Reasoning::Off)); + assert_eq!(parse("effort None"), Action::Effort(Reasoning::Off)); + assert_eq!( + parse("effort HIGH"), + Action::Effort(Reasoning::Buy(io_harness::Effort::High)), + ); +} + +/// **F1 — a word that is not a level says so, and says what is still in force.** +/// +/// Found by both adversarial reviewers. The old sentence for a rejected word was +/// the *report* sentence, so `/effort lwo` — an operator on `high` trying to spend +/// less — answered "every turn asks for high reasoning". That reads as +/// confirmation, the typo is invisible, and the expensive level goes on being +/// bought turn after turn. +#[test] +fn f1_a_word_that_is_not_a_level_is_refused_and_names_itself() { + use io_cli::app::reasoning_said; + use io_cli::commands::Reasoning; + + let said = reasoning_said( + &Reasoning::Unknown("lwo".into()), + Some(io_harness::Effort::High), + ); + assert!(said.contains("lwo"), "the word is quoted back: {said}"); + assert!( + said.contains("not a reasoning level"), + "the operator is told it was refused rather than obeyed: {said}", + ); + assert!( + said.contains("still asks for high"), + "and what is still in force, since nothing changed: {said}", ); } @@ -362,10 +402,10 @@ fn f1_asking_what_the_effort_is_does_not_change_it() { use io_cli::app::reasoning_of; use io_cli::commands::Reasoning; - assert_eq!(reasoning_of(Reasoning::Report), None); - assert_eq!(reasoning_of(Reasoning::Off), Some(None)); + assert_eq!(reasoning_of(&Reasoning::Report), None); + assert_eq!(reasoning_of(&Reasoning::Off), Some(None)); assert_eq!( - reasoning_of(Reasoning::Buy(io_harness::Effort::High)), + reasoning_of(&Reasoning::Buy(io_harness::Effort::High)), Some(Some(io_harness::Effort::High)), ); } @@ -382,7 +422,7 @@ fn f1_the_effort_line_names_the_level_now_in_force() { use io_cli::commands::Reasoning; let set = reasoning_said( - Reasoning::Buy(io_harness::Effort::High), + &Reasoning::Buy(io_harness::Effort::High), Some(io_harness::Effort::High), ); assert!(set.contains("high"), "{set}"); @@ -391,14 +431,14 @@ fn f1_the_effort_line_names_the_level_now_in_force() { "a level set holds for later turns: {set}" ); - let asked = reasoning_said(Reasoning::Report, Some(io_harness::Effort::Low)); + let asked = reasoning_said(&Reasoning::Report, Some(io_harness::Effort::Low)); assert!(asked.contains("low"), "{asked}"); assert!( !asked.contains("from here"), "a question reports what is already true rather than announcing a change: {asked}", ); - let none = reasoning_said(Reasoning::Off, None); + let none = reasoning_said(&Reasoning::Off, None); assert!( none.contains("no reasoning field"), "the absent case is the absence of the field, not a fourth level: {none}", diff --git a/tests/configure.rs b/tests/configure.rs index 2defe6d..16f0b5b 100644 --- a/tests/configure.rs +++ b/tests/configure.rs @@ -231,6 +231,23 @@ fn f2_the_catalogue_is_documented_rather_than_invented() { Either the key is invented or the example is out of date; both are this \ release's problem." ); + + // **And the section it lives under, because the last segment alone stopped + // being a check.** `model` is the last segment of two of 0.26.0's routing + // keys and occurs dozens of times in that file, so a mistyped sub-table — + // `routing.escalate.model` — satisfied the assertion above and every other + // gate. A key nested two or more levels deep names its own table, and the + // table header is what an operator actually has to type. + if let Some(parent) = key.rsplit_once('.').map(|(head, _)| head) { + if parent.matches('.').count() >= 2 { + assert!( + example.contains(&format!("[{parent}]")), + "`{key}` sits under `[{parent}]`, and docs/config.example.toml \ + has no such table. The last segment matching somewhere in the \ + file is not evidence that the path is right." + ); + } + } } } diff --git a/tests/contract.rs b/tests/contract.rs index 96f07cb..f92a1c7 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -373,9 +373,40 @@ fn f7_the_driver_reads_the_turn_kind_rather_than_counting_steps() { .replace("\r\n", "\n"); assert!( - text.contains("io_cli::app::answered_said(&result.kind)"), + text.contains("io_cli::app::answered_said(&result.kind, &result.outcome)"), "whether a turn was answered is a fact io-harness reports on \ - `TurnResult::kind`, not something to infer from what the run did not do", + `TurnResult::kind` — read WITH the outcome, because a `Reply` also covers \ + a completion refused at the token ceiling — and never something to infer \ + from what the run did not do", + ); +} + +/// **F1 — every turn that runs buys the reasoning the session asked for.** +/// +/// `contract::buying`'s own note counted three `contract::session` callers that +/// build a contract nothing runs — and there are five sites, not four: the startup +/// reading, the two reporting pages, the turn, and `resume_pending`, which drives +/// real completions and was missed. The consequence was that `/effort high` +/// applied to every turn except the half of the work an operator came back to +/// `/resume` and finish, while the status line went on saying `effort high`. +/// +/// **Counted, not `contains`.** The first version of this gate asked whether the +/// call appeared at all, which one site satisfies forever — so it could never have +/// caught the site that was missing. That is the vacuous-gate shape this suite has +/// now recorded three times. +#[test] +fn f1_every_turn_that_runs_applies_the_effort_level() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver) + .expect("the driver") + .replace("\r\n", "\n"); + + assert_eq!( + text.matches("io_cli::contract::buying(").count(), + 2, + "two sites drive a turn — the ordinary one and the resumed one — and both \ + buy the level the session asked for. A third `contract::session` caller \ + that runs completions needs one too.", ); } @@ -489,18 +520,50 @@ fn f3_a_routing_section_reaches_the_contract_and_an_absent_one_leaves_it_unset() /// rather than inferred. #[test] fn f7_only_a_reply_is_reported_as_having_been_answered() { + let finished = io_harness::RunOutcome::Finished { steps: 0 }; + assert!( - io_cli::app::answered_said(&io_harness::TurnKind::Reply) + io_cli::app::answered_said(&io_harness::TurnKind::Reply, &finished) .is_some_and(|said| said.contains("without opening a run")), "a question that was only a question has to say so, or it arrives as silence", ); assert_eq!( - io_cli::app::answered_said(&io_harness::TurnKind::Run), + io_cli::app::answered_said(&io_harness::TurnKind::Run, &finished), None, "an ordinary turn already accounts for itself", ); } +/// **F7 — a `Reply` that said nothing is not an answer.** +/// +/// `TurnKind::Reply` carries a second meaning io-harness documents on the variant +/// itself (`session.rs:1440-1443`): a turn whose one completion crossed the token +/// ceiling and was **refused rather than served** is also a `Reply`, because no run +/// was opened either way. Reading the kind alone told an operator with +/// `[run] max_tokens` set that their refused question had been answered — with no +/// answer anywhere on screen and no mention of the budget. +/// +/// Found by both adversarial reviewers independently, which is the strongest +/// signal this gate produces. +/// +/// The guard is io-harness's own, copied rather than invented: `session.rs:1202` +/// emits its `Answered` event only for a `Reply` whose outcome is `Finished`. +/// +/// Sabotage: drop the outcome from the match — under which only this test fails, +/// and it fails by reporting a message nobody wrote. +#[test] +fn f7_a_reply_refused_at_the_token_ceiling_is_not_an_answer() { + assert_eq!( + io_cli::app::answered_said( + &io_harness::TurnKind::Reply, + &io_harness::RunOutcome::CostBudgetExceeded { steps: 0 }, + ), + None, + "a completion refused at the budget was never served, so there is no \ + answer to announce", + ); +} + /// **F1 — a session that never says `/effort` sends no reasoning field.** /// /// The absent case is not a fourth level. `TaskContract::effort` is an diff --git a/tests/docs.rs b/tests/docs.rs index 32b7995..988e5b3 100644 --- a/tests/docs.rs +++ b/tests/docs.rs @@ -268,12 +268,12 @@ fn the_readme_documents_every_key_of_the_io_cli_section() { // a key added to `routing::Settings` has to break something to be noticed. routing: Some(io_cli::routing::Settings { escalate_after: Some(io_cli::routing::Escalation { - failures: 3, - model: "a-stronger-model".into(), + failures: Some(3), + model: Some("a-stronger-model".into()), }), downshift_under: Some(io_cli::routing::Downshift { - bytes: 2_000, - model: "a-cheaper-model".into(), + bytes: Some(2_000), + model: Some("a-cheaper-model".into()), }), }), }; diff --git a/tests/narrow.rs b/tests/narrow.rs index e1e0c06..74392c7 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -1711,6 +1711,88 @@ fn f11_the_branch_field_is_a_plain_name_and_costs_a_stated_number_of_columns() { } } +/// **0.26.0 N2 — setting an effort level does not take the posture off the row.** +/// +/// The sibling of `f11_a_branch_yields_before_the_standing_facts_at_eighty_columns` +/// and it exists because this release very nearly repeated the defect that one was +/// written for. `effort` was first pushed into the footer's right-hand group beside +/// `planning`, on the argument that it is a standing choice rather than a counter — +/// which reads well and is wrong, because `row` fits that group **all or nothing**. +/// Measured on this fixture, `effort high` takes the group past what eighty columns +/// can hold, so typing `/effort high` removed the posture, the containment mode and +/// the planning phase together. `branch` was moved out of that group one release +/// ago for exactly this reason. +/// +/// **The wide arm is not optional.** At eighty columns with this fixture the level +/// may yield, so an assertion made only there is equally true of a status line that +/// never drew the field at all — which is how 0.25.0 shipped a vacuous gate over +/// this very row. So one arm proves the field is drawn where there is room, and the +/// other proves what it does not cost where there is not. +/// +/// Sabotage: push the level into the `allowed` group instead — under which the +/// narrow arm fails, because the posture and the containment word disappear +/// together. +#[test] +fn n2_an_effort_level_yields_before_the_standing_facts_at_eighty_columns() { + use io_cli::status::Status; + + // The branch arm's fixture, and deliberately **without** `planning`: with it + // on, this row's standing group already does not fit at eighty columns, so + // every assertion below would have held whether or not the effort level cost + // anything. The first version of this test had it on and failed for that + // reason rather than for a defect — a gate over a field it could not observe, + // caught this time before it shipped rather than a release later. + let fixture = || { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.policy = Some("read-only".into()); + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + status.steps = Some(6); + status + }; + + for theme in themes() { + let set = theme.glyphs.name; + + // The control. Without it, everything below is equally true of a row that + // never carried the standing facts in the first place. + let bare = text_of(&fixture().footer(WIDTH, &theme)).join("\n"); + for standing in ["read-only", "workspace-write/macos-sandbox-exec"] { + assert!( + bare.contains(standing), + "{set}: the row did not hold `{standing}` to begin with, so nothing \ + below measures anything: {bare:?}", + ); + } + + let mut level = fixture(); + level.effort = Some(io_harness::Effort::High); + + // Wide, where the row holds everything: the level is drawn at all. + let wide = text_of(&level.footer(200, &theme)).join("\n"); + assert!( + wide.contains("effort high"), + "{set}: the level is not on the footer at any width: {wide:?}", + ); + + // Narrow, where it must not cost the standing facts. + let narrow = text_of(&level.footer(WIDTH, &theme)).join("\n"); + for row in narrow.lines() { + assert!( + row.chars().count() <= WIDTH as usize, + "{set}: a footer row overflowed eighty columns: {row:?}", + ); + } + for standing in ["read-only", "workspace-write/macos-sandbox-exec"] { + assert!( + narrow.contains(standing), + "{set}: setting an effort level took `{standing}` off the row — the \ + level belongs in the counters, which narrow one at a time, and not \ + in the group `row` keeps or drops whole: {narrow:?}", + ); + } + } +} + /// **0.26.0 N2 — the effort level is drawn in both glyph sets and costs no glyph /// it does not have.** /// diff --git a/tests/provider.rs b/tests/provider.rs index 4c7275f..40d94ab 100644 --- a/tests/provider.rs +++ b/tests/provider.rs @@ -268,16 +268,33 @@ struct Fake { label: String, fail: Option, calls: Arc, + /// Whether this link would take an image. + /// + /// Settable because the trait's default is `false`, and a fixture where every + /// link answers `false` cannot tell a conjunction from a disjunction or from a + /// head-only read — the first version of the image test asserted `!all` over + /// two defaulted links and would have passed against any of the three. + images: bool, } /// A counter to read afterwards, and the link that increments it. fn link(label: &str, fail: Option) -> (Fake, Arc) { + accepting(label, fail, true) +} + +/// A link that says whether it would take an image. +fn accepting( + label: &str, + fail: Option, + images: bool, +) -> (Fake, Arc) { let calls = Arc::new(AtomicUsize::new(0)); ( Fake { label: label.into(), fail, calls: Arc::clone(&calls), + images, }, calls, ) @@ -314,6 +331,10 @@ impl Provider for Fake { fn endpoints(&self) -> Vec<&str> { vec![&self.label] } + + fn accepts_images(&self) -> bool { + self.images + } } fn asking() -> CompletionRequest { @@ -518,14 +539,35 @@ fn f5_every_link_s_host_is_authorized_and_not_only_the_head_s() { assert_eq!(chain.endpoints(), vec!["first", "second"]); } +/// **The conjunction, asserted as one rather than read off a default.** +/// +/// io-harness's own rule for the same question, and the reason is that the +/// fall-through is the one call where it matters: reporting the head's answer +/// would let an image reach a link that cannot read it on exactly the call that +/// went wrong. +/// +/// The first version of this test built two links that both took the trait's +/// default of `false` and asserted `!accepts_images()`. Every possible +/// implementation passes that — `all`, `any`, and a head-only read alike — so it +/// was a gate over nothing, which the adversarial review caught. The pair below is +/// what makes it a gate: one mixed chain and one where every link agrees. #[test] fn f5_an_image_is_refused_unless_every_link_accepts_one() { - // The conjunction is io-harness's own rule for the same question, and the - // reason is that the fall-through is the one call where it matters. - let (first, _) = link("first", None); - let (second, _) = link("second", None); - let chain = Chain::of(vec![first, second]).expect("a chain"); - // `Fake` takes the trait's default, which is `false`, so the conjunction is - // asserted rather than a default being read back. - assert!(!chain.accepts_images()); + let (yes, _) = accepting("first", None, true); + let (no, _) = accepting("second", None, false); + let mixed = Chain::of(vec![yes, no]).expect("a chain"); + assert!( + !mixed.accepts_images(), + "one link that cannot read an image is enough to refuse it before the run \ + rather than midway through a fall-through", + ); + + let (one, _) = accepting("first", None, true); + let (two, _) = accepting("second", None, true); + let agreed = Chain::of(vec![one, two]).expect("a chain"); + assert!( + agreed.accepts_images(), + "a chain whose links all accept images accepts them — without this arm the \ + assertion above is satisfied by a function that always answers false", + ); } diff --git a/tests/routing.rs b/tests/routing.rs index d7fd8d9..8fe3822 100644 --- a/tests/routing.rs +++ b/tests/routing.rs @@ -32,12 +32,12 @@ const CHEAPER: &str = "vendor/cheaper-model"; fn both() -> Settings { Settings { escalate_after: Some(Escalation { - failures: 3, - model: STRONGER.to_string(), + failures: Some(3), + model: Some(STRONGER.to_string()), }), downshift_under: Some(Downshift { - bytes: 2_000, - model: CHEAPER.to_string(), + bytes: Some(2_000), + model: Some(CHEAPER.to_string()), }), } } @@ -48,14 +48,16 @@ fn both() -> Settings { #[test] fn a_section_naming_both_rules_carries_both_exactly_as_written() { - let routing = routing::routing(&both()).expect("two rules are a routing"); + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); assert_eq!(routing.escalate_after, Some((3, STRONGER.to_string()))); assert_eq!(routing.downshift_under, Some((2_000, CHEAPER.to_string()))); } #[test] fn a_section_that_names_no_rule_puts_no_routing_on_the_contract() { - assert_eq!(routing::routing(&Settings::default()), None); + assert_eq!(routing::routing(&Settings::default()), Ok(None)); } #[test] @@ -64,7 +66,7 @@ fn a_present_but_empty_section_puts_no_routing_on_the_contract() { assert_eq!(empty, Settings::default()); assert_eq!( routing::routing(&empty), - None, + Ok(None), "an empty Routing is a value where there was none, and it changes the contract" ); } @@ -75,7 +77,9 @@ fn a_section_naming_only_escalation_carries_only_escalation() { downshift_under: None, ..both() }; - let routing = routing::routing(&settings).expect("one rule is a routing"); + let routing = routing::routing(&settings) + .expect("one rule is obeyable") + .expect("one rule is a routing"); assert_eq!(routing.escalate_after, Some((3, STRONGER.to_string()))); assert_eq!(routing.downshift_under, None); } @@ -86,7 +90,9 @@ fn a_section_naming_only_downshift_carries_only_downshift() { escalate_after: None, ..both() }; - let routing = routing::routing(&settings).expect("one rule is a routing"); + let routing = routing::routing(&settings) + .expect("one rule is obeyable") + .expect("one rule is a routing"); assert_eq!(routing.escalate_after, None); assert_eq!(routing.downshift_under, Some((2_000, CHEAPER.to_string()))); } @@ -97,14 +103,18 @@ fn a_section_naming_only_downshift_carries_only_downshift() { #[test] fn under_the_failure_threshold_a_small_run_is_asked_of_the_cheaper_model() { - let routing = routing::routing(&both()).expect("two rules are a routing"); + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); assert_eq!(routing.model_for(0, 0), Some(CHEAPER)); assert_eq!(routing.model_for(2, 1_999), Some(CHEAPER)); } #[test] fn escalating_beats_downshifting_at_and_above_the_threshold() { - let routing = routing::routing(&both()).expect("two rules are a routing"); + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); // Both conditions hold: three consecutive failures, and fewer bytes written // than the downshift bound. io-harness answers with the stronger model. assert_eq!(routing.model_for(3, 0), Some(STRONGER)); @@ -113,7 +123,9 @@ fn escalating_beats_downshifting_at_and_above_the_threshold() { #[test] fn neither_condition_met_leaves_the_requests_model_alone() { - let routing = routing::routing(&both()).expect("two rules are a routing"); + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); assert_eq!(routing.model_for(2, 2_000), None); assert_eq!(routing.model_for(0, 10_000), None); } @@ -192,8 +204,8 @@ fn the_description_states_escalation_before_downshift_and_names_both_rules() { fn a_single_failure_threshold_is_described_in_the_singular() { let settings = Settings { escalate_after: Some(Escalation { - failures: 1, - model: STRONGER.to_string(), + failures: Some(1), + model: Some(STRONGER.to_string()), }), downshift_under: None, }; @@ -237,13 +249,140 @@ model = "vendor/cheaper-model" assert_eq!(settings, both()); } +/// **A half rule parses and is refused, and the difference is the whole finding.** +/// +/// These keys were required, which looked stricter and was far worse. A required +/// field is a *deserialization* failure, so `failures = 3` with no `model` did not +/// fail the rule — it failed `CliSettings`, and `settings::stored` then answered +/// `None` for the whole `[app.io-cli]` section. The theme, the keys, the ceilings, +/// the capabilities and the **verification gate** all silently reverted to their +/// defaults, because `contract::criterion_for` gives up on that same `None`. A gate +/// that stops gating without saying so is the most expensive failure this crate +/// has, and it was one missing line in a configuration file away. +/// +/// Worse, `/config` writes exactly one key per invocation, so **every** path +/// through that surface to a routing rule passed through this state. +/// +/// Found by the adversarial review. Sabotage: make either key required again — +/// under which this test fails at the `expect`, because the section no longer +/// parses at all. #[test] -fn a_rule_missing_half_of_itself_is_refused_rather_than_defaulted() { - // A threshold with no model is half a rule. Defaulted, it would be a rule - // routing to the empty string; refused, the operator hears the key name. - let refused = toml::from_str::("[escalate_after]\nfailures = 3\n"); +fn a_rule_missing_half_of_itself_parses_and_is_refused_by_name() { + let settings: Settings = toml::from_str("[escalate_after]\nfailures = 3\n") + .expect("half a rule must still parse, or it takes all of [app.io-cli] with it"); + + assert_eq!( + routing::routing(&settings), + Err(routing::Refusal::HalfARule { + rule: "escalate_after", + missing: "model", + }), + ); assert!( - refused.is_err(), - "half a rule must not deserialize: {refused:?}" + routing::notice(&settings).is_some_and(|said| said.contains("half a rule")), + "the operator hears which key is missing", + ); +} + +/// **The three values that are writable and disastrous.** +/// +/// io-harness obeys the thresholds literally (`contract.rs:1811-1822`), and none +/// of these is a shape TOML can refuse: +/// +/// * `failures = 0` satisfies `consecutive_gate_failures >= 0` at the first request +/// of every run, so the escalation model is used unconditionally and the +/// downshift — checked second — is never reached. An operator writing it means +/// "escalate readily" and gets "never use the model I configured". +/// * `bytes = 0` can never be true, so the rule is permanently inert. +/// * An empty model sends every request of the run with no model id. +/// +/// Found by the adversarial review, which noted that this file already named the +/// empty-model outcome as the thing being avoided and the code then accepted it. +#[test] +fn a_threshold_that_could_only_misfire_is_refused() { + let escalating_at_zero = Settings { + escalate_after: Some(routing::Escalation { + failures: Some(0), + model: Some(STRONGER.to_string()), + }), + downshift_under: None, + }; + assert_eq!( + routing::routing(&escalating_at_zero), + Err(routing::Refusal::EscalatesBeforeAnythingFailed), + ); + + let never_downshifts = Settings { + escalate_after: None, + downshift_under: Some(routing::Downshift { + bytes: Some(0), + model: Some(CHEAPER.to_string()), + }), + }; + assert_eq!( + routing::routing(&never_downshifts), + Err(routing::Refusal::NeverDownshifts), + ); + + let nameless = Settings { + escalate_after: Some(routing::Escalation { + failures: Some(3), + model: Some(" ".to_string()), + }), + downshift_under: None, + }; + assert_eq!( + routing::routing(&nameless), + Err(routing::Refusal::NoModel { + rule: "escalate_after" + }), + ); +} + +/// A refusal leaves the run unrouted, and every refusal has a sentence. +/// +/// The pair of `contract::gate_notice`: a section that is plainly in the +/// operator's file and is not doing anything has to say why, or the surface that +/// lists it is lying by omission. +#[test] +fn every_refusal_says_which_key_is_wrong_and_that_the_turn_is_not_routed() { + for settings in [ + Settings { + escalate_after: Some(routing::Escalation { + failures: Some(0), + model: Some(STRONGER.to_string()), + }), + downshift_under: None, + }, + Settings { + escalate_after: None, + downshift_under: Some(routing::Downshift { + bytes: Some(0), + model: Some(CHEAPER.to_string()), + }), + }, + Settings { + escalate_after: Some(routing::Escalation { + failures: Some(3), + model: None, + }), + downshift_under: None, + }, + ] { + let said = routing::notice(&settings).expect("a refused section says why"); + assert!( + said.contains("app.io-cli.routing"), + "a refusal names the section the operator has to open: {said}", + ); + assert!( + said.contains("not routed"), + "a refusal says what the run does instead: {said}", + ); + } + + assert_eq!( + routing::notice(&both()), + None, + "an obeyable section has nothing to explain", ); } From 229727e90313d6e87bb097edc19004c2ef362939 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 13:19:43 +0530 Subject: [PATCH 10/10] test(narrow): assert the invariant across widths, not arithmetic at one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first version of this gate picked eighty columns and asserted the standing facts were present there. Which width a field starts costing something depends on the posture string, the glyph set and every counter on the row, so that is arithmetic a test will get wrong — and it did: with the fixture trimmed so the control arm passed, the sabotage that put the level back in the all-or-nothing group sailed through it. The property has no number in it. Setting a level must never remove a standing fact that was on the row without it, at any width. Checked at six, with a guard that at least one of them held a standing fact to begin with — without which the whole loop is satisfied by a row that never carried one. --- tests/narrow.rs | 91 ++++++++++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/tests/narrow.rs b/tests/narrow.rs index 74392c7..15ef6ed 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -1736,60 +1736,75 @@ fn f11_the_branch_field_is_a_plain_name_and_costs_a_stated_number_of_columns() { fn n2_an_effort_level_yields_before_the_standing_facts_at_eighty_columns() { use io_cli::status::Status; - // The branch arm's fixture, and deliberately **without** `planning`: with it - // on, this row's standing group already does not fit at eighty columns, so - // every assertion below would have held whether or not the effort level cost - // anything. The first version of this test had it on and failed for that - // reason rather than for a defect — a gate over a field it could not observe, - // caught this time before it shipped rather than a release later. - let fixture = || { + let fixture = |effort: Option| { let mut status = Status::new("anthropic/claude-sonnet-4.5"); status.policy = Some("read-only".into()); status.containment = Some("workspace-write/macos-sandbox-exec".into()); - status.steps = Some(6); + status.planning = true; + status.effort = effort; status }; + // **The invariant across widths, rather than an assertion at one.** Which + // width a field starts costing something depends on the posture string, the + // glyph set and every counter on the row, so a test that picks one number is + // asserting arithmetic it will get wrong — the first version of this gate + // picked eighty, where this fixture's group does not fit either way, and the + // sabotage that put the level back in the all-or-nothing group passed it. The + // property that actually matters has no number in it: **setting a level must + // never remove a standing fact that was on the row without it.** + let standing = [ + "read-only", + "workspace-write/macos-sandbox-exec", + "planning", + ]; + for theme in themes() { let set = theme.glyphs.name; + let mut ever_held = false; - // The control. Without it, everything below is equally true of a row that - // never carried the standing facts in the first place. - let bare = text_of(&fixture().footer(WIDTH, &theme)).join("\n"); - for standing in ["read-only", "workspace-write/macos-sandbox-exec"] { - assert!( - bare.contains(standing), - "{set}: the row did not hold `{standing}` to begin with, so nothing \ - below measures anything: {bare:?}", - ); + for width in [80, 88, 96, 104, 120, 160] { + let bare = text_of(&fixture(None).footer(width, &theme)).join("\n"); + let level = + text_of(&fixture(Some(io_harness::Effort::High)).footer(width, &theme)).join("\n"); + + for row in level.lines() { + assert!( + row.chars().count() <= width as usize, + "{set}: a footer row overflowed {width} columns: {row:?}", + ); + } + + for fact in standing { + if bare.contains(fact) { + ever_held = true; + assert!( + level.contains(fact), + "{set}: at {width} columns, setting an effort level took \ + `{fact}` off the row. The level belongs in the counters, \ + which narrow one at a time, not in the group `row` keeps \ + or drops whole.\n without: {bare:?}\n with: {level:?}", + ); + } + } } - let mut level = fixture(); - level.effort = Some(io_harness::Effort::High); + // Without this the loop above is satisfied by a row that never carried a + // standing fact at any width — which is precisely how the first version of + // this test passed its own sabotage. + assert!( + ever_held, + "{set}: no width held a standing fact even without an effort level, so \ + nothing above measured anything", + ); - // Wide, where the row holds everything: the level is drawn at all. - let wide = text_of(&level.footer(200, &theme)).join("\n"); + // And the level is drawn where there is room for it, so this is a gate over + // a field that exists rather than over one that was never added. + let wide = text_of(&fixture(Some(io_harness::Effort::High)).footer(200, &theme)).join("\n"); assert!( wide.contains("effort high"), "{set}: the level is not on the footer at any width: {wide:?}", ); - - // Narrow, where it must not cost the standing facts. - let narrow = text_of(&level.footer(WIDTH, &theme)).join("\n"); - for row in narrow.lines() { - assert!( - row.chars().count() <= WIDTH as usize, - "{set}: a footer row overflowed eighty columns: {row:?}", - ); - } - for standing in ["read-only", "workspace-write/macos-sandbox-exec"] { - assert!( - narrow.contains(standing), - "{set}: setting an effort level took `{standing}` off the row — the \ - level belongs in the counters, which narrow one at a time, and not \ - in the group `row` keeps or drops whole: {narrow:?}", - ); - } } }