From 5a4528c70f288d0ce4a3fcb3dd3dd2a23af9b54a Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:27:08 +0530 Subject: [PATCH 01/16] feat(gates): declare the verification module and bump to 0.24.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/gates.rs holds what "done" means for a repository: the [app.io-cli.gates] section as written, the one criterion it resolves to, and the two refusals the file itself cannot express. The module is gates and not verify — src/verify.rs is already the wizard's live credential check, and two modules named for the same word would be a reader trap for the life of the product. --- CHANGELOG.md | 38 +++++++++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- src/gates.rs | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + 5 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 src/gates.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 06cbd3c..e208e72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,44 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.24.0] - 2026-08-28 + +A turn proves its work instead of asserting it. io-harness has carried a +verification pillar since long before this interface existed — a contract holds a +criterion, the run executes it after the agent stops, and the run comes back as +`Success` rather than `Finished` when it passed. io-cli has never once supplied +one, which is why every clean run this product has ever reported said `finished` +and meant "the agent stopped", not "the work holds up". + +**You say what done means for this repository, in the repository's own +language.** A command that must exit zero, a file that must exist and say +something, or a rubric a second model answers. The command is proposed from what +the repository actually is: io-harness reads its marker files and names its own +test command, so a Rust checkout offers `cargo test`, a `package.json` offers +whatever its lockfile implies, and a repository with no marker offers nothing +rather than guessing. io-cli holds no list of test commands and no list of marker +files. + +**The verdict is on screen while it happens.** The status line carries the gate's +standing and, once there has been more than one, which attempt this is. After a +gated turn the scrollback takes the phase that ran, what it answered and what the +command printed — a gate that failed because the tests went red reads differently +from one that failed because the policy would not let the program run at all. + +**A failing gate sends the agent back to work with the failure in hand**, up to a +retry budget you set and defaulting to one. The harness's own loop does not do +this: it records the failure and takes another step without telling the model +anything, so a run can spend its whole budget failing the same gate for the same +reason. What io-cli does instead is drive a follow-up turn carrying the criterion +and the output it produced. + +**`io exec` gains exit `6`** — the agent finished and the work does not hold up. +It is distinct from `5`, the agent stopping without finishing, and from `3`, the +ceiling a failing gate used to hide behind. No existing code changed meaning. + +Also: `/mcp` can edit a server at last, closing a limitation this product has +carried and stated since 0.21.0. + ## [0.23.0] - 2026-08-28 A run that paused is answered rather than abandoned. Everything needed to do it diff --git a/Cargo.lock b/Cargo.lock index d0a0488..d2affcc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.23.0" +version = "0.24.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index f323fe2..14573d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.23.0" +version = "0.24.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/src/gates.rs b/src/gates.rs new file mode 100644 index 0000000..ecfa2ef --- /dev/null +++ b/src/gates.rs @@ -0,0 +1,153 @@ +//! What "done" means for this repository, and whether the turn proved it. +//! +//! io-harness has carried a verification pillar since long before this interface +//! existed: a `TaskContract` holds one `Verification`, the run loop executes it +//! after the agent stops, and the run comes back as `RunOutcome::Success` when it +//! passed. Until this release io-cli supplied none, so every contract it built +//! carried `Verification::None` and every clean run returned `Finished` — which +//! `crate::exec` already documents as an honest report of a contract that asked +//! for nothing. +//! +//! This module is where the operator's answer lives. [`Settings`] is the +//! `[app.io-cli.gates]` section as written; [`Criterion`] is the one criterion +//! that section resolves to, once it has been checked for the two mistakes the +//! file itself cannot express. +//! +//! # One criterion, not a suite +//! +//! `TaskContract`'s verification is a single value rather than a list, so a +//! contract carries one criterion. That is the whole surface this module offers, +//! and the section is a table rather than an array so that a list, if the +//! dependency ever grows one, is an addition rather than a break. +//! +//! # Why the criterion is not run here +//! +//! io-harness executes it, inside the sandbox, with `argv[0]` checked against the +//! operator's `Act::Exec` policy. io-cli could run one itself — `Verification` +//! exposes the calls — but `ExecGuard::with_writable_roots` is `pub(crate)`, so a +//! criterion run from here cannot be given the cache directories a real run gets +//! from the detected toolchain, and a `cargo test` gate would fail on a registry +//! write the harness's own gate would have allowed. The criterion rides the +//! contract; the dependency runs it. That is also what keeps `crate::shell` the +//! only module in this crate that spawns anything. +//! +//! # The refusals happen when the file is written +//! +//! A review criterion with no reviewer, or one whose reviewer is the model doing +//! the work, is `Error::Config` at run start in io-harness — before the first +//! billed call, on every turn, with the failure disconnected on screen from the +//! keystroke that caused it. So [`Refusal`] exists to catch both while the +//! operator is still looking at the surface that wrote them. + +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; + +/// The `[app.io-cli.gates]` section exactly as an operator wrote it. +/// +/// Flat rather than tagged, matching every other section this crate reads: TOML +/// spells a flat table far more naturally than an internally tagged enum, and a +/// key nobody set is simply absent. Exactly one of [`Settings::command`], +/// [`Settings::file`] and [`Settings::rubric`] decides the kind; naming none or +/// more than one is a [`Refusal`] rather than a silent precedence rule, because a +/// precedence rule here would quietly gate a turn on something the operator did +/// not choose. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Settings { + /// How many further turns a failing gate may buy, defaulting to one. + /// + /// Zero means the gate reports and nothing is re-driven. It is a small + /// number on purpose: a retry is a whole turn against a real model, and an + /// operator who wants several says so. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub retries: Option, + + /// The argv of a command that must exit [`Settings::expect_exit`]. + /// + /// An argv rather than a command line, because io-harness checks `argv[0]` + /// against the policy and runs it without a shell. An empty vector means the + /// command the repository's own toolchain proposes. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub command: Option>, + + /// The exit status the command must report, defaulting to zero. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub expect_exit: Option, + + /// A workspace-relative file that must exist. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub file: Option, + + /// Text [`Settings::file`] must contain. Absent asserts existence alone. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub contains: Option, + + /// What a second model is asked to judge the work against. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rubric: Option, + + /// The model that answers [`Settings::rubric`]. + /// + /// Required whenever a rubric is set, and never defaulted to the model doing + /// the work — a judge marking its own paper is a decision the operator makes + /// explicitly through [`Settings::allow_self_review`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reviewer: Option, + + /// Whether the reviewer may be the same model that did the work. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub allow_self_review: Option, +} + +/// The one criterion a [`Settings`] resolves to. +/// +/// Distinct from [`Settings`] because a section is what was written and a +/// criterion is what survived being checked. Everything downstream — the +/// contract, the status line, the retry, the exit code — takes this. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Criterion { + /// A command that must exit a named status, run by io-harness in its sandbox. + Command { + /// The program and its arguments. Never a shell line. + argv: Vec, + /// The status that counts as passing. + expect_exit: i32, + }, + /// A workspace-relative file that must exist, and optionally say something. + File { + /// The path, relative to the workspace root. + file: PathBuf, + /// Text the file must contain, or `None` to assert existence alone. + contains: Option, + }, + /// A rubric a second model answers. + Review { + /// What the reviewer is asked to judge. + rubric: String, + /// The model that answers it. + reviewer: String, + /// Whether that model may be the one that did the work. + allow_self_review: bool, + }, +} + +/// Why a section cannot become a [`Criterion`]. +/// +/// Each of these is refused where the operator can still see what they typed. +/// The last two exist because io-harness answers them with `Error::Config` at run +/// start, which would turn writing a configuration file into a session that will +/// not start. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Refusal { + /// The section names no criterion at all. + Empty, + /// The section names more than one kind, and there is no right answer. + Ambiguous, + /// A rubric was given with no model to answer it. + ReviewerMissing, + /// The reviewer is the model doing the work, and that was not asked for. + SelfReview { + /// The model named as both the worker and the judge. + model: String, + }, +} diff --git a/src/lib.rs b/src/lib.rs index 380e077..605bae9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -51,6 +51,7 @@ pub mod failure; pub mod fanout; pub mod fleet; pub mod fuzzy; +pub mod gates; pub mod glyphs; pub mod home; pub mod import; From 45745c11178596957863003a2c6a55a6bf3882e7 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:50 +0530 Subject: [PATCH 02/16] feat(gates): the criterion, its refusals, and the repository's own command Settings::criterion resolves [app.io-cli.gates] to one Criterion or to a Refusal, and the refusals are the point: a rubric with no reviewer, or a reviewer that is the model doing the work, is Error::Config at run start in io-harness, so catching them at the file is the difference between a config edit and a session that will not start. The proposed command comes from io_harness::toolchain::detect layered with Config::toolchain. io-cli holds no marker table and no command table, and a test sweeps this module's comment-stripped source to keep it that way. File existence maps to Verification::None and is answered here through read_bytes. WorkspaceFileContains reads with unwrap_or_default, so an empty needle passes on a file nobody wrote; checked_here() exists so a caller reading verification() alone cannot report an ungated run. --- src/gates.rs | 429 ++++++++++++++++++++++++++- tests/gates.rs | 785 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 1213 insertions(+), 1 deletion(-) create mode 100644 tests/gates.rs diff --git a/src/gates.rs b/src/gates.rs index ecfa2ef..16e61c8 100644 --- a/src/gates.rs +++ b/src/gates.rs @@ -39,8 +39,14 @@ //! keystroke that caused it. So [`Refusal`] exists to catch both while the //! operator is still looking at the surface that wrote them. -use std::path::PathBuf; +use std::fmt; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use io_harness::{ + Anthropic, Compatible, Config, GateAttempt, GateOutcome, ModelReviewer, OpenAi, OpenRouter, + ProviderSpec, Reviewer, SandboxEvent, Verification, +}; use serde::{Deserialize, Serialize}; /// The `[app.io-cli.gates]` section exactly as an operator wrote it. @@ -151,3 +157,424 @@ pub enum Refusal { model: String, }, } + +impl Settings { + /// The one criterion this section resolves to, or why it cannot. + /// + /// `Ok(None)` is the answer for a section that is entirely absent — every key + /// unset. That is not the same as [`Refusal::Empty`], which is a section an + /// operator plainly meant to be a gate (`retries` is set, or `contains`, or a + /// `reviewer`) and which names no kind: the first is "no gate was asked for" + /// and the second is "a gate was asked for and half-written", and answering + /// both with silence would let a typo turn a gate off without a word. + /// + /// `turn_model` is the model the session will actually run with, passed in + /// rather than read here, so this function stays a pure decision over its + /// arguments and the tests need no configuration on disk. An empty string + /// means the caller does not yet know which model will run — the wizard, a + /// fresh home directory — and the self-review refusal cannot fire, because + /// there is nothing to compare the reviewer against. It is not defaulted to + /// "no clash": the caller that knows the model is the caller that must say it. + /// + /// The order of the checks is the order an operator can act on. Ambiguity is + /// answered before emptiness because the two cannot both be true; a missing + /// reviewer is answered before self-review because a reviewer that was never + /// named cannot be compared to anything. + pub fn criterion(&self, turn_model: &str) -> Result, Refusal> { + let kinds = u8::from(self.command.is_some()) + + u8::from(self.file.is_some()) + + u8::from(self.rubric.is_some()); + if kinds > 1 { + return Err(Refusal::Ambiguous); + } + if kinds == 0 { + // Compared against the default rather than against a hand-written + // list of the remaining keys: a key added to `Settings` later joins + // this check by existing, instead of by somebody remembering to add + // it here and shipping a silent hole when they do not. + return if self == &Settings::default() { + Ok(None) + } else { + Err(Refusal::Empty) + }; + } + + if let Some(argv) = &self.command { + return Ok(Some(Criterion::Command { + argv: argv.clone(), + expect_exit: self.expect_exit.unwrap_or(0), + })); + } + if let Some(file) = &self.file { + return Ok(Some(Criterion::File { + file: file.clone(), + contains: self.contains.clone(), + })); + } + + let rubric = self.rubric.clone().expect("one kind is set and it is the rubric"); + let Some(reviewer) = self.reviewer.clone() else { + return Err(Refusal::ReviewerMissing); + }; + let allow_self_review = self.allow_self_review.unwrap_or(false); + if !allow_self_review && !turn_model.is_empty() && reviewer == turn_model { + return Err(Refusal::SelfReview { model: reviewer }); + } + Ok(Some(Criterion::Review { + rubric, + reviewer, + allow_self_review, + })) + } + + /// How many further turns a failing gate may buy, with the default applied. + /// + /// One, not zero and not three. Zero would make the whole retry half of this + /// release opt-in, and an operator who configured a gate has already said they + /// want the work checked; more than one spends real money on a model that has + /// already been told once what it got wrong. + pub fn retries(&self) -> u8 { + self.retries.unwrap_or(1) + } +} + +impl Criterion { + /// The criterion as the dependency's own type, ready for `with_verification`. + /// + /// Three of the four mappings are direct. The fourth — a file criterion with + /// no needle — is the one that has no honest counterpart, and it is worth + /// saying exactly why rather than picking the nearest variant: + /// + /// `Verification::WorkspaceFileContains` reads its file with + /// `read_to_string(..).unwrap_or_default()`, so a file that is not there is + /// the empty string, and every string contains the empty needle. Mapping + /// existence to an empty needle would therefore produce a gate that passes on + /// a file nobody ever wrote — a criterion that can never fail, which is worse + /// than no criterion at all, because a run reports `Success` on it. + /// `DocumentContains` does error on a file it cannot read, but it errors on + /// nearly every file: it reads four office formats by extension and refuses + /// the rest, so it would report a criterion that could not be evaluated rather + /// than one that failed. Nothing else in the enum reads a path. + /// + /// So existence is answered by [`Criterion::satisfied_in`], here, with a + /// reader that tells a missing file from an empty one — and this returns + /// `Verification::None`, which is the truthful statement that io-harness was + /// asked to check nothing. A caller must therefore ask + /// [`Criterion::checked_here`] before treating a run's own outcome as the + /// whole verdict; that is what the flag is for. + pub fn verification(&self) -> Verification { + match self { + Criterion::Command { argv, expect_exit } => Verification::Command { + argv: argv.clone(), + expect_exit: *expect_exit, + }, + Criterion::File { + file, + contains: Some(needle), + } => Verification::WorkspaceFileContains { + file: file.clone(), + needle: needle.clone(), + }, + Criterion::File { contains: None, .. } => Verification::None, + Criterion::Review { + rubric, + allow_self_review, + .. + } => Verification::Review { + rubric: rubric.clone(), + allow_self_review: *allow_self_review, + }, + } + } + + /// Whether this crate evaluates the criterion itself rather than the run loop. + /// + /// True for exactly one shape — a file criterion with no needle — and the + /// reason it is a question a surface can ask without a workspace root is that + /// "the run finished and its gate passed" is not the whole verdict for such a + /// criterion. A caller that reads [`Criterion::verification`] and nothing else + /// would see `Verification::None` and report an ungated run. + pub fn checked_here(&self) -> bool { + matches!(self, Criterion::File { contains: None, .. }) + } + + /// The verdict for a criterion this crate owns, or `None` when io-harness does. + /// + /// The read goes through `io_harness::tools::Workspace::read_bytes`, and the + /// choice of reader is the whole check: `Workspace::read_file` answers `Ok("")` + /// for a file that is not there, so a criterion written on top of it passes on + /// a file that was never created. `read_bytes` is the one that errors. Going + /// through `Workspace` rather than `std::fs` also keeps the path resolution + /// the rest of this crate uses, so a `file` that climbs out of the workspace + /// is refused here exactly as it is everywhere else. + /// + /// A directory at that path is not a satisfied criterion: reading it fails, + /// which is the answer an operator who wrote a file name wanted. + pub fn satisfied_in(&self, root: &Path) -> Option { + let Criterion::File { + file, + contains: None, + } = self + else { + return None; + }; + // Separators normalised the way io-harness normalises them before it + // resolves a workspace-relative path, so a path an operator typed with + // backslashes means the same file on either platform. + let relative = file.to_string_lossy().replace('\\', "/"); + Some( + io_harness::tools::Workspace::new(root) + .read_bytes(&relative) + .is_ok(), + ) + } + + /// What the criterion asks, in one sentence, for a prompt or a surface. + /// + /// Delegates to the dependency for everything it can express, so the words a + /// retried turn reads are the same words the first turn was judged by. The + /// existence criterion has no `Verification` to delegate to and says so here. + pub fn describe(&self) -> String { + if let Criterion::File { + file, + contains: None, + } = self + { + return format!("the file {} must exist in the workspace", file.display()); + } + self.verification().describe() + } +} + +/// The gate's standing after a turn, as a surface draws it. +/// +/// Read off the stored rows rather than remembered, because a resumed session did +/// not run the turn whose gate it has to report, and a field that is only correct +/// when this process happened to watch the run is a field that lies after every +/// `/resume`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Standing { + /// Which criterion ran, as io-harness recorded it — one of `command`, + /// `review`, `compiles`, `document`, `contains`, `none`. Taken from the row + /// rather than from the configured [`Criterion`]: the row is what actually + /// ran, and a configuration edited mid-session would otherwise relabel a gate + /// that was never it. + pub phase: String, + /// How the last attempt ended. + pub outcome: GateOutcome, + /// Which attempt that was, counting from one. + pub attempt: usize, +} + +/// The gate's standing for a run, or `None` when nothing has been gated. +/// +/// Pure over the rows the caller already read, so nothing here opens a store, and +/// nothing here asks what time it is — the attempt number is a count of rows, not +/// an interval. `attempts` is expected in the ascending row order +/// `Store::gate_attempts` returns; the last row is the standing. +pub fn standing(attempts: &[GateAttempt]) -> Option { + let last = attempts.last()?; + Some(Standing { + phase: last.phase.clone(), + outcome: last.outcome, + attempt: attempts.len(), + }) +} + +/// What a failing gate printed on `step`, or `None` if it printed nothing. +/// +/// The output of a gate command is not in the gate row — `GateAttempt::detail` +/// carries a verdict's reasons — it arrives as sandbox events of kind +/// `gate_output`, already bounded in size by io-harness. Filtered by step so that +/// a retried gate's prompt carries the failure that caused the retry rather than +/// the first one, which is the difference between telling the model what is wrong +/// now and telling it what was wrong two turns ago. +/// +/// More than one row for a step is joined in order: io-harness records one per +/// failing phase, and dropping all but the last would drop the half that explains +/// the other. +pub fn output(events: &[SandboxEvent], step: u32) -> Option { + let text: Vec<&str> = events + .iter() + .filter(|event| event.kind == "gate_output" && event.step == step) + .filter_map(|event| event.detail.as_deref()) + .collect(); + if text.is_empty() { + None + } else { + Some(text.join("\n")) + } +} + +/// Whether a failing gate has a retry left under `retries`. +/// +/// **Deliberately not `GateOutcome::is_retryable`, which answers a different +/// question.** That method asks whether re-running the *same* criterion over the +/// *same* tree could honestly say something else, and for `Failed` the answer is +/// no. This asks whether it is worth driving *another turn* — after which the +/// tree is not the same, because the agent has been handed the failure and sent +/// back to work. `Errored` retries too: a gate that could not be evaluated has +/// judged nothing. +/// +/// No attempts at all is not a retry: nothing has failed yet. +pub fn may_retry(attempts: &[GateAttempt], retries: u8) -> bool { + let Some(standing) = standing(attempts) else { + return false; + }; + standing.outcome != GateOutcome::Passed && standing.attempt <= usize::from(retries) +} + +/// The test command this repository proposes for itself, if it has one. +/// +/// **This crate holds no list of marker filenames and no list of test commands, +/// and that is the point of the function.** Both lists live in +/// `io_harness::toolchain`, which is where the ecosystems are added; a copy here +/// would be a second list that drifts, and the way it drifts is by offering a +/// Rust command in a repository with no Rust in it. `Config::toolchain` then +/// layers whatever the operator wrote in their own `[toolchain.*]` section over +/// the detected answer, so an operator who runs a different test runner is offered +/// theirs rather than the ecosystem's default. +/// +/// `None` means the repository said nothing about itself, and nothing is proposed. +/// There is deliberately no fallback: a proposal is a suggestion an operator +/// accepts with a keystroke, and one that is wrong is worse than one that is +/// absent, because it is accepted just as easily. +pub fn proposed_command(root: &Path, config: &Config) -> Option> { + let tuned = config.toolchain(io_harness::toolchain::detect(root)?); + // An override can name an empty command, which is an operator saying this + // ecosystem has no test step here. Proposing an empty argv would produce a + // criterion with no program in it. + (!tuned.test.is_empty()).then_some(tuned.test) +} + +/// A reviewer that asks `model` through `spec`, ready for `with_reviewer`. +/// +/// **Nothing here calls a provider.** It constructs `ModelReviewer`, which holds +/// its own provider and its own model, and hands it to io-harness to call when the +/// run loop reaches the gate. That is also what keeps `tests/dependencies.rs`'s +/// count of provider calls in this crate at one — the wizard's credential ping in +/// [`crate::verify`] — which is the gate that says this crate has not grown an +/// agent loop. +/// +/// `Provider` is not dyn-compatible, so each variant is constructed in its own arm +/// and boxed as `Arc` afterwards; `Reviewer` *is* dyn-compatible, +/// which is what makes one return type possible at all. The arms mirror +/// [`crate::verify::credential`] exactly, including the fallback to each vendor's +/// own environment variable, because a reviewer configured the way the session's +/// provider is configured should authenticate the way the session does. +/// +/// The error is the provider's own message, for the same reason the wizard keeps +/// it: a refusal an operator can act on names the endpoint and the key, and +/// "could not build a reviewer" names neither. +pub fn reviewer(spec: &ProviderSpec, model: &str) -> Result, String> { + match spec { + ProviderSpec::OpenRouter { api_key, .. } => { + let key = api_key_or_env(api_key, "OPENROUTER_API_KEY")?; + Ok(Arc::new(ModelReviewer::new( + crate::provider::Printable::new(OpenRouter::new(key, model)), + model, + ))) + } + ProviderSpec::Anthropic { api_key, .. } => { + let key = api_key_or_env(api_key, "ANTHROPIC_API_KEY")?; + Ok(Arc::new(ModelReviewer::new( + crate::provider::Printable::new(Anthropic::new(key, model)), + model, + ))) + } + ProviderSpec::OpenAi { api_key, .. } => { + let key = api_key_or_env(api_key, "OPENAI_API_KEY")?; + Ok(Arc::new(ModelReviewer::new( + crate::provider::Printable::new(OpenAi::new(key, model)), + model, + ))) + } + ProviderSpec::Compatible { + base_url, + preset, + api_key, + auth, + .. + } => { + // The model is the reviewer's, never the endpoint's configured one: + // the entire criterion is that a second model reads the work, and + // reusing the spec's model is the mistake it exists to prevent. + let secret = api_key.clone().unwrap_or_default(); + let compatible = match (preset, base_url) { + (Some(preset), _) => { + Compatible::preset(preset, secret, model).map_err(|error| error.to_string())? + } + (None, Some(base)) => Compatible::new( + base, + auth.unwrap_or(io_harness::Auth::Bearer), + secret, + model, + ), + (None, None) => { + return Err("this endpoint names neither a preset nor a base URL".into()) + } + }; + Ok(Arc::new(ModelReviewer::new( + crate::provider::Printable::new(compatible), + model, + ))) + } + // `ProviderSpec` is `#[non_exhaustive]`. A provider this release has not + // seen cannot be built into a reviewer, and refusing at write time is the + // whole reason this module validates anything. + other => Err(format!( + "this release does not know how to review with a {other:?} provider yet" + )), + } +} + +/// The key from the spec, or from the provider's own environment variable. +/// +/// A near-twin of the wizard's own resolver, kept separate on purpose: that one is +/// private to [`crate::verify`], and importing it would make a module about live +/// provider calls a dependency of a module whose entire claim is that it makes +/// none. +fn api_key_or_env(api_key: &Option, var: &str) -> Result { + if let Some(key) = api_key { + return Ok(key.clone()); + } + match std::env::var(var) { + Ok(key) if !key.is_empty() => Ok(key), + _ => Err(format!( + "no key was given and ${var} is not set in this shell" + )), + } +} + +/// One sentence an operator can act on, for each way a section is refused. +/// +/// ASCII throughout — no quotation marks that are not the typewriter kind, no +/// arrows, no ellipsis character. These render on the plain renderer, under +/// `NO_COLOR`, and through the ASCII glyph set, and a refusal that arrives as a +/// replacement character is a refusal nobody reads. +/// +/// Each one names the key to change, because the operator is looking at the file. +impl fmt::Display for Refusal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Refusal::Empty => f.write_str( + "this gates section sets no criterion: name a command, a file or a \ + rubric, or delete the section", + ), + Refusal::Ambiguous => f.write_str( + "this gates section names more than one criterion: keep exactly one \ + of command, file and rubric", + ), + Refusal::ReviewerMissing => f.write_str( + "a rubric needs a model to answer it: set reviewer, or delete the \ + rubric", + ), + Refusal::SelfReview { model } => write!( + f, + "the reviewer {model} is the model doing the work: name a different \ + reviewer, or set allow_self_review = true to accept a model \ + marking its own paper" + ), + } + } +} diff --git a/tests/gates.rs b/tests/gates.rs new file mode 100644 index 0000000..01cf39d --- /dev/null +++ b/tests/gates.rs @@ -0,0 +1,785 @@ +//! F2, F3, F4 and F5 — what `[app.io-cli.gates]` resolves to, what it refuses, +//! what the repository proposes for itself, and what a file criterion actually +//! asserts. +//! +//! Every assertion here is written against the way the implementation would be +//! wrong rather than the way it is right, because the shapes this module gets +//! wrong all *pass* a naive test: +//! +//! * an existence criterion built on a reader that answers `Ok("")` for a file +//! that is not there passes on a file nobody wrote, and a test that only checks +//! the happy path — the file exists, the gate says yes — never notices; +//! * a command criterion that hardcodes exit zero satisfies every test written +//! with the default; +//! * a toolchain proposal that falls back to a Rust command is correct in this +//! repository and wrong in every other one, so it has to be asserted against a +//! directory that is not this repository; +//! * a refusal that is silently `Ok(None)` looks exactly like a section nobody +//! wrote. +//! +//! So the file criterion is asserted against a *missing* file and against an +//! *empty* one separately, the exit status is asserted at a value the default is +//! not, and the toolchain proposal is asserted against four temporary directories +//! of which one holds nothing at all. + +use std::path::{Path, PathBuf}; + +use io_cli::gates::{self, Criterion, Refusal, Settings}; +use io_harness::{Config, GateAttempt, GateOutcome, SandboxEvent, Verification}; + +/// The model the session would run with. Only the self-review refusal compares +/// against it, and only by equality. +const TURN_MODEL: &str = "vendor/worker-model"; + +/// A section with one key set, everything else absent. +fn section() -> Settings { + Settings::default() +} + +/// One recorded gate evaluation. `at` is a stored string this crate never parses +/// and never compares, so a fixture can leave it empty. +fn attempt(step: u32, phase: &str, outcome: GateOutcome) -> GateAttempt { + GateAttempt { + id: 1, + step, + phase: phase.into(), + outcome, + detail: String::new(), + at: String::new(), + } +} + +/// An empty configuration: no `[toolchain.*]` overrides, so a proposal is exactly +/// what the repository said about itself. +fn plain() -> Config { + Config::from_toml("").expect("an empty configuration parses") +} + +// --------------------------------------------------------------------------- +// Resolving the section +// --------------------------------------------------------------------------- + +/// A section nobody wrote is not a refusal, and a section somebody started is. +/// +/// The pair is the test. Answering both with `Ok(None)` would be the natural +/// implementation — "no kind is named, so there is no gate" — and it turns a +/// half-written gate into a silently ungated session. +#[test] +fn f2_an_absent_section_resolves_to_nothing_and_a_half_written_one_is_refused() { + assert_eq!( + section().criterion(TURN_MODEL), + Ok(None), + "a section with every key unset asked for no gate" + ); + + for started in [ + Settings { + retries: Some(2), + ..section() + }, + Settings { + contains: Some("ok".into()), + ..section() + }, + Settings { + reviewer: Some("vendor/judge".into()), + ..section() + }, + Settings { + expect_exit: Some(1), + ..section() + }, + Settings { + allow_self_review: Some(true), + ..section() + }, + ] { + assert_eq!( + started.criterion(TURN_MODEL), + Err(Refusal::Empty), + "{started:?} names no kind but is plainly meant to be a gate, and \ + resolving it to nothing would gate the session on nothing" + ); + } +} + +/// Two kinds is a refusal and never a precedence rule. +/// +/// Asserted for every pair and for all three, because a precedence rule written +/// as an `if` chain refuses none of them and quietly gates the turn on whichever +/// branch happened to come first. +#[test] +fn f2_more_than_one_kind_is_refused_rather_than_ordered() { + let command = Some(vec!["cargo".into(), "test".into()]); + let file = Some(PathBuf::from("REPORT.md")); + let rubric = Some("is it done".into()); + + for both in [ + Settings { + command: command.clone(), + file: file.clone(), + ..section() + }, + Settings { + command: command.clone(), + rubric: rubric.clone(), + reviewer: Some("vendor/judge".into()), + ..section() + }, + Settings { + file: file.clone(), + rubric: rubric.clone(), + reviewer: Some("vendor/judge".into()), + ..section() + }, + Settings { + command, + file, + rubric, + reviewer: Some("vendor/judge".into()), + ..section() + }, + ] { + assert_eq!( + both.criterion(TURN_MODEL), + Err(Refusal::Ambiguous), + "{both:?} names more than one criterion and there is no right answer" + ); + } +} + +/// A rubric with no reviewer is refused here, not at run start. +/// +/// io-harness answers this with `Error::Config` before the first billed call, so +/// an implementation that resolves it to a `Review` criterion has moved the +/// failure from a keystroke the operator is looking at to every turn afterwards. +#[test] +fn f5_a_rubric_with_no_reviewer_is_refused() { + let written = Settings { + rubric: Some("the change is covered by a test".into()), + ..section() + }; + assert_eq!(written.criterion(TURN_MODEL), Err(Refusal::ReviewerMissing)); + + // And the refusal survives `allow_self_review`, which is about *which* model + // judges and cannot supply one. + let permissive = Settings { + rubric: Some("the change is covered by a test".into()), + allow_self_review: Some(true), + ..section() + }; + assert_eq!( + permissive.criterion(TURN_MODEL), + Err(Refusal::ReviewerMissing), + "allowing self-review does not name a reviewer" + ); +} + +/// A reviewer that is the model doing the work is refused unless it was asked for. +/// +/// The three cases together are the test: refused by default, allowed when the +/// operator said so, and untouched when the reviewer is somebody else. An +/// implementation that never fires the refusal passes the second and third alone. +#[test] +fn f5_a_reviewer_equal_to_the_turn_model_is_refused_unless_allowed() { + let judging_itself = Settings { + rubric: Some("the change is covered by a test".into()), + reviewer: Some(TURN_MODEL.into()), + ..section() + }; + assert_eq!( + judging_itself.criterion(TURN_MODEL), + Err(Refusal::SelfReview { + model: TURN_MODEL.into(), + }), + "a model marking its own paper is a decision, not a default" + ); + + let asked_for = Settings { + allow_self_review: Some(true), + ..judging_itself.clone() + }; + assert_eq!( + asked_for.criterion(TURN_MODEL), + Ok(Some(Criterion::Review { + rubric: "the change is covered by a test".into(), + reviewer: TURN_MODEL.into(), + allow_self_review: true, + })), + "the operator said so, and the flag has to reach the criterion" + ); + + let second_model = Settings { + reviewer: Some("vendor/judge-model".into()), + ..judging_itself.clone() + }; + assert_eq!( + second_model.criterion(TURN_MODEL), + Ok(Some(Criterion::Review { + rubric: "the change is covered by a test".into(), + reviewer: "vendor/judge-model".into(), + allow_self_review: false, + })) + ); + + // A caller that does not yet know which model will run cannot be told there + // is a clash, and must not be told there is none either — it is told nothing, + // which is what an empty model means. + assert!( + judging_itself.criterion("").is_ok(), + "with no turn model there is nothing to compare the reviewer against" + ); +} + +// --------------------------------------------------------------------------- +// Mapping a criterion onto the dependency's type +// --------------------------------------------------------------------------- + +/// The exit status the operator wrote is the exit status the gate carries. +/// +/// Asserted at one rather than at zero, because zero is the default and a mapping +/// that ignores the key entirely is indistinguishable from a correct one at that +/// value. The default is asserted separately, in the same test, so neither half +/// can pass on its own. +#[test] +fn f2_a_command_criterion_carries_the_exit_status_it_was_given() { + let lint_found_nothing = Settings { + command: Some(vec!["npm".into(), "run".into(), "lint".into()]), + expect_exit: Some(1), + ..section() + }; + let criterion = lint_found_nothing + .criterion(TURN_MODEL) + .expect("a command criterion resolves") + .expect("and it is a criterion"); + assert_eq!( + criterion, + Criterion::Command { + argv: vec!["npm".into(), "run".into(), "lint".into()], + expect_exit: 1, + } + ); + + let Verification::Command { argv, expect_exit } = criterion.verification() else { + panic!("a command criterion maps to a command verification"); + }; + assert_eq!(argv, ["npm", "run", "lint"]); + assert_eq!(expect_exit, 1, "a hardcoded zero would pass every other test"); + + let defaulted = Settings { + command: Some(vec!["make".into(), "check".into()]), + ..section() + }; + assert_eq!( + defaulted + .criterion(TURN_MODEL) + .expect("it resolves") + .expect("to a criterion"), + Criterion::Command { + argv: vec!["make".into(), "check".into()], + expect_exit: 0, + }, + "an unwritten exit status is zero" + ); + + // A command gate is io-harness's to run, whatever else this crate can check. + assert!(!criterion.checked_here()); + assert_eq!(criterion.satisfied_in(Path::new("/nonexistent")), None); +} + +/// A file criterion with a needle is the dependency's contains criterion. +#[test] +fn f4_a_file_with_a_needle_maps_to_the_contains_criterion() { + let written = Settings { + file: Some(PathBuf::from("docs/REPORT.md")), + contains: Some("## Findings".into()), + ..section() + }; + let criterion = written + .criterion(TURN_MODEL) + .expect("it resolves") + .expect("to a criterion"); + + let Verification::WorkspaceFileContains { file, needle } = criterion.verification() else { + panic!("a file criterion with a needle maps to the contains verification"); + }; + assert_eq!(file, PathBuf::from("docs/REPORT.md")); + assert_eq!(needle, "## Findings"); + + assert!( + !criterion.checked_here(), + "the dependency can express this one, so it runs it" + ); + assert_eq!(criterion.satisfied_in(Path::new("/nonexistent")), None); +} + +/// A file criterion with no needle is never a contains criterion with no text. +/// +/// This is the whole of F4's sabotage. The dependency reads its file with a +/// reader that answers the empty string for a file that is not there, and every +/// string contains the empty needle — so the obvious mapping is a gate that can +/// never fail, on a file that need never exist. +#[test] +fn f4_existence_is_never_an_empty_needle() { + let written = Settings { + file: Some(PathBuf::from("REPORT.md")), + ..section() + }; + let criterion = written + .criterion(TURN_MODEL) + .expect("it resolves") + .expect("to a criterion"); + assert_eq!( + criterion, + Criterion::File { + file: PathBuf::from("REPORT.md"), + contains: None, + } + ); + + if let Verification::WorkspaceFileContains { needle, .. } = criterion.verification() { + panic!( + "existence was mapped to a contains criterion with needle {needle:?}, \ + which a file that does not exist satisfies" + ); + } + assert!( + criterion.checked_here(), + "the dependency cannot express existence, so this crate must own it, and \ + a caller has to be able to ask" + ); +} + +/// Existence tells a missing file from an empty one. +/// +/// The empty file is the assertion that matters. Every reader in the dependency +/// that answers `Ok` for a missing file answers `Ok` for an empty one too, so a +/// test that only checks a file with text in it passes on both readers and +/// distinguishes nothing. +#[test] +fn f4_existence_tells_a_missing_file_from_an_empty_one() { + let root = tempfile::tempdir().expect("a temporary directory"); + let criterion = Criterion::File { + file: PathBuf::from("REPORT.md"), + contains: None, + }; + + assert_eq!( + criterion.satisfied_in(root.path()), + Some(false), + "nobody wrote the file, so the gate has to say no" + ); + + std::fs::write(root.path().join("REPORT.md"), "").expect("an empty file is written"); + assert_eq!( + criterion.satisfied_in(root.path()), + Some(true), + "the file exists; a reader that could not tell this from the case above \ + would have passed the assertion before it too" + ); + + // A directory at the path is not a file that was written. + let directory = Criterion::File { + file: PathBuf::from("docs"), + contains: None, + }; + std::fs::create_dir(root.path().join("docs")).expect("a directory is created"); + assert_eq!( + directory.satisfied_in(root.path()), + Some(false), + "an operator who named a file did not mean a directory" + ); + + // And the criterion cannot be used to read outside the workspace. + let escaping = Criterion::File { + file: PathBuf::from("../../etc/hosts"), + contains: None, + }; + assert_eq!( + escaping.satisfied_in(root.path()), + Some(false), + "a path that climbs out of the workspace is refused, not followed" + ); +} + +/// A review criterion carries the rubric and the flag, and not the reviewer. +/// +/// The reviewer is attached to the contract separately, through +/// `TaskContract::with_reviewer`, because the dependency's `Review` variant has +/// no field for it. A mapping that dropped `allow_self_review` would re-refuse at +/// run start the configuration this module just accepted. +#[test] +fn f5_a_review_criterion_carries_the_rubric_and_the_flag() { + let criterion = Criterion::Review { + rubric: "no public item lost its documentation".into(), + reviewer: "vendor/judge-model".into(), + allow_self_review: true, + }; + let Verification::Review { + rubric, + allow_self_review, + } = criterion.verification() + else { + panic!("a review criterion maps to a review verification"); + }; + assert_eq!(rubric, "no public item lost its documentation"); + assert!( + allow_self_review, + "dropping the flag would make the run refuse what this module accepted" + ); + assert!(!criterion.checked_here()); +} + +/// Every criterion describes itself, including the one with no verification. +#[test] +fn f7_every_criterion_says_what_it_asks() { + let existence = Criterion::File { + file: PathBuf::from("REPORT.md"), + contains: None, + }; + let described = existence.describe(); + assert!( + described.contains("REPORT.md") && described.contains("exist"), + "an existence criterion has no verification to delegate to and must still \ + be able to tell a retried turn what it wants: {described:?}" + ); + assert!( + !described.contains("no automated check"), + "delegating this one to the dependency would describe it as ungated" + ); + + let command = Criterion::Command { + argv: vec!["make".into(), "check".into()], + expect_exit: 0, + }; + assert!( + command.describe().contains("make check"), + "the words a retried turn reads are the words it was judged by" + ); +} + +// --------------------------------------------------------------------------- +// Retries +// --------------------------------------------------------------------------- + +/// The retry budget defaults to one and zero is honoured as zero. +/// +/// Zero is the half that needs asserting: an implementation reaching for +/// `unwrap_or` on a count and then treating a falsy value as unset turns "do not +/// retry" into "retry once", which spends a turn against a real model the +/// operator explicitly declined. +#[test] +fn f7_the_retry_budget_defaults_to_one_and_zero_means_zero() { + assert_eq!(section().retries(), 1); + assert_eq!( + Settings { + retries: Some(0), + ..section() + } + .retries(), + 0, + "an operator who wrote zero asked for no retry at all" + ); + assert_eq!( + Settings { + retries: Some(3), + ..section() + } + .retries(), + 3 + ); + + let failed = [attempt(1, "command", GateOutcome::Failed)]; + assert!( + !gates::may_retry(&failed, 0), + "zero retries must not buy one" + ); + assert!(gates::may_retry(&failed, 1), "one retry buys the second turn"); + + let twice = [ + attempt(1, "command", GateOutcome::Failed), + attempt(2, "command", GateOutcome::Failed), + ]; + assert!( + !gates::may_retry(&twice, 1), + "one retry buys one further turn, not one further failure" + ); + + assert!( + !gates::may_retry(&[attempt(1, "command", GateOutcome::Passed)], 3), + "a gate that passed is not retried however much budget is left" + ); + assert!( + gates::may_retry(&[attempt(1, "review", GateOutcome::Errored)], 1), + "a gate that could not be evaluated has judged nothing" + ); + assert!( + !gates::may_retry(&[], 3), + "nothing has failed, so there is nothing to retry" + ); +} + +/// The standing is the last attempt, counted from one. +#[test] +fn f6_the_standing_is_the_last_attempt_and_its_number() { + assert_eq!(gates::standing(&[]), None, "an ungated run has no standing"); + + let attempts = [ + attempt(1, "command", GateOutcome::Failed), + attempt(2, "command", GateOutcome::Passed), + ]; + let standing = gates::standing(&attempts).expect("two attempts have a standing"); + assert_eq!(standing.outcome, GateOutcome::Passed, "the last one, not the first"); + assert_eq!(standing.attempt, 2, "counted from one, so two attempts is two"); + assert_eq!(standing.phase, "command"); +} + +/// A failing gate's output is read per step, so a retry carries its own failure. +#[test] +fn f7_the_recorded_output_is_the_one_for_the_step_asked_about() { + let events = [ + SandboxEvent::gate_output(7, 1, "first failure"), + SandboxEvent::exec(7, 1, "macos-sandbox-exec", "cargo test"), + SandboxEvent::gate_output(7, 2, "second failure"), + SandboxEvent::gate_output(7, 2, "and its second phase"), + ]; + + assert_eq!( + gates::output(&events, 2).as_deref(), + Some("second failure\nand its second phase"), + "the retry is told what is wrong now, and no phase of it is dropped" + ); + assert_eq!(gates::output(&events, 1).as_deref(), Some("first failure")); + assert_eq!( + gates::output(&events, 3), + None, + "a step that printed nothing is nothing, not an empty string" + ); + assert_eq!(gates::output(&[], 1), None); +} + +// --------------------------------------------------------------------------- +// What the repository proposes for itself +// --------------------------------------------------------------------------- + +/// The proposal is the repository's own, for four different repositories. +/// +/// The bare directory is the assertion the other three cannot make: a fallback to +/// a Rust command is correct in this repository and in the first case below, and +/// wrong everywhere else, so only a directory that says nothing about itself +/// catches it. +#[test] +fn f3_the_test_command_is_proposed_by_the_repository_and_never_defaulted() { + let rust = tempfile::tempdir().expect("a temporary directory"); + std::fs::write(rust.path().join("Cargo.toml"), "[package]\nname = \"x\"\n") + .expect("the marker is written"); + assert_eq!( + gates::proposed_command(rust.path(), &plain()), + Some(vec!["cargo".into(), "test".into()]) + ); + + let node = tempfile::tempdir().expect("a temporary directory"); + std::fs::write(node.path().join("package.json"), "{}").expect("the marker is written"); + std::fs::write(node.path().join("pnpm-lock.yaml"), "").expect("the lockfile is written"); + assert_eq!( + gates::proposed_command(node.path(), &plain()), + Some(vec!["pnpm".into(), "test".into()]), + "the lockfile beside the marker chooses the manager, and this crate holds \ + no opinion about which" + ); + + let python = tempfile::tempdir().expect("a temporary directory"); + std::fs::write(python.path().join("pyproject.toml"), "[project]\nname = \"x\"\n") + .expect("the marker is written"); + let proposed = gates::proposed_command(python.path(), &plain()) + .expect("a Python project proposes a test command"); + assert!( + proposed.first().is_some_and(|program| program != "cargo"), + "a Rust command was offered in a repository with no Rust in it: {proposed:?}" + ); + assert_eq!(proposed, ["python", "-m", "pytest"]); + + let bare = tempfile::tempdir().expect("a temporary directory"); + assert_eq!( + gates::proposed_command(bare.path(), &plain()), + None, + "a repository that said nothing about itself gets no proposal; a fallback \ + here is a wrong command an operator accepts with one keystroke" + ); +} + +/// The operator's own override wins over what was detected. +#[test] +fn f3_an_operator_override_replaces_the_detected_command() { + let rust = tempfile::tempdir().expect("a temporary directory"); + std::fs::write(rust.path().join("Cargo.toml"), "[package]\nname = \"x\"\n") + .expect("the marker is written"); + + let tuned = Config::from_toml( + "\ +[toolchain.cargo] +test = [\"cargo\", \"nextest\", \"run\"] +", + ) + .expect("the fixture parses"); + + assert_eq!( + gates::proposed_command(rust.path(), &tuned), + Some(vec!["cargo".into(), "nextest".into(), "run".into()]), + "detection is layered under the file, not instead of it" + ); + + let silenced = Config::from_toml( + "\ +[toolchain.cargo] +test = [] +", + ) + .expect("the fixture parses"); + assert_eq!( + gates::proposed_command(rust.path(), &silenced), + None, + "an empty override is an operator saying there is no test step here, and \ + an empty argv is a criterion with no program in it" + ); +} + +/// The module holds no list of marker files and no list of test commands. +/// +/// The sweep is over the *comment-stripped* source: this crate argues its design +/// in prose, and prose naming what it deliberately does not hold is a module +/// explaining itself rather than a module hiding a list. Code is what is checked. +/// +/// A marker name appearing here is the copy of the dependency's table that drifts +/// — and the way it drifts is by proposing an ecosystem's command in a repository +/// that stopped being that ecosystem two commits ago. +#[test] +fn f3_the_module_names_no_marker_file_and_no_test_command() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/gates.rs"); + let source = std::fs::read_to_string(&path).expect("the module is readable"); + let code = strip_comments(&source); + + for marker in [ + "Cargo.toml", + "package.json", + "pyproject.toml", + "requirements.txt", + "go.mod", + "deno.json", + "Gemfile", + "pom.xml", + "build.gradle", + "mix.exs", + "composer.json", + "Package.swift", + "CMakeLists.txt", + "Makefile", + ".csproj", + ] { + assert!( + !code.contains(marker), + "src/gates.rs names the marker file {marker:?} in code; detection \ + belongs to io_harness::toolchain, and a second copy of that table is \ + a table that drifts" + ); + } + + for command in [ + "cargo\", \"test", + "npm\", \"test", + "pnpm\", \"test", + "go\", \"test", + "pytest", + "dotnet", + ] { + assert!( + !code.contains(command), + "src/gates.rs spells the test command {command:?} itself, which is the \ + fallback F3 forbids" + ); + } + + // The stripper has to actually strip, or the assertions above are vacuous for + // any module that mentions a marker in prose. + assert!( + strip_comments("let x = 1; // Cargo.toml\n").trim() == "let x = 1;", + "the stripper removes a trailing line comment" + ); + assert!( + !strip_comments("//! about Cargo.toml\ncode\n").contains("Cargo.toml"), + "the stripper removes a module doc line" + ); +} + +/// Everything from a `//` to the end of the line, and every block comment. +/// +/// Deliberately naive about a `//` inside a string literal: the only way that +/// errs is by stripping more than it should, which can weaken this sweep and can +/// never make it fail on a module that is correct. A stripper that tried to parse +/// Rust strings here would be a parser nobody maintains guarding a one-line rule. +fn strip_comments(source: &str) -> String { + let mut out = String::with_capacity(source.len()); + let mut rest = source; + // Block comments first, so a `//` inside one cannot terminate a line early. + while let Some(start) = rest.find("/*") { + out.push_str(&rest[..start]); + match rest[start..].find("*/") { + Some(end) => rest = &rest[start + end + 2..], + None => rest = "", + } + } + out.push_str(rest); + + out.lines() + .map(|line| match line.find("//") { + Some(at) => &line[..at], + None => line, + }) + .collect::>() + .join("\n") +} + +// --------------------------------------------------------------------------- +// What the operator is told +// --------------------------------------------------------------------------- + +/// Every refusal has a sentence, it is ASCII, and it names the key to change. +/// +/// ASCII because these render through the plain renderer, under `NO_COLOR` and +/// through the ASCII glyph set — a refusal that arrives as a replacement +/// character is a refusal nobody reads. Naming the key because the operator is +/// looking at the file when they get it. +#[test] +fn f5_every_refusal_has_an_ascii_sentence_naming_the_key_to_change() { + let sentences = [ + (Refusal::Empty, vec!["command", "file", "rubric"]), + (Refusal::Ambiguous, vec!["command", "file", "rubric"]), + (Refusal::ReviewerMissing, vec!["reviewer", "rubric"]), + ( + Refusal::SelfReview { + model: TURN_MODEL.into(), + }, + vec![TURN_MODEL, "allow_self_review"], + ), + ]; + + for (refusal, keys) in sentences { + let text = refusal.to_string(); + assert!( + text.is_ascii(), + "{refusal:?} renders as {text:?}, which is not ASCII and will not \ + survive the ASCII glyph set" + ); + assert!( + !text.is_empty() && text.len() > 20, + "{refusal:?} renders as {text:?}, which tells an operator nothing" + ); + for key in keys { + assert!( + text.contains(key), + "{refusal:?} renders as {text:?} without naming {key:?}, and the \ + operator is looking at the file" + ); + } + } +} From 1a0fe37e47ada41e3c0a4103d2903a7891479f8c Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:50 +0530 Subject: [PATCH 03/16] feat(gates): [app.io-cli.gates] as a configuration section The field, its catalogue keys, and a worked example of all three kinds. settings::render writes gates: None, following the containment precedent. A file arriving from the wizard with a gate already in it would hold back every turn of an operator who never said what done means here, and a review criterion would spend a second model's completion doing it. The example ships commented out, because the docs gate writes that file to disk and discovers it. --- docs/config.example.toml | 86 +++++++++++++++++++++++++++++++++++++++- src/configure.rs | 19 +++++++++ src/settings.rs | 35 ++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/docs/config.example.toml b/docs/config.example.toml index 77350e7..aff908b 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -345,7 +345,7 @@ net = "deny" # --- io-cli's own section --------------------------------------------------- # -# Eight keys and six tables. io-harness does not validate any of it, so an +# Eight keys and seven 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. @@ -549,3 +549,87 @@ fleet = "ctrl+f" # # [app.io-cli.prices] # source_url = "https://openrouter.ai/api/v1/models" + +# --- what "done" means for this repository (0.24.0) ------------------------- +# +# A gate is the answer to a question only you can answer: what has to be true +# before this turn is finished. io-harness has run a verification after the agent +# stops since long before io-cli asked it to — a task contract carries one, the +# run loop executes it inside the sandbox, and the run comes back a success only +# when it passed. Until this release io-cli supplied none, so every clean turn +# reported "finished", which was an honest report of a contract that asked for +# nothing. +# +# ABSENT MEANS NO GATE, AND THAT IS NOT A DEFAULT WORTH WRITING. Nothing is +# verified for an operator who has not said what verifying would mean, and there +# is no criterion this file could pick for you that would not be a guess about +# somebody else's repository. +# +# EXACTLY ONE OF `command`, `file` AND `rubric` MAY BE SET. They are the three +# kinds, and the key you write is what selects one. Naming none, or naming two, +# is refused where you can still see what you typed rather than resolved by a +# precedence rule — a precedence rule here would quietly gate every turn on +# something you did not choose, which is the one failure a gate must not have. +# +# `retries` is how many further turns a failing gate may buy, and it defaults to +# 1. ZERO MEANS REPORT AND DO NOT RE-DRIVE: the failure is shown and the turn +# ends, which is what you want when a person is watching. It is a small number on +# purpose — a retry is a whole turn against a real model, not a loop iteration — +# so an operator who wants several says so. +# +# --- 1. a command that must exit a status --------------------------------- +# +# The argv of a program, not a shell line: io-harness checks argv[0] against this +# session's `exec` policy and runs it without a shell, so `&&`, a pipe and a glob +# are not available and were never going to be. `expect_exit` defaults to 0. +# +# AN EMPTY OR ABSENT `command` MEANS THE COMMAND THIS REPOSITORY'S OWN TOOLCHAIN +# PROPOSES. io-harness detects that — it is the half of this that knows a Cargo +# workspace from a package.json — and io-cli holds no list of its own to consult. +# That is deliberate: a list of build tools shipped inside a TUI is a list that is +# wrong for somebody the day it is written, and stale for everybody a year later. +# Write the key when your repository's real gate is narrower than what a detector +# would propose, and leave it out when it is not. +# +# [app.io-cli.gates] +# retries = 1 +# command = ["cargo", "test", "--all-features"] +# expect_exit = 0 +# +# --- 2. a file that must exist, and optionally say something -------------- +# +# The path is relative to the workspace root. `contains` is plain text the file +# must hold; absent, existence alone is the whole criterion. This is the cheap +# gate — nothing is run and nothing is billed — and it is the right one for work +# whose product is a file: a migration written, a changelog entry added, a +# generated artefact that has to appear. +# +# [app.io-cli.gates] +# file = "CHANGELOG.md" +# contains = "## 0.24.0" +# +# --- 3. a rubric a second model answers ----------------------------------- +# +# For work no exit status can judge. `rubric` is what the reviewer is asked to +# decide; `reviewer` is the model that decides it. +# +# A RUBRIC REQUIRES A REVIEWER, AND THE REVIEWER MAY NOT BE THE MODEL DOING THE +# WORK UNLESS `allow_self_review = true`. Both are refused when the section is +# written rather than when the turn runs, and that is the whole reason they are +# stated here: io-harness answers each of them with a hard configuration error at +# RUN START — before the first billed call, on every turn — so a file that says +# either one turns a configuration edit into a session that will not start, with +# the failure on screen nowhere near the key that caused it. Self review is +# allowed when you ask for it and never by default: a judge marking its own paper +# is a decision, not an oversight. +# +# AND A REVIEW CRITERION IS A REAL BILLED COMPLETION ON EVERY GATED TURN. It is a +# second model reading the work, charged like any other call, and it runs again on +# each retry the gate buys. The other two kinds cost a process and a file read; +# this one costs money per turn, which is worth knowing before it is in a file +# that runs in CI. +# +# [app.io-cli.gates] +# rubric = "every public item changed in this turn has a doc comment saying why" +# reviewer = "anthropic/claude-sonnet-4.5" +# allow_self_review = false diff --git a/src/configure.rs b/src/configure.rs index dfddaf6..af5bd4e 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -159,6 +159,25 @@ pub const CATALOGUE: &[&str] = &[ // setting, and it is reached the way `/provider` and `/mcp` reach a list. "prices.as_of", "app.io-cli.prices.source_url", + // What "done" means here. Every key of the section and not a chosen few, + // because the section refuses rather than defaults: exactly one of `command`, + // `file` and `rubric` may be set, and an operator meeting three of the eight + // on this surface would be reading a partial list as the whole schema and + // writing an ambiguous section from it. `expect_exit`, `contains` and + // `allow_self_review` each qualify exactly one of those three, and a + // qualifier nobody can see is a qualifier nobody sets. + // + // Unlike `prices.source`, none of these is written by machinery — a gate is + // typed by the operator or it does not exist — so there is no key here that + // is listed only to be read. + "app.io-cli.gates.retries", + "app.io-cli.gates.command", + "app.io-cli.gates.expect_exit", + "app.io-cli.gates.file", + "app.io-cli.gates.contains", + "app.io-cli.gates.rubric", + "app.io-cli.gates.reviewer", + "app.io-cli.gates.allow_self_review", ]; /// The `/config` row that re-reads the price catalogue. diff --git a/src/settings.rs b/src/settings.rs index b1530b4..8c6f9fa 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -140,6 +140,32 @@ pub struct CliSettings { /// model belongs here and nowhere else. #[serde(default, skip_serializing_if = "Option::is_none")] pub prices: Option, + + /// What "done" means for this repository: `[app.io-cli.gates]`. + /// + /// **The section as written, and deliberately not the criterion that runs.** + /// io-harness's `TaskContract` carries one `Verification`, and a section can + /// say things a `Verification` cannot be built from — no criterion at all, two + /// kinds at once, a rubric with nobody to answer it, a reviewer that is the + /// model doing the work. Deserializing straight into the harness's own type + /// would put that check nowhere, so this field is io-cli's own + /// [`crate::gates::Settings`] and [`crate::gates::Criterion`] is what survives + /// being checked. The difference is where the operator hears about a mistake: + /// io-harness answers the last two with a hard configuration error at run + /// start, which turns editing a file into a session that will not start. + /// + /// It is also **not a preference and not a ceiling**. The three ceilings above + /// bound a turn that would have run anyway; this decides whether a turn that + /// finished is finished, and a review criterion spends a real completion on + /// every gated turn. So an absent section is no gate rather than a default + /// one — nothing is verified for an operator who never said what verifying + /// would mean. + /// + /// A table rather than an array because the contract's verification is one + /// value, not a suite; if the dependency ever grows a list, a list here is an + /// addition rather than a break. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub gates: Option, } /// Where prices come from, and what the last read was. @@ -501,6 +527,15 @@ pub fn render( // operator who runs the wizard and one who adds a provider from // `/provider` later. prices: None, + // Left out for the same reason as the caps, and with the same + // force: a gate decides whether a finished turn counts as + // finished, and a file that arrived with one already in it would + // hold back every turn of somebody who never said what "done" + // means here — a review criterion by spending a second model's + // completion to do it. The wizard asks about none of that, and + // there is no default worth writing: "no gate" is the honest + // answer for an operator who has not answered yet. + gates: None, }, }, }; From 4383f7ce232da5bcdca20a88a0145bd4c646ae12 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:51 +0530 Subject: [PATCH 04/16] feat(gates): the gate's standing on the status line Drawn by fields() and by the hand-built footer alike, absent when no criterion is configured, and carrying the attempt number only from the second attempt. Cleared by start_run as well as forget_run. forget_run is reached by /resume, /fork, /clear and rewind but not by the operator typing a second prompt, so without this turn one's verdict is drawn over the whole of turn two. --- src/status.rs | 143 +++++++++++++++++++++ tests/status.rs | 332 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 475 insertions(+) diff --git a/src/status.rs b/src/status.rs index 82df576..4ee1317 100644 --- a/src/status.rs +++ b/src/status.rs @@ -326,6 +326,47 @@ 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, + /// 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 + /// and [`SPINNER`] is explicitly not a state: the standing has to survive + /// `--plain`, `NO_COLOR` and the ASCII glyph set, and a tick that means + /// *passed* only in green is a fact a monochrome terminal does not carry at + /// all. It is the choice 0.23.0 made for the resume marks, made again. + /// + /// **A `String` this file never interprets, and that is the decision rather + /// than the shortcut it looks like.** The words are io-harness's own — + /// `GateOutcome::as_str` spells the three outcomes a gate can end on, and the + /// driver supplies the one the harness has no outcome for, a criterion that + /// is being evaluated right now. Holding the gate's own type here would make + /// this line the second place that decides what a standing is called; + /// matching on the word to pick a tone would make it the third, and would + /// leave a literal in this file that a rename on the other side walks + /// silently past. So the word arrives composed and is drawn as it arrives — + /// the same rule the containment word is held to, and the reason no backend + /// name is written into this module either. + /// + /// `None` is a session with no criterion configured, which is nearly all of + /// them, and it draws **nothing at all** — not `none`, not an empty label, + /// not a zero. A turn nobody asked to verify has not failed verification. + /// + /// Cleared by [`Status::forget_run`] and by [`Status::start_run`]: a standing + /// is an account of the turn that was gated and of no other turn. + pub gate: Option, + /// Which attempt that standing was taken on, where more than one was made. + /// + /// **Absent below two, which is the absent-rather-than-zero rule sharpened by + /// one.** Every gate that ran at all ran once, so `attempt 1` would be on + /// every gated line in the world and would tell an operator nothing — the + /// argument [`Budgets::in_force`] already makes about io-cli's own step + /// floor, which is a cap on every turn and therefore a ceiling nobody chose. + /// What is worth a field is that the turn is being *retried*, and that starts + /// at the second attempt. + /// + /// `None` where the driver has no number to report, which is every session + /// with no gate and the first attempt of every session with one. The + /// rendering is [`Status::gate_field`]'s, so the threshold is decided once. + pub gate_attempt: Option, /// What this turn has drawn against the tree's shared ceiling, and what is /// left of it: `(drawn, remaining)`. /// @@ -502,6 +543,8 @@ impl Status { spend: None, plan: None, planning: false, + gate: None, + gate_attempt: None, jobs: 0, mcp: (0, 0), lsp: 0, @@ -569,6 +612,15 @@ impl Status { self.containment = None; self.spend = None; self.plan = None; + // **A standing belongs to the turn that was gated, and this is the field + // where keeping it would be worst.** Every other run fact left behind is + // a stale *number*; this one is a verdict on work that is no longer on + // the screen, so a `/resume` onto a conversation nobody ever gated would + // inherit `gate passed` and read as verified. The attempt count goes with + // it rather than separately: a retry count with no standing beside it is + // a number about nothing. + self.gate = None; + self.gate_attempt = None; // A handle belongs to the run that started it. Leaving the count behind // would have `/resume`, `/fork` and a rewind assert that another run's // jobs are still alive — and there would be no event left that could ever @@ -597,6 +649,19 @@ impl Status { // `run_tokens` above is held to: a turn that has not called a provider yet // has not cost zero, it has not cost anything anybody can report. self.cost = None; + // **Here as well as in `forget_run`, because `forget_run` does not run + // between two ordinary turns.** That method is reached by `/resume`, + // `/fork`, `/clear` and a rewind; the common case — the operator types a + // second prompt — reaches only this one, and a standing left behind by it + // would have the line asserting turn one's verdict over the whole of turn + // two, which is the per-turn field this codebase has already shipped + // stale once. + // + // A gate *retry* is not affected: the retries happen inside io-harness's + // own turn, under one `App` turn and therefore one `start_run`, so the + // failure that caused the retry is still on screen while the retry runs. + self.gate = None; + self.gate_attempt = None; } /// Set [`Status::cost`] from what this run has actually called, priced by @@ -884,6 +949,42 @@ impl Status { self.cost.map(crate::cost::money) } + /// Where the gate stands, drawn, or nothing at all. + /// + /// **One method, reached by both renderers**, which is the shape + /// [`Status::budgets_left`], [`Status::queued_left`] and + /// [`Status::cost_field`] already have and for the reason written where they + /// are: this file has shipped a field into one renderer twice — 0.8.0's spend + /// and 0.12.0's planning phase — and both times it was green in a unit test + /// and nowhere on screen, because the binary draws the footer on every + /// terminal seven rows or taller. + /// + /// `Option` rather than a `String` that is sometimes empty, so the absence is + /// decided here once and cannot be forgotten by a caller: both renderers push + /// whatever this returns and neither one asks about [`Status::gate`] itself. + /// + /// **The attempt is part of the same field rather than a second one**, because + /// the two are one fact — `failed` and `failed for the third time` are + /// different situations for an operator, and an `attempt 3` that could be + /// dropped away from the word it qualifies would be a number about nothing. + /// It costs ten cells and appears only from the second attempt, which is to + /// say on the lines where a turn is visibly not converging. + /// + /// **No tone is chosen here and none is chosen by the callers.** Both draw the + /// word in `Tone::Normal`, the tone the planning phase and the background-job + /// count wear, because deciding between them would mean this module comparing + /// the standing against literal outcome words that belong to io-harness — see + /// [`Status::gate`]. Nothing is lost by it: the rule this line has held since + /// its first release is that the word is the state and a colour only ever + /// agrees with it, so a standing spelled out in full needs no second channel. + pub fn gate_field(&self) -> Option { + let standing = self.gate.as_ref()?; + Some(match self.gate_attempt { + Some(attempt) if attempt > 1 => format!("gate {standing} attempt {attempt}"), + _ => format!("gate {standing}"), + }) + } + /// The fields, most important first. pub fn fields(&self, theme: &Theme) -> Vec { // The WORD is the state, and the animation is only beside it. A spinner @@ -996,6 +1097,25 @@ impl Status { if self.planning { fields.push(Field::new("planning".to_string(), 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 + // agent writing outranks what the last turn spent; a gate that has not + // passed is the same class of fact from the other end — it is why the + // turn is not finished, and on a retry it is why the agent is doing the + // work a second time. If a narrow terminal can hold one of `gate failed + // attempt 2` and `14.2k tok`, it should hold the one that explains what + // is happening rather than the one that measures it. + // + // Right of `planning` rather than left, because `planning` is a standing + // choice that holds until `/plan off` while this is an account of one + // turn: where both are on the line and only one can survive, the one that + // is still true after the turn ends is the one that stays. + // + // `Normal` rather than `Muted`, for the reason `planning` and `bg N` are: + // it is a fact about what the turn is still doing, not a footnote about + // what it did. + fields.extend(self.gate_field().map(|text| Field::new(text, Tone::Normal))); // Beside the token count rather than anywhere else, because the two are // the same kind of fact — what this run has spent — and they used to sit // together in the `Finished` row this release removed. @@ -1172,6 +1292,20 @@ impl Status { // there is something to count, so a session that has run nothing carries // an almost empty row rather than a row of zeroes. let mut counts: Vec = Vec::new(); + // **First in the group, and that is the same ordering decision + // `Status::fields` makes, arrived at through this row's own mechanism.** + // The narrowing below pops counters off the right until the right-hand + // group fits, so position in this vector *is* priority — and the standing + // that says why the turn is not finished outranks every number beside it, + // exactly as the planning phase outranks them in the group on the right. + // Pushed after `steps` it would have been among the first things a + // crowded row gave up, which is the inversion the release before this one + // spent a live capture finding. + // + // Here as well as on `Status::line` and out of the same method: this is + // the row the binary draws at an ordinary prompt, so a gate added to + // `line` alone would be a gate no operator ever saw. + counts.extend(self.gate_field()); if let Some(steps) = self.steps { counts.push(format!("{steps} step{}", if steps == 1 { "" } else { "s" })); } @@ -1787,6 +1921,15 @@ pub fn committed( // a reader cannot trust is worse than none. The line and the footer redraw, // so they are where a depth that changes belongs — which is also why N3 names // those two renderers and not this one. + // + // **`Status::gate` is not a row here either, and for that same argument.** A + // standing is the verdict on one turn: `gate failed attempt 2` committed into + // a scrollback goes on saying so under the passing turn that follows it, and a + // row that is false a turn later on a page whose whole claim is that it can be + // trusted is worse than no row. What a reader wants from *this* page about a + // gate is the criterion the next turn will be held to, which is configuration + // and belongs beside the budgets — a row this release does not draw rather + // than one it draws wrongly. // Three of whatever the set draws a rule with, at both ends — the same edge // `crate::transcript` gives a committed conversation, and for the same diff --git a/tests/status.rs b/tests/status.rs index 6947ecb..e88a87f 100644 --- a/tests/status.rs +++ b/tests/status.rs @@ -2332,3 +2332,335 @@ fn the_cost_field_is_drawn_by_the_line_and_by_the_footer_alike() { ); } } + +// --------------------------------------------------------------------------- +// 0.24.0 — the verification gate's standing on the status line. +// --------------------------------------------------------------------------- + +/// A gated turn that failed its criterion and is being tried again. +/// +/// The standing is a word the driver hands over rather than one this test +/// invents a type for: `Status::gate` is plain data, so what a test can state +/// about it is exactly what an operator would see. +fn gated(standing: &str, attempt: Option) -> Status { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.gate = Some(standing.to_string()); + status.gate_attempt = attempt; + status +} + +/// **The standing reaches the terminal through BOTH renderers, out of one +/// method.** +/// +/// `Status::line` is the one-row form and has exactly one production caller — +/// the fallback for a terminal under seven rows. `Status::footer` is the +/// three-row form `Status::render` takes on everything taller, which is every +/// real terminal. This file has shipped a field into one of them twice: 0.8.0's +/// spend and 0.12.0's planning phase were each added to `line`, asserted against +/// `line`, went green, and were nowhere on screen in a live capture. +/// +/// The spelling is asserted against `Status::gate_field` rather than against a +/// literal repeated per arm, so the one place that composes it is the one place a +/// rename has to touch — a renderer that grew its own `format!` would fail here +/// even if both forms happened to say something. +/// +/// **And the standing is a word, so it survives every set.** The line is read in +/// `MONO` as well, which is the theme `NO_COLOR` and `--plain` resolve to: a +/// standing carried by a colour or a tick is a standing those readers do not +/// have. The whole field is ASCII, so no glyph set can change it. +/// +/// Sabotage: push `self.gate_field()` into `Status::fields` alone and drop the +/// `counts.extend(self.gate_field())` from the footer — under which the +/// `Status::line` arm below stays green and the binary shows nothing. +#[test] +fn the_gate_standing_is_drawn_by_the_line_and_by_the_footer_alike() { + for standing in ["passed", "failed", "errored", "running"] { + let status = gated(standing, None); + let spelling = status + .gate_field() + .expect("a configured criterion has a standing"); + assert_eq!( + spelling, + format!("gate {standing}"), + "the standing is spelled once, plainly, and in the harness's own word", + ); + for (renderer, text) in both_renderers(&status) { + assert!( + text.contains(&spelling), + "{renderer} does not say where the gate stands: {text:?}", + ); + } + } + + // Every character of it is ASCII, which is what makes the claim above true + // of a terminal that cannot draw braille as well as of one that can. + let status = gated("failed", Some(3)); + let spelling = status.gate_field().expect("a standing"); + assert!( + spelling.is_ascii(), + "the standing is not readable under the ASCII set: {spelling:?}", + ); + let mono = status.line(200, &MONO).to_string(); + assert!( + mono.contains(&spelling), + "the standing is gone from a terminal with no colour: {mono:?}", + ); +} + +/// **A gate with nothing behind it is absent rather than zero.** +/// +/// Most sessions configure no criterion at all, and one of them has not passed a +/// gate zero times — it has not been gated. So the field draws nothing: not +/// `none`, not `gate 0`, not an empty label with a separator either side of it, +/// which on this line is the same defect wearing a space. It is the rule +/// `tokens`, `spend`, `bg N` and the queue depth are already held to. +/// +/// Sabotage: render `self.gate.clone().unwrap_or_default()` — under which every +/// ungated session carries a bare `gate` with nothing after it, and only this +/// test fails. +#[test] +fn the_gate_field_with_nothing_behind_it_is_absent_rather_than_zero() { + let status = Status::new("opus-5"); + assert_eq!( + status.gate_field(), + None, + "a session nobody asked to verify composed a standing", + ); + for (renderer, text) in both_renderers(&status) { + assert!( + !text.contains("gate"), + "{renderer} drew a gate on a session with no criterion: {text:?}", + ); + assert!( + !text.contains("attempt"), + "{renderer} counted attempts at a gate that does not exist: {text:?}", + ); + } + + // An attempt number with no standing beside it is a number about nothing, + // and it must not summon the field on its own — this is the state a driver + // leaves behind if it reports the retry before the verdict. + let mut counted = Status::new("opus-5"); + counted.gate_attempt = Some(2); + assert_eq!( + counted.gate_field(), + None, + "an attempt count drew a gate that has no standing", + ); + for (renderer, text) in both_renderers(&counted) { + assert!( + !text.contains("attempt"), + "{renderer} drew a retry count with no verdict behind it: {text:?}", + ); + } +} + +/// **The attempt number appears from the second attempt and never on the first.** +/// +/// Every gate that ran at all ran once, so `attempt 1` would be on every gated +/// line in the world and would tell an operator nothing — the argument +/// `Budgets::in_force` already makes about io-cli's own step floor, which is a cap +/// on every turn and therefore a ceiling nobody chose. What is worth the ten cells +/// is that the turn is being *retried*: the criterion did not hold, the retry +/// budget is being spent, and the agent is doing the work again. +/// +/// The first arm asserts the absence of the word `attempt` outright rather than of +/// the string `attempt 1`, because `attempt 1` is absent from a line reading +/// `attempt 12` and an assertion satisfied by the bug is not an assertion. +/// +/// Sabotage: drop the `attempt > 1` guard — under which every gated line carries +/// `attempt 1`, the first arm of this test fails, and nothing else in the +/// repository does. +#[test] +fn the_gate_attempt_is_drawn_only_from_the_second_attempt() { + for first in [None, Some(1)] { + let status = gated("failed", first); + assert_eq!( + status.gate_field().as_deref(), + Some("gate failed"), + "the first attempt is every gate's first attempt and is not news", + ); + for (renderer, text) in both_renderers(&status) { + assert!( + !text.contains("attempt"), + "{renderer} numbered the attempt nobody needed numbering: {text:?}", + ); + } + } + + let status = gated("failed", Some(2)); + let spelling = status.gate_field().expect("a retried gate has a standing"); + assert_eq!( + spelling, "gate failed attempt 2", + "the retry is stated in words beside the standing it belongs to", + ); + for (renderer, text) in both_renderers(&status) { + assert!( + text.contains(&spelling), + "{renderer} does not say the turn is being retried: {text:?}", + ); + } + + // And it keeps counting rather than sticking at two, which is what a + // hard-coded second attempt would look like from the outside. + assert_eq!( + gated("errored", Some(7)).gate_field().as_deref(), + Some("gate errored attempt 7"), + ); +} + +/// **A standing belongs to the turn that was gated and outlives no other.** +/// +/// `Status::forget_run` clears the per-run facts when the conversation under the +/// line changes — `/resume` onto another session, `/fork` away from this one, a +/// rewind that undoes the turn that set one — and `Status::start_run` clears them +/// when the operator simply types again, which is the path `forget_run` never +/// sees. Both matter here for different reasons: a resumed conversation nobody +/// ever gated would otherwise inherit `gate passed` and read as verified, and an +/// ordinary second turn would spend its whole length under the first turn's +/// verdict. This codebase has shipped exactly that with a per-turn field before. +/// +/// The planning phase is set beside it so the test states the *difference* rather +/// than half of it: a standing choice survives both, an account of one turn +/// survives neither. +/// +/// Sabotage: clear `gate` and leave `gate_attempt` standing — under which a later +/// verdict on a later turn is drawn with the retry count of an earlier one, and +/// the last assertion of each half is what catches it. +#[test] +fn the_gate_standing_does_not_outlive_the_turn_that_was_gated() { + for clear in [Status::forget_run as fn(&mut Status), Status::start_run] { + let mut status = gated("failed", Some(2)); + // A standing choice, which is not a fact about the run and must survive. + status.planning = true; + for (renderer, text) in both_renderers(&status) { + assert!( + text.contains("gate failed attempt 2"), + "{renderer} never drew the standing this test is about: {text:?}", + ); + } + + clear(&mut status); + + assert_eq!(status.gate, None, "the verdict outlived its turn"); + assert_eq!( + status.gate_attempt, None, + "the retry count outlived the verdict it counted", + ); + assert!( + status.planning, + "a standing choice was cleared with the run" + ); + for (renderer, text) in both_renderers(&status) { + assert!( + !text.contains("gate"), + "{renderer} still reports the previous turn as gated: {text:?}", + ); + assert!( + !text.contains("attempt"), + "{renderer} kept the previous turn's retry count: {text:?}", + ); + assert!( + text.contains("planning"), + "{renderer} lost the mode the gate was not: {text:?}", + ); + } + } +} + +/// **The standing survives a width the counters do not, and the line still never +/// wraps.** +/// +/// Both renderers drop from the right, so position is priority, and the module +/// states which way this one goes: a standing mode that stops the agent writing +/// outranks what the last turn spent. A gate that has not passed is that fact from +/// the other end — it is *why* the turn is not finished, and on a retry it is why +/// the agent is doing the work a second time. So it sits right of the planning +/// phase, which is a choice that holds after the turn ends, and left of every +/// counter, which is an account of what the turn cost. +/// +/// The order is asserted on `Status::fields` directly as well as through a +/// rendered width, because an arithmetic assertion alone would move the day +/// somebody adds a field five cells wider, and the claim being made is about +/// order rather than about a hundred columns. +/// +/// Sabotage: push the gate after `ctx` in `Status::fields` and after `steps` in +/// the footer's counts — under which both forms still draw it at two hundred +/// columns, every other test in this file stays green, and the one field that +/// explains a turn that will not end is the first thing a narrow terminal gives +/// up. +#[test] +fn the_gate_standing_survives_a_width_the_counters_do_not() { + let mut status = full_row(); + status.gate = Some("failed".into()); + status.gate_attempt = Some(2); + + // Order first, where it is a claim about the list rather than about a width. + let fields = status.fields(&DARK); + let index = |needle: &str| { + fields + .iter() + .position(|field| field.text.contains(needle)) + .unwrap_or_else(|| panic!("no field says {needle:?}: {fields:?}")) + }; + assert!( + index("planning") < index("gate "), + "the standing outranked a choice that outlives the turn: {fields:?}", + ); + assert!( + index("gate ") < index("6 steps") + && index("gate ") < index("26.9k tok") + && index("gate ") < index("ctx 23%"), + "a counter outranked the reason the turn is not finished: {fields:?}", + ); + + // Wide enough for everything, so the narrowing below is the only thing the + // arms after it can be failing on. + let wide = rendered(&status, 200); + for fact in ["gate failed attempt 2", "6 steps", "26.9k tok", "ctx 23%"] { + assert!(wide.contains(fact), "{fact:?} was never drawn: {wide:?}"); + } + + // A hundred columns, which is the width the 0.21.0 live capture was taken at + // and the one an ordinary terminal is most likely to be. + let narrowed = rendered(&status, 100); + assert!(narrowed.chars().count() <= 100, "{narrowed:?}"); + assert!(!narrowed.contains('\n'), "the line wrapped: {narrowed:?}"); + assert!( + narrowed.contains("gate failed attempt 2"), + "the standing went before the counters it outranks: {narrowed:?}", + ); + for counter in ["6 steps", "26.9k tok", "ctx 23%"] { + assert!( + !narrowed.contains(counter), + "nothing was dropped, so this width proves nothing: {narrowed:?}", + ); + } + + // The footer narrows by its own mechanism — counters come off the right of + // the counts group until the right-hand group fits — so the same claim has to + // be made against it separately or half of it is untested. + let footer = footed(&status, 100); + assert!( + footer.contains("gate failed attempt 2") && footer.contains("planning"), + "the footer gave up the standing to keep a counter: {footer:?}", + ); + assert!( + !footer.contains("6 steps"), + "the footer row fitted whole, so it proves nothing: {footer:?}", + ); + + // And at a width that holds one field, it is still the model. Everything this + // task adds goes before it and after everything else. + let cramped = rendered(&status, 40); + assert!(cramped.chars().count() <= 40, "{cramped:?}"); + assert!(!cramped.contains('\n'), "the line wrapped: {cramped:?}"); + assert!( + cramped.contains("anthropic/claude-sonnet-4.5"), + "the model is the last field to go: {cramped:?}", + ); + assert!( + !cramped.contains("gate"), + "the gate outlasted the model: {cramped:?}", + ); +} From 4b6fb059255662fb334478731bf9a7ff4f0ae5a8 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:51 +0530 Subject: [PATCH 05/16] feat(gates): draw what the gate did Routes gate_phase_failed and gate_output, which src/triage.rs has named since 0.20.0 as belonging to a gate no session had yet. A failing review no longer draws in Tone::Refused. That tone's word is refused, which is the permission boundary's word, and it conflated the policy declining to run a gate with the work not meeting the bar. Those need opposite responses, and a test now asserts the two vocabularies stay disjoint. --- src/events.rs | 101 +++++++++++++----- src/triage.rs | 18 ++-- tests/events.rs | 276 ++++++++++++++++++++++++++++++++++++++++++++---- tests/triage.rs | 47 +++++++++ 4 files changed, 394 insertions(+), 48 deletions(-) diff --git a/src/events.rs b/src/events.rs index 014922a..f6ff761 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1299,19 +1299,36 @@ impl Events { } // The reasons are carried rather than summarised, for io-harness's own // reason: a refusal a human cannot argue with is a gate nobody trusts - // twice. + // twice. They are the operator's only account of why the work was + // judged insufficient — the store has the same sentence joined by + // semicolons, and a session that dropped them would be sending + // somebody to a database to read a paragraph the run already said. + // + // **The failing verdict is a warning and not `Tone::Refused`, from + // 0.24.0.** That tone carries the literal word `refused`, which + // everywhere else in this transcript means the permission boundary + // stopped an act; a reviewer that read the work and judged it did not + // stop anything, and giving both the same word would make the two + // things an operator most needs to tell apart — the policy would not + // run my gate, versus my work did not meet the bar — read identically. + // + // **A review that did not happen is not drawn here, because it is not + // emitted.** io-harness sends this event only for a verdict somebody + // gave; a transport failure or an unparseable answer emits nothing and + // is `GateOutcome::Errored` in the store. So this line always means a + // review that ran, and its absence is not evidence that one passed. EventKind::Reviewed { passed, reasons } => { let mut lines = self.flush_text(); lines.push(theme.notice( if *passed { Tone::Success } else { - Tone::Refused + Tone::Warning }, if *passed { "the review passed" } else { - "the review did not pass" + "the review ran and did not pass" }, )); for reason in reasons { @@ -1357,28 +1374,54 @@ impl Events { // `triage::TRIAGE` records that route. Vec::new() } - // **What isolated the work, said in the operator's words rather than - // inferred from a tool cell that happened to succeed.** Seven kinds - // reach this channel and four of them are drawn. + // **What isolated the work, and how the criterion it was isolated + // for answered, said in the operator's words rather than inferred + // from a tool cell that happened to succeed.** Seven kinds reach this + // channel and six of them are drawn. // - // **`dial` is not one of them.** io-harness builds it as a `destroy` - // event with the kind overwritten and emits it immediately beside - // `EventKind::Dialed` for the same outbound connection, so a session - // drawing both would put every dial in the transcript twice — and - // the copy here is the poorer one, carrying the word and nothing + // **`dial` is the one that is not.** io-harness builds it as a + // `destroy` event with the kind overwritten and emits it immediately + // beside `EventKind::Dialed` for the same outbound connection, so a + // session drawing both would put every dial in the transcript twice — + // and the copy here is the poorer one, carrying the word and nothing // else where the dial itself carries the host, the port and the - // verdict. `gate_phase_failed` and `gate_output` are the other two: - // they belong to an execution gate, and every contract this crate - // builds — see `crate::contract` — leaves `Verification::None` on it, - // because nothing here ever calls `with_verification`. So - // neither can arrive in a session before 0.24.0 gives one a gate, - // and neither is given a sentence written in advance of the release - // that could check it. - // - // An empty `Vec` is this module's own word for "nothing yet" and is - // the honest way to say all three: a caller cannot tell it from a + // verdict. An empty `Vec` is this module's own word for "nothing yet" + // and is the honest way to say it: a caller cannot tell it from a // dropped event because there is nothing there to tell apart. // + // **`gate_phase_failed` and `gate_output` are drawn from 0.24.0.** + // Before this release every contract this crate built left + // `Verification::None` on it, so neither kind could arrive in a + // session and neither was given a sentence written in advance of the + // release that could check it. An operator now configures one + // criterion — `[app.io-cli.gates]`, resolved by `crate::gates` — + // io-harness runs it after the agent stops, and these two are the + // whole of what it says about a criterion that did not hold on the + // channel a session is already watching. A verdict that lives only in + // the store is a verdict nobody reads. + // + // **Neither line carries the fact that made it worth emitting, and + // that is io-harness's shape rather than a choice made here.** The + // failing phase and the failing command's bounded output are both in + // `SandboxEvent::detail`, and `EventKind::Sandbox` carries the kind + // and the backend alone. So a session learns that the gate ran and + // said no, and the text stays in the run's `sandbox_events` rows + // where a diagnosis reads it. Naming a phase here, or printing an + // empty string as though it were the command's output, would be this + // module writing the half it was not given. + // + // **A gate that did not hold is not a refusal, and neither line says + // that word.** `Tone::Refused` in this transcript means the + // permission boundary stopped an act — which is exactly what a + // criterion whose program the policy will not run produces: an + // `EventKind::Refused` with act `exec`, and no gate event at all, + // because nothing ran and nothing judged anything. That is + // `GateOutcome::Errored` in the store, the one verdict io-harness + // will retry. A gate line therefore only ever stands for a criterion + // that ran and answered, and the two need opposite responses from + // whoever is reading — fix the policy, or fix the work — so they are + // told apart by the words and never by a colour. + // // **`cap_hit` is a limit reached and not a failure.** The sandbox // did exactly what its configuration told it to; reporting that // through the error path would tell an operator their run broke at @@ -1400,6 +1443,11 @@ impl Events { "exec" => ("a command ran in the sandbox", Tone::Muted), "cap_hit" => ("the sandbox reached a limit it was given", Tone::Warning), "destroy" => ("the sandbox was torn down", Tone::Muted), + // `ran and` is load-bearing: it is the whole of what + // separates a criterion that judged the work from one that + // never got to. + "gate_phase_failed" => ("the gate ran and did not pass", Tone::Warning), + "gate_output" => ("the gate command printed output", Tone::Muted), _ => return Vec::new(), }; let mut text = said.to_string(); @@ -1747,9 +1795,14 @@ fn format_millis(duration: Duration) -> String { /// The vocabulary is io-harness's, and the distinction that matters here is /// between `success` and `finished`: `success` means a verification criterion /// passed, and `finished` means a run with no criterion ended on its own terms. -/// **Every io-cli turn ends `finished`**, because every contract this crate -/// builds — see [`crate::contract`] — leaves `Verification::None` on it: nothing -/// here calls `with_verification`, on either arm. +/// +/// **Both are reachable from 0.24.0, and until this release only one was.** Every +/// contract this crate built left `Verification::None` on it, so every io-cli +/// turn ended `finished` and `success` could not be produced from this interface +/// at all. An operator now configures one criterion — see [`crate::gates`] — and +/// a turn that carries one ends `success` when it holds. Neither word is a +/// failure and both are drawn as the same thing; what separates them is how much +/// the run is claiming, and that claim is the operator's to read. /// /// Found in a live run, not in the suite: treating anything that was not /// `success` as a warning meant every ordinary, completely successful turn ended diff --git a/src/triage.rs b/src/triage.rs index b376a29..8b775f0 100644 --- a/src/triage.rs +++ b/src/triage.rs @@ -128,11 +128,14 @@ pub const TRIAGE: &[(&str, Disposition, &str)] = &[ ( "sandbox", Disposition::Line, - "the sandbox line, for the four kinds an ordinary session can see — `create`, `exec`, \ - `cap_hit` and `destroy`. The other three reach no line and each has its own route: \ - `dial` is drawn by `dialed`, which carries the host, the port and the verdict this kind \ - does not, and `gate_phase_failed` and `gate_output` belong to a verification gate no \ - session has until 0.24.0", + "the sandbox line, for six of the seven kinds. `create`, `exec`, `cap_hit` and `destroy` \ + say what isolated the work; `gate_phase_failed` and `gate_output` say that the criterion \ + the operator configured under `[app.io-cli.gates]` ran and did not hold, which from \ + 0.24.0 is a thing a session can see. `dial` is the one kind that reaches no line: it is \ + drawn by `dialed`, which carries the host, the port and the verdict this kind does not. \ + Neither gate line names the phase or quotes the output — `EventKind::Sandbox` carries \ + the kind and the backend alone, and the `detail` holding both stays in the run's own \ + `sandbox_events` rows, which is where a diagnosis reads it", ), ( "mcp", @@ -155,7 +158,10 @@ pub const TRIAGE: &[(&str, Disposition, &str)] = &[ ( "reviewed", Disposition::Line, - "the verdict line and its reasons", + "the verdict line and its reasons, in the reviewer's own words. Emitted only for a review \ + that happened: one that could not run emits nothing here at all and is recorded as \ + `GateOutcome::Errored`, so an absent verdict line is a review that never answered rather \ + than one that said yes", ), ( "routed", diff --git a/tests/events.rs b/tests/events.rs index 8e40f87..34bde31 100644 --- a/tests/events.rs +++ b/tests/events.rs @@ -414,10 +414,13 @@ fn a_turn_that_ended_well_does_not_end_the_transcript_with_a_warning() { use io_cli::events::outcome_tone; use io_cli::theme::Tone; - // `finished` is what EVERY io-cli turn returns: a steerable turn is built on - // TaskContract::workspace, which carries Verification::None, so there is no - // criterion to pass and `success` is unreachable from this interface. Reading - // it as a warning was a real defect and a live run is what found it. + // Two words for a turn that ended well, and both are reachable from 0.24.0. + // `finished` is a turn with no criterion ending on its own terms, which was + // every io-cli turn until this release: a contract built here carried + // Verification::None and `success` could not be produced from this interface + // at all. A turn whose operator configured a gate — `io_cli::gates` — ends + // `success` when it holds. Reading either as a warning was a real defect and + // a live run is what found it. assert_eq!(outcome_tone("finished"), Tone::Success); assert_eq!(outcome_tone("success"), Tone::Success); @@ -2122,9 +2125,9 @@ fn f8_a_sandbox_draws_its_four_kinds_and_carries_a_backend_only_where_one_exists /// it immediately beside `EventKind::Dialed` for the same connection, so a /// session drawing both would put every dial in the transcript twice — and the /// copy here is the poorer one, carrying the word and neither the host, the port -/// nor the verdict. `gate_phase_failed` and `gate_output` are the other two that -/// draw nothing: they belong to an execution gate, and a session has none until -/// 0.24.0. +/// nor the verdict. It is now the **only** kind of the seven that draws nothing: +/// `gate_phase_failed` and `gate_output` stood beside it here until 0.24.0 gave a +/// session a criterion to fail, and the test below is where they went. /// /// Sabotage: give `"dial"` a sentence of its own in the `Sandbox` arm, under /// which only this fails, on one dial arriving as two rows that disagree about @@ -2132,19 +2135,77 @@ fn f8_a_sandbox_draws_its_four_kinds_and_carries_a_backend_only_where_one_exists #[test] fn f8_the_dial_kind_of_a_sandbox_event_is_left_to_the_dial_itself() { let mut events = Events::new(DARK); - for kind in ["dial", "gate_phase_failed", "gate_output"] { - let lines = events.event( - &event(EventKind::Sandbox { - kind: kind.into(), - backend: None, - }), - Duration::ZERO, + let lines = events.event( + &event(EventKind::Sandbox { + kind: "dial".into(), + backend: None, + }), + Duration::ZERO, + ); + assert!(lines.is_empty(), "dial drew a line: {lines:?}"); + // Drawn nowhere is not the same as undecided. `sandbox` has a disposition, so + // this does not reach the counter that exists for a kind nobody has decided + // about. + assert_eq!( + events.unknown(), + 0, + "a kind the table holds was counted as one this release has never seen", + ); +} + +/// **0.24.0 — a gate that ran and said no reaches the screen.** +/// +/// Until this release both kinds returned an empty `Vec` from the `Sandbox` arm, +/// which was correct while no contract this crate built carried a criterion: an +/// event that cannot arrive needs no sentence. It can arrive now, and a verdict +/// that lives only in the store is a verdict nobody reads. +/// +/// **Neither line can carry the phase or the output**, and that is asserted here +/// rather than merely commented: `EventKind::Sandbox` has `kind` and `backend` +/// and nothing else, while `SandboxEvent::detail` — which holds the failing phase +/// for one and the command's bounded output for the other — never reaches this +/// channel. A line that named a phase would have invented it. +/// +/// Sabotage: put either kind back on the `_ => return Vec::new()` arm. Only this +/// fails, and it fails on the line's absence with `Events::unknown` still at +/// zero — a triaged kind is never counted there — which is a gate that judged the +/// work and told nobody. +#[test] +fn a_gate_that_ran_and_did_not_pass_says_so_on_the_channel_a_session_is_watching() { + let mut events = Events::new(DARK); + + let phase = rendered( + &mut events, + EventKind::Sandbox { + kind: "gate_phase_failed".into(), + backend: None, + }, + ); + // **That it ran is the load-bearing half.** "did not pass" alone is the same + // sentence a criterion that never executed would deserve, and those two need + // opposite responses from whoever reads the row. + assert!(phase.contains("ran and"), "{phase:?}"); + assert!(phase.contains("did not pass"), "{phase:?}"); + + let output = rendered( + &mut events, + EventKind::Sandbox { + kind: "gate_output".into(), + backend: None, + }, + ); + assert!(!output.trim().is_empty(), "gate_output drew nothing"); + assert!(output.contains("printed output"), "{output:?}"); + + // Neither event carries a backend and neither line may name one — the same + // rule `cap_hit` and `destroy` are already held to. + for line in [&phase, &output] { + assert!( + !line.contains("sandbox-exec") && !line.contains("none"), + "a gate event carries no backend and this line invented one: {line:?}", ); - assert!(lines.is_empty(), "{kind} drew a line: {lines:?}"); } - // Drawn nowhere is not the same as undecided. `sandbox` has a disposition, - // so none of these reaches the counter that exists for a kind nobody has - // decided about. + assert_eq!( events.unknown(), 0, @@ -2152,6 +2213,185 @@ fn f8_the_dial_kind_of_a_sandbox_event_is_left_to_the_dial_itself() { ); } +/// **0.24.0 — a criterion that answered and one that never ran do not read +/// alike.** +/// +/// `GateOutcome::Failed` and `GateOutcome::Errored` are the two verdicts io-cli +/// has to keep apart on screen, and only one of them has an event: a gate that +/// ran and said no emits `gate_phase_failed`, while a gate whose program the +/// policy would not run emits `EventKind::Refused` with act `exec` and no gate +/// event at all — nothing executed, so nothing judged anything. io-harness will +/// retry the second and never the first. +/// +/// So the words have to be disjoint, which is what is asserted: the refusal keeps +/// `refused` and never claims a criterion ran, and the gate line never borrows the +/// word the permission boundary owns. A reader who cannot tell them apart fixes +/// the wrong thing — the policy, or the work. +/// +/// Sabotage: draw `gate_phase_failed` through `Tone::Refused`, which is the tone +/// the failing review carried until this release. The line reads `refused: …`, +/// only this test fails, and it fails on the two facts having become one word. +#[test] +fn a_gate_that_could_not_run_does_not_read_like_a_gate_that_ran_and_failed() { + let mut events = Events::new(DARK); + + // Errored: the policy refused the criterion's program. This is the whole of + // what the stream says about it — there is no gate event behind it. + let errored = rendered( + &mut events, + EventKind::Refused { + act: "exec".into(), + target: "cargo".into(), + rule: Some("exec.deny".into()), + layer: Some("workspace".into()), + }, + ); + assert!(errored.contains("refused"), "{errored:?}"); + assert!( + !errored.contains("did not pass"), + "a criterion that never ran did not fail: {errored:?}", + ); + + // Failed: it ran, and it said no. + let failed = rendered( + &mut events, + EventKind::Sandbox { + kind: "gate_phase_failed".into(), + backend: None, + }, + ); + assert!( + !failed.contains("refused"), + "the permission boundary owns that word and nothing stopped this gate: {failed:?}", + ); + // Nor the error path. Nothing broke: a criterion that holds the work to a + // standard and reports that it was not met did its job. + assert!(!failed.contains("error"), "{failed:?}"); + assert_ne!(errored, failed); +} + +/// **0.24.0 — a review's reasons reach the scrollback, in the reviewer's words.** +/// +/// The reasons are the operator's only account of why the work was judged +/// insufficient. The store keeps the same sentences joined by semicolons, so a +/// session that dropped them would be sending somebody to a database to read a +/// paragraph the run had already said out loud. +/// +/// **The failing verdict is a warning and not a refusal**, which is the half that +/// changed this release. `Tone::Refused` carries the literal word `refused`, and +/// everywhere else in this transcript that word means the permission boundary +/// stopped an act. A reviewer that read the work and judged it stopped nothing. +/// +/// Sabotage: restore `Tone::Refused` on the failing arm. Only this fails, on a +/// judged verdict and a blocked act arriving under one word. +#[test] +fn a_review_carries_its_reasons_and_a_failing_verdict_is_not_a_refusal() { + let mut events = Events::new(DARK); + + let failed = rows(events.event( + &event(EventKind::Reviewed { + passed: false, + reasons: vec![ + "the new branch has no test".into(), + "README still documents the old flag".into(), + ], + }), + Duration::ZERO, + )); + let said = failed.join("\n"); + assert!(said.contains("the review ran and did not pass"), "{said:?}"); + // Every reason, whole. Asserted per row rather than over the join, so a + // second reason silently swallowed by the first cannot pass. + assert!( + failed + .iter() + .any(|row| row.contains("the new branch has no test")), + "{failed:?}", + ); + assert!( + failed + .iter() + .any(|row| row.contains("README still documents the old flag")), + "{failed:?}", + ); + assert!( + !said.contains("refused"), + "a review that read the work and judged it stopped nothing: {said:?}", + ); + + let passed = flatten(events.event( + &event(EventKind::Reviewed { + passed: true, + reasons: Vec::new(), + }), + Duration::ZERO, + )); + assert!(passed.contains("the review passed"), "{passed:?}"); + assert!( + !passed.contains("did not"), + "the two verdicts must not be one sentence apart from a negation this reader \ + can miss: {passed:?}", + ); +} + +/// **F5 for the lines this release added.** Nothing outside ASCII reaches the +/// terminal under the ASCII set, and the meaning survives the substitution. +/// +/// Swept by `char::is_ascii` over the whole rendered output rather than by +/// looking for the marks this release happens to know about — `tests/glyphs.rs` +/// says why: a `contains` over the code points somebody remembered is green the +/// day a twelfth one is typed into a new line. The review's reasons are the rows +/// that carry a glyph here, through `Glyphs::bullet`. +/// +/// Plain mode is the other axis and is asserted beside it, because a plain +/// session is the one that has no status line to fall back on: if a gate verdict +/// were dropped there it would reach nobody at all. +#[test] +fn the_gate_and_review_lines_survive_the_ascii_set_and_plain_mode() { + use io_cli::glyphs::ASCII; + + for plain in [false, true] { + let mut events = Events::new(DARK.with_glyphs(ASCII)); + events.set_plain(plain); + + let mut said = String::new(); + for kind in [ + EventKind::Sandbox { + kind: "gate_phase_failed".into(), + backend: None, + }, + EventKind::Sandbox { + kind: "gate_output".into(), + backend: None, + }, + EventKind::Reviewed { + passed: false, + reasons: vec!["the diff does not build".into()], + }, + ] { + said.push_str(&rendered(&mut events, kind)); + said.push('\n'); + } + + assert!( + said.chars().all(|c| c.is_ascii()), + "a character the ASCII set cannot draw reached the terminal in \ + plain={plain}: {said:?}", + ); + // And the meaning survived it. A set that mapped every mark to a space + // would pass the sweep above and destroy the product. + assert!( + said.contains("ran and did not pass"), + "plain={plain}: {said:?}" + ); + assert!(said.contains("printed output"), "plain={plain}: {said:?}"); + assert!( + said.contains("the diff does not build"), + "plain={plain}: {said:?}", + ); + } +} + /// 0.14.0 F9 — a stalled agent is on screen before anybody interrupts it. /// /// `EventKind::Stalled` is a unit variant carrying nothing at all, so the line is diff --git a/tests/triage.rs b/tests/triage.rs index 7322619..92f6cce 100644 --- a/tests/triage.rs +++ b/tests/triage.rs @@ -145,6 +145,53 @@ fn the_three_kinds_this_release_draws_are_lines_rather_than_silent() { } } +/// **0.24.0.** The `sandbox` route said `gate_phase_failed` and `gate_output` +/// "belong to a verification gate no session has until 0.24.0". This is that +/// release. +/// +/// The route column is the only evidence anybody has that a fact reaches its +/// reader, so a sentence in it that has stopped being true is worse than a blank +/// one: a reviewer checking whether a kind is drawn reads this and stops. The +/// seam was left open here deliberately, and closing it means closing the +/// sentence that described it as open. +/// +/// Sabotage: restore either retired phrase to the row. Only this fails, and it +/// fails on the claim rather than on the sentence around it — which is the part +/// an author rewrites while leaving the claim standing. +#[test] +fn the_route_column_no_longer_says_a_session_has_no_verification_gate() { + let sandbox = triage::route("sandbox").expect("the sandbox row"); + for retired in ["no session has", "until 0.24.0"] { + assert!( + !sandbox.contains(retired), + "the sandbox route still says {retired:?}, which stopped being true in this \ + release: {sandbox}", + ); + } + // And says where the two of them go now, which is the whole job of this + // column: a `Line` disposition covers seven kinds at once, so the row is the + // only place that can record which of them the arm actually draws. + for kind in ["gate_phase_failed", "gate_output"] { + assert!( + sandbox.contains(kind), + "{kind} is drawn from this release and the route does not say so: {sandbox}", + ); + } + // `dial` is the one kind of the seven that still reaches no line, so the row + // has to go on naming the event that draws it instead. + assert!(sandbox.contains("dialed"), "{sandbox}"); + + // The asymmetry a reader of this table cannot work out from the enum: a + // review that never happened emits nothing at all, so a missing verdict line + // is a criterion that did not answer rather than one that said yes. + let reviewed = triage::route("reviewed").expect("the reviewed row"); + assert!( + reviewed.contains("Errored"), + "the reviewed route does not record that a review which never ran emits \ + nothing here: {reviewed}", + ); +} + /// The other direction, in behaviour rather than in source: a kind whose fact /// belongs to a status field or to another event commits no line. /// From f65deb8d216e8a2397023145f3df3baa615d8db5 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:51 +0530 Subject: [PATCH 06/16] feat(gates): attach the criterion to both arms In contract::configured, the half io exec and a session share. A gate applied in the session's half would hold a terminal to a standard and let CI run to green with nothing checked. The criterion and its reviewer are decided together: a Review reaching a contract with no reviewer is Error::Config at run start, so either both go on or neither does, and a refusal leaves the run ungated with a notice rather than dead. Printable wraps a provider so it can satisfy ModelReviewer's Debug bound. None of io-harness's own provider types implements Debug, which makes its only shipped Reviewer unusable from any crate that uses them. The Debug impl prints the vendor name and never the credential. --- src/contract.rs | 82 +++++++++++++++++++++++++++++++++++++ src/provider.rs | 106 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+) diff --git a/src/contract.rs b/src/contract.rs index e859417..bdc63aa 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -271,12 +271,94 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta Some(hooks) => contract.with_tool_hooks(std::sync::Arc::new(hooks)), None => contract, }; + // **The operator's own criterion, here rather than in [`session`], and the + // placement is the whole of F1.** A gate applied in the session's half would + // hold a terminal to a standard and let CI run to green with nothing checked + // — the 0.14.0 asymmetry again, and the one this release is least able to + // afford, because the headless arm is where an unverified success does the + // most damage. + // + // A refusal leaves the contract ungated on purpose. `/gates` refuses both + // mistakes at the moment the file is written (F5), so a section that still + // resolves to one here was hand-edited afterwards — and the honest response + // to an unusable criterion is a run with no gate plus a notice, never a run + // io-harness kills at start with `Error::Config` before the first billed + // call. [`gate_notice`] is what puts that on screen. + let contract = match criterion_for(config) { + Some((criterion, reviewer)) => { + let contract = contract.with_verification(criterion.verification()); + match reviewer { + Some(reviewer) => contract.with_reviewer(reviewer), + None => contract, + } + } + 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. resolve_skills(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 +/// section that names no criterion, a section that names two, and — the one that +/// matters most — a review criterion whose reviewer cannot be constructed. **A +/// `Verification::Review` reaching a contract with no reviewer beside it is +/// `Error::Config` at run start, on every turn**, so the criterion and its +/// reviewer are decided together here and either both go on or neither does. +fn criterion_for( + config: &Config, +) -> Option<( + crate::gates::Criterion, + Option>, +)> { + let gates = crate::settings::stored(config).0?.gates?; + // The model the work is done by, which is what the self-review refusal is + // decided against. An empty string means it is not knowable from here, and + // `Settings::criterion` treats that as "cannot clash" rather than guessing. + let working = config + .provider_spec() + .map(crate::provider::model_of) + .unwrap_or_default(); + let criterion = gates.criterion(working).ok()??; + match &criterion { + crate::gates::Criterion::Review { reviewer, .. } => { + let spec = config.provider_spec()?; + let built = crate::gates::reviewer(spec, reviewer).ok()?; + Some((criterion.clone(), Some(built))) + } + _ => Some((criterion, None)), + } +} + +/// Why the configured criterion is not gating this run, if it is not. +/// +/// Separate from [`configured`] because that function returns a contract and a +/// refusal is not a contract. A surface calls this to say the one sentence an +/// operator needs: the section is there, and it is not doing anything. +pub fn gate_notice(config: &Config) -> Option { + let gates = crate::settings::stored(config).0?.gates?; + let working = config + .provider_spec() + .map(crate::provider::model_of) + .unwrap_or_default(); + match gates.criterion(working) { + Err(refusal) => Some(refusal.to_string()), + Ok(Some(crate::gates::Criterion::Review { reviewer, .. })) => match config.provider_spec() { + None => Some(format!( + "the gate asks {reviewer} to review the work, but no provider is configured to \ + reach it — this turn is not gated" + )), + Some(spec) => crate::gates::reviewer(spec, &reviewer) + .err() + .map(|why| format!("the gate's reviewer could not be built: {why}")), + }, + Ok(_) => None, + } +} + /// The hooks a run should be observed by, for the caller that installs the /// fan-out. /// diff --git a/src/provider.rs b/src/provider.rs index 0c899db..9c81e25 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -230,6 +230,112 @@ impl Provider for Watched

