diff --git a/CHANGELOG.md b/CHANGELOG.md index aba8f14..04480e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,81 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.37.0] - 2026-09-04 + +The release that says where your context went. `/context` has read the real outgoing request +since 0.21.0, but it drew the tool catalogue as one number — so an operator who wanted to know +which of their bundles was expensive had no way to ask. It is now attributed: a row per MCP +server, named with the bundle that contributed it where a bundle did, and a `skill catalogue` +row for the block io-harness writes into the system prompt. That last row is the one that was +missing entirely; before this release a session's skills were counted inside `system block`, so +they appeared to cost nothing at all. `/plugin` and `/mcp` draw the same figure on the row where +you switch the thing on, from the same snapshot, so the two surfaces cannot disagree. + +The tiers are now stated rather than implied. Of the seven kinds of thing a bundle contributes, +three reach the wire — MCP servers, skills and agents — and four are free: hooks, templates, a +declared binary, and policy layers. A bundle that ships only hooks costs nothing on any request, +forever, and nothing told you that before. + +And `/context withhold ` takes a tool away and keeps it away until you say otherwise; +`/context allow ` puts that one back, and bare `/context allow` clears the whole mask. +This answers a limitation this product already published in its own words: a model that +reaches for `docx_write` in a session where you never meant a document to be written was stopped +by the write gate rather than by the absence of the tool. Now it can be stopped by name, before +anything starts. + +**A mask does not make a turn cheaper, and this release says so everywhere it could be +misread.** The roadmap entry this version was planned from assumed the opposite — that masking +would be the lever for cutting what a turn costs. io-harness 0.76.0's `ToolMask` is +*availability, never membership*: a masked run is offered a byte-identical catalogue, on purpose, +because the tool array sits ahead of the provider's cache breakpoint and dropping a definition +would save its tokens once and pay a cache *write* on every later turn. Withholding in fact makes +a request marginally larger, by one sentence naming the withheld tools, placed after the +observations where it costs no cache entry. So the two halves of this release do not compose: +attribution is the cost feature, masking is a scoping and safety feature, and no surface here +draws the second as the first. + +### Added + +- `/context withhold `, `/context allow ` and `/context allow` to clear the mask. + Verbs under an existing command rather than a thirty-seventh: `COMMANDS.len()` is still **36** + and the group occupancy is still `[7, 9, 10, 10]`, so the product's last free slot is still + free. Same shape as 0.29.0's marketplace verbs inside `/plugin`. +- A `skill catalogue` section on `/context`, located by the sentence io-harness writes above it. + The constant is gated against the pinned `run/prompts.rs` itself, so a pin that copy-edits that + sentence fails the build rather than silently reporting zero — which is what the two memory + heads beside it still risk, and which reads exactly like a session with no skills. +- A per-request token figure on `/plugin` and `/mcp` rows, and a statement of which contribution + kinds are free. + +### Changed + +- `/context`'s sections are seven rather than six, and MCP rows name the bundle that contributed + the server. A bundle's server is drawn in the spelling you read (`docs:search`) rather than the + wire spelling — the translation is applied only to a bundle's server, because a plugin id + cannot carry the separator while a bare `[[mcp]]` id from `io.toml` is validated by nobody and + may carry one legitimately. + +### Known limitations + +- **An agent roster is not attributed to the bundle that contributed it.** io-harness composes it + into the system block with no marker io-cli can locate. It is counted, inside `system block`; + it is not counted *against the bundle*. A number invented for it would be worse than none. +- **A mask does not reach a spawned child.** io-harness builds a child's contract fresh and + carries no mask into it, deliberately — the mask is a request about the operator's own turn. + So with `[app.io-cli.containment]` configured, a withheld tool can still be called inside the + fleet. This is upstream's boundary, not a gap here. +- **A server that has not been on a wire is drawn as unmeasured, not as zero.** The two mean + different things and a zero would report an unmeasured server as free. +- `/mcp` now carries two counts of different things — what the server announced at connect time + and what actually reached the wire on the last request. They can legitimately differ, and the + row says which is which rather than putting two bare numbers side by side. + +### Dependencies + +- io-harness stays at **0.76.0**, the newest published. The first release in eighteen where the + pin does not move. + ## [0.36.0] - 2026-09-03 The release that makes io reachable from an editor. The Agent Client Protocol is the field's @@ -3322,7 +3397,8 @@ 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.36.0...HEAD +[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.37.0...HEAD +[0.37.0]: https://github.com/initorigin/io-cli/compare/v0.36.0...v0.37.0 [0.36.0]: https://github.com/initorigin/io-cli/compare/v0.35.0...v0.36.0 [0.35.0]: https://github.com/initorigin/io-cli/compare/v0.34.1...v0.35.0 [0.34.1]: https://github.com/initorigin/io-cli/compare/v0.34.0...v0.34.1 diff --git a/Cargo.lock b/Cargo.lock index 4c5c1fe..6b64a04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.36.0" +version = "0.37.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index c2a2635..456fa9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.36.0" +version = "0.37.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/docs/CONTRACT.md b/docs/CONTRACT.md index 1215ae4..e0fd9bf 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -188,12 +188,18 @@ bare-host rule opens it. **Eleven commands run while a turn is in flight, and the rest are refused.** `/status`, `/context`, `/cost`, `/stats`, `/help`, `/theme`, `/copy`, `/expand`, `/fleet`, `/image` and -`/config` report while the agent works; `/` and `@` open the palette and path completion; +`/config` are admitted while the agent works; `/` and `@` open the palette and path completion; `/compact` and `/steer` reach the turn through their own arms as they always have. Every other command keeps its refusal, in the same sentence, and the rule is what a command *does* rather than how harmless it looks: anything that reassigns the session or the provider, writes the store or a configuration file, or submits a turn of its own is refused. +Ten of the eleven only report. **`/context withhold ` is the exception and it changes +something** — the session's tool mask — which is admitted because the moment you want a tool +withheld is the moment you watch the agent reach for it. It passes the rule above rather than +bending it: no file is written, nothing is reassigned, and the mask reaches the **next** turn +because this one's contract was built before the keystroke. + **`/config` is admitted bare and refused the moment it carries a word.** Through 0.32.0 it was refused in every form, because the bare list carried a row that re-read the provider's catalogue, wrote the prices into a scope file and reassigned the configuration the running turn was holding — diff --git a/docs/guide/accounting.md b/docs/guide/accounting.md index 3ab66b8..036e246 100644 --- a/docs/guide/accounting.md +++ b/docs/guide/accounting.md @@ -86,6 +86,49 @@ quotes USD and every provider it can connect to bills in it. Four decimals below unit and two above, in integer arithmetic — a turn costs a fraction of a cent and a month costs dollars, and one precision cannot show both. +### What your extensions cost, per request + +`/cost` is what a run spent. `/context` is what it is *carrying* — and since +0.37.0 it says where that came from rather than giving you one number for the +lot. The tool catalogue is split by what put each tool on the wire: a row per MCP +server, named with the bundle that contributed it where a bundle did, plus a +`skill catalogue` row for the block io-harness writes into the system prompt. That +last one is new; before 0.37.0 a session's skills were counted inside the system +block, so they appeared to cost nothing. + +`/plugin` and `/mcp` put the same figure on the row where you switch the thing on, +drawn from the same snapshot, so the two surfaces cannot tell you different +things about one server. + +**Not everything a bundle contributes costs anything, and the difference is +large.** Of the seven kinds of thing a bundle can contribute, three reach the +wire and four do not: + +| Contribution | Cost per request | +| --- | --- | +| MCP servers | every tool's name, description and JSON schema, every request | +| Skills | one line each — name and description. A body is read on demand by `read_skill` and only when the model asks | +| Agents | a roster in the system block | +| Hooks | nothing | +| Templates | nothing — io-cli expands them into your prompt | +| A declared binary | nothing | +| Policy layers | nothing | + +So a bundle that ships only hooks is free forever, and one that ships an MCP +server with forty tools is not. That is the decision `/plugin`'s cost column +exists to inform. + +Two honest gaps in the number. An **agent roster** is not counted against the +bundle that contributed it: io-harness composes it into the system block with no +marker io-cli can locate, and a number invented for it would be worse than none. +And a server that has not been on a wire yet is drawn as unmeasured rather than as +zero — the two mean different things, and only one of them is something you could +act on. + +**Withholding a tool does not reduce any of this.** See +[What this release is not](limits.md) — the catalogue sent is the same either way, +by design. The lever these numbers inform is turning a bundle or a server off. + --- [README](../../README.md) · [All guides](../CAPABILITIES.md) · [What you may depend on](../CONTRACT.md) diff --git a/docs/guide/commands.md b/docs/guide/commands.md index c7c3204..468b23b 100644 --- a/docs/guide/commands.md +++ b/docs/guide/commands.md @@ -40,7 +40,7 @@ above a row that ranked there for reasons having nothing to do with it. | --- | --- | | `/help` | this table | | `/status` | commit the whole session state into the scrollback | -| `/context` | what is in the model's window, read from the request that carried the turn | +| `/context` | what is in the model's window, read from the request that carried the turn, and withhold or allow a tool by name | | `/expand` | commit the last step's full detail into the scrollback | | `/fleet` | show the children this turn has spawned | | `/skills` | every skill, shipped or yours: what it is for, whether it is on, and its file; add and remove one | diff --git a/docs/guide/keys.md b/docs/guide/keys.md index d3e5211..70c936a 100644 --- a/docs/guide/keys.md +++ b/docs/guide/keys.md @@ -156,10 +156,16 @@ the defaults that shipped, and marks `Ctrl+C` as fixed. ## While a turn is running -**Eleven commands report while the agent works**, and they are `/status`, +**Eleven commands run while the agent works**, and they are `/status`, `/context`, `/cost`, `/stats`, `/help`, `/theme`, `/copy`, `/expand`, `/fleet`, `/image` and `/config`. +Ten of them only report. `/context withhold ` is the one that changes +something, and it is admitted because watching the agent reach for a tool is +exactly when you know you want it withheld. It takes effect on the **next** turn +— this one's contract was built before you typed — and it writes no file, which +is the test every command on this list has to pass. + The palette opens mid-turn too, and path completion with it — the `/` and the `@` in the table above. `/compact` and `/steer` reach the running turn through their own arms, as they always have, and are not part of this set. diff --git a/docs/guide/limits.md b/docs/guide/limits.md index c4b45c5..2b42298 100644 --- a/docs/guide/limits.md +++ b/docs/guide/limits.md @@ -76,7 +76,9 @@ and the agent may decide to use it on a model that cannot see — and io-cli can take a tool out of io-harness's own workspace tool set. Checking the model rather than the provider would mean reading the live catalogue on every attach, and it would still not close the door the agent opens. **If you work with images, choose -a model that accepts them.** +a model that accepts them.** From 0.37.0 you can also close that particular door +for a turn with `/context withhold view_image`, which refuses the call rather +than removing the tool — see below. **The twelve document tools cannot be taken out of that tool set either**, and six of them write. A model that reaches for `docx_write` in a session where you @@ -84,6 +86,32 @@ never meant a document to be written is stopped by the write gate rather than by the absence of the tool — which is what the gate is for, and why the writers are named one by one in [Documents](media.md#documents) rather than counted. +**Since 0.37.0 you can stop it earlier, by name.** `/context withhold docx_write` +refuses that tool before anything is started, and it stays refused until you say +otherwise — `/context withhold` is a posture, not a one-shot. `/context allow +docx_write` puts that one back; **bare `/context allow` clears the whole mask**, +so name the tool unless dropping every one of them is what you meant. Both +sentences above stay true: the tool is not taken out of the +set, the catalogue the model is offered is byte for byte the one it would have +been offered anyway, and the definition still costs its tokens on every request. +What a mask changes is what may be *called*. + +**A mask does not make a turn cheaper, and it is worth saying so plainly because +it is the obvious thing to assume.** io-harness sends the identical catalogue +either way on purpose — the tool array sits ahead of the provider's cache +breakpoint, so dropping a definition would save its tokens once and pay a cache +*write* on every later turn of the run. Withholding in fact makes a request +marginally larger: one sentence naming the withheld tools, placed where it costs +no cache entry. Use it to bound what the agent may do, and use `/context` and +`/plugin` to see what your extensions cost — those are two different questions +and only the second one is about money. + +**A mask does not reach a spawned child.** With `[app.io-cli.containment]` +configured, a contained turn's children get contracts io-harness builds fresh, +and it carries no mask into them deliberately — the mask is a request about your +turn, and a child's work is not your turn. So a tool you withheld can still be +called inside the fleet. See [The fleet](fleet.md). + **An image the agent was *given* rather than asked for is not shown.** A picture returned by an MCP tool, and a browser screenshot, both become images inside io-harness — but through private plumbing and with no event of any kind, so diff --git a/docs/guide/media.md b/docs/guide/media.md index 1e423e4..41e1ce7 100644 --- a/docs/guide/media.md +++ b/docs/guide/media.md @@ -40,6 +40,14 @@ twelve more of them: **io-cli cannot take a tool out of io-harness's workspace tool set**, so a feature this crate turns on is a tool the agent has, and the only honest thing to do with that is say so. See [Documents](#documents). +That is still true in 0.37.0, and it is worth being exact about what changed +beside it. `/context withhold ` does not remove a tool from the set — the +catalogue the model is offered is byte for byte the same one — it withholds +*permission to call it* for the turn, and a call is refused before anything is +started. So the tool is still there, still costs its definition's tokens, and the +agent is told in the same request that asking for it will do nothing. See +[What this release is not](limits.md). + A picture is drawn from half blocks — `▀` splits a cell into two halves that are each about square — fitted to your terminal's width and bounded in height. On kitty, ghostty, WezTerm and Konsole a PNG is drawn as the **real image** instead, diff --git a/src/commands.rs b/src/commands.rs index 5ca47a3..2800ace 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -152,7 +152,8 @@ pub const COMMANDS: &[(&str, &str)] = &[ // ask the question this answers. ( "/context", - "what is in the model's window, read from the request that carried the turn", + "what is in the model's window, read from the request that carried the turn, \ + and withhold or allow a tool by name", ), ( "/steer", @@ -1119,7 +1120,14 @@ pub enum Action { /// the catalogue on this page includes tools io-cli never registered — /// because it is the catalogue the model was given rather than the one this /// crate believes it asked for. - Context, + /// + /// **`None` is the page and a `Some` is a verb on the mask**, which is the + /// shape [`Action::Store`] already has and for the same reason: the bare form + /// is the only one that changes nothing, and a verb that could not be read must + /// report rather than quietly drawing the page. The mask the verbs edit is what + /// [`crate::contract::masking`] puts on the next turn's contract — a scoping + /// lever, not a cost one; see [`Masked`]. + Context(Option), /// Put something on the system clipboard over OSC 52. Copy(Copied), /// Ask the agent to describe the work this turn did, and to commit it. @@ -1394,6 +1402,56 @@ pub enum Copied { Diff, } +/// What a `/context` line asked for beyond the page. +/// +/// **Verbs under an existing command rather than a command of their own**, which +/// is 0.29.0's answer repeated: `marketplace` became words inside `/plugin` +/// because a thirty-seventh row buys a reader nothing that a second word on a row +/// they already know does not. `COMMANDS` stays at thirty-six and the four groups +/// stay at seven, nine, ten and ten — the last two are at the bound, and a command +/// added here is a command with nowhere to go. +/// +/// **The mask is a scoping and safety lever, never a cost lever.** io-harness +/// sends a byte-identical tool catalogue whether or not a mask is set — the tool +/// array sits ahead of a cache breakpoint, so dropping a definition would pay a +/// cache write on every later turn — and a mask *adds* a sentence to the user +/// prompt naming what was withheld. Withholding three tools costs marginally more +/// than not withholding them. [`crate::contract::masking`] carries the long form +/// of that, and no sentence this surface prints may suggest otherwise. +/// +/// The refusal variants are variants rather than an `Err`, for [`Keep`]'s reason: +/// they are answers this surface gives in its own words, and a verb that failed to +/// parse must never fall through to the page — somebody who typed a withhold and +/// got a report would believe the tool was withheld. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Masked { + /// `/context withhold ` — one more name the next turn may not call. + Withhold(String), + /// `/context allow ` — that name back out of the mask. + Allow(String), + /// `/context allow` with nothing after it — the whole mask dropped. + /// + /// **Bare `allow` clears, and it is not guarded.** The considered alternative + /// was [`Keep::NoDate`]'s: refuse a verb that names nothing, so an Enter + /// pressed one word early cannot undo a posture. It is refused *there* because + /// `/store sweep` deletes sessions and nothing brings them back. Here the whole + /// state is a set of names in this process: no file is written, the page one + /// keystroke away lists what is withheld, the driver's own line names every + /// tool re-offered, and putting the mask back is the command that built it. + /// A refusal would cost a word on every deliberate use to save a keystroke on + /// an accident that announces itself and is reversible in one command. + /// + /// `withhold` with no name is [`Masked::NoTool`] and not the mirror of this: + /// "withhold everything" is not what an interrupted line means, and guessing it + /// would be this surface taking away tools nobody named. + Clear, + /// `withhold` or `allow` with no tool named — `allow`'s bare form is + /// [`Masked::Clear`], so this is `withhold`'s alone. + NoTool, + /// A second word that is neither verb, carried so the refusal can name it. + Unknown(String), +} + // --------------------------------------------------------------------------- // The memory page, and the scope question `/remember` asks // --------------------------------------------------------------------------- @@ -2066,6 +2124,32 @@ fn profile_verb(input: &str) -> ProfileVerb { } } +/// The verb on a `/context` line, or `None` for the bare page. +/// +/// `hide` is admitted beside `withhold` and `unhide` beside `allow`, under the +/// rule `/plugins` and `/gates` already follow: both words are in the field's +/// vocabulary and refusing the other spelling teaches nobody anything. Neither +/// spelling is a second surface — both edit the one mask. +fn mask_verb(input: &str) -> Option { + let mut words = input.split_whitespace(); + // The command itself. + let _ = words.next(); + let verb = words.next()?; + let name = words.next(); + match (verb.to_ascii_lowercase().as_str(), name) { + ("withhold" | "hide", Some(tool)) => Some(Masked::Withhold(tool.to_string())), + ("withhold" | "hide", None) => Some(Masked::NoTool), + ("allow" | "unhide", Some(tool)) => Some(Masked::Allow(tool.to_string())), + // Bare `allow` drops the whole mask; `Masked::Clear` says why that is not + // guarded and why `withhold`'s bare form is not its mirror. + ("allow" | "unhide", None) => Some(Masked::Clear), + // **`verb` and not the lowercased match subject**, the trap `profile_verb` + // records: the refusal quotes this back, and quoting `Withhold` at somebody + // who typed `WITHHOLD` sends them looking for a word they did not write. + _ => Some(Masked::Unknown(verb.to_string())), + } +} + pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { match input.split_whitespace().next().unwrap_or("help") { "help" | "?" => Action::Print(help(keys, theme, Newline::here())), @@ -2416,7 +2500,26 @@ pub fn parse(input: &str, keys: &Keys, theme: &Theme) -> Action { // name for a surface nobody has typed yet is a name to keep working // forever in exchange for nothing. "status" => Action::Status, - "context" => Action::Context, + // **Verbs on the page, not a command of their own.** `COMMANDS` carries one + // `/context` row and thirty-six rows in total; the mask is reached by a + // second word on the row an operator already knows, which is what 0.29.0 + // did with `/plugin marketplace` and for the same reason — the two biggest + // groups are at the ten-per-group bound, so a new command has nowhere to be + // filed. + // + // **The tool name is the third word and nothing after it is read.** A tool + // name is one token — `write_file`, `browser_click` — unlike a profile + // name, which `profile_verb` joins because a profile may be called `slow + // ci`. Joining here would let a stray word become part of a name that no + // tool answers to, and io-harness keeps such a name rather than rejecting + // it, so the mistake would be silent. + // + // Lowercased before matching, for the reason `/effort` and `/profile` + // record: matching literally made one spelling of a word work and another + // fall through. The name itself is NOT lowercased — a tool name is what the + // catalogue calls it, and folding its case would build a mask that withholds + // nothing. + "context" => Action::Context(mask_verb(input)), // `/clear` and `/new` mean the same thing, for the reason `/resume` and // `/continue` do: both words are in the field's vocabulary and a reader // arrives having been taught one of them by another agent. @@ -2597,6 +2700,16 @@ fn table>(rows: &[(S, S)], width: usize, theme: &Theme) -> Vec BTreeMap { + let mut out = BTreeMap::new(); + for tool in &seen.tools { + if !tool.name.starts_with(MCP_TOOL_PREFIX) { + continue; + } + *out.entry(server_of(&tool.name, contract)).or_insert(0) += + estimate_tokens(&tool_text(tool)); + } + out +} + +/// What each bundle cost this request, by plugin id. +/// +/// **Two of the three costing kinds, and the third is named rather than guessed +/// at.** Of the seven things a bundle contributes, only skills, agents and MCP +/// servers reach the wire — hooks, templates, a declared binary and a policy layer +/// cost nothing at all, which is the fact an operator least expects and most needs. +/// This sums a bundle's MCP tools and its catalogue lines. An agent definition is +/// **not** included: io-harness composes a roster into the system block with no +/// marker this module can locate, so a number for it would be invented. That is +/// stated as a limitation rather than approximated. +/// +/// A bundle's skills are namespaced `__` before they reach the +/// catalogue, so a catalogue line attributes itself and no second lookup is needed. +pub fn bundle_cost(seen: &Request, contract: &TaskContract) -> BTreeMap { + let per_server = server_cost(seen, contract); + let mut out = BTreeMap::new(); + for plugin in contract.plugins.iter() { + let tools: u64 = plugin + .mcp_servers() + .iter() + .filter_map(|s| per_server.get(&s.id)) + .sum(); + let skills: u64 = skills_in(&seen.system) + .into_iter() + .flat_map(str::lines) + .filter(|line| { + line.strip_prefix("- ") + .and_then(|rest| rest.strip_prefix(plugin.id())) + .is_some_and(|rest| rest.starts_with(NAMESPACE)) + }) + .map(estimate_tokens) + .sum(); + // Present at zero, unlike a server: a bundle contributing only hooks is + // genuinely free, and that is the row worth drawing rather than hiding. + out.insert(plugin.id().to_string(), tools + skills); + } + out +} + +/// What `/context withhold` and `/context allow` do to the mask, and what is said +/// about it (0.37.0). +/// +/// **A function of the mask rather than a method on the driver**, for the reason +/// [`crate::contract::buying`] gives about itself: nothing under `tests/` links +/// `src/main.rs`, so a transition written there could be neither asserted nor +/// sabotaged. The driver takes the new mask and prints the line. +/// +/// The returned sentence is where this release is most able to lie, so it is +/// written once, here, and said in the direction that is true: withholding costs a +/// little more rather than saving anything. A reader who is told a tool was +/// withheld will assume the request got smaller unless told otherwise, and the +/// assumption is the one the roadmap this release was planned from also made. +/// `offered` is the catalogue the last request actually carried, and it is a +/// parameter because a name that answers to nothing is a mask that does nothing. +/// io-harness **keeps** an unknown name rather than rejecting it, deliberately, so +/// that a mask written against one build stays portable to another with different +/// cargo features — which means the harness cannot warn and this is the only place +/// that can. Withholding a misspelling therefore succeeds, silently, and leaves the +/// tool fully callable: `mask_gate` matches on the exact name. +/// +/// So the name is still added — refusing it here would break the portability the +/// harness designed for — and the operator is told it matches nothing on the wire. +/// The `Allow` arm has always refused to be a silent no-op; this is the same +/// courtesy on the direction where the cost of not having it is a safety lever the +/// operator believes is on. +pub fn withhold( + mask: &io_harness::ToolMask, + said: &crate::commands::Masked, + offered: &[String], +) -> (ToolMask, String) { + use crate::commands::Masked; + + let mut names: Vec = mask.names().map(str::to_string).collect(); + match said { + Masked::Withhold(tool) => { + if names.iter().any(|n| n == tool) { + let line = format!("{tool} is already withheld until you allow it again"); + return (ToolMask::withholding(names), line); + } + names.push(tool.clone()); + // **Only when a catalogue is known.** Before the first turn `offered` + // is empty, and warning that every name is unrecognised when nothing + // has been offered yet would be noise that trains the reader to ignore + // the warning that matters. + let unknown = !offered.is_empty() && !offered.iter().any(|name| name == tool); + let line = if unknown { + format!( + "{tool} is withheld, but no tool of that name was in the last request — \ + check the spelling against /context, which lists what was offered. \ + A mask entry that matches nothing withholds nothing. {} withheld now.", + names.len() + ) + } else { + // Said in full every time rather than only on the first withhold: + // the cost direction is the thing an operator is most likely to + // have wrong, and a note that appears once is a note most people + // never see. + format!( + "{tool} is withheld until you allow it again — the model is still offered \ + it and it still costs its definition, and calling it will be refused \ + before anything starts. {} withheld now.", + names.len() + ) + }; + (ToolMask::withholding(names), line) + } + Masked::Allow(tool) => { + let before = names.len(); + names.retain(|n| n != tool); + let line = if names.len() == before { + // Never silently a no-op: an operator who mistyped a name and got + // nothing back would believe the tool was allowed again. + format!("{tool} was not withheld, so nothing changed") + } else { + format!("{tool} may be called again") + }; + (ToolMask::withholding(names), line) + } + // Every tool named, because this is the one transition that changes + // several things at once and an operator who typed it one word early is + // owed the list of what came back. + Masked::Clear => { + let line = if names.is_empty() { + "nothing was withheld".to_string() + } else { + format!("no tool is withheld now — {} may be called again", { + names.sort(); + names.join(", ") + }) + }; + (ToolMask::none(), line) + } + Masked::NoTool => ( + ToolMask::withholding(names), + "withhold needs a tool to withhold — /context withhold docx_write. \ + /context lists what the model was offered." + .to_string(), + ), + Masked::Unknown(word) => ( + ToolMask::withholding(names), + format!("{word} is not a verb here — /context takes withhold or allow"), + ), + } +} + +/// The names the last request offered, for [`withhold`] to check a spelling +/// against. +/// +/// Empty before any turn has run, which [`withhold`] reads as "no catalogue is +/// known" rather than as "nothing is offered" — the two must not collapse, or +/// every name typed at a fresh prompt would be reported as a misspelling. +pub fn offered(seen: Option<&Request>) -> Vec { + seen.map(|request| request.tools.iter().map(|t| t.name.clone()).collect()) + .unwrap_or_default() +} + +/// What the mask is, drawn for the page. +/// +/// Absent entirely when nothing is withheld, because a row saying "nothing is +/// withheld" on every `/context` is a row that trains a reader to skip the place +/// the answer appears. +pub fn withheld_line(mask: &io_harness::ToolMask, dash: &str) -> Option { + if mask.is_empty() { + return None; + } + let names: Vec<&str> = mask.names().collect(); + // **"until you allow it again", never "for the next turn".** io-harness + // documents `ToolMask` as a request about one turn, and it is — but io-cli + // re-applies this one to every turn until `/context allow`, so the harness's + // wording describes the contract's lifetime and would describe the operator's + // posture wrongly. A reader told "the next turn" reasonably stops thinking + // about it, which is the 0.26.0 `/effort` defect in the opposite direction. + Some(format!( + "withheld: {} {dash} refused until you allow it again. The catalogue above is \ + unchanged: withholding costs one extra sentence and saves nothing.", + names.join(", ") + )) +} + /// The text one tool occupies on the wire. /// /// Name, description and schema, because all three are sent. A catalogue counted @@ -221,13 +496,23 @@ fn server_of(tool: &str, contract: &TaskContract) -> String { Some(bare) => bare, None => return String::new(), }; - for server in &contract.mcp { - if bare - .strip_prefix(&server.id) - .is_some_and(|r| r.starts_with("__")) - { - return server.id.clone(); - } + // **The LONGEST matching id, not the first.** `contract.mcp` lists the + // operator's own `[[mcp]]` entries before the plugin servers `Plugins::apply_to` + // appends, and a bare id is validated by nobody — so `github` and + // `github__enterprise` can both be configured, and a first-match loop charges + // every enterprise tool to `github` while `github__enterprise` reads + // "not yet on a request" for the life of the session. The same shape fires when + // an operator's bare `docs` collides with a bundle `docs` contributing `search`. + if let Some(server) = contract + .mcp + .iter() + .filter(|server| { + bare.strip_prefix(&server.id) + .is_some_and(|r| r.starts_with("__")) + }) + .max_by_key(|server| server.id.len()) + { + return server.id.clone(); } bare.split_once("__") .map(|(server, _)| server.to_string()) @@ -283,7 +568,75 @@ fn memory_in(haystack: &str) -> Option<&str> { Some(&haystack[at..end]) } -/// The six sections of one request, in the order the page draws them. +/// The region of `haystack` occupied by the skill catalogue (0.37.0). +/// +/// `with_skill_catalog` appends one paragraph naming what a skill is and how to +/// read one, then a newline, then `Skills::catalog()` — which is one `- name: +/// description` line per skill and nothing else. So the block is the head +/// paragraph plus the `- ` lines that follow it, and it ends at the first line +/// that is neither. That is the shape [`memory_in`] already scans for, and it is +/// deliberately the same shape rather than a second technique. +/// +/// `None` means this request carried no catalogue — a session with no skills +/// configured — and the page says so rather than drawing a zero, because a zero +/// here is exactly what a drifted needle produces and the two must stay +/// distinguishable. +/// +/// **Why this is not `Skills::catalog()` used as the needle.** That was the plan +/// this release was specified with. It needs a `Skills` value, and +/// `TaskContract::skills` is an `Option` — a directory — so io-cli would +/// have to run `Skills::discover` a second time to build one. That is a second +/// walk of the operator's disk on every draw, and worse, a second opinion about a +/// set the run already resolved: the two could disagree, and the page would report +/// the one that is not on the wire. Locating io-harness's own output is the read +/// this module is built on everywhere else. +fn skills_in(haystack: &str) -> Option<&str> { + let at = haystack.find(SKILLS_HEAD)?; + let mut end = at; + for (n, line) in haystack[at..].split_inclusive('\n').enumerate() { + let trimmed = line.trim_end_matches('\n'); + // The head paragraph is one line; every line after it is a catalogue + // entry. A catalogue with no entries cannot occur — `with_skill_catalog` + // returns `base` untouched when the set is empty. + if n > 0 && !trimmed.starts_with("- ") { + break; + } + end += line.len(); + } + let block = &haystack[at..end]; + // And cut the planning directive back off the last line, where io-harness + // appended it with no newline. See [`PLAN_DIRECTIVE_HEADS`] — without this the + // row over-reports by the directive's length and one bundle is charged for it. + let cut = PLAN_DIRECTIVE_HEADS + .iter() + .filter_map(|head| block.find(head)) + .min() + .map(|at| block[..at].trim_end().len()) + .unwrap_or(block.len()); + Some(&block[..cut]) +} + +/// The bundle that contributed `server`, or `None` for one the operator declared. +/// +/// `Plugins::apply_to` extends `contract.mcp` with every loaded plugin's servers +/// and stores itself on `contract.plugins`, so the contract already knows this and +/// nothing needs resolving. Matched on the id rather than on the separator: a +/// plugin id cannot contain `__` (io-harness validates it to +/// `[a-z0-9][a-z0-9-]{0,31}`) but a bare `[[mcp]]` id from `io.toml` is validated +/// by nobody and may. +/// +/// This reads the `plugins` **field**, never `Config::plugins()`, which is a full +/// parse of every declared manifest and is confined to `crate::resolved` by an +/// exact-path gate in `tests/dependencies.rs`. +fn bundle_of<'a>(server: &str, contract: &'a TaskContract) -> Option<&'a str> { + contract + .plugins + .iter() + .find(|plugin| plugin.mcp_servers().iter().any(|s| s.id == server)) + .map(|plugin| plugin.id()) +} + +/// The seven sections of one request, in the order the page draws them. /// /// Every one is a region of `seen` and no two overlap, so /// [`total`] is a statement about the request rather than a sum of unrelated @@ -294,13 +647,20 @@ fn memory_in(haystack: &str) -> Option<&str> { pub fn sections(seen: &Request, contract: &TaskContract) -> Vec
{ let mut out = Vec::new(); - // 1 and 2 — the system block, split at the guidance io-cli put in it. + // 1, 2 and 3 — the system block, split at the guidance io-cli put in it and + // at the skill catalogue io-harness appended to it. Both are subtracted from + // the block rather than reported beside it, or the same bytes would be + // counted twice and the total would exceed the request. let guidance = instructions_in(&seen.system, &contract.instructions); let guidance_tokens = guidance.map(estimate_tokens).unwrap_or(0); + let skills = skills_in(&seen.system); + let skill_tokens = skills.map(estimate_tokens).unwrap_or(0); out.push(Section { label: "system block".into(), detail: "what io-harness composed for this turn".into(), - tokens: estimate_tokens(&seen.system).saturating_sub(guidance_tokens), + tokens: estimate_tokens(&seen.system) + .saturating_sub(guidance_tokens) + .saturating_sub(skill_tokens), }); out.push(Section { label: "repository instructions".into(), @@ -313,6 +673,24 @@ pub fn sections(seen: &Request, contract: &TaskContract) -> Vec
{ }, tokens: guidance_tokens, }); + out.push(Section { + label: SKILL_CATALOGUE.into(), + detail: match skills { + // Counted off the block on the wire, not off the skills io-cli can + // see on disk: a skill added since the last turn is not in this + // request, and the page is about this request. + // + // One line per skill, name and description, never a body — a body is + // loaded on demand by `read_skill`, which is why a session with + // twenty skills pays for twenty *lines* here and not twenty files. + Some(block) => format!( + "{} skill(s) named, bodies read on demand", + block.lines().filter(|l| l.starts_with("- ")).count() + ), + None => "no skill was named to this request".into(), + }, + tokens: skill_tokens, + }); // 3 and 4 — the catalogue, split at the MCP prefix. let (from_mcp, workspace): (Vec<&ToolSpec>, Vec<&ToolSpec>) = seen @@ -346,9 +724,27 @@ pub fn sections(seen: &Request, contract: &TaskContract) -> Vec
{ }); } for (id, tools) in &servers { + // A bundle's server is drawn in the spelling an operator reads and named + // with the bundle that brought it, because "where did this come from" is + // the question a cost row exists to answer. `naming::display` is applied + // only to a server a bundle contributed: a plugin id cannot carry the + // separator, so the translation is unambiguous there, while a bare + // `[[mcp]]` id is validated by nobody and may carry one legitimately. + let bundle = bundle_of(id, contract); + let shown = match bundle { + Some(_) => crate::naming::display(id), + None => id.clone(), + }; out.push(Section { - label: format!("mcp {id}"), - detail: format!("{} tool(s) offered", tools.len()), + label: format!("mcp {shown}"), + detail: match bundle { + Some(plugin) => format!( + "{} tool(s) offered, from the {} bundle", + tools.len(), + crate::naming::display(plugin) + ), + None => format!("{} tool(s) offered", tools.len()), + }, tokens: tools .iter() .map(|tool| estimate_tokens(&tool_text(tool))) @@ -412,6 +808,11 @@ pub fn committed( seen: Option<&Request>, contract: &TaskContract, remaining: Option, + // What the next turn may not call. Drawn on this page because this is where + // the tools are named, and an operator who withheld one an hour ago should + // not have to remember it — but drawn only when there is one, so the row is + // information rather than furniture. + mask: &io_harness::ToolMask, theme: &Theme, width: u16, ) -> Vec> { @@ -474,6 +875,14 @@ pub fn committed( } } + // Below the catalogue and not above it, because it is a statement *about* the + // list of tools directly above — and drawn whether or not a turn has run, so + // that a mask set at the prompt before the first message is visible rather + // than silently in force. + if let Some(line) = withheld_line(mask, dash) { + push(line, Tone::Refused); + } + lines.push(Line::from(Span::styled( format!("{rule}{rule}{rule} context ends"), theme.style(Tone::Accent), diff --git a/src/contract.rs b/src/contract.rs index a561e08..da498b3 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -389,6 +389,52 @@ pub fn buying(contract: TaskContract, effort: Option) -> Tas } } +/// Apply what `/context withhold` said to the contract of a turn that is about to +/// run. +/// +/// **A sibling of [`buying`] rather than a field set inside [`session`], and the +/// argument is [`buying`]'s own, unchanged.** Three of that builder's callers make +/// a contract nothing runs — the startup reading and the two reporting pages — and +/// a mask is a statement about *one turn*, not about the session that owns them. +/// io-harness draws the same line: `ToolMask` is documented as a request about one +/// turn, beside `TaskContract::fold_now`, not as a setting that governs a run. So +/// this is applied where a turn is, which is one call in the driver, and never at +/// the point a page is built. +/// +/// **A function and not a line in the driver**, for [`buying`]'s reason and no +/// other: nothing under `tests/` links `src/main.rs`, so a conditional written +/// there could not be asserted or sabotaged. The decision is here; the driver +/// holds one call. +/// +/// **An empty mask returns the contract untouched, and the early return is the +/// point.** `TaskContract::tool_mask` is a plain `ToolMask` whose default is empty +/// — not an `Option` — so calling `with_tool_mask(ToolMask::none())` would in fact +/// produce an identical contract today. Returning early makes that a property of +/// this function rather than a coincidence of io-harness's default, which is what +/// keeps `tests/contract.rs`'s +/// `f2_nothing_configured_is_the_contract_the_session_built_before` — a Debug +/// equality over the whole contract — **structurally untouched by this release** +/// rather than merely still green: a session that never says `/context withhold` +/// has no call to this that can change anything, and the gate does not depend on +/// what io-harness chose for an unset field. +/// +/// **Masking is a scoping and safety lever, never a cost lever, and nothing in +/// this crate may imply otherwise.** io-harness sends a byte-identical tool +/// catalogue whether or not a mask is set — deliberately, because the tool array +/// sits ahead of a cache breakpoint and removing a definition would save its +/// tokens once and pay a cache *write* on every later turn (`src/tools/mod.rs:40`). +/// A mask in fact **adds** a sentence to the user prompt naming the withheld tools +/// (`src/run/prompts.rs:976`, `withheld_sentence`), placed after the observations +/// precisely so it costs no cache entry. A turn that withholds three tools is +/// marginally more expensive than the same turn without the mask, not less. +#[must_use] +pub fn masking(contract: TaskContract, mask: &io_harness::ToolMask) -> TaskContract { + if mask.is_empty() { + return contract; + } + contract.with_tool_mask(mask.clone()) +} + /// The criterion this configuration resolves to, with its reviewer already built. /// /// `None` covers every case in which the run must not be gated: no section, a diff --git a/src/main.rs b/src/main.rs index dd9927c..b246537 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1104,6 +1104,19 @@ async fn loop_over Result>( // 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; + // **What the next turn may not call — a posture, exactly like `effort` above.** + // Read by each turn and cleared by nothing but `/context allow`, session state + // rather than configuration: there is no `[app.io-cli]` key for it, because a + // mask an operator sets while watching a turn is an answer to what is + // happening now and not a standing preference. + // + // **It withholds permission, never a definition.** io-harness offers a masked + // turn a byte-identical catalogue on purpose — the tool array sits ahead of the + // provider's cache breakpoint, so removing one would save its tokens once and + // pay a cache write on every later turn — and it appends one sentence naming + // what is withheld. So this makes a request marginally larger and never + // smaller, and no line this driver prints may suggest otherwise. + let mut mask = io_harness::ToolMask::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 @@ -1532,6 +1545,7 @@ async fn loop_over Result>( &capabilities, &seen, effort, + &mask, run_id, pending, ) @@ -4996,7 +5010,23 @@ async fn loop_over Result>( } } Action::Mcp => { - let list = io_cli::servers::servers(&config, &app.servers); + let mut list = io_cli::servers::servers(&config, &app.servers); + // **What each server's tools weighed on the last request.** + // Read off `opening` for the reason `/compact` reads + // `opening.compaction` rather than building a contract of its + // own: `tests/contract.rs` counts `contract::session` callers + // and fails at five, and nothing in the file can move a + // server between two turns of one session anyway. + // + // A server with no entry keeps `None` and draws "not yet on a + // request", which is not the same claim as zero. + io_cli::servers::costed( + &mut list, + &io_cli::context::server_cost( + &seen.latest().unwrap_or_default(), + &opening, + ), + ); if list.is_empty() { // The "not configured" shape this product uses // everywhere: an empty section is not an error. @@ -5243,7 +5273,29 @@ async fn loop_over Result>( // the operator has just edited on disk is reflected without a // reload — and a bundle they have just broken shows up as a // refusal in the same breath. + // **`with_costs`, and it is the whole reason the column is not + // decoration.** `view()` has ten call sites that draw no cost, + // so the figure is attached here rather than taken as a + // parameter — which means this one line is the only thing + // between a working column and 0.31.0's shape, where every row + // the release added was listable and the pipeline behind it had + // no caller at all behind a fully green suite. + // **Costs attached only when a request has actually gone out.** + // `bundle_cost` lists every loaded bundle, so a `Request::default()` + // stand-in returns a map of real zeroes — and a zero on this + // column says "this bundle is free", which is the one claim that + // stops an operator looking. `/mcp` gets this right for free + // because an unmeasured server is simply absent from its map; + // `/plugin`'s rows are keyed the other way and need the decision + // made here. Without it the two surfaces disagree on the first + // draw of a session, which is exactly what they exist not to do. let view = io_cli::pluginview::view(holdings.loaded()); + let view = match seen.latest() { + Some(request) => { + view.with_costs(io_cli::context::bundle_cost(&request, &opening)) + } + None => view, + }; // **The same call `/skills` makes, deliberately.** A bundle // whose declared skills directory is absent ends every turn of // the session, and both surfaces have to say so — but saying @@ -6409,7 +6461,14 @@ async fn loop_over Result>( fold_next = said == io_cli::compact::Said::Armed; app.say(Tone::Muted, said.line(dash)); } - Action::Context => { + // **One variant carrying the verb, rather than a second `Action`.** + // The mid-turn match below ends in a `_ => debug_assert!(false, …)`, + // so a new variant would compile, pass every debug test and be + // silently inert mid-turn in a release build — which is the door + // an operator most wants this on. Changing this variant's shape + // made both sites compile errors instead, which is how they were + // found. + Action::Context(None) => { // `reading` for the same reason the arm above binds it that // way — `tests/plan.rs` reads the plan-gate argument off a // binding called `contract`, and this is not a turn's. @@ -6427,11 +6486,25 @@ async fn loop_over Result>( seen.latest().as_ref(), &reading, reading.max_tokens, + &mask, &app.theme, screen.width(), ); screen.commit(&lines).map_err(|error| error.to_string())?; } + Action::Context(Some(said)) => { + // The transition and the sentence are both `context::withhold`'s + // — nothing under `tests/` links this binary, so a rule written + // here could be neither asserted nor sabotaged. + // The catalogue the last request carried, so a misspelling is + // caught here — io-harness keeps an unknown name deliberately, + // to keep a mask portable across feature sets, so nothing + // downstream can warn. + let offered = io_cli::context::offered(seen.latest().as_ref()); + let (next, line) = io_cli::context::withhold(&mask, &said, &offered); + mask = next; + app.say(Tone::Muted, line); + } Action::Copy(what) => { let last = last_run(&session, &store); let (payload, said) = to_copy(last.as_ref(), &store, what); @@ -6695,6 +6768,10 @@ async fn loop_over Result>( // `mem::take` here would make every level last exactly one // turn — the defect F1's sabotage names. effort, + // Borrowed rather than read, unlike `effort` above: this + // turn's own `/context withhold` writes through it, and + // what it writes governs the turn after this one. + &mut mask, text, // **This turn's own clock, not the session's.** What a // reader wants of the row above the prompt is how long the @@ -7246,6 +7323,14 @@ async fn turn( // than a fourth level, so a session that never types `/effort` sends the // request body this crate sent before 0.26.0. effort: Option, + // **What this turn may not call — borrowed rather than copied, because + // `/context withhold` runs mid-turn and this is the session's posture.** + // Beside `effort` because it is the same kind of thing, and unlike it in the + // one way that matters: a level cannot change while a turn runs and a mask + // can, since watching the agent reach for a tool is exactly when an operator + // knows they want it withheld. A change here reaches the *next* turn — this + // one's contract was built before the keystroke. + mask: &mut io_harness::ToolMask, text: String, started: Instant, ) -> Result { @@ -7391,6 +7476,12 @@ async fn turn( // 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); + // **What `/context withhold` last said, applied for `buying`'s own reason.** + // A mask withholds permission and never a definition: the catalogue this + // contract carries to the wire is byte for byte the one an unmasked turn + // carries, and io-harness appends one sentence naming what is withheld. So + // this line makes the request marginally larger and can never make it smaller. + let contract = io_cli::contract::masking(contract, mask); // 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. @@ -7922,16 +8013,33 @@ async fn turn( ); screen.commit(&lines).map_err(|e| e.to_string())?; } - Action::Context => { + Action::Context(None) => { let lines = io_cli::context::committed( seen.latest().as_ref(), &contract, contract.max_tokens, + mask, &app.theme, screen.width(), ); screen.commit(&lines).map_err(|e| e.to_string())?; } + // **The one place this surface is worth having + // mid-turn**, and the reason it is admissible: + // watching the agent reach for a tool is when + // an operator knows they want it withheld. It + // takes effect on the *next* turn — this one's + // contract was built before the keystroke — and + // the sentence says so rather than implying the + // running turn was changed. + Action::Context(Some(said)) => { + let offered = + io_cli::context::offered(seen.latest().as_ref()); + let (next, line) = + io_cli::context::withhold(mask, &said, &offered); + *mask = next; + app.say(Tone::Muted, line); + } Action::Copy(what) => { let (payload, said) = to_copy(facts.last.as_ref(), store, what); match payload { @@ -10617,6 +10725,14 @@ async fn resume_pending( // same reasoning — see `contract::buying` at the bottom of this function for why // this parameter exists at all. effort: Option, + // **The mask reaches this door too, and by value because nothing here can + // change it.** A resumed run is a turn like any other and the posture applies + // to it — the alternative is a session where `/context withhold docx_write` + // holds everywhere except the one path that answers a parked question, which + // is the shape `/effort` shipped broken in 0.26.0 and this comment's neighbour + // below records. There is no mid-turn surface on this path, so unlike `turn` + // it needs no borrow. + mask: &io_harness::ToolMask, run_id: i64, pending: io_cli::resume::Pending, ) -> Result<(), String> { @@ -10721,6 +10837,10 @@ async fn resume_pending( // 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); + // And the mask, for the same reason and on the same door — see the parameter's + // own note. A posture that held on every turn except a resumed one would be + // the 0.26.0 defect above in a second shape. + let continuing = io_cli::contract::masking(continuing, mask); let (observer, mut events) = bridge::channel(); let canceller = observer.canceller(); let (approver, mut asks) = approval::channel(); diff --git a/src/pluginview.rs b/src/pluginview.rs index 85a9436..d2ecebf 100644 --- a/src/pluginview.rs +++ b/src/pluginview.rs @@ -109,16 +109,183 @@ //! carried on [`View::adapters`] — and it is still not a second read of any file. //! See [`Listed::adapted`]. //! +//! # What a bundle costs, and the four kinds that cost nothing +//! +//! A bundle is a thing an operator pays for on **every** request, and until +//! 0.37.0 this surface said what a bundle contributed and never what that +//! weighed. [`View::costs`] carries [`crate::context::bundle_cost`]'s answer and +//! [`rows`] draws it beside the kinds. +//! +//! **The more useful half of the answer is that most of it is free.** Of the +//! seven kinds a bundle can contribute, only skills, agents and MCP servers reach +//! a provider at all; hooks, templates, a declared binary and a policy layer cost +//! nothing, ever. That is stated from [`COSTS`] — an exhaustive table keyed by +//! io-harness's own wire names, where "free" is a recorded decision with a reason +//! beside it — rather than from a list somebody wrote down, so an eighth kind +//! added upstream fails `tests/pluginview.rs` instead of being drawn as free. +//! +//! **No number here is something a mask or `/context` can reduce.** io-harness +//! sends a byte-identical tool catalogue on a masked turn and an unmasked one, +//! deliberately, because the tool array sits ahead of the provider's cache +//! breakpoint. The only lever these figures inform is switching the bundle off, +//! which is [`enable`] — the verb this surface already offers, which is why the +//! figures belong on this row and not on a page somewhere else. +//! //! Like [`crate::servers`] and [`crate::skillview`], this is a data model and //! functions over it: [`view`] is where the machine is read, once, and everything //! that draws is pure over what it returned. The driver in `src/main.rs` owns the //! keyboard and applies what [`add`] and [`remove`] return. +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use crate::glyphs::Glyphs; use crate::picker::{fit, fit_left, Row}; +/// Whether one kind of contribution reaches a provider, and whether io-cli can +/// put a number on it. +/// +/// **`Free` is a recorded decision, never a default.** See [`COSTS`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Wire { + /// It is on **every** request, and [`crate::context::bundle_cost`] locates + /// and counts it. + Every, + /// It is on **every** request and `bundle_cost` cannot locate it, so a + /// bundle contributing this draws a figure that under-reports. Said in the + /// row — see [`AT_LEAST`] — rather than approximated. + EveryUncounted, + /// It never reaches a provider at all. Not "small", not "cheap": nothing + /// about it is in any request, ever. + Free, +} + +/// What every kind of contribution a bundle can make costs on the wire, and why. +/// +/// # Why this is a table and not a hand-written list of the free ones +/// +/// The fact an operator least expects and most needs is that **four of the seven +/// kinds cost nothing at all** — a bundle contributing only hooks is genuinely +/// free forever, and no amount of budget pressure is a reason to remove it. The +/// failure mode of stating that from a list of names in a `match` with a `_` +/// arm is that io-harness adds an eighth kind, it falls to the wildcard, and the +/// surface silently calls it free. Nobody would ever see it happen. +/// +/// So this is [`crate::acp_map::MAPPING`]'s and [`crate::triage::TRIAGE`]'s shape, +/// for their reason: keyed by **io-harness's own wire name** — the strings +/// `Plugin::contributions` returns — with the decision and the sentence that +/// justifies it beside each. `tests/pluginview.rs` reads the locked harness's own +/// `contributions()` and fails **by name** when the two sets differ, so an eighth +/// kind is a red suite rather than a row that quietly under-reports. +/// +/// The order is io-harness's own, which is the order [`rows`] draws the kinds in. +pub const COSTS: &[(&str, Wire, &str)] = &[ + ( + "skills", + Wire::Every, + "io-harness appends the skill catalogue to the system block of every \ + request — one `- name: description` line per skill, namespaced by the \ + bundle, which is what `context::bundle_cost` sums", + ), + ( + "templates", + Wire::Free, + "a prompt template is expanded into a message the operator chose to send; \ + a request that does not use one carries no trace that the directory \ + exists, so the standing cost of declaring templates is nothing", + ), + ( + "agents", + Wire::EveryUncounted, + "io-harness composes the roster into the system block of every request, \ + so it does cost — but it writes no marker `crate::context` can locate, \ + and a number invented for it would be worse than an honest floor", + ), + ( + "mcp", + Wire::Every, + "every tool the server offered is in the tool array of every request, \ + with its description and its whole JSON schema, which is where nearly \ + all of the weight is", + ), + ( + "hooks", + Wire::Free, + "a hook is a program io-harness runs on this machine around a step; the \ + model is never told it exists and no request carries a byte of it", + ), + ( + "bin", + Wire::Free, + "a declared executable is put on the run's PATH; nothing names it in a \ + request, and it costs tokens only if the model is separately told to \ + run it", + ), + ( + "policy", + Wire::Free, + "a policy layer is a deny rule io-harness evaluates locally before a call \ + happens; the model is not shown the rules and learns of one only by \ + being refused", + ), +]; + +/// What [`COSTS`] says about one kind, or `None` for a kind it does not name. +/// +/// `None` is the only unsafe answer here and every caller reads it the +/// conservative way: an unknown kind is **not** free and its cost is **not** +/// counted, so a bundle carrying one draws a floor rather than a `free` it did +/// not earn. The test is what turns that from a quiet safety net into a failure. +#[must_use] +pub fn wire(kind: &str) -> Option { + COSTS + .iter() + .find(|(name, _, _)| *name == kind) + .map(|(_, wire, _)| *wire) +} + +/// Whether nothing this bundle contributes reaches a request, ever. +/// +/// Every kind must be a *recorded* [`Wire::Free`]: an empty list is not free (it +/// is a bundle contributing nothing, which the row already says in words), and a +/// kind [`COSTS`] does not name is not free either. +#[must_use] +pub fn free(contributions: &[&str]) -> bool { + !contributions.is_empty() + && contributions + .iter() + .all(|kind| wire(kind) == Some(Wire::Free)) +} + +/// Whether this bundle's figure is a floor rather than the whole of what it costs. +/// +/// True for a bundle contributing agents — [`crate::context::bundle_cost`] leaves +/// the roster out on purpose, because io-harness composes it into the system block +/// with no marker to find — and true for a kind [`COSTS`] does not name, which is +/// the same situation arriving through a dependency bump. +#[must_use] +pub fn under_reported(contributions: &[&str]) -> bool { + contributions + .iter() + .any(|kind| !matches!(wire(kind), Some(Wire::Free | Wire::Every))) +} + +/// What a bundle costing nothing at all is drawn as. +/// +/// **Words rather than `0 tokens every request`, and the difference is the point.** +/// A zero is a measurement — "this was on the wire and weighed nothing" — and a +/// bundle contributing only hooks was never on a wire to be weighed. It is also +/// the answer to a question a number cannot answer: not "what did it cost this +/// time" but "will it ever cost anything", and the table says no, permanently. +/// +/// A comma rather than a dash, because this string is drawn in both glyph sets and +/// in `--plain`, and punctuation that needs a glyph set is punctuation that has a +/// fallback to get wrong. +pub const FREE: &str = "free, nothing on the wire"; + +/// The word in front of a figure that is a floor. See [`under_reported`]. +pub const AT_LEAST: &str = "at least "; + /// The mark on a bundle that loaded. /// /// **One ASCII character in both glyph sets, and that is the ASCII form N4 asks @@ -331,9 +498,41 @@ pub struct View { /// could be under, so no row is marked and every bundle draws as the native /// one it must be. pub adapters: Option, + /// What each bundle's contributions weighed on the last request that went + /// out, by plugin id, from [`crate::context::bundle_cost`]. + /// + /// Carried on the view for [`View::adapters`]'s reason: it is a fact about + /// the session rather than about the terminal, and threading it through + /// [`rows`] would put it in the signature of every drawing function + /// including the ones that never show it. + /// + /// **Empty is "no request has gone out yet", and a missing id is not a + /// zero.** `bundle_cost` inserts every loaded bundle it was given, at zero + /// where the bundle contributed nothing to that request — so an id that is + /// absent means the map was never built, and [`rows`] draws + /// [`crate::servers::NOT_ON_A_REQUEST`] for it rather than a figure. + /// + /// **Not something an operator can shrink by masking a tool.** io-harness + /// sends a byte-identical tool catalogue on a masked turn and an unmasked + /// one, deliberately, because the tool array sits ahead of the provider's + /// cache breakpoint. The lever this informs is [`enable`] — switching the + /// bundle off — which is the verb this surface already offers. + pub costs: BTreeMap, } impl View { + /// This view, told what the last request carried. + /// + /// A builder rather than an argument to [`view`], because `view` is called + /// from ten places that are not drawing a cost column — an id lookup, a + /// disclosure, a `--plain` listing — and every one of them would otherwise + /// have to hold a request snapshot it has no use for. + #[must_use] + pub fn with_costs(mut self, costs: BTreeMap) -> View { + self.costs = costs; + self + } + /// Whether the configuration declared no bundle at all. /// /// **Both lists, because a configuration whose only bundle was refused has @@ -493,6 +692,10 @@ pub fn view(plugins: &io_harness::Plugins) -> View { // would make three pure functions read this machine's environment and // leave a test unable to say what any of them saw. adapters: crate::home::adapters(), + // Empty here and filled by [`View::with_costs`]: the figure comes from + // the last request that went out, which is not a fact this function's + // one input — the bundles io-harness read — can answer. + costs: BTreeMap::new(), } } @@ -526,7 +729,13 @@ pub fn view(plugins: &io_harness::Plugins) -> View { /// /// The id is the label. The contributions are unconditional, because they are the /// answer to what the bundle *did to this session* and are the only field here -/// with no other home. The version follows when there is one, the description +/// with no other home. **What they cost follows them immediately and is +/// unconditional too** — see `cost_word` below — because it is the sentence about +/// that list rather than a fact of its own, and because the answer is very often +/// [`FREE`]: four of the seven kinds never reach a request, so an operator under +/// budget pressure has to be able to see, without opening anything, that a bundle +/// contributing only hooks is not what to remove. The version follows when there +/// is one, the description /// takes what is left, and **the root path is the field that gives way** — below /// a floor this module keeps privately it is dropped whole rather than drawn as an /// ellipsis and an extension. At eighty columns a row is therefore the id, what it @@ -577,6 +786,22 @@ pub fn rows(view: &View, width: u16, glyphs: &Glyphs) -> Vec { // nothing, and saying so is the whole point of the row. detail = "contributes nothing".to_string(); } + // **Immediately after the kinds, because it is the sentence about them.** + // "skills, mcp · 4.2k tokens every request" reads as one fact; the same + // figure after the version reads as a fact about the version. Drawn + // before the width budget below is taken, so the cost competes with the + // description and the root for room rather than being appended past the + // end of the row. + // + // Nothing at all on a switched-off bundle, which contributed none of it: + // that row already leads with "switched off", and either a figure or the + // word `free` under it would be a claim about a session this bundle is + // not in. Nothing on a bundle contributing nothing either — the row + // already says so in words, and `free` would add no fact. + if plugin.enabled && !plugin.contributions.is_empty() { + detail.push_str(separator); + detail.push_str(&cost_word(plugin, &view.costs)); + } if let Some(version) = &plugin.version { detail.push_str(separator); detail.push_str(version); @@ -653,6 +878,41 @@ pub fn rows(view: &View, width: u16, glyphs: &Glyphs) -> Vec { out } +/// What one bundle's row says about what it costs, in one field. +/// +/// Four answers, and the order they are asked in is the whole of the logic: +/// +/// 1. **[`FREE`]**, when every kind the bundle contributes is a recorded +/// [`Wire::Free`]. Asked *first*, so it beats every number: it is a statement +/// about all future requests, from [`COSTS`], and it must not depend on +/// whether a request happens to have gone out yet. +/// 2. **[`crate::servers::NOT_ON_A_REQUEST`]**, when no request has gone out — +/// the bundle is absent from `costs`, which is empty until +/// [`View::with_costs`] is given a snapshot. Never `0`: see that constant. +/// 3. **[`AT_LEAST`] and the figure**, when the bundle contributes something +/// [`crate::context::bundle_cost`] cannot locate — an agent roster today, an +/// eighth kind a later io-harness adds. The figure is a floor and the row +/// says so, which is the honest form of a number io-cli cannot complete. +/// 4. **The figure**, otherwise. +/// +/// A bundle can legitimately draw `0 tokens every request` here — it contributes +/// an MCP server that has offered no tool to a request yet — and that is *not* +/// [`FREE`]: one is a measurement of the last request, the other is a promise +/// about every request, and collapsing them would tell an operator a server they +/// are about to pay for is free. +fn cost_word(plugin: &Listed, costs: &BTreeMap) -> String { + if free(&plugin.contributions) { + return FREE.to_string(); + } + match costs.get(&plugin.id) { + None => crate::servers::NOT_ON_A_REQUEST.to_string(), + Some(&tokens) if under_reported(&plugin.contributions) => { + format!("{AT_LEAST}{}", crate::servers::per_request(tokens)) + } + Some(&tokens) => crate::servers::per_request(tokens), + } +} + /// The picker rows for one bundle: what it actually put into the session. /// /// Grouped under headings, which are free here and are not free in [`rows`]: this @@ -1229,6 +1489,10 @@ mod tests { // are what the rows wear. The adapted case is asserted in // `tests/plugins.rs`, against a bundle whose root really is under one. adapters: None, + // No request has gone out, which is the state these row-shape tests + // are about. The cost figures are asserted in `tests/pluginview.rs` + // against a snapshot `context::bundle_cost` was given. + costs: BTreeMap::new(), } } @@ -1317,6 +1581,7 @@ mod tests { error: "/repo/bundles/empty: no plugin.toml".to_string(), }], adapters: None, + costs: BTreeMap::new(), }; for glyphs in [&UNICODE, &ASCII] { let rows = rows(&view, 200, glyphs); @@ -1419,6 +1684,7 @@ mod tests { error: sentence.to_string(), }], adapters: None, + costs: BTreeMap::new(), }; for glyphs in [&UNICODE, &ASCII] { let rows = rows(&view, 200, glyphs); @@ -1450,6 +1716,7 @@ mod tests { error: sentence.to_string(), }], adapters: None, + costs: BTreeMap::new(), }; for glyphs in [&UNICODE, &ASCII] { let rows = rows(&view, 80, glyphs); diff --git a/src/servers.rs b/src/servers.rs index e609b0e..24167e2 100644 --- a/src/servers.rs +++ b/src/servers.rs @@ -36,6 +36,24 @@ //! bound on what is offered, and the two numbers are drawn as two numbers: one //! replacing the other would answer a question nobody asked. //! +//! # A third number, from a third source, and what it is not +//! +//! [`Server::cost`] is what the server's tools **weighed on the last request's +//! wire**, from [`crate::context::server_cost`]. That is neither of the counts +//! above: the announcing event says what a server offered at connect time, this +//! module counts what the session called, and only the request says what the +//! catalogue cost. **They can legitimately disagree** — a server that came up +//! after the last completion has announced its tools and has been on no wire — +//! so all three are drawn with the word that says which they are, and a server +//! that has been on no wire draws [`NOT_ON_A_REQUEST`] and never `0`. +//! +//! **It is a standing charge, not something a mask can shave.** io-harness sends +//! a byte-identical tool catalogue on a masked turn and an unmasked one, +//! deliberately, because the tool array sits ahead of the provider's cache +//! breakpoint — so `/context` and a narrowed toolset do not reduce this figure +//! by a token. The lever it informs is [`switch`], which is the verb this panel +//! already offers, and that is the whole reason the number belongs on this row. +//! //! # Disable, and why the write that was refused in 0.29.0 is correct in 0.30.0 //! //! Through io-harness 0.69.0 this paragraph said `McpServer` was `id`, `transport` @@ -184,6 +202,31 @@ pub struct Server { /// [`Server::state`] rather than a variant of [`Reached`]. A server that is /// off has *no* state to have reached. pub enabled: bool, + /// What this server's tools cost on **every** request the session makes, by + /// [`crate::context::server_cost`] over the last one that went out. + /// + /// **A third number beside [`Reached`]'s two, from a third source, and the + /// row says which is which.** `Reached::Answered`'s `offered` is what the + /// server *announced at connect time*, on an `EventKind::Mcp`; its `tools` is + /// what this session has *called*. This is what the server's catalogue + /// *weighed on the wire* of the most recent request. They legitimately + /// disagree — a server that came up after the last completion has announced + /// its tools and put none of them on a wire yet — so all three are drawn with + /// the word that says what they are, never as bare figures side by side. + /// + /// **`None` is "no request has carried it yet", and it is not zero.** A zero + /// would say the server is free, which is the one wrong answer this number + /// can give. [`servers`] leaves it `None` and [`costed`] is what fills it, so + /// a surface that never asked for the figure draws [`NOT_ON_A_REQUEST`] + /// rather than a number it invented. + /// + /// **Not something an operator can shrink by withholding the server from a + /// turn.** io-harness sends a byte-identical tool catalogue whether or not a + /// tool is masked, deliberately, because the tool array sits ahead of the + /// provider's cache breakpoint. The lever this number informs is + /// [`switch`] — turning the server off in the file — which is the verb this + /// panel already offers. + pub cost: Option, } /// Per-server facts accumulated from the run's own events. @@ -301,11 +344,54 @@ pub fn servers(config: &Config, observed: &Observed) -> Vec { }, state: observed.of(&server.id), enabled: server.enabled, + // Left unstated here, and filled by [`costed`]: the figure comes + // from the last request that went out, which is a different + // source from `observed` and is not this function's to reach for. + cost: None, } }) .collect() } +/// What a token figure on one of these rows means, spelled once. +/// +/// **"Every request", not "this request".** The catalogue is re-sent whole on +/// every completion a turn makes, so this is a standing charge rather than a +/// measurement of one moment — and an operator reading it as the latter would +/// reach for the wrong lever. See [`Server::cost`]. +pub fn per_request(tokens: u64) -> String { + format!( + "{} tokens every request", + crate::status::format_tokens(tokens) + ) +} + +/// What a thing no request has carried yet is drawn as. +/// +/// **A phrase, and never `0`.** Zero is a measurement — "this was on the wire and +/// weighed nothing" — and this is the absence of a measurement. Drawing them the +/// same would report a server that has not been on a wire as free, which is the +/// one claim that would stop an operator looking at it. +/// +/// Shared with [`crate::pluginview`] rather than spelled twice, so `/mcp` and +/// `/plugin` cannot drift into two phrases for one state. +pub const NOT_ON_A_REQUEST: &str = "not yet on a request"; + +/// Fill in what each server's tools weighed on the last request. +/// +/// `cost` is [`crate::context::server_cost`]'s map, keyed on the **wire** id, +/// which is the id [`Server`] already holds — so no translation happens here and +/// there is no second spelling to get wrong. +/// +/// A server absent from the map keeps its `None`, which is the whole reason this +/// is a lookup rather than a `get(..).unwrap_or(0)`: absent means no request has +/// carried it, and zero would mean it is free. See [`Server::cost`]. +pub fn costed(servers: &mut [Server], cost: &BTreeMap) { + for server in servers { + server.cost = cost.get(&server.id).copied(); + } +} + /// How a server is reached, in one short string. fn transport(server: &io_harness::McpServer) -> String { // Exhaustive, with no wildcard: `McpTransport` is NOT `#[non_exhaustive]`, @@ -322,7 +408,23 @@ fn transport(server: &io_harness::McpServer) -> String { /// The rows as the picker draws them. /// /// Content before metadata: the id is the label, and the detail carries what the -/// session saw then how it is reached. +/// session saw, what it costs, then how it is reached. +/// +/// # Three numbers, and every one of them wears its word +/// +/// `10 offered · 2 used · 4.2k tokens every request` is three answers to three +/// questions from three sources — the announcing event, this session's own call +/// count, and the catalogue on the last request's wire. Two of them can +/// legitimately disagree with the third: a server that came up after the last +/// completion has announced its tools and put none of them on a wire yet, and +/// draws [`NOT_ON_A_REQUEST`] beside a stated `offered`. **Unlabelled figures +/// side by side are how a surface starts lying**, so none of the three is ever +/// drawn as a bare number. +/// +/// A switched-off server draws none of them, for the reason it draws no state: +/// there is nothing it did this session and nothing it put on a wire, and +/// `0 tokens every request` over it would read as a measurement rather than as +/// the file's own instruction. pub fn rows(servers: &[Server]) -> Vec { servers .iter() @@ -349,8 +451,17 @@ pub fn rows(servers: &[Server]) -> Vec { // "not reached this session" over a server the file switched off is // true and reads as a server that simply has not been called yet, // which is the one wrong answer available here. + // + // **A third clause, with its own word, and never a bare figure.** It + // answers a different question from either count beside it and comes + // from a different source — see this function's docs — and `None` is + // drawn as the absence it is rather than as a zero. let state = if server.enabled { - state + let cost = match server.cost { + Some(tokens) => per_request(tokens), + None => NOT_ON_A_REQUEST.to_string(), + }; + format!("{state} · {cost}") } else { DISABLED.to_string() }; diff --git a/tests/commands.rs b/tests/commands.rs index 4662f75..f83278e 100644 --- a/tests/commands.rs +++ b/tests/commands.rs @@ -1784,3 +1784,142 @@ fn f10_an_unparseable_verb_never_falls_through_to_something_destructive() { "a file literally called `trace` is reachable as `./trace`", ); } + +/// **F5 — the masking verbs are words on a row that already exists.** +/// +/// The count and the group occupancy are asserted by +/// [`o2_the_palette_did_not_grow_and_no_group_was_refiled`], which this release +/// leaves alone — that is the whole of the claim there, and the strongest form of +/// it is a literal nobody edited. What is left to state is the half a count cannot +/// see: that no row was added for a verb. `COMMANDS` carries `/copy diff` as a row +/// of its own, so "a verb never gets a row" is not a rule this table follows on its +/// own and a reader could reasonably have added `/context withhold` beside it. +/// +/// Sabotage: add `("/context withhold", "…")` to `COMMANDS` — under which this +/// fails by name, and the count and occupancy gates fail beside it, which is the +/// point: three gates for one slot because the two big groups are at the bound. +#[test] +fn f5_the_mask_verbs_take_no_row_of_their_own() { + let context_rows: Vec<&str> = COMMANDS + .iter() + .map(|(name, _)| *name) + .filter(|name| name.split_whitespace().next() == Some("/context")) + .collect(); + + assert_eq!( + context_rows, + vec!["/context"], + "0.29.0 made `marketplace` words inside `/plugin` rather than a command, \ + and the mask verbs are the same decision: `Group::Turn` and \ + `Group::Inspect` are at the ten-per-group bound, so a row here is a row \ + with nowhere to be filed.", + ); +} + +/// **F5 — every `/context` form parses to what it says, and the bare word is still +/// the page.** +/// +/// The bare form is asserted first and by equality, because it is the one that +/// must not have moved: `/context` was a page before this release and an operator +/// who types it is asking for the page, not for a verb they have never heard of. +/// +/// The refusals are the `/store` family's, for the reason that file records — +/// somebody who typed a withhold and got a report would believe the tool was +/// withheld. `withhold` with no name is refused rather than read as "withhold +/// everything"; `allow` with no name clears, which is the asymmetry +/// [`io_cli::commands::Masked::Clear`] argues for at length. +/// +/// Sabotage: fold the two bare forms together — return `Masked::Clear` for a bare +/// `withhold` as well — under which only this fails, and it fails by taking every +/// tool away from a line that named none. +#[test] +fn f5_the_context_verbs_parse_and_a_bare_context_is_still_the_page() { + use io_cli::commands::Masked; + + let parse = + |text: &str| commands::parse(text.strip_prefix('/').unwrap_or(text), &defaults(), &DARK); + + assert_eq!(parse("/context"), Action::Context(None)); + + assert_eq!( + parse("/context withhold docx_write"), + Action::Context(Some(Masked::Withhold("docx_write".into()))), + ); + assert_eq!( + parse("/context allow docx_write"), + Action::Context(Some(Masked::Allow("docx_write".into()))), + ); + // Bare `allow` drops the whole mask. Deliberate, and the argument is in + // `Masked::Clear`: no file is written, the page one keystroke away lists what + // is withheld, and the command that built the mask puts it back. + assert_eq!( + parse("/context allow"), + Action::Context(Some(Masked::Clear)) + ); + // `withhold` with nothing named is NOT the mirror of that. + assert_eq!( + parse("/context withhold"), + Action::Context(Some(Masked::NoTool)), + ); + + // The name keeps its case; the verb does not need to. + assert_eq!( + parse("/context WITHHOLD Docx_Write"), + Action::Context(Some(Masked::Withhold("Docx_Write".into()))), + "a verb typed in another case is the same verb, and a tool name folded to \ + lower case is a name no catalogue answers to — io-harness keeps an \ + unknown name rather than rejecting it, so the mistake would be silent", + ); + + // A near-miss verb must not reach the page. + for typo in ["/context withold docx_write", "/context deny docx_write"] { + assert!( + matches!(parse(typo), Action::Context(Some(Masked::Unknown(_)))), + "`{typo}` must be named as unknown rather than showing the page: {:?}", + parse(typo), + ); + } +} + +/// **F8 — the masking verbs run while a turn is in flight.** +/// +/// `/context` has been in `MID_TURN` since the page existed and the verbs inherit +/// it, which is the answer rather than an oversight: the mask is state this process +/// owns, it writes no file, and `contract::masking` applies it to the *next* turn's +/// contract — so a withhold typed mid-turn cannot reach the turn already running. +/// That is what makes it unlike `/config`, which is in `BARE_ONLY_MID_TURN` +/// because its argued forms write a scope file. +/// +/// `/config prices.as_of` is asserted here beside them, unchanged, so a release +/// that reached for `BARE_ONLY_MID_TURN` to guard the mask cannot quietly take the +/// guard off the thing it was built for. +/// +/// Sabotage: add `"/context"` to `BARE_ONLY_MID_TURN` — under which only this test +/// fails, and it fails by refusing mid-turn exactly the lever an operator reaches +/// for *because* a turn is running and calling something they would rather it did +/// not. +#[test] +fn f8_the_mask_verbs_are_admissible_mid_turn() { + use io_cli::commands::runs_mid_turn; + + for line in [ + "context", + "context withhold docx_write", + "context allow docx_write", + "context allow", + "context withhold", + ] { + assert!( + runs_mid_turn(line), + "`/{line}` reads the session's own state and writes no file; refusing \ + it mid-turn refuses it when it is most wanted", + ); + } + + assert!(runs_mid_turn("config"), "the bare list only reports"); + assert!( + !runs_mid_turn("config prices.as_of"), + "the bare-only mechanism exists for the command that descends toward a \ + write, and this release does not borrow it", + ); +} diff --git a/tests/context.rs b/tests/context.rs index b310582..fea531e 100644 --- a/tests/context.rs +++ b/tests/context.rs @@ -14,16 +14,20 @@ //! reconstructs instead of reading fails on all three, and it fails on the exact //! numbers rather than on a missing row. +use io_cli::commands::Masked; use io_cli::context::{self, Request, Seen}; use io_cli::glyphs::ASCII; use io_cli::provider::{watching, Watched}; use io_cli::theme::{Theme, DARK}; use io_harness::context::estimate_tokens; use io_harness::{ - CompletionRequest, CompletionResponse, McpServer, Message, Provider, TaskContract, ToolSpec, + CompletionRequest, CompletionResponse, McpServer, Message, Provider, TaskContract, ToolMask, + ToolSpec, }; use ratatui::text::Line; +mod support; + // --------------------------------------------------------------------------- // Fixtures // --------------------------------------------------------------------------- @@ -371,6 +375,7 @@ fn f7_the_catalogue_names_a_tool_only_the_request_knew_about() { Some(&Request::of(&request())), &contract(), None, + &ToolMask::none(), &ascii(), 80, )); @@ -427,6 +432,7 @@ fn f7_the_system_block_is_tokens_of_its_own_text_and_not_a_byte_count() { Some(&seen), &contract, None, + &ToolMask::none(), &ascii(), 80, )); @@ -456,7 +462,14 @@ fn f7_the_system_block_is_tokens_of_its_own_text_and_not_a_byte_count() { #[test] fn f7_the_page_draws_in_ascii_and_says_so_before_a_turn_has_run() { - let empty = drawn(&context::committed(None, &contract(), None, &ascii(), 80)); + let empty = drawn(&context::committed( + None, + &contract(), + None, + &ToolMask::none(), + &ascii(), + 80, + )); assert!( empty .iter() @@ -469,6 +482,7 @@ fn f7_the_page_draws_in_ascii_and_says_so_before_a_turn_has_run() { Some(&Request::of(&request())), &contract(), None, + &ToolMask::none(), &ascii(), 80, )); @@ -589,3 +603,439 @@ fn every_site_that_forgets_a_run_forgets_the_window_with_it() { one that did not call it", ); } + +// --------------------------------------------------------------------------- +// 0.37.0 — the catalogue is attributed to what put it on the wire +// --------------------------------------------------------------------------- + +/// The skill catalogue io-harness appends, exactly as `with_skill_catalog` writes +/// it: one paragraph of framing, a newline, then one `- name: description` line +/// per skill. The second entry is namespaced, because a bundle's skills are. +const SKILL_BLOCK: &str = "Skills available to you — instructions written for this repository. \ + Only each skill's name and description is shown; call `read_skill` with a name to read that \ + skill's full text when its description matches what you are doing.\n\ + - alpha: the operator's own skill, unqualified\n\ + - docs__writing: a skill the docs bundle ships"; + +/// The system block of a turn whose session had skills configured. +fn system_with_skills() -> String { + format!("{}\n\n{SKILL_BLOCK}", system()) +} + +/// **F2.** The head this module scans for is a sentence io-harness actually +/// writes, read out of the pinned source rather than trusted. +/// +/// The two memory heads above it are matched against a dependency's prose with no +/// such gate, and their failure mode is the one this exists to close: io-harness +/// copy-edits the sentence, the section silently reports zero, and a zero is +/// indistinguishable from a session that configured no skills. A wrong number is +/// worse here than a missing row, because the page's whole claim is that its parts +/// sum to the request. +/// +/// Sabotage: change one word of `SKILLS_HEAD` in `src/context.rs`. Nothing else in +/// the suite notices — every other arm builds its own fixture from the same +/// constant, so they all agree with each other and all disagree with the wire. +#[test] +fn f2_the_skill_catalogue_head_is_a_sentence_io_harness_writes() { + let source = support::harness_source_at(&["run", "prompts.rs"]); + assert!( + source.contains(context::SKILLS_HEAD), + "`SKILLS_HEAD` is `{}`, which is not in io-harness's own run/prompts.rs. \ + The pin moved the sentence and the skill catalogue section now reports zero \ + for every session — which reads exactly like a session with no skills.", + context::SKILLS_HEAD, + ); + assert!( + source.contains("fn with_skill_catalog"), + "the function that writes the catalogue is gone from run/prompts.rs, so the \ + needle above is being matched against something else" + ); +} + +/// **F2.** A catalogue is located and counted; a perturbed one is reported absent +/// rather than as zero. +/// +/// Both directions, because one direction is what let 0.30.0 document a door with +/// no caller. The failing direction is the one that matters: a needle that has +/// drifted produces `None`, the page says "no skill was named to this request", +/// and an operator with twelve skills reads that as the truth. +/// +/// Sabotage: make `skills_in` return `Some` unconditionally, or drop the +/// `starts_with("- ")` line test so the block runs to the end of the system +/// prompt. The first fails the absent arm; the second fails the token equality, +/// because the block would then swallow the framing after it. +#[test] +fn f2_a_skill_catalogue_is_located_and_a_perturbed_one_is_absent() { + let mut seen = Request::of(&CompletionRequest { + system: system_with_skills(), + ..request() + }); + let sections = context::sections(&seen, &contract()); + let row = section(§ions, context::SKILL_CATALOGUE); + assert_eq!( + row.tokens, + estimate_tokens(SKILL_BLOCK), + "the section must be the block io-harness wrote, whole and nothing else" + ); + assert!( + row.detail.contains("2 skill(s)"), + "two catalogue lines, counted off the wire: {}", + row.detail + ); + + // The same request with the head sentence perturbed — which is what a pin + // that copy-edits it produces. + seen.system = seen.system.replace("Skills available", "Skills offered"); + let sections = context::sections(&seen, &contract()); + let row = section(§ions, context::SKILL_CATALOGUE); + assert_eq!(row.tokens, 0); + assert!( + row.detail.contains("no skill"), + "an unlocated catalogue must say so, never draw a bare zero: {}", + row.detail + ); +} + +/// **F1.** The sections still partition the request once the catalogue is one of +/// them. +/// +/// The skill block is inside `request.system`, so a section that reported it +/// beside the system block rather than out of it would count those bytes twice and +/// the total would exceed the request. This is the arithmetic the page's whole +/// design rests on and it is asserted against the request rather than against the +/// sum of the rows — a sum of the rows compared to itself is a tautology. +/// +/// Sabotage: drop the second `saturating_sub` in `sections`. Every row still draws +/// and only this equality fails. +#[test] +fn f1_the_sections_still_sum_to_the_request_with_a_catalogue_present() { + let seen = Request::of(&CompletionRequest { + system: system_with_skills(), + ..request() + }); + let sections = context::sections(&seen, &contract()); + + let request_tokens = estimate_tokens(&seen.system) + + seen + .tools + .iter() + .map(|t| estimate_tokens(&format!("{}\n{}\n{}", t.name, t.description, t.parameters))) + .sum::() + + estimate_tokens(&seen.conversation); + + assert_eq!( + context::total(§ions), + request_tokens, + "the sections are a partition of the request, so they sum to it exactly" + ); +} + +/// **F3.** `server_cost` is the same measurement the page draws, keyed for a +/// surface to look up. +/// +/// `/mcp` and `/plugin` draw their numbers from these maps and `/context` draws +/// its rows from `sections`; if the two could disagree, two surfaces would state +/// different costs for one server and the operator would have no way to tell which +/// was real. Asserted from **one** snapshot, which is the same discipline that +/// keeps `ctx N%` and the page total in agreement. +/// +/// Sabotage: count `tool.name` alone in either function. The other keeps counting +/// the schema, and this equality is the only thing that fails. +#[test] +fn f3_server_cost_is_the_number_the_page_draws_for_the_same_server() { + let seen = Request::of(&request()); + let contract = contract(); + let costs = context::server_cost(&seen, &contract); + let sections = context::sections(&seen, &contract); + + assert_eq!(costs.len(), 1, "one server offered a tool: {costs:?}"); + let (id, tokens) = costs.iter().next().expect("the one server"); + assert_eq!( + *tokens, + section(§ions, &format!("mcp {id}")).tokens, + "the map and the page must be one measurement, not two" + ); +} + +/// **F3.** A server that has not been on a wire is absent from the map rather than +/// present at zero. +/// +/// The two mean different things and only one of them is true. A configured server +/// that has offered nothing yet is unmeasured; zero would tell an operator it is +/// free, which is the opposite of unknown and is the claim they would act on. +/// +/// Sabotage: seed the map from `contract.mcp` before the loop. Every other arm +/// passes. +#[test] +fn f3_a_server_that_has_not_been_on_a_wire_is_absent_not_zero() { + let seen = Request::of(&CompletionRequest { + tools: vec![tool("read_file", "Read a file from the workspace.")], + ..request() + }); + let costs = context::server_cost(&seen, &contract()); + assert!( + costs.is_empty(), + "the contract configures `docs`, but no tool of its reached this request, \ + so it is unmeasured rather than free: {costs:?}" + ); +} + +/// **F9.** The sentence a withhold prints says the cost went **up**, not down. +/// +/// This is the one line in the release that can be got backwards, and getting it +/// backwards is invisible: a reader told a tool was withheld assumes the request +/// shrank, because that is what the word means everywhere else and it is what this +/// release's own roadmap entry assumed. io-harness sends a byte-identical +/// catalogue and appends a sentence naming the withheld tools, so a mask costs a +/// little more and saves nothing. +/// +/// Sabotage: delete the "still costs its definition" clause from +/// `context::withhold`, or write "saving" anywhere in it. The mask still works, +/// every other arm passes, and the product lies to its operator. +#[test] +fn f9_a_withhold_says_the_tool_is_still_offered_and_still_costs() { + let catalogue = vec!["docx_write".to_string()]; + let (mask, line) = context::withhold( + &ToolMask::none(), + &Masked::Withhold("docx_write".into()), + &catalogue, + ); + assert!(mask.withholds("docx_write")); + assert!( + line.contains("still costs its definition"), + "the cost direction must be stated: {line}" + ); + assert!( + line.contains("refused"), + "and what actually happens must be stated: {line}" + ); + for wrong in ["saves", "saving", "cheaper", "smaller", "reduces"] { + assert!( + !line.to_lowercase().contains(wrong), + "`{wrong}` in a withhold's own sentence is the release's top risk: {line}" + ); + } +} + +/// **F9.** The page's withheld row says the same thing, and is absent when +/// nothing is withheld. +/// +/// Absent rather than "nothing is withheld": a row that appears on every draw and +/// says nothing trains a reader to skip the place the answer appears, which is the +/// one place it must not be skipped. +/// +/// Sabotage: return `Some` unconditionally from `withheld_line`, or drop its +/// closing clause. The first fails the empty arm, the second the wording arm. +#[test] +fn f9_the_withheld_row_is_absent_until_there_is_one_and_then_states_its_cost() { + assert_eq!(context::withheld_line(&ToolMask::none(), "-"), None); + + let mask = ToolMask::withholding(["docx_write", "xlsx_write"]); + let line = context::withheld_line(&mask, "-").expect("a mask draws a row"); + assert!(line.contains("docx_write") && line.contains("xlsx_write")); + assert!( + line.contains("saves nothing"), + "the row must not let a reader infer a saving: {line}" + ); + + // And it reaches the drawn page, which is the surface an operator reads — + // the function above being right is not evidence that anything calls it. + let page = drawn(&context::committed( + Some(&Request::of(&request())), + &contract(), + None, + &mask, + &ascii(), + 80, + )); + assert!( + page.iter().any(|row| row.contains("withheld:")), + "the mask must reach the page: {page:#?}" + ); +} + +/// **F9.** `allow` never silently does nothing, and bare `allow` names what came +/// back. +/// +/// An operator who mistypes a name and gets no answer believes the tool was +/// allowed again. And a bare `allow` changes several things at once, so it owes +/// the list — which is the whole reason it is safe to leave unguarded. +/// +/// Sabotage: return the same line whether or not `retain` removed anything, or +/// drop the name list from the `Clear` arm. +#[test] +fn f9_allow_reports_a_no_op_and_a_clear_names_what_returned() { + let mask = ToolMask::withholding(["docx_write", "pdf_write"]); + + let (_, line) = context::withhold(&mask, &Masked::Allow("never_withheld".into()), &[]); + assert!( + line.contains("was not withheld"), + "a mistyped name must not read as success: {line}" + ); + + let (cleared, line) = context::withhold(&mask, &Masked::Clear, &[]); + assert!(cleared.is_empty()); + assert!( + line.contains("docx_write") && line.contains("pdf_write"), + "clearing must name every tool it re-offered: {line}" + ); +} + +// --------------------------------------------------------------------------- +// Defects the adversarial review found behind a fully green suite +// --------------------------------------------------------------------------- + +/// **F9 — withholding a name the catalogue does not carry says so.** +/// +/// **The defect this replaces was the safety lever being silently inert.** +/// io-harness keeps an unknown mask name rather than rejecting it, deliberately, +/// so a mask stays portable across builds with different cargo features +/// (`io-harness-0.76.0/src/tools/mod.rs:55-58`). That means `mask_gate` matches on +/// the exact string and a misspelling withholds nothing — while the operator was +/// told "calling it will be refused before anything starts" and `/context` drew it +/// on the withheld row. `/context withhold Docx_Write` and the file gets written. +/// +/// The asymmetry was the tell: `Masked::Allow` already refused to be a silent +/// no-op and was tested for it. The guard existed only on the harmless direction. +/// +/// The name is still added — refusing it would break the portability io-harness +/// designed for — and only the sentence changes. +/// +/// Sabotage: drop the `unknown` branch, or pass `&[]` at either call site in +/// `src/main.rs`. The second is the 0.35.0 shape exactly, where a correct guard +/// was handed an empty slice by the only door that reached it. +#[test] +fn f9_withholding_a_name_the_catalogue_does_not_carry_says_so() { + let catalogue: Vec = vec!["docx_write".into(), "write_file".into()]; + + // The case the operator actually hits: right tool, wrong case. + let (mask, line) = context::withhold( + &ToolMask::none(), + &Masked::Withhold("Docx_Write".into()), + &catalogue, + ); + assert!( + line.contains("no tool of that name"), + "a name matching nothing on the wire must not be confirmed as refused: {line}" + ); + assert!( + mask.withholds("Docx_Write"), + "it is still added — io-harness keeps unknown names so a mask stays \ + portable, and refusing here would break that" + ); + + // A real name is confirmed, without the warning. + let (_, line) = context::withhold( + &ToolMask::none(), + &Masked::Withhold("docx_write".into()), + &catalogue, + ); + assert!(!line.contains("no tool of that name"), "{line}"); + + // And before any turn there is no catalogue to check against, so nothing is + // warned about — a warning on every name at a fresh prompt is noise that + // trains the reader to ignore the one that matters. + let (_, line) = context::withhold(&ToolMask::none(), &Masked::Withhold("anything".into()), &[]); + assert!(!line.contains("no tool of that name"), "{line}"); +} + +/// **F2 — the planning directive is not counted as part of the skill catalogue.** +/// +/// **io-harness glues the directive onto the last catalogue line with no newline +/// between them**, so a line scan cannot see the boundary: `compose` calls +/// `with_skill_catalog` and then `out.push_str(&directive)` +/// (`run/prompts.rs:73-76`), `Skills::catalog()` ends with no trailing newline +/// (`skills.rs:470-476`), and `planning_directive` begins with a space +/// (`run/gate.rs:282-286`). The final line therefore still starts with `- ` and a +/// naive scan swallows roughly 117 tokens of directive. +/// +/// It is reachable on **every contained turn**, because registering a plan gate +/// turns io-harness's planning phase on. And the damage is not only the row: this +/// block is what `bundle_cost` splits per line, so the directive's tokens are +/// charged to whichever bundle owns the alphabetically last skill, and that +/// bundle's `/plugin` figure moves when the operator toggles `/plan`. +/// +/// Sabotage: delete the `PLAN_DIRECTIVE_HEADS` cut from `skills_in`. The partition +/// still sums correctly — the bytes are inside `request.system` either way — which +/// is exactly why the sum test cannot see this and this arm has to exist. +#[test] +fn f2_the_planning_directive_is_not_part_of_the_skill_catalogue() { + // The wire shape: catalogue, then the directive glued to the last line. + let directive = " Before you do anything else you must call `propose_plan` with the ordered \ + steps you intend to take, and wait. Until that plan is approved you may \ + read, search and think."; + let system = format!("{}\n\n{SKILL_BLOCK}{directive}", system()); + + let seen = Request::of(&CompletionRequest { + system, + ..request() + }); + let sections = context::sections(&seen, &contract()); + let row = section(§ions, context::SKILL_CATALOGUE); + + assert_eq!( + row.tokens, + estimate_tokens(SKILL_BLOCK), + "the catalogue row must be the catalogue, not the catalogue plus whatever \ + io-harness appended to it without a newline" + ); + assert!( + row.detail.contains("2 skill(s)"), + "and the line count is unchanged by the directive: {}", + row.detail + ); +} + +/// **F2 — both directive openers are sentences io-harness actually writes.** +/// +/// The same gate `SKILLS_HEAD` gets, for the same reason: a constant matched +/// against a dependency's prose fails silently when the dependency rewords it, and +/// the failure here is a row that over-reports with nothing going red. +/// +/// Sabotage: reword either constant. Nothing else in the suite notices, because +/// every other fixture builds its directive from these same constants. +#[test] +fn f2_the_planning_directive_openers_are_io_harness_s_own() { + let source = support::harness_source_at(&["run", "gate.rs"]); + for head in context::PLAN_DIRECTIVE_HEADS { + assert!( + source.contains(head), + "`{head}` is not in io-harness's run/gate.rs, so the skill catalogue \ + row silently swallows the planning directive on every contained turn", + ); + } +} + +/// **F1 — one server does not absorb another's cost when an id is a prefix.** +/// +/// `contract.mcp` lists the operator's own `[[mcp]]` entries before the plugin +/// servers `Plugins::apply_to` appends, and a bare id is validated by nobody — so +/// `github` and `github__enterprise` can both be configured. Taking the first +/// match charged every enterprise tool to `github` and left `github__enterprise` +/// reading "not yet on a request" for the life of the session. +/// +/// Sabotage: change `max_by_key` back to a first-match return in `server_of`. +#[test] +fn f1_the_longest_configured_server_id_wins_not_the_first() { + let contract = TaskContract::workspace("goal", "/repo").with_mcp([ + McpServer::stdio("github", "gh"), + McpServer::stdio("github__enterprise", "ghe"), + ]); + let seen = Request::of(&CompletionRequest { + tools: vec![tool( + "mcp__github__enterprise__list_repos", + "List the repositories.", + )], + ..request() + }); + + let costs = context::server_cost(&seen, &contract); + assert!( + costs.contains_key("github__enterprise"), + "the enterprise server's own tool was charged elsewhere: {costs:?}" + ); + assert!( + !costs.contains_key("github"), + "`github` was charged for a tool that is not its own: {costs:?}" + ); +} diff --git a/tests/contract.rs b/tests/contract.rs index 68f65df..d997cb2 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -676,6 +676,110 @@ fn f1_the_level_asked_for_is_the_level_the_turn_buys() { } } +/// **N4 — a session that never withholds anything sends the contract it always +/// sent.** +/// +/// The same instrument as [`f1_no_effort_asked_for_leaves_the_contract_byte_for_byte`] +/// 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. +/// +/// **What this can and cannot see.** `TaskContract::tool_mask` is a plain +/// `ToolMask` with an empty default rather than an `Option`, so +/// `with_tool_mask(ToolMask::none())` would *also* be Debug-identical — the early +/// return in `contract::masking` is therefore not observable here, and this test +/// does not claim it is. What it guards is the value: that nothing is withheld +/// from an operator who withheld nothing. +/// +/// Sabotage: give `masking` a default of +/// `ToolMask::withholding(["write_file"])` for the empty case — the shape +/// `buying`'s `Effort::Medium` sabotage takes — under which only this test fails, +/// and it fails by taking a tool away from every operator who never asked. +#[test] +fn n4_no_mask_asked_for_leaves_the_contract_byte_for_byte() { + let contract = TaskContract::workspace("a turn", root()); + let before = format!("{contract:?}"); + + let after = io_cli::contract::masking(contract, &io_harness::ToolMask::none()); + + assert_eq!(format!("{after:?}"), before); + assert!( + after.tool_mask.is_empty(), + "an empty mask is the absence of a mask, not a mask of nothing that some \ + later reader might render a sentence for", + ); +} + +/// **F6, the half `contract::masking` owns — the names asked for are the names the +/// turn withholds.** +/// +/// That the mask reaches *every door* is a count over the driver's call sites, the +/// shape [`f1_every_turn_that_runs_applies_the_effort_level`] takes; this is the +/// other half, and it is the one a count cannot see: a door could apply a function +/// that quietly withheld the wrong set. +/// +/// Two names rather than one, and a third asserted *absent*, because a mask that +/// carried only its first name would satisfy every `is_empty()` check and still +/// hand the model the second tool — the same defect the `/effort` mapping test +/// guards by walking all three levels instead of asserting `is_some()`. +/// +/// Sabotage: build the mask in `masking` from `mask.names().take(1)` — under which +/// only this test fails, and it fails by silently offering a tool the operator +/// withheld. +#[test] +fn f6_the_tools_withheld_are_the_tools_the_turn_may_not_call() { + let mask = io_harness::ToolMask::withholding(["docx_write", "write_file"]); + + let contract = io_cli::contract::masking(TaskContract::workspace("a turn", root()), &mask); + + assert_eq!(contract.tool_mask.len(), 2); + assert!(contract.tool_mask.withholds("docx_write")); + assert!(contract.tool_mask.withholds("write_file")); + assert!( + !contract.tool_mask.withholds("grep"), + "a deny set withholds what it names and nothing else", + ); +} + +/// **N4 — the mask is applied beside `contract::session` and never inside it.** +/// +/// This is the arm that makes +/// [`f2_nothing_configured_is_the_contract_the_session_built_before`] +/// *structurally* untouched by this release rather than merely still green. That +/// gate is a Debug equality over everything `session` builds; if the mask were a +/// field set in there — behind an `if` on a posture, say — the gate would go on +/// passing for the session that set no mask and stop saying anything at all about +/// the one that did. `contract::masking` is a sibling of `contract::buying` for +/// exactly that reason, and this counts the sites. +/// +/// Comment lines are stripped before counting: the prose in `contract::masking` +/// names the io-harness method it declines to call, and a gate that a doc comment +/// can break is a gate that gets deleted the first time it is wrong. +/// +/// Sabotage: set the mask inside `contract::session` — under which the count is +/// two and only this test fails, on the one property `f2` can no longer see for +/// itself. +#[test] +fn n4_the_mask_is_set_in_one_place_and_it_is_not_the_session_builder() { + let module = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/contract.rs"); + let text = std::fs::read_to_string(module) + .expect("the contract module") + .replace("\r\n", "\n"); + let code = text + .lines() + .filter(|line| !line.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + + assert_eq!( + code.matches("with_tool_mask(").count(), + 1, + "one call, and it is `contract::masking`'s. A mask set inside \ + `contract::session` would ride the startup reading and both reporting \ + pages, which build contracts nothing runs, and would put a per-turn \ + posture on the one contract this suite compares field for field.", + ); +} + /// **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 f5727ae..1c6877e 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -2242,3 +2242,49 @@ fn f5_every_submission_that_rewrites_the_prompt_also_carries_the_typed_text() { "both echo sites must re-add the slash the parse removed", ); } + +/// **F3 — the cost columns have a caller, and the caller is the driver.** +/// +/// **This gate exists because of 0.31.0 and it is the most important test in this +/// release.** That release shipped `adapt::generate`, `marketplace::fetched`, +/// `Disclosure::withheld` and `orphaned` — every one reachable only from tests, +/// every row it added listable and uninstallable, behind 1,666 green tests. 0.35.0 +/// then shipped a guard whose caller handed it `Vec::new()`, and 0.36.0 shipped a +/// frame builder whose only caller was its own test file. Three releases running, +/// the same shape: the unit is correct, the door does not open it. +/// +/// `servers::costed` and `View::with_costs` are exactly that shape. Both are +/// perfectly testable in isolation and both are inert unless `src/main.rs` calls +/// them — and nothing under `tests/` links `src/main.rs`, so no ordinary test can +/// tell the difference between a wired column and a column that draws +/// "not yet on a request" for every server forever. +/// +/// Asserted on the driver's own text, which is the only instrument that reaches +/// this. Sabotage: delete either call from `src/main.rs`. The whole suite stays +/// green and both surfaces silently stop reporting. +#[test] +fn f3_the_cost_columns_are_filled_by_the_driver_and_not_only_by_tests() { + let driver = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/main.rs"); + let code = code_of(&std::fs::read_to_string(&driver).expect("the driver is readable")); + + for (call, surface) in [("servers::costed(", "/mcp"), (".with_costs(", "/plugin")] { + assert!( + code.contains(call), + "`{call}` has no call site in src/main.rs, so {surface}'s cost column \ + is drawn from an empty map on every keystroke — the 0.31.0 shape, \ + where a release's whole pipeline was reachable only from its tests.", + ); + } + + // And each is fed from the request that actually went out rather than from a + // fresh contract. A `contract::session` here would also trip that function's + // own caller count, which fails at five — so the two facts guard each other. + for needle in ["context::server_cost(", "context::bundle_cost("] { + assert!( + code.contains(needle), + "the driver must fill the column from `{needle}`, which reads the \ + request the provider was handed; a figure computed any other way is \ + io-cli's belief about a catalogue rather than the catalogue", + ); + } +} diff --git a/tests/docs.rs b/tests/docs.rs index ddfac13..c23e018 100644 --- a/tests/docs.rs +++ b/tests/docs.rs @@ -2073,8 +2073,15 @@ fn the_prose_splits_the_commands_the_way_runs_mid_turn_splits_them() { admitted.len(), ); + // **`run`, not `report`, since 0.37.0.** The needle said "report" and that was + // true of all eleven until `/context withhold` was admitted: it changes the + // session's mask rather than describing anything. The word was the claim, so + // the claim moved rather than the count — which did not move at all, because + // `/context` was already in the mid-turn set and gained verbs rather than a + // row. Weakening this to a substring that spans both words would be the + // repair that costs the gate its meaning. let opens = format!( - "**{} commands report while the agent works**", + "**{} commands run while the agent works**", capitalised(count) ); assert!( @@ -2503,3 +2510,75 @@ fn f6_the_headless_guide_carves_out_the_provider_endpoint() { ); } } + +/// **N3 — no shipped page says withholding a tool saves anything.** +/// +/// **This is the release's top risk written as a gate, and it is written because +/// the risk is a sentence rather than a branch.** io-harness offers a masked turn a +/// byte-identical catalogue on purpose: the tool array sits ahead of the provider's +/// cache breakpoint, so dropping a definition would save its tokens once and pay a +/// cache *write* on every later turn (`io-harness-0.76.0/src/tools/mod.rs:33-42`). +/// Withholding in fact makes the request marginally **larger**, by one sentence +/// naming what is withheld (`src/run/prompts.rs:976`). +/// +/// The roadmap entry 0.37.0 was planned from assumed the opposite and said so in +/// its headline. That framing is what a writer reaches for, because "withhold" means +/// "remove" everywhere else in computing — so the wrong sentence is the *natural* +/// one to write and nothing else in the suite can see it. A page claiming a saving +/// compiles, passes every other gate, and misleads the operator into using the +/// wrong lever for the problem they have. +/// +/// Scoped to sentences that put a saving verb near the masking vocabulary rather +/// than to the verbs alone: `docs/guide/accounting.md` legitimately discusses cost +/// and cheapness throughout, and a blanket ban on the word "cheaper" would forbid +/// the page that exists to talk about money from doing so. +/// +/// Sabotage: write "withholding a tool makes the turn cheaper" into any guide page. +/// Every other test in the repository stays green. +#[test] +fn n3_no_shipped_page_claims_a_mask_reduces_what_a_turn_costs() { + // The words that make a sentence about masking into a claim about cost. + const SAVING: [&str; 8] = [ + "saves", + "saving", + "cheaper", + "reduces", + "reduce", + "shrink", + "smaller", + "less context", + ]; + // The vocabulary that makes a sentence be about masking at all. + const MASKING: [&str; 4] = ["withhold", "withheld", "tool mask", "/context allow"]; + + let mut bad = Vec::new(); + for (name, text) in shipped_prose() { + for sentence in flat(&text).split(['.', '!', '?']) { + let lower = sentence.to_lowercase(); + if !MASKING.iter().any(|m| lower.contains(m)) { + continue; + } + // A sentence may say a mask does NOT save — that is the correction + // this release exists to make, and forbidding it would forbid the fix. + let denied = lower.contains("not") + || lower.contains("never") + || lower.contains("nothing") + || lower.contains("no cache") + || lower.contains("does not"); + if denied { + continue; + } + if let Some(word) = SAVING.iter().find(|w| lower.contains(**w)) { + bad.push(format!("{name}: {word:?} in {:?}", sentence.trim())); + } + } + } + assert!( + bad.is_empty(), + "a shipped page claims withholding a tool reduces what a turn costs. It does \ + not: the catalogue sent is byte-identical and the mask ADDS a sentence. \ + This is the one claim 0.37.0 exists to get right, and it is the one the \ + roadmap entry got wrong.\n{}", + bad.join("\n"), + ); +} diff --git a/tests/events.rs b/tests/events.rs index 6925649..2d9aef0 100644 --- a/tests/events.rs +++ b/tests/events.rs @@ -3122,3 +3122,85 @@ fn f5_the_echoed_line_reads_as_a_command_and_not_as_a_prompt() { "the row must read as the command that was typed: {echoed:?}", ); } + +/// **F7.** A withheld call renders as one refused cell naming the mask, and +/// nothing on screen says the tool ran. +/// +/// **The sequence is the one io-harness actually emits, and it is the point of +/// this test.** `announce()` runs at `run/dispatch.rs:182` and `mask_gate` at +/// `:185`, so a withheld call opens a `ToolCall` *before* it is refused — the call +/// is announced, then refused, then the step commits. A test built from the +/// `Refused` event alone would pass over a transcript showing an open call that +/// never resolves, which is what an operator would actually see. +/// +/// io-harness feeds its own sentence back as the step's decision +/// (`"{tool} refused: withheld from this turn"`, `run/read.rs:1230`), so the cell +/// pairs to that rather than to a word io-cli invented. What this asserts is the +/// pairing survives: one call, one decision, so `paired` holds and the cell +/// carries the harness's sentence instead of falling through to the step verdict. +/// +/// Sabotage: drop `layer` from the `Refused` arm's rendered text, or make the +/// refusal close the open call so the cell never commits. The first loses the +/// only words that say *why*; the second loses the cell. +#[test] +fn f7_a_withheld_call_is_one_refused_cell_that_names_the_mask() { + let mut events = Events::new(DARK); + + // 1. The call is announced — before the mask is consulted, upstream. + let opened = events.event( + &event(EventKind::ToolCall { + name: "docx_write".into(), + target: "report.docx".into(), + }), + Duration::ZERO, + ); + assert!( + opened.is_empty(), + "a call still commits nothing when it is announced: {opened:?}" + ); + + // 2. The mask refuses it. io-harness's own act, rule and layer. + let refusal = rendered( + &mut events, + EventKind::Refused { + act: "tool".into(), + target: "docx_write".into(), + rule: Some("docx_write".into()), + layer: Some("turn tool mask".into()), + }, + ); + assert!( + refusal.contains("docx_write"), + "the refusal names the tool: {refusal:?}" + ); + assert!( + refusal.contains("turn tool mask"), + "and names what refused it, which is the fact no other agent prints: {refusal:?}" + ); + + // 3. The step commits anyway — a refusal is an observation, not an ending. + let cell = rendered( + &mut events, + EventKind::Step { + decision: "docx_write refused: withheld from this turn".into(), + tool_call: "docx_write".into(), + tokens: 12, + changed: false, + }, + ); + assert!( + cell.contains("withheld from this turn"), + "the cell carries io-harness's own sentence rather than a bare verdict: {cell:?}" + ); + + // Nothing anywhere in the three may read as the tool having run. `no change` + // is the step verdict the cell falls back to when the decision cannot be + // paired, and it is exactly the wrong thing to say about a call that was + // never started. + let whole = format!("{refusal}\n{cell}"); + assert!( + !whole.contains("no change"), + "a withheld call must not fall through to the step's verdict, which \ + describes a call that ran and changed nothing: {whole:?}" + ); +} diff --git a/tests/glyphs.rs b/tests/glyphs.rs index ed36a6e..ea2eac4 100644 --- a/tests/glyphs.rs +++ b/tests/glyphs.rs @@ -961,6 +961,9 @@ fn plugin_marks() -> Vec<&'static str> { error: "no plugin.toml".to_string(), }], adapters: Some(adapters), + // This sweep collects the row *marks*, and a cost figure carries none — + // an empty map keeps the fixture about the glyph set it is named for. + costs: std::collections::BTreeMap::new(), }; io_cli::pluginview::rows(&view, 200, &ASCII) .iter() diff --git a/tests/live.rs b/tests/live.rs index cb01c37..9e9b17a 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -3991,3 +3991,108 @@ async fn live_f9_whichever_conditional_events_a_real_run_emits() { events.len() ); } + +/// **O4 — a withheld tool is refused by the mask, against a real provider.** +/// +/// **The assertion is that the mechanism was used, never that the file is +/// unwritten.** 0.34.0 shipped a live arm that passed over a feature which placed +/// nothing, because the model found the thing by walking the workspace and the +/// happy string appeared anyway. Here the equivalent trap is louder: a model asked +/// to write a file may simply decline, or write it with a different tool, and the +/// absence of `notes.txt` would look exactly like a working mask. So this asserts +/// io-harness emitted `EventKind::Refused` with `layer: "turn tool mask"` — a fact +/// only the mask can produce — and it deliberately does **not** assert the file is +/// missing. +/// +/// The goal names `write_file` explicitly so the model reaches for the one tool +/// that is withheld. If it never tries, the run produces no refusal and this fails +/// loudly rather than passing on an absence, which is the whole point. +#[tokio::test] +#[ignore = "live: needs OPENROUTER_API_KEY"] +async fn live_f6_a_withheld_tool_is_refused_by_the_mask_and_says_so() { + 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 provider = io_harness::OpenRouter::new(&key, model()); + let policy = workspace_policy(); + + let (answerer, _questions) = io_cli::intent::channel(); + let contract = io_cli::contract::session( + "Create a file called notes.txt containing the single word hello. \ + Use the write_file tool.", + root.to_path_buf(), + &no_configuration(), + &no_configuration().plugins(), + &io_cli::contract::Capabilities::default(), + Arc::new(answerer), + None, + ) + .with_max_steps(6); + + // Through io-cli's own applier, not `with_tool_mask` directly: what is being + // proved is that the product's path from `/context withhold` to the wire + // works, and `contract::masking` is the whole of that path. + let contract = + io_cli::contract::masking(contract, &io_harness::ToolMask::withholding(["write_file"])); + + let collected = Arc::new(Mutex::new(Vec::new())); + let observer = Collector { + events: Arc::clone(&collected), + }; + + let result = session + .turn_bounded_observed(&contract, &provider, &store, &policy, &DenyAll, &observer) + .await + .expect("the turn runs"); + + let events = collected.lock().expect("not poisoned"); + println!("outcome: {:?} kind: {:?}", result.outcome, result.kind); + for event in events.iter() { + println!(" {:?}", event.kind); + } + + // **The mask has two observable outcomes and only one of them is a refusal.** + // The first draft of this arm demanded `EventKind::Refused` and failed against a + // working mask, which is the finding worth keeping: io-harness *announces* the + // mask in the user prompt — "Unavailable this turn — these tools are listed + // above but calling one is refused and starts nothing: write_file" + // (`run/prompts.rs:976`) — so a compliant model never attempts the call and + // never produces the refusal. The run above said so in its own reasoning: "the + // previous turns show write_file was refused, so I used a shell redirect + // instead". That is the mask working at its best, not evidence of absence. + // + // So what is asserted is the guarantee itself: **`write_file` is never + // successfully called.** Either it was attempted and refused with the mask's own + // layer, or it was never attempted. Both are the mask; a successful call is the + // only thing that is not. + let refused = events.iter().any(|event| { + matches!( + &event.kind, + EventKind::Refused { layer, target, .. } + if layer.as_deref() == Some("turn tool mask") && target == "write_file" + ) + }); + let committed = events.iter().any(|event| { + matches!(&event.kind, EventKind::Step { tool_call, .. } if tool_call.starts_with("write_file:")) + }); + println!("refused: {refused} committed a write_file step: {committed}"); + + assert!( + !committed, + "a `write_file` call committed a step while the tool was withheld — the mask \ + did not reach the wire. outcome: {:?}", + result.outcome, + ); + // And the run still did the work, by a route the mask left open. Without this + // the arm would pass on a run that failed for any unrelated reason, which is + // the absence-is-not-evidence trap in its other direction. + assert!( + root.join("notes.txt").exists(), + "the goal was not reached at all, so this arm proves nothing about the mask \ + — it needs a run that got there without the withheld tool. outcome: {:?}", + result.outcome, + ); +} diff --git a/tests/namespacing.rs b/tests/namespacing.rs index 541867e..0869166 100644 --- a/tests/namespacing.rs +++ b/tests/namespacing.rs @@ -557,10 +557,13 @@ fn f7_the_plugin_pane_translates_every_contribution_it_names() { // The pane's list of bundles names no contribution, and the root it draws // is a path like any other. + // No costs: this sweep is about the separator, and a bundle with a cost + // figure draws a number rather than a name — nothing this file asserts. let view = pluginview::View { plugins: vec![listed.clone()], refused: Vec::new(), adapters: None, + costs: std::collections::BTreeMap::new(), }; // `u16::MAX` again: at eighty columns the root is the field that gives // way, and a path dropped for width would fail the assertion below for a diff --git a/tests/pluginview.rs b/tests/pluginview.rs new file mode 100644 index 0000000..a8267c4 --- /dev/null +++ b/tests/pluginview.rs @@ -0,0 +1,378 @@ +//! F12 — what a bundle costs on every request, and the four kinds that cost +//! nothing at all. +//! +//! **The free tier is the half an operator does not expect.** Of the seven kinds +//! io-harness lets a bundle contribute, only skills, agents and MCP servers reach +//! a provider; hooks, prompt templates, a declared binary and a policy layer are +//! never in any request. A bundle contributing only hooks is free forever, and an +//! operator trimming their window should be told that rather than left to guess +//! it — which is why `pluginview::COSTS` is an exhaustive table with a reason +//! beside every decision, and why the first test here reads io-harness's own +//! `Plugin::contributions` and fails **by name** when the two sets differ. +//! +//! **No figure on this surface is something a mask can shave.** io-harness sends +//! a byte-identical tool catalogue on a masked turn and an unmasked one, because +//! the tool array sits ahead of the provider's cache breakpoint. The lever these +//! numbers inform is switching the bundle off, which is the verb `/plugin` +//! already offers. + +mod support; + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use io_cli::context::{self, Request}; +use io_cli::glyphs::ASCII; +use io_cli::pluginview::{self, Listed, View}; +use io_harness::config::{Config, LOCAL_FILE}; +use io_harness::{TaskContract, PLUGIN_FILE}; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +/// A loaded bundle with nothing on it but what each test overrides. +/// +/// Built here rather than off a disk fixture, because every test below except the +/// last one is about the *drawing* rule for a set of contribution kinds, and a +/// manifest that really declared all seven would need a user-scope file and the +/// one environment lock this binary has. +fn listed(contributions: Vec<&'static str>) -> Listed { + Listed { + id: "rust-review".to_string(), + enabled: true, + description: None, + version: None, + root: PathBuf::from("/repo/bundles/rust-review"), + contributions, + skills: None, + templates: None, + agents: Vec::new(), + servers: Vec::new(), + hooks: Vec::new(), + bin: Vec::new(), + layers: Vec::new(), + } +} + +fn view_of(plugin: Listed, costs: BTreeMap) -> View { + View { + plugins: vec![plugin], + refused: Vec::new(), + adapters: None, + costs, + } +} + +/// The detail of the row `id` draws, at a width where nothing has to give way. +fn detail_of(view: &View, id: &str) -> String { + pluginview::rows(view, 400, &ASCII) + .into_iter() + .find(|row| row.label == id) + .and_then(|row| row.detail) + .unwrap_or_else(|| panic!("`{id}` draws a row with a detail")) +} + +/// One bundle's row, for a bundle contributing `contributions` and costing +/// whatever `costs` says. +fn drawn(contributions: Vec<&'static str>, costs: &[(&str, u64)]) -> String { + let costs = costs + .iter() + .map(|(id, tokens)| ((*id).to_string(), *tokens)) + .collect(); + detail_of(&view_of(listed(contributions), costs), "rust-review") +} + +/// Every kind of contribution the locked io-harness can report, read out of the +/// `present` array in its own `Plugin::contributions`. +/// +/// **Read from the dependency rather than listed here**, for the reason +/// `support::harness_error_variants` is: a hand-written list of seven cannot +/// notice an eighth, and an eighth kind classified by nothing would be drawn +/// through whichever arm happens to catch it. +fn harness_contribution_kinds() -> Vec { + let source = support::harness_source("plugin.rs"); + let body = source + .split_once("pub fn contributions(&self) -> Vec<&'static str> {") + .expect("io-harness declares `contributions` in src/plugin.rs") + .1; + let body = body + .split_once("];") + .expect("the `present` array in `contributions` is closed") + .0; + body.match_indices("(\"") + .map(|(at, _)| { + body[at + 2..] + .split('"') + .next() + .expect("a quoted wire name") + .to_string() + }) + .collect() +} + +// --------------------------------------------------------------------------- +// The table is exhaustive, and "free" is a decision +// --------------------------------------------------------------------------- + +/// Sabotage: delete one line from `pluginview::COSTS` — or, equivalently, add an +/// eighth entry to io-harness's own `present` array. Every row still renders and +/// no other test in this repository notices, because an unclassified kind falls +/// through `under_reported`'s conservative arm and simply draws a floor. This +/// fails by name, in both directions. +#[test] +fn f12_every_contribution_kind_is_classified_costing_or_free() { + let kinds = harness_contribution_kinds(); + assert!( + !kinds.is_empty(), + "the reader found no contribution kinds at all, so it is asserting nothing", + ); + let named: Vec<&str> = pluginview::COSTS.iter().map(|(kind, _, _)| *kind).collect(); + + for kind in &kinds { + assert!( + named.contains(&kind.as_str()), + "io-harness lets a bundle contribute `{kind}` and `pluginview::COSTS` does not say \ + what it costs, so a bundle contributing it is drawn by whichever arm happens to \ + catch it: the table names {named:?}", + ); + } + for name in &named { + assert!( + kinds.iter().any(|kind| kind == name), + "`pluginview::COSTS` classifies `{name}`, which io-harness no longer contributes: \ + it names {kinds:?}", + ); + } + + // A decision with no reason beside it is a default wearing a table's + // clothes. Every row of `COSTS` has to say why, or the next release cannot + // check the claim. + for (name, _, why) in pluginview::COSTS { + assert!( + why.len() > 40, + "`{name}` is classified with no reason worth reading: {why:?}", + ); + } +} + +/// Sabotage: make `pluginview::wire` answer `Some(Wire::Free)` for a kind it does +/// not name — `.unwrap_or(Wire::Free)` at either call site. A kind an upgraded +/// io-harness added would then be drawn as costing nothing, permanently, and this +/// is the only test that fails. +#[test] +fn f12_a_kind_the_table_does_not_name_is_never_drawn_as_free() { + // The eighth kind, arriving through a dependency bump: unclassifiable, and + // therefore neither free nor fully counted. + let detail = drawn(vec!["hooks", "resources"], &[("rust-review", 0)]); + assert!( + !detail.contains(pluginview::FREE), + "a kind nothing classifies was drawn as free: {detail:?}", + ); + assert!( + detail.contains(pluginview::AT_LEAST), + "a bundle carrying an uncountable kind draws its figure as if it were the whole of it: \ + {detail:?}", + ); +} + +// --------------------------------------------------------------------------- +// The free tier +// --------------------------------------------------------------------------- + +/// Sabotage: in `pluginview::cost_word`, ask `costs` before asking `free` — swap +/// the leading `if free(..)` below the `match`. A hooks-only bundle then draws +/// `0 tokens every request` on a session that has made a request and +/// `not yet on a request` on one that has not, and the permanent fact is never +/// stated at all. +#[test] +fn f12_a_bundle_contributing_only_hooks_is_stated_free() { + // Both sessions: one that has made no request at all, and one that has made + // a request this bundle contributed nothing to. The claim is the same in + // both, because it is a claim about the table rather than about the wire. + let no_request: &[(&str, u64)] = &[]; + let a_request: &[(&str, u64)] = &[("rust-review", 0)]; + for costs in [no_request, a_request] { + let detail = drawn(vec!["hooks"], costs); + assert!( + detail.contains(pluginview::FREE), + "a bundle contributing only hooks is not stated free: {detail:?}", + ); + assert!( + !detail.contains("tokens"), + "a bundle that is never on a wire was given a token figure: {detail:?}", + ); + } + + // All four free kinds together, because the claim is about the set and not + // about hooks in particular. + let detail = drawn(vec!["templates", "hooks", "bin", "policy"], &[]); + assert!(detail.contains(pluginview::FREE), "{detail:?}"); +} + +/// Sabotage: state `free` from the figure rather than from the table — return +/// `FREE` from `cost_word` whenever `costs.get(&plugin.id) == Some(&0)`. This +/// bundle contributes an MCP server whose tools were not on the last request, and +/// it would then be promised as free forever. +#[test] +fn f12_a_measured_zero_is_not_the_promise_that_it_is_free() { + let detail = drawn(vec!["mcp"], &[("rust-review", 0)]); + assert!( + !detail.contains(pluginview::FREE), + "a server that has offered no tool yet was promised free forever: {detail:?}", + ); + assert!( + detail.contains("0 tokens every request"), + "the measured zero is not drawn as the measurement it is: {detail:?}", + ); +} + +/// Sabotage: read `costs.get(..).copied().unwrap_or(0)` in `cost_word`. A bundle +/// on a session that has made no request would then be reported as free, which is +/// the same lie `servers::NOT_ON_A_REQUEST` exists to stop `/mcp` telling. +#[test] +fn f12_a_bundle_no_request_has_carried_is_not_drawn_as_a_zero() { + let detail = drawn(vec!["skills", "mcp"], &[]); + assert!( + detail.contains(io_cli::servers::NOT_ON_A_REQUEST), + "a costing bundle with no request behind it draws: {detail:?}", + ); + assert!(!detail.contains("0 tokens"), "{detail:?}"); + assert!(!detail.contains(pluginview::FREE), "{detail:?}"); +} + +/// Sabotage: drop the `Wire::EveryUncounted` arm — classify `agents` as +/// `Wire::Every`. The row then presents a figure that leaves out the agent roster +/// as though it were the whole cost, and only this test says so. +#[test] +fn f12_a_figure_that_leaves_out_the_agent_roster_says_so() { + // `context::bundle_cost` deliberately does not count agents: io-harness + // composes the roster into the system block with no marker io-cli can locate, + // so the honest form is a floor rather than an invented number. + let detail = drawn(vec!["agents"], &[("rust-review", 900)]); + assert!( + detail.contains(pluginview::AT_LEAST), + "a bundle whose agents are uncounted draws its floor as a total: {detail:?}", + ); + assert!(detail.contains("900 tokens every request"), "{detail:?}"); + + // A bundle with nothing uncounted in it says no such thing. + let plain = drawn(vec!["skills"], &[("rust-review", 900)]); + assert!( + !plain.contains(pluginview::AT_LEAST), + "a fully counted bundle hedges a figure that is exact: {plain:?}", + ); +} + +/// Sabotage: draw the cost clause outside the `plugin.enabled` arm in +/// `pluginview::rows`. A bundle contributing none of itself to this session then +/// carries a figure or a promise about it, and this fails. +#[test] +fn f12_a_switched_off_bundle_states_no_cost_at_all() { + let mut off = listed(vec!["hooks", "mcp"]); + off.enabled = false; + let costs = BTreeMap::from([("rust-review".to_string(), 4_200_u64)]); + let detail = detail_of(&view_of(off, costs), "rust-review"); + + assert!(detail.starts_with("switched off"), "{detail:?}"); + assert!(!detail.contains("tokens"), "{detail:?}"); + assert!(!detail.contains(pluginview::FREE), "{detail:?}"); + assert!( + !detail.contains(io_cli::servers::NOT_ON_A_REQUEST), + "{detail:?}", + ); +} + +// --------------------------------------------------------------------------- +// The two surfaces cannot disagree +// --------------------------------------------------------------------------- + +/// The head io-harness's `with_skill_catalog` writes, and the namespaced lines +/// under it. The `__` is io-harness's own namespace separator, applied at load. +fn system_with_catalogue() -> String { + "You are a coding agent working in a repository.\n\nSkills available to you: a skill is a \ + folder of instructions you may read when it applies.\n- rust-review__reviewer: read a Rust \ + diff and report the defects in it, with the file and line of each one\n- \ + rust-review__formatter: run the formatter and say what it changed\n- docs-bot__writer: \ + write the documentation for a module\n- housekeeping: a skill the operator put in their \ + own directory, belonging to no bundle\n" + .to_string() +} + +/// A bundle directory contributing a skills directory and nothing else. +fn bundle(root: &Path, at: &str, name: &str) { + let dir = root.join(at); + std::fs::create_dir_all(dir.join("skills")).expect("the skills directory"); + std::fs::write( + dir.join(PLUGIN_FILE), + format!("name = \"{name}\"\nskills = \"skills\"\n"), + ) + .expect("the manifest"); +} + +/// Sabotage: in `pluginview::cost_word`, look the figure up by anything but the +/// bundle's own id — `costs.values().sum()`, or the first entry of the map. The +/// two bundles here cost different amounts, so any such change draws one bundle's +/// charge on the other's row and this fails on both rows. +/// +/// This is the test that stops `/plugin` and `/context` from being able to +/// disagree: the figure drawn is asserted to be `context::bundle_cost`'s own +/// answer for the same request snapshot, through `servers::per_request`, which is +/// the single spelling both surfaces use. +#[test] +fn f12_the_figure_a_row_draws_is_the_one_bundle_cost_computed() { + let dir = tempfile::tempdir().expect("a temporary directory"); + let root = dir.path(); + bundle(root, "review", "rust-review"); + bundle(root, "docs", "docs-bot"); + std::fs::write( + root.join(LOCAL_FILE), + "[[plugin]]\npath = \"review\"\n\n[[plugin]]\npath = \"docs\"\n", + ) + .expect("the configuration"); + + // `Config::discover` reads a user-scope file where one exists, and the + // environment is process-global — the same lock every discovery in + // `tests/plugins.rs` is taken under. + let config = { + let _guard = support::env_lock(); + Config::discover(root).expect("the fixture loads") + }; + let plugins = config.plugins(); + assert!( + plugins.dropped().is_empty(), + "the fixture bundles did not load: {:?}", + plugins.dropped(), + ); + + let contract = plugins.apply_to(TaskContract::workspace("ship it", root)); + let seen = Request { + system: system_with_catalogue(), + ..Request::default() + }; + let costs = context::bundle_cost(&seen, &contract); + + // Two bundles, two different charges, neither of them zero — which is what + // makes a wrong-key lookup visible at all. + let review = *costs.get("rust-review").expect("a row for `rust-review`"); + let docs = *costs.get("docs-bot").expect("a row for `docs-bot`"); + assert!(review > 0 && docs > 0, "{costs:?}"); + assert_ne!( + review, docs, + "the fixture must charge the two bundles differently or a wrong-key lookup survives it", + ); + + let view = { + let _guard = support::env_lock(); + pluginview::view(&plugins) + } + .with_costs(costs.clone()); + + for (id, tokens) in [("rust-review", review), ("docs-bot", docs)] { + let detail = detail_of(&view, id); + assert!( + detail.contains(&io_cli::servers::per_request(tokens)), + "`/plugin` draws {detail:?} for `{id}`, and `context::bundle_cost` says {tokens}", + ); + } +} diff --git a/tests/servers.rs b/tests/servers.rs index 1c38e6e..79d9ff1 100644 --- a/tests/servers.rs +++ b/tests/servers.rs @@ -352,6 +352,134 @@ fn f9_a_server_never_reached_shows_neither_number() { assert!(drawn.contains("not reached this session"), "{drawn:?}"); } +// --- F11: what a server costs on every request -------------------------------- + +/// The same row, with what the last request carried applied to it. +/// +/// Deliberately a second helper rather than a change to [`detail`]: the two are +/// different sources, and a test that could not draw a row *without* the cost map +/// would not be able to assert what an un-costed surface says. +fn costed_detail( + config: &Config, + observed: &Observed, + cost: &BTreeMap, + id: &str, +) -> String { + let mut list = servers::servers(config, observed); + servers::costed(&mut list, cost); + let rows = servers::rows(&list); + let at = list + .iter() + .position(|s| s.id == id) + .expect("a configured id"); + rows[at].detail.clone().expect("every row carries a detail") +} + +/// Sabotage: drop the cost clause from `servers::rows` — restore +/// `let state = if server.enabled { state } else { … }` without the +/// `format!("{state} · {cost}")`. The figure then reaches no operator and only +/// this test says so. +#[test] +fn f11_a_server_on_the_wire_says_what_it_costs_every_request() { + // The figure is not derivable from either count beside it: this server + // announced ten tools and this session has called none of them, and it still + // costs every one of those tokens on every request, because io-harness sends + // the whole catalogue whether a tool is called or not. + let config = configured(); + let mut observed = Observed::default(); + observed.event(&reached_offering("docs", 10)); + let cost = BTreeMap::from([("docs".to_string(), 4_200_u64)]); + + let drawn = costed_detail(&config, &observed, &cost, "docs"); + assert!( + drawn.contains("4.2k tokens every request"), + "the row does not say what the server costs: {drawn:?}", + ); +} + +/// Sabotage: `cost.get(&server.id).copied().or(Some(0))` in `servers::costed`, or +/// `None => per_request(0)` in `servers::rows`. Either one turns "no request has +/// carried this server" into "this server is free", and this is the only test +/// that fails. +#[test] +fn f11_a_server_no_request_has_carried_is_never_drawn_as_a_zero() { + // `search` is demonstrably alive — it announced ten tools this session — and + // is absent from the map, which is what a server that came up after the last + // completion looks like. Absent is "no figure yet", never "costs nothing". + let config = configured(); + let mut observed = Observed::default(); + observed.event(&reached_offering("search", 10)); + let cost = BTreeMap::from([("docs".to_string(), 4_200_u64)]); + + let drawn = costed_detail(&config, &observed, &cost, "search"); + assert!( + drawn.contains(servers::NOT_ON_A_REQUEST), + "a server no request has carried draws {drawn:?}", + ); + assert!( + !drawn.contains("0 tokens"), + "a missing figure was drawn as a measured zero: {drawn:?}", + ); + assert!( + !drawn.contains("tokens every request"), + "a figure was invented for a server that has been on no wire: {drawn:?}", + ); +} + +/// Sabotage: swap the two counts in `servers::rows` — +/// `format!("answered · {tools} offered · {offered} used")`. The row still draws +/// three plausible numbers and this test fails on both of them by name; the +/// values are chosen distinct so that no swap of any pair survives. +#[test] +fn f11_the_three_numbers_on_one_row_each_wear_the_word_that_says_which() { + let config = configured(); + let mut observed = Observed::default(); + observed.event(&reached_offering("docs", 10)); + observed.event(&called("docs", "search_docs", Some(true))); + observed.event(&called("docs", "get_page", Some(true))); + let cost = BTreeMap::from([("docs".to_string(), 512_u64)]); + + let drawn = costed_detail(&config, &observed, &cost, "docs"); + // Ten announced, two called, 512 tokens on the wire: three sources, three + // values, and every one of them attached to the word for its own question. + assert!(drawn.contains("10 offered"), "{drawn:?}"); + assert!(drawn.contains("2 used"), "{drawn:?}"); + assert!(drawn.contains("512 tokens every request"), "{drawn:?}"); + + let at = |needle: &str| { + drawn + .find(needle) + .unwrap_or_else(|| panic!("{needle:?} is not on the row: {drawn:?}")) + }; + assert!(at("offered") < at("used"), "{drawn:?}"); + assert!(at("used") < at("tokens every request"), "{drawn:?}"); +} + +/// Sabotage: draw the cost clause for a switched-off server as well — move the +/// `format!("{state} · {cost}")` outside the `server.enabled` arm. The row then +/// carries a figure for a server that put nothing on any wire, and this fails. +#[test] +fn f11_a_switched_off_server_states_no_figure_at_all() { + // A stale figure is the danger here, not a missing one: the map still holds + // what this server cost before it was switched off, and drawing it would + // report a charge the operator has already stopped paying. + let config = loaded( + "\ +[[mcp]] +id = \"docs\" +transport = \"stdio\" +command = \"mcp-docs\" +enabled = false +", + ); + let cost = BTreeMap::from([("docs".to_string(), 4_200_u64)]); + + let drawn = costed_detail(&config, &Observed::default(), &cost, "docs"); + assert!(drawn.starts_with(servers::DISABLED), "{drawn:?}"); + assert!(!drawn.contains("tokens"), "{drawn:?}"); + assert!(!drawn.contains(servers::NOT_ON_A_REQUEST), "{drawn:?}"); +} + // --- F6: the writes ----------------------------------------------------------- #[test] @@ -869,6 +997,9 @@ fn f6_the_file_a_row_names_is_the_file_its_position_is_read_from() { // an ordinary entry produces — and `declared_at` locates by id, which is // why a switched-off server is still editable and still removable. enabled: true, + // Nothing to do with where an entry is declared: this is what the last + // request carried, and `declared_at` reads a file. + cost: None, }; let at = servers::declared_at(&row).expect("the file names `search`"); assert_eq!(at.index(), 1); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 9ce00b3..dcecb06 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -355,6 +355,19 @@ pub fn harness_source(file: &str) -> String { .replace("\r\n", "\n") } +/// The same, for a file under a subdirectory of `src/` (0.37.0). +/// +/// [`harness_source`] takes one component, and a `"run/prompts.rs"` handed to it +/// becomes a single file name on Windows — where CI runs — failing with "io-harness +/// is not unpacked" rather than anything naming the real mistake. That is the +/// reasoning `harness_source_file` already carries; this is the public door to it, +/// added the first time a gate needed a nested file. +pub fn harness_source_at(components: &[&str]) -> String { + std::fs::read_to_string(harness_source_file(components)) + .expect("io-harness's source is readable from the registry") + .replace("\r\n", "\n") +} + /// Every variant `io_harness::Error` declares, in the source this crate is locked /// to, in declaration order and spelled as Rust spells them. ///