diff --git a/CHANGELOG.md b/CHANGELOG.md index 15aea29..8e526b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,84 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.26.0] - 2026-08-28 + +A turn asks for the model, the vendor and the amount of thinking the work +actually needs. Three of the four things in this release were already sitting in +io-harness, reachable and unasked for; the fourth was a list this interface had +been drawing for five releases without ever running it. + +**`/effort low`, `/effort medium`, `/effort high`, `/effort off`.** How much +reasoning a turn buys, said once and held: the level applies to that turn and to +every turn after it until you change it, it shows on the status line as `effort +high`, and a bare `/effort` reports what is in force and changes nothing. +io-harness owns the translation to each vendor's spelling — `reasoning_effort` on +the OpenAI wire, `reasoning: { effort }` on OpenRouter, a converted thinking +budget on Anthropic — and io-cli names a level and nothing else. `off` is not a +fourth level below `low`: it goes back to sending no reasoning field at all, which +is what every release before this one sent, and the line it commits says that +rather than naming a level. The level is the session's and is written to no file. + +**`/profile` moves from *this turn* to *the session*, and it is a correction +rather than a reshuffle** — the third time after 0.19.0's `/mcp` and `/provider` +and 0.22.0's `/image` and `/copy`. *this turn* means a command acting on the work +the turn just finished; `/profile` changes which configuration overlay every later +turn is built from, which is a property of the session. It is where `/help` lists +it now, which is the only visible trace of the move. That it also made room for +`/effort` is the order the bound was meant to force: the group stood at ten of +ten, and the answer written down in advance was to re-file what is in the wrong +group rather than widen the bound. + +**`[app.io-cli.routing]` changes the model mid-run.** `escalate_after` moves up to +a stronger model after that many consecutive failed verification-gate attempts; +`downshift_under` asks a cheaper one while the run has written fewer than that +many bytes to disk. Escalation happens once and does not come back down, and it +wins over downshifting where both apply — io-harness's rules, and it evaluates +them, so io-cli holds no counter of its own. + +**A rule that could only misfire is refused by name.** Half a rule, `failures = 0` +— which io-harness reads as "escalate before anything has failed", pinning every +run to the escalation model from its first request — `bytes = 0`, which can never +be true, and a model written as the empty string are each refused with the key +named, and the run goes unrouted rather than obeying them. The two keys inside +each rule are optional for the same reason: making them required meant a half +rule failed to deserialize `[app.io-cli]` entirely, which silently took the +theme, the keys, the ceilings, the capabilities and the verification gate with +it. + +**Routing does not reach a contained turn, and this release says so rather than +letting it be discovered.** io-harness applies routing in its flat workspace loop +only; a turn run under `[app.io-cli.containment]` takes each agent's model from +that agent's own roster entry and never consults the rules. For an operator with +containment configured the section parses, is listed by `/config`, reaches the +contract and never fires. A session with both is told at start, on `/config`, and +when `/contain on` is typed, and a session without containment is told nothing, because a caveat attached to a working +feature is how somebody learns to stop reading the notices. `io exec` uses the +flat loop, so routing works there, and so does a turn taken with `/contain off`. +There is no `require_primary` key: io-harness has the field, it gates on +`Provider::reachable`, which is defaulted to yes and which no provider in +io-harness 0.69 overrides, so a key for it would be permanently inert. + +**The `[[provider]]` chain is finally what runs, and this changes behaviour for a +file that already has more than one entry.** `/provider` has drawn the whole chain +since 0.21.0 while the product only ever asked its head. From this release the +next entry answers when the first fails in a way another vendor might survive — a +transport error, a timeout, a rate limit, a 5xx — with the fall-through committed +to the scrollback and the status line's provider field moving to whoever actually +answered. **A failure that will fail identically everywhere does not fall +through**, a bad API key above all, so a wrong credential on the primary cannot +start spending at the secondary. The predicate is io-harness's own. `--provider` +on `io exec` replaces the whole chain rather than heading it, because a run scoped +to one vendor on the command line must not spend at another. + +**A question that is only a question says so.** io-harness has answered such a +turn in one completion — no steps, no tools, no gate — for longer than this +interface has existed, and io-cli drew every line it draws about a turn from +events that turn does not emit, so what reached you was silence. It now commits +`answered without opening a run`. `conversational = false` in `[app.io-cli]` turns +the classification off so every prompt opens a full run; absent leaves the +behaviour exactly as it was. + ## [0.25.0] - 2026-08-28 The work a turn does ends as something somebody can review. io-harness has @@ -2113,6 +2191,6 @@ client, tool, sandbox, policy engine or session store of its own. - There is no crates.io publish and `cargo install` is not an install path. - No test in this release asserts on wall-clock time. -[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.23.0...HEAD +[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.26.0...HEAD [0.1.1]: https://github.com/initorigin/io-cli/releases/tag/v0.1.1 [0.1.0]: https://github.com/initorigin/io-cli/releases/tag/v0.1.0 diff --git a/Cargo.lock b/Cargo.lock index 12e7027..2fde11f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.25.0" +version = "0.26.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index b407a14..60e5bf1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.25.0" +version = "0.26.0" edition = "2021" # io-harness declares 1.95, set by `libsqlite3-sys` calling the std `cfg_select!` # macro from its build script. Nothing here can be lower than what the harness diff --git a/README.md b/README.md index 717f808..8473a02 100644 --- a/README.md +++ b/README.md @@ -334,6 +334,52 @@ same session — io re-reads the configuration for every turn, so an edit you ma in your own editor counts too, and a file that stops parsing leaves the last good one in force rather than ending the session. +### Answered without opening a run + +**A prompt that is only a question is answered in one completion**, with no steps, +no tools, no checkpoint and no verification gate. io-harness has classified turns +that way for longer than this interface has existed — a contract carrying no +criterion turns the classification on, and the contract io-cli builds carries none +until you write a gate — so a greeting has always come back from a single call. +What io-cli never did was *say* so. Every line this product draws about a turn is +drawn from events that a turn like this does not emit, so what reached you was +silence, and a fast answer with nothing above it reads as something having gone +wrong. From 0.26.0 such a turn commits one line: `answered without opening a run — +one completion, no steps and no tools`. + +**`conversational` in `[app.io-cli]` is where you overrule it.** `false` opens a +full run for every prompt; absent leaves the behaviour exactly as it is, which is +why there is nothing to write here for the ordinary case. The key earns its place +on the other side of the gate: attaching a criterion is what io-harness reads as +"this turn is not a conversation", so a repository with a gate would otherwise +have every idle question turned into an agent run that then runs the test suite +after each of its steps. io-cli sets classification back on wherever a criterion +was attached, and this key is how you say you wanted the runs. + +### How much reasoning a turn buys + +**`/effort low`, `/effort medium`, `/effort high`, `/effort off`.** The level is a +posture and not a one-shot: it holds for the turn you set it on and for every turn +after it until you change it, it sits on the status line as `effort high`, and a +bare `/effort` reports what is in force and changes nothing. + +What reaches the wire is the vendor's own spelling, and io-harness owns the +translation: `reasoning_effort` on the OpenAI wire, `reasoning: { effort }` on +OpenRouter, and a converted thinking budget on Anthropic — 1024, 4096 and 16384 +tokens for `low`, `medium` and `high`. io-cli names a level and nothing else, +because a per-vendor number chosen here would be a second opinion about somebody +else's request body. + +**`off` is not a fourth level below `low`.** It goes back to sending no reasoning +field at all, which is what every release before 0.26.0 sent, and the line it +commits says that rather than naming a level — calling it "off" on screen would +suggest a setting between `low` and nothing. + +The level is this session's and is written to no file. There is no `[app.io-cli]` +key for it: how much thinking to buy is a thing you change while you work — a +cheap question, then a hard one — and a value on disk would be one more setting +that is in force on a session you have forgotten setting it for. + ## Keys @@ -452,6 +498,7 @@ above a row that ranked there for reasons having nothing to do with it. | `/clear` | start a new conversation; this one stays in /resume | | `/resume` | reopen an earlier session and answer whatever its last run is waiting on | | `/fork` | continue from an earlier turn of this conversation | +| `/profile` | switch to a named profile from the configuration, for this session | | `/setup` | run the first-run wizard again | | `/exit` | leave | @@ -460,9 +507,9 @@ above a row that ranked there for reasons having nothing to do with it. | Command | Does | | --- | --- | | `/model` | change the model the next turn is sent to | +| `/effort` | how much reasoning the next turn buys: low, medium, high, or off | | `/contain` | run turns contained, so the agent can fan out: on, off, or ask | | `/plan` | make turns propose a plan before they work: on, off, or ask | -| `/profile` | switch to a named profile from the configuration, for this session | | `/steer` | send what is queued into the turn that is already running | | `/compact` | fold this conversation into a summary, at the next step | | `/image` | draw an attached image again: /image 1 | @@ -497,6 +544,27 @@ above a row that ranked there for reasons having nothing to do with it. | `/gates` | the check a turn must pass before it is done: a command, a file, or a rubric | | `/import` | bring instructions, MCP servers, skills and a model across from another agent | +`/effort` is new in 0.26.0 and sits under **this turn** beside `/model`, because +the two are the same question asked twice: which model the work goes to, and how +much thinking it is worth buying from it. It is a posture rather than a one-shot — +the level it sets holds for that turn and for every turn after it until you change +it — and a bare `/effort` reports the level in force and changes nothing. +`/effort off` is not a fourth level below `low`: it goes back to sending no +reasoning field at all, which is what every release before this one sent. See [How +much reasoning a turn buys](#how-much-reasoning-a-turn-buys). + +`/profile` sits under **the session** from 0.26.0, and it is a correction rather +than a reshuffle — the third time this sentence has been written, after 0.19.0's +`/mcp` and `/provider` and 0.22.0's `/image` and `/copy`. **this turn** means a +command acting on the work the turn just finished, and `/profile` acts on nothing +that has happened: it changes which configuration overlay every *later* turn is +built from, which is a property of the session. It was filed under **this turn** +because switching one feels like something you do between turns, and where a +command sits is decided by what it acts on rather than by when it is typed. That +it also made room for `/effort` is the order the bound was meant to force: the +group stood at ten of ten, and the answer had been written down in advance — +re-file what is in the wrong group, do not widen the bound. + `/commit` is new in 0.25.0 and sits under **this turn** beside `/copy` and `/copy diff` because it is the third thing you do with work that has just finished: one puts the answer somewhere, one puts the patch somewhere, and this one makes the @@ -1217,6 +1285,69 @@ back with. `io exec` gains exit `6` for this — the agent finished and the work does not hold up. See [Exit status](#exit-status). +## Which model a run asks + +A gate says what "done" means. This says what to do when the model that has to +reach it keeps missing, and what to do when the work turns out not to have needed +the model you started with. `[app.io-cli.routing]` holds two optional rules, each +written as a sub-table because a rule is a threshold and a model that only mean +anything together — a threshold with no model and a model with no threshold are +both half a rule, and a sub-table makes the pair the unit the file itself +enforces: + +```toml +[app.io-cli.routing.escalate_after] +failures = 3 +model = "a-stronger-model" + +[app.io-cli.routing.downshift_under] +bytes = 2000 +model = "a-cheaper-model" +``` + +**`escalate_after` moves up after that many consecutive failed gate attempts** — +the gates above, counted consecutively rather than cumulatively, because a run +that fails, recovers, and fails again much later is a run doing hard work rather +than one that needs a bigger model. **`downshift_under` asks the cheaper model +while the run has written fewer than that many bytes to disk**, measured on what +was actually written rather than on what was planned, so it is a fact about the +run rather than a forecast of it. Neither key defaults: half a rule is a parse +error naming the key you left out, rather than a threshold quietly reading as +zero and escalating on the first gate attempt of every turn. + +Escalation happens **once** and does not come back down, and escalation **wins** +over downshifting where both apply. Both of those are io-harness's rules rather +than io-cli's: it owns the consecutive-failure count, the byte total and the +decision, taken after every step of the run. io-cli evaluates none of it, because +a second implementation here would be a second answer that drifts from the one the +run actually used. + +**Routing does not reach a contained turn, and that is the first thing to know +about it.** io-harness applies routing in its flat workspace loop only; a turn run +under `[app.io-cli.containment]` takes each agent's model from that agent's own +roster entry and never consults the rules. So for an operator who has configured +containment, the section parses, is listed by `/config`, reaches the contract — +and never fires. A session that has both is told so at three moments: at start, when `/config` +is opened on the keys themselves, and when `/contain on` is typed — which is the +one that matters most, because that operator began uncontained, was told nothing +because nothing applied, and has just moved into the mode where their rules do +not fire. A session with containment off is told nothing, because a caveat +attached to a feature that is working is how an operator learns to stop reading +the notices. A turn taken with +`/contain off` routes normally, and `io exec` uses the flat loop, so routing works +there. Nothing in io-cli can close this: the loop that would have to consult the +rules is the dependency's, and what this interface owes you meanwhile is the +disclosure. + +**There is no `require_primary` key**, and its absence is a decision rather than +an omission. io-harness's own `Routing` carries the field, and it gates on +`Provider::reachable` — a defaulted trait method whose body answers yes, and which +no provider in io-harness 0.69 overrides. A key for it would be offered on a +surface, accepted from a file, and permanently inert: you would set it, believe an +unattended overnight run now refuses to start against a dead endpoint, and get +exactly the behaviour you had before. It goes in when a provider answers the +question. + ## Git A gate says the work holds up. This says what becomes of it. @@ -1711,6 +1842,25 @@ Reordering moves an entry with its own comments and its own keys rather than rebuilding the array, because a chain rebuilt from io-cli's model would silently drop whatever io-cli does not model. +**From 0.26.0 the chain is what runs.** Since 0.21.0 that panel has drawn the +whole order while the product only ever asked its head, so a second +`[[provider]]` entry was a row on a screen and nothing else. It answers now: when +the first provider fails in a way another vendor might survive — a transport +error, a timeout, a rate limit, a 5xx — the next entry is asked, the fall-through +is committed to the scrollback as it happens, and the status line's provider field +moves to whoever actually answered. **A failure that will fail identically +everywhere does not fall through**, a bad API key above all, so a wrong credential +on the primary can never start spending at the secondary. That predicate is +io-harness's own — the same one its `Fallback` and its in-run retry ask — and +io-cli holds no opinion here and must not grow one. **This is a behaviour change +for a file that already has more than one entry**: the second one starts being +used. + +`--provider` on `io exec` and `io resume` replaces the whole chain rather than +heading it. Naming a provider on the command line is saying which endpoint this +run uses, and keeping the file's fallbacks underneath it would let a run you +scoped to one vendor spend at another. + **Both of those panels could only list until 0.21.0.** The writers were there and tested and called from nothing, while three places in this documentation said they "add, edit, disable and remove". They genuinely write now, and the word *disable* @@ -1730,7 +1880,7 @@ bundles](#capability-bundles). **`/profile`** switches to a named `[profile.]` for the session, and `--profile ` picks one for a single run without writing anything. -Eight keys live there, and seven tables: +Nine keys live there, and eight tables: | Key | Is | | --- | --- | @@ -1742,12 +1892,14 @@ Eight keys live there, and seven tables: | `max_parallel_reads` | how many read-only tool calls one turn may run at once. Absent, it is io-harness's own 10; `0` is clamped to 1 rather than meaning none. A `TaskContract` field with no io-harness configuration key of its own, which is why it is named here. | | `spawn_background_after_secs` | how long a spawned child may run before it is backgrounded. Absent, a child is waited for however long it takes. | | `detached_spawns` | whether a spawn may detach at all. Absent, it may. `false` buys a trace with every child's whole life in it, which a detached child gives up. | +| `conversational` | whether a prompt that is only a question may be answered in one completion, with no steps and no tools. Absent leaves io-harness's own classification where it is, which is what every release before 0.26.0 did; `false` opens a full run for every prompt. See [Answered without opening a run](#answered-without-opening-a-run). | | `[app.io-cli.keys]` | the session's keys, by action name. See [Moving a key](#moving-a-key). | | `[app.io-cli.containment]` | the caps a fan-out runs under. Absent, a session cannot decompose anything. See [The fleet](#the-fleet). | | `[[app.io-cli.mcp]]` | MCP servers for the turn, in io-harness's own shape. Merged with the top-level `[[mcp]]`, and wins a collision of ids. | | `[[app.io-cli.lsp]]` | language servers for this workspace. Merged with the top-level `[[lsp]]`, and wins a collision of ids. | | `[app.io-cli.browser]` | a browser the agent may drive. Never downloaded — it is one you already have. | | `[app.io-cli.gates]` | what "done" means for this repository: one of `command` (with `expect_exit`), `file` (with `contains`), or `rubric` (with `reviewer`, and `allow_self_review` if the judge may be the model that did the work), plus `retries`, which defaults to 1 and is report-only at 0. Naming none of the three, or more than one, is refused rather than resolved by precedence. See [Verification gates](#verification-gates). | +| `[app.io-cli.routing]` | when a run should change models, and to which: `escalate_after` with `failures` and `model`, `downshift_under` with `bytes` and `model`, each a sub-table and both optional. Absent, a run asks one model from the first token to the last. **The rules do not fire under `[app.io-cli.containment]`**, which the session says at start, on `/config`, and when `/contain on` is typed. A rule that cannot be obeyed — half a rule, a threshold of zero, or an empty model — is refused by name and leaves the run unrouted. See [Which model a run asks](#which-model-a-run-asks). | | `[app.io-cli.prices]` | where the rates in `[prices]` came from: `source_url` names a catalogue to read instead of io-harness's default, and `source` and `models` record what the last read was and how many models it priced. The last two are written by a fetch rather than by hand. See [Where a price comes from](#where-a-price-comes-from). | Because the section is unvalidated by design, an unrecognised *value* reads as the diff --git a/docs/config.example.toml b/docs/config.example.toml index 1c3d61b..b4197ea 100644 --- a/docs/config.example.toml +++ b/docs/config.example.toml @@ -388,7 +388,7 @@ net = "deny" # --- io-cli's own section --------------------------------------------------- # -# Eight keys and seven tables. io-harness does not validate any of it, so an +# Nine keys and eight tables. io-harness does not validate any of it, so an # unrecognised VALUE reads as the default rather than stopping a session. A # table io-harness cannot parse at all is different: the session starts on the # defaults and says so, carrying the harness's own message. @@ -425,6 +425,24 @@ diff = "unified" # says which way round it is. plain = false # +# Whether a prompt that is only a question may be answered in one completion. +# +# io-harness has classified turns since long before io-cli asked it to: a contract +# carrying no verification criterion turns classification on, and a question then +# comes back after ONE completion — no steps, no tools, no checkpoint and no gate. +# Absent leaves that where it is, which is what every release before 0.26.0 did, +# so there is nothing to write here for the ordinary case. +# +# false OPENS A FULL RUN FOR EVERY PROMPT, which is what you want in a session +# meant to work rather than to answer. The key earns its place on the other side +# of a gate: attaching a criterion is what io-harness reads as "this turn is not a +# conversation", so a repository with `[app.io-cli.gates]` would otherwise have +# every idle question become an agent run that then runs the gate after each of +# its steps. io-cli sets classification back on wherever it attached a criterion, +# and this key is how you say you wanted the runs after all. +# +# conversational = false +# # How many steps one turn may take. # # DEPRECATED IN 0.14.0 AND REMOVED IN 0.16.0. It existed because the flat turn @@ -687,3 +705,56 @@ fleet = "ctrl+f" # rubric = "every public item changed in this turn has a doc comment saying why" # reviewer = "anthropic/claude-sonnet-4.5" # allow_self_review = false + +# --- which model a run asks (0.26.0) ---------------------------------------- +# +# Two optional rules. Each is a SUB-TABLE rather than a pair of flat keys because +# a rule is a threshold and a model that only mean anything together: a threshold +# with no model and a model with no threshold are both half a rule. Neither key +# defaults, so half a rule is a parse error naming the key you left out, rather +# than a threshold quietly reading as zero — which would escalate on the very +# first gate attempt of every turn. +# +# `escalate_after` moves UP after that many CONSECUTIVE failed gate attempts — +# the gates above. Consecutive rather than cumulative: a run that fails, recovers +# and fails again much later is doing hard work, not asking for a bigger model. +# +# `downshift_under` asks the cheaper model while the run has written fewer than +# that many bytes to disk, measured on what was WRITTEN rather than on what was +# planned — a fact about the run rather than a forecast of it. +# +# ESCALATION HAPPENS ONCE AND DOES NOT COME BACK DOWN, AND IT WINS OVER +# DOWNSHIFTING WHERE BOTH APPLY. Both are io-harness's own rules: it owns the +# failure count, the byte total and the decision, taken after every step. io-cli +# evaluates none of it, because a second implementation here would be a second +# answer that drifts from the one the run actually used. +# +# THESE RULES DO NOT FIRE UNDER [app.io-cli.containment]. io-harness applies +# routing in its flat workspace loop only, and a contained turn takes each agent's +# model from that agent's own definition without ever consulting them. The section +# still parses, still shows in /config and still reaches the contract — it simply +# never fires, and a session configured for both is told so at start, on /config, +# and when /contain on is typed. `io exec` uses the flat loop, and a turn taken +# with /contain off routes normally. +# +# A RULE THAT COULD ONLY MISFIRE IS REFUSED BY NAME rather than obeyed: half a +# rule, `failures = 0` (which io-harness reads as "escalate before anything has +# failed", pinning every run to the escalation model from its first request), +# `bytes = 0` (which can never be true), and a model written as the empty string. +# The refusal names the key and says the turn is not routed. +# +# THERE IS NO `require_primary` KEY, AND THAT IS A DECISION. io-harness's own +# Routing carries the field; it gates on Provider::reachable, a defaulted trait +# method whose body answers yes and which no provider in io-harness 0.69 +# overrides. A key for it would be accepted from this file, believed, and +# permanently inert — you would set it, expect an unattended run to refuse to +# start against a dead endpoint, and get exactly the behaviour you had before. It +# goes in when a provider answers the question. +# +# [app.io-cli.routing.escalate_after] +# failures = 3 +# model = "a-stronger-model" +# +# [app.io-cli.routing.downshift_under] +# bytes = 2000 +# model = "a-cheaper-model" diff --git a/src/app.rs b/src/app.rs index ada46be..7a2217d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2367,6 +2367,110 @@ fn gate_said( .or_else(|| (!last.detail.trim().is_empty()).then(|| last.detail.clone())) } +/// The line a turn that was answered rather than run commits, if it was one. +/// +/// **io-harness has answered questions conversationally for longer than this +/// interface has existed, and this crate has never read the field that says so.** +/// `session.rs:1125-1127` turns classification on whenever the contract carries +/// `Verification::None`, which `TaskContract::workspace` does — so a greeting has +/// always come back after one completion with **no steps row, no gate attempt, no +/// checkpoint, no snapshot and no tool loop** (`run/step.rs:312-320`). What reached +/// the operator was silence, because every line this product draws about a turn is +/// drawn from events a conversational turn does not emit. +/// +/// `None` for a run, which must stay byte-identical to what it was: an ordinary +/// turn already accounts for itself and a second sentence saying it was a run +/// would appear under every turn anybody ever takes. +/// +/// **A kind this build does not know reports as a run.** `TurnKind` is +/// `#[non_exhaustive]` (`session.rs:1434`), and the conservative arm is the one +/// that says nothing: claiming a turn was answered when it may have used tools is +/// the one error here that would mislead about what happened to somebody's files. +/// +/// **`TurnKind::Reply` alone is not "answered", and reading it alone shipped a +/// lie.** io-harness documents the second meaning in the variant itself +/// (`session.rs:1440-1443`): a turn whose one completion *crossed the token +/// ceiling and was refused rather than served* is also a `Reply`, because no run +/// was opened either way and "`outcome` is what says how it ended". So an operator +/// with `[run] max_tokens` set, whose question is refused at the budget, would have +/// been told the turn was answered — with no answer anywhere on screen. +/// +/// The guard is io-harness's own, copied deliberately rather than invented: +/// `session.rs:1202` emits `EventKind::Answered` only +/// `if kind == TurnKind::Reply && matches!(result.outcome, RunOutcome::Finished { .. })`, +/// with the comment that announcing it otherwise "would be reporting a message +/// nobody wrote". That is exactly this function's job, so it asks exactly that +/// question. +#[must_use] +pub fn answered_said( + kind: &io_harness::TurnKind, + outcome: &io_harness::RunOutcome, +) -> Option { + match (kind, outcome) { + (io_harness::TurnKind::Reply, io_harness::RunOutcome::Finished { .. }) => Some( + "answered without opening a run — one completion, no steps and no tools".to_string(), + ), + _ => None, + } +} + +/// What `/effort` changes the session's level to, or `None` to change nothing. +/// +/// The outer `Option` is the question and the inner one is the answer, which reads +/// awkwardly and is the honest shape: `Some(None)` is `/effort off` — a level was +/// set, and the level set is the absence of one — while `None` is a bare `/effort`, +/// which is a question and must leave the session as it found it. +/// +/// Here rather than in the driver because nothing under `tests/` links +/// `src/main.rs`: an assignment written there could be neither asserted nor +/// sabotaged, and this release's F1 turns on exactly the difference between a level +/// that survives the turn and one that does not. +#[must_use] +pub fn reasoning_of(said: &crate::commands::Reasoning) -> Option> { + match said { + crate::commands::Reasoning::Buy(level) => Some(Some(*level)), + crate::commands::Reasoning::Off => Some(None), + // A question and a rejected word both leave the level exactly where it is. + // They say different things, which is [`reasoning_said`]'s job, not this + // one's. + crate::commands::Reasoning::Report | crate::commands::Reasoning::Unknown(_) => None, + } +} + +/// The line `/effort` commits into the scrollback. +/// +/// `now` is the level in force **after** the command has been applied, so one +/// sentence covers setting and reporting: what an operator wants to read back is +/// the state they are now in, not the instruction they gave. +/// +/// The absent case says what it means rather than naming a level. "No reasoning +/// field" is the fact — io-harness sends the pre-0.31.0 request body — and calling +/// it "off" on screen would suggest a fourth setting between `low` and nothing. +#[must_use] +pub fn reasoning_said( + said: &crate::commands::Reasoning, + now: Option, +) -> String { + let level = match now { + Some(level) => format!("{level} reasoning"), + None => "no reasoning field, which is what this product sent before 0.26.0".to_string(), + }; + match said { + // **A rejected word says so first**, and says the level it did not change + // second. Reporting alone read as an answer: an operator on `high` who + // typed `/effort lwo` to spend less was told "every turn asks for high + // reasoning" and went on paying for it, with the typo invisible. + crate::commands::Reasoning::Unknown(word) => format!( + "`{word}` is not a reasoning level — say low, medium, high, or off. \ + Every turn still asks for {level}" + ), + crate::commands::Reasoning::Report => { + format!("every turn asks for {level}") + } + _ => format!("every turn from here asks for {level}"), + } +} + /// The one line a turn's gate commits into the scrollback, with its tone. /// /// `None` for a turn nothing gated, which is the ordinary case and not a thing to diff --git a/src/commands.rs b/src/commands.rs index febb302..c460680 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -239,6 +239,10 @@ pub const COMMANDS: &[(&str, &str)] = &[ "/profile", "switch to a named profile from the configuration, for this session", ), + ( + "/effort", + "how much reasoning the next turn buys: low, medium, high, or off", + ), ( "/contain", "run turns contained, so the agent can fan out: on, off, or ask", @@ -356,9 +360,25 @@ impl Group { /// compile error in a file nobody was editing, rather than a test that says /// which command has no home. pub const GROUPS: &[(Group, &[&str])] = &[ + // **`/profile` moved here in 0.26.0, and it is a correction rather than a way + // of making room** — the third time this sentence has been written, after + // 0.19.0's `/mcp` and `/provider` and 0.22.0's `/image` and `/copy`. `Turn` + // means a command that acts on the work the turn just finished, which is the + // argument the note above that group makes at length. `/profile` acts on + // nothing that has happened: it changes which configuration overlay every + // *later* turn is built from, which is a property of the session. It was filed + // under `Turn` because switching one feels like something you do between turns, + // and where a command sits is decided by what it acts on rather than by when it + // is typed. + // + // That this is also what makes room for `/effort` is the order the bound was + // meant to force: `src/commands.rs` pre-committed the answer for the release + // that met it — re-file what is in the wrong group, do not widen the bound — + // and a correction that pays for itself is the correction working, not a + // loophole in it. ( Group::Session, - &["/clear", "/resume", "/fork", "/setup", "/exit"], + &["/clear", "/resume", "/fork", "/profile", "/setup", "/exit"], ), // **`/image`, `/copy` and `/copy diff` moved here in 0.22.0, and it is a // correction rather than a way of making room** — the same sentence 0.19.0 @@ -379,9 +399,16 @@ pub const GROUPS: &[(Group, &[&str])] = &[ Group::Turn, &[ "/model", + // **`/effort` takes the slot `/profile` left**, and it belongs here on + // this group's own rule: it decides how much reasoning the next turn + // buys, which is a property of the turn and of nothing else. It asks + // the store nothing, which is what `Inspect` means, and it writes no + // configuration file, which is what `Configure` means — the level is + // this session's and dies with it, for the reason the release contract + // records under `open_questions`. + "/effort", "/contain", "/plan", - "/profile", "/steer", "/compact", "/image", @@ -886,6 +913,35 @@ pub fn skills( ) } +/// What `/effort` was asked to do. +/// +/// **Three words and not four, because the absent case is not a fourth level.** +/// `TaskContract::effort` is an `Option` and `None` sends no reasoning +/// field at all — the request body this product sent before 0.26.0, byte for byte. +/// So "off" is a real answer an operator can want and is not the same as "low", +/// which buys a thinking budget of 1,024 tokens on Anthropic and sets +/// `reasoning_effort` on the OpenAI wire. +/// Not `Copy` since the rejected word is carried, which is why every reader takes +/// it by reference. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Reasoning { + /// Buy this much, for this turn and every turn after it. + Buy(io_harness::Effort), + /// Send no reasoning field at all. + Off, + /// Say what is in force, and change nothing. + Report, + /// A word that is not a level, kept so it can be quoted back. + /// + /// **A fourth variant rather than falling back to [`Self::Report`], and the + /// difference is money.** Collapsing the two made `/effort lwo` — an operator + /// on `high` trying to cut what a turn costs — answer "every turn asks for high + /// reasoning", which reads as a state report rather than as a rejection: the + /// typo is invisible and the expensive level goes on being bought. The only + /// tell was a one-word difference between two sentences nobody was comparing. + Unknown(String), +} + /// What the driver should do about a slash command. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Action { @@ -1096,6 +1152,11 @@ pub enum Action { Provider, /// List the named profiles, and switch to one for this session. Profile, + /// Say how much reasoning the next turn should buy, or report what is in force. + /// + /// The word is spelled the way `/contain on` is: an argument on the existing + /// command rather than a second command, and a bare `/effort` is a question. + Effort(Reasoning), /// List the capability bundles a `[[plugin]]` entry declared: what each one /// contributed, and what each dropped bundle failed on. /// @@ -1731,6 +1792,33 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { "skills" => Action::Skills, "provider" | "providers" => Action::Provider, "profile" | "profiles" => Action::Profile, + // `Effort` carries `FromStr`, so the three levels are spelled io-harness's + // way rather than this crate's — a second spelling of `low` would be a + // second vocabulary for one wire field. "off" is handled beside it because + // the harness has no word for the absent case; it is an `Option`, not a + // fourth variant. + "effort" | "reasoning" => match input.split_whitespace().nth(1) { + // Lowercased before the match, because `Effort::FromStr` trims and + // lowercases and this arm sits in front of it: matching `off` + // literally made `/effort OFF` fall through to a parse that fails, + // land on a report, and leave the level exactly where it was — while + // `/effort HIGH` worked, since the harness lowercases for itself. + // Two spellings of the same word behaving differently is the kind of + // asymmetry nobody reports and everybody trips on once. + Some(word) if matches!(word.to_ascii_lowercase().as_str(), "off" | "none") => { + Action::Effort(Reasoning::Off) + } + Some(word) => match word.parse::() { + Ok(level) => Action::Effort(Reasoning::Buy(level)), + // An unrecognised word is refused by name and never guessed at. + // Choosing the nearest level for `/effort hgih` would spend a + // turn's reasoning budget on a typo; reporting instead — which is + // what this did — hid the typo behind a sentence that reads like + // an answer. + Err(_) => Action::Effort(Reasoning::Unknown(word.to_string())), + }, + None => Action::Effort(Reasoning::Report), + }, // `/plugins` is admitted for the same reason `/servers` and `/providers` // are: the thing being listed is plural, so the plural is what a hand // reaches for, and refusing it teaches nothing. diff --git a/src/configure.rs b/src/configure.rs index af5bd4e..6ed1470 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -178,6 +178,25 @@ pub const CATALOGUE: &[&str] = &[ "app.io-cli.gates.rubric", "app.io-cli.gates.reviewer", "app.io-cli.gates.allow_self_review", + // Whether a question that is only a question opens a run. One key rather than + // a section, and listed because its default is io-harness's rather than + // io-cli's: an operator who wants every prompt to be a run has no other way to + // discover that the answer is currently no. + "app.io-cli.conversational", + // When a run changes model, and to which. Every key of both rules for the + // reason the gate's eight are all here: a threshold without its model is half + // a rule, and an operator meeting one half on this surface would write a + // section that refuses to parse. `require_primary` is deliberately absent — + // see `crate::routing` for the provider method that never answers. + // + // **This surface is also where the containment disclosure is owed.** The rules + // reach the contract on every arm and fire only on the flat one, so an + // operator editing them with `[app.io-cli.containment]` in the same file is + // told here as well as at startup — see `routing::inert_under_containment`. + "app.io-cli.routing.escalate_after.failures", + "app.io-cli.routing.escalate_after.model", + "app.io-cli.routing.downshift_under.bytes", + "app.io-cli.routing.downshift_under.model", ]; /// The `/config` row that re-reads the price catalogue. diff --git a/src/contract.rs b/src/contract.rs index e095764..bf8230a 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -305,12 +305,77 @@ pub fn configured(text: impl Into, root: PathBuf, config: &Config) -> Ta } None => contract, }; + // **The operator's own answer, and it outranks the gate's.** The block above + // sets classification on because a criterion would otherwise turn it off; this + // is the only place an operator can say what they actually want, so it is + // applied last and wins. `None` changes nothing at all — not + // `with_conversational_turns(true)`, which would be io-cli deciding for the + // ungated majority a question io-harness already answers for them, and would + // put a field on every contract that was not there before. + let contract = match crate::settings::stored(config) + .0 + .and_then(|s| s.conversational) + { + Some(want) => contract.with_conversational_turns(want), + None => contract, + }; + // **`[app.io-cli.routing]`, in `configured` and not in `session`, because + // `io exec` is where it matters most.** An unattended run is the one that + // cannot notice a model failing its gate four times and reach for a better + // one, and the headless arm uses io-harness's flat loop — the only loop that + // consults the rules at all (`run/step.rs:1097`). + // + // A section that names no rule leaves the contract's routing unset rather than + // putting a default `Routing` on it; `routing::routing` owns that decision and + // says why at length. + // + // **A refused section leaves the turn unrouted rather than killing it**, which + // is the same answer the gate block above gives an unusable criterion: the + // honest response to a rule that cannot be obeyed is a run with no routing plus + // a notice, never a turn that will not start. `routing::notice` is what puts + // that on screen, and it is the only place this product says why a section that + // is plainly in the operator's file is not doing anything. + let contract = match crate::settings::stored(config) + .0 + .and_then(|s| s.routing) + .and_then(|routing| crate::routing::routing(&routing).ok().flatten()) + { + Some(routing) => contract.with_routing(routing), + None => contract, + }; // `[run] skills` has had its say, and `io exec` reads no other key that can // name one — so for the headless arm this is already the point after every // key. [`session`] calls this again once `[app.io-cli]` has had its own. resolve_skills(contract) } +/// Apply what `/effort` said to the contract of a turn that is about to run. +/// +/// **A function here rather than a seventh parameter of [`session`], and the +/// argument is the one `src/main.rs` already makes for `/compact`'s fold.** Three +/// of that builder's callers make a contract nothing runs — the startup reading +/// and the two reporting pages — so a parameter would be three `None`s of noise +/// and a signature break, against one call on the contract that is actually a +/// turn. +/// +/// **A function and not a line in the driver**, because nothing under `tests/` +/// links `src/main.rs`: a conditional written there could not be asserted or +/// sabotaged. The decision is here, the driver holds one call. +/// +/// **`None` must produce the contract unchanged, byte for byte.** +/// `TaskContract::effort` is an `Option` and its absence sends no +/// reasoning field at all — `openai_wire.rs:1443` and `anthropic.rs:1529` are the +/// pre-0.31.0 bodies — so an unconditional `with_effort` carrying a default would +/// change what every operator's turn asks for, silently, while passing every test +/// that only looks at the level it set. +#[must_use] +pub fn buying(contract: TaskContract, effort: Option) -> TaskContract { + match effort { + Some(effort) => contract.with_effort(effort), + None => contract, + } +} + /// The criterion this configuration resolves to, with its reviewer already built. /// /// `None` covers every case in which the run must not be gated: no section, a diff --git a/src/exec.rs b/src/exec.rs index 80ca05c..1ec5a9e 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -493,12 +493,18 @@ pub fn widening(sandbox: Option) -> Option<&'static str> { /// [`resume_main`] rather than written twice, because the sentence a run with no /// provider gets is the one thing both of them are certain to hit on a machine /// that has never run `io setup`. +/// +/// **`--provider` replaces the whole chain rather than heading it.** An operator +/// naming a provider on the command line has said which endpoint this run uses; +/// silently keeping the file's fallbacks underneath would let a run they scoped to +/// one vendor spend at another. The file's own chain is head-plus-fallbacks, which +/// is what [`provider::chain_of`] answers. pub fn spec_for( which: Option, config: &Config, model_override: Option<&str>, -) -> Result { - match (which, config.provider_spec().cloned()) { +) -> Result, String> { + match (which, config.provider_spec().is_some()) { (Some(which), _) => { let (key_var, model_var) = which.vars(); provider::spec_from( @@ -508,9 +514,10 @@ pub fn spec_for( .map(str::to_string) .or_else(|| std::env::var(model_var).ok()), ) + .map(|spec| vec![spec]) } - (None, Some(spec)) => Ok(spec), - (None, None) => Err( + (None, true) => Ok(provider::chain_of(config)), + (None, false) => Err( "no provider is configured; run `io setup`, or pass `--provider` with \ its credential in the environment" .into(), diff --git a/src/lib.rs b/src/lib.rs index 055dc12..7c16be9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -90,6 +90,7 @@ pub mod repo; pub mod resume; pub mod reviewer; pub mod rewind; +pub mod routing; pub mod servers; pub mod sessions; pub mod settings; diff --git a/src/main.rs b/src/main.rs index 1a0d405..1bb4f93 100644 --- a/src/main.rs +++ b/src/main.rs @@ -490,7 +490,7 @@ async fn drive( // dyn-compatible, so what comes back from that call is not a provider: it is // this session, run from inside the arm that built one. provider::build( - spec, + provider::chain_of(&config), model_override, Interactive { screen, @@ -960,6 +960,16 @@ async fn loop_over Result>( let mut planning = false; // One `/compact` typed at an idle prompt, spent by the next turn that starts. let mut fold_next = false; + // **How much reasoning every later turn buys, and it is not spent by being + // used.** The sibling of `fold_next` and its opposite in the one way that + // matters: a fold is a one-shot and a level is a posture, so this is read by + // each turn and cleared by nothing but `/effort off`. `None` is the absence of + // the reasoning field rather than a fourth level — see `contract::buying`. + // + // Session state and not configuration: there is no `[app.io-cli] effort` key, + // and it deliberately does not live on `Capabilities`, which is rebuilt from + // the file on every turn and would wipe the level each time. + let mut effort: Option = None; // **The file, held so it can be read again — which through 0.17.0 it never // was.** io-harness composes the instruction files inside `Config::discover` // and stores the result privately; there is no `Config::reload`, so a @@ -979,6 +989,29 @@ async fn loop_over Result>( // it can carry has already been disclosed by `drive`, which is where that // read is documented as *the read that discloses*. let (settings_in_force, _) = settings::stored(&config); + // **The one thing a routing section cannot say about itself**, said before the + // configuration is handed on so it reads from the settings actually in force + // rather than from a copy of them. io-harness consults the rules only in its + // flat loop (`run/step.rs:1097`), so a contained turn parses them, carries them + // and never fires them. + // + // Conditional on the session actually being contained, and that is F4's whole + // point: warning every operator who wrote a routing section would tell the + // majority — who have no containment at all — about a limitation that does not + // apply to them. `routing::inert_under_containment` owns the condition; this is + // its one caller. + // `contained` rather than `containment.is_some()`, which are equal here and stop + // being equal the moment the operator types `/contain off`. Keying the sentence + // on what the session is actually doing is what lets the same call answer at + // `/config` and after a `/contain` switch. + let inert_routing = settings_in_force + .as_ref() + .and_then(|stored| stored.routing.as_ref()) + .and_then(|routing| io_cli::routing::inert_under_containment(routing, contained)); + let refused_routing = settings_in_force + .as_ref() + .and_then(|stored| stored.routing.as_ref()) + .and_then(io_cli::routing::notice); let mut configuration = io_cli::reload::Configuration::new( session.root().to_path_buf(), config.clone(), @@ -988,6 +1021,24 @@ async fn loop_over Result>( let notice = settings::contained_notice(caps, app.theme.glyphs.dash); app.say(Tone::Muted, notice); } + // Beside the containment notice, because it is a qualification of it — and + // **`record` and not `say`**, which is the difference between a sentence that + // stays and one nobody reads. `App::say` writes `status.notice`, a single slot + // that `App::key` clears on the first keystroke, so a `say` here would first + // overwrite the containment notice immediately above (the two conditions are + // true in exactly the same case, since the disclosure only fires when + // contained) and would then be wiped by the operator's first character. This + // file already states that rule where the migration report is committed, and + // the `/config` and `/contain on` sites both got it right. + if let Some(notice) = inert_routing { + app.record(Tone::Warning, notice); + } + // And a section that cannot be obeyed at all, said once at the start for the + // reason every other startup notice is: an operator meets this one without + // having asked a question. + if let Some(why) = refused_routing { + app.record(Tone::Refused, why); + } // **The session no longer keeps a clock, because nothing shows one.** The // clock on screen belongs to the turn — it starts at zero when one starts and // stops where it stopped — so the reading a session-long `Instant` gave was @@ -1242,6 +1293,7 @@ async fn loop_over Result>( .flatten(), &capabilities, &seen, + effort, run_id, pending, ) @@ -3016,6 +3068,38 @@ async fn loop_over Result>( } } Action::Config(None) => { + // **What the routing rules say, and where they will not fire**, + // said on the surface that edits them. `/config` is where an + // operator meets `[app.io-cli.routing]`, and a section they can + // change here without being told it is inert for their session + // is the defect this release exists partly to avoid. Read from + // the configuration in force rather than the startup snapshot, + // so an edit made this session is what is described. + if let Some(routing) = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.routing) + { + if let Some(said) = io_cli::routing::describe(&routing) { + app.record(Tone::Muted, said); + } + // **Why the section is not routing anything, if it is + // not.** The pair of `contract::gate_notice`: this + // surface lists the four routing keys, so a section + // that is plainly in the operator's file and doing + // nothing has to say why here or nowhere. + if let Some(why) = io_cli::routing::notice(&routing) { + app.record(Tone::Refused, why); + } + // `contained` and not `containment.is_some()`: `/contain off` + // puts the session back on the flat loop, where the rules do + // fire, so a warning keyed on the configuration would go on + // being shown to an operator it no longer applies to. + if let Some(notice) = + io_cli::routing::inert_under_containment(&routing, contained) + { + app.record(Tone::Warning, notice); + } + } let settings = io_cli::configure::settings(&config); let mut paths: Vec = settings.iter().map(|s| s.path.clone()).collect(); let mut rows = io_cli::configure::rows(&settings); @@ -3290,6 +3374,29 @@ async fn loop_over Result>( // the completion above and the turn below build it — so what may // be attached and what the agent may read are the same set by // construction rather than by two agreeing lists. + // **A posture and not a one-shot**, which is the whole of what the + // three words mean here: a level set now is what every later turn + // buys until `/effort off`. The sentence is built by + // `io_cli::app::reasoning_said` so it can be asserted; this arm + // holds the assignment and nothing else. + Action::Effort(said) => { + if let Some(level) = io_cli::app::reasoning_of(&said) { + effort = level; + // The status line carries the same value, not a second + // opinion about it. Set beside the assignment so the two + // cannot drift — the 0.25.0 defect where one wave gave + // `App` a branch field and another gave `Status` one, and + // the line drew the half nothing wrote. + app.status.effort = level; + } + // A word that is not a level is refused rather than + // reported, so the tone is the refusal's. + let tone = match said { + io_cli::commands::Reasoning::Unknown(_) => Tone::Refused, + _ => Tone::Muted, + }; + app.record(tone, io_cli::app::reasoning_said(&said, effort)); + } Action::Contain(want) => match (&containment, want) { // Nothing to switch. Said as the configuration gap it is, // with the key that closes it, rather than as a refusal — @@ -3321,6 +3428,22 @@ async fn loop_over Result>( contained = true; let notice = settings::contained_notice(caps, app.theme.glyphs.dash); app.record(Tone::Muted, notice); + // **Turning containment on is entering the state the routing + // disclosure warns about**, so it is said here as well as at + // start. An operator who begins uncontained, reads nothing + // about routing because nothing applied, and then types + // `/contain on` has silently moved into the one mode where + // their rules do not fire. + if let Some(said) = io_cli::settings::stored(&config) + .0 + .and_then(|stored| stored.routing) + .as_ref() + .and_then(|routing| { + io_cli::routing::inert_under_containment(routing, contained) + }) + { + app.record(Tone::Warning, said); + } } (Some(_), Some(false)) => { contained = false; @@ -3785,6 +3908,13 @@ async fn loop_over Result>( // Taken rather than read: one request, one turn. A queue // of three prompts must not fold three times. std::mem::take(&mut fold_next), + // **Read rather than taken, which is the whole difference + // between a posture and a one-shot.** `/compact` folds the + // turn it was typed before and nothing after it; `/effort` + // says how hard to think until it is told otherwise, so a + // `mem::take` here would make every level last exactly one + // turn — the defect F1's sabotage names. + effort, text, // **This turn's own clock, not the session's.** What a // reader wants of the row above the prompt is how long the @@ -4091,6 +4221,11 @@ async fn turn( // with `mem::take`, so a contract reused for every turn would not fold every // turn; io-cli builds a fresh one anyway. fold: bool, + // How much reasoning this turn buys — what `/effort` last said, or `None` for a + // session that has never said it. `None` is the absence of the field rather + // than a fourth level, so a session that never types `/effort` sends the + // request body this crate sent before 0.26.0. + effort: Option, text: String, started: Instant, ) -> Result { @@ -4202,6 +4337,10 @@ async fn turn( } else { contract }; + // **What `/effort` last said, applied here for the reason above.** The decision + // is `contract::buying`'s, not this file's — nothing under `tests/` links this + // binary, so a conditional written here could not be asserted or sabotaged. + let contract = io_cli::contract::buying(contract, effort); // Set while a fold has been asked for and no `Compacted` event has arrived. // A one-shot: io-harness spends the request whether or not it folds, so what // this guards is the report and never a retry. @@ -4811,6 +4950,17 @@ async fn turn( ), ); } + // **A turn that was answered rather than run, said out loud at last.** + // io-harness has classified these since before this interface existed + // and io-cli has never read `TurnResult::kind`, so the commonest turn + // there is — a question that is only a question — arrived as silence: + // every line this product draws about a turn comes from events a + // conversational turn does not emit. The sentence is + // `app::answered_said`'s, which also owns the decision that an unknown + // kind reports as a run. + if let Some(said) = io_cli::app::answered_said(&result.kind, &result.outcome) { + app.record(Tone::Muted, said); + } } } app.status.elapsed = started.elapsed(); @@ -6119,6 +6269,10 @@ async fn resume_pending( // through a new door. `tests/context_share.rs` counts the two against each // other so this cannot be forgotten on a path added later. seen: &io_cli::context::Seen, + // What `/effort` last said. A resumed run is a turn like any other and buys the + // same reasoning — see `contract::buying` at the bottom of this function for why + // this parameter exists at all. + effort: Option, run_id: i64, pending: io_cli::resume::Pending, ) -> Result<(), String> { @@ -6206,6 +6360,15 @@ async fn resume_pending( // planning phase back on for the continuation. None, ); + // **The resumed run buys the same reasoning the session is buying**, and this + // is the second of the two sites that run a turn rather than read one. + // `contract::buying`'s own note counts three callers that build a contract + // nothing runs — the startup reading and the two reporting pages — and it + // undercounted: this is a fifth `contract::session` call site and it drives + // real completions. Without it, `/effort high` applied to every turn except + // the half of the work an operator came back to `/resume` and finish, while + // the status line went on saying `effort high`. + let continuing = io_cli::contract::buying(continuing, effort); let (observer, mut events) = bridge::channel(); let canceller = observer.canceller(); let (approver, mut asks) = approval::channel(); diff --git a/src/provider.rs b/src/provider.rs index 2fe76d4..41acc92 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -68,30 +68,392 @@ pub fn model_of(spec: &ProviderSpec) -> &str { } } -pub async fn build( - spec: ProviderSpec, - model_override: Option, - with: W, -) -> Result { - let model = |configured: String| model_override.unwrap_or(configured); +/// The whole chain a configuration names, head first. +/// +/// `provider_spec()` is the head and `fallback_specs()` is the tail — io-harness's +/// own split, and the same one [`crate::providers::chain`] draws in the panel. The +/// panel has shown this list since 0.21.0 while [`build`] ran only its head; asking +/// for it in one place is what keeps the chain that runs and the chain that is drawn +/// from being two different answers. +pub fn chain_of(config: &io_harness::Config) -> Vec { + config + .provider_spec() + .into_iter() + .chain(config.fallback_specs()) + .cloned() + .collect() +} + +/// One provider, whichever vendor built it. +/// +/// **A chain of arbitrary length needs one concrete type, and this is it.** +/// `Provider::complete` returns `impl Future` (RPITIT), so the trait is not +/// dyn-compatible and there is no `Box` to put in a `Vec` — +/// `io_harness::provider::Fallback`'s own module says as much. An enum is the +/// remaining shape: four arms, one type, and `Fallback` can nest over it. +/// +/// **It wraps and never constructs.** Every value inside one of these arms was +/// built in [`build`], which is still the only site in this crate that names a +/// vendor constructor — the rule `tests/provider.rs` enforces and which this +/// release does not widen. +/// +/// Every method delegates across all four arms, for the reason [`Watched`] gives +/// at length: the trait has eight defaulted methods, and a default that fired here +/// would change behaviour silently rather than fail. The async ones are written as +/// `async fn` rather than by returning the inner future, because four arms return +/// four distinct future types and only an `async` body can unify them. +pub enum Vendor { + OpenRouter(OpenRouter), + Anthropic(Anthropic), + OpenAi(OpenAi), + Compatible(Compatible), +} + +impl Provider for Vendor { + async fn complete(&self, request: CompletionRequest) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.complete(request).await, + Self::Anthropic(p) => p.complete(request).await, + Self::OpenAi(p) => p.complete(request).await, + Self::Compatible(p) => p.complete(request).await, + } + } + + async fn complete_streaming( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + ) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.complete_streaming(request, on_token).await, + Self::Anthropic(p) => p.complete_streaming(request, on_token).await, + Self::OpenAi(p) => p.complete_streaming(request, on_token).await, + Self::Compatible(p) => p.complete_streaming(request, on_token).await, + } + } + + async fn complete_streaming_calls( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + on_call: &(dyn Fn(usize, &ToolCall) + Send + Sync), + ) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.complete_streaming_calls(request, on_token, on_call).await, + Self::Anthropic(p) => p.complete_streaming_calls(request, on_token, on_call).await, + Self::OpenAi(p) => p.complete_streaming_calls(request, on_token, on_call).await, + Self::Compatible(p) => p.complete_streaming_calls(request, on_token, on_call).await, + } + } + + async fn models(&self) -> io_harness::Result> { + match self { + Self::OpenRouter(p) => p.models().await, + Self::Anthropic(p) => p.models().await, + Self::OpenAi(p) => p.models().await, + Self::Compatible(p) => p.models().await, + } + } + + async fn reachable(&self) -> io_harness::Result { + match self { + Self::OpenRouter(p) => p.reachable().await, + Self::Anthropic(p) => p.reachable().await, + Self::OpenAi(p) => p.reachable().await, + Self::Compatible(p) => p.reachable().await, + } + } + + fn model_hint(&self) -> Option<&str> { + match self { + Self::OpenRouter(p) => p.model_hint(), + Self::Anthropic(p) => p.model_hint(), + Self::OpenAi(p) => p.model_hint(), + Self::Compatible(p) => p.model_hint(), + } + } + + fn name(&self) -> &str { + match self { + Self::OpenRouter(p) => p.name(), + Self::Anthropic(p) => p.name(), + Self::OpenAi(p) => p.name(), + Self::Compatible(p) => p.name(), + } + } + + fn prompt_family(&self) -> PromptFamily { + match self { + Self::OpenRouter(p) => p.prompt_family(), + Self::Anthropic(p) => p.prompt_family(), + Self::OpenAi(p) => p.prompt_family(), + Self::Compatible(p) => p.prompt_family(), + } + } + + fn accepts_images(&self) -> bool { + match self { + Self::OpenRouter(p) => p.accepts_images(), + Self::Anthropic(p) => p.accepts_images(), + Self::OpenAi(p) => p.accepts_images(), + Self::Compatible(p) => p.accepts_images(), + } + } + + fn endpoint(&self) -> Option<&str> { + match self { + Self::OpenRouter(p) => p.endpoint(), + Self::Anthropic(p) => p.endpoint(), + Self::OpenAi(p) => p.endpoint(), + Self::Compatible(p) => p.endpoint(), + } + } + + fn endpoints(&self) -> Vec<&str> { + match self { + Self::OpenRouter(p) => p.endpoints(), + Self::Anthropic(p) => p.endpoints(), + Self::OpenAi(p) => p.endpoints(), + Self::Compatible(p) => p.endpoints(), + } + } + + fn last_served(&self) -> Option { + match self { + Self::OpenRouter(p) => p.last_served(), + Self::Anthropic(p) => p.last_served(), + Self::OpenAi(p) => p.last_served(), + Self::Compatible(p) => p.last_served(), + } + } +} + +/// A chain of providers, each answering when the one before it fails. +/// +/// **The decision to fall over is io-harness's**, not this crate's: +/// [`io_harness::ProviderErrorKind::is_retryable`] is the same predicate its own +/// `Fallback` and its own in-run retry both ask, and the reasoning is its own — a +/// failure about the request or about the caller's configuration will happen +/// identically at the next vendor, so falling over on one fails twice and spends +/// twice. io-cli holds no opinion here and must not grow one. +/// +/// **Flat rather than nested, and that is not a preference.** The obvious shape is +/// `Fallback>`, folded recursively so the tail is +/// itself one type. It does not compile: `Provider::complete` returns `impl Future` +/// (RPITIT), so a `Chain` whose secondary is a `Chain` makes the future's auto-trait +/// inference depend on itself, and rustc reports a type cycle that `Box::pin` on the +/// recursive arm does not break. A list and a loop have neither problem. +/// +/// **`last_served` answers `None` when the head served, and that is a deliberate +/// difference from `Fallback`.** io-harness emits `EventKind::FellBackTo` for any +/// `Some` (`run/step.rs:503`), while `Fallback::last_served` answers `Some` for its +/// *primary* too — so a chain built from that type would report "the provider fell +/// over" on every step of every run, including the ones where nothing did. Reporting +/// only a link that is not the head makes the event mean what its name says, and +/// makes a chain whose head is answering indistinguishable from no chain at all, +/// which is what an operator would expect. +/// +/// Generic over the link so the fall-through is assertable. The chain a +/// configuration builds is `Chain`; a test builds `Chain` over links +/// that fail on demand with a chosen `ProviderErrorKind`, which is the only way to +/// prove that a retryable failure falls through and an `Auth` failure does not +/// without a network and without spending anything. +pub struct Chain