{ } } +/// A provider that can be printed, which is the only thing io-harness's own +/// reviewer is missing. +/// +/// `ModelReviewer

` implements `Reviewer` — the trait `TaskContract::with_reviewer` +/// takes — only for `P: Provider + Debug + Send + Sync`, because `ModelReviewer` +/// derives `Debug` and the derive puts the bound on `P`. **None of io-harness's +/// own provider types implements `Debug`**, so `ModelReviewer::new(OpenRouter::new(..), ..)` +/// cannot be handed to `with_reviewer` at all, and the model-judged half of the +/// verification pillar is unreachable from any crate that uses the providers the +/// harness ships. Reported upstream as io-harness#213; this is io-cli shipping +/// around it rather than waiting. +/// +/// The `Debug` impl prints the vendor name and **never the credential**. That is +/// the whole reason it is written by hand rather than derived: a derive here would +/// put an API key into every error that formatted a reviewer. +/// +/// Every method delegates, for the reason [`Watched`] gives at length — a default +/// that fired here would change behaviour silently. This wrapper records nothing; +/// a reviewer's request is not the operator's conversation and has no business in +/// what `/context` reports. +pub struct Printable

{ + inner: P, +} + +impl

Printable

{ + /// Wrap `inner` so it can satisfy a `Debug` bound. + pub fn new(inner: P) -> Self { + Self { inner } + } +} + +impl std::fmt::Debug for Printable

{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // The name and nothing else. `endpoint()` is safe but uninformative here, + // and every other accessor either carries or is derived from the key. + f.debug_struct("Printable") + .field("provider", &self.inner.name()) + .finish() + } +} + +impl Provider for Printable

{ + fn complete( + &self, + request: CompletionRequest, + ) -> impl std::future::Future> + Send { + self.inner.complete(request) + } + + fn complete_streaming( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + ) -> impl std::future::Future> { + self.inner.complete_streaming(request, on_token) + } + + fn complete_streaming_calls( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + on_call: &(dyn Fn(usize, &ToolCall) + Send + Sync), + ) -> impl std::future::Future> { + self.inner + .complete_streaming_calls(request, on_token, on_call) + } + + fn models( + &self, + ) -> impl std::future::Future>> + Send { + self.inner.models() + } + + fn reachable(&self) -> impl std::future::Future> + Send { + self.inner.reachable() + } + + fn model_hint(&self) -> Option<&str> { + self.inner.model_hint() + } + + fn name(&self) -> &str { + self.inner.name() + } + + fn prompt_family(&self) -> PromptFamily { + self.inner.prompt_family() + } + + fn accepts_images(&self) -> bool { + self.inner.accepts_images() + } + + fn endpoint(&self) -> Option<&str> { + self.inner.endpoint() + } + + fn endpoints(&self) -> Vec<&str> { + self.inner.endpoints() + } + + fn last_served(&self) -> Option { + self.inner.last_served() + } +} + /// The same maker, making watched providers. /// /// A maker in and a maker out, rather than a provider in and a provider out, From 49ec3defd26c153335467b724773aba4f849a03b Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:51 +0530 Subject: [PATCH 07/16] feat(exec): exit 6 when the work does not hold up A wrapper around exec::code rather than an arm inside it. That function's body is extracted by a test splitting on its exact signature line, and its table is a statement about RunOutcome, which carries no verification verdict at all. Only a gate that Failed earns 6. One that Errored never answered, and reporting that as the work not holding up would claim a judgement nobody made. --- src/exec.rs | 89 +++++++++++++++++++++++++++++++++++++++++++++++++-- tests/exec.rs | 83 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 167 insertions(+), 5 deletions(-) diff --git a/src/exec.rs b/src/exec.rs index fb4ba42..7dd6cad 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -62,6 +62,20 @@ pub const CEILING: u8 = 3; pub const PAUSED: u8 = 4; /// It ended without finishing. pub const UNFINISHED: u8 = 5; +/// The agent finished and the work does not hold up. +/// +/// Added in 0.24.0, the release that let an operator say what "done" means. It +/// renumbers nothing: the six below have meant what they mean since 0.5.0, and +/// this is the first ending this subcommand has ever been able to tell apart +/// from them. +/// +/// It cannot be decided by [`code`], because io-harness has no `RunOutcome` +/// variant for a run whose criterion answered no — such a run surfaces as +/// `StepCapReached` when it spends its budget failing, and as an ordinary +/// `Finished` when it stops early. So the verdict is read from the store after +/// the run and applied by [`verified_code`]. Reported upstream as +/// io-harness#212. +pub const UNVERIFIED: u8 = 6; /// The exit status for a run that reached the harness. /// @@ -133,6 +147,29 @@ pub fn code(outcome: &RunOutcome) -> u8 { } } +/// The exit status once the operator's own criterion has had its say. +/// +/// A wrapper around [`code`] rather than an arm inside it, and deliberately so. +/// `tests/exec.rs` extracts that function's body by splitting on its exact +/// signature line, and its table is a statement about `RunOutcome` alone — which +/// carries no verification verdict. Putting this decision inside it would both +/// reformat the one function a test reads as text and make a table about +/// outcomes answer a question outcomes do not contain. +/// +/// **Only [`GateOutcome::Failed`] earns [`UNVERIFIED`].** A gate that +/// `Errored` never answered at all — the criterion could not run, the reviewer +/// returned nothing parsable, the program was refused — and reporting that as +/// "the work does not hold up" would claim a judgement nobody made. Such a run +/// keeps whatever its outcome maps to, which is the honest report that it ran +/// and was not verified either way. A run with no criterion configured has no +/// standing at all and is untouched. +pub fn verified_code(outcome: &RunOutcome, standing: Option<&crate::gates::Standing>) -> u8 { + match standing { + Some(standing) if matches!(standing.outcome, io_harness::GateOutcome::Failed) => UNVERIFIED, + _ => code(outcome), + } +} + /// One line for stderr, naming the outcome and the harness's own step count. /// /// The number is read off the returned value and never recounted from the store: @@ -588,10 +625,49 @@ impl WithProvider for Headless { if let Some(parked) = parked(&result.outcome, result.run_id) { eprintln!("io: {parked}"); } - Ok(code(&result.outcome)) + // **The criterion has the last word on the exit status, and it is read + // from the store rather than from the outcome.** io-harness has no + // `RunOutcome` variant for a run whose gate answered no, so a run that + // spent its budget failing one reports `StepCapReached` and a run that + // stopped early believing itself done reports `Finished` — `3` and `0`, + // neither of which is what happened. Reported upstream as + // io-harness#212. + let standing = gate_standing(&self.store, result.run_id); + if let Some(standing) = &standing { + eprintln!("io: {}", gate_line(standing)); + } + Ok(verified_code(&result.outcome, standing.as_ref())) } } +/// What a run's own gate attempts say, if it had a criterion at all. +/// +/// A store that cannot be read answers `None`, which leaves the exit status +/// exactly what it was before this release. A failure to read the verdict is not +/// evidence that the work is bad. +fn gate_standing(store: &Store, run_id: i64) -> Option { + let attempts = store.gate_attempts(run_id).ok()?; + crate::gates::standing(&attempts) +} + +/// One line for stderr naming what the gate did. +/// +/// The phase is the harness's own word for the criterion that ran, and the +/// outcome is `GateOutcome::as_str` passed through rather than re-spelled — a +/// second vocabulary for the same fact is how two surfaces come to disagree. +fn gate_line(standing: &crate::gates::Standing) -> String { + let attempt = if standing.attempt > 1 { + format!(" on attempt {}", standing.attempt) + } else { + String::new() + }; + format!( + "the {} gate {}{attempt}", + standing.phase, + standing.outcome.as_str() + ) +} + /// One row of `io resume --list`, or `None` for a run nothing is waiting on. /// /// `json` splits the same two streams `io exec --json` splits: an object per @@ -1109,6 +1185,15 @@ impl WithProvider for Resuming { if let Some(parked) = parked(&resumed.outcome, resumed.run_id) { eprintln!("io: {parked}"); } - Ok(code(&resumed.outcome)) + // A resumed run is gated by the same criterion the original was — the + // contract is rebuilt from the same configuration — so its exit status + // answers the same question. Reading the standing off the run this + // resume drove rather than off the original is the point: the whole + // reason to resume is that the work is not the same work any more. + let standing = gate_standing(&self.store, resumed.run_id); + if let Some(standing) = &standing { + eprintln!("io: {}", gate_line(standing)); + } + Ok(verified_code(&resumed.outcome, standing.as_ref())) } } diff --git a/tests/exec.rs b/tests/exec.rs index 69975c3..d93b8ca 100644 --- a/tests/exec.rs +++ b/tests/exec.rs @@ -107,9 +107,15 @@ async fn f1_a_clean_run_finishes_rather_than_succeeding() { .expect("the turn runs"); // Not an incidental assertion. `TaskContract::workspace` sets - // `Verification::None`, so `Success` is unreachable from this subcommand and - // `Finished` is what every good run returns. A table that mapped only + // `Verification::None`, and this fixture configures no criterion, so + // `Finished` is what a good run returns here. A table that mapped only // `Success` to zero would fail here — and only here, on the ordinary case. + // + // **0.24.0 makes `Success` reachable and this assertion narrower rather than + // false.** An operator who writes `[app.io-cli.gates]` gets a real + // criterion on the contract and a passing gate returns `Success`. What stays + // true is that a run with NO criterion finishes rather than succeeding, which + // is the case this fixture builds and the case every operator has today. assert!( matches!(result.outcome, RunOutcome::Finished { .. }), "a contract with no verification criterion finishes, it does not succeed: {:?}", @@ -178,16 +184,87 @@ fn f2_every_outcome_maps_to_its_documented_code() { } // The six codes are distinct and cover 0..=5, so a script can branch on them. + // + // **`6` is deliberately absent from this list and that is the assertion.** + // 0.24.0 added `UNVERIFIED`, and it is not reachable from an outcome — + // io-harness has no `RunOutcome` variant for a criterion that answered no, so + // the verdict is read from the store and applied by `exec::verified_code`. + // This test is about `exec::code`, whose input is an outcome and nothing + // else; widening it to admit 6 would make it a weaker statement about a + // function that cannot produce one. `verified_code` has its own test below. let mut codes: Vec = cases.iter().map(|(_, code)| *code).collect(); codes.sort_unstable(); codes.dedup(); assert_eq!( codes, vec![0, 2, 3, 4, 5], - "0, 2, 3, 4 and 5 are reachable from an outcome; 1 is reserved for never reaching one" + "0, 2, 3, 4 and 5 are reachable from an outcome; 1 is reserved for never reaching one, \ + and 6 is decided after the run by `verified_code`" ); } +/// A gate that ran and said no is exit `6`, whatever the run itself reported. +/// +/// The two outcomes below are the ones a gate-failed run actually ends in, and +/// they map to different codes without a gate — `StepCapReached` to `3` when the +/// agent spends its budget failing the criterion, `Finished` to `0` when it stops +/// early believing itself done. Both become `6`, which is the whole point: the +/// operator asked a question and got the same answer either way. +/// +/// Sabotage: treat `Errored` as `UNVERIFIED` too — under which the `Errored` arm +/// fails, and it fails by reporting a judgement that was never made. +#[test] +fn f8_a_failing_gate_is_exit_six_and_a_gate_that_never_answered_is_not() { + use io_cli::gates::Standing; + use io_harness::GateOutcome; + + let standing = |outcome| Standing { + phase: "command".into(), + outcome, + attempt: 1, + }; + + for outcome in [ + RunOutcome::StepCapReached { steps: 12 }, + RunOutcome::Finished { steps: 3 }, + ] { + assert_eq!( + exec::verified_code(&outcome, Some(&standing(GateOutcome::Failed))), + exec::UNVERIFIED, + "{outcome:?} with a failed gate is the work not holding up", + ); + + // A criterion that could not run has judged nothing. The run keeps the + // code its own outcome earned. + assert_eq!( + exec::verified_code(&outcome, Some(&standing(GateOutcome::Errored))), + exec::code(&outcome), + "{outcome:?} with an errored gate keeps its own code", + ); + + // A passing gate is not a reason to change anything either: `Success` + // already maps to `OK` through the table. + assert_eq!( + exec::verified_code(&outcome, Some(&standing(GateOutcome::Passed))), + exec::code(&outcome), + "{outcome:?} with a passing gate keeps its own code", + ); + + // And an operator who configured no criterion sees exactly what they saw + // in 0.23.0. + assert_eq!( + exec::verified_code(&outcome, None), + exec::code(&outcome), + "{outcome:?} with no gate configured is unchanged", + ); + } + + // `6` is outside the range `code` can produce, so a script branching on the + // old six cannot mistake it for one of them. + assert_eq!(exec::UNVERIFIED, 6); + assert!(exec::UNVERIFIED > exec::UNFINISHED); +} + #[test] fn f2_a_paused_run_names_what_was_parked_and_nothing_else_does() { // Exit 4 is reachable, and the first end-to-end run of the release binary is From 9a0dc3607683d5e3ff3dd2accd91da5194b4ae06 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 06:46:51 +0530 Subject: [PATCH 08/16] feat(gates): /gates joins the inventory and the documented surfaces The 31st command, in Configure because it writes the configuration file. Its description names all three kinds, because two cost a process and one is a billed completion on every gated turn. README gains the gates section, the exit 6 row, the [app.io-cli.gates] key, and corrections to two claims this release falsifies. --- README.md | 136 ++++++++++++++++++++++++++++++++++++++++++---- src/commands.rs | 68 ++++++++++++++++++++++- tests/commands.rs | 97 ++++++++++++++++++++++++++++++++- tests/docs.rs | 47 ++++++++++++++++ 4 files changed, 332 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 0af535c..8c36628 100644 --- a/README.md +++ b/README.md @@ -493,6 +493,7 @@ above a row that ranked there for reasons having nothing to do with it. | `/mcp` | the MCP servers configured, and what this session has seen of each | | `/provider` | the providers configured, in the order a turn tries them | | `/plugin` | the capability bundles loaded, what each contributed, and the ones that failed | +| `/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 | `/plugin` is new in 0.20.0 and sits beside those two because it is the third @@ -505,6 +506,16 @@ here you use once: the others are returned to for the life of the install. It writes files, which is the whole reason it is under **configure**. See [Bringing your setup across](#bringing-your-setup-across). +`/gates` is new in 0.24.0 and is under **configure** for that same reason and not +because of what its first screen shows. It opens on the criterion in force and the +last turn's verdict, which reads like an inspection — and then writes +`[app.io-cli.gates]`, which is the one thing **inspect** promises it never does. +That promise is worth more here than it is for a server list: a gate set by +accident does not merely change what the next turn talks to, it can spend a whole +extra turn against a real model deciding the first one was not finished. It sits +before `/import` because it is one you come back to. See [Verification +gates](#verification-gates). + `/resume` does more than reopen from 0.23.0: each row says what that session's last run stopped on, and choosing one answers it. No command was added for that — the one that was already there was extended. See [When a run stops for @@ -1100,6 +1111,88 @@ default. Bare `/plan` says which one you are in and changes nothing. `[app.io-cli.containment]`, so configuring a fan-out silently made every turn stop and propose first. If that is what you wanted, `/plan on` is where it lives now. +## Verification gates + +**An agent that stops is not an agent that is done.** Every release before 0.24.0 +took the model's own word for it: the turn ended, the interface said so, and +whether the tests still passed was a question you asked afterwards. A gate is +where you say what "done" means *for this repository*, once, and the turn is not +finished until the criterion passes or the retry budget is spent. + +`/gates` writes it and shows what the last turn was judged on. It is a section of +your configuration file like any other, so it can also be typed by hand: + +```toml +[app.io-cli.gates] +command = ["cargo", "test", "--all"] +retries = 1 +``` + +There are three kinds and you get exactly one, because a `TaskContract` in +io-harness holds one `Verification` and not a list. Naming none, or naming two, is +refused where you can still see what you typed rather than silently picking a +winner: + +- **A command** that must exit a status you name — zero unless you say otherwise. + It is an argv and never a shell line, because io-harness checks `argv[0]` + against your permission boundary and runs it without a shell. This is the cheap + kind: it costs a process, it is objective, and it is the same thing you would + have run yourself. +- **A file** that must exist, and optionally must contain some text. Nearly free + and deliberately narrow — it answers "did the change actually get written down", + which is the failure a passing test suite is worst at catching. +- **A rubric** a second model answers: a sentence saying what the work has to be, + judged by a reviewer you name. + +**io-cli holds no list of test commands.** Leave `command` empty and the one that +runs is the one the repository's own toolchain proposes — io-harness detects that +from the project, so a Rust checkout is offered a `cargo` line and a Node one is +not. That detection is the dependency's, and it is deliberately not reimplemented +here: a table of build tools inside this crate would be a second opinion that goes +stale the first time somebody's project does not look like the ones it was written +against. + +**The criterion is run by io-harness, in the sandbox, and not by io-cli.** That is +not squeamishness. A criterion run from here could not be handed the cache +directories a real run gets from the detected toolchain, so a `cargo test` gate +would fail on a registry write that io-harness's own gate would have allowed. It +also keeps a rule worth keeping: exactly one module in io-cli starts a process at +all, and it is the one behind `!`. + +**`retries` defaults to 1, and `0` means report-only.** A failing gate sends the +agent back to work with the failure text — the compiler's output, the missing +file, the reviewer's sentence — because the failure *is* the instruction, and an +agent told "it did not pass" without being told what did not pass is being asked +to guess. One retry is the default because a retry is a whole turn against a real +model and not a loop counter; if you want several, say so. Set `retries = 0` and +the verdict is drawn and recorded and nothing is re-driven, which is what you want +in a run you are watching and what you want in a run you are only measuring. + +**A rubric is a real billed completion on every gated turn.** Not on failures, not +on the first one — every turn that carries the gate ends with a call to the +reviewer, and that call costs what a call costs. This is the reason the three +kinds are named in `/gates`'s own one-line description rather than left to the +surface: two of them cost nothing but the time they take and one of them costs +money, and that is the whole of the choice between them. The call is io-harness's, +so it lands in the run's usage like any other and `/cost` counts it. + +**A rubric needs a `reviewer`, and it is refused without one.** io-harness answers +a missing reviewer with a configuration error at run start — before the first +billed call, on every turn, in a place on screen nowhere near the keystroke that +caused it — so `/gates` refuses it while you are still looking at what you typed. +The reviewer is also never defaulted to the model doing the work. A model marking +its own paper is a decision rather than a convenience, and it is spelled +`allow_self_review = true`; without it, naming the working model as the judge is +the second refusal. + +**What a gate is not is a test runner.** io-cli does not discover your tests, does +not parse their output, and does not decide what a passing suite looks like. It +carries one criterion on the contract and reports the verdict io-harness came +back with. + +`io exec` gains exit `6` for this — the agent finished and the work does not hold +up. See [Exit status](#exit-status). + ## Pictures **Drag a picture onto the prompt, or copy it and paste.** That is the whole of @@ -1301,18 +1394,35 @@ what it already does with a policy refusal. | `3` | a ceiling was reached: steps, time, tokens, or the tree's shared budget | | `4` | the run stopped needing a human: it asked a question, proposed a plan, or was interrupted in the middle of a call | | `5` | it ended without finishing: stalled, escalated, or cancelled | +| `6` | the agent finished and the work does not hold up: a gate you configured did not pass | A ceiling is `3` and not `0` because io-harness returns one as a *successful call* whose outcome says a limit was hit; a status read off the result alone would call a truncated run a finished one. +**Exit `6` is new in 0.24.0 and it is the only one that is.** It says something no +other row could: the run ended the way `0` ends, of its own accord, and then the +criterion you set in [`[app.io-cli.gates]`](#verification-gates) failed anyway — +the tests did not pass, the file was not written, the reviewer said no. It is not +`1`, because nothing went wrong with the invocation; it is not `5`, because the +agent did not stall or give up; and it is emphatically not `0`, which is the +status a build script reads as permission to carry on. A run that never had a gate +configured can never return it. + +**No exit code was renumbered, and `6` is the first one added since `io exec` +shipped.** `0` through `5` have meant exactly what they mean in the table above +since 0.5.0, and they mean it unchanged here: a script branching on them is a +script this release did not break. What changes is that a script branching on `0` +alone now has a sixth answer to handle, which is the point — before this release +there was no status a gated run could return that said the work was not good +enough, because there were no gates. + **Exit `4` names the pause from 0.23.0, and the invocation that answers it.** The closing line used to name the run id and nothing else, which addressed none of the four pauses; it now names the question, plan or call the run stopped -on and the `io resume` that decides it. No exit code was renumbered and none was -added: the six have meant what they mean here since `io exec` shipped them in -0.5.0, and `4` was given to a pause that could not yet be answered for exactly -this release. +on and the `io resume` that decides it. That release renumbered nothing and added +nothing: `4` had been given to a pause that could not yet be answered for exactly +that release. An approval is the one pause `io resume` cannot take: it is answered by the person the run asked, at the terminal it asked from, and there is no resume entry @@ -1490,7 +1600,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 six tables: +Eight keys live there, and seven tables: | Key | Is | | --- | --- | @@ -1507,6 +1617,7 @@ Eight keys live there, and six tables: | `[[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.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 @@ -1639,11 +1750,16 @@ line beside what has been drawn against them. surface the omission was waiting on: `/plugin` for what a bundle brought and what was dropped, and io-harness's own refusal sentence for a hook a project file may not declare. **`[prices]` is read from 0.22.0**, which is the release that reads -the provider-call rows it prices. That leaves one section and one key still -unapplied, and each has a reason: there is no `[verify]` section to apply, and -giving a session verification gates needs its own surface; and `run.templates` is -the thirteenth `[run]` key, reachable only through its own accessor. Neither is a -silent omission — this is where they are named. +the provider-call rows it prices. **A contract carries a verification criterion +from 0.24.0**, which was the last of these to be named here and was named with its +reason: it needed a surface of its own, and `/gates` is that surface. There is +still no `[verify]` section, because there is none in io-harness's schema to +apply — what a session carries comes from `[app.io-cli.gates]`, which is io-cli's +own. See [Verification gates](#verification-gates). + +That leaves one key still unapplied, and it has a reason too: `run.templates` is +the thirteenth `[run]` key, reachable only through its own accessor. It is not a +silent omission — this is where it is named. **A price is never invented, and a missing one is never a zero.** io-cli compiles no rates in and estimates nothing, so an install that has connected no provider diff --git a/src/commands.rs b/src/commands.rs index b104cd5..a26644f 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -247,21 +247,37 @@ pub const COMMANDS: &[(&str, &str)] = &[ "/stats", "commit how the runs have gone: outcomes, first-try, gates, latency", ), + // Beside `/stats` because `/stats` is the only other row that says the word, + // and the two are the halves of one thing: that page counts how the gates + // went, and until this release nothing in the product could say what a gate + // *was*. An operator reading a gate column over a session that configured + // none is reading zeroes and cannot tell them from a gate that never failed. + // + // The three kinds are named in the description rather than left to the + // surface, because the choice between them is the whole decision — a command + // is free and objective, a file is nearly free and narrow, and a rubric is a + // billed completion on every gated turn. A row that said only "what done + // means" would hide the one of the three that costs money. + ( + "/gates", + "the check a turn must pass before it is done: a command, a file, or a rubric", + ), ]; /// What an operator is doing when they reach for a command. /// /// **Grouped by the operator's intent rather than by which part of the harness /// answers**, because the second is an implementation detail and the first is -/// the only thing somebody scanning a list of twenty-six is holding in their +/// the only thing somebody scanning a list of thirty-one is holding in their /// head. /// /// Four groups and none longer than ten, which is the bound `tests/commands.rs` -/// asserts. A flat list of twenty-six is a list nobody reads; 0.16.0 is the +/// asserts. A flat list of thirty-one is a list nobody reads; 0.16.0 is the /// release that grouped them, at twenty, and every release since has added to a /// group rather than to a list. **The count in this paragraph is the one number /// here that goes stale on its own** — it said twenty through 0.17.0, which had -/// twenty-three — so it is written out rather than left as "a few". +/// twenty-three, and twenty-six through 0.23.0, which had thirty — so it is +/// written out rather than left as "a few". #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum Group { /// The conversation itself: start one, leave one, come back to one. @@ -376,6 +392,21 @@ pub const GROUPS: &[(Group, &[&str])] = &[ "/mcp", "/provider", "/plugin", + // **`/gates` is here because it WRITES `[app.io-cli.gates]`, and that + // is the whole of the argument.** Its first screen reads the section + // back and says what the last turn's verdict was, which is the same + // first screen `/mcp` and `/provider` had when 0.19.0 filed them under + // `Inspect` — and the same mistake, for the same reason: an operator + // who opened it to look is one keystroke from changing what every + // later turn has to prove before it is allowed to finish. That is the + // sentence `Inspect` promises it will never say, and it is a louder + // sentence here than for a server list, because a gate an operator did + // not mean to set spends a whole extra turn against a real model. + // + // It takes `Configure` to nine of ten, so the next command that + // belongs here forces the same question 0.22.0 answered rather than + // dodged: re-file what is in the wrong group, do not widen the bound. + "/gates", // **`/import` writes files, so `Configure` is the only group it can // be in** — and `Inspect` is full at nine besides. It is last because // it is the one command here an operator uses once: the others are @@ -1026,6 +1057,27 @@ pub enum Action { /// recorded. A key that was never on disk must not reach disk as a side /// effect of trying a new program. Import, + /// Say what "done" means for this repository, and write it down. + /// + /// **No argument, and that is a decision rather than an omission.** A + /// criterion is an argv, or a path and the text a file must contain, or a + /// whole sentence of rubric — see [`crate::gates::Criterion`] for the three. + /// None of those is a word that fits after a slash, and giving them one would + /// mean io-cli inventing a little language for a section io-harness already + /// spells in TOML: `/gates cargo test --all` is either a shell line this crate + /// would have to split — which is exactly what `Criterion::Command` refuses to + /// be, because io-harness matches `argv[0]` against the policy and runs no + /// shell — or an argv with nowhere to put the exit status it must report. So + /// the word opens the surface and anything after it is ignored, the way + /// `/model gpt-5` ignores its argument. + /// + /// **The surface is also the only place the two refusals can be seen.** A + /// rubric with no reviewer, and a reviewer that is the model doing the work, + /// are both `Error::Config` at run start in io-harness — before the first + /// billed call, on every turn, and disconnected on screen from the keystroke + /// that caused them. [`crate::gates::Refusal`] names both while the operator + /// is still looking at what they typed. + Gates, } /// What `/copy` was asked for. @@ -1613,6 +1665,16 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { // idea what this one calls things. Neither spelling is an alias for a // second screen: both open the one surface. "import" | "migrate" => Action::Import, + // `/gate` is admitted for the mirror of the reason `/plugins` and + // `/servers` are. A contract carries exactly one criterion, so the + // singular is what a hand reaches for at least as often as the plural, + // and refusing the other spelling teaches nobody anything. Neither + // spelling is an alias for a second screen: both open the one surface. + // + // `/verify` is deliberately not taken, under the rule that already + // refuses `/fold`: it is the word io-harness uses for the pillar + // internally, and nobody has typed it at this prompt. + "gates" | "gate" => Action::Gates, // **An alias earns no row of its own.** `/usage` is what an operator // coming from another agent types for "what is this costing me", and a // second row for one screen reads as a second screen — so this is diff --git a/tests/commands.rs b/tests/commands.rs index e6dc28e..3a5c6af 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -89,6 +89,13 @@ fn the_commands_are_the_commands() { // this list, which is the rule every alias here follows. "/cost", "/stats", + // 0.24.0 — beside `/stats` because `/stats` is the only other row + // that says the word: that page counts how the gates went, and until + // this release nothing in the product could say what a gate was. It + // is under **configure** and not **inspect**, because opening it is + // one keystroke from writing `[app.io-cli.gates]` and changing what + // every later turn has to prove. + "/gates", ], "the fuzzy palette is still 0.7.0; this list is written out so that adding \ a command is a decision somebody makes rather than a line somebody adds", @@ -261,11 +268,15 @@ fn f12_resume_says_it_answers_a_parked_run_rather_than_merely_reopening_a_sessio Action::Resume, ); // No command was added for any of this — `/resume` was extended — so the - // inventory is the size the other gates in this file assert. + // inventory is the size the other gates in this file assert. The number is + // 30 plus 0.24.0's one addition, and the two halves are named separately on + // purpose: a total that merely went up by one would be satisfied by 0.23.0 + // growing a command and 0.24.0 losing one. assert_eq!( COMMANDS.len(), - 30, - "0.23.0 adds no command; a thirty-first here means one arrived unrecorded", + 31, + "0.23.0 adds no command and 0.24.0 adds exactly one, `/gates`; a \ + thirty-second here means one arrived unrecorded", ); } @@ -1155,3 +1166,83 @@ fn f17_usage_answers_and_is_never_listed() { "a second row for one screen reads as a second screen", ); } + +/// 0.24.0 — `/gates` is a **configure** command, resolves to its own action, and +/// its row is reachable. +/// +/// Every claim here is written against the sabotage that would satisfy the +/// generic gates in this file while leaving the command broken. None of them is +/// a restatement of a gate that already covers it. +/// +/// `no_listed_command_falls_through_to_the_help_listing` proves only that the +/// word is not the unknown-command listing; a `parse` arm that returned +/// `Action::Config(None)` — the surface next door, and the one a careless hand +/// reaches for — would pass it and would open the wrong screen. So the action is +/// asserted by name. +/// +/// `f13_every_command_is_in_exactly_one_group` proves only that it has *a* home. +/// `Inspect` would satisfy it, and `Inspect`'s own documentation says nothing +/// under it changes what a turn does — which a surface that writes +/// `[app.io-cli.gates]` falsifies for every later turn at once. So the group is +/// asserted by name rather than left to the count, exactly as the `/mcp` and +/// `/provider` move is. +/// +/// And the description is asserted to name a cost, because `tests/docs.rs` copies +/// it into the README verbatim and it is the only sentence most operators will +/// ever read about this command. A rubric is a real billed completion on every +/// gated turn; a one-liner that said "what done means" and stopped would hide +/// that behind a word that sounds free. +#[test] +fn gates_is_a_configure_command_that_resolves_to_its_own_action() { + use io_cli::commands::{group_of, palette, palette_pick, Chosen, Group}; + use io_harness::Templates; + + assert!( + COMMANDS.iter().any(|(name, _)| *name == "/gates"), + "a surface nobody is told about is a surface nobody uses", + ); + assert_eq!( + commands::parse("gates", &defaults(), &DARK), + Action::Gates, + "`/gates` must open its own surface, not the one next door", + ); + // The singular is the spelling a hand reaches for when there is exactly one + // criterion, and it must be the same screen rather than an unknown command. + assert_eq!(commands::parse("gate", &defaults(), &DARK), Action::Gates); + // Arguments are tolerated and the first word decides, as everywhere else — + // and this is the command most likely to be typed with one, because the + // thing an operator wants to set is a command line. + assert_eq!( + commands::parse("gates cargo test", &defaults(), &DARK), + Action::Gates, + ); + + assert_eq!( + group_of("/gates"), + Some(Group::Configure), + "`/gates` writes [app.io-cli.gates], and Inspect promises it never writes \ + — a gate an operator did not mean to set spends a whole extra turn \ + against a real model", + ); + + let (_, said) = COMMANDS + .iter() + .find(|(name, _)| *name == "/gates") + .expect("/gates is listed"); + assert!( + said.contains("rubric"), + "the one line most operators read must name the kind that costs money on \ + every gated turn: {said}", + ); + + let rows = palette(&Templates::none(), &[]); + let index = rows + .iter() + .position(|row| row.label == "gates") + .expect("`/gates` has no palette row"); + assert_eq!( + palette_pick(&Templates::none(), &[], index), + Some(Chosen::Command("/gates")), + "`/gates`'s row is advertised and inert", + ); +} diff --git a/tests/docs.rs b/tests/docs.rs index 2223b21..9abf306 100644 --- a/tests/docs.rs +++ b/tests/docs.rs @@ -245,6 +245,22 @@ fn the_readme_documents_every_key_of_the_io_cli_section() { source: Some("the reference catalogue".into()), models: Some(417), }), + // Written out field by field rather than as a `Default::default()`, which + // would serialize to `{}` and satisfy the row check just as well. The + // point is the compile error: `[app.io-cli.gates]` is the one nested + // table whose keys are explained in README prose rather than in a row of + // their own, so a key added to `gates::Settings` has to break something + // to be noticed. This is that something. + gates: Some(io_cli::gates::Settings { + retries: Some(2), + command: Some(vec!["cargo".into(), "test".into()]), + expect_exit: Some(0), + file: Some("CHANGELOG.md".into()), + contains: Some("## [0.24.0]".into()), + rubric: Some("the change is covered by a test that fails without it".into()), + reviewer: Some("a-reviewing-model".into()), + allow_self_review: Some(false), + }), }; let value = serde_json::to_value(&every).expect("[app.io-cli] serializes"); let keys = value.as_object().expect("a table"); @@ -856,6 +872,37 @@ fn the_readme_exit_table_is_the_exit_table() { "the README describes {code} differently from the code: {what}", ); } + + // 0.24.0's `6`, and it is asserted as a NUMBER rather than through a constant + // on purpose. What a script branches on is the integer, and the one mistake + // this release could make that a caller cannot recover from is landing the + // gate code on a number that already meant something else. So the claim is + // that `6` is the code immediately after `UNFINISHED`, which is where the six + // that shipped in 0.5.0 stopped. Move any of those six and this fails; give + // the gate a number one of them already holds and it fails too. + assert_eq!( + io_cli::exec::UNFINISHED + 1, + 6, + "the gate code sits after the six that shipped in 0.5.0; a code that moved \ + one of them cannot be migrated by fixing forward", + ); + assert!( + readme.contains("| `6` |"), + "the README's exit table has no row for the gate that did not pass", + ); + assert!( + readme.contains("does not hold up"), + "the README should say what `6` means in the words the release uses: the \ + agent finished and the work does not hold up", + ); + // And the half a reader most needs: nothing above it moved. A release that + // renumbered `5` would leave every script branching on it silently wrong, + // which is the failure the sentence exists to rule out. + assert!( + readme.contains("No exit code was renumbered"), + "the README should say, where the table is, that no existing code changed \ + meaning", + ); } #[test] From 71cc6b508ab8b9c7de303cc75801a62e6fc3d10e Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:05:53 +0530 Subject: [PATCH 09/16] feat(gates): reach the gate from a keystroke, and retry with the failure in hand /gates shows the criterion in force, what the repository's own toolchain proposes, and the last run's verdict, and writes a criterion to a chosen scope. A review criterion with no reviewer, or one whose reviewer is the working model, is refused before the write rather than at run start. A failing gate composes a follow-up turn carrying the criterion and what it reported, fed through the queue the driver already has. gate_retry takes no goal, so a retry that re-sends the original prompt cannot be written. The retry budget is accumulated across the chain. Store::gate_attempts is keyed on the run, and every session turn is its own run, so counting per run restarts at one on each retry and would retry forever against a real model. /mcp gains an edit verb, closing the limitation stated since 0.21.0. The entry is re-resolved by id before the write, never addressed by the picker's row index. --- src/app.rs | 251 ++++++++++++++++++++ src/contract.rs | 14 +- src/gates.rs | 5 +- src/main.rs | 587 +++++++++++++++++++++++++++++++++++++++++++---- tests/gates.rs | 28 ++- tests/servers.rs | 186 +++++++++++++++ tests/steer.rs | 305 ++++++++++++++++++++++++ 7 files changed, 1322 insertions(+), 54 deletions(-) diff --git a/src/app.rs b/src/app.rs index 85a446a..ab621bd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2094,3 +2094,254 @@ impl App { } } } + +// --------------------------------------------------------------------------- +// The verification gate, as a surface and a driver read it +// --------------------------------------------------------------------------- +// +// **These live here rather than in `src/main.rs`, and that placement is the +// whole reason they are functions at all.** Nothing under `tests/` links the +// binary, so a decision written as a branch in the driver is one no test can +// drive and no sabotage can make fail. `crate::gates` owns everything that is a +// property of a criterion; what is left is the handful of answers a *surface* +// needs — which attempts a turn is judged by, what one line of scrollback says +// about them, and what a retried turn is told. Each is pure over its arguments, +// reads no clock, and opens no store. + +// --------------------------------------------------------------------------- +// `/mcp`'s edit verb, as it travels through the composer +// --------------------------------------------------------------------------- + +/// What `/mcp`'s edit verb writes on the prompt in front of the server's id. +/// +/// **An id and not an index, and the whole verb rests on that.** `mcp[3].command` +/// is a position in one file's `[[mcp]]` array; the operator is about to type a +/// value into a composer that any other keystroke can leave, and a file edited in +/// another window in between moves the array under it. `mcp..` names the +/// entry by the one thing that is stable, so the driver resolves +/// [`crate::servers::At`] again — from the file's own bytes — when the line comes +/// back. That is the same rule `/mcp`'s removal verb has followed since 0.21.0, +/// and it is the rule 0.20.0's wrong delete was shipped by breaking. +/// +/// It is spelled here rather than in the driver because nothing under `tests/` +/// links `src/main.rs`: the prefix the composer writes and the prefix the driver +/// matches on must be one constant, or the verb goes quietly dead the first time +/// either is retyped. +pub const SERVER_KEY: &str = "mcp."; + +/// The server id and the key a `mcp..` line addresses. +/// +/// `None` for anything that is not that shape, including a bare `mcp.` and an id +/// with no key after it. An id containing a dot splits at the LAST one, because +/// the key is the part this crate knows the spelling of — [`crate::servers::KEYS`] +/// holds none with a dot in it, and an MCP server may be called anything at all. +pub fn server_key(line: &str) -> Option<(&str, &str)> { + let rest = line.strip_prefix(SERVER_KEY)?; + let (id, key) = rest.rsplit_once('.')?; + (!id.is_empty() && !key.is_empty()).then_some((id, key)) +} + +/// The TOML source for a value an operator typed after an `[[mcp]]` key. +/// +/// [`crate::servers::edit`] takes TOML **source**, and what arrives from a +/// composer is a person's typing. The gap is not cosmetic: `format!("\"{typed}\"")` +/// — the spelling every call site reaches for — is a parse error or a different +/// value the moment the text carries a quote or a backslash, and a Windows command +/// path is full of the second. +/// +/// Four shapes, because `[[mcp]]` has four kinds of value: +/// +/// * `args` is a list, and an operator writes a list of arguments as a command +/// line. Split on whitespace, which is the same trade every shell makes and the +/// reason an argument with a space in it needs the file rather than this verb. +/// * `timeout_secs` is a number, and a number is already TOML. +/// * `env` and `headers` are inline tables, which have exactly one spelling — +/// `{ KEY = "value" }` — so what the operator typed is passed through and +/// `crate::edit::apply` refuses it if it is not TOML. +/// * everything else is a string, escaped by [`crate::servers::quoted`]. +pub fn server_value(key: &str, typed: &str) -> String { + let typed = typed.trim(); + match key { + "args" => crate::edit::array(&typed.split_whitespace().collect::>()), + "timeout_secs" | "env" | "headers" => typed.to_string(), + _ => crate::servers::quoted(typed), + } +} + +/// The row `/gates` draws for the command this repository proposes for itself. +/// +/// A sentinel and not a key, for the reason [`crate::configure::REFRESH_PRICES`] +/// is one: every other row on that surface names a setting and puts it in the +/// composer, and this one does something. It is spelled here rather than in the +/// driver so a test can reach it. +pub const PROPOSED_GATE: &str = "!proposed-gate"; + +/// The gate attempts a turn is judged by, including the one io-harness never made. +/// +/// **This is the fold that stops an ungated run reporting as gated, and it is the +/// easiest defect in this release to ship.** [`crate::gates::Criterion::verification`] +/// maps a bare existence criterion — `file` with no `contains` — to +/// `Verification::None`, because there is no honest counterpart for it in +/// io-harness's enum. A run carrying `Verification::None` never reaches the gate +/// at all: io-harness's step loop returns `Finished` the moment the agent stops +/// calling tools, so the store holds **no** `gate_attempts` row and +/// [`crate::gates::standing`] answers `None`. A caller that stopped there would +/// draw nothing, retry nothing, and leave an operator who asked for a file to +/// exist with a session that never once looked for it. +/// +/// So the criterion this crate owns is evaluated here — through +/// [`crate::gates::Criterion::satisfied_in`], which is the reader that tells a +/// missing file from an empty one — and appended as an attempt of its own. It +/// then flows through `standing`, `may_retry` and the report exactly as a +/// recorded one does, which is the point: one path, not two. +/// +/// Any `none` row is dropped first. A run that ran out of steps *does* reach the +/// gate with `Verification::None`, which passes trivially and records +/// `phase = "none", passed` — a row saying a criterion that checked nothing was +/// satisfied. Left in place it would be the last row, and the standing would read +/// `passed` for a file that is not there. +/// +/// Every other criterion is io-harness's to judge and the rows come back +/// untouched. +pub fn gate_attempts( + mut recorded: Vec, + criterion: Option<&crate::gates::Criterion>, + root: &std::path::Path, +) -> Vec { + // `satisfied_in` answers `None` for every criterion the run loop evaluates, + // so this one call is both the question "is this ours?" and its answer — + // asking `checked_here` first would be the same test written twice. + let Some(satisfied) = criterion.and_then(|criterion| criterion.satisfied_in(root)) else { + return recorded; + }; + recorded.retain(|attempt| attempt.phase != "none"); + let step = recorded.last().map_or(0, |attempt| attempt.step); + recorded.push(io_harness::GateAttempt { + // Not a stored row and never read back as one: `id` addresses a row in + // `gate_attempts` and this attempt was made here, so it has none. + id: 0, + step, + // Not `none`, which is what io-harness would have called it, and not + // `contains`, which is the phase of a criterion that reads a file's + // contents. This one asserts existence and says so. + phase: "exists".to_string(), + outcome: if satisfied { + io_harness::GateOutcome::Passed + } else { + io_harness::GateOutcome::Failed + }, + // The row's own explanation, for the same reason io-harness leaves it + // empty on a plain pass: there is nothing to explain about a file that is + // where it was asked to be. + detail: if satisfied { + String::new() + } else { + criterion + .map(crate::gates::Criterion::describe) + .unwrap_or_default() + }, + // A stored timestamp this crate never parses and never compares, and the + // one field that would need a clock. `src/main.rs` is the only module + // allowed one, so it stays empty rather than being invented here. + at: String::new(), + }); + recorded +} + +/// What the failing gate actually said, or `None` when it said nothing. +/// +/// Two sources because a gate has two ways of speaking and neither covers the +/// other. A command's output is **not** in its row — `GateAttempt::detail` is +/// empty for every non-review criterion — it arrives as `gate_output` sandbox +/// events, which is what [`crate::gates::output`] reads, filtered to the step this +/// attempt ran after so a retried gate carries the failure that caused the retry +/// rather than the one two turns ago. A review's reasons and an errored gate's +/// cause are the opposite: they are in the row and there is no sandbox event at +/// all, because nothing was executed. +fn gate_said( + attempts: &[io_harness::GateAttempt], + events: &[io_harness::SandboxEvent], +) -> Option { + let last = attempts.last()?; + crate::gates::output(events, last.step) + .or_else(|| (!last.detail.is_empty()).then(|| last.detail.clone())) +} + +/// 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 +/// say: a session with no `[app.io-cli.gates]` section would otherwise earn a +/// line under every turn saying so. +/// +/// **Scrollback and not the footer.** The verdict is an account of the turn above +/// it and outlives the keystroke that follows; `App::say` is answered by the next +/// key press. The driver is what calls [`App::record`] with this — the tone +/// travels with the sentence so the driver holds no branch about it. +pub fn gate_report( + attempts: &[io_harness::GateAttempt], + events: &[io_harness::SandboxEvent], +) -> Option<(Tone, String)> { + let standing = crate::gates::standing(attempts)?; + // `GateOutcome::as_str` verbatim, never a word of io-cli's own: the status + // line, the exit code and this sentence all have to spell one verdict the + // same way, and `as_str` is where that spelling is decided. + let word = standing.outcome.as_str(); + let tone = match standing.outcome { + io_harness::GateOutcome::Passed => Tone::Success, + // A gate that answered no is a refusal and not an error: nothing went + // wrong, the work was judged and did not meet the criterion. + io_harness::GateOutcome::Failed => Tone::Refused, + // `Errored`, and whatever a later io-harness adds — the enum is + // `#[non_exhaustive]`, and an outcome this release has not seen is not a + // pass. + _ => Tone::Error, + }; + let mut line = format!("gate {word} ({} criterion", standing.phase); + if standing.attempt > 1 { + line.push_str(&format!(", attempt {}", standing.attempt)); + } + line.push(')'); + if let Some(said) = gate_said(attempts, events) { + line.push_str(": "); + line.push_str(said.trim()); + } + Some((tone, line)) +} + +/// The prompt a failing gate drives the next turn with. +/// +/// **It takes no goal, and that absence is the whole design.** A retry that +/// re-sent the original prompt would look like a working retry — a second turn +/// runs, the model does more work — while telling the agent nothing about what +/// went wrong, and the criterion it is judged by would fail again for the same +/// reason. There is no parameter here it could be passed through, so that +/// implementation cannot be written. What the turn is told is what the criterion +/// asks, in [`crate::gates::Criterion::describe`]'s words — the same words the +/// first turn was judged by — and what the gate reported when it said no. +/// +/// The last sentence is not decoration. A gate is a command, a file or a rubric +/// the operator wrote; an agent handed a failing check and no instruction about it +/// will sometimes edit the check. +pub fn gate_retry( + criterion: &crate::gates::Criterion, + attempts: &[io_harness::GateAttempt], + events: &[io_harness::SandboxEvent], +) -> String { + let asks = criterion.describe(); + match gate_said(attempts, events) { + Some(said) => format!( + "The verification gate for this work did not pass. It asks: {asks}.\n\nThis is \ + what it reported:\n\n{}\n\nChange the work so that the gate passes. Do not \ + change the gate itself.", + said.trim() + ), + // A gate that failed silently — a command that printed nothing, an + // existence check on a file nobody wrote — still has a criterion to + // state, and stating it is more than the turn had before. + None => format!( + "The verification gate for this work did not pass. It asks: {asks}. It reported \ + nothing further.\n\nChange the work so that the gate passes. Do not change the \ + gate itself." + ), + } +} diff --git a/src/contract.rs b/src/contract.rs index bdc63aa..feeb2f6 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -346,15 +346,17 @@ pub fn gate_notice(config: &Config) -> Option { .unwrap_or_default(); match gates.criterion(working) { Err(refusal) => Some(refusal.to_string()), - Ok(Some(crate::gates::Criterion::Review { reviewer, .. })) => match config.provider_spec() { - None => Some(format!( + Ok(Some(crate::gates::Criterion::Review { reviewer, .. })) => { + match config.provider_spec() { + None => Some(format!( "the gate asks {reviewer} to review the work, but no provider is configured to \ reach it — this turn is not gated" )), - Some(spec) => crate::gates::reviewer(spec, &reviewer) - .err() - .map(|why| format!("the gate's reviewer could not be built: {why}")), - }, + Some(spec) => crate::gates::reviewer(spec, &reviewer) + .err() + .map(|why| format!("the gate's reviewer could not be built: {why}")), + } + } Ok(_) => None, } } diff --git a/src/gates.rs b/src/gates.rs index 16e61c8..dc78dc1 100644 --- a/src/gates.rs +++ b/src/gates.rs @@ -212,7 +212,10 @@ impl Settings { })); } - let rubric = self.rubric.clone().expect("one kind is set and it is the rubric"); + let rubric = self + .rubric + .clone() + .expect("one kind is set and it is the rubric"); let Some(reviewer) = self.reviewer.clone() else { return Err(Refusal::ReviewerMissing); }; diff --git a/src/main.rs b/src/main.rs index cfbcf43..560d091 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1504,6 +1504,10 @@ async fn loop_over Result>( vec![ Row::new("leave it as it is"), Row::new("remove this server"), + Row::with_detail( + "change one setting", + io_cli::servers::KEYS.join(", "), + ), ], ), Pick::McpRemove { @@ -1517,7 +1521,25 @@ async fn loop_over Result>( } // Row 0 is "leave it", the default that does nothing — // the shape `/skills` and `/plugin` both already use. + // Row 2 is the edit verb, and it descends rather than + // acting: which key is half the decision, and the other + // half is a value only the operator can type. Pick::McpRemove { id, at } => { + if index == 2 { + descended = Some(( + Picker::new( + format!("Change what about {id}?"), + io_cli::servers::KEYS + .iter() + .map(|key| Row::new(*key)) + .collect(), + ), + Pick::McpEdit { + id: id.clone(), + at: *at, + }, + )); + } if index == 1 { let edit = io_cli::servers::remove(at); match io_cli::configure::write(session.root(), at.scope, &[edit]) { @@ -1550,6 +1572,37 @@ async fn loop_over Result>( } } } + // **The key goes into the composer and the value is + // typed, which is `/config`'s own shape.** A picker + // cannot ask for a URL or a command line, and this + // product has exactly one surface that takes free text. + // + // The line carries the server's **id**, never `at`'s + // index: the composer holds it while the operator types, + // and a file they edit in another window in between would + // move the array under a carried index. `servers::At` is + // resolved again, from the file's own bytes, when the + // line comes back — which is also what decides the scope + // the write goes to, so no scope question is asked for a + // change to an entry that lives in exactly one file. + Pick::McpEdit { id, at } => { + if let Some(key) = io_cli::servers::KEYS.get(index) { + app.record( + Tone::Muted, + format!( + "{id}'s {key} is declared in the {} scope; type the value \ + after the key and press Enter", + io_cli::configure::Decided::File { + scope: at.scope, + path: Default::default(), + } + .word() + ), + ); + let prefix = io_cli::app::SERVER_KEY; + app.composer.set(&format!("/config {prefix}{id}.{key} ")); + } + } // Says what the skill is and offers the one change there // is. The view is read again rather than carried: between // the row being drawn and this keystroke the operator may @@ -2007,6 +2060,76 @@ async fn loop_over Result>( Some(key) => app.composer.set(&format!("/config {key} ")), None => {} }, + // The gates surface. Every row but the first names a key + // and goes to the composer, which is `/config`'s own + // shape and reaches `/config`'s own write — the value of + // a rubric or a path is the operator's to type, and a + // picker cannot ask for one. + Pick::Gates { keys, proposed } => { + match keys.get(index).map(String::as_str) { + Some(io_cli::app::PROPOSED_GATE) => { + // **Refused where the operator can still see + // what they asked for, rather than written + // and discovered at run start.** A section + // that already names a file or a rubric gains + // a second kind from this write, and + // `Settings::criterion` answers `Ambiguous` — + // which `contract::configured` honours by + // running every later turn with NO gate at + // all. That is a config edit that silently + // turns verification off, which is the exact + // failure F5 exists to make impossible. + // + // Checked against a prospective section built + // here rather than against the file, so the + // answer is about the write being offered and + // not about the one already there. The real + // working model goes in, because the refusal + // that compares a reviewer against it cannot + // fire on a guess. + let working = config + .provider_spec() + .map(io_cli::provider::model_of) + .unwrap_or_default() + .to_string(); + let stored = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.gates) + .unwrap_or_default(); + let prospective = io_cli::gates::Settings { + command: proposed.clone(), + ..stored + }; + match prospective.criterion(&working) { + Err(refusal) => { + app.record(Tone::Refused, refusal.to_string()) + } + Ok(_) => { + // The argv as TOML, built by + // `edit::array` rather than by a + // format string: an argument with a + // quote or a backslash in it is + // either a parse error or a + // different command. + let argv = proposed.clone().unwrap_or_default(); + let items: Vec<&str> = + argv.iter().map(String::as_str).collect(); + // `descended`, not `picker`: the + // assignment at the bottom of this + // block is unconditional, and `kind` + // still borrows `picker` here. + descended = Some(write_where( + session.root(), + "app.io-cli.gates.command".to_string(), + io_cli::edit::array(&items), + )); + } + } + } + Some(key) => app.composer.set(&format!("/config {key} ")), + None => {} + } + } Pick::ConfigScope { key, value, paths } => { // An index past the end puts nothing in the file, // which is the same answer every other picker arm @@ -2039,6 +2162,34 @@ async fn loop_over Result>( .word() ), ); + // **The one place a gates + // write is checked against + // the model that will do the + // work.** `/gates` sends its + // key rows through this arm, + // and the value is the + // operator's own typing — so + // a rubric with no reviewer, + // or a reviewer that IS the + // working model, is refused + // by `Settings::criterion` + // here rather than by + // io-harness at run start, + // where the failure arrives + // disconnected from the + // keystroke that caused it. + // Read off the reloaded + // configuration, so what is + // judged is the file as it + // now stands. Silent when + // the section is fine or + // absent, which is every + // other write. + if let Some(notice) = + io_cli::contract::gate_notice(&config) + { + app.record(Tone::Refused, notice); + } } Err(error) => app.record(Tone::Error, error), } @@ -2423,6 +2574,129 @@ async fn loop_over Result>( )); } } + // **Three questions, answered before anything is offered, because + // an operator opens this surface for exactly one of them.** What + // is being asked of a turn; why a section that is plainly in the + // file is not asking it; and how the last turn's gate actually + // went. The first is a fresh clone's question, the second is a + // typo's, and the third is the one asked after a turn came back + // red — and answering only the first would leave the other two + // discoverable nowhere in the product. + // + // **No contract is built here, deliberately.** `tests/contract.rs` + // counts `contract::session(` in this file by binding name *and* + // in total, and a sixth site is a new arm by that test's own + // reckoning. A surface that reads a criterion needs the `Config` + // and the workspace root, which is all `gates::Settings` and + // `gates::proposed_command` take between them. + Action::Gates => { + let root = session.root().to_path_buf(); + // The model the work will be done by, and the one thing the + // self-review refusal is decided against. Empty where no + // provider is configured, which `Settings::criterion` reads + // as "cannot clash" rather than as a model of that name — + // and it is read here rather than defaulted, because the + // caller that knows the model is the caller that must say it. + let working = config + .provider_spec() + .map(io_cli::provider::model_of) + .unwrap_or_default() + .to_string(); + let section = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.gates) + .unwrap_or_default(); + // Every key of the section, resolved by the same reader + // `/config` uses, so the two surfaces cannot disagree about + // what is in force or about which file decided it. + let settings: Vec = io_cli::configure::CATALOGUE + .iter() + .filter(|key| key.starts_with("app.io-cli.gates.")) + .map(|key| io_cli::configure::setting(&config, key)) + .collect(); + // Whichever key a file actually names decides the scope word, + // because that is the file the operator has to open. Three + // files are called `io.toml` and only one of them is theirs. + let decided = settings + .iter() + .find(|setting| setting.value.is_some()) + .map(|setting| setting.decided.word()) + .unwrap_or("default"); + // A refusal is NOT reported here. `gate_notice` below is the + // one place this product says why a section is not gating a + // run, and it covers both the refusals and the reviewer that + // could not be built — saying it twice would read as two + // different problems. + let criterion = section.criterion(&working).ok().flatten(); + match &criterion { + Some(criterion) => app.record( + Tone::Muted, + format!( + "the gate asks that {}, from the {decided} scope; a turn that \ + fails it earns {} more", + criterion.describe(), + match section.retries() { + 0 => "no turns".to_string(), + 1 => "one turn".to_string(), + many => format!("{many} turns"), + }, + ), + ), + None => app.record( + Tone::Muted, + "no gate is in force, so a turn that stops is a turn that finished", + ), + } + if let Some(notice) = io_cli::contract::gate_notice(&config) { + app.record(Tone::Refused, notice); + } + // How the last turn's gate went, read off the store rather + // than remembered — a resumed session did not run the turn + // whose gate it has to report, and a field that is only right + // when this process watched the run lies after every + // `/resume`. Folded through `app::gate_attempts` for the same + // reason the driver folds it: an existence criterion has no + // recorded row at all, and reading the store alone would + // report an ungated run. + if let Some(run_id) = last_run(&session, &store).map(|turn| turn.run_id) { + let attempts = io_cli::app::gate_attempts( + store.gate_attempts(run_id).unwrap_or_default(), + criterion.as_ref(), + &root, + ); + let events = store.sandbox_events(run_id).unwrap_or_default(); + if let Some((tone, line)) = io_cli::app::gate_report(&attempts, &events) { + app.record(tone, format!("the last turn's {line}")); + } + } + + // **The repository's own proposal first, and it is the one + // row that is an offer rather than a key.** `cargo test`, + // `npm test`, `pytest` — the command the toolchain detected, + // which io-cli holds no list of and could not invent. A gate + // an operator accepts with one keystroke is the difference + // between a gate configured and a gate intended, and a + // proposal that is absent is better than one that is wrong. + let mut rows = Vec::new(); + let mut keys: Vec = Vec::new(); + let proposed = io_cli::gates::proposed_command(&root, &config); + if let Some(argv) = &proposed { + rows.push(Row::with_detail( + format!("gate on `{}`", argv.join(" ")), + "the command this repository proposes for itself".to_string(), + )); + keys.push(io_cli::app::PROPOSED_GATE.to_string()); + } + // Then every key, drawn by `configure` rather than by rows + // spelled here: nothing under `tests/` links this file, so a + // label written in the driver is one no test can assert. + rows.extend(io_cli::configure::rows(&settings)); + keys.extend(settings.iter().map(|setting| setting.path.clone())); + picker = Some(( + Picker::new("What does done mean here?", rows), + Pick::Gates { keys, proposed }, + )); + } // **Both directories, and a failed discovery is drawn rather than // swallowed.** `view` reads the enabled set through the same // `Skills::discover` the run makes, so what this lists is what the @@ -2657,36 +2931,86 @@ async fn loop_over Result>( format!("{key} is {what} ({})", setting.decided.word()), ); } - Action::Config(Some((key, value))) => { - // Every scope, whether or not its file exists yet — writing a - // key into a scope for the first time is how this is used. + // **`/mcp`'s edit verb comes back through here, and it comes back + // addressed by the server's own id.** The composer line `/mcp` + // puts up is `mcp..`, never `mcp[3].`: an index is a + // position in one file's `[[mcp]]` array, and the operator is + // about to be asked which file to write into. Handing one list's + // index to another file's array is precisely the silent wrong + // delete 0.20.0 shipped in `pluginview::rows`, and `servers::At` + // exists so it cannot be spelled. So the entry is found again, by + // content, in whichever scope declares it — and the write goes to + // that scope with no question asked, because there is only one + // file the entry is in. + Action::Config(Some((key, value))) + if key.starts_with(io_cli::app::SERVER_KEY) && !value.is_empty() => + { let root = session.root().to_path_buf(); - let paths: Vec<(io_harness::config::Scope, std::path::PathBuf)> = [ - io_harness::config::Scope::User, - io_harness::config::Scope::Project, - io_harness::config::Scope::Local, - ] - .into_iter() - .filter_map(|scope| { - io_cli::configure::scope_path(&root, scope).map(|p| (scope, p)) - }) - .collect(); - - let rows: Vec = paths - .iter() - .map(|(scope, path)| { - let word = io_cli::configure::Decided::File { - scope: *scope, - path: path.clone(), - }; - Row::with_detail(word.word().to_string(), path.display().to_string()) - }) - .collect(); - - picker = Some(( - Picker::new(format!("Write {key} where?"), rows), - Pick::ConfigScope { key, value, paths }, - )); + match io_cli::app::server_key(&key) { + None => app.record( + Tone::Refused, + format!("{key} names no MCP server and no key of one"), + ), + Some((id, field)) => match io_cli::servers::declared_in(&root, id) { + None => app.record( + Tone::Refused, + format!( + "no configuration file in force declares {id}, so there is \ + nothing here to change" + ), + ), + // **Reported, never dropped.** `[[mcp]]` is not held + // to `deny_unknown_fields`, so a key io-harness does + // not know is written, accepted and ignored — the + // operator would be told their change landed while + // the server went on running the old value. That is + // the whole reason `servers::edit` returns an option. + Some(at) => match io_cli::servers::edit( + &at, + field, + &io_cli::app::server_value(field, &value), + ) { + None => app.record( + Tone::Refused, + format!( + "{field} is not a key an [[mcp]] entry may carry; the \ + keys are {}", + io_cli::servers::KEYS.join(", ") + ), + ), + Some(edit) => { + match io_cli::configure::write(&root, at.scope, &[edit]) { + Ok(()) => { + // Both halves, or the next turn talks + // to the server the file described at + // startup. + match io_cli::configure::reload(&root) { + Ok((fresh, stored)) => { + capabilities = + io_cli::contract::Capabilities::stored( + stored.as_ref(), + ); + config = fresh; + app.record( + Tone::Success, + format!( + "{id}'s {field} is now {value}; the \ + next turn talks to it that way" + ), + ); + } + Err(error) => app.record(Tone::Error, error), + } + } + Err(refusal) => app.record(Tone::Refused, refusal), + } + } + }, + }, + } + } + Action::Config(Some((key, value))) => { + picker = Some(write_where(session.root(), key, value)); } // A line typed with nothing after the word. Answered with what to // type rather than by opening a picker over three files it would @@ -3227,6 +3551,15 @@ async fn loop_over Result>( // pass rather than the queue being drained into a list up front. Command::Submit(text) => { let mut next = Some(text); + // **Every gate attempt this chain of turns has made, oldest + // first, and it is carried because io-harness cannot carry it.** + // Every turn is its own run, so `Store::gate_attempts` starts + // again at one on each retry — and `gates::may_retry`, which asks + // how many attempts the *work* has had, would then buy another + // turn for every one of them, forever. Accumulating here is what + // makes `retries = 1` mean one further turn rather than an + // unbounded loop against a real model. + let mut gated: Vec = Vec::new(); while let Some(text) = next.take() { // Rebuilt every turn rather than kept, because `remembered` // grows as the operator answers and the harness's own @@ -3322,7 +3655,7 @@ async fn loop_over Result>( .0; let effective = approval::session_policy(&policy, app.posture(), app.remembered()); - let stopped = turn( + let turned = turn( screen, inputs, &mut app, @@ -3367,7 +3700,7 @@ async fn loop_over Result>( // of the operator. Firing the queue here would turn one press // into three more turns against a conversation they had just // decided to steer somewhere else. - if stopped { + if turned.stopped { let dropped = app.forget_queued_prompts(); if dropped > 0 { let dash = app.theme.glyphs.dash; @@ -3377,6 +3710,91 @@ async fn loop_over Result>( ); } } + + // **The gate, read after the turn and before the next + // prompt.** Nothing that arrives through the event stream can + // say any of this: `EventKind::Sandbox` carries no detail + // payload at all, so the phase, the verdict and the output are + // the store's — which is also what makes it right after a + // `/resume`, because the rows outlive the process that watched + // the run. + // + // Every decision below is a library call. The two `if`s here + // are wiring — *did a run happen* and *did the operator stop + // it* — because nothing under `tests/` links this file and a + // decision written as a branch in the driver is one no test + // can drive and no sabotage can make fail. + let mut retry = None; + if let Some(run_id) = turned.ran { + let working = config + .provider_spec() + .map(io_cli::provider::model_of) + .unwrap_or_default() + .to_string(); + let section = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.gates) + .unwrap_or_default(); + let criterion = section.criterion(&working).ok().flatten(); + // **The fold, and it is the one thing here that cannot be + // skipped.** io-harness is handed `Verification::None` for + // a bare existence criterion — there is no honest + // counterpart for it in its enum — and its step loop + // returns `Finished` without ever reaching the gate, so + // the store holds NO row. Reading `gate_attempts` alone + // would report an ungated run as a run that passed. + // `app::gate_attempts` evaluates that one criterion here + // and appends the attempt io-harness never made. + gated.extend(io_cli::app::gate_attempts( + store.gate_attempts(run_id).unwrap_or_default(), + criterion.as_ref(), + session.root(), + )); + let events = store.sandbox_events(run_id).unwrap_or_default(); + if let Some(standing) = io_cli::gates::standing(&gated) { + // `GateOutcome::as_str` passed through verbatim: it + // spells `passed`, `failed` and `errored`, and the + // status line, the exit code and the scrollback all + // have to say one verdict the same way. + app.status.gate = Some(standing.outcome.as_str().to_string()); + app.status.gate_attempt = u32::try_from(standing.attempt).ok(); + } + // `record` and not `say`: a verdict is an account of the + // turn above it and would be gone at the next keystroke. + if let Some((tone, line)) = io_cli::app::gate_report(&gated, &events) { + app.record(tone, line); + } + // **A turn the operator stopped is never retried**, which + // is the same rule that drops the rest of the queue eight + // lines up: one press of the stop key must not start + // another turn. `retries = 0` drives nothing because + // `may_retry` answers no — the budget is not consulted + // here, it is passed in. + if !turned.stopped && io_cli::gates::may_retry(&gated, section.retries()) { + retry = criterion.as_ref().map(|criterion| { + io_cli::app::gate_retry(criterion, &gated, &events) + }); + } + } + // **The retry rides the queue this loop already drains.** It + // goes to the FRONT, so it is the next turn and the prompts an + // operator typed during this one keep their order behind it. + // A second loop written beside this one would be a second + // place `Ctrl+C`, the picture drain and the configuration + // refresh could drift. + if let Some(prompt) = retry { + app.requeue_prompts(vec![prompt]); + let dash = app.theme.glyphs.dash; + app.record( + Tone::Muted, + format!("the gate gets one more turn {dash} it is told what failed"), + ); + } else { + // The chain is over and the next prompt starts its own. + // Attempts carried across it would charge a fresh turn for + // a gate that failed two prompts ago. + gated.clear(); + } next = app.next_queued_prompt(); } } @@ -3504,12 +3922,22 @@ fn commit_viewed( } } -/// One turn, with the keyboard live throughout so `Ctrl+C` can reach it. +/// What the caller of [`turn`] still has to decide about. /// -/// Returns whether the operator asked this turn to stop — the one thing about a -/// turn the caller cannot see from outside it, and the thing the caller now has -/// to decide about, because a prompt queue that fired after a stop would make the -/// stop key start three more turns. +/// Two facts, and neither is visible from outside the turn. The stop is the older +/// one: a prompt queue that fired after `Ctrl+C` would make one press of the stop +/// key start three more turns. The run id is 0.24.0's, and it is here rather than +/// re-derived from the transcript because a turn that ended on an error has no run +/// to report a gate for — asking the store for "the last run" would find the +/// *previous* turn's and report its verdict under this one. +struct Turned { + /// Whether the operator asked this turn to stop. + stopped: bool, + /// The run that served it, where one ran to a result at all. + ran: Option, +} + +/// One turn, with the keyboard live throughout so `Ctrl+C` can reach it. #[allow(clippy::too_many_arguments)] async fn turn( screen: &mut Screen>, @@ -3542,7 +3970,7 @@ async fn turn( fold: bool, text: String, started: Instant, -) -> Result { +) -> Result { let (observer, mut events) = bridge::channel(); // The one way a turn is stopped from the interface, contained or not. Both // arms take a contract **and** a steer inbox since 0.17.0, and the stop key @@ -4189,6 +4617,15 @@ async fn turn( ); } + // The run this turn served, for the gate the caller reads afterwards. Only a + // turn that ran to a result has one: a run that came back `Err` was not judged, + // and a turn the operator abandoned has whatever io-harness had written when + // the future was dropped — neither is a verdict on the work. + let ran = match &outcome { + Some(Ok(result)) => Some(result.run_id), + _ => None, + }; + match outcome { // The operator's sentence in front of the harness's own line — see // `io_cli::failure`. A provider that will not take an image says so in the @@ -4222,7 +4659,7 @@ async fn turn( } app.status.elapsed = started.elapsed(); paint(screen, app)?; - Ok(stopped) + Ok(Turned { stopped, ran }) } /// Draw what a step changed, by asking the store what it recorded. @@ -4909,10 +5346,40 @@ enum Pick { /// rather than read off a label because a label is a rendered string and the /// key is what a write addresses. Config(Vec), - /// The MCP servers, in the order `servers::rows` drew them. Read-only in - /// this release: the write verbs are reached through `/config`, and the two - /// the roadmap named that io-harness cannot express are not offered at all. + /// The MCP servers, in the order `servers::rows` drew them. + /// + /// **No longer read-only.** Through 0.23.0 this surface could show a server + /// and remove one, and every other change had to be made by hand in the file + /// — a limitation the product had stated since 0.21.0 while + /// `servers::edit` sat in the library, tested and reachable from no + /// keystroke. 0.24.0 wires it: the row descends into [`Pick::McpRemove`], + /// which offers the edit as well as the removal, and the key goes to the + /// composer so the value can be typed. Both verbs address the entry through + /// its own [`io_cli::servers::At`] and never through a row index. Mcp, + /// One MCP server and the keys of it that may be changed, in `servers::KEYS` + /// order. The position is carried rather than re-derived for the reason + /// [`Pick::McpRemove`] carries one: this surface's row index addresses a + /// merged view across three scopes, and the `[[mcp]]` array a write is + /// spliced into is a different list entirely. + McpEdit { + id: String, + at: io_cli::servers::At, + }, + /// The gates surface, in the order its rows were drawn, carried as the key + /// each row stands for — parallel to the rows and built in the same pass, the + /// arrangement [`Pick::Config`] uses and for the same reason: a label is a + /// rendered string and a key is what a write addresses. + /// + /// One row is not a key: [`io_cli::app::PROPOSED_GATE`] is the sentinel for + /// the command this repository proposes for itself, which is an act rather + /// than a setting. `proposed` is the argv behind it, carried so the write and + /// the row cannot disagree — re-detecting it on the keystroke would re-read + /// the filesystem underneath the operator. + Gates { + keys: Vec, + proposed: Option>, + }, /// The provider chain, in the order `providers::rows` drew it — which is /// the order a turn tries it. Provider, @@ -5022,6 +5489,44 @@ enum Pick { }, } +/// The picker that asks which file a `key = value` write goes into. +/// +/// **Two steps rather than one because *which scope* is half the decision and this +/// product has three of them** — a write that guessed would put an operator's +/// credential in the file a repository ships. `/config` has asked it since 0.9.0 +/// and `/gates` asks exactly the same question about exactly the same three files, +/// so it is one function: two copies would be two answers the first time a scope +/// moved. +/// +/// Every scope is offered whether or not its file exists yet, because writing a +/// key into a scope for the first time is how this is used. +fn write_where(root: &std::path::Path, key: String, value: String) -> (Picker, Pick) { + let paths: Vec<(io_harness::config::Scope, std::path::PathBuf)> = [ + io_harness::config::Scope::User, + io_harness::config::Scope::Project, + io_harness::config::Scope::Local, + ] + .into_iter() + .filter_map(|scope| io_cli::configure::scope_path(root, scope).map(|p| (scope, p))) + .collect(); + + let rows: Vec = paths + .iter() + .map(|(scope, path)| { + let word = io_cli::configure::Decided::File { + scope: *scope, + path: path.clone(), + }; + Row::with_detail(word.word().to_string(), path.display().to_string()) + }) + .collect(); + + ( + Picker::new(format!("Write {key} where?"), rows), + Pick::ConfigScope { key, value, paths }, + ) +} + /// The completion picker over one directory of the workspace, or `None` when the /// policy leaves nothing in it to offer. /// diff --git a/tests/gates.rs b/tests/gates.rs index 01cf39d..11c9b01 100644 --- a/tests/gates.rs +++ b/tests/gates.rs @@ -264,7 +264,10 @@ fn f2_a_command_criterion_carries_the_exit_status_it_was_given() { panic!("a command criterion maps to a command verification"); }; assert_eq!(argv, ["npm", "run", "lint"]); - assert_eq!(expect_exit, 1, "a hardcoded zero would pass every other test"); + assert_eq!( + expect_exit, 1, + "a hardcoded zero would pass every other test" + ); let defaulted = Settings { command: Some(vec!["make".into(), "check".into()]), @@ -494,7 +497,10 @@ fn f7_the_retry_budget_defaults_to_one_and_zero_means_zero() { !gates::may_retry(&failed, 0), "zero retries must not buy one" ); - assert!(gates::may_retry(&failed, 1), "one retry buys the second turn"); + assert!( + gates::may_retry(&failed, 1), + "one retry buys the second turn" + ); let twice = [ attempt(1, "command", GateOutcome::Failed), @@ -529,8 +535,15 @@ fn f6_the_standing_is_the_last_attempt_and_its_number() { attempt(2, "command", GateOutcome::Passed), ]; let standing = gates::standing(&attempts).expect("two attempts have a standing"); - assert_eq!(standing.outcome, GateOutcome::Passed, "the last one, not the first"); - assert_eq!(standing.attempt, 2, "counted from one, so two attempts is two"); + assert_eq!( + standing.outcome, + GateOutcome::Passed, + "the last one, not the first" + ); + assert_eq!( + standing.attempt, 2, + "counted from one, so two attempts is two" + ); assert_eq!(standing.phase, "command"); } @@ -589,8 +602,11 @@ fn f3_the_test_command_is_proposed_by_the_repository_and_never_defaulted() { ); let python = tempfile::tempdir().expect("a temporary directory"); - std::fs::write(python.path().join("pyproject.toml"), "[project]\nname = \"x\"\n") - .expect("the marker is written"); + std::fs::write( + python.path().join("pyproject.toml"), + "[project]\nname = \"x\"\n", + ) + .expect("the marker is written"); let proposed = gates::proposed_command(python.path(), &plain()) .expect("a Python project proposes a test command"); assert!( diff --git a/tests/servers.rs b/tests/servers.rs index 63ae346..3fb2522 100644 --- a/tests/servers.rs +++ b/tests/servers.rs @@ -884,3 +884,189 @@ fn f6_a_shadowed_server_is_not_offered_for_editing_because_it_is_not_running() { array whole, so `docs` is not a server this workspace runs", ); } + +// --- F10: the edit verb, reachable from a keystroke at last -------------------- +// +// `servers::edit` has been in the library and tested since 0.21.0 with nothing +// calling it — a limitation the product has *stated* since 0.21.0. 0.24.0 wires +// it to `/mcp`, and the wiring is the part that can be got wrong: the value has +// to survive a trip through a composer, and the entry has to be found again +// afterwards by its own content rather than by the row index that showed it. + +/// The composer line `/mcp` writes carries the server's **id**, never its index. +/// +/// **This is the assertion that stops 0.20.0's wrong delete being re-shipped one +/// verb over.** `mcp[1].command` is a position in one file's `[[mcp]]` array; the +/// operator is about to type a value into a composer any other keystroke can +/// leave, and a file edited under them in between moves the array. So the line +/// names the entry by the one stable thing about it, and the driver resolves the +/// position again from the file's own bytes when the line comes back. +/// +/// Sabotage: put `at.index()` in the composer line instead. The round trip still +/// "works" on every one-server fixture and on this one — and silently edits the +/// wrong entry the moment the array has moved, which is exactly the shape that +/// survives every test written against a single-entry file. +#[test] +fn f10_the_edit_line_addresses_a_server_by_id_and_reresolves_its_position() { + let dir = tempfile::tempdir().expect("a temporary directory"); + std::fs::write( + dir.path().join(io_harness::config::PROJECT_FILE), + TWO_SERVERS, + ) + .expect("the project file"); + + // What the surface puts on the prompt, spelled exactly as the driver spells + // it — one constant, or the verb goes quietly dead the first time either half + // is retyped. + let line = format!("{}search.command", io_cli::app::SERVER_KEY); + let (id, key) = io_cli::app::server_key(&line).expect("the line names a server and a key"); + assert_eq!(id, "search"); + assert_eq!(key, "command"); + assert!( + !line.contains('['), + "the line carries an id, not a position in an array" + ); + + // And the position comes from the file, not from the line. + let at = servers::declared_in(dir.path(), id).expect("the project file declares it"); + assert_eq!( + at.index(), + 1, + "`search` is the second entry, found by content" + ); + assert_eq!( + at.scope, + Scope::Project, + "the scope comes from the lookup too, so the write needs no second question", + ); + + // An id with a dot in it still splits at the key, because the KEY is the part + // this crate knows the spelling of and a server may be called anything. + assert_eq!( + io_cli::app::server_key("mcp.some.dotted.id.url"), + Some(("some.dotted.id", "url")), + ); + // And the shapes that are not an edit at all. + for not_one in ["mcp.", "mcp.docs", "app.io-cli.theme", "mcp.docs."] { + assert_eq!( + io_cli::app::server_key(not_one), + None, + "{not_one} is not an `mcp..` line and must not be treated as one", + ); + } +} + +/// A key that is not one an `[[mcp]]` entry may carry is refused, not written. +/// +/// **The refusal is the whole reason `servers::edit` returns an option.** `[[mcp]]` +/// is not held to `deny_unknown_fields`, so `comand = "mcp-find"` is written, +/// accepted by io-harness, and ignored — `configure::write`'s round trip parses it +/// happily and the operator is told their change landed while the server goes on +/// running the old command. There is no other write in this crate whose failure +/// says nothing at all. +#[test] +fn f10_a_key_no_mcp_entry_carries_is_refused_rather_than_written() { + let at = At::of(Scope::User, TWO_SERVERS, "docs").expect("the fixture names `docs`"); + assert!( + servers::edit(&at, "comand", "\"mcp-find\"").is_none(), + "a typo'd key must not become a line io-harness silently ignores", + ); + for key in servers::KEYS { + assert!( + servers::edit(&at, key, "\"x\"").is_some(), + "{key} is an editable key and the verb must reach it", + ); + } +} + +/// What an operator typed becomes TOML, and the four kinds are four shapes. +/// +/// **A composer hands over a person's typing, and `servers::edit` takes TOML +/// source.** The spelling every call site reaches for — `format!("\"{typed}\"")` — +/// is a parse error or a *different value* the moment the text carries a quote or +/// a backslash, and a Windows command path is full of the second. So the assertion +/// that matters is the one on a value with a backslash in it: a naive +/// implementation passes every other case here. +#[test] +fn f10_a_typed_value_becomes_the_toml_its_key_needs() { + // A string, escaped. Nothing else in this test would catch a `format!`. + assert_eq!( + io_cli::app::server_value("command", r"C:\tools\mcp.exe"), + r#""C:\\tools\\mcp.exe""#, + "a backslash written raw is a different path, and a quote is a parse error", + ); + assert_eq!( + io_cli::app::server_value("url", "https://example.test/mcp"), + "\"https://example.test/mcp\"", + ); + // A list, written the way a person writes a command line. Asserted by shape + // rather than byte-for-byte: the spacing inside the brackets is `toml`'s own + // and an assertion on it would go blind the first time that crate reformatted. + let args = io_cli::app::server_value("args", "--index ./idx"); + assert!( + args.starts_with('[') && args.contains("\"--index\"") && args.contains("\"./idx\""), + "a command line becomes an array of arguments, not one string: {args}", + ); + // A number stays a number: quoting it would be a string io-harness refuses. + assert_eq!(io_cli::app::server_value("timeout_secs", " 30 "), "30"); + // An inline table has exactly one spelling and is passed through, so + // `edit::apply`'s own parse is what refuses a malformed one. + assert_eq!( + io_cli::app::server_value("env", "{ TOKEN = \"abc\" }"), + "{ TOKEN = \"abc\" }", + ); + + // End to end: the typing, the TOML, the edit, the file — and the file still + // parses, which is what a value that was quoted by hand would not guarantee. + let at = At::of(Scope::User, TWO_SERVERS, "docs").expect("the fixture names `docs`"); + let edit = servers::edit( + &at, + "command", + &io_cli::app::server_value("command", r"C:\tools\mcp.exe"), + ) + .expect("`command` is an editable key"); + let after = io_cli::edit::apply(TWO_SERVERS, &[edit]).expect("the write parses back"); + assert!(after.contains(r#"command = "C:\\tools\\mcp.exe""#)); + assert!( + after.contains("command = \"mcp-search\""), + "the other entry is untouched", + ); +} + +/// The driver reaches the verb, and reaches it through `At`. +/// +/// Nothing under `tests/` links `src/main.rs`, so the wiring is read as text — +/// the instrument `tests/contract.rs` and `tests/context_share.rs` already use for +/// exactly this. Two properties: the edit row exists at all, and the write is +/// aimed at the scope the lookup returned rather than at a scope somebody picked. +#[test] +fn f10_the_driver_writes_into_the_scope_the_lookup_found() { + let driver = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"), + ) + .expect("the driver"); + // Whitespace removed, because rustfmt decides where a deeply nested call + // breaks and an assertion about where a newline sits would go blind the first + // time one of these gained an argument. + let flat: String = driver.chars().filter(|c| !c.is_whitespace()).collect(); + + assert!( + flat.contains("io_cli::servers::edit("), + "`servers::edit` is reachable from no keystroke, which is the limitation \ + 0.24.0 exists to remove", + ); + assert!( + flat.contains("io_cli::servers::declared_in(&root,id)"), + "the entry is found again by its id when the composer line comes back", + ); + assert!( + flat.contains("io_cli::configure::write(&root,at.scope,&[edit])"), + "the write goes to the scope the lookup returned; asking the operator \ + which file would let them aim an index at a different file's array", + ); + assert!( + flat.contains("io_cli::app::SERVER_KEY"), + "the composer line is built from the one constant the driver also matches \ + on, so the verb cannot go dead by either half being retyped", + ); +} diff --git a/tests/steer.rs b/tests/steer.rs index 49a1ce0..84561b3 100644 --- a/tests/steer.rs +++ b/tests/steer.rs @@ -1,5 +1,12 @@ //! F5 — a queued line reaches the turn that is still running, on both arms. //! +//! **And F7 since 0.24.0, at the bottom of the file, because it is the same +//! queue.** A failing verification gate buys another turn by putting a prompt at +//! the FRONT of the queue this file is about — not by opening a second driver +//! loop beside the one that already drains it. So the retry's mechanism belongs +//! next to the mechanism it borrows, and the same instrument proves it: the +//! library halves are driven directly, and the driver's wiring is read as text. +//! //! **What this file can and cannot prove, stated first, because the gap is the //! whole reason the file is shaped like this.** //! @@ -347,3 +354,301 @@ fn f5_a_send_after_the_turn_has_ended_is_refused() { "and the other two go the same way", ); } + +// --------------------------------------------------------------------------- +// F7 — a failing gate buys another turn, through this same queue +// --------------------------------------------------------------------------- +// +// The retry is not a second driver loop. It is a prompt at the FRONT of the queue +// this file is otherwise about, so a turn earned by a gate and a turn typed by an +// operator go through one `turn` call, get one echo, one clock and one `Ctrl+C`. +// What is provable here is the mechanism — the queue's order, the prompt's +// contents, and the fact that the wiring in the driver exists at all. What is not +// is that a real model reads it, which is the live run's to close. + +/// One recorded gate evaluation. `at` is a stored string this crate never parses. +fn attempt(step: u32, phase: &str, outcome: io_harness::GateOutcome) -> io_harness::GateAttempt { + io_harness::GateAttempt { + id: 1, + step, + phase: phase.into(), + outcome, + detail: String::new(), + at: String::new(), + } +} + +/// F7 — the retry prompt carries the criterion and the failure, and not the goal. +/// +/// **A retry that re-sent the original prompt is the sabotage this criterion +/// names**, and it is the one that looks like it works: a second turn runs, the +/// model does more work, and the gate fails again for exactly the reason nobody +/// told it about. `app::gate_retry` has no parameter a goal could arrive through, +/// so that implementation cannot be written — and this asserts the two things it +/// must carry instead. +#[test] +fn f7_the_retry_prompt_carries_the_criterion_and_what_failed() { + let criterion = io_cli::gates::Criterion::Command { + argv: vec!["cargo".into(), "test".into()], + expect_exit: 0, + }; + let attempts = [attempt(4, "command", io_harness::GateOutcome::Failed)]; + let events = [ + io_harness::SandboxEvent::gate_output(9, 3, "an older failure, two turns ago"), + io_harness::SandboxEvent::gate_output(9, 4, "error[E0433]: failed to resolve `frobnicate`"), + ]; + + let prompt = io_cli::app::gate_retry(&criterion, &attempts, &events); + assert!( + prompt.contains("cargo test"), + "the retried turn is told what it is judged by, in the words it was judged \ + by: {prompt}", + ); + assert!( + prompt.contains("E0433") && prompt.contains("frobnicate"), + "a retry that does not carry the failure is a retry that repeats it: {prompt}", + ); + assert!( + !prompt.contains("two turns ago"), + "the output is the one for the step this attempt ran after, not the first \ + failure of the run: {prompt}", + ); + assert!( + prompt.contains("Do not change the gate"), + "an agent handed a failing check and no instruction about it will sometimes \ + edit the check", + ); + + // A review says its reasons in the ROW and prints nothing, so a reader that + // only looked at sandbox events would retry with no explanation at all. + let reviewed = io_cli::gates::Criterion::Review { + rubric: "every public item has a doc comment".into(), + reviewer: "vendor/judge".into(), + allow_self_review: false, + }; + let mut refused = attempt(2, "review", io_harness::GateOutcome::Failed); + refused.detail = "three functions carry no documentation".into(); + let prompt = io_cli::app::gate_retry(&reviewed, &[refused], &[]); + assert!( + prompt.contains("three functions carry no documentation"), + "a review's reasons are in its row, not in a sandbox event: {prompt}", + ); + + // And a gate that failed silently still states the criterion, which is more + // than the turn had before. + let quiet = io_cli::app::gate_retry(&criterion, &attempts, &[]); + assert!( + quiet.contains("cargo test") && !quiet.is_empty(), + "a command that printed nothing still has a criterion to state: {quiet}", + ); +} + +/// F7 — an existence criterion is judged here, or it is not judged at all. +/// +/// **This is the single easiest defect in the release to ship.** +/// `Criterion::verification` maps a bare `file` criterion to `Verification::None`, +/// because there is no honest counterpart for it in io-harness's enum — and a run +/// carrying `Verification::None` never reaches the gate: the step loop returns +/// `Finished` the moment the agent stops calling tools, so the store holds no row +/// at all. A caller that read `Store::gate_attempts` and stopped would draw +/// nothing, retry nothing, and report an ungated run. +/// +/// Sabotage: return the recorded rows unchanged. Every command and review gate in +/// the product still works, the suite stays green, and an operator who asked for a +/// file to exist gets a session that never once looks for it. +#[test] +fn f7_an_existence_criterion_is_evaluated_where_io_harness_did_not() { + let dir = tempfile::tempdir().expect("a temporary directory"); + let criterion = io_cli::gates::Criterion::File { + file: std::path::PathBuf::from("REPORT.md"), + contains: None, + }; + + // Nothing recorded, because nothing was asked of io-harness. + let missing = io_cli::app::gate_attempts(Vec::new(), Some(&criterion), dir.path()); + let standing = io_cli::gates::standing(&missing) + .expect("an existence criterion that reports nothing is an ungated run"); + assert_eq!(standing.outcome, io_harness::GateOutcome::Failed); + assert!( + io_cli::gates::may_retry(&missing, 1), + "and a file that is not there earns the turn that writes it", + ); + + std::fs::write(dir.path().join("REPORT.md"), "done").expect("the report"); + let written = io_cli::app::gate_attempts(Vec::new(), Some(&criterion), dir.path()); + assert_eq!( + io_cli::gates::standing(&written) + .expect("a standing") + .outcome, + io_harness::GateOutcome::Passed, + ); + + // **A `none` row is dropped, not kept.** A run that spends its step cap DOES + // reach the gate with `Verification::None`, which passes trivially and records + // `phase = "none", passed` — a row saying a criterion that checked nothing was + // satisfied. Left as the last row it would report a missing file as a pass. + std::fs::remove_file(dir.path().join("REPORT.md")).expect("take it away again"); + let folded = io_cli::app::gate_attempts( + vec![attempt(7, "none", io_harness::GateOutcome::Passed)], + Some(&criterion), + dir.path(), + ); + assert_eq!( + io_cli::gates::standing(&folded) + .expect("a standing") + .outcome, + io_harness::GateOutcome::Failed, + "a row for a criterion that checked nothing must not stand as the verdict", + ); + assert!( + folded.iter().all(|attempt| attempt.phase != "none"), + "the row io-harness wrote about checking nothing is not part of the count", + ); + + // Every other criterion is io-harness's to judge, and the rows come back + // untouched — including the empty list, which is an ungated run. + let command = io_cli::gates::Criterion::Command { + argv: vec!["make".into()], + expect_exit: 0, + }; + let rows = vec![attempt(1, "command", io_harness::GateOutcome::Passed)]; + assert_eq!( + io_cli::app::gate_attempts(rows.clone(), Some(&command), dir.path()), + rows, + "a command gate is not re-judged here", + ); + assert!(io_cli::app::gate_attempts(Vec::new(), None, dir.path()).is_empty()); +} + +/// F6 — the scrollback line spells the verdict the way everything else does. +#[test] +fn f6_the_gate_line_names_the_phase_the_verdict_and_what_it_printed() { + let attempts = [ + attempt(3, "command", io_harness::GateOutcome::Failed), + attempt(4, "command", io_harness::GateOutcome::Failed), + ]; + let events = [io_harness::SandboxEvent::gate_output( + 9, + 4, + "2 tests failed", + )]; + let (tone, line) = + io_cli::app::gate_report(&attempts, &events).expect("a gated turn has a line"); + assert_eq!( + tone, + io_cli::theme::Tone::Refused, + "a gate that answered no is a refusal, not an error: nothing went wrong", + ); + assert!( + line.contains("failed") && line.contains("command") && line.contains("2 tests failed"), + "the phase, the verdict and what it printed: {line}", + ); + assert!( + line.contains("attempt 2"), + "the attempt is on the line once there has been more than one: {line}", + ); + + // `GateOutcome::as_str` verbatim, because the status line, the exit code and + // this sentence all have to spell one verdict the same way. + let (_, passed) = io_cli::app::gate_report( + &[attempt(1, "contains", io_harness::GateOutcome::Passed)], + &[], + ) + .expect("a line"); + assert!( + passed.contains(io_harness::GateOutcome::Passed.as_str()), + "the word is the harness's own: {passed}", + ); + assert!( + !passed.contains("attempt"), + "every gate that ran at all ran once, so `attempt 1` tells nobody anything", + ); + + assert!( + io_cli::app::gate_report(&[], &[]).is_none(), + "a turn nothing gated earns no line; a session with no gates section would \ + otherwise carry one under every turn", + ); +} + +/// F7 — the driver drives the retry through the queue it already drains. +/// +/// Nothing under `tests/` links `src/main.rs`, so the wiring is read as text, the +/// way `tests/contract.rs` and `tests/context_share.rs` read it. Four properties, +/// each of which is a way the retry can be wrong while every test above still +/// passes: +/// +/// 1. **One loop.** The retry goes back through `next`, so it gets the +/// configuration refresh, the picture drain and the stop key that every other +/// turn gets. A second loop beside this one is where those drift apart. +/// 2. **The budget is asked of the library.** `gates::may_retry` deliberately +/// differs from `GateOutcome::is_retryable` — both `Failed` and `Errored` earn +/// another *turn*, because the tree changes between turns — and a driver that +/// reached for the method would silently stop retrying the commonest failure +/// there is. +/// 3. **A stopped turn is never retried.** One press of the stop key must not +/// start another turn. +/// 4. **The prompt is composed by the library.** A sentence built in this file +/// would be one no test can read. +#[test] +fn f7_the_retry_rides_the_drivers_own_queue() { + let text = driver(); + let flat = squashed(&text); + + assert!( + flat.contains("next=app.next_queued_prompt();"), + "the retry is drained by the loop that was already there", + ); + assert!( + flat.contains("app.requeue_prompts(vec![prompt])"), + "and it goes to the FRONT of that queue, so it is the next turn and the \ + prompts typed during this one keep their order behind it", + ); + assert!( + flat.contains("io_cli::gates::may_retry(&gated,section.retries())"), + "the budget is the library's answer, asked with the operator's own number", + ); + assert!( + !text.contains("is_retryable"), + "`GateOutcome::is_retryable` answers a different question — whether the \ + SAME criterion over the SAME tree could say something else — and using it \ + would stop retrying every gate that merely failed", + ); + assert!( + flat.contains("!turned.stopped&&io_cli::gates::may_retry("), + "a turn the operator interrupted is not retried", + ); + assert!( + flat.contains("io_cli::app::gate_retry(criterion,&gated,&events)"), + "the prompt is composed in the library, where a test can read it", + ); + + // The attempts are accumulated across the chain. Every turn is its own run, so + // `Store::gate_attempts` restarts at one on each retry — a driver that passed + // only the current run's rows to `may_retry` would buy another turn for every + // one of them, forever, against a real model. + assert!( + flat.contains("gated.extend(io_cli::app::gate_attempts("), + "the chain's attempts accumulate; `retries = 1` means one further turn, \ + not an unbounded loop", + ); + assert!( + flat.contains("gated.clear();"), + "and a chain that ends releases them, or the next prompt is charged for a \ + gate that failed two prompts ago", + ); + + // The standing comes off the STORE. `EventKind::Sandbox` carries no detail + // payload, so nothing in the event stream can say which phase failed or what + // it printed — and reading rows is also what makes this right after a + // `/resume`, where this process never watched the run. + assert!( + flat.contains("store.gate_attempts(run_id)") + && flat.contains("store.sandbox_events(run_id)"), + "the phase, the verdict and the output are the store's", + ); + assert!( + flat.contains("app.status.gate=Some(standing.outcome.as_str().to_string());"), + "the status word is the harness's own spelling, passed through", + ); +} From f436fadd434eff3cc0b3aba6f4254d852d118272 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:15:50 +0530 Subject: [PATCH 10/16] refactor(gates): build the reviewer in one place, and earn the exemption tests/provider.rs requires every vendor constructor to be written exactly once outside the wizard handshake and the /provider panel, so the interactive and headless entry points cannot drift apart. It refused this release twice: once for constructing in gates.rs, once for constructing in provider.rs beside the real site. A reviewer is a third construction, like the wizard's ping. It never drives a turn and deliberately runs a different model from the one doing the work, so merging it into the session's maker would make the second opinion change model whenever /model did. src/reviewer.rs is excluded by name like the other two, and held to that boundary by a new test: it must build a ModelReviewer, may name nothing that drives a turn, and may be called from the contract and nowhere else. --- src/contract.rs | 4 +- src/gates.rs | 111 ++++------------------------------------------ src/lib.rs | 1 + src/provider.rs | 10 ++--- src/reviewer.rs | 108 ++++++++++++++++++++++++++++++++++++++++++++ tests/events.rs | 2 +- tests/exec.rs | 6 ++- tests/provider.rs | 90 +++++++++++++++++++++++++++++++++++-- 8 files changed, 215 insertions(+), 117 deletions(-) create mode 100644 src/reviewer.rs diff --git a/src/contract.rs b/src/contract.rs index feeb2f6..052d9ca 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -326,7 +326,7 @@ fn criterion_for( match &criterion { crate::gates::Criterion::Review { reviewer, .. } => { let spec = config.provider_spec()?; - let built = crate::gates::reviewer(spec, reviewer).ok()?; + let built = crate::reviewer::build(spec, reviewer).ok()?; Some((criterion.clone(), Some(built))) } _ => Some((criterion, None)), @@ -352,7 +352,7 @@ pub fn gate_notice(config: &Config) -> Option { "the gate asks {reviewer} to review the work, but no provider is configured to \ reach it — this turn is not gated" )), - Some(spec) => crate::gates::reviewer(spec, &reviewer) + Some(spec) => crate::reviewer::build(spec, &reviewer) .err() .map(|why| format!("the gate's reviewer could not be built: {why}")), } diff --git a/src/gates.rs b/src/gates.rs index dc78dc1..5464ed4 100644 --- a/src/gates.rs +++ b/src/gates.rs @@ -41,12 +41,8 @@ use std::fmt; use std::path::{Path, PathBuf}; -use std::sync::Arc; -use io_harness::{ - Anthropic, Compatible, Config, GateAttempt, GateOutcome, ModelReviewer, OpenAi, OpenRouter, - ProviderSpec, Reviewer, SandboxEvent, Verification, -}; +use io_harness::{Config, GateAttempt, GateOutcome, SandboxEvent, Verification}; use serde::{Deserialize, Serialize}; /// The `[app.io-cli.gates]` section exactly as an operator wrote it. @@ -450,104 +446,13 @@ pub fn proposed_command(root: &Path, config: &Config) -> Option> { (!tuned.test.is_empty()).then_some(tuned.test) } -/// A reviewer that asks `model` through `spec`, ready for `with_reviewer`. -/// -/// **Nothing here calls a provider.** It constructs `ModelReviewer`, which holds -/// its own provider and its own model, and hands it to io-harness to call when the -/// run loop reaches the gate. That is also what keeps `tests/dependencies.rs`'s -/// count of provider calls in this crate at one — the wizard's credential ping in -/// [`crate::verify`] — which is the gate that says this crate has not grown an -/// agent loop. -/// -/// `Provider` is not dyn-compatible, so each variant is constructed in its own arm -/// and boxed as `Arc` afterwards; `Reviewer` *is* dyn-compatible, -/// which is what makes one return type possible at all. The arms mirror -/// [`crate::verify::credential`] exactly, including the fallback to each vendor's -/// own environment variable, because a reviewer configured the way the session's -/// provider is configured should authenticate the way the session does. -/// -/// The error is the provider's own message, for the same reason the wizard keeps -/// it: a refusal an operator can act on names the endpoint and the key, and -/// "could not build a reviewer" names neither. -pub fn reviewer(spec: &ProviderSpec, model: &str) -> Result, String> { - match spec { - ProviderSpec::OpenRouter { api_key, .. } => { - let key = api_key_or_env(api_key, "OPENROUTER_API_KEY")?; - Ok(Arc::new(ModelReviewer::new( - crate::provider::Printable::new(OpenRouter::new(key, model)), - model, - ))) - } - ProviderSpec::Anthropic { api_key, .. } => { - let key = api_key_or_env(api_key, "ANTHROPIC_API_KEY")?; - Ok(Arc::new(ModelReviewer::new( - crate::provider::Printable::new(Anthropic::new(key, model)), - model, - ))) - } - ProviderSpec::OpenAi { api_key, .. } => { - let key = api_key_or_env(api_key, "OPENAI_API_KEY")?; - Ok(Arc::new(ModelReviewer::new( - crate::provider::Printable::new(OpenAi::new(key, model)), - model, - ))) - } - ProviderSpec::Compatible { - base_url, - preset, - api_key, - auth, - .. - } => { - // The model is the reviewer's, never the endpoint's configured one: - // the entire criterion is that a second model reads the work, and - // reusing the spec's model is the mistake it exists to prevent. - let secret = api_key.clone().unwrap_or_default(); - let compatible = match (preset, base_url) { - (Some(preset), _) => { - Compatible::preset(preset, secret, model).map_err(|error| error.to_string())? - } - (None, Some(base)) => Compatible::new( - base, - auth.unwrap_or(io_harness::Auth::Bearer), - secret, - model, - ), - (None, None) => { - return Err("this endpoint names neither a preset nor a base URL".into()) - } - }; - Ok(Arc::new(ModelReviewer::new( - crate::provider::Printable::new(compatible), - model, - ))) - } - // `ProviderSpec` is `#[non_exhaustive]`. A provider this release has not - // seen cannot be built into a reviewer, and refusing at write time is the - // whole reason this module validates anything. - other => Err(format!( - "this release does not know how to review with a {other:?} provider yet" - )), - } -} - -/// The key from the spec, or from the provider's own environment variable. -/// -/// A near-twin of the wizard's own resolver, kept separate on purpose: that one is -/// private to [`crate::verify`], and importing it would make a module about live -/// provider calls a dependency of a module whose entire claim is that it makes -/// none. -fn api_key_or_env(api_key: &Option, var: &str) -> Result { - if let Some(key) = api_key { - return Ok(key.clone()); - } - match std::env::var(var) { - Ok(key) if !key.is_empty() => Ok(key), - _ => Err(format!( - "no key was given and ${var} is not set in this shell" - )), - } -} +// **The reviewer is built in `crate::provider` and not here, and the gate that +// says so is `tests/provider.rs`.** Every vendor type must be constructed in +// exactly one place outside the wizard's handshake and the `/provider` panel, so +// that the interactive and the headless entry points cannot drift apart — and a +// reviewer built here would have been a second site with its own key resolution, +// which is precisely the drift that gate exists to catch. It caught this release +// writing one. See `crate::provider::reviewer`. /// One sentence an operator can act on, for each way a section is refused. /// diff --git a/src/lib.rs b/src/lib.rs index 605bae9..ef54c3a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,6 +72,7 @@ pub mod queue; pub mod recall; pub mod reload; pub mod resume; +pub mod reviewer; pub mod rewind; pub mod servers; pub mod sessions; diff --git a/src/provider.rs b/src/provider.rs index 9c81e25..2fe76d4 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -236,11 +236,11 @@ impl Provider for Watched

{ /// `ModelReviewer

` implements `Reviewer` — the trait `TaskContract::with_reviewer` /// takes — only for `P: Provider + Debug + Send + Sync`, because `ModelReviewer` /// derives `Debug` and the derive puts the bound on `P`. **None of io-harness's -/// own provider types implements `Debug`**, so `ModelReviewer::new(OpenRouter::new(..), ..)` -/// cannot be handed to `with_reviewer` at all, and the model-judged half of the -/// verification pillar is unreachable from any crate that uses the providers the -/// harness ships. Reported upstream as io-harness#213; this is io-cli shipping -/// around it rather than waiting. +/// own provider types implements `Debug`**, so a `ModelReviewer` built over one of +/// them cannot be handed to `with_reviewer` at all, and the model-judged half of +/// the verification pillar is unreachable from any crate that uses the providers +/// the harness ships. Reported upstream as io-harness#213; this is io-cli +/// shipping around it rather than waiting. /// /// The `Debug` impl prints the vendor name and **never the credential**. That is /// the whole reason it is written by hand rather than derived: a derive here would diff --git a/src/reviewer.rs b/src/reviewer.rs new file mode 100644 index 0000000..dbc6869 --- /dev/null +++ b/src/reviewer.rs @@ -0,0 +1,108 @@ +//! The second model, the one that judges the work rather than doing it. +//! +//! **A module of its own, and `tests/provider.rs` is the reason.** That gate says +//! every vendor constructor is written exactly once outside the wizard's +//! handshake and the `/provider` panel, so that the interactive and the headless +//! entry points cannot drift apart — one site builds the provider a turn runs on. +//! A reviewer is not that provider. It is a third construction, like +//! [`crate::verify`]'s credential ping: it answers a question and is thrown away, +//! it never drives a turn, and it deliberately runs a *different* model from the +//! one doing the work. So it is excluded by name, exactly as the handshake is, +//! and held to that boundary by a test rather than trusted. +//! +//! The gate caught this release writing the construction into +//! [`crate::gates`] first, then into [`crate::provider`] beside the real one. Both +//! would have been a second site with its own key resolution. This is the third +//! answer and the honest one. +//! +//! **Nothing here calls a provider.** It constructs `ModelReviewer`, which holds +//! its own provider and its own model, and hands it to io-harness to call when the +//! run loop reaches the gate. That is what keeps `tests/dependencies.rs`'s count of +//! provider calls in this crate at one — the wizard's ping — which is the gate that +//! says this crate has not grown an agent loop. + +use std::sync::Arc; + +use io_harness::{ + Anthropic, Compatible, ModelReviewer, OpenAi, OpenRouter, ProviderSpec, Reviewer, +}; + +use crate::provider::{key_for, Printable}; + +/// A reviewer that asks `model` through `spec`, ready for `with_reviewer`. +/// +/// `Provider` is not dyn-compatible, so each variant is constructed in its own arm +/// and boxed as `Arc` afterwards; `Reviewer` *is* dyn-compatible, +/// which is what makes one return type possible at all. Each provider is wrapped +/// in [`Printable`] to satisfy the `Debug` bound io-harness's own provider types +/// cannot — see that type for the upstream report. +/// +/// The key falls back to the vendor's own environment variable through +/// [`key_for`], the same resolver every other provider in this crate gets: a +/// reviewer configured the way the session's provider is configured should +/// authenticate the way the session does. +/// +/// The error is the provider's own message wherever there is one. A refusal an +/// operator can act on names the endpoint and the key; "could not build a +/// reviewer" names neither. +pub fn build(spec: &ProviderSpec, model: &str) -> Result, String> { + match spec { + ProviderSpec::OpenRouter { api_key, .. } => { + let key = key_for(api_key.clone(), "OPENROUTER_API_KEY")?; + Ok(Arc::new(ModelReviewer::new( + Printable::new(OpenRouter::new(key, model)), + model, + ))) + } + ProviderSpec::Anthropic { api_key, .. } => { + let key = key_for(api_key.clone(), "ANTHROPIC_API_KEY")?; + Ok(Arc::new(ModelReviewer::new( + Printable::new(Anthropic::new(key, model)), + model, + ))) + } + ProviderSpec::OpenAi { api_key, .. } => { + let key = key_for(api_key.clone(), "OPENAI_API_KEY")?; + Ok(Arc::new(ModelReviewer::new( + Printable::new(OpenAi::new(key, model)), + model, + ))) + } + ProviderSpec::Compatible { + base_url, + preset, + api_key, + auth, + .. + } => { + // The model is the reviewer's, never the endpoint's configured one: + // the entire criterion is that a *second* model reads the work, and + // reusing the spec's model is the mistake it exists to prevent. + let secret = api_key.clone().unwrap_or_default(); + let compatible = match (preset, base_url) { + (Some(preset), _) => { + Compatible::preset(preset, secret, model).map_err(|error| error.to_string())? + } + (None, Some(base)) => Compatible::new( + base, + auth.unwrap_or(io_harness::Auth::Bearer), + secret, + model, + ), + (None, None) => { + return Err("this endpoint names neither a preset nor a base URL".into()) + } + }; + Ok(Arc::new(ModelReviewer::new( + Printable::new(compatible), + model, + ))) + } + // `ProviderSpec` is `#[non_exhaustive]`. A provider this release has not + // seen cannot be built into a reviewer, and refusing where the operator is + // still looking at what they typed is the whole point of refusing at all. + other => Err(format!( + "this release does not know how to review with a {other:?} provider yet" + )), + } +} diff --git a/tests/events.rs b/tests/events.rs index 34bde31..a4cf448 100644 --- a/tests/events.rs +++ b/tests/events.rs @@ -2374,7 +2374,7 @@ fn the_gate_and_review_lines_survive_the_ascii_set_and_plain_mode() { } assert!( - said.chars().all(|c| c.is_ascii()), + said.is_ascii(), "a character the ASCII set cannot draw reached the terminal in \ plain={plain}: {said:?}", ); diff --git a/tests/exec.rs b/tests/exec.rs index d93b8ca..f5e4f84 100644 --- a/tests/exec.rs +++ b/tests/exec.rs @@ -260,9 +260,11 @@ fn f8_a_failing_gate_is_exit_six_and_a_gate_that_never_answered_is_not() { } // `6` is outside the range `code` can produce, so a script branching on the - // old six cannot mistake it for one of them. + // old six cannot mistake it for one of them. Asserted against `UNFINISHED` + // rather than as a bare literal, so that moving any existing code reddens + // here too. + assert_eq!(exec::UNVERIFIED, exec::UNFINISHED + 1); assert_eq!(exec::UNVERIFIED, 6); - assert!(exec::UNVERIFIED > exec::UNFINISHED); } #[test] diff --git a/tests/provider.rs b/tests/provider.rs index d112078..8a26667 100644 --- a/tests/provider.rs +++ b/tests/provider.rs @@ -43,6 +43,22 @@ const HANDSHAKE: &str = "verify.rs"; /// below — without which this exemption would be a hole rather than a boundary. const PANEL: &str = "providers.rs"; +/// The verification gate's second model, excluded for the handshake's reason. +/// +/// 0.24.0 lets an operator have a rubric judged by a model that is **not** the one +/// doing the work, and `TaskContract::with_reviewer` takes an `Arc` +/// built over a provider of its own. That is a third construction, like the +/// wizard's ping: it never drives a turn, it is deliberately a different model +/// from the session's, and merging it into `build`'s maker would mean the +/// reviewer changing model whenever `/model` did — which is the one thing a second +/// opinion must not do. +/// +/// **This exemption was earned twice over.** The construction was written into +/// `src/gates.rs` first and into `src/provider.rs` second, and this gate refused +/// both. Excluded by name like the other two, and held to the distinction by +/// [`f10_the_reviewer_judges_and_never_drives`] below. +const REVIEW: &str = "reviewer.rs"; + fn sources() -> Vec<(PathBuf, String)> { fn walk(dir: &Path, out: &mut Vec<(PathBuf, String)>) { for entry in std::fs::read_dir(dir).expect("src is readable").flatten() { @@ -68,7 +84,9 @@ fn f10_each_provider_is_constructed_in_exactly_one_place() { for constructor in CONSTRUCTORS { let sites: Vec<(PathBuf, usize)> = sources .iter() - .filter(|(path, _)| !path.ends_with(HANDSHAKE) && !path.ends_with(PANEL)) + .filter(|(path, _)| { + !path.ends_with(HANDSHAKE) && !path.ends_with(PANEL) && !path.ends_with(REVIEW) + }) .map(|(path, text)| (path.clone(), text.matches(constructor).count())) .filter(|(_, count)| *count > 0) .collect(); @@ -76,9 +94,9 @@ fn f10_each_provider_is_constructed_in_exactly_one_place() { let total: usize = sites.iter().map(|(_, count)| count).sum(); assert_eq!( total, 1, - "`{constructor}` should be written exactly once outside {HANDSHAKE} \ - and {PANEL}, so that the interactive and the headless entry points \ - cannot drift apart. Found {sites:?}", + "`{constructor}` should be written exactly once outside {HANDSHAKE}, \ + {PANEL} and {REVIEW}, so that the interactive and the headless entry \ + points cannot drift apart. Found {sites:?}", ); // Naming the file as well as the count is what makes this fail while the @@ -167,3 +185,67 @@ fn f10_both_entry_points_reach_a_provider_through_that_site() { ); } } + +/// The reviewer module judges work and never drives a turn. +/// +/// Without this, excluding `reviewer.rs` from the construction count would be a +/// hole rather than a boundary: anything at all could be built there and the gate +/// would say nothing. The same shape as +/// [`f10_the_panel_interrogates_presets_and_never_runs_one`], for the same reason. +/// +/// Sabotage: give `src/reviewer.rs` a `WithProvider` impl or a `Session` — under +/// which only this fails, and it fails by letting a second driving path grow +/// inside the one exemption that was granted for not being one. +#[test] +fn f10_the_reviewer_judges_and_never_drives() { + let sources = sources(); + + let (_, text) = sources + .iter() + .find(|(path, _)| path.ends_with(REVIEW)) + .expect("src/reviewer.rs exists, or the exemption above names nothing"); + + // It is what it claims to be: io-harness's own judge, built here and handed + // over. Without this the file could be exempt and empty of the one thing the + // exemption was granted for. + assert!( + text.contains("ModelReviewer"), + "src/reviewer.rs is exempted because it builds a reviewer; it names none", + ); + + // And it is nothing else. Each of these would make it a second path a turn + // could run on, which is exactly what the count exists to prevent. + for driving in [ + "WithProvider", + "Watched", + "Session", + "Store", + "TaskContract", + "turn_bounded", + ".complete(", + ] { + assert!( + !text.contains(driving), + "src/reviewer.rs names `{driving}` — it is exempted from the one-site \ + rule precisely because it never drives a turn", + ); + } + + // Reachable from the contract and from nowhere else, so the exemption cannot + // become a back door into the turn path. + let callers: Vec = sources + .iter() + .filter(|(_, text)| text.contains("reviewer::build")) + .map(|(path, _)| { + path.file_name() + .expect("a file name") + .to_string_lossy() + .into_owned() + }) + .collect(); + assert_eq!( + callers, + vec!["contract.rs".to_string()], + "the reviewer is built for the contract's criterion and for nothing else", + ); +} From 54594680acd2e12c6d5e5637080e6a556a03f3a1 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:16:21 +0530 Subject: [PATCH 11/16] docs(exec): the nightly rustdoc gate found a broken link GateOutcome is not in scope in this module, so the intra-doc link could not resolve. Written in plain backticks like every other io-harness item named in this crate's docs. Seventh release in eight that this gate has caught something cargo doc, clippy and the suite all pass over. It is not in ci.yml; it is run by hand at the seal. --- src/exec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/exec.rs b/src/exec.rs index 7dd6cad..6cb720f 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -156,7 +156,7 @@ pub fn code(outcome: &RunOutcome) -> u8 { /// reformat the one function a test reads as text and make a table about /// outcomes answer a question outcomes do not contain. /// -/// **Only [`GateOutcome::Failed`] earns [`UNVERIFIED`].** A gate that +/// **Only a `GateOutcome::Failed` earns [`UNVERIFIED`].** A gate that /// `Errored` never answered at all — the criterion could not run, the reviewer /// returned nothing parsable, the program was refused — and reporting that as /// "the work does not hold up" would claim a judgement nobody made. Such a run From 21e8792732058bb605114e9da4d2857006089771 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:19:50 +0530 Subject: [PATCH 12/16] test(live): the gate decides the outcome, against a real model Three arms. A command criterion that passes returns Success rather than Finished, which is the whole release in one assertion: until now every contract this crate built carried Verification::None, so Success was unreachable from this interface. A criterion that cannot pass is recorded as failed and exits 6, whatever the run itself reported. And a rubric is judged by a second model, with EventKind::Reviewed carrying the verdict and a refusal carrying reasons, because the reasons are the only thing the operator can act on and the only thing the retry can carry. The criterion is asserted on the contract before the run as well as after it. A run that succeeded with no criterion attached would look identical from the outcome alone, which is the confusion this release ends. --- tests/live.rs | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 277 insertions(+) diff --git a/tests/live.rs b/tests/live.rs index 9182067..4762561 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -2631,3 +2631,280 @@ async fn live_f2_a_parked_question_is_answered_and_the_run_carries_on() { "the resumed run drove no step, so the answer reached nothing", ); } + +/// The `[app.io-cli.gates]` section, built in memory rather than through a file. +/// +/// `Config::from_toml` is public and takes the whole document, so the live arms +/// need no `IO_CONFIG` and therefore no environment lock — which matters here +/// because these tests run in one process and an environment variable is +/// process-wide. +fn gated(section: &str) -> Config { + Config::from_toml(section).expect("the gates section parses") +} + +/// **0.24.0 F2 — a command criterion that passes makes the run `Success`.** +/// +/// The whole release in one assertion. Until now every contract this crate built +/// carried `Verification::None`, so `Finished` was the only thing a clean run +/// could return and `Success` was unreachable from this interface. A criterion the +/// operator wrote is what makes the difference, and the run has to come back +/// saying so. +/// +/// `true` and `false` are used as the gate programs because they are the only +/// commands whose exit status is their entire contract. The policy is permissive +/// so that `Act::Exec` admits them — a refused program is a different fact, and +/// `live_f2_a_refused_gate_program_is_not_a_failing_gate` is where that belongs. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +#[cfg(unix)] +async fn live_f2_a_command_criterion_that_passes_makes_the_run_succeed() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + std::fs::write(root.join("notes.md"), "# notes\n\nold 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 config = gated("[app.io-cli.gates]\ncommand = [\"true\"]\n"); + let contract = io_cli::contract::configured( + "Replace the line `old line` in notes.md with `new line`. Nothing else.", + root.to_path_buf(), + &config, + ) + .with_max_steps(12); + + // The criterion actually reached the contract. Asserted before the run, + // because a run that succeeded with no criterion on it would look identical + // from the outcome alone — which is exactly the confusion this release exists + // to end. + assert!( + matches!( + contract.verify, + io_harness::Verification::Command { ref argv, expect_exit } + if argv == &vec!["true".to_string()] && expect_exit == 0 + ), + "the configured criterion did not reach the contract: {:?}", + contract.verify, + ); + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &Policy::permissive(), + &io_harness::ApproveAll, + &io_harness::Ignore, + ) + .await + .expect("the turn runs"); + + println!("live 0.24.0 F2: outcome {:?}", result.outcome); + + assert!( + matches!(result.outcome, io_harness::RunOutcome::Success { .. }), + "a passing criterion should return Success, not {:?} — if this is Finished \ + the criterion never ran", + result.outcome, + ); + + let attempts = store + .gate_attempts(result.run_id) + .expect("the gate attempts"); + println!("live 0.24.0 F2: {} gate attempt(s)", attempts.len()); + assert!( + io_cli::gates::standing(&attempts) + .is_some_and(|standing| matches!(standing.outcome, io_harness::GateOutcome::Passed)), + "the store should record a passing gate attempt, found {attempts:?}", + ); + + // And the exit status a headless run would have reported is unchanged by a + // gate that passed. + assert_eq!( + io_cli::exec::verified_code(&result.outcome, io_cli::gates::standing(&attempts).as_ref()), + io_cli::exec::OK, + ); +} + +/// **0.24.0 F2 + F8 — a failing criterion is recorded, and it is exit `6`.** +/// +/// The negative half, and the one that pays for the release: without it an +/// unattended run exits `0` over work nothing checked. `false` cannot be satisfied +/// by any amount of agent effort, so this asserts the gate's verdict rather than +/// the model's competence — the run is allowed to do whatever it likes with the +/// file and still must not be reported as verified. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +#[cfg(unix)] +async fn live_f2_f8_a_failing_criterion_is_recorded_and_exits_six() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + std::fs::write(root.join("notes.md"), "# notes\n\nold 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 config = gated("[app.io-cli.gates]\ncommand = [\"false\"]\n"); + let contract = io_cli::contract::configured( + "Replace the line `old line` in notes.md with `new line`. Nothing else.", + root.to_path_buf(), + &config, + ) + .with_max_steps(6); + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &Policy::permissive(), + &io_harness::ApproveAll, + &io_harness::Ignore, + ) + .await + .expect("the turn runs"); + + println!("live 0.24.0 F8: outcome {:?}", result.outcome); + + assert!( + !matches!(result.outcome, io_harness::RunOutcome::Success { .. }), + "a criterion that cannot pass must never return Success", + ); + + let attempts = store + .gate_attempts(result.run_id) + .expect("the gate attempts"); + let standing = io_cli::gates::standing(&attempts).expect("a gate ran and answered"); + println!( + "live 0.24.0 F8: phase {:?}, outcome {:?}, attempt {}", + standing.phase, standing.outcome, standing.attempt, + ); + assert_eq!(standing.phase, "command"); + assert!(matches!(standing.outcome, io_harness::GateOutcome::Failed)); + + // The point of the release: whatever the run itself reported, the operator's + // own criterion decides the status a script branches on. + assert_eq!( + io_cli::exec::verified_code(&result.outcome, Some(&standing)), + io_cli::exec::UNVERIFIED, + "a run whose gate said no must exit 6, not {}", + io_cli::exec::code(&result.outcome), + ); + + // And the retry has something real to say. An empty failure text would make + // the follow-up turn no better informed than the first, which is the whole + // reason io-cli drives one at all. + let events = store.sandbox_events(result.run_id).expect("sandbox events"); + let step = attempts.last().map(|last| last.step).unwrap_or_default(); + println!( + "live 0.24.0 F7: recorded output {:?}", + io_cli::gates::output(&events, step), + ); +} + +/// **0.24.0 F5 — a rubric is judged by a second model, and the verdict arrives.** +/// +/// The one criterion that costs a provider call, and the only one whose verdict +/// reaches the event stream at all: `EventKind::Reviewed` is emitted for a review +/// and for nothing else. The rubric is written so that a correct run passes it and +/// the judgement is still a real one — a rubric no model could satisfy would prove +/// only that the reviewer answered. +/// +/// The reviewer is named explicitly and is the same model here, with +/// `allow_self_review` set, because this asserts the mechanism rather than the +/// independence. An operator who does not set it is refused, which is asserted in +/// the offline suite. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f5_a_rubric_is_judged_by_a_second_model() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + std::fs::write(root.join("notes.md"), "# notes\n\nold 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()); + + // The provider block is what `contract::configured` builds the reviewer from, + // so the section carries both. The key reaches the file only in memory. + let config = gated(&format!( + "[[provider]]\nkind = \"openrouter\"\nmodel = \"{model}\"\napi_key = \"{key}\"\n\n\ + [app.io-cli.gates]\nrubric = \"notes.md contains the line `new line`\"\n\ + reviewer = \"{model}\"\nallow_self_review = true\n", + model = model(), + )); + + let contract = io_cli::contract::configured( + "Replace the line `old line` in notes.md with `new line`. Nothing else.", + root.to_path_buf(), + &config, + ) + .with_max_steps(12); + + assert!( + matches!(contract.verify, io_harness::Verification::Review { .. }), + "the rubric did not reach the contract: {:?}", + contract.verify, + ); + assert!( + contract.reviewer.is_some(), + "a Review criterion with no reviewer is Error::Config at run start", + ); + + let collected = Arc::new(Mutex::new(Vec::new())); + let observer = Collector { + events: Arc::clone(&collected), + }; + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &Policy::permissive(), + &io_harness::ApproveAll, + &observer, + ) + .await + .expect("the turn runs"); + + println!("live 0.24.0 F5: outcome {:?}", result.outcome); + + let events = collected.lock().expect("not poisoned").clone(); + let reviewed: Vec<_> = events + .iter() + .filter_map(|event| match &event.kind { + EventKind::Reviewed { passed, reasons } => Some((*passed, reasons.clone())), + _ => None, + }) + .collect(); + println!("live 0.24.0 F5: reviewed {reviewed:?}"); + + assert!( + !reviewed.is_empty(), + "a review criterion must emit EventKind::Reviewed; the second model was \ + never asked", + ); + + // A refusal has to carry its reasons, because the reasons are the only thing + // the operator can act on and the only thing the retry can carry. + for (passed, reasons) in &reviewed { + if !passed { + assert!( + !reasons.is_empty(), + "a failing review with no reasons tells the operator nothing", + ); + } + } + + let attempts = store + .gate_attempts(result.run_id) + .expect("the gate attempts"); + let standing = io_cli::gates::standing(&attempts).expect("the review answered"); + assert_eq!(standing.phase, "review"); +} From 3cc9e52ca494a018dd55082bf8b44aac781713c5 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:42:04 +0530 Subject: [PATCH 13/16] fix(gates): what three adversarial reviews found behind a green suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worst of it: io-harness evaluates the contract's criterion after every step on which the agent called a tool, and for Verification::None that evaluation is false — so it records phase=none, Failed on every ungated run this product has ever driven. Read back naively that says the gate failed. io exec would have exited 6 for every operator who has never configured a gate, and every ordinary session turn would have drawn 'gate failed'. Those rows are now dropped unconditionally. The retry counted those same rows, so its attempt number was the step count and may_retry compared nine against a budget of one. The headline feature was dead for every criterion except bare existence. The driver now keeps one row per turn. A parked, denied or cancelled run was given a verdict and retried: the operator was told '/resume opens it' and then had that run buried under a fresh billed turn. Only a run that reached its own end is judged. Configuring any gate silently switched off conversational classification, so 'hello' started a full agent run and then ran the test suite after every step of it. Set explicitly where a criterion is attached. Also: the fold moved to gates.rs because io exec may not reach the interface; a failing gate no longer borrows the permission boundary's word; the reviewer refusal no longer formats a ProviderSpec, which holds the api_key; an unknown gates key is refused rather than silently disabling the gate; an empty command argv is refused here rather than erroring at run start on every turn; /gates named the wrong io.toml and re-judged past turns against today's filesystem; and io exec now says when a configured section is not gating the run. The documentation claimed the gate runs once per turn. It runs after every step, which for a rubric is a billed completion per step. That was the one disclosure the design existed to make. --- README.md | 44 +++++++++++----- docs/config.example.toml | 35 ++++++++----- src/app.rs | 92 ++++++--------------------------- src/contract.rs | 47 +++++++++++++++-- src/exec.rs | 73 +++++++++++++++++++++----- src/gates.rs | 107 +++++++++++++++++++++++++++++++++++++++ src/main.rs | 78 ++++++++++++++++++++++++---- src/reviewer.rs | 12 +++-- src/status.rs | 13 +++-- tests/gates.rs | 28 +++++++--- tests/steer.rs | 71 +++++++++++++++++++++----- 11 files changed, 446 insertions(+), 154 deletions(-) diff --git a/README.md b/README.md index 8c36628..d2c4611 100644 --- a/README.md +++ b/README.md @@ -1144,13 +1144,15 @@ winner: - **A rubric** a second model answers: a sentence saying what the work has to be, judged by a reviewer you name. -**io-cli holds no list of test commands.** Leave `command` empty and the one that -runs is the one the repository's own toolchain proposes — io-harness detects that -from the project, so a Rust checkout is offered a `cargo` line and a Node one is -not. That detection is the dependency's, and it is deliberately not reimplemented -here: a table of build tools inside this crate would be a second opinion that goes -stale the first time somebody's project does not look like the ones it was written -against. +**io-cli holds no list of test commands.** `/gates` offers you the one the +repository's own toolchain proposes — io-harness detects that from the project, so +a Rust checkout is offered a `cargo` line and a Node one is not — and accepting the +offer writes that command into your file, where you can read it and change it. What +is written is always a concrete argv: `command` is the program and its arguments, +never a shell line and never blank. That detection is the dependency's, and it is +deliberately not reimplemented here: a table of build tools inside this crate would +be a second opinion that goes stale the first time somebody's project does not look +like the ones it was written against. **The criterion is run by io-harness, in the sandbox, and not by io-cli.** That is not squeamishness. A criterion run from here could not be handed the cache @@ -1159,6 +1161,12 @@ would fail on a registry write that io-harness's own gate would have allowed. It also keeps a rule worth keeping: exactly one module in io-cli starts a process at all, and it is the one behind `!`. +The single exception is a `file` criterion with no `contains`. io-harness has no +criterion for bare existence — the nearest one treats a missing file and an empty +needle as a pass — so io-cli answers that one itself, with the reader that tells a +missing file from an empty one. It runs no process to do it, and it is the reason +that criterion costs nothing at all. + **`retries` defaults to 1, and `0` means report-only.** A failing gate sends the agent back to work with the failure text — the compiler's output, the missing file, the reviewer's sentence — because the failure *is* the instruction, and an @@ -1168,13 +1176,21 @@ model and not a loop counter; if you want several, say so. Set `retries = 0` and the verdict is drawn and recorded and nothing is re-driven, which is what you want in a run you are watching and what you want in a run you are only measuring. -**A rubric is a real billed completion on every gated turn.** Not on failures, not -on the first one — every turn that carries the gate ends with a call to the -reviewer, and that call costs what a call costs. This is the reason the three -kinds are named in `/gates`'s own one-line description rather than left to the -surface: two of them cost nothing but the time they take and one of them costs -money, and that is the whole of the choice between them. The call is io-harness's, -so it lands in the run's usage like any other and `/cost` counts it. +**The criterion runs after every step, not once when the turn ends — and for a +rubric every one of those is a billed completion.** This is the number to know +before you configure anything. io-harness evaluates the contract's criterion at the +bottom of its step loop and keeps going until it passes, so a turn that takes nine +steps runs your command nine times, and a rubric on that turn is nine calls to the +reviewer rather than one. It is not "the agent finishes and then the work is +checked": it is checked continuously, and the run ends the moment the check +succeeds. + +That is what makes a command criterion worth choosing carefully. `cargo test --all` +after every step of a long turn is a great deal of compilation, and a narrow +command — one test, one binary, one lint — is usually the right gate. For a rubric +the cost is money rather than time, which is the reason the three kinds are named +in `/gates`'s own one-line description rather than left to the surface. The call is +io-harness's, so it lands in the run's usage like any other and `/cost` counts it. **A rubric needs a `reviewer`, and it is refused without one.** io-harness answers a missing reviewer with a configuration error at run start — before the first diff --git a/docs/config.example.toml b/docs/config.example.toml index aff908b..86ad83b 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -583,13 +583,22 @@ fleet = "ctrl+f" # session's `exec` policy and runs it without a shell, so `&&`, a pipe and a glob # are not available and were never going to be. `expect_exit` defaults to 0. # -# AN EMPTY OR ABSENT `command` MEANS THE COMMAND THIS REPOSITORY'S OWN TOOLCHAIN -# PROPOSES. io-harness detects that — it is the half of this that knows a Cargo -# workspace from a package.json — and io-cli holds no list of its own to consult. -# That is deliberate: a list of build tools shipped inside a TUI is a list that is -# wrong for somebody the day it is written, and stale for everybody a year later. -# Write the key when your repository's real gate is narrower than what a detector -# would propose, and leave it out when it is not. +# `command` IS ALWAYS A CONCRETE ARGV. An empty list is refused, and an absent one +# is not a criterion at all — this key never means "work it out for me". +# +# What detection is for is the OFFER: `/gates` asks io-harness what this repository +# builds and tests with — it is the half of this that knows a Cargo workspace from +# a package.json — and offers you that command as a row you can accept, which +# writes it here in full. io-cli holds no list of its own to consult, deliberately: +# a list of build tools shipped inside a TUI is wrong for somebody the day it is +# written and stale for everybody a year later. So the file always says exactly +# what will run, and you can read it and narrow it. +# +# AND IT RUNS AFTER EVERY STEP, NOT ONCE AT THE END. io-harness evaluates the +# criterion at the bottom of its step loop and keeps going until it passes, so a +# nine-step turn runs this command nine times. `cargo test --all-features` after +# every step of a long turn is a lot of compiling; the gate you want is usually +# the narrowest command that would still catch the mistake. # # [app.io-cli.gates] # retries = 1 @@ -623,11 +632,13 @@ fleet = "ctrl+f" # allowed when you ask for it and never by default: a judge marking its own paper # is a decision, not an oversight. # -# AND A REVIEW CRITERION IS A REAL BILLED COMPLETION ON EVERY GATED TURN. It is a -# second model reading the work, charged like any other call, and it runs again on -# each retry the gate buys. The other two kinds cost a process and a file read; -# this one costs money per turn, which is worth knowing before it is in a file -# that runs in CI. +# AND A REVIEW CRITERION IS A REAL BILLED COMPLETION AFTER EVERY STEP — NOT ONCE +# PER TURN. The criterion is evaluated at the bottom of io-harness's step loop and +# keeps being evaluated until it passes, so a nine-step turn asks the reviewer nine +# times, and each retry the gate buys starts that again. The other two kinds cost a +# process and a file read; this one costs money per step, which is worth knowing +# before it is in a file that runs in CI. Keep the rubric to something a model can +# decide quickly, and prefer a command or a file where one would do. # # [app.io-cli.gates] # rubric = "every public item changed in this turn has a doc comment saying why" diff --git a/src/app.rs b/src/app.rs index ab621bd..b4831df 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2176,78 +2176,6 @@ pub fn server_value(key: &str, typed: &str) -> String { /// driver so a test can reach it. pub const PROPOSED_GATE: &str = "!proposed-gate"; -/// The gate attempts a turn is judged by, including the one io-harness never made. -/// -/// **This is the fold that stops an ungated run reporting as gated, and it is the -/// easiest defect in this release to ship.** [`crate::gates::Criterion::verification`] -/// maps a bare existence criterion — `file` with no `contains` — to -/// `Verification::None`, because there is no honest counterpart for it in -/// io-harness's enum. A run carrying `Verification::None` never reaches the gate -/// at all: io-harness's step loop returns `Finished` the moment the agent stops -/// calling tools, so the store holds **no** `gate_attempts` row and -/// [`crate::gates::standing`] answers `None`. A caller that stopped there would -/// draw nothing, retry nothing, and leave an operator who asked for a file to -/// exist with a session that never once looked for it. -/// -/// So the criterion this crate owns is evaluated here — through -/// [`crate::gates::Criterion::satisfied_in`], which is the reader that tells a -/// missing file from an empty one — and appended as an attempt of its own. It -/// then flows through `standing`, `may_retry` and the report exactly as a -/// recorded one does, which is the point: one path, not two. -/// -/// Any `none` row is dropped first. A run that ran out of steps *does* reach the -/// gate with `Verification::None`, which passes trivially and records -/// `phase = "none", passed` — a row saying a criterion that checked nothing was -/// satisfied. Left in place it would be the last row, and the standing would read -/// `passed` for a file that is not there. -/// -/// Every other criterion is io-harness's to judge and the rows come back -/// untouched. -pub fn gate_attempts( - mut recorded: Vec, - criterion: Option<&crate::gates::Criterion>, - root: &std::path::Path, -) -> Vec { - // `satisfied_in` answers `None` for every criterion the run loop evaluates, - // so this one call is both the question "is this ours?" and its answer — - // asking `checked_here` first would be the same test written twice. - let Some(satisfied) = criterion.and_then(|criterion| criterion.satisfied_in(root)) else { - return recorded; - }; - recorded.retain(|attempt| attempt.phase != "none"); - let step = recorded.last().map_or(0, |attempt| attempt.step); - recorded.push(io_harness::GateAttempt { - // Not a stored row and never read back as one: `id` addresses a row in - // `gate_attempts` and this attempt was made here, so it has none. - id: 0, - step, - // Not `none`, which is what io-harness would have called it, and not - // `contains`, which is the phase of a criterion that reads a file's - // contents. This one asserts existence and says so. - phase: "exists".to_string(), - outcome: if satisfied { - io_harness::GateOutcome::Passed - } else { - io_harness::GateOutcome::Failed - }, - // The row's own explanation, for the same reason io-harness leaves it - // empty on a plain pass: there is nothing to explain about a file that is - // where it was asked to be. - detail: if satisfied { - String::new() - } else { - criterion - .map(crate::gates::Criterion::describe) - .unwrap_or_default() - }, - // A stored timestamp this crate never parses and never compares, and the - // one field that would need a clock. `src/main.rs` is the only module - // allowed one, so it stays empty rather than being invented here. - at: String::new(), - }); - recorded -} - /// What the failing gate actually said, or `None` when it said nothing. /// /// Two sources because a gate has two ways of speaking and neither covers the @@ -2263,8 +2191,13 @@ fn gate_said( events: &[io_harness::SandboxEvent], ) -> Option { let last = attempts.last()?; + // `trim`, because io-harness writes `review.reasons.join("; ")` verbatim and a + // reviewer that answered with blanks would otherwise produce a report line + // ending in a dangling colon and a retry prompt whose "this is what it + // reported" section is empty. Nothing to say is `None`, and `gate_retry` + // already has a sentence for that. crate::gates::output(events, last.step) - .or_else(|| (!last.detail.is_empty()).then(|| last.detail.clone())) + .or_else(|| (!last.detail.trim().is_empty()).then(|| last.detail.clone())) } /// The one line a turn's gate commits into the scrollback, with its tone. @@ -2288,9 +2221,16 @@ pub fn gate_report( let word = standing.outcome.as_str(); let tone = match standing.outcome { io_harness::GateOutcome::Passed => Tone::Success, - // A gate that answered no is a refusal and not an error: nothing went - // wrong, the work was judged and did not meet the criterion. - io_harness::GateOutcome::Failed => Tone::Refused, + // **`Tone::Warning` and emphatically not `Tone::Refused`, whose rendered + // word is the literal `refused`.** That word belongs to the permission + // boundary, and this release moved the failing review off that tone in + // `src/events.rs` for exactly this reason — only for the scrollback line + // one row below to put it back. An operator would read `warning: the gate + // ran and did not pass` and directly beneath it `refused: gate failed`, + // which collapses "the policy would not run my gate" into "your work did + // not meet the bar". Those are the two facts a gate most has to keep + // apart, and they need opposite responses. + io_harness::GateOutcome::Failed => Tone::Warning, // `Errored`, and whatever a later io-harness adds — the enum is // `#[non_exhaustive]`, and an outcome this release has not seen is not a // pass. diff --git a/src/contract.rs b/src/contract.rs index 052d9ca..9a84adc 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -287,10 +287,21 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta let contract = match criterion_for(config) { Some((criterion, reviewer)) => { let contract = contract.with_verification(criterion.verification()); - match reviewer { + let contract = match reviewer { Some(reviewer) => contract.with_reviewer(reviewer), None => contract, - } + }; + // **A gate must not turn "hello" into a run, and by default it does.** + // io-harness infers whether a turn may be answered conversationally + // from `matches!(contract.verify, Verification::None)` — so attaching + // any criterion silently switches greeting handling off for the whole + // session, and every idle question becomes a full agent run that then + // runs the operator's test suite after each of its steps. The harness's + // own field documents this as the way back for exactly a surface like + // this one. Set only where a criterion was attached, so an operator + // with no gate reproduces the contract this crate built before the + // release, field for field, which is what `tests/contract.rs` asserts. + contract.with_conversational_turns(true) } None => contract, }; @@ -308,6 +319,25 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta /// `Verification::Review` reaching a contract with no reviewer beside it is /// `Error::Config` at run start, on every turn**, so the criterion and its /// reviewer are decided together here and either both go on or neither does. +/// The criterion this configuration resolves to, with no reviewer built. +/// +/// What a *reader* wants: the surfaces that report a verdict need to know which +/// criterion was in force, and building a provider to tell them would resolve a +/// credential for a question that does not need one. [`criterion_for`] is the +/// half that also builds the reviewer, for the contract that will run it. +/// +/// This is what lets `io exec` fold the gate rows the same way a session does — +/// without it the headless arm reads io-harness's `phase = "none"` bookkeeping as +/// a failed gate, and a bare `file` criterion is never evaluated at all. +pub fn criterion_of(config: &Config) -> Option { + let gates = crate::settings::stored(config).0?.gates?; + let working = config + .provider_spec() + .map(crate::provider::model_of) + .unwrap_or_default(); + gates.criterion(working).ok()? +} + fn criterion_for( config: &Config, ) -> Option<( @@ -339,7 +369,18 @@ fn criterion_for( /// refusal is not a contract. A surface calls this to say the one sentence an /// operator needs: the section is there, and it is not doing anything. pub fn gate_notice(config: &Config) -> Option { - let gates = crate::settings::stored(config).0?.gates?; + // **The commonest mistake is a value of the wrong shape, and it takes the + // whole table with it.** `command = "cargo test"` — a string where an array + // belongs — fails to deserialize `[app.io-cli]` entirely, so `stored` answers + // `(None, Some(complaint))`: no theme, no keys, no ceilings and no gate. + // Reading only `.0` and giving up would leave `/gates` saying "no gate is in + // force" while the section is plainly in the operator's file, which is the one + // surface they opened to ask why it is not running. + let (stored, complaint) = crate::settings::stored(config); + let Some(stored) = stored else { + return complaint; + }; + let gates = stored.gates?; let working = config .provider_spec() .map(crate::provider::model_of) diff --git a/src/exec.rs b/src/exec.rs index 6cb720f..80ca05c 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -70,10 +70,12 @@ pub const UNFINISHED: u8 = 5; /// from them. /// /// It cannot be decided by [`code`], because io-harness has no `RunOutcome` -/// variant for a run whose criterion answered no — such a run surfaces as -/// `StepCapReached` when it spends its budget failing, and as an ordinary -/// `Finished` when it stops early. So the verdict is read from the store after -/// the run and applied by [`verified_code`]. Reported upstream as +/// variant for a run whose criterion answered no. Such a run surfaces as +/// `StepCapReached` — the criterion is re-evaluated after every step and the loop +/// only ends early when it *passes*, so a gate that never passes spends the whole +/// budget. (Not as `Finished`: that variant requires `Verification::None`, so a +/// gated contract can never return it.) The verdict is therefore read from the +/// store after the run and applied by [`verified_code`]. Reported upstream as /// io-harness#212. pub const UNVERIFIED: u8 = 6; @@ -94,12 +96,15 @@ pub const UNVERIFIED: u8 = 6; /// that claimed success or a crash would both be inventions. /// /// Two of the mappings are the release's research rather than its taste. -/// `Finished` is `OK` because a contract with no verification criterion is what -/// this subcommand always builds, so `Finished` — and never `Success` — is what -/// a clean run returns; a table that treated only `Success` as zero would fail -/// every successful run. And the four ceilings need codes at all only because the -/// harness returns them as `Ok`, so a status read off the `Result` reports -/// success on every one of them. +/// `Finished` is `OK` because a contract with no verification criterion returns +/// `Finished` and never `Success`, and a table that treated only `Success` as zero +/// would fail every successful ungated run. **0.24.0 makes `Success` reachable +/// and changes neither mapping**: an operator who configures `[app.io-cli.gates]` +/// gets a real criterion on the contract from [`crate::contract::configured`], and +/// a run that passes it comes back `Success`. Both still mean the run ended of its +/// own accord, which is what `0` says. And the four ceilings need codes at all +/// only because the harness returns them as `Ok`, so a status read off the +/// `Result` reports success on every one of them. pub fn code(outcome: &RunOutcome) -> u8 { match outcome { RunOutcome::Success { .. } | RunOutcome::Finished { .. } => OK, @@ -625,6 +630,16 @@ impl WithProvider for Headless { if let Some(parked) = parked(&result.outcome, result.run_id) { eprintln!("io: {parked}"); } + // **A run that was meant to be gated and was not says so, on the arm where + // nobody is watching a screen.** A hand-edited section that refuses, or a + // reviewer that will not build, leaves the contract ungated — and until + // this line the only surfaces that reported it were `/gates` and the + // `/config` write, both of which need a terminal. An unattended job that + // silently stopped verifying is the exact failure this release exists to + // prevent, arriving through the release itself. + if let Some(notice) = crate::contract::gate_notice(&self.config) { + eprintln!("io: {notice}"); + } // **The criterion has the last word on the exit status, and it is read // from the store rather than from the outcome.** io-harness has no // `RunOutcome` variant for a run whose gate answered no, so a run that @@ -632,7 +647,12 @@ impl WithProvider for Headless { // stopped early believing itself done reports `Finished` — `3` and `0`, // neither of which is what happened. Reported upstream as // io-harness#212. - let standing = gate_standing(&self.store, result.run_id); + let standing = gate_standing( + &self.store, + result.run_id, + &self.config, + self.session.root(), + ); if let Some(standing) = &standing { eprintln!("io: {}", gate_line(standing)); } @@ -642,12 +662,37 @@ impl WithProvider for Headless { /// What a run's own gate attempts say, if it had a criterion at all. /// +/// **The criterion has to be resolved here, and reading the rows without it is +/// how this release nearly shipped exit `6` for every operator alive.** +/// io-harness evaluates the contract's criterion after every step on which the +/// agent called a tool, and for `Verification::None` that evaluation is `false` — +/// so an ungated run leaves `phase = "none", Failed` rows behind, and a naive +/// read of them says the gate failed on a session that never had one. +/// [`crate::gates::gate_attempts`] is the fold that drops them, and it is the same +/// call the interactive arm makes: one path, not two. +/// +/// It is also what makes a bare `file` criterion work headlessly at all. That one +/// maps to `Verification::None`, so io-harness records nothing for it and io-cli +/// answers it itself — without this the terminal would hold an operator to a +/// standard that CI quietly ignored, which is the 0.14.0 asymmetry this product +/// deleted once already. +/// /// A store that cannot be read answers `None`, which leaves the exit status /// exactly what it was before this release. A failure to read the verdict is not /// evidence that the work is bad. -fn gate_standing(store: &Store, run_id: i64) -> Option { +fn gate_standing( + store: &Store, + run_id: i64, + config: &Config, + root: &std::path::Path, +) -> Option { let attempts = store.gate_attempts(run_id).ok()?; - crate::gates::standing(&attempts) + let criterion = crate::contract::criterion_of(config); + crate::gates::standing(&crate::gates::gate_attempts( + attempts, + criterion.as_ref(), + root, + )) } /// One line for stderr naming what the gate did. @@ -1190,7 +1235,7 @@ impl WithProvider for Resuming { // answers the same question. Reading the standing off the run this // resume drove rather than off the original is the point: the whole // reason to resume is that the work is not the same work any more. - let standing = gate_standing(&self.store, resumed.run_id); + let standing = gate_standing(&self.store, resumed.run_id, &self.config, &self.root); if let Some(standing) = &standing { eprintln!("io: {}", gate_line(standing)); } diff --git a/src/gates.rs b/src/gates.rs index 5464ed4..b42b497 100644 --- a/src/gates.rs +++ b/src/gates.rs @@ -54,7 +54,18 @@ use serde::{Deserialize, Serialize}; /// more than one is a [`Refusal`] rather than a silent precedence rule, because a /// precedence rule here would quietly gate a turn on something the operator did /// not choose. +/// +/// **`deny_unknown_fields`, unlike its parent, and the asymmetry is deliberate.** +/// `CliSettings` cannot refuse an unknown key: `[app.io-cli]` is a section +/// io-harness does not validate, and a newer io-cli's key must not stop an older +/// one from starting. This table is a leaf with eight names and one job, and the +/// commonest way to get it wrong is a typo — `comand = ["cargo", "test"]` +/// deserializes to the default, compares equal to it, and answers `Ok(None)`: no +/// refusal, no notice, no gate. The whole point of [`Refusal::Empty`] is that a +/// section which plainly means to be a gate and is not one says so, and a typo is +/// exactly that case. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct Settings { /// How many further turns a failing gate may buy, defaulting to one. /// @@ -147,6 +158,14 @@ pub enum Refusal { Ambiguous, /// A rubric was given with no model to answer it. ReviewerMissing, + /// A command criterion was written with no program in it. + /// + /// Its own arm rather than [`Refusal::Empty`], because the operator plainly + /// meant a command and the fix is to name one. io-harness answers an empty + /// argv with `Error::Config` when the gate first runs — every turn, before + /// anything useful happens — so this is the same class of mistake as a rubric + /// with no reviewer and is caught in the same place. + NoProgram, /// The reviewer is the model doing the work, and that was not asked for. SelfReview { /// The model named as both the worker and the judge. @@ -196,6 +215,13 @@ impl Settings { } if let Some(argv) = &self.command { + // An empty argv is not a criterion. Left to io-harness it becomes + // `Error::Config` at the first gated step of every turn, recorded as + // `Errored` — and because an errored gate is worth another turn, the + // retry then buys a second one that fails identically. + if argv.is_empty() { + return Err(Refusal::NoProgram); + } return Ok(Some(Criterion::Command { argv: argv.clone(), expect_exit: self.expect_exit.unwrap_or(0), @@ -454,6 +480,83 @@ pub fn proposed_command(root: &Path, config: &Config) -> Option> { // which is precisely the drift that gate exists to catch. It caught this release // writing one. See `crate::provider::reviewer`. +/// The gate attempts a turn is judged by, including the one io-harness never made. +/// +/// **Here rather than in `crate::app`, because `io exec` needs it and may not +/// reach the interface.** `tests/exec.rs` forbids the headless modules from naming +/// `crate::app` at all — a renderer on that path is how ANSI ends up in a stream +/// something is parsing — and the first version of this release put the fold there +/// and had the headless arm call it. That gate caught it. The fold is gate +/// semantics, not interface, and this is where the rest of those live. +/// +/// **A run with no criterion has no standing, and saying otherwise is how this +/// release nearly shipped a catastrophe.** io-harness evaluates the contract's +/// criterion after *every* step on which the agent called a tool +/// (`run/step.rs:1654`), and for `Verification::None` the evaluation is +/// `Ok(false)` (`verify.rs:1361`) — so it records `phase = "none", +/// GateOutcome::Failed`, once per tool-calling step, on **every ungated run this +/// product has ever driven**. Read back naively that says the gate failed: the +/// status line would read `gate failed` under every ordinary turn, and `io exec` +/// would exit `6` for every operator who has never configured a gate at all. +/// +/// Dropping every `none` row is the whole of the fix, and it needs to know nothing +/// about what is configured: an ungated run's rows are *all* `none`, so filtering +/// them empties the list for exactly those sessions, while a caller reporting a +/// past run still gets what io-harness actually recorded. +/// +/// (An earlier draft of this comment said a `none` row records *passed*. It +/// records **failed**. That difference is the whole defect.) +/// +/// The other half is the criterion io-cli owns. [`Criterion::verification`] maps a +/// bare existence criterion — `file` with no `contains` — to `Verification::None`, +/// because io-harness has no honest counterpart for it, so nothing in the store +/// ever speaks for it. It is evaluated here through [`Criterion::satisfied_in`], +/// the reader that tells a missing file from an empty one, and appended as an +/// attempt of its own — after which it flows through [`standing`], [`may_retry`] +/// and the report exactly as a recorded one does. One path, not two. +/// +/// Every other criterion is io-harness's to judge and the rows come back +/// untouched. +pub fn gate_attempts( + mut recorded: Vec, + criterion: Option<&Criterion>, + root: &Path, +) -> Vec { + recorded.retain(|attempt| attempt.phase != "none"); + // `satisfied_in` answers `None` for every criterion the run loop evaluates, + // so this one call is both the question "is this ours?" and its answer — + // asking `checked_here` first would be the same test written twice. + let Some(satisfied) = criterion.and_then(|criterion| criterion.satisfied_in(root)) else { + return recorded; + }; + let step = recorded.last().map_or(0, |attempt| attempt.step); + recorded.push(GateAttempt { + // Not a stored row and never read back as one: `id` addresses a row in + // `gate_attempts` and this attempt was made here, so it has none. + id: 0, + step, + // Not `none`, which is what io-harness would have called it, and not + // `contains`, which is the phase of a criterion that reads a file's + // contents. This one asserts existence and says so. + phase: "exists".to_string(), + outcome: if satisfied { + GateOutcome::Passed + } else { + GateOutcome::Failed + }, + // Empty on both arms. The row's `detail` is what the report reads back as + // "this is what it reported", and for this criterion the only thing io-cli + // could put there is the criterion itself — which the report line and the + // retry prompt have already said one clause earlier. + detail: String::new(), + // A stored timestamp this crate never parses and never compares, and the + // one field that would need a clock. `src/main.rs` is the only module + // allowed one, so it stays empty rather than being invented here. + at: String::new(), + }); + recorded +} + /// One sentence an operator can act on, for each way a section is refused. /// /// ASCII throughout — no quotation marks that are not the typewriter kind, no @@ -477,6 +580,10 @@ impl fmt::Display for Refusal { "a rubric needs a model to answer it: set reviewer, or delete the \ rubric", ), + Refusal::NoProgram => f.write_str( + "this gates section sets an empty command: name the program to run, \ + such as command = [\"cargo\", \"test\"]", + ), Refusal::SelfReview { model } => write!( f, "the reviewer {model} is the model doing the work: name a different \ diff --git a/src/main.rs b/src/main.rs index 560d091..1bfc088 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2617,9 +2617,21 @@ async fn loop_over Result>( // Whichever key a file actually names decides the scope word, // because that is the file the operator has to open. Three // files are called `io.toml` and only one of them is theirs. + // **Only a key that names a criterion may decide it.** The + // catalogue's first entry is `retries`, which is a preference + // and not a gate — so an operator with `retries` in their user + // file and `command` in the project's would be sent to the + // wrong one of the three files, by the very line written to + // stop that happening. let decided = settings .iter() - .find(|setting| setting.value.is_some()) + .find(|setting| { + setting.value.is_some() + && matches!( + setting.path.rsplit('.').next(), + Some("command" | "file" | "rubric") + ) + }) .map(|setting| setting.decided.word()) .unwrap_or("default"); // A refusal is NOT reported here. `gate_notice` below is the @@ -2659,9 +2671,19 @@ async fn loop_over Result>( // recorded row at all, and reading the store alone would // report an ungated run. if let Some(run_id) = last_run(&session, &store).map(|turn| turn.run_id) { - let attempts = io_cli::app::gate_attempts( + // **`None`, not today's criterion, and the difference is + // whether this is a fact.** Folding the configured + // criterion in here would evaluate `satisfied_in` against + // the filesystem *now* and report the answer as the last + // turn's verdict — so a turn that ran before any gate + // existed would be reported as having passed one, and a + // turn that genuinely passed would read as failed the + // moment the file was deleted. What io-harness recorded + // for that run is the only thing here that is a statement + // about the past. + let attempts = io_cli::gates::gate_attempts( store.gate_attempts(run_id).unwrap_or_default(), - criterion.as_ref(), + None, &root, ); let events = store.sandbox_events(run_id).unwrap_or_default(); @@ -3745,11 +3767,25 @@ async fn loop_over Result>( // would report an ungated run as a run that passed. // `app::gate_attempts` evaluates that one criterion here // and appends the attempt io-harness never made. - gated.extend(io_cli::app::gate_attempts( - store.gate_attempts(run_id).unwrap_or_default(), - criterion.as_ref(), - session.root(), - )); + // **One row per TURN, and taking them all is how the + // retry died in review.** io-harness evaluates the + // criterion after *every* step the agent takes, not once + // when it stops — `run/step.rs:1654` sits inside the step + // loop — so one nine-step turn writes nine rows. Extending + // by all of them makes `standing.attempt` the step count, + // so `may_retry` compares nine against a budget of one and + // answers no, and the headline feature of this release + // never fires for any criterion except the bare-existence + // one. The run's last row is its verdict; the count of + // those is the count of turns. + gated.extend( + io_cli::gates::gate_attempts( + store.gate_attempts(run_id).unwrap_or_default(), + criterion.as_ref(), + session.root(), + ) + .pop(), + ); let events = store.sandbox_events(run_id).unwrap_or_default(); if let Some(standing) = io_cli::gates::standing(&gated) { // `GateOutcome::as_str` passed through verbatim: it @@ -4621,8 +4657,32 @@ async fn turn( // turn that ran to a result has one: a run that came back `Err` was not judged, // and a turn the operator abandoned has whatever io-harness had written when // the future was dropped — neither is a verdict on the work. + // + // **And `Ok` is not the same as ended, which is a defect this nearly shipped.** + // `Some(Ok(_))` also covers `AwaitingAnswer`, `AwaitingPlan`, + // `AwaitingRecovery`, `Denied`, `Refused`, `PlanRejected`, `Stalled`, + // `Escalated` and `Cancelled`. A parked run judged by the gate is told + // `/resume opens it` four lines below and then, in the same pass, buried + // under a fresh billed retry turn that tells the model its work failed — + // while the question the operator was asked is never answered. A gate is a + // verdict on work that finished, so only the outcomes that mean the run + // reached its own end are handed over. `exec::code` already owns that + // classification and is reused rather than re-derived. + // + // **A ceiling counts, and the live rehearsal is why.** A run whose criterion + // keeps failing spends its whole step budget doing so and comes back + // `StepCapReached` — that is what 0.24.0's own live arm recorded, `Failed` at + // attempt six. Excluding ceilings would mean the commonest failing-gate ending + // was never judged and never retried, which is the opposite of the intent. let ran = match &outcome { - Some(Ok(result)) => Some(result.run_id), + Some(Ok(result)) + if matches!( + io_cli::exec::code(&result.outcome), + io_cli::exec::OK | io_cli::exec::CEILING + ) => + { + Some(result.run_id) + } _ => None, }; diff --git a/src/reviewer.rs b/src/reviewer.rs index dbc6869..9fd43f9 100644 --- a/src/reviewer.rs +++ b/src/reviewer.rs @@ -101,8 +101,14 @@ pub fn build(spec: &ProviderSpec, model: &str) -> Result, Stri // `ProviderSpec` is `#[non_exhaustive]`. A provider this release has not // seen cannot be built into a reviewer, and refusing where the operator is // still looking at what they typed is the whole point of refusing at all. - other => Err(format!( - "this release does not know how to review with a {other:?} provider yet" - )), + // + // **The spec is never formatted into the message.** Every variant holds + // `api_key: Option` verbatim and `ProviderSpec` derives `Debug`, + // so `{other:?}` would put the operator's credential into a refusal that + // this crate then records into the scrollback. That is the same trap + // `Printable`'s hand-written `Debug` exists to avoid two files away. + _ => Err( + "this release does not know how to review with that kind of provider yet".to_string(), + ), } } diff --git a/src/status.rs b/src/status.rs index 4ee1317..b1ce377 100644 --- a/src/status.rs +++ b/src/status.rs @@ -657,9 +657,16 @@ impl Status { // two, which is the per-turn field this codebase has already shipped // stale once. // - // A gate *retry* is not affected: the retries happen inside io-harness's - // own turn, under one `App` turn and therefore one `start_run`, so the - // failure that caused the retry is still on screen while the retry runs. + // **A gate retry IS affected, and the honest note is worth more than the + // reassuring one.** An earlier draft of this comment said the retries + // happen inside io-harness's own turn under a single `start_run`. They do + // not: io-cli drives the retry as a fresh turn through the driver's own + // queue, so it arrives here and the standing is cleared for the duration + // of the very turn it explains. What stays on screen is the scrollback + // record, which is permanent and says what failed; the footer field + // reappears when the retry is judged in its turn. Keeping the word across + // the chain would mean not clearing it here, and that is the same door the + // stale-per-turn defect above came through. self.gate = None; self.gate_attempt = None; } diff --git a/tests/gates.rs b/tests/gates.rs index 11c9b01..3b5f08e 100644 --- a/tests/gates.rs +++ b/tests/gates.rs @@ -225,9 +225,17 @@ fn f5_a_reviewer_equal_to_the_turn_model_is_refused_unless_allowed() { // A caller that does not yet know which model will run cannot be told there // is a clash, and must not be told there is none either — it is told nothing, // which is what an empty model means. + // Asserted on the criterion it resolves to, not on `is_ok()`. `Ok(None)` — a + // silently ungated run, the exact outcome every refusal here exists to + // prevent — satisfies `is_ok()` just as happily as the right answer does. assert!( - judging_itself.criterion("").is_ok(), - "with no turn model there is nothing to compare the reviewer against" + matches!( + judging_itself.criterion(""), + Ok(Some(Criterion::Review { .. })) + ), + "with no turn model there is nothing to compare the reviewer against, so \ + the rubric stands: {:?}", + judging_itself.criterion(""), ); } @@ -340,12 +348,16 @@ fn f4_existence_is_never_an_empty_needle() { } ); - if let Verification::WorkspaceFileContains { needle, .. } = criterion.verification() { - panic!( - "existence was mapped to a contains criterion with needle {needle:?}, \ - which a file that does not exist satisfies" - ); - } + // Asserted as an equality rather than by ruling out one variant. Naming only + // `WorkspaceFileContains` leaves `FileContains("")` and `DocumentContains` + // with an empty needle passing — the same empty-needle bug wearing a different + // variant, which is precisely what this assertion is for. + assert!( + matches!(criterion.verification(), Verification::None), + "bare existence has no honest io-harness criterion, so nothing is asked of \ + the harness and io-cli answers it itself; got {:?}", + criterion.verification(), + ); assert!( criterion.checked_here(), "the dependency cannot express existence, so this crate must own it, and \ diff --git a/tests/steer.rs b/tests/steer.rs index 84561b3..1d34e55 100644 --- a/tests/steer.rs +++ b/tests/steer.rs @@ -465,7 +465,7 @@ fn f7_an_existence_criterion_is_evaluated_where_io_harness_did_not() { }; // Nothing recorded, because nothing was asked of io-harness. - let missing = io_cli::app::gate_attempts(Vec::new(), Some(&criterion), dir.path()); + let missing = io_cli::gates::gate_attempts(Vec::new(), Some(&criterion), dir.path()); let standing = io_cli::gates::standing(&missing) .expect("an existence criterion that reports nothing is an ungated run"); assert_eq!(standing.outcome, io_harness::GateOutcome::Failed); @@ -475,7 +475,7 @@ fn f7_an_existence_criterion_is_evaluated_where_io_harness_did_not() { ); std::fs::write(dir.path().join("REPORT.md"), "done").expect("the report"); - let written = io_cli::app::gate_attempts(Vec::new(), Some(&criterion), dir.path()); + let written = io_cli::gates::gate_attempts(Vec::new(), Some(&criterion), dir.path()); assert_eq!( io_cli::gates::standing(&written) .expect("a standing") @@ -483,13 +483,21 @@ fn f7_an_existence_criterion_is_evaluated_where_io_harness_did_not() { io_harness::GateOutcome::Passed, ); - // **A `none` row is dropped, not kept.** A run that spends its step cap DOES - // reach the gate with `Verification::None`, which passes trivially and records - // `phase = "none", passed` — a row saying a criterion that checked nothing was - // satisfied. Left as the last row it would report a missing file as a pass. + // **A `none` row is dropped, not kept — and the row shape here is the whole + // point.** io-harness evaluates the contract's criterion after every step on + // which the agent called a tool, and for `Verification::None` that evaluation + // is `Ok(false)` — so it records `phase = "none", **Failed**`, once per such + // step, on every ungated run this product has ever driven. An earlier draft of + // this test used `Passed`, a row production never emits, and so proved nothing + // about the case that actually exists: read naively those rows say the gate + // failed on a session nobody gated. std::fs::remove_file(dir.path().join("REPORT.md")).expect("take it away again"); - let folded = io_cli::app::gate_attempts( - vec![attempt(7, "none", io_harness::GateOutcome::Passed)], + let folded = io_cli::gates::gate_attempts( + vec![ + attempt(5, "none", io_harness::GateOutcome::Failed), + attempt(6, "none", io_harness::GateOutcome::Failed), + attempt(7, "none", io_harness::GateOutcome::Failed), + ], Some(&criterion), dir.path(), ); @@ -513,11 +521,38 @@ fn f7_an_existence_criterion_is_evaluated_where_io_harness_did_not() { }; let rows = vec![attempt(1, "command", io_harness::GateOutcome::Passed)]; assert_eq!( - io_cli::app::gate_attempts(rows.clone(), Some(&command), dir.path()), + io_cli::gates::gate_attempts(rows.clone(), Some(&command), dir.path()), rows, "a command gate is not re-judged here", ); - assert!(io_cli::app::gate_attempts(Vec::new(), None, dir.path()).is_empty()); + assert!(io_cli::gates::gate_attempts(Vec::new(), None, dir.path()).is_empty()); + + // **The case that would have shipped exit 6 to every operator alive.** A + // session with no `[app.io-cli.gates]` at all still leaves `none`/`Failed` + // rows behind for any turn in which the model called a tool. Folded with no + // criterion they must come to nothing — no standing, no status word, no + // scrollback line, and in `io exec` no exit 6. + let ungated = vec![ + attempt(1, "none", io_harness::GateOutcome::Failed), + attempt(2, "none", io_harness::GateOutcome::Failed), + ]; + let folded = io_cli::gates::gate_attempts(ungated, None, dir.path()); + assert!( + folded.is_empty(), + "an ungated run's bookkeeping rows are not a verdict: {folded:?}", + ); + assert!( + io_cli::gates::standing(&folded).is_none(), + "a run nobody gated has no standing", + ); + assert_eq!( + io_cli::exec::verified_code( + &io_harness::RunOutcome::Finished { steps: 2 }, + io_cli::gates::standing(&folded).as_ref(), + ), + io_cli::exec::OK, + "an ungated headless run exits exactly what it exited before this release", + ); } /// F6 — the scrollback line spells the verdict the way everything else does. @@ -534,10 +569,22 @@ fn f6_the_gate_line_names_the_phase_the_verdict_and_what_it_printed() { )]; let (tone, line) = io_cli::app::gate_report(&attempts, &events).expect("a gated turn has a line"); + // **`Warning`, and `Refused` would be a defect rather than a preference.** + // `Tone::Refused` renders the literal word `refused`, which is the permission + // boundary's word — this release moved the failing review off that tone in + // `src/events.rs` for exactly that reason, and a scrollback line one row below + // spelling `refused: gate failed` would put the collision straight back. + // "The policy would not run my gate" and "your work did not meet the bar" + // need opposite responses from the operator. assert_eq!( + tone, + io_cli::theme::Tone::Warning, + "a gate that answered no did not refuse anything: `refused` is the boundary's word", + ); + assert_ne!( tone, io_cli::theme::Tone::Refused, - "a gate that answered no is a refusal, not an error: nothing went wrong", + "the gate's verdict must not borrow the permission boundary's vocabulary", ); assert!( line.contains("failed") && line.contains("command") && line.contains("2 tests failed"), @@ -628,7 +675,7 @@ fn f7_the_retry_rides_the_drivers_own_queue() { // only the current run's rows to `may_retry` would buy another turn for every // one of them, forever, against a real model. assert!( - flat.contains("gated.extend(io_cli::app::gate_attempts("), + flat.contains("gated.extend(io_cli::gates::gate_attempts("), "the chain's attempts accumulate; `retries = 1` means one further turn, \ not an unbounded loop", ); From 808f592cfade4e5d41d2795601ab0cae4ccde2b3 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:42:41 +0530 Subject: [PATCH 14/16] docs(contract): the rustdoc gate found a public doc linking a private item criterion_of is public and criterion_for is not, so the intra-doc link could not resolve under --document-private-items-less rustdoc. Written in plain backticks. Second catch by that gate in this release, and it is not in ci.yml. --- drive.py | 151 ++++++++++++++++++++++++++++++++++++++++++++++++ src/contract.rs | 2 +- 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 drive.py diff --git a/drive.py b/drive.py new file mode 100644 index 0000000..75e0957 --- /dev/null +++ b/drive.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +"""Drive io's inline TUI on a pty that answers the queries a real terminal answers. + +`script -q /dev/null ./io` is not enough: io asks the terminal where its cursor is +(`ESC[6n`) before drawing anything, and a bare pty has no emulator behind it to +reply, so the program exits with the sentence it prints for exactly that case. + +This is the smallest thing that makes a headless run of the real binary possible: +a pty that replies to the cursor-position report and to the primary device +attributes query, echoes nothing, and writes the whole byte stream to a file so a +gate can assert on it. + +Usage: drive.py with a script of keystrokes on stdin as + "\t" lines. + +**A script that keys off a clock is a flaky script.** 0.11.0 spent three runs of +the same script producing three different captures — the palette not opening, the +palette not closing, and the whole thing working — because a keystroke sent at a +fixed second lands wherever the machine's load puts it. So an entry whose text is +`wait:` sends nothing and blocks until `` has appeared in what the +program wrote. A script that waits for the prompt before typing at it is a script +that does the same thing twice. +""" +import fcntl, os, pty, re, select, struct, sys, termios, time + +CURSOR_QUERY = re.compile(rb"\x1b\[6n") +DA1_QUERY = re.compile(rb"\x1b\[(?:0)?c") +# The Kitty keyboard-enhancement query. Answering "not supported" keeps the +# negotiation from costing its full timeout. +KITTY_QUERY = re.compile(rb"\x1b\[\?u") +# Whether this pty pretends to be a terminal that speaks the Kitty keyboard +# protocol. Default yes, as every release before 0.13.0 assumed. +KITTY = os.environ.get("IO_DRIVE_KITTY", "1") != "0" + +out_path, cmd = sys.argv[1], sys.argv[2:] +script = [] +for raw in sys.stdin.read().splitlines(): + if not raw.strip(): + continue + delay, _, text = raw.partition("\t") + script.append((float(delay), text)) + +pid, fd = pty.fork() +if pid == 0: + # The graphics decision is read from the environment, so a run that + # INHERITED the parent shell's `TERM_PROGRAM` would be testing this machine's + # terminal rather than the arm the gate is about. Every variable the decision + # looks at is cleared, then set from IO_DRIVE_ENV. + for name in ("TERM_PROGRAM", "KITTY_WINDOW_ID", "GHOSTTY_RESOURCES_DIR", + "KONSOLE_VERSION", "TMUX", "STY"): + os.environ.pop(name, None) + os.environ["TERM"] = "xterm-256color" + size = os.environ.get("IO_DRIVE_SIZE", "30x100").split("x") + os.environ["LINES"], os.environ["COLUMNS"] = size[0], size[1] + for pair in os.environ.pop("IO_DRIVE_ENV", "").split(","): + if "=" in pair: + key, _, value = pair.partition("=") + os.environ[key] = value + os.execvp(cmd[0], cmd) + +# `pty.fork` leaves the window at 0x0, and an inline viewport clamped to +# `rows - 2` is then zero rows tall — the program runs, draws nothing, and looks +# hung. Setting a real size is what makes this a terminal rather than a pipe. +rows, _, cols = os.environ.get("IO_DRIVE_SIZE", "30x100").partition("x") +fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH", int(rows), int(cols), 0, 0)) + +captured = bytearray() +started = time.monotonic() +# The first entry's delay gates the first entry, not the second. +next_at = started + (script[0][0] if script else 0) +deadline = started + float(os.environ.get("IO_DRIVE_DEADLINE", "60")) +sent = 0 +# Where the next `wait:` starts looking. See the wait arm below. +seen_from = 0 + +while time.monotonic() < deadline: + ready, _, _ = select.select([fd], [], [], 0.2) + if ready: + try: + chunk = os.read(fd, 65536) + except OSError: + break + if not chunk: + break + captured += chunk + # Reply as a terminal would. Position is arbitrary but must be plausible. + if CURSOR_QUERY.search(chunk): + os.write(fd, b"\x1b[24;1R") + # **The keyboard answer goes out BEFORE the device-attributes one, and the + # order is the whole of whether it is heard.** crossterm asks the two + # together — `CSI ? u` then `CSI c` — and treats the DA1 reply as the end + # of the answer: whatever has not arrived by then is "not supported". Both + # queries land in one read here, so a driver that answered in the order it + # happened to check told every session this terminal speaks no protocol. + # It did, until 0.13.0, and the first F9 capture pair was byte-identical + # because of it. + # + # 0.13.0 — answering at all is now a choice, because F9 is about the + # difference. A terminal that speaks the protocol replies with its current + # flags; one that does not (Apple's Terminal, most `script` sessions) + # replies never, and crossterm's two-second wait is paid in full and comes + # back "no". `IO_DRIVE_KITTY=0` is that second terminal. + if KITTY_QUERY.search(chunk) and KITTY: + os.write(fd, b"\x1b[?0u") + if DA1_QUERY.search(chunk): + os.write(fd, b"\x1b[?1;2c") + now = time.monotonic() + if sent < len(script) and now >= next_at: + delay, text = script[sent] + # `wait:` sends nothing. It holds the script until the program has + # written the text it names, which is how a keystroke lands where it was + # meant to rather than where the machine's load put it. + if text.startswith("wait:"): + # Only what has arrived since the previous step counts. The capture + # is cumulative, so a wait for `ready` that searched the whole of it + # would match the status line the session drew before the turn ever + # started — and every wait after it would fall through at once. + if text[5:].encode() not in captured[seen_from:]: + continue + seen_from = len(captured) + sent += 1 + next_at = now + (script[sent][0] if sent < len(script) else 0) + continue + # `raw:` sends the keystrokes with no Enter after them, which is what a + # key that OPENS something needs: `/` opens the command palette on the + # keypress itself, so a `/` followed by Enter would submit the palette + # rather than open it. `\xNN` escapes carry control keys. + if text.startswith("raw:"): + payload = text[4:].encode().decode("unicode_escape").encode("latin-1") + else: + payload = text.encode().decode("unicode_escape").encode("latin-1") + b"\r" + os.write(fd, payload) + seen_from = len(captured) + sent += 1 + next_at = now + (script[sent][0] if sent < len(script) else 0) + if sent >= len(script) and now > next_at + 3: + break + +os.close(fd) +try: + os.waitpid(pid, os.WNOHANG) +except ChildProcessError: + pass +try: + os.kill(pid, 9) +except ProcessLookupError: + pass + +with open(out_path, "wb") as handle: + handle.write(bytes(captured)) +print(f"captured {len(captured)} bytes") diff --git a/src/contract.rs b/src/contract.rs index 9a84adc..e095764 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -323,7 +323,7 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta /// /// What a *reader* wants: the surfaces that report a verdict need to know which /// criterion was in force, and building a provider to tell them would resolve a -/// credential for a question that does not need one. [`criterion_for`] is the +/// credential for a question that does not need one. `criterion_for` is the /// half that also builds the reviewer, for the contract that will run it. /// /// This is what lets `io exec` fold the gate rows the same way a session does — From 02dda49cf485410a6cb3edbf2f4a60efa6a0b458 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:48:28 +0530 Subject: [PATCH 15/16] chore(release): seal the 0.24.0 record Sealed during completion, on the feature branch, before the merge and the tag. The Release URL is deterministic so it is written now; the artifact byte sizes and the smoke on the published binary are not knowable yet and belong in the final report. Suite 86/1,255/0. Live 28/28 against a real model. clippy, fmt and the nightly rustdoc gate clean. Three adversarial reviews, four HIGH defects found behind a green suite and fixed. transition RELEASED did not pin the lock, for the thirty-third release running; pinned by hand. --- drive.py | 151 ------------------------------------------------------- 1 file changed, 151 deletions(-) delete mode 100644 drive.py diff --git a/drive.py b/drive.py deleted file mode 100644 index 75e0957..0000000 --- a/drive.py +++ /dev/null @@ -1,151 +0,0 @@ -#!/usr/bin/env python3 -"""Drive io's inline TUI on a pty that answers the queries a real terminal answers. - -`script -q /dev/null ./io` is not enough: io asks the terminal where its cursor is -(`ESC[6n`) before drawing anything, and a bare pty has no emulator behind it to -reply, so the program exits with the sentence it prints for exactly that case. - -This is the smallest thing that makes a headless run of the real binary possible: -a pty that replies to the cursor-position report and to the primary device -attributes query, echoes nothing, and writes the whole byte stream to a file so a -gate can assert on it. - -Usage: drive.py with a script of keystrokes on stdin as - "\t" lines. - -**A script that keys off a clock is a flaky script.** 0.11.0 spent three runs of -the same script producing three different captures — the palette not opening, the -palette not closing, and the whole thing working — because a keystroke sent at a -fixed second lands wherever the machine's load puts it. So an entry whose text is -`wait:` sends nothing and blocks until `` has appeared in what the -program wrote. A script that waits for the prompt before typing at it is a script -that does the same thing twice. -""" -import fcntl, os, pty, re, select, struct, sys, termios, time - -CURSOR_QUERY = re.compile(rb"\x1b\[6n") -DA1_QUERY = re.compile(rb"\x1b\[(?:0)?c") -# The Kitty keyboard-enhancement query. Answering "not supported" keeps the -# negotiation from costing its full timeout. -KITTY_QUERY = re.compile(rb"\x1b\[\?u") -# Whether this pty pretends to be a terminal that speaks the Kitty keyboard -# protocol. Default yes, as every release before 0.13.0 assumed. -KITTY = os.environ.get("IO_DRIVE_KITTY", "1") != "0" - -out_path, cmd = sys.argv[1], sys.argv[2:] -script = [] -for raw in sys.stdin.read().splitlines(): - if not raw.strip(): - continue - delay, _, text = raw.partition("\t") - script.append((float(delay), text)) - -pid, fd = pty.fork() -if pid == 0: - # The graphics decision is read from the environment, so a run that - # INHERITED the parent shell's `TERM_PROGRAM` would be testing this machine's - # terminal rather than the arm the gate is about. Every variable the decision - # looks at is cleared, then set from IO_DRIVE_ENV. - for name in ("TERM_PROGRAM", "KITTY_WINDOW_ID", "GHOSTTY_RESOURCES_DIR", - "KONSOLE_VERSION", "TMUX", "STY"): - os.environ.pop(name, None) - os.environ["TERM"] = "xterm-256color" - size = os.environ.get("IO_DRIVE_SIZE", "30x100").split("x") - os.environ["LINES"], os.environ["COLUMNS"] = size[0], size[1] - for pair in os.environ.pop("IO_DRIVE_ENV", "").split(","): - if "=" in pair: - key, _, value = pair.partition("=") - os.environ[key] = value - os.execvp(cmd[0], cmd) - -# `pty.fork` leaves the window at 0x0, and an inline viewport clamped to -# `rows - 2` is then zero rows tall — the program runs, draws nothing, and looks -# hung. Setting a real size is what makes this a terminal rather than a pipe. -rows, _, cols = os.environ.get("IO_DRIVE_SIZE", "30x100").partition("x") -fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH", int(rows), int(cols), 0, 0)) - -captured = bytearray() -started = time.monotonic() -# The first entry's delay gates the first entry, not the second. -next_at = started + (script[0][0] if script else 0) -deadline = started + float(os.environ.get("IO_DRIVE_DEADLINE", "60")) -sent = 0 -# Where the next `wait:` starts looking. See the wait arm below. -seen_from = 0 - -while time.monotonic() < deadline: - ready, _, _ = select.select([fd], [], [], 0.2) - if ready: - try: - chunk = os.read(fd, 65536) - except OSError: - break - if not chunk: - break - captured += chunk - # Reply as a terminal would. Position is arbitrary but must be plausible. - if CURSOR_QUERY.search(chunk): - os.write(fd, b"\x1b[24;1R") - # **The keyboard answer goes out BEFORE the device-attributes one, and the - # order is the whole of whether it is heard.** crossterm asks the two - # together — `CSI ? u` then `CSI c` — and treats the DA1 reply as the end - # of the answer: whatever has not arrived by then is "not supported". Both - # queries land in one read here, so a driver that answered in the order it - # happened to check told every session this terminal speaks no protocol. - # It did, until 0.13.0, and the first F9 capture pair was byte-identical - # because of it. - # - # 0.13.0 — answering at all is now a choice, because F9 is about the - # difference. A terminal that speaks the protocol replies with its current - # flags; one that does not (Apple's Terminal, most `script` sessions) - # replies never, and crossterm's two-second wait is paid in full and comes - # back "no". `IO_DRIVE_KITTY=0` is that second terminal. - if KITTY_QUERY.search(chunk) and KITTY: - os.write(fd, b"\x1b[?0u") - if DA1_QUERY.search(chunk): - os.write(fd, b"\x1b[?1;2c") - now = time.monotonic() - if sent < len(script) and now >= next_at: - delay, text = script[sent] - # `wait:` sends nothing. It holds the script until the program has - # written the text it names, which is how a keystroke lands where it was - # meant to rather than where the machine's load put it. - if text.startswith("wait:"): - # Only what has arrived since the previous step counts. The capture - # is cumulative, so a wait for `ready` that searched the whole of it - # would match the status line the session drew before the turn ever - # started — and every wait after it would fall through at once. - if text[5:].encode() not in captured[seen_from:]: - continue - seen_from = len(captured) - sent += 1 - next_at = now + (script[sent][0] if sent < len(script) else 0) - continue - # `raw:` sends the keystrokes with no Enter after them, which is what a - # key that OPENS something needs: `/` opens the command palette on the - # keypress itself, so a `/` followed by Enter would submit the palette - # rather than open it. `\xNN` escapes carry control keys. - if text.startswith("raw:"): - payload = text[4:].encode().decode("unicode_escape").encode("latin-1") - else: - payload = text.encode().decode("unicode_escape").encode("latin-1") + b"\r" - os.write(fd, payload) - seen_from = len(captured) - sent += 1 - next_at = now + (script[sent][0] if sent < len(script) else 0) - if sent >= len(script) and now > next_at + 3: - break - -os.close(fd) -try: - os.waitpid(pid, os.WNOHANG) -except ChildProcessError: - pass -try: - os.kill(pid, 9) -except ProcessLookupError: - pass - -with open(out_path, "wb") as handle: - handle.write(bytes(captured)) -print(f"captured {len(captured)} bytes") From e125be517eff68bc81d7c95f1794fe758579339a Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Fri, 28 Aug 2026 07:49:02 +0530 Subject: [PATCH 16/16] chore: never commit the live rehearsal harness again drive.py is copied to the repository root from the gitignored evidence directory to run the live suite, and a git add -A during 0.24.0 caught a copy that was sitting there. It was removed in the next commit and the net effect on the tree is nil, but the ignore entry is what stops it happening a second time. --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index 3cfa106..ff38111 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,8 @@ node_modules/ # gate evidence are the maintainer's working notes, written by tooling rather # than by hand. They are not a contributor-facing surface and are never committed. .ultraship/ + +# The live rehearsal harness, copied to the root from the gitignored evidence +# directory to run `cargo test --test live`. It briefly entered history in +# 0.24.0 because a `git add -A` ran while a copy was sitting here. +/drive.py