{ + links: Vec

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

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

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

{ + async fn complete(&self, request: CompletionRequest) -> io_harness::Result { + let mut last = None; + for (index, link) in self.links.iter().enumerate() { + match link.complete(request.clone()).await { + Ok(response) => { + self.note(index); + return Ok(response); + } + Err(error) if Self::worth_another(&error) => last = Some(error), + // Not worth another vendor: a bad credential or a malformed + // request fails the same way everywhere, and trying the next link + // would spend somebody else's budget to be told so again. + Err(error) => return Err(error), + } + } + Err(last.expect("a chain has at least one link, so the loop ran at least once")) + } + + async fn complete_streaming( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + ) -> io_harness::Result { + let mut last = None; + for (index, link) in self.links.iter().enumerate() { + match link.complete_streaming(request.clone(), on_token).await { + Ok(response) => { + self.note(index); + return Ok(response); + } + Err(error) if Self::worth_another(&error) => last = Some(error), + Err(error) => return Err(error), + } + } + Err(last.expect("a chain has at least one link, so the loop ran at least once")) + } + + async fn complete_streaming_calls( + &self, + request: CompletionRequest, + on_token: &(dyn Fn(&str) + Send + Sync), + on_call: &(dyn Fn(usize, &ToolCall) + Send + Sync), + ) -> io_harness::Result { + let mut last = None; + for (index, link) in self.links.iter().enumerate() { + match link + .complete_streaming_calls(request.clone(), on_token, on_call) + .await + { + Ok(response) => { + self.note(index); + return Ok(response); + } + Err(error) if Self::worth_another(&error) => last = Some(error), + Err(error) => return Err(error), + } + } + Err(last.expect("a chain has at least one link, so the loop ran at least once")) + } + + async fn models(&self) -> io_harness::Result> { + self.head().models().await + } + + async fn reachable(&self) -> io_harness::Result { + self.head().reachable().await + } + + fn model_hint(&self) -> Option<&str> { + self.head().model_hint() + } + + fn name(&self) -> &str { + self.head().name() + } + + fn prompt_family(&self) -> PromptFamily { + self.head().prompt_family() + } + + /// Every link, or none of them — io-harness's own rule for the same question. + /// + /// Reporting the head's answer would let an image reach a link that cannot read + /// it on the one call that matters, the fall-through. The link would refuse it + /// anyway, so the conjunction only changes *when* the operator finds out: before + /// the run rather than midway through a failure. + fn accepts_images(&self) -> bool { + self.links.iter().all(Provider::accepts_images) + } + + fn endpoint(&self) -> Option<&str> { + self.head().endpoint() + } + + /// **Every link's hosts, and this is the reason `endpoints` exists at all.** + /// io-harness's egress policy is deny-by-default and a run authorizes its + /// provider's hosts before its first step, so a chain that reported only the + /// head's host would make a fall-through a way to reach a host the policy never + /// saw. + fn endpoints(&self) -> Vec<&str> { + self.links.iter().flat_map(Provider::endpoints).collect() + } + + /// The link that answered, unless it was the head. + /// + /// See the type's own documentation: io-harness emits `EventKind::FellBackTo` + /// for any `Some`, so answering `Some` for the head would report a fall-through + /// on every step of every run that configured a chain. + fn last_served(&self) -> Option { + match self.served.load(std::sync::atomic::Ordering::Relaxed) { + UNSERVED | 0 => None, + index => self.links.get(index).map(|link| { + link.last_served() + .unwrap_or_else(|| link.name().to_string()) + }), + } + } +} + +/// One link's maker: a closure that builds this vendor for a named model. +/// +/// Boxed because the four arms produce four closure types and the chain holds a +/// list of them. The credential is captured here and nowhere else, which is what +/// keeps it out of every struct in this crate. +type Maker = Box Result + Send + Sync>; + +/// The maker for one spec, and the model that spec names. +fn maker_for(spec: ProviderSpec) -> Result<(Maker, String), String> { match spec { - ProviderSpec::OpenRouter { model: m, api_key } => { + ProviderSpec::OpenRouter { model, api_key } => { let key = key_for(api_key, "OPENROUTER_API_KEY")?; - let make = move |name: &str| Ok(OpenRouter::new(key.clone(), name)); - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| { + Ok(Vendor::OpenRouter(OpenRouter::new(key.clone(), name))) + }), + model, + )) } - ProviderSpec::Anthropic { model: m, api_key } => { + ProviderSpec::Anthropic { model, api_key } => { let key = key_for(api_key, "ANTHROPIC_API_KEY")?; - let make = move |name: &str| Ok(Anthropic::new(key.clone(), name)); - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| { + Ok(Vendor::Anthropic(Anthropic::new(key.clone(), name))) + }), + model, + )) } - ProviderSpec::OpenAi { model: m, api_key } => { + ProviderSpec::OpenAi { model, api_key } => { let key = key_for(api_key, "OPENAI_API_KEY")?; - let make = move |name: &str| Ok(OpenAi::new(key.clone(), name)); - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| Ok(Vendor::OpenAi(OpenAi::new(key.clone(), name)))), + model, + )) } ProviderSpec::Compatible { - model: m, + model, preset, base_url, api_key, @@ -103,16 +465,25 @@ pub async fn build( return Err("this provider names neither a preset nor a base URL".into()); } let auth = auth.unwrap_or(Auth::Bearer); - let make = move |name: &str| match (&preset, &base_url) { - (Some(preset), _) => { - Compatible::preset(preset, key.clone(), name).map_err(|error| error.to_string()) - } - (None, Some(base)) => Ok(Compatible::new(base.clone(), auth, key.clone(), name)), - // Refused above, before anything was built, so this arm exists - // only to make the match total. - (None, None) => Err("this provider names neither a preset nor a base URL".into()), - }; - Ok(with.call(make, model(m)).await) + Ok(( + Box::new(move |name: &str| match (&preset, &base_url) { + (Some(preset), _) => Compatible::preset(preset, key.clone(), name) + .map(Vendor::Compatible) + .map_err(|error| error.to_string()), + (None, Some(base)) => Ok(Vendor::Compatible(Compatible::new( + base.clone(), + auth, + key.clone(), + name, + ))), + // Refused above, before anything was built, so this arm exists + // only to make the match total. + (None, None) => { + Err("this provider names neither a preset nor a base URL".into()) + } + }), + model, + )) } // `ProviderSpec` is `#[non_exhaustive]`: a provider the harness gains and // this release has not seen is refused by name rather than driven wrongly. @@ -122,6 +493,53 @@ pub async fn build( } } +/// Build the chain a configuration names and hand it to `with`. +/// +/// `specs` is head first — [`chain_of`] is how a caller gets one. A single-element +/// list builds exactly one provider and no `Fallback` at all, which is what keeps +/// an operator who has configured one provider on precisely the code path they +/// were on before this release. +/// +/// `model_override` is `-m/--model`: it replaces the model the configuration names +/// for the **head only**, which is why it is applied to the extracted model rather +/// than by rewriting the spec. A fallback link keeps the model its own entry names — +/// naming one model for a whole chain would ask a second vendor for a model id only +/// the first one serves. +pub async fn build( + specs: Vec, + model_override: Option, + with: W, +) -> Result { + // **Iterators and not a `for`, which is a rule rather than a taste.** + // `tests/dependencies.rs` refuses a loop in any file that calls a provider, + // because a loop beside a provider call is the shape of a second agent loop. + // The one loop this module is permitted is the chain's own over its links, and + // that gate now names it by path and holds it to iterating nothing else — so + // turning configuration into closures, which is what this does, has to be + // written without one. + let built: Result, String> = specs.into_iter().map(maker_for).collect(); + let (makers, mut models): (Vec, Vec) = built?.into_iter().unzip(); + let Some(head) = models.first().cloned() else { + return Err("no provider is configured; run `io setup`".into()); + }; + let head = model_override.unwrap_or(head); + // The tail's models are fixed at build time; only the head's follows the + // maker's argument, so a `/model` switch changes what is asked first and + // leaves every fallback answering as its own entry says it should. + let tail: Vec = models.split_off(1); + let make = move |name: &str| { + let head = std::iter::once(makers[0](name)?); + let rest: Vec = makers[1..] + .iter() + .zip(tail.iter()) + .map(|(maker, model)| maker(model)) + .collect::>()?; + Chain::of(head.chain(rest).collect()) + .ok_or_else(|| "no provider is configured; run `io setup`".to_string()) + }; + Ok(with.call(make, head).await) +} + /// A provider that keeps a copy of every request on the way past. /// /// **The only way io-cli can say what is in the context window.** io-harness diff --git a/src/routing.rs b/src/routing.rs new file mode 100644 index 0000000..82f50b5 --- /dev/null +++ b/src/routing.rs @@ -0,0 +1,432 @@ +//! Which model a run asks, once the run has told you something about itself. +//! +//! io-harness has carried the decision since 0.55.0: a `TaskContract` holds one +//! [`io_harness::Routing`], the flat run loop asks it after every step, and the +//! answer replaces the model on the next request. Until this release io-cli +//! supplied none, so every contract it built carried no routing and every run +//! asked one model from the first token to the last. +//! +//! This module is where the operator's answer lives, and it is deliberately thin: +//! [`Settings`] is the `[app.io-cli.routing]` section as written, [`routing`] +//! turns it into the dependency's own value, and the other two functions are what +//! a surface says about it. **No rule is evaluated here.** io-harness owns +//! `Routing::model_for` (`contract.rs:1811`), it is the only code that knows the +//! consecutive-failure count and the byte total, and a second implementation in +//! this crate would be a second answer that drifts from the one the run uses. +//! +//! # Why this section is not deserialized straight into the harness's type +//! +//! `[app.io-cli.containment]` is `Option` in +//! [`crate::settings::CliSettings`] because that type is `Serialize` and +//! `Deserialize` for exactly that purpose. [`io_harness::Routing`] is not: it +//! derives `Debug, Clone, Default, PartialEq, Eq` and nothing else +//! (`contract.rs:1745`), and it is `#[non_exhaustive]`, so this crate can neither +//! deserialize into it nor write a struct literal for it. Both halves of that +//! matter — a `serde` derive added upstream later would still leave the literal +//! refused, because `#[non_exhaustive]` is a promise about fields yet to exist. +//! +//! So io-cli defines its own `Deserialize` shapes below and builds through the +//! three builder methods, which are the supported way in and which keep working +//! when the harness grows a fourth field. +//! +//! # What an operator writes +//! +//! ```toml +//! [app.io-cli.routing.escalate_after] +//! failures = 3 +//! model = "a-stronger-model" +//! +//! [app.io-cli.routing.downshift_under] +//! bytes = 2000 +//! model = "a-cheaper-model" +//! ``` +//! +//! Each rule is a sub-table rather than a pair of flat keys +//! (`escalate_after_failures`, `escalate_after_model`) because a rule is two +//! numbers that only mean anything together: a threshold with no model and a +//! model with no threshold are both half a rule, and a sub-table makes the pair +//! the unit TOML itself enforces. +//! +//! **Both fields are nonetheless optional, and a half rule is refused rather than +//! rejected.** Making them required looked like the stricter choice and was the +//! more damaging one: a required field is a *deserialization* failure, so a +//! threshold with no model did not fail the rule, it failed `CliSettings` — and +//! `crate::settings::stored` then answered `None` for the whole `[app.io-cli]` +//! section, taking the theme, the keys, the ceilings, the capabilities and the +//! **verification gate** with it, silently. [`routing`] refuses the half rule by +//! name instead, which is the shape [`crate::gates::Settings::criterion`] already +//! uses, and [`notice`] is what says so on screen. +//! +//! The same function refuses the three values that are writable and disastrous: +//! `failures = 0`, which io-harness reads as "escalate before anything has +//! failed" and which therefore pins every run to the escalation model from its +//! first request; `bytes = 0`, which can never be true; and a model named as the +//! empty string, which would send every request of the run with no model id. +//! +//! # `require_primary` is not offered, and that is a decision +//! +//! [`io_harness::Routing`] has a third field: `require_primary`, which refuses to +//! start when the primary provider says it is unreachable. It is not exposed +//! here. It gates on `Provider::reachable`, a **defaulted** trait method whose +//! body is `async { Ok(true) }` (`provider/mod.rs:1745`), and **no provider in +//! io-harness 0.69 overrides it** — the only other mention of the name in that +//! crate is the doc example demonstrating how one *could*. A key for it would +//! therefore be advertised on a surface, accepted from a file, and permanently +//! inert: an operator would set it, believe an unattended overnight job now +//! refuses to start against a dead endpoint, and get exactly the behaviour they +//! had before. That is worse than the setting not existing. It goes in when a +//! provider answers the question. +//! +//! # Routing does not reach a contained turn +//! +//! `apply_routing` has exactly one call site in io-harness: `run/step.rs:1097`, +//! inside the flat workspace loop. The contained loop takes each agent's model +//! from that agent's own `AgentDef` (`run/tree.rs:638`), and the root is entered +//! with its identity passed as `None` (`run.rs:3959`), which every provider reads +//! as "the model you were built with". So for an operator who has configured +//! `[app.io-cli.containment]`, a routing section parses, reaches the contract, +//! and never fires. +//! +//! Nothing in this crate can fix that — the loop that would have to consult the +//! rules is the dependency's — so what this module owes the operator is the +//! disclosure, and [`inert_under_containment`] is it. It is deliberately narrow: +//! see that function for why warning every operator with a routing section would +//! be worse than warning none. + +use serde::{Deserialize, Serialize}; + +/// The `[app.io-cli.routing]` section exactly as an operator wrote it. +/// +/// Two optional rules and nothing else. There is no `enabled` key and no +/// `require_primary` key: a section that names a rule is a section that wants it, +/// and the third field of [`io_harness::Routing`] is inert in io-harness 0.69 for +/// the reasons in the module documentation. +/// +/// Not `deny_unknown_fields`, unlike [`crate::gates::Settings`], and the +/// asymmetry is worth stating. That section's failure mode is a typo landing on +/// the default and silently producing no gate at all, which is why it refuses +/// names it does not know. Here the two names are the *tables*, and a mistyped +/// table is a table nobody reads — the section then names no rule, [`routing`] +/// answers `None`, and the run behaves exactly as it did before the operator +/// touched the file. Wrong, but not a run that reports success on work nobody +/// checked. Inside each table a typo is likewise a key nobody reads, and the half +/// rule it leaves behind is refused by [`routing`] by name rather than by serde — +/// see the module documentation for why the stricter-looking choice, making those +/// keys required, was the more damaging one. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Settings { + /// Move up to a stronger model after this many consecutive failed gates. + #[serde(default)] + pub escalate_after: Option, + /// Stay on a cheaper model while the run has written little. + #[serde(default)] + pub downshift_under: Option, +} + +/// The upward rule: `[app.io-cli.routing.escalate_after]`. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Escalation { + /// How many *consecutive* failed gate attempts trigger the change. + /// + /// Consecutive rather than cumulative, and that is io-harness's counting + /// rather than a choice made here (`contract.rs:1748`): a run that fails, + /// recovers, and fails again much later is a run doing hard work, not a run + /// that needs a bigger model. + /// + /// **Optional, and that is a correction rather than a loosening.** It was + /// required, and a required field is a *deserialization* failure — so half a + /// rule did not fail the rule, it failed `CliSettings` entirely, and + /// `crate::settings::stored` then answered `None` for the whole `[app.io-cli]` + /// section. An operator who wrote a threshold and forgot the model silently + /// lost their theme, their keys, their ceilings, their capabilities and — worst + /// of all — their **verification gate**, because `contract::criterion_for` + /// gives up on the same `None`. A gate that stops gating without saying so is + /// the most expensive failure this crate has. Half a rule now parses and is + /// refused by name at [`routing`], which is the shape + /// [`crate::gates::Settings::criterion`] already uses. + #[serde(default)] + pub failures: Option, + /// The model asked from then on. + #[serde(default)] + pub model: Option, +} + +/// The downward rule: `[app.io-cli.routing.downshift_under]`. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Downshift { + /// The byte total below which the cheaper model is asked. + /// + /// Measured on what the run has already written to disk, not on what it + /// planned to write — again io-harness's definition (`contract.rs:1759`). + /// + /// Optional for the reason [`Escalation::failures`] gives at length. + #[serde(default)] + pub bytes: Option, + /// The model asked while the run is under that total. + #[serde(default)] + pub model: Option, +} + +/// A routing section that cannot be obeyed, and why. +/// +/// The sibling of [`crate::gates::Refusal`] and it exists for the same reason: a +/// mistake TOML itself cannot express has to be caught where the operator can be +/// told about it, rather than reaching io-harness and changing every request of +/// every run. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Refusal { + /// A threshold with no model, or a model with no threshold. + HalfARule { + /// `escalate_after` or `downshift_under`. + rule: &'static str, + /// The key that is missing. + missing: &'static str, + }, + /// `failures = 0`, which escalates before anything has failed. + /// + /// io-harness compares `consecutive_gate_failures >= failures` + /// (`contract.rs:1813`), so zero is true at the first request of every run — + /// the stronger model is used unconditionally, from the start, and + /// `downshift_under` is never reached because escalation is checked first. An + /// operator writing it means "escalate readily" and gets "never use the model + /// I configured". + EscalatesBeforeAnythingFailed, + /// `bytes = 0`, which can never be true. + /// + /// `written < 0` is false for every run, so the rule is inert. Unlike the + /// above it costs nothing, and it is still refused rather than ignored: a rule + /// that silently does nothing is what the operator will not find when they + /// wonder why the cheap model never appears. + NeverDownshifts, + /// A model named as the empty string. + /// + /// `apply_routing` sets `request.model = Some("")` and every request of the run + /// goes to the vendor with an empty model id. + NoModel { + /// Which rule named it. + rule: &'static str, + }, +} + +impl std::fmt::Display for Refusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::HalfARule { rule, missing } => write!( + f, + "[app.io-cli.routing.{rule}] names no `{missing}`, and a threshold \ + without a model is half a rule — this turn is not routed" + ), + Self::EscalatesBeforeAnythingFailed => write!( + f, + "[app.io-cli.routing.escalate_after] has `failures = 0`, which is \ + true before anything has failed — every run would start on the \ + escalation model and never reach the downshift. This turn is not \ + routed." + ), + Self::NeverDownshifts => write!( + f, + "[app.io-cli.routing.downshift_under] has `bytes = 0`, and a run \ + that has written fewer than zero bytes does not exist — the rule \ + could never fire. This turn is not routed." + ), + Self::NoModel { rule } => write!( + f, + "[app.io-cli.routing.{rule}] names an empty model, which would send \ + every request of the run with no model id — this turn is not routed" + ), + } + } +} + +/// The section as the dependency's own type, ready for the contract, or `None`. +/// +/// **`None` for a section that names neither rule, including a section that is +/// present and empty, and that is not tidiness.** `io_harness::Routing::default()` +/// is a perfectly constructible value whose `model_for` always answers `None`, so +/// putting one on the contract looks harmless. It is not the same thing as +/// leaving the contract's routing unset: it is a value where there was absence, +/// it is what the run records and what a later reader of that run sees, and it +/// says the operator asked for routing when they asked for nothing. An operator +/// who writes `[app.io-cli.routing]` and no rule under it gets the contract they +/// would have had without the line. +/// +/// Built through the builders rather than by literal because +/// [`io_harness::Routing`] is `#[non_exhaustive]` — see the module +/// documentation. The builders are `#[must_use]` and consuming, so the chain +/// below is the whole construction. +/// **The two thresholds are checked rather than trusted**, because io-harness +/// obeys them literally and each has a value that is both writable and disastrous. +/// `failures = 0` satisfies `consecutive_gate_failures >= 0` at the first request +/// of every run, so the escalation model is used unconditionally and the downshift +/// — checked second — never runs at all. `bytes = 0` can never be true. An empty +/// model sends every request with no model id. None of the three is a shape TOML +/// can refuse, and all three are one keystroke from a plausible file. +pub fn routing(settings: &Settings) -> Result, Refusal> { + let mut routing = io_harness::Routing::new(); + let mut asked = false; + if let Some(escalation) = &settings.escalate_after { + let failures = escalation.failures.ok_or(Refusal::HalfARule { + rule: "escalate_after", + missing: "failures", + })?; + let model = escalation.model.as_deref().ok_or(Refusal::HalfARule { + rule: "escalate_after", + missing: "model", + })?; + if failures == 0 { + return Err(Refusal::EscalatesBeforeAnythingFailed); + } + if model.trim().is_empty() { + return Err(Refusal::NoModel { + rule: "escalate_after", + }); + } + routing = routing.escalate_after(failures, model.to_string()); + asked = true; + } + if let Some(downshift) = &settings.downshift_under { + let bytes = downshift.bytes.ok_or(Refusal::HalfARule { + rule: "downshift_under", + missing: "bytes", + })?; + let model = downshift.model.as_deref().ok_or(Refusal::HalfARule { + rule: "downshift_under", + missing: "model", + })?; + if bytes == 0 { + return Err(Refusal::NeverDownshifts); + } + if model.trim().is_empty() { + return Err(Refusal::NoModel { + rule: "downshift_under", + }); + } + routing = routing.downshift_under(bytes, model.to_string()); + asked = true; + } + Ok(asked.then_some(routing)) +} + +/// Why the configured routing is not routing this run, if it is not. +/// +/// The sibling of [`crate::contract::gate_notice`], and it exists for the reason +/// that one does: a refusal is not a `Routing`, so the function that answers with a +/// value cannot also be the one that explains itself. +#[must_use] +pub fn notice(settings: &Settings) -> Option { + routing(settings).err().map(|refusal| refusal.to_string()) +} + +/// What the rules do, in one sentence, for `/config` and the startup notices. +/// +/// `None` when no rule is named, so a caller writes nothing rather than a line +/// saying nothing happens. +/// +/// Escalation is stated first because it is the rule that overrides the other, +/// and the two sentences an operator needs after that are both io-harness's +/// documented behaviour rather than io-cli's: escalation wins over downshifting +/// when both conditions hold (`contract.rs:1808`, and the ordering of the two +/// checks in `model_for` at `contract.rs:1812`), and it happens once and does not +/// come back down (`contract.rs:1753`) — because a run oscillating between two +/// models mid-flight is a behaviour nobody asked for. Both clauses are stated +/// only when they can bite: a section with no escalation rule cannot escalate, +/// and one with only an escalation rule has nothing for it to win over. +/// +/// ASCII throughout, as [`crate::gates::Refusal`]'s sentences are: this renders +/// on the plain renderer, under `NO_COLOR`, and through the ASCII glyph set, and +/// a notice that arrives as a replacement character is a notice nobody reads. +pub fn describe(settings: &Settings) -> Option { + // **"asks for" and not "is in force", because this sentence is printed beside + // one that may contradict it.** `/config` records this and then, for a + // contained session, `inert_under_containment`'s warning that the rules will + // not fire — so a prefix asserting the rules are active made two adjacent + // lines disagree. What this function knows is what the operator *wrote*; + // whether it fires is the other function's subject. `/config` is also typed at + // an idle prompt, so "this run" named nothing. + let mut sentence = String::from("the routing rules ask for "); + match (&settings.escalate_after, &settings.downshift_under) { + (None, None) => return None, + (Some(escalation), None) => { + sentence.push_str(&escalation_clause(escalation)); + sentence.push_str(". The change happens once and does not come back down."); + } + (None, Some(downshift)) => { + sentence.push_str(&downshift_clause(downshift)); + sentence.push('.'); + } + (Some(escalation), Some(downshift)) => { + sentence.push_str(&escalation_clause(escalation)); + sentence.push_str(", and "); + sentence.push_str(&downshift_clause(downshift)); + sentence.push_str( + ". Escalating wins over downshifting when both apply, and it happens once and \ + does not come back down.", + ); + } + } + Some(sentence) +} + +/// The upward rule as a clause, with the count read in English. +/// +/// One attempt is singular. A notice that says "after 1 consecutive failed gate +/// attempts" is a notice that reads as unfinished, and the surfaces this appears +/// on are the ones an operator reads once and trusts. +fn escalation_clause(escalation: &Escalation) -> String { + // A half rule is refused by `routing` before any caller reaches a sentence, so + // the fallbacks below describe a rule that cannot reach the contract. They are + // written rather than unwrapped because a surface that panics on a + // configuration file is worse than one that says "unset". + let failures = escalation.failures.unwrap_or_default(); + format!( + "{} after {} consecutive failed gate attempt{}", + escalation.model.as_deref().unwrap_or("an unnamed model"), + failures, + if failures == 1 { "" } else { "s" }, + ) +} + +/// The downward rule as a clause. +fn downshift_clause(downshift: &Downshift) -> String { + format!( + "{} while it has written fewer than {} bytes", + downshift.model.as_deref().unwrap_or("an unnamed model"), + downshift.bytes.unwrap_or_default(), + ) +} + +/// The one thing a contained operator has to be told, or `None`. +/// +/// Returned only when both halves are true: the session runs contained **and** +/// the section names at least one rule. Either alone is silence. +/// +/// **The narrowness is the whole function, and it has a name.** Returning the +/// sentence whenever a routing section exists would warn every operator who +/// configured routing — the great majority of whom have no +/// `[app.io-cli.containment]` at all, whose runs go through the flat loop, and +/// whose rules therefore work exactly as written — about a limitation that does +/// not apply to them. A caveat attached to a working feature is how an operator +/// learns to stop reading the notices. And the mirror mistake is as bad the other +/// way: a contained session with no rules configured has nothing to disclose, and +/// a line about routing there is a line about a feature the operator never asked +/// for. +/// +/// `contained` is passed in rather than read here, so this stays a pure decision +/// over its arguments and a test needs no configuration on disk — the same reason +/// [`crate::gates::Settings::criterion`] takes its model as an argument. The +/// caller knows whether *this* session is contained, which is not the same +/// question as whether caps are configured: `/contain off` is a real switch, and +/// a turn run with it takes the flat loop and routes normally. The sentence says +/// so, because the operator who reads it has a way out. +pub fn inert_under_containment(settings: &Settings, contained: bool) -> Option { + if !contained || (settings.escalate_after.is_none() && settings.downshift_under.is_none()) { + return None; + } + Some( + "these routing rules will not fire while [app.io-cli.containment] is configured: \ + io-harness applies routing in its flat workspace loop only, and a contained turn takes \ + each agent's model from that agent's own definition. A turn run with /contain off routes \ + normally." + .to_string(), + ) +} diff --git a/src/settings.rs b/src/settings.rs index 8c6f9fa..e015633 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -166,6 +166,38 @@ pub struct CliSettings { /// addition rather than a break. #[serde(default, skip_serializing_if = "Option::is_none")] pub gates: Option, + /// Whether a question that is only a question may be answered without opening + /// a run. + /// + /// **The behaviour is io-harness's and it has been on all along.** + /// `session.rs:1125-1127` reads + /// `contract.conversational.unwrap_or(matches!(contract.verify, + /// Verification::None))`, and `TaskContract::workspace` starts at + /// `Verification::None` — so every ungated operator has had greetings answered + /// in one completion, with no steps row, no gate attempt and no snapshot, since + /// before this interface existed. 0.24.0 turned it on for gated operators too, + /// because attaching a criterion would otherwise have switched it off. + /// + /// What has never existed is a way to say *no*. This key is that, and it is + /// `false`-only in spirit: `true` is what an unset file already does in every + /// case io-cli produces. An operator who wants every prompt to open a run — + /// because a run is what their hooks, their gate or their trace expect — has + /// had no way to ask for one. + /// + /// Absent leaves the decision exactly where it was, which is what + /// `tests/contract.rs`'s field-for-field gate requires. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub conversational: Option, + /// `[app.io-cli.routing]` — when a run should change models, and to which. + /// + /// io-cli's own type rather than `io_harness::Routing`, which is the one place + /// this section differs from `[app.io-cli.containment]`: `Containment` + /// deserializes straight into the harness's type, and `Routing` derives no + /// serde at all and is `#[non_exhaustive]` besides (`contract.rs:1745`). So the + /// shape an operator writes is [`crate::routing::Settings`] and the conversion + /// goes through the harness's three builders. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub routing: Option, } /// Where prices come from, and what the last read was. @@ -536,6 +568,17 @@ pub fn render( // there is no default worth writing: "no gate" is the honest // answer for an operator who has not answered yet. gates: None, + // Left out for the strongest version of the same reason: absent + // is what io-harness already does, and it is what almost every + // operator wants. Writing `conversational = true` into a fresh + // file would state a default as though it were a choice, and + // writing `false` would make a new install open a run to answer + // "hello". + conversational: None, + // Left out for the caps' reason: routing names models, and the + // wizard has asked about exactly one. A rule pointing at a model + // the operator never chose is worse than no rule. + routing: None, }, }, }; diff --git a/src/status.rs b/src/status.rs index d52e362..c4beab8 100644 --- a/src/status.rs +++ b/src/status.rs @@ -353,6 +353,19 @@ pub struct Status { /// `false` renders as nothing at all, on the rule this line already holds: /// a session that has not asked to plan is not a session planning zero times. pub planning: bool, + /// How much reasoning every turn is buying, from `/effort`. + /// + /// A standing choice like [`Self::planning`], and it survives + /// [`Status::forget_run`] for the same reason: it holds until the operator + /// says otherwise, and the turn it was set on is over by the time anybody + /// reads it. + /// + /// `None` renders as nothing at all, on the rule this line already holds — a + /// session that has never said `/effort` is not a session buying zero + /// reasoning, it is one sending no reasoning field. Rendering a word for the + /// default would also put a field on every operator's status line for a + /// release that changed nothing about their turns. + pub effort: Option, /// Where this turn's verification gate stands, in one word. /// /// **A word and never a mark**, for the reason [`WORDS`] is a list of words @@ -559,6 +572,7 @@ impl Status { Self { model: model.into(), provider: None, + effort: None, steps: None, unknown: 0, policy: None, @@ -1207,6 +1221,13 @@ impl Status { if self.planning { fields.push(Field::new("planning".to_string(), Tone::Normal)); } + // Beside `planning` because it is the same class of fact: a standing choice + // the operator made that holds until they unmake it, and one that changes + // what every later turn costs. `Normal` for that reason too — it is not a + // footnote about the turn that just ended. + if let Some(effort) = self.effort { + fields.push(Field::new(format!("effort {effort}"), Tone::Normal)); + } // **Immediately right of the planning phase and left of every counter, // which is a decision about a narrow terminal and not a grouping.** The // rule this row already states is that a standing mode which stops the @@ -1463,6 +1484,15 @@ impl Status { // operator ever saw. 0.12.0's planning field, again, and 0.8.0's spend // field before it. counts.extend(self.cost_field()); + // **The effort level is a count-side field, not a standing one, and the + // reason is arithmetic rather than taxonomy** — see the note beside + // `planning` below for what putting it there cost. Pushed last, so + // narrowing takes it before it takes any number: of everything on this row, + // a level the operator has just set and can read back by typing `/effort` + // is the one they can most afford to lose to a narrow terminal. + if let Some(effort) = self.effort { + counts.push(format!("effort {effort}")); + } // **Here as well as on `Status::line`, from the same method, and that is // deliberate rather than tidy.** This is the row the binary draws at an // ordinary prompt — `Status::render` takes the footer on any terminal @@ -1544,6 +1574,18 @@ impl Status { } allowed.push(Span::styled("planning", muted)); } + // **The effort level is NOT here, and 0.25.0 already paid for the lesson.** + // The obvious home for a standing choice is beside `planning`, on the + // argument that it describes the circumstances a turn works in rather than + // counting anything — which reads well and is wrong, because of how the two + // groups yield. `row` fits this right-hand group all or nothing, so adding + // `effort high` to it takes the group past what eighty columns can hold and + // the operator loses the posture and the containment word **together**, by + // typing `/effort high`. That is the failure + // `f4_a_full_counts_row_drops_a_counter_and_not_the_planning_phase` exists + // to prevent, and it is why `branch` was moved out of this group one + // release ago. The level goes into `counts` below, where narrowing takes + // one field at a time. // **`counts` and NOT the right-hand group, and this release measured why.** // The branch first went in beside the posture, on the argument that it // describes the circumstances the agent works in rather than counting diff --git a/tests/commands.rs b/tests/commands.rs index 278ef2d..2fe1683 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -82,6 +82,7 @@ fn the_commands_are_the_commands() { // whose subject is a tool that is not io. "/import", "/profile", + "/effort", "/contain", // 0.12.0 — the planning phase stopped being something // `[app.io-cli.containment]` switched on by accident, so it needs a @@ -308,12 +309,175 @@ fn f12_resume_says_it_answers_a_parked_run_rather_than_merely_reopening_a_sessio // release growing a command and losing another. assert_eq!( COMMANDS.len(), - 32, - "0.25.0 adds exactly one command, `/commit`, on top of the thirty-one \ - 0.24.0 shipped; a thirty-third here means one arrived unrecorded", + 33, + "0.26.0 adds exactly one command, `/effort`, on top of the thirty-two \ + 0.25.0 shipped; a thirty-fourth here means one arrived unrecorded", ); } +/// **F1 — `/effort` is a posture, and a bare `/effort` is a question.** +/// +/// The three words the command understands, plus the two ways of asking rather +/// than setting. `Reasoning::Report` is what a question resolves to and it is what +/// makes the sentence below safe to print: the driver only assigns when +/// [`io_cli::app::reasoning_of`] answers `Some`. +/// +/// Sabotage: resolve an unrecognised word to the nearest level — under which this +/// fails on `/effort hgih`, and it fails by spending a turn's reasoning budget on +/// a typo. +#[test] +fn f1_effort_parses_three_levels_the_absence_and_the_question() { + use io_cli::commands::Reasoning; + let parse = |text: &str| commands::parse(text, &defaults(), &DARK); + + assert_eq!( + parse("effort low"), + Action::Effort(Reasoning::Buy(io_harness::Effort::Low)), + ); + assert_eq!( + parse("effort medium"), + Action::Effort(Reasoning::Buy(io_harness::Effort::Medium)), + ); + assert_eq!( + parse("effort high"), + Action::Effort(Reasoning::Buy(io_harness::Effort::High)), + ); + assert_eq!(parse("effort off"), Action::Effort(Reasoning::Off)); + assert_eq!(parse("effort"), Action::Effort(Reasoning::Report)); + assert_eq!( + parse("effort hgih"), + Action::Effort(Reasoning::Unknown("hgih".into())), + "an unrecognised word is refused by name rather than guessed at — and \ + never quietly reported, which read as an answer and left an operator \ + paying for the level they were trying to leave", + ); + // **Both spellings of `off`, because only one of them used to work.** + // `Effort::FromStr` lowercases for itself, so `/effort HIGH` was fine while + // `/effort OFF` fell past the literal match, failed to parse, and reported — + // leaving the level exactly where it was. Two spellings of one word behaving + // differently is the asymmetry nobody reports and everybody hits once. + assert_eq!(parse("effort OFF"), Action::Effort(Reasoning::Off)); + assert_eq!(parse("effort None"), Action::Effort(Reasoning::Off)); + assert_eq!( + parse("effort HIGH"), + Action::Effort(Reasoning::Buy(io_harness::Effort::High)), + ); +} + +/// **F1 — a word that is not a level says so, and says what is still in force.** +/// +/// Found by both adversarial reviewers. The old sentence for a rejected word was +/// the *report* sentence, so `/effort lwo` — an operator on `high` trying to spend +/// less — answered "every turn asks for high reasoning". That reads as +/// confirmation, the typo is invisible, and the expensive level goes on being +/// bought turn after turn. +#[test] +fn f1_a_word_that_is_not_a_level_is_refused_and_names_itself() { + use io_cli::app::reasoning_said; + use io_cli::commands::Reasoning; + + let said = reasoning_said( + &Reasoning::Unknown("lwo".into()), + Some(io_harness::Effort::High), + ); + assert!(said.contains("lwo"), "the word is quoted back: {said}"); + assert!( + said.contains("not a reasoning level"), + "the operator is told it was refused rather than obeyed: {said}", + ); + assert!( + said.contains("still asks for high"), + "and what is still in force, since nothing changed: {said}", + ); +} + +/// **F1 — a question changes nothing, and `off` is a change.** +/// +/// The outer `Option` [`io_cli::app::reasoning_of`] returns is the difference +/// between the two, and it is the difference the driver's assignment turns on. A +/// `Report` that answered `Some(None)` would make every bare `/effort` silently +/// clear the level the operator set — a question with a side effect. +#[test] +fn f1_asking_what_the_effort_is_does_not_change_it() { + use io_cli::app::reasoning_of; + use io_cli::commands::Reasoning; + + assert_eq!(reasoning_of(&Reasoning::Report), None); + assert_eq!(reasoning_of(&Reasoning::Off), Some(None)); + assert_eq!( + reasoning_of(&Reasoning::Buy(io_harness::Effort::High)), + Some(Some(io_harness::Effort::High)), + ); +} + +/// **F1 — the sentence says the state the session is now in.** +/// +/// Setting and reporting share one sentence because what an operator wants read +/// back is where they now are, and the absent case names the fact rather than a +/// level: "no reasoning field" is what goes on the wire, and calling it "off" +/// would suggest a setting between `low` and nothing. +#[test] +fn f1_the_effort_line_names_the_level_now_in_force() { + use io_cli::app::reasoning_said; + use io_cli::commands::Reasoning; + + let set = reasoning_said( + &Reasoning::Buy(io_harness::Effort::High), + Some(io_harness::Effort::High), + ); + assert!(set.contains("high"), "{set}"); + assert!( + set.contains("from here"), + "a level set holds for later turns: {set}" + ); + + let asked = reasoning_said(&Reasoning::Report, Some(io_harness::Effort::Low)); + assert!(asked.contains("low"), "{asked}"); + assert!( + !asked.contains("from here"), + "a question reports what is already true rather than announcing a change: {asked}", + ); + + let none = reasoning_said(&Reasoning::Off, None); + assert!( + none.contains("no reasoning field"), + "the absent case is the absence of the field, not a fourth level: {none}", + ); +} + +/// The group bound was met and paid for by a re-file, not by widening it. +/// +/// `src/commands.rs` pre-committed this answer when `Configure` reached nine and +/// again when `/commit` took `Turn` to ten: re-file what is in the wrong group, do +/// not widen the bound. This release is the one that had to keep it, so the keeping +/// is asserted rather than described — `f13_no_group_is_longer_than_ten` would pass +/// just as well if a later release had moved the bound to eleven. +#[test] +fn f2_effort_is_a_turn_command_and_profile_moved_to_the_session() { + assert_eq!( + commands::group_of("/effort"), + Some(commands::Group::Turn), + "`/effort` decides what the next turn buys, which is what `Turn` means", + ); + assert_eq!( + commands::group_of("/profile"), + Some(commands::Group::Session), + "`/profile` changes the overlay every later turn is built from, which is \ + not the work the turn just finished — it was misfiled, and moving it is \ + what made room rather than widening the bound", + ); + // The bound itself, restated here so this test fails as one story: a release + // that added `/effort` without the re-file would leave `Turn` at eleven. + for (group, names) in commands::GROUPS { + assert!( + names.len() <= 10, + "the {group:?} group holds {}; the bound is ten and this release \ + does not move it", + names.len(), + ); + } +} + #[test] fn an_unknown_command_says_what_does_exist() { let Action::Print(lines) = commands::parse("models", &defaults(), &DARK) else { diff --git a/tests/configure.rs b/tests/configure.rs index 2defe6d..16f0b5b 100644 --- a/tests/configure.rs +++ b/tests/configure.rs @@ -231,6 +231,23 @@ fn f2_the_catalogue_is_documented_rather_than_invented() { Either the key is invented or the example is out of date; both are this \ release's problem." ); + + // **And the section it lives under, because the last segment alone stopped + // being a check.** `model` is the last segment of two of 0.26.0's routing + // keys and occurs dozens of times in that file, so a mistyped sub-table — + // `routing.escalate.model` — satisfied the assertion above and every other + // gate. A key nested two or more levels deep names its own table, and the + // table header is what an operator actually has to type. + if let Some(parent) = key.rsplit_once('.').map(|(head, _)| head) { + if parent.matches('.').count() >= 2 { + assert!( + example.contains(&format!("[{parent}]")), + "`{key}` sits under `[{parent}]`, and docs/config.example.toml \ + has no such table. The last segment matching somewhere in the \ + file is not evidence that the path is right." + ); + } + } } } diff --git a/tests/contract.rs b/tests/contract.rs index 2613b43..f92a1c7 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -284,6 +284,334 @@ fn f2_nothing_configured_is_the_contract_the_session_built_before() { // reads it off the built contract, which is where it can actually be wrong. } +/// **F1 — the effort level is read by each turn, never taken from the session.** +/// +/// A driver-text gate, and it is here because there is no other instrument. +/// Nothing under `tests/` links `src/main.rs`, so the difference between a posture +/// and a one-shot — which is the whole of what `/effort` promises — lives in a +/// binding this suite cannot call. The sibling `fold_next` is deliberately a +/// `std::mem::take` two lines above it, so the wrong shape is not merely +/// imaginable here: it is written out, correctly, immediately adjacent. +/// +/// Weak, and the only one available. Four gates of this kind already exist in this +/// file and in `tests/steer.rs` for the same reason. +/// +/// Sabotage: change the argument to `std::mem::take(&mut effort)` — under which +/// only this test fails, and it fails by making every level last exactly one turn +/// while every other assertion about `/effort` stays green. +#[test] +fn f1_the_driver_reads_the_effort_level_rather_than_taking_it() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver).expect("the driver"); + // Line endings normalised: a Windows checkout has `\r\n`, and slicing on a + // bare `\n` has now shipped a defect in a test twice — 0.19.0 and 0.23.0. + let text = text.replace("\r\n", "\n"); + + assert!( + !text.contains("std::mem::take(&mut effort)"), + "the effort level is a posture: taking it would spend the level on one \ + turn, which is what `fold_next` beside it is supposed to be alone in doing", + ); + assert!( + text.contains("io_cli::contract::buying(contract, effort)"), + "the level reaches the turn through `contract::buying`, which is where \ + the decision is so that it can be asserted at all", + ); +} + +/// **F4 — the routing disclosure has callers, and `describe` has one at all.** +/// +/// The third driver-text gate, and it exists because tracing callers found this +/// release about to ship `routing::describe` with none — a public function +/// reachable from no keystroke, no subcommand and no event arm, which is exactly +/// what 0.20.0 shipped seven of behind 1,077 passing tests and what this release's +/// own `risks` calls this codebase's proven blind spot. `tests/routing.rs` covered +/// the function thoroughly and could not see that nothing called it. +/// +/// Three call sites, because there are three moments an operator can be in this +/// state: at session start, when they open the surface that edits the rules, and +/// when they type `/contain on` and enter the state mid-session. The last was +/// missing too. +#[test] +fn f4_the_routing_disclosure_is_reachable_from_the_driver() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver) + .expect("the driver") + .replace("\r\n", "\n"); + + assert!( + text.contains("io_cli::routing::describe("), + "`routing::describe` says what the rules are and nothing calls it — a \ + function no operator can reach is not a surface, it is dead code with \ + tests", + ); + assert_eq!( + text.matches("io_cli::routing::inert_under_containment(") + .count(), + 3, + "the disclosure belongs at session start, on `/config` where the rules are \ + edited, and on `/contain on`, which is the keystroke that puts an operator \ + into the state it warns about", + ); +} + +/// **F7 — the driver asks the turn's own kind and never infers it.** +/// +/// The second driver-text gate, and the criterion's named sabotage is exactly what +/// it forbids: a run cancelled before its first step also has zero steps and was +/// not an answer, so a report derived from a step count would call an interrupted +/// turn a conversation. +/// +/// Sabotage: report on `result.outcome`'s step count instead — under which only +/// this test fails, because `app::answered_said` would still be correct and simply +/// never called. +#[test] +fn f7_the_driver_reads_the_turn_kind_rather_than_counting_steps() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver) + .expect("the driver") + .replace("\r\n", "\n"); + + assert!( + text.contains("io_cli::app::answered_said(&result.kind, &result.outcome)"), + "whether a turn was answered is a fact io-harness reports on \ + `TurnResult::kind` — read WITH the outcome, because a `Reply` also covers \ + a completion refused at the token ceiling — and never something to infer \ + from what the run did not do", + ); +} + +/// **F1 — every turn that runs buys the reasoning the session asked for.** +/// +/// `contract::buying`'s own note counted three `contract::session` callers that +/// build a contract nothing runs — and there are five sites, not four: the startup +/// reading, the two reporting pages, the turn, and `resume_pending`, which drives +/// real completions and was missed. The consequence was that `/effort high` +/// applied to every turn except the half of the work an operator came back to +/// `/resume` and finish, while the status line went on saying `effort high`. +/// +/// **Counted, not `contains`.** The first version of this gate asked whether the +/// call appeared at all, which one site satisfies forever — so it could never have +/// caught the site that was missing. That is the vacuous-gate shape this suite has +/// now recorded three times. +#[test] +fn f1_every_turn_that_runs_applies_the_effort_level() { + let driver = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let text = std::fs::read_to_string(driver) + .expect("the driver") + .replace("\r\n", "\n"); + + assert_eq!( + text.matches("io_cli::contract::buying(").count(), + 2, + "two sites drive a turn — the ordinary one and the resumed one — and both \ + buy the level the session asked for. A third `contract::session` caller \ + that runs completions needs one too.", + ); +} + +/// **F8 — `conversational = false` makes a greeting a run, and absent changes +/// nothing.** +/// +/// Both files, because an assertion on one of them cannot tell a wired key from a +/// default. io-harness decides this for io-cli today — +/// `contract.conversational.unwrap_or(matches!(contract.verify, +/// Verification::None))` at `session.rs:1125-1127` — so with the key absent the +/// contract must carry `None` and leave that decision where it is, and with the key +/// present it must carry exactly what was written. +/// +/// Sabotage: pass the key's value only when it is `true` — under which this fails +/// on the `false` file, which is the only file anybody would write the key into. +#[test] +fn f8_the_conversational_key_reaches_the_contract_in_both_directions() { + let _guard = env_lock(); + let _home = HomeFixture::new(); + let (answerer, _questions) = io_cli::intent::channel(); + let responder: Arc = Arc::new(answerer); + + let built = |body: &str| { + let (dir, config) = discovered(&[("io.toml", body)]); + let root = dir.path().to_path_buf(); + let contract = session( + "say hello", + root, + &config, + &Capabilities::default(), + responder.clone(), + None, + ); + (dir, contract.conversational) + }; + + let (_empty, absent) = built(""); + assert_eq!( + absent, None, + "with no key, the decision stays io-harness's — it is on for an ungated \ + contract and io-cli must not restate that as a choice of its own", + ); + + let (_off, refused) = built("[app.io-cli]\nconversational = false\n"); + assert_eq!( + refused, + Some(false), + "an operator who wants every prompt to open a run has asked for one", + ); + + let (_on, wanted) = built("[app.io-cli]\nconversational = true\n"); + assert_eq!(wanted, Some(true)); +} + +/// **F3 — a routing section reaches the contract, and no section leaves it alone.** +/// +/// The rules themselves are asserted in `tests/routing.rs` against +/// `io_harness::Routing::model_for`, which is io-harness's own pure function and +/// the only implementation of the decision. What belongs here is the seam: that +/// what the operator wrote arrives on the contract, and that a file which names no +/// rule does not put a default `Routing` where there was nothing. +#[test] +fn f3_a_routing_section_reaches_the_contract_and_an_absent_one_leaves_it_unset() { + let _guard = env_lock(); + let _home = HomeFixture::new(); + let (answerer, _questions) = io_cli::intent::channel(); + let responder: Arc = Arc::new(answerer); + + let built = |body: &str| { + let (dir, config) = discovered(&[("io.toml", body)]); + let root = dir.path().to_path_buf(); + let contract = session( + "bring the docs up to date", + root, + &config, + &Capabilities::default(), + responder.clone(), + None, + ); + (dir, contract.routing) + }; + + let (_none, unset) = built(""); + assert_eq!(unset, None, "a file with no rules asks for no routing"); + + let (_empty, still_unset) = built("[app.io-cli.routing]\n"); + assert_eq!( + still_unset, None, + "a present but empty section names no rule, and a default `Routing` is a \ + value where there was absence", + ); + + let (_both, routed) = built( + "[app.io-cli.routing.escalate_after]\nfailures = 3\nmodel = \"stronger\"\n\ + [app.io-cli.routing.downshift_under]\nbytes = 2000\nmodel = \"cheaper\"\n", + ); + let routed = routed.expect("a section naming both rules routes"); + assert_eq!(routed.escalate_after, Some((3, "stronger".to_string()))); + assert_eq!(routed.downshift_under, Some((2000, "cheaper".to_string()))); +} + +/// **F7 — a turn that was answered is reported as answered, and a run is not.** +/// +/// The `Run` half is the one that must not move: every existing report about an +/// ordinary turn stays byte-identical, so this arm answers `None` and the driver +/// records nothing. +/// +/// Sabotage: report `Reply` for any run whose step count is zero — under which this +/// still passes and F7's live arm fails, because a turn cancelled before its first +/// step also has zero steps and was not an answer. That is why the kind is read +/// rather than inferred. +#[test] +fn f7_only_a_reply_is_reported_as_having_been_answered() { + let finished = io_harness::RunOutcome::Finished { steps: 0 }; + + assert!( + io_cli::app::answered_said(&io_harness::TurnKind::Reply, &finished) + .is_some_and(|said| said.contains("without opening a run")), + "a question that was only a question has to say so, or it arrives as silence", + ); + assert_eq!( + io_cli::app::answered_said(&io_harness::TurnKind::Run, &finished), + None, + "an ordinary turn already accounts for itself", + ); +} + +/// **F7 — a `Reply` that said nothing is not an answer.** +/// +/// `TurnKind::Reply` carries a second meaning io-harness documents on the variant +/// itself (`session.rs:1440-1443`): a turn whose one completion crossed the token +/// ceiling and was **refused rather than served** is also a `Reply`, because no run +/// was opened either way. Reading the kind alone told an operator with +/// `[run] max_tokens` set that their refused question had been answered — with no +/// answer anywhere on screen and no mention of the budget. +/// +/// Found by both adversarial reviewers independently, which is the strongest +/// signal this gate produces. +/// +/// The guard is io-harness's own, copied rather than invented: `session.rs:1202` +/// emits its `Answered` event only for a `Reply` whose outcome is `Finished`. +/// +/// Sabotage: drop the outcome from the match — under which only this test fails, +/// and it fails by reporting a message nobody wrote. +#[test] +fn f7_a_reply_refused_at_the_token_ceiling_is_not_an_answer() { + assert_eq!( + io_cli::app::answered_said( + &io_harness::TurnKind::Reply, + &io_harness::RunOutcome::CostBudgetExceeded { steps: 0 }, + ), + None, + "a completion refused at the budget was never served, so there is no \ + answer to announce", + ); +} + +/// **F1 — a session that never says `/effort` sends no reasoning field.** +/// +/// The absent case is not a fourth level. `TaskContract::effort` is an +/// `Option`, and `None` sends the pre-0.31.0 request body byte for byte — +/// `openai_wire.rs:1443` and `anthropic.rs:1529`. So the whole of what must be +/// proven here is that [`io_cli::contract::buying`] leaves the contract *identical* +/// when nothing was asked for, which Debug equality states exactly. +/// +/// Asserted through the same instrument as +/// [`f2_nothing_configured_is_the_contract_the_session_built_before`] and for the +/// same reason: a field set by accident is invisible to a test that only looks at +/// the field it meant to set. +/// +/// Sabotage: give `buying` a default of `Effort::Medium` for the `None` case — +/// under which only this test fails, and it fails by buying reasoning on every +/// turn of every operator who never asked for any. +#[test] +fn f1_no_effort_asked_for_leaves_the_contract_byte_for_byte() { + let root = std::path::PathBuf::from("/nowhere"); + let contract = TaskContract::workspace("a turn", root); + let before = format!("{contract:?}"); + + let after = io_cli::contract::buying(contract, None); + + assert_eq!(format!("{after:?}"), before); + assert_eq!(after.effort, None); +} + +/// **F1 — a level that was asked for is on the contract, and it is the one asked +/// for.** +/// +/// All three levels rather than one, because the mapping is a place a swap is +/// invisible: `Effort` is `Ord`, so `Low` and `High` transposed would still be a +/// valid contract and would still pass a test that only checked `is_some()`. +#[test] +fn f1_the_level_asked_for_is_the_level_the_turn_buys() { + let root = std::path::PathBuf::from("/nowhere"); + for level in [ + io_harness::Effort::Low, + io_harness::Effort::Medium, + io_harness::Effort::High, + ] { + let contract = + io_cli::contract::buying(TaskContract::workspace("a turn", root.clone()), Some(level)); + assert_eq!(contract.effort, Some(level)); + } +} + /// **F1 — the prompt is on the contract itself, so both arms carry it.** /// /// The difference the test above allows, asserted as the one it is: a contract diff --git a/tests/dependencies.rs b/tests/dependencies.rs index 1e8aaca..a3b7d17 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -359,6 +359,39 @@ fn f7_no_source_file_loops_over_provider_responses() { ); } + // **The 0.26.0 exemption, held to being a boundary.** `src/provider.rs` is the + // one file allowed a loop beside a provider call, because the chain asks each + // configured vendor in turn when the one before it failed. Two properties keep + // that from being an agent loop, and both are asserted rather than intended. + // + // First: every loop in the file iterates the chain's own links. A loop over + // anything else — steps, messages, attempts at the same vendor — is the shape + // this gate exists to refuse, and it would be refused here by name. + let forwarding_code = code_of(&forwarding); + for line in forwarding_code.lines().filter(|line| { + let trimmed = line.trim_start(); + trimmed.starts_with("for ") + || trimmed.starts_with("loop {") + || trimmed.starts_with("while ") + }) { + assert!( + line.contains("self.links.iter()"), + "src/provider.rs loops over something that is not the chain's links: \ + `{}`. The exemption covers asking the next vendor and nothing else.", + line.trim(), + ); + } + + // Second: the decision to try another vendor is io-harness's own predicate. + // A condition written here would be this crate forming an opinion about which + // failures are worth retrying — which is the judgement that turns a fallover + // into a loop with a policy. + assert!( + forwarding.contains("kind.is_retryable()"), + "the chain must fall through on `ProviderErrorKind::is_retryable`, which is \ + what io-harness's own `Fallback` and its own in-run retry both ask", + ); + let mut calls = Vec::new(); for (path, text) in sources() { // The forwards are subtracted rather than the file being skipped, so the @@ -377,22 +410,100 @@ fn f7_no_source_file_loops_over_provider_responses() { // A provider call and a loop in the same file is the shape of a second // agent loop. Asserted per file rather than per function on purpose: it is // coarse, and being coarse is what makes it hard to creep past. - for keyword in ["\n loop {", "\n while ", "\n for "] { - assert!( - !text.contains(keyword), - "{} both calls a provider and contains a top-level `{}` — \ - which is what an agent loop looks like", - path.display(), - keyword.trim(), - ); + // + // **0.26.0 amends this gate rather than relaxing it, and the amendment + // makes it stricter in the place it was weakest.** Until this release the + // three needles were anchored at four spaces, so they saw a loop written + // at the top level of a function and were blind to one written inside an + // `impl` — where an agent loop would actually be written. The release that + // introduced this crate's first loop around provider calls is the release + // that has to notice that, so the needles are now unanchored and every + // loop in a provider-calling file is seen. + // + // `src/provider.rs`'s chain is exempted BY PATH, and the exemption is a + // boundary rather than a hole: the assertions below it require that the + // only thing it loops over is its own list of links, and that the decision + // to try the next one is io-harness's `is_retryable` and not a predicate + // of this crate's. Iterating vendors on a failure the dependency has + // classified is not an agent loop — it never looks at a successful + // response, which the needles above already require. + // + // The shape is 0.7.0's amendment of the spawn ban and 0.17.0's of this + // gate, for the same reason both times. + // + // **Comments are stripped before the sweep looks**, which is the other + // half of unanchoring the needles. `while` and `for` are ordinary English + // words, and the first run of the wider gate refused `src/verify.rs` for + // three doc comments containing the word "while" — a gate that reads prose + // forbids a file from explaining itself, which this repository has now + // paid for in 0.16.0, 0.19.0 and twice in one wave in 0.25.0. The rule + // learned there is that the stripping belongs in the SWEEP and not only in + // the exception, so it is here. + let code = code_of(&text); + if path != delegating { + for keyword in ["loop {", "while ", "for "] { + assert!( + !code.contains(keyword), + "{} both calls a provider and contains a `{}` — \ + which is what an agent loop looks like", + path.display(), + keyword.trim(), + ); + } } } - let total: usize = calls.iter().map(|(_, count)| count).sum(); + // **The count is asserted over every file EXCEPT the delegating one, and that + // is 0.26.0's amendment.** Subtracting the provider module's calls one by one + // was how this held through 0.17.0, and it stopped being honest the moment the + // module gained a chain: `Vendor` delegates across four arms and `Chain` asks + // each link in turn, so the subtraction would have had to grow until it + // cancelled everything the file contains — a gate that goes vacuous without + // going red, which is the failure this suite has recorded twice. + // + // So the file is taken out of the count and held instead to the four + // properties above, each of which is specific and each of which fails loudly: + // its loops iterate only the chain's links, the decision to try another vendor + // is io-harness's own predicate, it never reads a response, and it forwards + // every completion method the trait declares. Those say what "no second agent + // loop" actually means here, where a number no longer can. + let elsewhere: usize = calls + .iter() + .filter(|(path, _)| path != &delegating) + .map(|(_, count)| count) + .sum(); + assert_eq!( + elsewhere, 1, + "outside src/provider.rs, io-cli should call a provider exactly once — the \ + wizard's verification handshake. Found {calls:?}", + ); + + // And the exempted file is still held to naming what it calls. Every call it + // makes is on a link of the chain, on the inner provider of a decorator, or on + // the arm of the vendor enum — never on a provider this module built in order + // to ask it something, which is what the count used to prevent. + // + // Counted on whitespace-squashed text rather than line by line, for the reason + // the forward count above already gives: rustfmt decides where a long call + // breaks, and an assertion about where a newline sits is an assertion about + // formatting. `link.complete_streaming_calls(..)` is split across three lines + // by rustfmt and was refused by the first, line-based version of this check. + let provider_module = code_of(&std::fs::read_to_string(&delegating).expect("the module")); + let squashed_module: String = provider_module + .chars() + .filter(|c| !c.is_whitespace()) + .collect(); + let made = squashed_module.matches(".complete").count(); + let named: usize = ["self.inner.complete", "p.complete", "link.complete"] + .iter() + .map(|receiver| squashed_module.matches(receiver).count()) + .sum(); assert_eq!( - total, 1, - "io-cli should call a provider exactly once, for the wizard's verification \ - handshake. Found {calls:?}", + made, named, + "src/provider.rs makes {made} provider calls and only {named} of them are on \ + a decorator's inner provider, a vendor arm or a chain link — the rest are \ + this module asking a provider something of its own, which is what the count \ + outside this file exists to prevent", ); } diff --git a/tests/docs.rs b/tests/docs.rs index 9abf306..988e5b3 100644 --- a/tests/docs.rs +++ b/tests/docs.rs @@ -261,6 +261,21 @@ fn the_readme_documents_every_key_of_the_io_cli_section() { reviewer: Some("a-reviewing-model".into()), allow_self_review: Some(false), }), + conversational: Some(false), + // Written out field by field for `gates`' reason, and with the same + // consequence: `[app.io-cli.routing]`'s two rules are sub-tables whose + // keys are explained in README prose rather than in rows of their own, so + // a key added to `routing::Settings` has to break something to be noticed. + routing: Some(io_cli::routing::Settings { + escalate_after: Some(io_cli::routing::Escalation { + failures: Some(3), + model: Some("a-stronger-model".into()), + }), + downshift_under: Some(io_cli::routing::Downshift { + bytes: Some(2_000), + model: Some("a-cheaper-model".into()), + }), + }), }; let value = serde_json::to_value(&every).expect("[app.io-cli] serializes"); let keys = value.as_object().expect("a table"); diff --git a/tests/live.rs b/tests/live.rs index e7f6639..e02a5a9 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -3200,3 +3200,198 @@ async fn live_f1_f2_an_asking_posture_refuses_git_and_the_allowance_lifts_it() { `App::note_git` explains and what `/commit allow` lifts; got {refusals:?}", ); } + +/// **F1 live — a reasoning level goes out on a real wire and the endpoint takes +/// it.** +/// +/// The half of F1 no fixture can reach. `contract::buying` puts an `Effort` on the +/// contract and `tests/contract.rs` asserts that; what only a real request can +/// settle is that io-harness translates it into a field the vendor accepts, and +/// that a turn carrying one still finishes. A wrong spelling would be a 400 from +/// the endpoint rather than a failing assertion here, which is exactly why this arm +/// exists. +/// +/// **The assertion is that the turn finishes, and that is the whole of it.** A +/// reasoning field the endpoint does not recognise is an HTTP 400 from the vendor, +/// not a failing comparison — so a turn that completes against a real endpoint +/// with `contract.effort` set is the evidence, and there is nothing further to +/// read back. io-cli's own `context::Request` deliberately does not carry the +/// field: `/context` reports what is in the window, and a reasoning level is not +/// context. +/// +/// Nothing is asserted about the answer's words, for the reason every arm in this +/// file gives: identical code and the same model produce different prose. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f1_a_reasoning_level_reaches_the_wire_and_the_turn_still_finishes() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + std::fs::write(root.join("notes.md"), "one line\n").expect("the fixture file"); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let provider = io_harness::OpenRouter::new(&key, model()); + let policy = workspace_policy(); + + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::buying( + io_cli::contract::session( + "What is in notes.md? Answer in one sentence.", + root.to_path_buf(), + &no_configuration(), + &io_cli::contract::Capabilities::default(), + Arc::new(answerer), + None, + ), + Some(io_harness::Effort::Low), + ); + + assert_eq!( + contract.effort, + Some(io_harness::Effort::Low), + "the contract has to carry the level before the wire can", + ); + + let result = session + .turn_bounded_observed( + &contract, + &provider, + &store, + &policy, + &DenyAll, + &io_harness::Ignore, + ) + .await + .expect("a turn carrying a reasoning level still finishes"); + + println!( + "live 0.26.0 F1: outcome {:?} kind {:?}", + result.outcome, result.kind + ); + + assert!( + result + .reply + .as_deref() + .is_some_and(|text| !text.trim().is_empty()), + "a turn carrying a reasoning level produced no answer, which is what a \ + field the endpoint refuses looks like from here: {:?}", + result.outcome, + ); +} + +/// **F5 live — a primary that cannot be reached is fallen through, against a real +/// endpoint underneath.** +/// +/// The other half no fixture settles. `tests/provider.rs` proves the chain's logic +/// with a fake link that fails on demand; what it cannot prove is that a *real* +/// transport failure is classified as retryable by io-harness and therefore +/// reaches the next link at all. The head here is a `Compatible` pointed at a port +/// nothing is listening on, which is the cheapest genuine `ProviderErrorKind:: +/// Transport` available — no credential is spent on it, because the connection +/// never opens. +/// +/// The tail is the real provider, so a pass means an answer that actually came +/// from the second link, and `last_served` names it. That name is also what +/// io-harness turns into `EventKind::FellBackTo` (`run/step.rs:503`), which is the +/// scrollback arm this release makes reachable for the first time. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f5_an_unreachable_primary_falls_through_to_the_provider_underneath() { + let key = key(); + let dir = tempfile::tempdir().expect("a workspace"); + let root = dir.path(); + + let store = Store::open(root.join("runs.db")).expect("a store"); + let mut session = Session::open(&store, root).expect("a session"); + let policy = workspace_policy(); + + // A base URL on the loopback with nothing behind it. The failure is a refused + // connection, which is `Transport` — retryable, and therefore worth another + // vendor by io-harness's own predicate. + let specs = vec![ + ProviderSpec::Compatible { + model: model(), + preset: None, + base_url: Some("http://127.0.0.1:9/v1".into()), + api_key: Some("unused".into()), + auth: None, + name: None, + reference_prices: false, + }, + ProviderSpec::OpenRouter { + model: model(), + api_key: Some(key), + }, + ]; + + struct Falling<'a> { + session: &'a mut Session, + store: &'a Store, + policy: &'a Policy, + root: std::path::PathBuf, + } + + impl io_cli::provider::WithProvider for Falling<'_> { + type Out = (String, Option); + + async fn call( + self, + make: impl Fn(&str) -> Result, + model: String, + ) -> Self::Out { + let provider = make(&model).expect("the chain builds"); + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::session( + "Reply with the single word: ready.", + self.root.clone(), + &no_configuration(), + &io_cli::contract::Capabilities::default(), + Arc::new(answerer), + None, + ); + let result = self + .session + .turn_bounded_observed( + &contract, + &provider, + self.store, + self.policy, + &DenyAll, + &io_harness::Ignore, + ) + .await + .expect("the second link answers"); + ( + result.reply.clone().unwrap_or_default(), + provider.last_served(), + ) + } + } + + let (reply, served) = io_cli::provider::build( + specs, + None, + Falling { + session: &mut session, + store: &store, + policy: &policy, + root: root.to_path_buf(), + }, + ) + .await + .expect("a chain of two builds"); + + println!("live 0.26.0 F5: served {served:?} reply {reply:?}"); + + assert!( + !reply.trim().is_empty(), + "the turn produced no answer, so nothing fell through to anything", + ); + assert!( + served.is_some(), + "a fall-through happened and `last_served` must name the link that \ + answered — it is what io-harness turns into `EventKind::FellBackTo`", + ); +} diff --git a/tests/narrow.rs b/tests/narrow.rs index 4b85eb3..15ef6ed 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -1711,6 +1711,171 @@ fn f11_the_branch_field_is_a_plain_name_and_costs_a_stated_number_of_columns() { } } +/// **0.26.0 N2 — setting an effort level does not take the posture off the row.** +/// +/// The sibling of `f11_a_branch_yields_before_the_standing_facts_at_eighty_columns` +/// and it exists because this release very nearly repeated the defect that one was +/// written for. `effort` was first pushed into the footer's right-hand group beside +/// `planning`, on the argument that it is a standing choice rather than a counter — +/// which reads well and is wrong, because `row` fits that group **all or nothing**. +/// Measured on this fixture, `effort high` takes the group past what eighty columns +/// can hold, so typing `/effort high` removed the posture, the containment mode and +/// the planning phase together. `branch` was moved out of that group one release +/// ago for exactly this reason. +/// +/// **The wide arm is not optional.** At eighty columns with this fixture the level +/// may yield, so an assertion made only there is equally true of a status line that +/// never drew the field at all — which is how 0.25.0 shipped a vacuous gate over +/// this very row. So one arm proves the field is drawn where there is room, and the +/// other proves what it does not cost where there is not. +/// +/// Sabotage: push the level into the `allowed` group instead — under which the +/// narrow arm fails, because the posture and the containment word disappear +/// together. +#[test] +fn n2_an_effort_level_yields_before_the_standing_facts_at_eighty_columns() { + use io_cli::status::Status; + + let fixture = |effort: Option| { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.policy = Some("read-only".into()); + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + status.planning = true; + status.effort = effort; + status + }; + + // **The invariant across widths, rather than an assertion at one.** Which + // width a field starts costing something depends on the posture string, the + // glyph set and every counter on the row, so a test that picks one number is + // asserting arithmetic it will get wrong — the first version of this gate + // picked eighty, where this fixture's group does not fit either way, and the + // sabotage that put the level back in the all-or-nothing group passed it. The + // property that actually matters has no number in it: **setting a level must + // never remove a standing fact that was on the row without it.** + let standing = [ + "read-only", + "workspace-write/macos-sandbox-exec", + "planning", + ]; + + for theme in themes() { + let set = theme.glyphs.name; + let mut ever_held = false; + + for width in [80, 88, 96, 104, 120, 160] { + let bare = text_of(&fixture(None).footer(width, &theme)).join("\n"); + let level = + text_of(&fixture(Some(io_harness::Effort::High)).footer(width, &theme)).join("\n"); + + for row in level.lines() { + assert!( + row.chars().count() <= width as usize, + "{set}: a footer row overflowed {width} columns: {row:?}", + ); + } + + for fact in standing { + if bare.contains(fact) { + ever_held = true; + assert!( + level.contains(fact), + "{set}: at {width} columns, setting an effort level took \ + `{fact}` off the row. The level belongs in the counters, \ + which narrow one at a time, not in the group `row` keeps \ + or drops whole.\n without: {bare:?}\n with: {level:?}", + ); + } + } + } + + // Without this the loop above is satisfied by a row that never carried a + // standing fact at any width — which is precisely how the first version of + // this test passed its own sabotage. + assert!( + ever_held, + "{set}: no width held a standing fact even without an effort level, so \ + nothing above measured anything", + ); + + // And the level is drawn where there is room for it, so this is a gate over + // a field that exists rather than over one that was never added. + let wide = text_of(&fixture(Some(io_harness::Effort::High)).footer(200, &theme)).join("\n"); + assert!( + wide.contains("effort high"), + "{set}: the level is not on the footer at any width: {wide:?}", + ); + } +} + +/// **0.26.0 N2 — the effort level is drawn in both glyph sets and costs no glyph +/// it does not have.** +/// +/// The field is a word and a level: `effort low`, `effort medium`, `effort high`. +/// It carries no marker, no bullet and no arrow, so it is the same characters in +/// both sets — which is the property to assert rather than assume, because every +/// other standing field on this row was at some point a glyph the ASCII set did +/// not have. `Status::line` hardcoded a `•` for four releases for exactly that +/// reason. +/// +/// **Two widths, which is 0.25.0's lesson and not a flourish.** At eighty columns +/// with a crowded row a field may not fit either way, and a gate that asserts only +/// there is equally true of a status line that never drew the field at all — that +/// release shipped one and found it by sabotage. So the wide arm proves it is +/// drawn, and the narrow arm proves the row still fits when it is. +/// +/// Sabotage: draw the level with a glyph — under which the ASCII arm fails on the +/// `is_ascii` assertion, which is the one a monochrome or ASCII terminal actually +/// pays for. +#[test] +fn n2_the_effort_field_is_a_plain_word_in_both_glyph_sets() { + use io_cli::status::Status; + + for theme in themes() { + let set = theme.glyphs.name; + let mut status = Status::new("io/model"); + + // Absent until it is set, asserted on both renderers — a default that + // drew `effort medium` would put a field on every operator's status line + // for a release that changed nothing about their turns. + let quiet = text_of(&[status.line(WIDTH, &theme)]).remove(0); + assert!( + !quiet.contains("effort"), + "{set}: a session that has never said `/effort` is not a session \ + buying zero reasoning: {quiet:?}", + ); + + for level in [ + io_harness::Effort::Low, + io_harness::Effort::Medium, + io_harness::Effort::High, + ] { + status.effort = Some(level); + let drawn = format!("effort {level}"); + assert!( + drawn.is_ascii(), + "{set}: the field needs a glyph it does not have: {drawn:?}", + ); + + // Wide, where the row has room: the field is there and whole. + let wide = text_of(&[status.line(200, &theme)]).remove(0); + assert!( + wide.contains(&drawn), + "{set}: the level every turn is buying is not on the line: {wide:?}", + ); + + // And at eighty, where the row must still fit whatever it decided to + // keep. The field may yield here — it is a narrowing decision, not a + // promise — but the line may never overrun. + let narrow = text_of(&[status.line(WIDTH, &theme)]).remove(0); + assert!( + narrow.chars().count() <= WIDTH as usize, + "{set}: the status line overran eighty columns: {narrow:?}", + ); + } + } +} + /// **0.25.0 F11 — the branch reaches the footer, and yields before the standing /// facts when the row cannot hold it.** /// diff --git a/tests/provider.rs b/tests/provider.rs index 8a26667..40d94ab 100644 --- a/tests/provider.rs +++ b/tests/provider.rs @@ -249,3 +249,325 @@ fn f10_the_reviewer_judges_and_never_drives() { "the reviewer is built for the contract's criterion and for nothing else", ); } + +// --------------------------------------------------------------------------- +// F5 and F6 — the chain that runs is the chain the panel draws. + +use io_cli::provider::Chain; +use io_harness::{CompletionRequest, CompletionResponse, Provider, ProviderErrorKind}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; + +/// A link that answers, or fails in a chosen way, and counts what it was asked. +/// +/// The counter lives outside the link because [`Chain::of`] takes ownership of its +/// links. It is what makes F6 assertable at all: "the second provider is never +/// called" is a statement about a call that did not happen, and no message can tell +/// that apart from a call that happened and was discarded. +struct Fake { + label: String, + fail: Option, + calls: Arc, + /// Whether this link would take an image. + /// + /// Settable because the trait's default is `false`, and a fixture where every + /// link answers `false` cannot tell a conjunction from a disjunction or from a + /// head-only read — the first version of the image test asserted `!all` over + /// two defaulted links and would have passed against any of the three. + images: bool, +} + +/// A counter to read afterwards, and the link that increments it. +fn link(label: &str, fail: Option) -> (Fake, Arc) { + accepting(label, fail, true) +} + +/// A link that says whether it would take an image. +fn accepting( + label: &str, + fail: Option, + images: bool, +) -> (Fake, Arc) { + let calls = Arc::new(AtomicUsize::new(0)); + ( + Fake { + label: label.into(), + fail, + calls: Arc::clone(&calls), + images, + }, + calls, + ) +} + +fn asked(calls: &Arc) -> usize { + calls.load(Ordering::Relaxed) +} + +impl Provider for Fake { + async fn complete( + &self, + _request: CompletionRequest, + ) -> io_harness::Result { + self.calls.fetch_add(1, Ordering::Relaxed); + match self.fail { + Some(kind) => Err(io_harness::Error::Provider { + kind, + status: None, + retry_after: None, + message: format!("{} was asked and refused", self.label), + }), + None => Ok(CompletionResponse { + text: Some(self.label.clone()), + ..Default::default() + }), + } + } + + fn name(&self) -> &str { + &self.label + } + + fn endpoints(&self) -> Vec<&str> { + vec![&self.label] + } + + fn accepts_images(&self) -> bool { + self.images + } +} + +fn asking() -> CompletionRequest { + CompletionRequest { + system: "s".into(), + user: "u".into(), + ..Default::default() + } +} + +#[tokio::test] +async fn f5_a_retryable_failure_falls_through_to_the_next_link() { + let (first, first_calls) = link("first", Some(ProviderErrorKind::Server)); + let (second, second_calls) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("two links are a chain"); + + let answer = chain.complete(asking()).await.expect("the second answers"); + + assert_eq!( + answer.text.as_deref(), + Some("second"), + "a head that failed in a way another vendor might survive must not end the turn", + ); + assert_eq!( + chain.last_served().as_deref(), + Some("second"), + "the link that answered is what io-harness records and what the status line moves to", + ); + assert_eq!((asked(&first_calls), asked(&second_calls)), (1, 1)); +} + +#[tokio::test] +async fn f5_the_head_is_asked_first_and_a_link_below_it_is_not_asked_at_all() { + let (first, first_calls) = link("first", None); + let (second, second_calls) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + + let answer = chain.complete(asking()).await.expect("the head answers"); + + assert_eq!( + answer.text.as_deref(), + Some("first"), + "the operator's first choice is the provider a question is put to", + ); + assert_eq!( + (asked(&first_calls), asked(&second_calls)), + (1, 0), + "a link below a head that answered is not asked, so it is not billed", + ); +} + +#[tokio::test] +async fn f5_a_head_that_answered_reports_no_fallover() { + // **The whole reason this crate does not build on `io_harness::Fallback`.** + // io-harness emits `EventKind::FellBackTo` for any `Some` from `last_served` + // (`run/step.rs:503`), and `Fallback::last_served` answers `Some` for its own + // primary — so a chain built from that type would tell the operator "the + // provider fell over" on every step of every run. + let (first, _) = link("first", None); + let (second, _) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + chain.complete(asking()).await.expect("the head answers"); + + assert_eq!( + chain.last_served(), + None, + "nothing fell over, so nothing may be reported as having fallen over", + ); +} + +#[tokio::test] +async fn f5_one_provider_is_a_chain_that_behaves_exactly_as_no_chain_did() { + let (only, _) = link("only", None); + let chain = Chain::of(vec![only]).expect("one link is a chain"); + + let answer = chain + .complete(asking()) + .await + .expect("the only link answers"); + + assert_eq!(answer.text.as_deref(), Some("only")); + assert_eq!( + chain.last_served(), + None, + "an operator who configured one provider must stay on the path they were on", + ); +} + +#[tokio::test] +async fn f6_a_bad_credential_on_the_head_does_not_spend_the_link_below_it() { + let (first, first_calls) = link("first", Some(ProviderErrorKind::Auth)); + let (second, second_calls) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + + let refused = chain.complete(asking()).await; + + assert!( + refused.is_err(), + "a wrong key is not a failure another vendor can survive", + ); + assert_eq!( + (asked(&first_calls), asked(&second_calls)), + (1, 0), + "the second link is never called, so a typo in a key cannot start spending elsewhere", + ); + assert_eq!( + chain.last_served(), + None, + "nobody answered, so no provider may be named as having done so", + ); +} + +/// What a caller learns about the chain `provider::build` handed it. +/// +/// The only way to assert `build` itself. Everything above tests [`Chain`] +/// directly, which cannot catch a `build` that assembles the wrong links — and +/// "assembles the wrong links" is precisely F5's named sabotage, since dropping +/// the head still produces a perfectly working chain of the operator's second +/// choice. +struct Probe; + +impl io_cli::provider::WithProvider for Probe { + /// The model the chain will ask first, and every host it may reach. + type Out = (String, usize); + + async fn call( + self, + make: impl Fn(&str) -> Result, + model: String, + ) -> Self::Out { + let provider = make(&model).expect("the chain builds from two valid specs"); + ( + provider.model_hint().unwrap_or_default().to_string(), + provider.endpoints().len(), + ) + } +} + +#[tokio::test] +async fn f5_build_asks_the_operators_first_choice_first() { + // Two vendors rather than two of one, so the links are distinguishable by + // something other than the model name — and the credential is in the spec so + // no test has to touch the process environment to run. + let specs = vec![ + io_harness::ProviderSpec::OpenRouter { + model: "head-model".into(), + api_key: Some("k".into()), + }, + io_harness::ProviderSpec::Anthropic { + model: "tail-model".into(), + api_key: Some("k".into()), + }, + ]; + + let (asked_first, hosts) = io_cli::provider::build(specs, None, Probe) + .await + .expect("a chain of two"); + + assert_eq!( + asked_first, "head-model", + "the head of the chain is the provider the operator wrote first; a chain \ + folded from the tail alone answers every request from their second choice", + ); + assert_eq!( + hosts, 2, + "every link's host reaches the egress policy, or a fall-through is a way \ + to reach a host the policy never saw", + ); +} + +#[tokio::test] +async fn f5_a_model_override_replaces_the_heads_model_and_not_the_tails() { + // `-m/--model` names one model, and a chain has several. Applying it to every + // link would ask a second vendor for a model id only the first one serves. + let specs = vec![ + io_harness::ProviderSpec::OpenRouter { + model: "head-model".into(), + api_key: Some("k".into()), + }, + io_harness::ProviderSpec::Anthropic { + model: "tail-model".into(), + api_key: Some("k".into()), + }, + ]; + + let (asked_first, _) = io_cli::provider::build(specs, Some("chosen".into()), Probe) + .await + .expect("a chain of two"); + + assert_eq!(asked_first, "chosen"); +} + +#[test] +fn f5_every_link_s_host_is_authorized_and_not_only_the_head_s() { + // io-harness's egress policy is deny-by-default and authorizes the provider's + // hosts before the first step. A chain reporting only its head's host would + // make a fall-through a way to reach a host the policy never saw. + let (first, _) = link("first", None); + let (second, _) = link("second", None); + let chain = Chain::of(vec![first, second]).expect("a chain"); + + assert_eq!(chain.endpoints(), vec!["first", "second"]); +} + +/// **The conjunction, asserted as one rather than read off a default.** +/// +/// io-harness's own rule for the same question, and the reason is that the +/// fall-through is the one call where it matters: reporting the head's answer +/// would let an image reach a link that cannot read it on exactly the call that +/// went wrong. +/// +/// The first version of this test built two links that both took the trait's +/// default of `false` and asserted `!accepts_images()`. Every possible +/// implementation passes that — `all`, `any`, and a head-only read alike — so it +/// was a gate over nothing, which the adversarial review caught. The pair below is +/// what makes it a gate: one mixed chain and one where every link agrees. +#[test] +fn f5_an_image_is_refused_unless_every_link_accepts_one() { + let (yes, _) = accepting("first", None, true); + let (no, _) = accepting("second", None, false); + let mixed = Chain::of(vec![yes, no]).expect("a chain"); + assert!( + !mixed.accepts_images(), + "one link that cannot read an image is enough to refuse it before the run \ + rather than midway through a fall-through", + ); + + let (one, _) = accepting("first", None, true); + let (two, _) = accepting("second", None, true); + let agreed = Chain::of(vec![one, two]).expect("a chain"); + assert!( + agreed.accepts_images(), + "a chain whose links all accept images accepts them — without this arm the \ + assertion above is satisfied by a function that always answers false", + ); +} diff --git a/tests/routing.rs b/tests/routing.rs new file mode 100644 index 0000000..8fe3822 --- /dev/null +++ b/tests/routing.rs @@ -0,0 +1,388 @@ +//! F3 and F4 — what `[app.io-cli.routing]` puts on the contract, and what a +//! contained session is told about it. +//! +//! F3 is the section reaching an `io_harness::Routing`: the keys an operator +//! types, the two rules they name, and the run's behaviour under them. F4 is the +//! disclosure that those rules do not fire for a contained turn. +//! +//! Two things here are asserted the way the implementation would be wrong rather +//! than the way it is right. +//! +//! **The behaviour is asserted through `io_harness::Routing::model_for` and never +//! re-implemented.** The precedence rule — escalating beats downshifting when +//! both conditions hold — belongs to io-harness, and a test that computed the +//! expected model from io-cli's own reading of the rules would pass against an +//! io-cli that had quietly disagreed with the harness about which model a run +//! asks. Asking the harness's own method is the only assertion that catches that. +//! +//! **The empty cases are asserted separately from the absent ones.** A section +//! that is present and names no rule and a section that was never written must +//! both leave the contract's routing unset; building a `Routing::default()` for +//! the first is the mistake that looks harmless, so it gets its own test. + +use io_cli::routing::{self, Downshift, Escalation, Settings}; + +/// The models the fixtures route between. Names, not real models: nothing here +/// connects to anything, and a name that looks like a real one invites a reader +/// to think it matters which. +const STRONGER: &str = "vendor/stronger-model"; +const CHEAPER: &str = "vendor/cheaper-model"; + +/// A section naming both rules. +fn both() -> Settings { + Settings { + escalate_after: Some(Escalation { + failures: Some(3), + model: Some(STRONGER.to_string()), + }), + downshift_under: Some(Downshift { + bytes: Some(2_000), + model: Some(CHEAPER.to_string()), + }), + } +} + +// --------------------------------------------------------------------------- +// F3 — the section becomes a Routing +// --------------------------------------------------------------------------- + +#[test] +fn a_section_naming_both_rules_carries_both_exactly_as_written() { + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); + assert_eq!(routing.escalate_after, Some((3, STRONGER.to_string()))); + assert_eq!(routing.downshift_under, Some((2_000, CHEAPER.to_string()))); +} + +#[test] +fn a_section_that_names_no_rule_puts_no_routing_on_the_contract() { + assert_eq!(routing::routing(&Settings::default()), Ok(None)); +} + +#[test] +fn a_present_but_empty_section_puts_no_routing_on_the_contract() { + let empty: Settings = toml::from_str("").expect("an empty routing section parses"); + assert_eq!(empty, Settings::default()); + assert_eq!( + routing::routing(&empty), + Ok(None), + "an empty Routing is a value where there was none, and it changes the contract" + ); +} + +#[test] +fn a_section_naming_only_escalation_carries_only_escalation() { + let settings = Settings { + downshift_under: None, + ..both() + }; + let routing = routing::routing(&settings) + .expect("one rule is obeyable") + .expect("one rule is a routing"); + assert_eq!(routing.escalate_after, Some((3, STRONGER.to_string()))); + assert_eq!(routing.downshift_under, None); +} + +#[test] +fn a_section_naming_only_downshift_carries_only_downshift() { + let settings = Settings { + escalate_after: None, + ..both() + }; + let routing = routing::routing(&settings) + .expect("one rule is obeyable") + .expect("one rule is a routing"); + assert_eq!(routing.escalate_after, None); + assert_eq!(routing.downshift_under, Some((2_000, CHEAPER.to_string()))); +} + +// --------------------------------------------------------------------------- +// F3 — what the harness then does with it +// --------------------------------------------------------------------------- + +#[test] +fn under_the_failure_threshold_a_small_run_is_asked_of_the_cheaper_model() { + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); + assert_eq!(routing.model_for(0, 0), Some(CHEAPER)); + assert_eq!(routing.model_for(2, 1_999), Some(CHEAPER)); +} + +#[test] +fn escalating_beats_downshifting_at_and_above_the_threshold() { + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); + // Both conditions hold: three consecutive failures, and fewer bytes written + // than the downshift bound. io-harness answers with the stronger model. + assert_eq!(routing.model_for(3, 0), Some(STRONGER)); + assert_eq!(routing.model_for(9, 1_999), Some(STRONGER)); +} + +#[test] +fn neither_condition_met_leaves_the_requests_model_alone() { + let routing = routing::routing(&both()) + .expect("two rules are obeyable") + .expect("two rules are a routing"); + assert_eq!(routing.model_for(2, 2_000), None); + assert_eq!(routing.model_for(0, 10_000), None); +} + +// --------------------------------------------------------------------------- +// F4 — the containment disclosure +// --------------------------------------------------------------------------- + +#[test] +fn a_contained_session_with_rules_is_told_they_will_not_fire() { + let notice = routing::inert_under_containment(&both(), true) + .expect("contained plus rules is the one case that discloses"); + assert!( + notice.contains("[app.io-cli.containment]"), + "the notice must name the section that caused it: {notice}" + ); + assert!( + notice.contains("/contain off"), + "the notice must name the way out: {notice}" + ); +} + +#[test] +fn an_uncontained_session_with_rules_is_told_nothing() { + assert_eq!( + routing::inert_under_containment(&both(), false), + None, + "the majority of operators have no containment, and their rules work" + ); +} + +#[test] +fn a_contained_session_with_no_rules_is_told_nothing() { + assert_eq!( + routing::inert_under_containment(&Settings::default(), true), + None + ); +} + +#[test] +fn an_uncontained_session_with_no_rules_is_told_nothing() { + assert_eq!( + routing::inert_under_containment(&Settings::default(), false), + None + ); +} + +// --------------------------------------------------------------------------- +// F3 — the sentence a surface draws +// --------------------------------------------------------------------------- + +#[test] +fn the_description_states_escalation_before_downshift_and_names_both_rules() { + let sentence = routing::describe(&both()).expect("two rules describe as something"); + let escalation = sentence + .find(STRONGER) + .expect("the stronger model is named"); + let downshift = sentence.find(CHEAPER).expect("the cheaper model is named"); + assert!( + escalation < downshift, + "escalation is the rule that overrides the other and is stated first: {sentence}" + ); + assert!(sentence.contains("3 consecutive failed gate attempts")); + assert!(sentence.contains("2000 bytes")); + assert!( + sentence.contains("wins over downshifting"), + "io-harness's precedence rule has to be in the sentence: {sentence}" + ); + assert!( + sentence.contains("happens once"), + "escalation not coming back down is the other half: {sentence}" + ); +} + +#[test] +fn a_single_failure_threshold_is_described_in_the_singular() { + let settings = Settings { + escalate_after: Some(Escalation { + failures: Some(1), + model: Some(STRONGER.to_string()), + }), + downshift_under: None, + }; + let sentence = routing::describe(&settings).expect("one rule describes as something"); + assert!( + sentence.contains("1 consecutive failed gate attempt"), + "the count is stated: {sentence}" + ); + assert!( + !sentence.contains("attempts"), + "one attempt is singular: {sentence}" + ); +} + +#[test] +fn a_section_with_no_rules_describes_as_nothing() { + assert_eq!(routing::describe(&Settings::default()), None); +} + +// --------------------------------------------------------------------------- +// F3 — the keys an operator actually types +// --------------------------------------------------------------------------- + +/// The spellings are pinned by a test because they are the operator's interface. +/// A field renamed in `src/routing.rs` compiles, passes every other test in this +/// file, and silently stops reading a file somebody already wrote. +#[test] +fn the_section_deserializes_from_the_toml_an_operator_writes() { + let settings: Settings = toml::from_str( + r#" +[escalate_after] +failures = 3 +model = "vendor/stronger-model" + +[downshift_under] +bytes = 2000 +model = "vendor/cheaper-model" +"#, + ) + .expect("the documented section parses"); + assert_eq!(settings, both()); +} + +/// **A half rule parses and is refused, and the difference is the whole finding.** +/// +/// These keys were required, which looked stricter and was far worse. A required +/// field is a *deserialization* failure, so `failures = 3` with no `model` did not +/// fail the rule — it failed `CliSettings`, and `settings::stored` then answered +/// `None` for the whole `[app.io-cli]` section. The theme, the keys, the ceilings, +/// the capabilities and the **verification gate** all silently reverted to their +/// defaults, because `contract::criterion_for` gives up on that same `None`. A gate +/// that stops gating without saying so is the most expensive failure this crate +/// has, and it was one missing line in a configuration file away. +/// +/// Worse, `/config` writes exactly one key per invocation, so **every** path +/// through that surface to a routing rule passed through this state. +/// +/// Found by the adversarial review. Sabotage: make either key required again — +/// under which this test fails at the `expect`, because the section no longer +/// parses at all. +#[test] +fn a_rule_missing_half_of_itself_parses_and_is_refused_by_name() { + let settings: Settings = toml::from_str("[escalate_after]\nfailures = 3\n") + .expect("half a rule must still parse, or it takes all of [app.io-cli] with it"); + + assert_eq!( + routing::routing(&settings), + Err(routing::Refusal::HalfARule { + rule: "escalate_after", + missing: "model", + }), + ); + assert!( + routing::notice(&settings).is_some_and(|said| said.contains("half a rule")), + "the operator hears which key is missing", + ); +} + +/// **The three values that are writable and disastrous.** +/// +/// io-harness obeys the thresholds literally (`contract.rs:1811-1822`), and none +/// of these is a shape TOML can refuse: +/// +/// * `failures = 0` satisfies `consecutive_gate_failures >= 0` at the first request +/// of every run, so the escalation model is used unconditionally and the +/// downshift — checked second — is never reached. An operator writing it means +/// "escalate readily" and gets "never use the model I configured". +/// * `bytes = 0` can never be true, so the rule is permanently inert. +/// * An empty model sends every request of the run with no model id. +/// +/// Found by the adversarial review, which noted that this file already named the +/// empty-model outcome as the thing being avoided and the code then accepted it. +#[test] +fn a_threshold_that_could_only_misfire_is_refused() { + let escalating_at_zero = Settings { + escalate_after: Some(routing::Escalation { + failures: Some(0), + model: Some(STRONGER.to_string()), + }), + downshift_under: None, + }; + assert_eq!( + routing::routing(&escalating_at_zero), + Err(routing::Refusal::EscalatesBeforeAnythingFailed), + ); + + let never_downshifts = Settings { + escalate_after: None, + downshift_under: Some(routing::Downshift { + bytes: Some(0), + model: Some(CHEAPER.to_string()), + }), + }; + assert_eq!( + routing::routing(&never_downshifts), + Err(routing::Refusal::NeverDownshifts), + ); + + let nameless = Settings { + escalate_after: Some(routing::Escalation { + failures: Some(3), + model: Some(" ".to_string()), + }), + downshift_under: None, + }; + assert_eq!( + routing::routing(&nameless), + Err(routing::Refusal::NoModel { + rule: "escalate_after" + }), + ); +} + +/// A refusal leaves the run unrouted, and every refusal has a sentence. +/// +/// The pair of `contract::gate_notice`: a section that is plainly in the +/// operator's file and is not doing anything has to say why, or the surface that +/// lists it is lying by omission. +#[test] +fn every_refusal_says_which_key_is_wrong_and_that_the_turn_is_not_routed() { + for settings in [ + Settings { + escalate_after: Some(routing::Escalation { + failures: Some(0), + model: Some(STRONGER.to_string()), + }), + downshift_under: None, + }, + Settings { + escalate_after: None, + downshift_under: Some(routing::Downshift { + bytes: Some(0), + model: Some(CHEAPER.to_string()), + }), + }, + Settings { + escalate_after: Some(routing::Escalation { + failures: Some(3), + model: None, + }), + downshift_under: None, + }, + ] { + let said = routing::notice(&settings).expect("a refused section says why"); + assert!( + said.contains("app.io-cli.routing"), + "a refusal names the section the operator has to open: {said}", + ); + assert!( + said.contains("not routed"), + "a refusal says what the run does instead: {said}", + ); + } + + assert_eq!( + routing::notice(&both()), + None, + "an obeyable section has nothing to explain", + ); +} diff --git a/tests/status.rs b/tests/status.rs index 79e4e8a..5e12cd8 100644 --- a/tests/status.rs +++ b/tests/status.rs @@ -1253,6 +1253,59 @@ fn both_renderers(status: &Status) -> [(&'static str, String); 2] { ] } +/// **F1 — the effort level is on both renderers, and absent from both when unset.** +/// +/// Both, because a field added to `Status::fields` alone is green on +/// `Status::line` and nowhere the binary draws: `Status::render` takes the footer +/// on any terminal seven rows or taller. That is 0.12.0's planning field and +/// 0.8.0's spend field, and [`both_renderers`] exists because of them. +/// +/// The absent half is asserted as an absence rather than assumed. A default that +/// rendered `effort medium` would put a field on every operator's status line for +/// a release that changed nothing about their turns. +/// +/// Sabotage: draw the level on `Status::line` only — under which this fails on the +/// footer arm alone, which is exactly the shape of the two defects above. +#[test] +fn f1_the_effort_level_is_on_both_renderers_and_absent_until_it_is_set() { + let mut status = Status::new("a-model"); + + for (which, drawn) in both_renderers(&status) { + assert!( + !drawn.contains("effort"), + "{which} names an effort level in a session that has never set one: {drawn}", + ); + } + + status.effort = Some(io_harness::Effort::High); + for (which, drawn) in both_renderers(&status) { + assert!( + drawn.contains("effort high"), + "{which} does not carry the level every turn is buying: {drawn}", + ); + } +} + +/// **F1 — the level survives the turn it was set on.** +/// +/// `Status::forget_run` clears everything that was true of one run. A standing +/// choice is not one of those, and it is cleared beside them exactly as often as +/// somebody adds a field to that function without asking which kind it is — +/// `policy`, `budgets` and `planning` are all there for the same reason. +#[test] +fn f1_forgetting_a_run_does_not_forget_the_effort_level() { + let mut status = Status::new("a-model"); + status.effort = Some(io_harness::Effort::Low); + + status.forget_run(); + + assert_eq!( + status.effort, + Some(io_harness::Effort::Low), + "a level holds until `/effort` says otherwise, not until a run ends", + ); +} + /// A contract carrying all three budgets, the way an operator's `[run]` table /// leaves one. fn budgeted() -> TaskContract {