diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3b0aaca..c76f28f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -215,7 +215,7 @@ jobs: # Release titles are the bare form this line of repositories uses: # "io-cli 0.1.0". - - uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3 + - uses: softprops/action-gh-release@efb35369e0ad2afab669f228072c1b0d510eae64 # v3 with: tag_name: v${{ needs.checks.outputs.version }} name: io-cli ${{ needs.checks.outputs.version }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b038bb..7efd9b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,77 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.35.0] - 2026-09-02 + +The release that renders a boundary somebody measured. This interface has drawn the +sandbox backend that *answered* rather than the mode that was asked for since 0.2.0, +deliberately, because those are two different facts — and until io-harness 0.74.0 the +answer could still be false. On macOS a crafted `workdir` re-granted write and network +on `/` while the backend still reported `MacosSandboxExec`; on Windows `shell` and `git` +spawned entirely unwrapped under a reported `WindowsAppContainer`. 0.74.0 makes each +backend try to write outside its roots and dial outside its egress at run start and +records what happened, so what an operator reads is now a measurement. + +That pin is a security release, so taking it *is* most of this release. + +### Changed + +**The io-harness pin moves to 0.74.0, and it refuses much more of what a file inside a +workspace may say.** `[[provider]]`, `[[mcp]]` and `[[lsp]]` are refused from any +project-scoped file; `[[hook]]` is now refused in `io.local.toml` as well as `io.toml`; +a plugin whose manifest sits inside the workspace may not contribute `[[bin]]` or +`[[hook]]`; twelve keys may no longer be widened where five could not before; and +`run.skills`/`run.templates` may not be absolute or climb out with `..`. **A refusal +takes the whole file, not the key.** If you keep any of those in a committed or +workspace-root file, move them to your own — `$IO_CONFIG`, else `$IO_CONFIG_HOME/io.toml`, +else `~/.config/io/io.toml`. io refuses at `io config set` and `/config` before writing, +names the destination, and reports the harness's own sentence when a file is declined. + +**A local provider endpoint is refused at run start, and no configuration key lifts +it.** io-harness 0.74.0 puts a floor under the network boundary: loopback, link-local, +CGNAT, ULA and RFC 1918 addresses, and `localhost`, `*.localhost` and `*.local`, are +refused before the first step *whatever the policy says*. **If you point io at Ollama, LM +Studio or llama.cpp, this release stops working until you set +`IO_HARNESS_ALLOW_LOCAL_ADDRESSES=1`** for that run. The variable is deliberate: the +harness gave the lift no key so it stays an explicit, per-invocation choice. io names it +in the refusal and sets it nowhere. + +**A boundary refusal on the provider endpoint exits `2`.** It exited `1`, which +`docs/CONTRACT.md` publishes as FAILED, because the typed error was flattened to a +string before an exit code was chosen. This is the mirror of the collision 0.34.1 +removed and it is a behaviour change: a script branching on `1` for this case will now +see `2`, which is the documented meaning. Only `Error::Refused` moved — `Error::Sandbox` +is "the sandbox never ran the code" and `Error::Config` is "fix the configuration", and +both still exit `1`. + +**A foreign bundle's adapter ships what it contributes.** `io` generated an adapter +manifest whose `skills` and `templates` were absolute paths into the marketplace clone. +0.74.0 refuses that everywhere — every `*.md` under such a directory reaches the model's +system prompt — so **every foreign-bundle install was broken on this pin**. The adapter +now copies those directories into itself and names them relatively. **Installing again is +therefore how you update**: a `git pull` of the clone changes the clone, not the copy. +`plugin add` says which directories moved, writes no second entry when one already names +the bundle, and leaves a working adapter untouched when a refresh is refused. + +**`/import` writes MCP servers where the harness accepts them.** Every server imported +from Claude or Codex came back refused on this pin, because the import targeted a +project-scoped file. + +### Added + +**The status line, the footer and `/status` name what the backend's probe found.** +`boundary verified` when a write outside the sandbox's roots *and* a dial outside its +egress were both attempted and both refused; `boundary unverified` otherwise. `/status` +prints io-harness's own measurement, which is the only surface with room to separate an +arm that landed from an arm that was never attempted. A run with no containment was +never probed and says nothing, because "unverified" about a measurement nobody took +would be worse than silence. + +**`io exec` applies `[run] templates`.** A headless goal written as `/name` is rendered +from the template of that name, through the same function the palette uses. A goal that +does not begin with `/` is a prompt and is unchanged. The key was never unapplied — it +was applied on one door only, and the limits page named the wrong thing as the gap. + ## [0.34.1] - 2026-09-01 A patch about honesty rather than capability. This product machine-enforces its own @@ -3175,7 +3246,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.34.1...HEAD +[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.35.0...HEAD +[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 [0.34.0]: https://github.com/initorigin/io-cli/compare/v0.33.0...v0.34.0 [0.33.0]: https://github.com/initorigin/io-cli/compare/v0.32.0...v0.33.0 diff --git a/Cargo.lock b/Cargo.lock index 1c519f4..7175a50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.34.1" +version = "0.35.0" dependencies = [ "clap", "crossterm", @@ -1867,9 +1867,9 @@ dependencies = [ [[package]] name = "io-harness" -version = "0.73.0" +version = "0.74.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1de74280eb7a5984f677008c4288922d5dbe17233e2c67688c84b29ede389559" +checksum = "1e279891225dc1fe491c1deffe1d50f1bfc145df19b1f79fad86354065c409ca" dependencies = [ "base64 0.23.1", "calamine", diff --git a/Cargo.toml b/Cargo.toml index b4b5c05..e2ec170 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.34.1" +version = "0.35.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 @@ -63,7 +63,7 @@ path = "src/lib.rs" # still correct about what this *renderer* can be handed; it is simply no longer # load-bearing on the build. Turning `barcode` back off is what would restore # it, and `documents` would then have to be spelled out as its other four. -io-harness = { version = "0.73", features = ["media", "browser", "documents"] } +io-harness = { version = "0.74", features = ["media", "browser", "documents"] } # **`tui-textarea` is gone as of 0.18.0, and it is why this pair was frozen.** # The comment that stood here said ratatui stayed at 0.29 and crossterm at 0.28 # because the composer was built out of that widget and the 0.30 line carried diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index 9f4fa91..d5673b2 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -64,12 +64,14 @@ produce that pause today at all** — it declines every approval rather than def session and headless alike — so the carve-out is what would be printed if it ever did, and is written down because the code that would print it exists and is reached by nothing. -**One boundary refusal does not reach `2` today, and this is a known defect rather than the -contract.** A headless run whose provider endpoint lands in the `ask` tier is refused by io-harness -before the run's first step; that typed refusal is flattened to a message before an exit code is -chosen, so it exits `1`. It is reachable only through an explicit `act = "net", effect = "ask"` -rule matching the provider host — see [the headless guide](guide/headless.md), which describes it -where an operator meets it. Every other refusal exits `2`. +**Every boundary refusal reaches `2`, from either headless door.** A run whose provider endpoint +lands in the `ask` tier is refused by io-harness before the run's first step, and `io resume` +re-authorizes that same endpoint before it carries anything on, so the refusal reaches both doors. +Through 0.34.1 both exited `1` — the typed refusal was flattened to a message before an exit code +was chosen — which that release documented as a known defect rather than fixing, because the +repair changes a published status. It is reachable only through an explicit +`act = "net", effect = "ask"` rule matching the provider host — see +[the headless guide](guide/headless.md), which describes it where an operator meets it. ## Configuration @@ -194,7 +196,7 @@ surfaces and fails on `io_harness::NAMESPACE` appearing in any of them. Invoking as a command is the one place the two forms are side by side on purpose: the scrollback echoes the line you typed, `/ultraship:brainstorm …`, and the prompt behind it carries `ultraship__brainstorm`. -**A bundle's `[[bin]]` is resolvable inside a run and nowhere else.** io-harness 0.73.0 accepts the +**A bundle's `[[bin]]` is resolvable inside a run and nowhere else.** io-harness 0.74.0 accepts the key, resolves the path against the bundle root and places nothing; io-cli **appends** the directory holding that file to its own process `PATH`, which every command a run spawns inherits. Appended and never prepended, so nothing a bundle ships can shadow a system command. No file is written and diff --git a/docs/guide/headless.md b/docs/guide/headless.md index 8f57a59..0945967 100644 --- a/docs/guide/headless.md +++ b/docs/guide/headless.md @@ -37,9 +37,10 @@ provider host reaches this: the harness contributes an allow for that endpoint from a layer of its own, so an ordinary `net`-denying posture still gets to the model, and `io setup` writes no such rule. Worth knowing because nothing warns first — the notice above reads `write` and `exec` and never `net`. **A run refused -this way exits `1` and not the `2` the table below gives for a boundary**: the -typed refusal is flattened to a message before an exit code is chosen. That is a -known defect rather than the contract. +this way exits `2`**, which is what the table below gives for a boundary. Through +0.34.1 it exited `1`: the typed refusal was flattened to a message before an exit +code was chosen, and that release documented the gap rather than closing it, +because closing it changes a published status. ### Exit status @@ -217,7 +218,12 @@ you](resume.md#when-a-run-stops-for-you) for why it cannot be carried on. The exit status is the table above: a resumed run that pauses again exits `4` naming the new pause, and `io resume --list` exits `0` whether or not it found -anything. +anything. **A boundary refusal exits `2` from this door too.** A resume +re-authorizes the provider's own endpoint before it carries anything on, so the +`ask`-tier refusal described above reaches this door exactly as it reaches +`io exec`. The refusals in this section are `1`, and the difference is who spoke: +an interrupted turn, a flag for the wrong pause and a bare run with no `--goal` +are io-cli declining to drive anything, with no boundary asked. ### Managing the configuration without a session diff --git a/docs/guide/limits.md b/docs/guide/limits.md index a5168ad..c4b45c5 100644 --- a/docs/guide/limits.md +++ b/docs/guide/limits.md @@ -19,9 +19,15 @@ own. See [Verification gates](verification.md#verification-gates). **`[run.commi other section, and until there was a commit to author it was a value nothing in this interface had cause to look at. See [Git](git.md#git). -That leaves one key still unapplied, and it has a reason too: `run.templates` is -the thirteenth `[run]` key, reachable only through its own accessor. It is not a -silent omission — this is where it is named. +**Every `[run]` key now reaches both doors.** `run.templates` was named here as +unapplied until 0.35.0, and the sentence was wrong about which half was missing: +the key was read, through its own accessor, but only by an interactive session — +so `io exec` ignored a key the configuration page documents. A configuration key +that works in the terminal and does nothing in CI is worse than one that works +nowhere, because nothing tells you. A headless goal written as `/name` is now +rendered from the template of that name; a goal that does not begin with `/` is a +prompt and is passed through untouched. Both doors render through one function +with one argument list, so a template cannot mean two things. **A price is never invented, and a missing one is never a zero.** io-cli compiles no rates in and estimates nothing, so an install that has connected no provider diff --git a/docs/guide/plugins.md b/docs/guide/plugins.md index 239cf27..9175905 100644 --- a/docs/guide/plugins.md +++ b/docs/guide/plugins.md @@ -247,8 +247,20 @@ GitHub is refused, because the only string io hands `git` is one io built. the generated manifest's directory on the row — that file is what you open when io-harness drops the bundle, and nothing else names it. io writes it under `~/.io-cli/adapters////plugin.toml`, never inside the clone. -The paths in it are absolute and point into the clone, so io-harness loads it as -an ordinary bundle. The author's checkout is not written to. +The directories it contributes are **copied into it** and named relatively, so +io-harness loads it as an ordinary bundle. The author's checkout is not written +to. + +Until 0.35.0 those paths were absolute and pointed back into the clone. io-harness +0.74.0 refuses that in every scope, and the reason is one this product would give +itself: every `*.md` under a contributed directory is read into the model's system +prompt on every turn, so a manifest must ship what it contributes rather than +point at somebody else's checkout. The adapter now holds a copy. + +**So installing again is how you update.** A `git pull` of the marketplace clone +changes the clone, not the copy. `io plugin add ` regenerates the adapter and +tells you which directories moved; it writes no second entry when one already names +the bundle, and a refused refresh leaves the installed adapter exactly as it was. **A plugin name that is not a usable id is refused rather than mangled.** An id is what you type at `plugin add` and what prefixes every name the bundle diff --git a/docs/guide/skills.md b/docs/guide/skills.md index e8dcfa5..865ef1b 100644 --- a/docs/guide/skills.md +++ b/docs/guide/skills.md @@ -78,7 +78,7 @@ io-harness's own `read_skill` tool, under this session's policy, like any other read. So a skill costs the prompt one line until it is relevant, and it is subject to the same boundary as everything else in the session. -**A skill can also read the files beside it.** On io-harness 0.73.0 `read_skill` +**A skill can also read the files beside it.** On io-harness 0.74.0 `read_skill` takes an optional `path`, so a skill whose bundle ships `shared/style.md` or a `references/` directory reaches them through the same tool and the same policy — where before the only route was a shell command, which is a program run for what diff --git a/src/adapt.rs b/src/adapt.rs index b87fca9..e632f07 100644 --- a/src/adapt.rs +++ b/src/adapt.rs @@ -57,16 +57,26 @@ //! found in a marketplace has to acquire one before it can contribute anything. //! [`generate`] writes it. //! -//! **The clone is never written to.** [`crate::marketplace`]'s docs state that the -//! stranger's checkout is not touched, and an adapter would be the first thing to -//! touch it — a file io wrote inside somebody's git working tree is a dirty tree -//! at their next `git pull`. The manifest goes to [`crate::home::adapters`] -//! instead, and every path inside it is **absolute** and points back into the -//! clone. That is not a convenience: -//! [`Plugin::skills_dir`](io_harness::Plugin::skills_dir) is `self.root.join(d)` -//! (`io-harness-0.73.0/src/plugin.rs:311`), and `Path::join` of an absolute -//! argument discards the base — so an absolute `skills` is the one spelling that -//! reaches a directory outside the manifest's own root. +//! **The clone is never written to, and the adapter ships what it contributes.** +//! [`crate::marketplace`]'s docs state that the stranger's checkout is not +//! touched, and an adapter would be the first thing to touch it — a file io wrote +//! inside somebody's git working tree is a dirty tree at their next `git pull`. So +//! the manifest goes to [`crate::home::adapters`] instead, and the two directories +//! it names are **copied out of the clone into the adapter's own root** and +//! written as paths relative to it. +//! +//! Until 0.35.0 those paths were absolute and pointed back into the clone, which +//! worked because [`Plugin::skills_dir`](io_harness::Plugin::skills_dir) is +//! `self.root.join(d)` and `Path::join` of an absolute argument discards the base. +//! io-harness 0.74.0 refuses that spelling for `skills` and for `templates` in +//! **every** scope, `Scope::User` included +//! (`io-harness-0.74.0/src/plugin.rs:1097`), and the reason is the one this +//! product would give: every `*.md` under such a directory is read into the +//! model's system prompt on every turn, so a manifest contributes a directory it +//! *ships* rather than one it points at somewhere else on the machine. Copying is +//! how an adapter comes to ship one. It is redone on every install and every +//! update, so a file the bundle's author deleted does not live on in the adapter, +//! and [`Adapter::copied`] says which directories moved. //! //! **This module writes TOML and still parses none.** `src/edit.rs` is the crate's //! only permitted TOML parser by path (`tests/dependencies.rs`), and that rule is @@ -367,11 +377,50 @@ pub const SKILLS_DIR: &str = "skills"; /// io-harness calls the same thing a **template**, and the two formats are the /// same file: markdown, optional frontmatter, `$ARGUMENTS` for what the operator /// typed. `Templates::discover` takes every `*.md` in a directory -/// (`io-harness-0.73.0/src/template.rs:279`), so what is translated is the -/// directory and never the files in it — nothing is copied and nothing is -/// rewritten. +/// (`io-harness-0.74.0/src/template.rs:279`), so what is translated is the +/// directory and never the files in it — the bytes are copied verbatim into +/// [`TEMPLATES_DIR`] and nothing in them is rewritten. pub const COMMANDS_DIR: &str = "commands"; +/// What [`COMMANDS_DIR`] is called inside the adapter, and the value the +/// generated manifest's `templates` key carries. +/// +/// The key's own word rather than the bundle's, because the adapter is an +/// io-harness bundle: a reader opening `/templates` and a reader reading +/// `templates = "templates"` are looking at one thing, and the foreign format's +/// vocabulary stops at the clone. +pub const TEMPLATES_DIR: &str = "templates"; + +/// The most one contributed directory may weigh, in bytes, before io refuses to +/// copy it into an adapter. 16 MiB. +/// +/// A bundle is a stranger's directory, and copying one is unbounded work over +/// unbounded input unless a number says otherwise. So the ceiling is stated rather +/// than implied, and it is stated here once for both directories. +/// +/// **Measured rather than guessed.** The 48 `skills/`, `commands/` and `agents/` +/// directories published by the marketplaces cloned on the machine this was +/// written on — `zeroonething/ultraship`, `zeroonething/caveman`, +/// `zeroonething/ponytail` and `anthropics/claude-plugins-official` — carry +/// markdown and nothing else. The largest of the 48 is +/// `claude-plugins-official`'s `plugin-dev/skills` at 488,003 bytes over 77 +/// entries, and most are under 40 KiB. 16 MiB is thirty-four times the largest one +/// on that disk, which leaves room for a bundle an order of magnitude bigger than +/// anything published today and still refuses a directory carrying a disk image, a +/// model file or a vendored `node_modules`. +pub const MOST_BYTES: u64 = 16 * 1024 * 1024; + +/// The most entries one contributed directory may hold — files and directories +/// together, at every depth — before io refuses to copy it. 2,000. +/// +/// [`MOST_BYTES`]'s companion, because the two runaways are not the same runaway: +/// a hundred thousand empty files weigh nothing at all and still cost an install +/// its afternoon, one `create_dir` and one `copy` at a time. 2,000 against a +/// measured largest of 77, on the survey [`MOST_BYTES`] describes — twenty-six +/// times the biggest directory any of those marketplaces publishes, and a number +/// small enough that the gate asserting it can build a directory past it. +pub const MOST_ENTRIES: usize = 2_000; + /// The conventional directory a bundle keeps its agent definitions in. pub const AGENTS_DIR: &str = "agents"; @@ -405,6 +454,15 @@ pub struct Adapter { /// Reported rather than dropped, for the module docs' reason: a key that /// silently does nothing is a capability the bundle's author believes in. pub disclosed: Vec, + /// The directories copied out of the clone into the adapter's own root, named + /// as the manifest names them — [`SKILLS_DIR`], [`TEMPLATES_DIR`], or both. + /// + /// **Data, and never a line this module prints.** An adapter is not only a + /// file any more: it holds a copy of somebody else's markdown, taken at + /// install time and replaced at the next one, and an operator wondering why + /// their edit inside the clone changed nothing is owed the sentence that says + /// so. Which surface says it, and in what words, is that surface's business. + pub copied: Vec, } /// The adapter directory for one bundle, under an adapters root. @@ -435,11 +493,21 @@ pub fn at(root: &Path, owner: &str, repo: &str, name: &str) -> PathBuf { /// /// `skills/` becomes `skills`, `commands/` becomes `templates`, each /// `agents/*.md` becomes an `[[agent]]`, and each server in `.mcp.json` becomes an -/// `[[mcp]]`. Every path written is **absolute** and points into the clone: -/// `Plugin::skills_dir` is `self.root.join(d)` -/// (`io-harness-0.73.0/src/plugin.rs:311`) and `Path::join` of an absolute -/// argument discards the base, so an absolute path is what lets a manifest in -/// io's home name a directory in somebody else's checkout. +/// `[[mcp]]`. +/// +/// **The two directories are copied into `into` and named relative to it.** +/// io-harness 0.74.0 refuses an absolute or a `..`-carrying `skills` or +/// `templates` in every scope (`io-harness-0.74.0/src/plugin.rs:1097`), because +/// every `*.md` under one is read into the model's system prompt — so a bundle +/// contributes a directory it ships. An adapter comes to ship one by copying it, +/// and [`Adapter::copied`] reports which moved. The copy refuses a symbolic link +/// rather than following it, refuses anything that is not a plain file or +/// directory, and is bounded by [`MOST_BYTES`] and [`MOST_ENTRIES`]. +/// +/// An `[[mcp]]` path is the exception and stays absolute, because it is not one of +/// the two keys io-harness resolves against the plugin root: a `${CLAUDE_PLUGIN_ROOT}` +/// in a server's argv names a program inside the clone, which is where that +/// program is and where its own files are beside it. /// /// # Validated through io-harness, on the bytes that were written /// @@ -460,7 +528,11 @@ pub fn at(root: &Path, owner: &str, repo: &str, name: &str) -> PathBuf { /// /// A string naming what stopped it, for every case: an unusable id, a bundle that /// cannot be read, a `${...}` that cannot honestly be expanded, an MCP transport -/// io-harness does not speak, and a manifest io-harness itself refuses. +/// io-harness does not speak, a manifest io-harness itself refuses, and a +/// contributed directory io will not copy — a symbolic link, an entry that is +/// neither a plain file nor a directory, or one past [`MOST_BYTES`] or +/// [`MOST_ENTRIES`]. **`into` is removed on every one of them**, so a half-copied +/// adapter is never left where an operator could go on to declare it. pub fn generate(bundle: &Path, name: &str, into: &Path) -> Result { let id = normalised(name).ok_or_else(|| { format!( @@ -473,11 +545,13 @@ pub fn generate(bundle: &Path, name: &str, into: &Path) -> Result Result Result Result plugin, Err(refused) => { // Removed rather than left for an operator to find: a manifest @@ -568,11 +686,41 @@ pub fn generate(bundle: &Path, name: &str, into: &Path) -> Result Result Result<(), String> { + let refuse = |what: &str| { + format!( + "{}: the bundle's `{key}` directory {what}, so io wrote no adapter for it. An adapter \ + ships the directories it contributes rather than pointing at somebody else's \ + checkout, and every `*.md` under this one is read into the model's system prompt on \ + every turn — so io copies what it can vouch for and refuses the rest.", + bundle.display(), + ) + }; + if std::fs::symlink_metadata(from) + .map_err(|e| format!("{}: {e}", from.display()))? + .file_type() + .is_symlink() + { + return Err(refuse("is a symbolic link")); + } + + let mut bytes = 0u64; + let mut entries = 0usize; + let mut pending = vec![(from.to_path_buf(), to.to_path_buf())]; + while let Some((src, dst)) = pending.pop() { + crate::home::create(&dst).map_err(|e| format!("{}: {e}", dst.display()))?; + // Sorted so a refusal names the same entry every time it is run: the one + // thing an operator does with this sentence is go and look at the file. + let mut items = std::fs::read_dir(&src) + .map_err(|e| format!("{}: {e}", src.display()))? + .map(|entry| entry.map(|entry| entry.path())) + .collect::, _>>() + .map_err(|e| format!("{}: {e}", src.display()))?; + items.sort(); + + for item in items { + let held = + std::fs::symlink_metadata(&item).map_err(|e| format!("{}: {e}", item.display()))?; + let kind = held.file_type(); + let Some(name) = item.file_name() else { + continue; + }; + let dest = dst.join(name); + entries += 1; + if entries > MOST_ENTRIES { + return Err(refuse(&format!("holds more than {MOST_ENTRIES} entries"))); + } + if kind.is_symlink() { + return Err(refuse(&format!( + "holds the symbolic link `{}`", + item.display() + ))); + } + if kind.is_dir() { + pending.push((item, dest)); + continue; + } + if !kind.is_file() { + return Err(refuse(&format!( + "holds `{}`, which is neither a plain file nor a directory", + item.display() + ))); + } + bytes += held.len(); + if bytes > MOST_BYTES { + return Err(refuse(&format!("weighs more than {MOST_BYTES} bytes"))); + } + std::fs::copy(&item, &dest).map_err(|e| format!("{}: {e}", dest.display()))?; + } + } + Ok(()) +} + /// The directory `named` points at inside `root`, or the conventional one, or /// `None` where neither is a directory. /// -/// Canonical for [`generate`]'s reason, and it also tidies the spelling: a Codex -/// manifest writes `"./skills/"`, and `root.join("./skills/")` is a path that -/// works and reads as though io-cli did not know where it was pointing. The -/// uncanonical path where that call fails, rather than `None`: `root` is already -/// canonical and the directory was just seen, so a failure here is a race, and -/// dropping the key would answer it by leaving a capability out of the manifest -/// with nobody told. +/// **Not canonicalized, and that is what 0.35.0 changed here.** The answer used to +/// be written into the manifest as an absolute path, where it had to survive being +/// joined onto a root somewhere else; it is now the *source of a copy*, and +/// resolving it would quietly follow a `skills` that is a symbolic link out of the +/// clone — which is one of the two moves [`generate`]'s copy exists to refuse, and +/// a refusal cannot be made about a path that has already been resolved past. +/// `root` is canonical and `inside` passes only plain components, so what comes +/// back is a plain path under it whatever the foreign manifest spelled: a Codex +/// manifest writes `"./skills/"` and the `.` is dropped rather than joined. fn directory(root: &Path, named: Option<&str>, conventional: &str) -> Option { for candidate in named.into_iter().chain(std::iter::once(conventional)) { // **Every component that is not a plain name is dropped, and this is the // one value in the release that decides which directory io reads.** A // manifest's `skills` is a stranger's string; `Path::join` replaces the // base outright on an absolute argument and keeps `..` verbatim, so - // `"skills": "../../.."` would write the operator's own home into a - // generated manifest and `Skills::discover` would read every `*.md` in it - // into the prompt. The same filter `crate::marketplace` applies to a - // source path and to a hooks path, applied here for the same reason. + // `"skills": "../../.."` would name the operator's own home here — which + // 0.35.0 made worse rather than better, because the answer used to be + // written into a manifest and is now the source of a **copy** into io's + // home, where `Skills::discover` goes on reading every `*.md` of it into + // the prompt long after the clone is gone. The same filter + // `crate::marketplace` applies to a source path and to a hooks path, + // applied here for the same reason. let Some(at) = inside(root, candidate) else { continue; }; + // `is_dir` follows a link, so a `skills` that is one is answered here and + // refused by name in the copy rather than skipped in silence — a bundle + // that ships a directory io will not take is told so. if at.is_dir() { - return Some(std::fs::canonicalize(&at).unwrap_or(at)); + return Some(at); } } None @@ -650,8 +912,9 @@ fn shown(path: &Path) -> String { /// string**, which is [`crate::edit::array`]'s argument at the one other place /// this crate spells a value: an absolute Windows path is full of backslashes and /// `\U` opens an escape in a TOML basic string, so `format!("\"{value}\"")` is -/// either a parse error or a *different* path — and a `skills` key that parses to -/// a directory nobody named is the quietest failure this crate can ship. +/// either a parse error or a *different* path — and a `description` carrying an +/// expanded `${CLAUDE_PLUGIN_ROOT}`, which is exactly such a path on Windows, is +/// one of the values that comes through here. /// /// `format!("{value:?}")` is not adequate either, and it is the near miss worth /// naming because it looks as though it would be. `Debug` for `str` escapes for @@ -705,7 +968,7 @@ fn substitution(text: &str) -> Option { /// adapted, so expanding it reads nothing of the host — it writes down a path io /// already knew. `${env:…}`, `${file:…}`, `${cmd:…}` and every shell variable a /// bundle writes are the opposite: io-harness refuses each of them inside a -/// manifest in every scope (`io-harness-0.73.0/src/plugin.rs:841`), because a +/// manifest in every scope (`io-harness-0.74.0/src/plugin.rs:977`), because a /// bundle is a third party's directory even when the file naming it is the /// operator's own. /// @@ -921,7 +1184,7 @@ fn unquoted(said: &str) -> &str { /// /// **A transport io-harness does not speak is a refusal naming the server**, not /// a server quietly left out. io-harness speaks stdio and streamable HTTP -/// (`io-harness-0.73.0/src/mcp.rs:289`), so a bundle declaring `sse` is declaring +/// (`io-harness-0.74.0/src/mcp.rs:301`), so a bundle declaring `sse` is declaring /// a capability no adapter can deliver — and a manifest carrying one fewer server /// than the bundle does is exactly the silent absence this module's docs exist to /// end. The whole generation stops, because a bundle is installed for what it diff --git a/src/commands.rs b/src/commands.rs index 9bcb64c..e2fd735 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1431,7 +1431,7 @@ pub const READ_MARK: &str = "+"; /// The case this page exists for. A file that is there and is not read looks, /// from everywhere else in this product, exactly like one that is read: nothing /// warns, and a missing or skipped instruction file is passed over in silence -/// (`read_instructions`, `io-harness-0.73.0/src/config.rs:2111`). +/// (`read_instructions`, `io-harness-0.74.0/src/config.rs:2680`). pub const UNREAD_MARK: &str = "-"; /// What one row of the memory page stands for. @@ -1647,7 +1647,7 @@ pub fn memory_notes(view: &crate::recall::View, glyphs: &crate::glyphs::Glyphs) /// writing: a file that exists and is not read. It names what would make it /// read rather than only reporting the fact, because `[instructions] files` /// **replaces** the default list rather than adding to it -/// (`DEFAULT_INSTRUCTIONS`, `io-harness-0.73.0/src/config.rs:159`, read at +/// (`DEFAULT_INSTRUCTIONS`, `io-harness-0.74.0/src/config.rs:190`, read at /// `:2120`) and there is nothing on any /// other surface to pull on. pub fn instruction_said( diff --git a/src/configure.rs b/src/configure.rs index 45d9beb..fd99ef9 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -255,7 +255,7 @@ pub enum Kind { /// /// **Both halves are the dependency's since io-harness 0.71.0, and neither is /// written here any more**: the list is `Effect::ALL` -/// (`io-harness-0.73.0/src/policy.rs:127`) and each spelling is `Effect::as_str` +/// (`io-harness-0.74.0/src/policy.rs:129`) and each spelling is `Effect::as_str` /// (`:143`), which is the word io-harness's own deserializer reads. /// /// Until this release io-cli held a copy of both — an array naming three variants @@ -282,7 +282,7 @@ pub fn effects() -> Vec { /// The `ExecMode` variants, spelled by io-harness itself. /// -/// **The list is `ExecMode::ALL` (`io-harness-0.73.0/src/sandbox.rs:424`) and the +/// **The list is `ExecMode::ALL` (`io-harness-0.74.0/src/sandbox.rs:453`) and the /// spellings are `ExecMode::as_str` (`:431`).** io-cli wrote the variant list out /// by hand until this release for a reason that was the dependency's and not a /// choice made here: `ExecMode` is `#[non_exhaustive]` (`sandbox.rs:378-381`), and @@ -386,7 +386,7 @@ pub fn kind_of(key: &str) -> Option { /// preference — but half of the old reason is now false and the correction is /// worth writing down.** io-harness 0.71.0 names its own defaults: /// `DEFAULT_MAX_STEPS` = 8, `DEFAULT_WORKSPACE_MAX_STEPS` = 12 and -/// `DEFAULT_MAX_RETRIES` = 2 (`io-harness-0.73.0/src/contract.rs:652,670,686`), +/// `DEFAULT_MAX_RETRIES` = 2 (`io-harness-0.74.0/src/contract.rs:652,670,686`), /// re-exported at the crate root. "There is nothing to read" was true when this /// was written and is not true now. What is still true is that none of it anchors /// *this* ladder: @@ -537,7 +537,7 @@ pub fn shape_of(key: &str, config: &Config) -> Option { /// The models `[prices.models]` names, across every scope, sorted and deduplicated. /// /// **Read from the dependency's own table since io-harness 0.71.0, not scraped -/// out of the files.** `PriceTable::models` (`io-harness-0.73.0/src/pricing.rs:268`) +/// out of the files.** `PriceTable::models` (`io-harness-0.74.0/src/pricing.rs:268`) /// lists every model the table can actually price, and [`Config::prices`] has /// always built that table out of the three scopes — so the merged question this /// used to hand-roll is precisely the one the accessor answers, and the gap filed @@ -563,7 +563,7 @@ pub fn shape_of(key: &str, config: &Config) -> Option { /// /// **This takes the `Config` the caller already holds, and must never re-discover /// one.** `Config::discover` resolves every `${env:}`, `${file:}` and `${cmd:}` as -/// it reads (`io-harness-0.73.0/src/config.rs:517`), so a second discovery re-runs +/// it reads (`io-harness-0.74.0/src/config.rs:605`), so a second discovery re-runs /// an operator's credential commands — which for a `${cmd:}` fetching a key out of /// a keychain means a Touch-ID prompt raised in order to draw a menu, every time /// the picker opens. Taking a `&Config` is not an optimisation; it is the @@ -600,36 +600,59 @@ pub fn destination(config: &Config, key: &str) -> (Scope, bool) { } } -/// Whether writing `value` to `key` would be refused in a **project-scoped** file. -/// -/// io-harness refuses five (key, value) pairs in a committed `io.toml` -/// (`PROJECT_WIDENING`, `io-harness-0.73.0/src/config.rs:1998-2008`): the two acts -/// defaulted to `allow`, egress re-opened inside the sandbox, the portable floor -/// switched off, and the widest exec mode. The narrowing value of each stays legal, -/// which is what the scope is for. -/// -/// **Mirrored here because a menu that offers a value the destination file will -/// refuse is a menu that lies, and the cost is not one key.** `refuse_widening` -/// runs before deserialization, so the refusal takes the *whole file*: an operator -/// who picks `full-access` on a key their project `io.toml` decides does not get a -/// rejected setting, they get a configuration that no longer parses. `write` -/// already re-reads and refuses with io-harness's own sentence — this is what lets -/// the row say so beforehand instead. -/// -/// The pairs are io-harness's and are spelled here because it exposes no reader for -/// them; `tests/configure.rs` round-trips each one through `Config::from_toml`, so a -/// pair the dependency adds or drops is caught by the gate rather than by an -/// operator. +/// Whether writing `value` to `key` would be refused in a file that lives inside +/// the workspace — `io.toml` or `io.local.toml`. +/// +/// **Asked of io-harness rather than mirrored from it, and the mirror is why.** +/// Until 0.35.0 this function held a hand-copied list of five (key, value) pairs. +/// io-harness 0.74.0's `PROJECT_WIDENING` holds **thirteen pairs over twelve +/// keys**: it added `policy.defaults.read` and `write` (the shipped defaults are +/// `allow` and `ask`, and a project scope *overrides* the user scope, so a cloned +/// file writing `write = "allow"` turned every unmatched write from a question +/// into a silent grant), `sandbox.mode = "workspace-write"` beside `full-access` +/// (the same override raised an operator's own `read-only` back to the default), +/// and each of the five `sandbox.limits.*` written as `0`, which means *no cap*. +/// +/// So the copy was a strict subset of the rule it claimed to state, and `/config` +/// and `io config set` were offering eight values the destination file would +/// refuse. **No test failed**, because the gate iterated io-cli's own list and a +/// list cannot notice what is missing from it. It was found by reading the +/// dependency's diff. +/// +/// Rather than lengthen the copy and wait for the next divergence, this asks the +/// dependency: it writes the one key into a one-line document, hands it to +/// `Config::from_toml` — which parses at `Scope::Project` and runs +/// `refuse_widening` before it deserializes anything — and reports whether the +/// refusal was a *widening* refusal. There is nothing left to keep in step. +/// +/// **Why the answer is worth asking for at all.** `refuse_widening` runs before +/// deserialization, so the refusal takes the *whole file*: an operator who picks +/// `full-access` on a key their project `io.toml` decides does not get a rejected +/// setting, they get a configuration that no longer parses. `write` already +/// re-reads and refuses in io-harness's own sentence — this is what lets the row +/// say so beforehand instead of writing a file that has to be rolled back. +/// +/// **Every value is written quoted, whatever the key's real type.** +/// `refuse_widening` reads the raw table before deserialization and compares the +/// *written* form of a string, a boolean or an integer alike, so `"false"` and +/// `"0"` match the boolean and integer pairs. A value that does not widen goes on +/// to deserialization and may fail there for a type error instead — which is why +/// the answer is the refusal's own clause and not merely `is_err`. #[must_use] -pub fn widens_project(key: &str, value: &str) -> bool { - matches!( - (key, value.trim().trim_matches('"')), - ("policy.defaults.exec", "allow") - | ("policy.defaults.net", "allow") - | ("sandbox.allow_network", "true") - | ("sandbox.force_floor", "false") - | ("sandbox.mode", "full-access") - ) +pub fn widens_workspace(key: &str, value: &str) -> bool { + /// The clause io-harness's widening refusal always carries + /// (`io-harness-0.74.0/src/config.rs:2509`). Matched rather than the whole + /// sentence, which interpolates the path, the key and the destination scope. + const WIDENS: &str = "widens the boundary"; + + let value = value.trim().trim_matches('"'); + let escaped = value.replace('\\', "\\\\").replace('"', "\\\""); + let document = match key.rsplit_once('.') { + Some((table, leaf)) => format!("[{table}]\n{leaf} = \"{escaped}\"\n"), + None => format!("{key} = \"{escaped}\"\n"), + }; + + Config::from_toml(&document).is_err_and(|refusal| refusal.to_string().contains(WIDENS)) } /// The `/config` row that re-reads the price catalogue. @@ -753,7 +776,7 @@ fn is_credential(path: &str) -> bool { /// /// **There are three substitution forms and not two.** io-harness resolves /// `${env:...}`, `${file:...}` **and** `${cmd:...}` -/// (`substitute`, `io-harness-0.73.0/src/config.rs:2150`, the `cmd` arm at +/// (`substitute`, `io-harness-0.74.0/src/config.rs:2719`, the `cmd` arm at /// `:2241`); this comment claimed two until /// 0.21.0, and the sentence it claimed it in was the argument for which forms /// pass through here. The third is deliberately not one of them: a `${env:}` or @@ -894,28 +917,45 @@ pub fn scope_path(root: &std::path::Path, scope: Scope) -> Option { } } -/// What to print when the configuration cannot be read at all. +/// What to print when io-harness will not take the configuration. /// /// **The whole of io-harness's sentence, and one line of io-cli's own.** A -/// `Config::discover` that fails is not always a broken file: since io-harness -/// refuses a project-scoped `[[hook]]` outright — a hook runs a command, and -/// `io.toml` is the file a `git clone` delivers — the commonest way for a -/// perfectly well-formed repository to stop io from starting is a table somebody -/// added in good faith to the wrong one of three files. -/// -/// io-harness's own message names the key, says why, and names the two files that -/// may carry it. Nothing io-cli could write would be better, so it is passed -/// through whole. What io-cli adds is the one thing the harness cannot know: which -/// directory was being read, because an operator who ran `io` in the wrong place -/// is looking at a message about a file they have never opened. +/// `Config::discover` that fails is very often not a broken file at all: io-harness +/// refuses whole sections outright from any file that lives in a workspace, so the +/// commonest way for a perfectly well-formed repository to stop io from starting is +/// a table somebody added in good faith to the wrong file. +/// +/// **0.74.0 made that far more likely and this sentence had to change with it.** +/// The refused set grew from `[[hook]]` and `[browser]` to include `[[provider]]`, +/// `[[mcp]]`, `[[lsp]]` and `[web]`; `[[hook]]` is now refused in `io.local.toml` +/// as well as `io.toml`; ten policy and sandbox values may no longer be widened; +/// and `run.skills`/`run.templates` may not be absolute or climb out with `..`. An +/// operator who clones a repository can meet three of those at once. +/// +/// **"was not accepted" rather than "could not be read", and the wording is the +/// point.** A refusal is not a malfunction — the file parsed, and io-harness +/// declined what it said. "Could not be read" told an operator their file was +/// broken when it was intact and in the wrong place, which is a different problem +/// with a different fix. The phrasing covers both cases honestly without io-cli +/// having to classify which one it is: `Error::Config` carries no structure, only +/// a sentence, so any classification here would be a list of the harness's clauses +/// that goes stale the next time it tightens — which is exactly the mistake +/// `widens_workspace` was rewritten to stop making. +/// +/// io-harness's own message names the key, says why, and names the scope that may +/// carry it. Nothing io-cli could write would be better, so it is passed through +/// whole. What io-cli adds is the one thing the harness cannot know: which +/// directory was being read, because an operator who ran `io` in the wrong place is +/// looking at a message about a file they have never opened. /// /// **This lives here and not in `main.rs`.** Nothing under `tests/` links the /// binary, so a sentence composed in that file is one no test drives and no /// sabotage can make fail — the same reasoning that put plain-mode resolution in -/// the library. +/// the library. It is also why all three doors that meet a refusal route through +/// this one function. pub fn refusal(root: &std::path::Path, error: &io_harness::Error) -> String { format!( - "the configuration in {} could not be read:\n{error}", + "the configuration in {} was not accepted:\n{error}", root.display() ) } @@ -979,7 +1019,15 @@ pub fn write( pub fn reload( root: &std::path::Path, ) -> Result<(Config, Option), String> { - let config = Config::discover(root).map_err(|e| e.to_string())?; + // **Through `refusal` and not `to_string`, so all three doors say one thing.** + // The startup door has framed a refused configuration since 0.16.0; this one + // handed back io-harness's bare sentence, so the same `[[mcp]]` in the same + // file read as a refusal at start-up and as an unattributed error at the turn + // boundary that noticed the edit — and the reload door is the one an operator + // meets *while working*, having just saved the file. It also gains the root, + // which is the fact io-harness cannot supply and the one that matters when the + // refused file is in a directory the operator has never opened. + let config = Config::discover(root).map_err(|error| refusal(root, &error))?; let (stored, _) = crate::settings::stored(&config); Ok((config, stored)) } diff --git a/src/contract.rs b/src/contract.rs index 43538da..122efb9 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -535,7 +535,7 @@ pub fn skills_dir(config: &Config, capabilities: &Capabilities, root: PathBuf) - /// **The existence test is not caution, it is the whole of what makes this /// default safe.** `Skills::discover` does not return early on a directory that /// is not there — it returns `Error::Config("skills directory … does not exist")` -/// (`io-harness-0.73.0/src/skills.rs`), and `TaskContract::discover_skills` +/// (`io-harness-0.74.0/src/skills.rs`), and `TaskContract::discover_skills` /// propagates it from `run.rs` at run start, before the first completion. A /// contract that named this directory unconditionally would therefore fail every /// turn of every operator who has never made one, which is almost all of them. @@ -560,7 +560,7 @@ fn default_skills() -> Option { /// /// **One expansion for two keys, applied after both have had their say.** /// io-harness substitutes `${env:…}`, `${file:…}` and `${cmd:…}` and nothing else -/// (`substitute`, `io-harness-0.73.0/src/config.rs:2150` — there is no tilde +/// (`substitute`, `io-harness-0.74.0/src/config.rs:2719` — there is no tilde /// branch anywhere in it, and 0.71.0 narrowed the forms rather than widening /// them: a plugin manifest now refuses all three), so a `~` an operator wrote in /// `[run] skills` or `[app.io-cli] skills` diff --git a/src/exec.rs b/src/exec.rs index aa1fd64..8dae83f 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -225,6 +225,135 @@ pub fn verified_code(outcome: &RunOutcome, standing: Option<&crate::gates::Stand } } +/// The layer io-harness 0.74.0 stamps on a refusal from its local-address floor. +/// +/// **Matched rather than re-derived, and that is the whole design of this +/// notice.** The floor refuses a loopback, link-local, CGNAT, ULA or RFC 1918 +/// provider endpoint — and `localhost`, `*.localhost`, `*.local` — before the +/// run's first step, *whatever the policy says*. io-cli could inspect the +/// configured endpoint and decide for itself which of those it is; it does not, +/// because 0.30.0 shipped a copy of one of this dependency's address checks and it +/// **failed open** on five shapes, including a URL where a bracketed host +/// swallowed the real one. A copy's test table is written from the copier's +/// imagination rather than from the original's bug list. Reading the layer off the +/// refusal the harness actually produced is right for every shape it refuses now +/// and every shape it adds later. +/// +/// The string is spelled here because io-harness's own `FLOOR_LAYER` is +/// `pub(crate)`. `f11_the_local_address_floor_layer_is_spelled_as_io_harness_spells_it` +/// holds this against the locked source, so a rename upstream fails a test that +/// names it rather than silently losing the remedy. +pub const LOCAL_ADDRESS_FLOOR: &str = "local-address floor"; + +/// What to do about it, which the harness deliberately does not say. +/// +/// io-harness gives this lift **no configuration key** on purpose: it is meant to +/// be an operator's explicit, per-invocation choice rather than something a file +/// can grant. io-cli respects that — it names the variable and sets nothing. +/// `f11_io_cli_never_sets_the_local_address_variable` is the gate, and it is a +/// source-text gate because an absence has no other site. +const LOCAL_ADDRESS_REMEDY: &str = "io: this is the harness's local-address floor, and no \ + configuration key lifts it. If you meant to reach a model on this machine — Ollama, LM \ + Studio, llama.cpp — set IO_HARNESS_ALLOW_LOCAL_ADDRESSES=1 for the run that should be \ + allowed out to it."; + +/// A failure that reached the harness, carrying the exit status it earns. +/// +/// **It exists because `to_string()` at the two headless doors threw away the one +/// thing the table above needs.** io-harness answers a boundary refusal with a +/// typed `Error::Refused`, and both doors flattened every harness error to a +/// `String` before an exit code was chosen — so `main.rs`, which has only +/// `Err(_) -> FAILED` to work with, exited `1` for a run the policy refused. That +/// is the same class of defect 0.34.1 removed from the other end of this table: a +/// script is told the wrong thing by the one surface this product offers to be +/// scripted against. Documented as a known defect by 0.34.1 rather than fixed, +/// because it is a behaviour change and 0.34.1 was a patch. +/// +/// `message` is the error's own `Display` and nothing else, so the sentence an +/// operator reads is byte-identical to the one they read before — only the status +/// beside it moves. +#[derive(Debug)] +pub struct Ending { + /// The exit status: [`REFUSED`] for a boundary refusal, [`FAILED`] for + /// everything else. + pub code: u8, + /// What the operator is told, taken from the error itself. + pub message: String, +} + +impl std::fmt::Display for Ending { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl From for Ending { + /// **The arm is one variant wide, and that is the whole audit.** + /// + /// `Error::Refused` is the only variant io-harness types as a boundary + /// saying no — its own documentation says it is "typed separately from + /// `Error::Config` so a refusal is distinguishable from a malfunction". The + /// two neighbours a wildcard would swallow are the two that must not move: + /// `Error::Sandbox` is "the sandbox failed to start", typed apart from + /// `Error::Io` so a caller can tell "the sandbox never ran the code" from + /// "the code ran and failed"; and `Error::Config` is "configuration was + /// missing or invalid", which the crate's own example handles as "fix the + /// configuration". Reporting either as `REFUSED` would tell a CI job a + /// boundary had spoken when nothing had — the mirror of the defect this + /// removes, and the third time this table would have been given away by a + /// convenient catch-all. + /// + /// So everything else — including every variant a later harness adds, which + /// `#[non_exhaustive]` guarantees there will be — is [`FAILED`]: the run + /// reached the harness and did not run, and only a refusal has a code of its + /// own to claim. + fn from(error: io_harness::Error) -> Self { + Self { + code: match &error { + io_harness::Error::Refused { .. } => REFUSED, + _ => FAILED, + }, + message: match &error { + io_harness::Error::Refused { + layer: Some(layer), .. + } if layer == LOCAL_ADDRESS_FLOOR => { + format!("{error}\n{LOCAL_ADDRESS_REMEDY}") + } + _ => error.to_string(), + }, + } + } +} + +impl From for Ending { + /// The resume door's own error type, which carries the harness's inside + /// [`crate::resume::Failure::Harness`] and answers for the rest itself. + /// + /// Every other variant is io-cli's own refusal to drive a resume — a run that + /// was interrupted, a question that belongs elsewhere, a head that moved — + /// and each is [`FAILED`] for the reason `io exec --policy ask-writes` is: + /// the boundary never got a chance to say anything. + /// + /// This door is not a copy of the other one's problem, it is the same problem + /// with more ways in. All four drivers funnel into + /// `io_harness::resume_with_observed`, which calls `authorize_provider` before + /// it carries anything on — so the provider-endpoint refusal that reaches + /// `io exec` reaches every resume too. io-harness 0.74.0 then adds two shapes + /// only a resume can raise, a persisted approval it cannot replay and one + /// rewritten since the checkpoint; `io` produces neither today, for the reason + /// it produces no approval pause at all, and both are typed the same way and + /// would land here already handled. + fn from(failure: crate::resume::Failure) -> Self { + match failure { + crate::resume::Failure::Harness(error) => Self::from(error), + other => Self { + code: FAILED, + message: other.to_string(), + }, + } + } +} + /// One line for stderr, naming the outcome and the harness's own step count. /// /// The number is read off the returned value and never recounted from the store: @@ -475,7 +604,7 @@ pub async fn turn( goal: String, sandbox: Option, observer: &dyn Observer, -) -> Result { +) -> Result { session .turn_bounded_observed( &contract(config, session, goal, sandbox), @@ -500,7 +629,7 @@ pub async fn turn( observer, ) .await - .map_err(|error| error.to_string()) + .map_err(Ending::from) } /// The task contract, assembled from the harness's defaults, the configuration @@ -604,6 +733,35 @@ pub fn spec_for( } } +/// A headless goal, with a prompt template expanded where one was named. +/// +/// **`[run] templates` was never unapplied — it was applied on one door only, and +/// the limits page named the wrong thing as the gap.** `commands::templates` has +/// read the key through its own accessor since the palette learned templates, but +/// only an interactive session ever called it, so `io exec` ignored a key +/// `docs/config.example.toml` documents. A configuration key that works in the +/// terminal and silently does nothing in CI is the asymmetry this product deleted +/// in 0.14.0, arriving again through a different door. +/// +/// **`/name` and nothing else, which is the session's own spelling.** A goal that +/// does not begin with `/` is a prompt and is passed through untouched — the +/// overwhelmingly common case, and one this must not change. A goal that does is +/// rendered through [`crate::commands::expand`], the *same* function the palette +/// calls, with the same empty argument list: two doors rendering one template two +/// ways is exactly the divergence this exists to remove. +/// +/// The notice is returned rather than printed, because this module prints on +/// stderr from one place and a library function that writes to a stream is one no +/// test can read back. +pub fn goal_for(config: &Config, goal: &str) -> Result<(String, Option), String> { + let Some(name) = goal.strip_prefix('/') else { + return Ok((goal.to_string(), None)); + }; + let (templates, notice) = crate::commands::templates(config); + let rendered = crate::commands::expand(&templates, name)?; + Ok((rendered, notice)) +} + /// `io exec`, from the command line to an exit status. pub async fn main( args: crate::cli::Exec, @@ -615,6 +773,14 @@ pub async fn main( // a refused posture costs nothing and leaves no run behind. let posture = args.policy.map(posture_for).transpose()?; + // And before those too: a template that does not resolve is a goal that does + // not exist, and a run started on one would spend a provider call to say so. + let (goal, templates_notice) = goal_for(&config, &args.goal)?; + if let Some(notice) = templates_notice { + eprintln!("io: {notice}"); + } + let args = crate::cli::Exec { goal, ..args }; + // **A bundle's own program, placed here as well as on the session's startup.** // This door returns from `main` before the interactive path resolves // anything, so the placement written there reaches neither headless door. @@ -710,7 +876,20 @@ impl WithProvider for Headless { self.args.sandbox.map(crate::cli::Sandbox::mode), watcher, ) - .await?; + .await; + // **Printed here and returned as a status, rather than propagated as an + // `Err`.** `main.rs` has one answer for an `Err` and it is [`FAILED`], so a + // `?` on this line is what made a refused run exit `1`. The sentence is the + // same sentence `main.rs` would have printed, on the same stream, with the + // same prefix — only the number beside it is now the one the table + // publishes. + let result = match result { + Ok(result) => result, + Err(ending) => { + eprintln!("io: {ending}"); + return Ok(ending.code); + } + }; // stdout is the data and stderr is everything else, so that // `io exec --json … | jq` needs no filtering and a plain run can be @@ -1304,7 +1483,7 @@ impl WithProvider for Resuming { // other pause kind has both forms — `resume_tree_with_answer` beside // `resume_with_answer`, `resume_tree_with_plan_decision` beside its flat // one, `resume_tree_with_decision` beside `resume_with_decision` - // (`io-harness-0.73.0/src/run.rs:1770`, `:2089`, `:3003`). Recovery has + // (`io-harness-0.74.0/src/run.rs:1790`, `:2089`, `:3003`). Recovery has // `resume_with_recovery_observed` (`:2551`) and nothing tree-aware, so it // is the one pause a contained run cannot be resumed from. Not an oversight // this crate can route around: a fleet's shared ceiling lives in the tree @@ -1379,7 +1558,16 @@ impl WithProvider for Resuming { .await } } - .map_err(|failure| failure.to_string())?; + .map_err(Ending::from); + // The same seam `Headless::call` has, for the same reason: this door reaches + // a boundary refusal too, and 0.74.0 gave it two of its own. + let resumed = match resumed { + Ok(resumed) => resumed, + Err(ending) => { + eprintln!("io: {ending}"); + return Ok(ending.code); + } + }; // stdout is the data and stderr is everything else, the split `io exec` // already makes. diff --git a/src/home.rs b/src/home.rs index 2efdb20..abab417 100644 --- a/src/home.rs +++ b/src/home.rs @@ -9,7 +9,7 @@ //! This module names one. It is deliberately **not** a second configuration //! system: io-harness resolves `$IO_CONFIG`, then `$IO_CONFIG_HOME`, then the //! platform's own place, reading the environment at call time -//! (`io-harness-0.73.0/src/config.rs:1891`, the Windows branch at `:1910`), and +//! (`io-harness-0.74.0/src/config.rs:2096`, the Windows branch at `:1910`), and //! there is no caller-supplied home //! anywhere in `Config`'s public surface — no `discover_in`, no builder. So the //! one lever is `$IO_CONFIG_HOME`, set once, before the first diff --git a/src/import.rs b/src/import.rs index c28e5bf..bddb589 100644 --- a/src/import.rs +++ b/src/import.rs @@ -30,7 +30,7 @@ //! decision="allow")` and Claude's `permissions.ask` says `Bash(cargo yank *)` — //! both of which match a *command line*. io-harness's `Act::Exec` matches a //! **binary name and nothing else**; it has no argument matching at all -//! (`Act::Exec`, `io-harness-0.73.0/src/policy.rs:63`). So the nearest thing to a faithful +//! (`Act::Exec`, `io-harness-0.74.0/src/policy.rs:63`). So the nearest thing to a faithful //! import of `bun install` is a blanket allow on `bun`, which is a wider //! permission than the operator ever granted, written by a tool they were //! trusting to be careful. A boundary half-imported is worse than one left alone, @@ -44,6 +44,35 @@ //! hands the caller the [`crate::edit::Edit`] once a vendor has been chosen, //! built by [`crate::providers::add`] rather than by a second speller here. //! +//! # An MCP server lands in the user scope, whatever scope was asked for +//! +//! io-harness 0.74.0 refuses `[[mcp]]` in **any** configuration file under the +//! workspace root — `io.toml` and `io.local.toml` alike — because such a file +//! arrives with a `git clone`, or sits in a root the run's own agent can write to, +//! and an `[[mcp]]` entry is a command line this process spawns at run start. The +//! user scope is the sole exemption, and it is the right destination here for the +//! same reason it is the exemption: `$IO_CONFIG` is outside every workspace, so a +//! run that can write its own root cannot reach it — and an import is the +//! operator's own act on their own machine rather than something a repository +//! handed them. +//! +//! So [`plan`]'s `scope` argument decides where **instructions** go, and nothing +//! else. Threading it into the `[[mcp]]` items too is what made every server +//! `/import` offered come back refused: [`crate::configure::write`] round-trips +//! through `Config::discover`, the harness refused the section, and the file was +//! rolled back with nothing in it. Choosing the scope here rather than letting a +//! caller pass one is what makes that unrepeatable — there is exactly one scope an +//! `[[mcp]]` entry can be written to, so it is not a caller's decision to get +//! wrong. +//! +//! Nothing else this module writes has the problem. Instructions are markdown +//! through [`crate::memory::remember`], and the `[instructions] files` list that +//! makes them readable is written by `memory::install` at [`Scope::User`] already; +//! a skill is a plain file under io's own home; a model and an allowlist write +//! nothing at all, and the `[[provider]]` a caller may build from +//! [`Item::provider_edit`] — the one other section 0.74.0 moved to the user scope +//! — is written there by its caller for the same reason. +//! //! # The ceiling is the feature //! //! `Skills::discover` rejects a directory holding more than @@ -379,6 +408,11 @@ pub enum Destination { Instructions(Scope), /// A `[[mcp]]` entry in the configuration file for this scope, through /// [`crate::configure::write`]. + /// + /// Everything [`plan`] builds carries [`Scope::User`] here, and the module + /// note says why: it is the only scope io-harness 0.74.0 lets an `[[mcp]]` + /// entry live in. The scope is still carried rather than assumed, so [`apply`] + /// writes to the file [`Destination::path`] already showed the operator. Config(Scope), /// A file written at exactly this path. File(PathBuf), @@ -476,9 +510,11 @@ impl Item { /// expected can look at [`detect`]'s output to see what was there; an operator /// seeing a plan that half-applied could not. /// -/// `scope` is where instructions and configuration go — [`Scope::Project`] is the -/// answer for a repository the operator is standardising, [`Scope::User`] for a -/// machine they are moving onto. +/// `scope` is where **instructions** go — [`Scope::Project`] is the answer for a +/// repository the operator is standardising, [`Scope::User`] for a machine they +/// are moving onto. It does not reach the `[[mcp]]` items: io-harness 0.74.0 +/// allows that section in the user scope alone, so there is no choice to make and +/// the module note gives the reasoning. #[must_use] pub fn plan(found: &[Found], home: &Path, scope: Scope) -> Vec { let mut items: Vec = Vec::new(); @@ -526,12 +562,12 @@ pub fn plan(found: &[Found], home: &Path, scope: Scope) -> Vec { ) => items.extend(instructions(one.source, path, scope)), (Source::Claude, "settings.json") => items.extend(claude_settings(path)), (Source::Claude, ".claude.json") => { - items.extend(foreign_mcp(one.source, path, scope)); + items.extend(foreign_mcp(one.source, path)); } - (Source::Codex, "config.toml") => items.extend(codex_config(path, scope)), + (Source::Codex, "config.toml") => items.extend(codex_config(path)), (Source::Codex, "default.rules") => items.extend(codex_rules(path)), (Source::Gemini, "mcp_config.json") => { - items.extend(foreign_mcp(one.source, path, scope)); + items.extend(foreign_mcp(one.source, path)); } // Every skill file has already been taken by the guard above, so // what reaches here is a file [`files`] produces and no arm @@ -638,7 +674,7 @@ struct ForeignProject { /// io-harness reads as two servers under one name, and every tool call after that /// is ambiguous — so the duplicate is dropped here rather than written and /// discovered later. -fn foreign_mcp(source: Source, path: &Path, scope: Scope) -> Vec { +fn foreign_mcp(source: Source, path: &Path) -> Vec { let Ok(text) = std::fs::read_to_string(path) else { return Vec::new(); }; @@ -665,15 +701,7 @@ fn foreign_mcp(source: Source, path: &Path, scope: Scope) -> Vec { continue; } let names: Vec = server.env.into_keys().collect(); - items.push(mcp_item( - source, - path, - scope, - &id, - &command, - server.args, - &names, - )); + items.push(mcp_item(source, path, &id, &command, server.args, &names)); } items } @@ -683,7 +711,7 @@ fn foreign_mcp(source: Source, path: &Path, scope: Scope) -> Vec { /// Read through [`crate::edit::value_at`] and [`crate::edit::sections`] rather /// than by parsing — see the module note on why this file may not hold a second /// TOML reader. -fn codex_config(path: &Path, scope: Scope) -> Vec { +fn codex_config(path: &Path) -> Vec { let Ok(text) = std::fs::read_to_string(path) else { return Vec::new(); }; @@ -718,15 +746,7 @@ fn codex_config(path: &Path, scope: Scope) -> Vec { .map(|value| strings(&value)) .unwrap_or_default(); let names = env_names(&text, &name); - items.push(mcp_item( - Source::Codex, - path, - scope, - &name, - &command, - args, - &names, - )); + items.push(mcp_item(Source::Codex, path, &name, &command, args, &names)); } items @@ -783,10 +803,15 @@ fn env_names(text: &str, server: &str) -> Vec { /// Each key is rendered with `toml::Value`'s own inline form, which is why `env` /// comes out as an inline table: an appended `[[mcp]]` block cannot carry a /// `[mcp.env]` header after it without that header attaching to the wrong entry. +/// +/// **The scope is decided here and is not an argument.** io-harness 0.74.0 refuses +/// `[[mcp]]` in every file under the workspace root, so [`Scope::User`] is not the +/// best of three destinations, it is the only one that exists — see the module +/// note. A `scope` parameter here would be a way to build an item that cannot be +/// written, which is precisely what it used to be. fn mcp_item( source: Source, from: &Path, - scope: Scope, id: &str, command: &str, args: Vec, @@ -823,7 +848,7 @@ fn mcp_item( source.word() ), to: if form.is_some() { - Destination::Config(scope) + Destination::Config(Scope::User) } else { Destination::Nowhere }, @@ -1206,7 +1231,7 @@ pub fn apply(plan: &[Item], root: &Path) -> Report { // same server declared by two tools — `context7` and `playwright` // are in both Claude's and Gemini's files on a real machine — // arrived twice; and nothing at all compared the plan against what - // `io.toml` already declared, so a second `/import`, or the + // the configuration already declared, so a second `/import`, or the // first-run offer followed by `/import` later, appended a second // entry under an id that was already there. io-harness validates // no such thing, so `configure::write`'s round trip accepts it and diff --git a/src/main.rs b/src/main.rs index e579c0d..212d583 100644 --- a/src/main.rs +++ b/src/main.rs @@ -48,13 +48,30 @@ fn main() -> ExitCode { // than patched at each early return, because the next early return would not // know to do it. let mut report = Vec::new(); - match runtime.block_on(run(&mut report)) { + let outcome = runtime.block_on(run(&mut report)); + // **Drained on the way out rather than inside an arm, because the hole was the + // arm that did not do it (0.35.0).** Until this release only `Err` delivered + // what was left, and the wizard's own decline is `Ok`: `io setup` answered with + // Esc returns `Ok(exec::OK)` from `run` with the report untouched. So an + // operator upgrading from a pre-0.15.0 install had `io.toml`, `runs.db` and the + // rest moved into `~/.io-cli`, pressed one key, and was told nothing at all — + // with their old directory now empty. That is the exact reading the comment + // above says this mechanism exists to prevent, and the same arm was swallowing + // the line that says a file could **not** be moved because another `io` holds + // it. + // + // Draining here rather than adding a second `for` to the `Ok` arm is the point: + // every arm that has somewhere better to speak has already taken the report by + // `std::mem::take`, so this sees only what nobody said — and the next early + // return that gets written cannot reopen the hole by not knowing about it. + // + // Printed after the terminal has been restored, never into raw mode. + for line in &report { + eprintln!("{line}"); + } + match outcome { Ok(code) => ExitCode::from(code), Err(error) => { - // Printed after the terminal has been restored, never into raw mode. - for line in report { - eprintln!("{line}"); - } eprintln!("io: {error}"); ExitCode::from(io_cli::exec::FAILED) } @@ -3476,15 +3493,18 @@ async fn loop_over Result>( } else if let Some(value) = index.checked_sub(1).and_then(|at| values.get(at)) { - if *scope == io_harness::config::Scope::Project - && io_cli::configure::widens_project(key, value) + if matches!( + *scope, + io_harness::config::Scope::Project + | io_harness::config::Scope::Local + ) && io_cli::configure::widens_workspace(key, value) { app.record( Tone::Refused, format!( - "{key} is decided by the project file, and a committed \ - file may not set it to {value} — io-harness refuses \ - the whole file for it, not just the key" + "{key} is decided by a file inside the workspace, and \ + such a file may not set it to {value} — io-harness \ + refuses the whole file for it, not just the key" ), ); } else { @@ -7503,6 +7523,7 @@ async fn turn( // step landing and the first completion call being seen. note_context(app, store, &event, seen, &contract); note_cost(app, store, config, &event); + note_boundary(app, store, &event); note_fleet(app, store, &event, &contract); commit_viewed(screen, app, &root, policy, &event)?; commit_fold(app, store, &event, &mut folding); @@ -8048,6 +8069,7 @@ async fn turn( // one whose event the select loop loses, and it is also the step that // makes the largest single difference to what the turn cost. note_cost(app, store, config, &event); + note_boundary(app, store, &event); note_fleet(app, store, &event, &contract); // And the picture, for the same reason and the same race: a `view_image` // on the turn's last step is exactly the one the drain would otherwise @@ -8393,6 +8415,29 @@ fn note_cost( app.status.note_cost_from(store, event.run_id, &table); } +/// Take the backend's own boundary measurement, once, when it is announced. +/// +/// **The event is the trigger and the store is the payload, because io-harness +/// splits them.** `EventKind::Sandbox` carries `kind` and `backend` and nothing +/// else — `record_sandbox_step` copies those two and drops the rest — so the +/// stream says *that* a probe happened and only the `sandbox_events` row says +/// what it found. +/// +/// Gated on the announcing event rather than on `Step` like `note_cost` above: +/// io-harness writes this row once per run, at step 0, so a per-step read would +/// re-read a row that cannot change. A run with no containment is never probed +/// and emits nothing here, which is what leaves the field absent rather than +/// drawn as unmeasured. +fn note_boundary(app: &mut App, store: &Store, event: &io_harness::RunEvent) { + let io_harness::EventKind::Sandbox { kind, .. } = &event.kind else { + return; + }; + if kind != "boundary_probe" { + return; + } + app.status.note_boundary_from(store, event.run_id); +} + /// Report a fold that was asked for — once, and only from the event that /// announces it. /// diff --git a/src/manage.rs b/src/manage.rs index 486e988..ded385e 100644 --- a/src/manage.rs +++ b/src/manage.rs @@ -806,10 +806,28 @@ pub fn plan( *scope, chosen.dir(), (chosen.from() != chosen.dir()).then(|| chosen.from()), + chosen.copied(), ) }) .transpose() .inspect_err(|_| crate::marketplace::unmake(chosen.made()))?; + // **And nothing is written at all when the entry is already there + // (0.35.0).** Since the adapter became a copy of what it contributes, + // **installing again is the update path** — and `pluginview::add` + // appends unconditionally while `Edit::append` always splices at end + // of file, so every refresh added a *second* `[[plugin]]` naming the + // same directory. io-harness drops it with "a plugin with id `x` is + // already declared and switched on", so the bundle kept working and + // the operator's file grew an ignored entry on each update, quietly, + // forever. + // + // There is no edit to make: the adapter was regenerated on disk by + // `chosen` above, before this point, and the declaration that names it + // is already correct. The disclosure still travels, because what the + // refresh moved is exactly what the operator needs to see. + let already = declared + .iter() + .any(|(_, path)| path.as_path() == chosen.dir()); Plan { made: chosen.made().map(Path::to_path_buf), scope: *scope, @@ -819,7 +837,11 @@ pub fn plan( // have io-harness read it at all; `inspect` has replaced that // round trip, so there is no longer an entry in the file that the // operator has not agreed to. See `pluginview::add_off`. - edits: vec![crate::pluginview::add(&written)], + edits: if already { + Vec::new() + } else { + vec![crate::pluginview::add(&written)] + }, disclosure, } } @@ -1372,7 +1394,7 @@ fn mcp_add(args: &Args) -> Result { // Built by hand rather than through `McpServer::stdio(…).with_args(…)`, // because `env` and `headers` have no builder at all and `with_args` is a - // silent no-op on an HTTP server (`io-harness-0.73.0/src/mcp.rs:439-447`: the + // silent no-op on an HTTP server (`io-harness-0.74.0/src/mcp.rs:439-447`: the // body writes only into the `Stdio` arm) — a constructor chain here would drop // the arguments of half the servers it was handed and say nothing. // Asked of the harness rather than written as literals, the way `servers::add` @@ -1548,11 +1570,22 @@ fn config_set(args: &Args) -> Result { // is caught there by `configure::write`'s round trip in io-harness's own // words rather than here. Guessing at parse time would mean discovering the // workspace from a function whose whole job is reading a line of text. - if scope == Some(Scope::Project) && crate::configure::widens_project(&key, &value) { + // **Both untrusted scopes, and the advice names neither of them (0.35.0).** + // This guard read `Scope::Project` alone, so `--scope local` walked straight + // past io-cli's own check and was caught only by `configure::write`'s + // round-trip rollback — and the sentence it printed sent the operator to + // `--scope local`, which io-harness 0.74.0 now refuses for exactly the same + // reason. `io.local.toml` is not committed, but it sits in the workspace root + // a run's own agent can write to, so one `write_file` of an unremarkable name + // was an escalation. The user scope is the only destination left. + if matches!(scope, Some(Scope::Project | Scope::Local)) + && crate::configure::widens_workspace(&key, &value) + { return Err(format!( - "io-harness refuses `{key} = {value}` in a project file, because `io.toml` is what a \ - `git clone` hands to everyone and this widens what they may do without asking them; \ - write it with `--scope local` for this checkout or `--scope user` for yourself" + "io-harness refuses `{key} = {value}` in a file inside the workspace, because \ + `io.toml` is what a `git clone` hands to everyone and `io.local.toml` sits in a root \ + this run's own agent can write to — and this widens what they may do without asking \ + you; write it with `--scope user` for yourself" )); } diff --git a/src/marketplace.rs b/src/marketplace.rs index 9e47b52..fa08a5d 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -1462,9 +1462,29 @@ pub struct Prepared { /// The bundle's own directory — the author's files. What the disclosure reads /// hooks from, because the generated manifest carries none. pub from: PathBuf, - /// The directory io created for this install, if it created one. Removed when - /// the operator declines, so a refused install leaves nothing behind. + /// The directory io created **for this install**, if it created one. Removed + /// when the operator declines, so a refused install leaves nothing behind. + /// + /// **`None` for an adapter that was already there**, and that is the whole of + /// what makes a re-install safe. Installing a bundle a second time is the + /// update path (see [`prepared`]), so by the time the operator is asked to + /// consent there is an adapter directory that a `[[plugin]]` entry already + /// names — and answering "leave it" would otherwise delete the bundle they + /// installed last week, out of a file this module never opened and cannot see. + /// A decline then takes back what this install created, which for an update is + /// nothing. pub made: Option, + /// The directories the adapter copied out of the clone, in + /// [`crate::adapt::Adapter::copied`]'s own words — `skills`, `templates`, or + /// both. Empty for a native bundle, which is declared where it sits. + /// + /// **Carried because an adapter is a snapshot and an operator has to be told + /// so.** io-harness 0.74.0 refuses a `skills` or `templates` pointing out of + /// the bundle in every scope, so an adapter ships copies rather than pointing + /// into the clone; a `git pull` there therefore changes nothing until the + /// bundle is installed again. [`adapted_disclosure`] is where that is said, in + /// the one place the operator is already reading before they consent. + pub copied: Vec, } /// Resolve a word to a bundle and bring it to the point of being installable. @@ -1480,6 +1500,18 @@ pub struct Prepared { /// directory unchanged and the generation is skipped, so `plugin add` on a /// `plugin.toml` marketplace does exactly what it did in 0.30.2. /// +/// **Installing a bundle again is how an adapter is updated, and it is the only +/// way.** [`crate::adapt::generate`] rebuilds the adapter directory on every run — +/// it does not merge, so a file the author withdrew upstream cannot survive in +/// io's home and go on being read into the model's prompt — and since 0.35.0 that +/// directory holds *copies* of the bundle's `skills/` and `commands/` rather than +/// a manifest pointing at the clone. io-harness 0.74.0 is why: it refuses a +/// `skills` or `templates` that leaves the bundle, in every scope, because every +/// `*.md` under one reaches the model's system prompt. So a clone the operator +/// pulled is a clone this session has not read, and running the install again is +/// what moves it across. [`Prepared::copied`] is what moved, and +/// [`adapted_disclosure`] says so before the operator answers. +/// /// **The adapter is written before consent and removed if consent is refused**, /// and that is the honest order rather than a convenience. The disclosure is /// io-harness reading the bundle, and io-harness has no loader that takes a @@ -1509,6 +1541,7 @@ pub fn prepared( declare: from.clone(), from, made: None, + copied: Vec::new(), }); } // The **normalised id**, not the label. It is what the generated manifest @@ -1523,11 +1556,19 @@ pub fn prepared( ) })?; let into = crate::adapt::at(adapters, &market.named.owner, &market.named.repo, &name); - crate::adapt::generate(&from, &name, &into)?; + // **Asked before the generator answers, because the generator removes it.** + // `adapt::generate` rebuilds the adapter directory from scratch on every run — + // that is what makes an update an update — so after the call there is no way + // left to tell an install from a re-install. See [`Prepared::made`]: the + // difference decides whether a decline deletes a bundle the operator installed + // some other day. + let already = into.is_dir(); + let written = crate::adapt::generate(&from, &name, &into)?; Ok(Prepared { declare: into.clone(), from, - made: Some(into), + made: (!already).then_some(into), + copied: written.copied, }) } @@ -1598,6 +1639,18 @@ impl Chosen { } } + /// The directories this install copied out of the clone. See + /// [`Prepared::copied`]. + /// + /// Empty for a directory the operator typed, which nothing copied anywhere. + #[must_use] + pub fn copied(&self) -> &[String] { + match self { + Self::Path(_) => &[], + Self::Held(prepared) => &prepared.copied, + } + } + /// Whether declaring it owes the operator a disclosure before it loads. /// /// One method rather than a `matches!` at the call site, so the rule has one @@ -1618,9 +1671,14 @@ impl Chosen { pub struct Disclosure { /// The bundle's id, for the confirmation's title. pub id: String, - /// One row per fact, in io-harness's own contribution order — exactly what - /// `/plugin`'s own pane draws, so the two surfaces cannot say different things - /// about one bundle. + /// One row per fact, in io-harness's own contribution order — what `/plugin`'s + /// own pane draws, so the two surfaces cannot say different things about one + /// bundle. + /// + /// **One row is not the pane's**, and it is here rather than in a field of its + /// own so that no door has to learn about it: the last row of an adapted + /// bundle's disclosure names the directories io copied out of the clone. See + /// [`adapted_disclosure`] for why an operator is owed it. /// /// Rows rather than finished lines because the glyph set belongs to the door: /// the TUI writes into a terminal whose set the operator chose, and the argv @@ -1716,7 +1774,7 @@ impl Disclosure { /// one that *does* show, the separator between a row's two fields, is applied by /// the door in [`Disclosure::said`]. pub fn disclosure(scope: Scope, dir: &Path) -> Result { - adapted_disclosure(scope, dir, None) + adapted_disclosure(scope, dir, None, &[]) } /// The sentence every withheld hook carries, spelled once. @@ -1743,18 +1801,44 @@ pub const NOT_CARRIED: &str = "io does not run this — a hook here is a shell l /// /// `None` for a native bundle, where the two are the same directory and nothing is /// withheld. +/// +/// `copied` is [`Prepared::copied`] — the directories this install moved out of the +/// clone, in the generator's own words rather than in a second reading of the +/// adapter directory. It becomes one more row, and it is the answer to the question +/// an operator asks a week later: they edited a skill inside the clone, or pulled +/// the repository, and the session went on using what io copied. Empty for every +/// bundle that copied nothing, where the row would be a warning about nothing. pub fn adapted_disclosure( scope: Scope, dir: &Path, from: Option<&Path>, + copied: &[String], ) -> Result { let plugin = io_harness::Plugins::inspect(scope, dir).map_err(|error| error.to_string())?; // `true`, because nothing has been written: this is what the directory *is*, // not what some `[[plugin]]` entry said about it. See `pluginview::copy_out`. let listed = crate::pluginview::copy_out(&plugin, true); + let mut rows = crate::pluginview::detail(&listed, u16::MAX, &crate::glyphs::ASCII); + // A row rather than a field of its own, because a field is a line only a door + // that knows about it prints — and both doors already print every row of this, + // in the order they come. The paths are whole: this surface elides nothing (see + // above), and the adapter's path is the directory an operator goes looking for + // when they want to know what io is reading. + if let Some(source) = from.filter(|_| !copied.is_empty()) { + rows.push(Row::with_detail( + format!("copied `{}`", copied.join("` and `")), + format!( + "out of {} into {} — an adapter ships the directories it contributes rather than \ + pointing at somebody else's checkout, so an edit or a `git pull` in the clone \ + reaches this session when the bundle is installed again, and not before", + source.display(), + dir.display(), + ), + )); + } Ok(Disclosure { id: listed.id.clone(), - rows: crate::pluginview::detail(&listed, u16::MAX, &crate::glyphs::ASCII), + rows, withheld: from.map(withheld_hooks).unwrap_or_default(), }) } diff --git a/src/memory.rs b/src/memory.rs index a9b837e..fdeb187 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -27,7 +27,7 @@ //! //! Worth knowing before a surface promises otherwise. `read_instructions` joins //! each name in `[instructions] files` to the **discovery root** -//! (`io-harness-0.73.0/src/config.rs:2111`), and the default list is exactly +//! (`io-harness-0.74.0/src/config.rs:2680`), and the default list is exactly //! `["AGENTS.md"]` (`DEFAULT_INSTRUCTIONS`, `config.rs:159`). So `AGENTS.md` is read by every project //! with no configuration at all; `AGENTS.local.md` is read only where a file //! names it; and `IO.md` is not reachable by a bare name at all, because a @@ -464,7 +464,7 @@ fn bullet(line: &str) -> Option { /// The order the three files are named in, and it is the order they reach the /// model in: `read_instructions` pushes one constraint per name in list order -/// (`read_instructions`, `io-harness-0.73.0/src/config.rs:2111`). +/// (`read_instructions`, `io-harness-0.74.0/src/config.rs:2680`). /// /// Project, then local, then user — widest audience first, so the operator's own /// standing note is the last thing said. It also matches the order the three diff --git a/src/reload.rs b/src/reload.rs index da01a2e..a36bb0b 100644 --- a/src/reload.rs +++ b/src/reload.rs @@ -2,7 +2,7 @@ //! //! **io-harness takes the instruction files as a snapshot and never takes //! another.** `read_instructions` runs inside `Config::discover` -//! (`io-harness-0.73.0/src/config.rs:2111`), composes each named file into a +//! (`io-harness-0.74.0/src/config.rs:2680`), composes each named file into a //! constraint, and stores the result in a private `Config.instructions` field. //! There is no `Config::reload`, and `Config::with_profile` clones the field //! rather than going back to disk — so a `Config` is exactly as old as the call diff --git a/src/status.rs b/src/status.rs index c4a0cce..aa5b51d 100644 --- a/src/status.rs +++ b/src/status.rs @@ -338,6 +338,35 @@ pub struct Status { /// showing the mode alone is reading an intention — `workspace-write` reaching /// a portable floor means resource caps and nothing else. pub containment: Option, + /// What the sandbox backend's own boundary probe measured, as io-harness + /// spelled it. + /// + /// **The field beside [`Status::containment`] is the whole point of the + /// release.** That one names the backend that *answered*, which this interface + /// has drawn since 0.2.0 in preference to the mode that was asked for. Until + /// io-harness 0.74.0 the answer could still be false: on macOS a crafted + /// `workdir` re-granted write and network on `/` while the backend still said + /// `MacosSandboxExec`, and on Windows `shell` and `git` spawned entirely + /// unwrapped under a reported `WindowsAppContainer`. 0.74.0 makes each backend + /// attempt a write outside its writable roots and a dial outside its allowed + /// egress at run start and records what happened, so the boundary an operator + /// reads is a thing that was measured rather than a thing that was claimed. + /// + /// **Held raw, as `BoundaryProbe::trace_label` wrote it**, and reduced to a + /// word only where a word is what fits — see [`boundary_word`]. `/status` has + /// room for the measurement itself and prints it. + /// + /// **Read from the store, because the event does not carry it.** + /// `EventKind::Sandbox` has `kind` and `backend` and nothing else; + /// `record_sandbox_step` copies those two and drops the rest. The write and + /// dial outcomes exist only in `Store::sandbox_events`'s `detail` column. That + /// puts this on exactly the footing of [`Status::cost`], which re-reads the + /// store for the same reason and says so. + /// + /// `None` where io-harness recorded no row, which is every run with no + /// containment — and drawing "unverified" there would be a claim about a + /// measurement nobody took. + pub boundary: Option, /// The branch the working tree is on, or `None` where there is no answer. /// /// **A fact about the checkout, so neither [`Status::forget_run`] nor @@ -607,6 +636,7 @@ impl Status { cost: None, context: None, containment: None, + boundary: None, branch: None, spend: None, plan: None, @@ -686,6 +716,10 @@ impl Status { self.steps = None; self.context = None; self.containment = None; + // Beside the containment word it qualifies, and for the same reason: a + // measurement belongs to the run that took it, and carrying it onto the + // next one would report a boundary that was never probed. + self.boundary = None; self.spend = None; self.plan = None; // **A standing belongs to the turn that was gated, and this is the field @@ -859,6 +893,68 @@ impl Status { self.cost = (total.micros > 0).then_some(total.micros); } + /// What the probe proved, as its own field. + /// + /// **A field of its own rather than a word appended to the containment, and + /// the footer measured why.** The right-hand group — posture, containment, + /// planning — is fit **all or nothing**, so a longer containment word does not + /// truncate: at eighty columns it takes the posture and the containment off + /// the row **together**, and an operator loses both without being told. That is + /// verbatim the failure this module already paid for twice, once for `effort` + /// (0.26.0) and once for `branch` (0.25.0), and both were moved into `counts` + /// for it — where narrowing takes one field at a time. + /// + /// The same argument decides this one, and this module states it: the fact an + /// operator can afford to lose is the one still readable a keystroke away on + /// `/status`, and a posture that has silently vanished is not. So the word + /// yields before the posture does, and `/status` carries the measurement + /// itself. + /// + /// Named `boundary` rather than drawn bare, because `verified` alone on a row + /// of counters says nothing about what was verified. + #[must_use] + pub fn boundary_field(&self) -> Option { + let detail = self.boundary.as_ref()?; + Some(format!("boundary {}", boundary_word(detail))) + } + + /// The containment and its probe together, for a surface with room for both. + /// + /// `/status` only. The two width-bound surfaces draw them as separate fields + /// so that the probe word can yield on its own — see [`Status::boundary_field`]. + #[must_use] + pub fn contained(&self) -> Option { + let containment = self.containment.as_ref()?; + Some(match &self.boundary { + Some(detail) => format!("{containment} {}", boundary_word(detail)), + None => containment.clone(), + }) + } + + /// Set [`Status::boundary`] from what this run's backend probe measured. + /// + /// **Read from the store because io-harness puts it nowhere else.** + /// `EventKind::Sandbox` carries `kind` and `backend`; the arms live only in the + /// `sandbox_events` row's `detail`. Same place and cadence as + /// [`Status::note_cost_from`], which re-reads for the same kind of reason. + /// + /// The row is written once per run, at step 0, and **only for a run that was + /// contained** — a run with no containment records nothing, which is what + /// leaves this `None` rather than "unmeasured". A proxied tree records the row + /// with both arms unmeasured, which is a different fact and reads as one. + /// + /// Silent on every failure, like the cost read: a store that cannot be read is + /// a status line without this field, and a notice about a failed read of a + /// decorative field is worse than the field's absence. + pub fn note_boundary_from(&mut self, store: &io_harness::Store, run_id: i64) { + let Ok(events) = store.sandbox_events(run_id) else { + return; + }; + if let Some(probe) = events.iter().find(|event| event.kind == BOUNDARY_PROBE) { + self.boundary.clone_from(&probe.detail); + } + } + /// Set [`Status::context`] from an observation section of `est_tokens`. /// /// **The denominator is [`Budgets::window`] and never a budget built here, and @@ -1384,6 +1480,13 @@ impl Status { if let Some(containment) = &self.containment { fields.push(Field::new(containment.clone(), Tone::Muted)); } + // Immediately right of the backend it qualifies. `Status::fields` narrows + // one field at a time from the right, so this is the first of the pair to + // go — which is the order that keeps a backend on the row without a + // verdict rather than a verdict with no backend. + if let Some(text) = self.boundary_field() { + fields.push(Field::new(text, Tone::Muted)); + } // **Immediately right of the containment word, which is its nearest kin on // this line, and left of the plan claim.** Both are standing facts about // the circumstances the agent is working in rather than counters of what a @@ -1683,6 +1786,20 @@ impl Status { // that has silently vanished is not. Pushed last, so `counts.pop()` takes // it before it takes any number — a narrow row keeps what it measured and // gives up where it measured it. + // **And the probe word, for the reason the branch is here.** It qualifies + // the containment word in the group to the right, and putting it there + // would take that group past what eighty columns hold — costing the + // posture and the containment together, which is the failure this + // paragraph and the two above it all record. It is also exactly the class + // of fact that belongs here: still readable on `/status`, which carries + // the measurement rather than the verdict. + // + // Pushed before the branch so `counts.pop()` takes the branch first: of + // the two, the branch is the one an operator is least likely to be + // reading the row for while a boundary is in force. + if let Some(text) = self.boundary_field() { + counts.push(text); + } if let Some(text) = self.branch_field() { counts.push(text); } @@ -1910,6 +2027,57 @@ pub fn format_containment(mode: &str, backend: &str) -> String { format!("{mode}/{backend}") } +/// The `sandbox_events` kind io-harness writes its startup probe under. +/// +/// Its own doc comment on `SandboxEvent::kind` does not list this one — it names +/// `create`, `exec`, `cap_hit`, `destroy`, `dial`, `gate_phase_failed` and +/// `gate_output`, all of which predate 0.74.0. Filed upstream; matched here +/// against the value `record_sandbox_step` actually writes. +const BOUNDARY_PROBE: &str = "boundary_probe"; + +/// One word for what the probe measured, from the line io-harness wrote. +/// +/// **A word and not the measurement, because the row it goes on is a row.** +/// `BoundaryProbe::trace_label` is +/// `"{backend} write-outside={refused|landed|unmeasured} dial-outside={…}"`, which +/// is far too wide for a status line already carrying seventeen fields. `/status` +/// prints the label itself; this is what the line and the footer can afford. +/// +/// **Parsing a dependency's string is a thing this product is right to be wary +/// of, so here is why it is the honest option.** io-harness exposes the two arms +/// nowhere else: they are not on `EventKind::Sandbox`, and `BoundaryProbe`'s own +/// fields never leave the harness. The label is documented as "one stable line" +/// and is `key=value` rather than prose, so what is matched is the key, not a +/// sentence anyone might reword. An accessor has been asked for upstream. +/// +/// **Two words and not three, and the missing one is deliberate.** +/// `BoundaryProbe::contradicts_claim` is the harness's own notion of a backend +/// that did not do what it said — and it needs `claimed_confinement` and +/// `claimed_egress_denial`, **which the label does not carry**. So io-cli cannot +/// tell "this backend claimed confinement and a write landed anyway" from "this +/// backend never claimed that arm". Drawing `contradicted` on the second would be +/// a false alarm on the one field an operator is meant to trust, which is worse +/// than drawing nothing — the same reasoning that made a refused host render +/// `web refused` rather than a word that reads like a visit. +/// +/// So: `verified` only when both arms were attempted and both were refused; +/// `unverified` for anything else, which covers an arm that landed and an arm +/// that could not be attempted alike. `/status` carries the distinction, because +/// `/status` has the room to state it without implying a verdict. +#[must_use] +pub fn boundary_word(detail: &str) -> &'static str { + let refused = |arm: &str| { + detail + .split_whitespace() + .any(|word| word == format!("{arm}=refused")) + }; + if refused("write-outside") && refused("dial-outside") { + "verified" + } else { + "unverified" + } +} + /// The whole session state, as the lines `/status` commits into the scrollback. /// /// **Every field here is a value io-harness supplied, and the ones that are not @@ -2084,11 +2252,19 @@ pub fn committed( // sandbox available reaches the portable floor — and it is the second word // that says what is enforcing anything. It is `EventKind::Contained`'s pair, // carried on this struct by `App::event`; nothing here names a backend. + // + // **And the measurement itself, here and nowhere else.** The line and the + // footer can afford one word — `verified` or `unverified` — and this page can + // afford the label io-harness wrote, which is what separates an arm that was + // attempted and landed from an arm that could not be attempted at all. Those + // are different facts and only one of them is a failure; a surface with the + // room to tell them apart should. facts.push(( "sandbox".into(), - match &status.containment { - Some(word) => word.clone(), - None => format!( + match (status.contained(), &status.boundary) { + (Some(word), Some(detail)) => format!("{word} {dash} {detail}"), + (Some(word), None) => word, + (None, _) => format!( "not known until a turn has run {dash} the mode and the backend are \ reported when one starts" ), diff --git a/tests/adapt.rs b/tests/adapt.rs index e3af4d4..fb00815 100644 --- a/tests/adapt.rs +++ b/tests/adapt.rs @@ -17,10 +17,12 @@ //! spelled would assert that this crate can quote its own output, which is not the //! question: the question is whether io-harness loads it. +mod support; + use std::path::{Path, PathBuf}; use io_cli::adapt::{self, Source}; -use io_harness::config::{Config, Scope, LOCAL_FILE}; +use io_harness::config::Scope; use io_harness::{Plugins, PLUGIN_FILE}; /// A clone directory, empty. @@ -463,33 +465,37 @@ fn four_kinds(root: &Path) -> PathBuf { /// The adapter at `into`, loaded the way a configuration loads it. /// -/// `io.local.toml` rather than `io.toml`, because an adapter contributes an -/// `[[mcp]]` and a plugin declared in the committed, cloned project file may not — -/// io-harness refuses that manifest whole rather than shortening it. The path is -/// spelled through `edit::spell` rather than by a format string: it is absolute, -/// and an absolute Windows path is full of backslashes that a `"{path}"` would -/// turn into escapes. -fn loaded(root: &Path, into: &Path) -> Plugins { +/// **The user-scope file, and an adapter directory outside the workspace — which +/// is where a real adapter is and which file really names it.** io-harness 0.74.0 +/// decides what a manifest may contribute by where the manifest sits: a +/// `plugin.toml` inside the discovery root may not carry an `[[mcp]]`, whatever +/// declared it, because the run's own agent writes paths inside the root. An +/// adapter is written under `home::adapters()` — `~/.io-cli/adapters` — which is +/// outside every workspace, and `manage`'s `plugin add` resolves an unstated scope +/// to the user's. So the fixture discovers against an empty workspace of its own +/// and names the adapter from `$IO_CONFIG`, which is the only arrangement in which +/// a generated manifest's `[[mcp]]` survives the load. +/// +/// The path is spelled through `edit::spell` rather than by a format string: it is +/// absolute, and an absolute Windows path is full of backslashes that a `"{path}"` +/// would turn into escapes. +fn loaded(into: &Path) -> Plugins { let declared = io_cli::edit::spell(&into.display().to_string()); - std::fs::write( - root.join(LOCAL_FILE), - format!("[[plugin]]\npath = {declared}\n"), - ) - .expect("the configuration"); - Config::discover(root) - .expect("the configuration loads") + support::user_scope(&format!("[[plugin]]\npath = {declared}\n")) + .config .plugins() } /// **F7.** A bundle carrying skills, commands, an agent and an MCP server becomes /// a manifest io-harness loads, and what it reports is what the bundle carried. /// -/// Sabotage: write the `skills` key as the relative `"skills"` the bundle's own -/// directory is called. Nothing else changes — the manifest still parses, the -/// bundle still loads, `dropped()` is still empty, and `contributions()` still -/// says `skills` — and every skill in the bundle is silently absent, because -/// `Plugin::skills_dir` is `self.root.join(d)` and the root is io's adapter -/// directory rather than the clone. Only the equality below fails. +/// Sabotage: write the two directory keys as the absolute paths into the clone +/// that io wrote until 0.35.0. The generator still runs and the fixture still +/// builds — and `dropped()` carries the whole bundle, because io-harness 0.74.0 +/// refuses an absolute `skills` or `templates` in every scope, `Scope::User` +/// included (`io-harness-0.74.0/src/plugin.rs:1097`): a manifest contributes a +/// directory it ships rather than one it points at somewhere else on the machine, +/// since every `*.md` under it is read into the model's system prompt. #[test] fn f7_a_four_kind_bundle_becomes_a_manifest_io_harness_loads_with_nothing_dropped() { let (_dir, root) = clone(); @@ -516,7 +522,7 @@ fn f7_a_four_kind_bundle_becomes_a_manifest_io_harness_loads_with_nothing_droppe "and the clone is untouched", ); - let plugins = loaded(&root, &into); + let plugins = loaded(&into); assert!( plugins.dropped().is_empty(), "io-harness refused the manifest io wrote: {:?}", @@ -531,17 +537,25 @@ fn f7_a_four_kind_bundle_becomes_a_manifest_io_harness_loads_with_nothing_droppe // asserting that this crate can quote its own output. assert_eq!( plugin.skills_dir(), - Some(std::fs::canonicalize(bundle.join("skills")).expect("the skills directory")), - "the manifest is in io's home and the directory is in the clone, so an absolute \ - path is the only spelling that reaches it: `skills_dir` is `root.join(d)`, and \ - a join of an absolute path discards the root", + Some(into.join("skills")), + "the directory the adapter contributes is one it **ships**: io-harness 0.74.0 \ + resolves `skills` by joining it onto the plugin root and refuses an absolute \ + value in every scope, so the manifest names its own `skills/` and the bundle's \ + was copied into it", ); assert_eq!( plugin.templates_dir(), - Some(std::fs::canonicalize(bundle.join("commands")).expect("the commands directory")), + Some(into.join("templates")), "`commands/` and a templates directory are one file in two vocabularies — \ - markdown, optional frontmatter, `$ARGUMENTS` — so the directory is named and \ - nothing is copied", + markdown, optional frontmatter, `$ARGUMENTS` — so the files are copied verbatim \ + under io-harness's own word for the directory", + ); + assert_eq!( + std::fs::read_to_string(into.join("skills/review/SKILL.md")).expect("the copied skill"), + "# review\n\nHow we review.\n", + "and the copy is asserted on its bytes rather than on `exists`: an adapter \ + holding an empty `skills/` satisfies an existence check forever, loads without \ + a complaint, and contributes nothing", ); assert_eq!( plugin.agents().len(), @@ -566,6 +580,14 @@ fn f7_a_four_kind_bundle_becomes_a_manifest_io_harness_loads_with_nothing_droppe "and what io reports is `Plugin::contributions()` on the file it wrote, in \ io-harness's own order — never io-cli's account of what it spelled", ); + assert_eq!( + written.copied, + ["skills", "templates"], + "the two directories that moved are reported as data, so the surface installing \ + this bundle can tell the operator their edits inside the clone will not be seen \ + until the next update: {:?}", + written.copied, + ); assert_eq!( written.disclosed.len(), 3, @@ -606,7 +628,7 @@ fn f7_a_bundle_carrying_none_of_the_four_kinds_contributes_nothing() { adapt::generate(&bundle, "bare", &into).expect("a bundle carrying nothing still adapts"); - let plugins = loaded(&root, &into); + let plugins = loaded(&into); let plugin = plugins .get("bare") .expect("loaded under the id io declared"); @@ -666,6 +688,69 @@ fn f8_the_generated_manifest_round_trips_and_an_invented_key_refuses_the_bundle( ); } +/// **A refused refresh leaves the working adapter exactly as it was.** +/// +/// **This is a defect 0.35.0 created and caught before it shipped.** Until the +/// adapter became a copy, `generate` ran only on a first install: it wiped `into` +/// up front and removed it again on every refusal, and there was nothing there to +/// lose. Now that the adapter ships what it contributes, **installing again is the +/// update path** — so a refusal on an update was deleting a working adapter that +/// an existing `[[plugin]]` entry still named, and an operator lost a bundle by +/// trying to refresh it. The generator stages in a sibling and swaps in on +/// success, so nothing below `into` is touched until everything has succeeded. +/// +/// The refusal used here is the copy stage rather than an earlier one on purpose: +/// a bundle whose `skills` climbs out of itself is refused by `inside` before any +/// directory is made, which would pass this test without exercising the staging at +/// all. +/// +/// Sabotage: generate into `into` directly and remove it on failure, which is the +/// pre-0.35.0 body. The adapter is gone and this fails on the skill it can no +/// longer read. +#[cfg(unix)] +#[test] +fn a_refused_refresh_leaves_the_installed_adapter_untouched() { + let (_dir, root) = clone(); + let bundle = four_kinds(&root); + let into = root.join("adapters/rust-review"); + + adapt::generate(&bundle, "rust-review", &into).expect("the first install writes an adapter"); + let skill = into.join("skills/review/SKILL.md"); + let before = std::fs::read_to_string(&skill).expect("the adapter ships the bundle's skill"); + + // Make the *next* generate fail, at the copy, by putting a symbolic link where + // the adapter would have to follow it out of the bundle. + std::os::unix::fs::symlink(&root, bundle.join("skills/escape")).expect("a link is made"); + + let refused = adapt::generate(&bundle, "rust-review", &into) + .expect_err("a symbolic link in a contributed directory refuses the bundle"); + + assert!( + into.is_dir(), + "the refused refresh removed the installed adapter, which a `[[plugin]]` entry still \ + names: {refused}", + ); + assert_eq!( + std::fs::read_to_string(&skill).ok().as_deref(), + Some(before.as_str()), + "and it is the adapter that was working, byte for byte, rather than a rebuilt one", + ); + + // And the staging directory it built into is not left beside it, where the + // marketplace listing walks. + let strays: Vec<_> = std::fs::read_dir(into.parent().expect("a parent")) + .expect("the adapters directory is readable") + .filter_map(Result::ok) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| name != "rust-review") + .collect(); + assert!( + strays.is_empty(), + "a failed refresh left {strays:?} behind: a half-built adapter under a path the listing \ + walks reads exactly like a working one", + ); +} + /// A server that is a script inside the bundle — named the only way a bundle can /// name one, because the bundle does not know where it will be cloned to. const PLUGIN_ROOT_SERVER: &str = r#"{ @@ -769,3 +854,273 @@ fn f9_a_substitution_io_cannot_expand_refuses_and_names_the_server() { exactly like a working one until something loads it", ); } + +// --------------------------------------------------------------------------- +// The adapter ships what it contributes +// --------------------------------------------------------------------------- + +/// Neither directory key is a path at all — one plain component each, and nothing +/// in the generated bytes climbs out of the adapter root. +/// +/// `check_dirs` refuses an absolute value and a `..` and says nothing about a +/// `skills/../../..`, which is neither: it is a relative path made of ordinary +/// components that io-harness would resolve, and it would resolve to the operator's +/// disk. So this asserts the shape io actually writes rather than the shape +/// io-harness happens to refuse — the two are not the same rule, and the narrower +/// one is io-cli's to keep. +#[test] +fn every_directory_the_generated_manifest_names_is_one_plain_component() { + let (_dir, root) = clone(); + let bundle = four_kinds(&root); + let into = root.join("adapters/rust-review"); + adapt::generate(&bundle, "rust-review", &into).expect("the adapter is written"); + + let text = std::fs::read_to_string(into.join(PLUGIN_FILE)).expect("the manifest"); + let named: Vec<&str> = text + .lines() + .filter(|line| line.starts_with("skills = ") || line.starts_with("templates = ")) + .collect(); + assert_eq!( + named.len(), + 2, + "two directory keys, counted — a `starts_with` over a manifest that grew a third \ + would say nothing about it: {text}", + ); + for line in named { + let said = line.split_once(" = ").expect("a key and a value").1; + assert!( + said == "\"skills\"" || said == "\"templates\"", + "a contributed directory is one plain name under the adapter root and never a \ + path: `{line}` in\n{text}", + ); + } + + // And io-harness resolves each of them back inside the adapter, asked of + // io-harness rather than of the string above. + let plugin = Plugins::inspect(Scope::User, &into).expect("the manifest io wrote parses"); + for dir in [plugin.skills_dir(), plugin.templates_dir()] { + let dir = dir.expect("both directories are contributed"); + assert!( + dir.starts_with(&into), + "every path io-harness resolves out of this manifest is inside the adapter: {}", + dir.display(), + ); + } +} + +/// The adapter holds the bundle's markdown, byte for byte, rather than a path to +/// where it used to be. +/// +/// Sabotage: write the two keys and copy nothing. The manifest still parses, +/// `dropped()` is still empty, `contributions()` still says `skills` and +/// `templates`, and `Skills::discover` finds an empty directory — the bundle +/// installs and contributes nothing, silently. Only the two equalities below fail. +#[test] +fn f7_the_adapter_ships_the_bundle_s_directories_rather_than_pointing_at_them() { + let (_dir, root) = clone(); + let bundle = four_kinds(&root); + let into = root.join("adapters/rust-review"); + + let written = adapt::generate(&bundle, "rust-review", &into).expect("the adapter is written"); + + assert_eq!(written.copied, ["skills", "templates"]); + assert_eq!( + std::fs::read_to_string(into.join("skills/review/SKILL.md")).expect("the copied skill"), + "# review\n\nHow we review.\n", + "the file's own bytes, nested a directory deep — the copy is a walk and not one \ + `read_dir`", + ); + assert_eq!( + std::fs::read_to_string(into.join("templates/ship.md")).expect("the copied command"), + "Ship $ARGUMENTS.\n", + "`commands/` arrives under io-harness's own word for it, contents unchanged: a \ + command and a template are one file in two vocabularies", + ); + assert!( + !bundle.join(PLUGIN_FILE).exists() && bundle.join("skills/review/SKILL.md").is_file(), + "and the clone still holds its own copy and nothing of io's — a file io wrote in \ + a stranger's checkout is a dirty tree at their next `git pull`", + ); +} + +/// A regenerated adapter is the bundle as it is **now**. +/// +/// The one that matters for an update. A copy that merged would leave a skill the +/// bundle's author deleted in io's home for good — still found by +/// `Skills::discover`, still composed into the model's system prompt on every turn, +/// and invisible to the operator, who would be reading the clone. +#[test] +fn a_regenerated_adapter_replaces_what_it_holds_rather_than_merging_into_it() { + let (_dir, root) = clone(); + let bundle = four_kinds(&root); + let into = root.join("adapters/rust-review"); + adapt::generate(&bundle, "rust-review", &into).expect("the adapter is written"); + assert!(into.join("skills/review/SKILL.md").is_file()); + + // The upstream edit: one skill withdrawn, another published. + std::fs::remove_dir_all(bundle.join("skills/review")).expect("the withdrawn skill"); + file( + &bundle, + "skills/audit/SKILL.md", + "# audit\n\nHow we audit.\n", + ); + + adapt::generate(&bundle, "rust-review", &into).expect("the adapter is regenerated"); + + assert!( + !into.join("skills/review").exists(), + "a skill the author withdrew is gone from the adapter too", + ); + assert_eq!( + std::fs::read_to_string(into.join("skills/audit/SKILL.md")).expect("the new skill"), + "# audit\n\nHow we audit.\n", + "and the one they published is there", + ); +} + +/// **The 0.31.0 finding, in its own arm.** A `"skills"` naming a directory outside +/// the bundle contributes nothing, and nothing outside the bundle is copied. +/// +/// `adapt::directory` returned the operator's `$HOME` for this value in 0.31.0, +/// because `Path::join` keeps `..` verbatim — and `Skills::discover` reads every +/// `*.md` under what it is given into the model's system prompt. 0.35.0 moved the +/// destination from a path in a manifest to the source of a **copy**, which is a +/// second way to lose the same directory, so the filter is asserted in front of the +/// copy rather than assumed to have survived the change. +#[test] +fn a_skills_key_that_climbs_out_of_the_bundle_copies_nothing_and_contributes_nothing() { + let (_dir, root) = clone(); + // Beside the bundle, where `../../..` reaches and the bundle does not. + file(&root, "notes/private.md", "# the operator's own notes\n"); + let bundle = root.join("bundle"); + file( + &bundle, + ".claude-plugin/plugin.json", + r#"{ "name": "greedy", "skills": "../../.." }"#, + ); + let into = root.join("adapters/greedy"); + + adapt::generate(&bundle, "greedy", &into).expect("the bundle still adapts"); + + let plugin = Plugins::inspect(Scope::User, &into).expect("the manifest io wrote parses"); + assert_eq!( + plugin.skills_dir(), + None, + "a manifest that names `../../..` has not named a directory in this bundle at \ + all, and quietly reading the one above it would be io choosing a directory the \ + author did not", + ); + let held: Vec = std::fs::read_dir(&into) + .expect("the adapter directory") + .flatten() + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .collect(); + assert_eq!( + held, + [PLUGIN_FILE], + "and the adapter holds the manifest and nothing else — the value is refused \ + before a single byte is read off the disk, which is the half a `skills_dir()` \ + assertion alone would not see: {held:?}", + ); +} + +/// A symbolic link in a contributed directory refuses the bundle rather than being +/// followed or reproduced. +/// +/// io-harness 0.74.0 made this decision for `sandbox::copy_back`, and it is the +/// same act: a link is the obvious way to defeat "the adapter ships what it +/// contributes" — copy it and the adapter points somewhere else again, follow it +/// and io has copied a directory nobody named into a tree whose every `*.md` +/// reaches the model's prompt. +#[cfg(unix)] +#[test] +fn a_symbolic_link_inside_a_contributed_directory_refuses_the_bundle() { + let (_dir, root) = clone(); + file(&root, "elsewhere/private.md", "# not the bundle's\n"); + let bundle = root.join("bundle"); + file(&bundle, "skills/review/SKILL.md", "# review\n"); + std::os::unix::fs::symlink(root.join("elsewhere"), bundle.join("skills/borrowed")) + .expect("the link the copy has to refuse"); + let into = root.join("adapters/linked"); + + let refused = adapt::generate(&bundle, "linked", &into) + .expect_err("a link is refused rather than followed or reproduced"); + + assert!( + refused.contains("symbolic link"), + "the refusal says what it found: {refused}", + ); + assert!( + refused.contains(&bundle.display().to_string()), + "and names the bundle it is about — an operator with several installed has to \ + know which: {refused}", + ); + assert!( + !into.join(PLUGIN_FILE).exists() && !into.join("skills").exists(), + "and nothing is left behind, manifest or copy", + ); +} + +/// The bound is a stated number and a bundle past it is refused by name. +/// +/// A bundle is a stranger's directory and a copy of one is unbounded work over +/// unbounded input. Asserted against `adapt::MOST_BYTES` rather than a literal, so +/// a release that moves the ceiling moves this fixture with it and cannot pass by +/// having quietly raised it. +#[test] +fn a_contributed_directory_past_the_bound_is_refused_and_the_refusal_names_the_bundle() { + let (_dir, root) = clone(); + let bundle = root.join("bundle"); + std::fs::create_dir_all(bundle.join("skills")).expect("the skills directory"); + std::fs::write( + bundle.join("skills/big.md"), + vec![b'#'; usize::try_from(adapt::MOST_BYTES).expect("the bound fits a usize") + 1], + ) + .expect("one file past the ceiling"); + let into = root.join("adapters/big"); + + let refused = + adapt::generate(&bundle, "big", &into).expect_err("a directory past the bound is refused"); + + assert!( + refused.contains(&adapt::MOST_BYTES.to_string()), + "the ceiling is a number the sentence states, not one the reader has to find in \ + the source: {refused}", + ); + assert!( + refused.contains(&bundle.display().to_string()), + "and the sentence names the bundle: {refused}", + ); + assert!( + !into.join(PLUGIN_FILE).exists(), + "and nothing an operator could go on to declare is left behind", + ); +} + +/// The entry count is the other half of the bound, and it is a different runaway: +/// these files weigh nothing at all. +#[test] +fn a_contributed_directory_past_the_entry_bound_is_refused_too() { + let (_dir, root) = clone(); + let bundle = root.join("bundle"); + let skills = bundle.join("skills"); + std::fs::create_dir_all(&skills).expect("the skills directory"); + for at in 0..=adapt::MOST_ENTRIES { + std::fs::File::create(skills.join(format!("{at}.md"))).expect("one empty skill"); + } + let into = root.join("adapters/many"); + + let refused = adapt::generate(&bundle, "many", &into) + .expect_err("a directory past the entry bound is refused"); + + assert!( + refused.contains(&adapt::MOST_ENTRIES.to_string()), + "a hundred thousand empty files pass a byte bound and still cost an install its \ + afternoon, one `copy` at a time — so the count is bounded separately and says \ + its own number: {refused}", + ); + assert!( + !into.join(PLUGIN_FILE).exists(), + "and nothing is left behind", + ); +} diff --git a/tests/bundle_path.rs b/tests/bundle_path.rs index 5b940af..a8e7202 100644 --- a/tests/bundle_path.rs +++ b/tests/bundle_path.rs @@ -13,20 +13,22 @@ //! a process environment. `install` is the one line that is not, and it is one //! line for that reason. +mod support; + use std::ffi::OsString; use std::path::{Path, PathBuf}; use io_cli::bundle_path; -use io_harness::config::{Config, LOCAL_FILE}; use io_harness::PLUGIN_FILE; /// The entries this fixture's own bundles contributed. /// -/// **`Config::discover` layers the machine's real user-scope file**, which on a -/// developer's machine declares bundles of its own — so an assertion of equality -/// against the whole list is green on a clean runner and red here, or the other -/// way round. Filtered to the temporary root, which is the only thing this test -/// put anything in. +/// **Filtered to the temporary root, which is the only thing this test put +/// anything in.** `support::user_scope` points `IO_CONFIG` at a file of its own, +/// so the operator's real one is out of the way for the duration — but the filter +/// stays, because `Config::discover` still reads two more candidates and an +/// assertion of equality against the whole list is the shape that goes green on a +/// clean runner and red on a developer's machine. fn ours(plugins: &io_harness::Plugins, root: &Path) -> Vec { bundle_path::entries(plugins) .into_iter() @@ -93,24 +95,33 @@ fn bundle(root: &Path, at: &str, manifest: &str) -> PathBuf { /// The plugins a configuration declaring each of `bundles` loads. /// -/// **Declared in the local-scope file, and that is not a detail.** io-harness -/// refuses a `[[bin]]` from a project-scoped `io.toml` — the file arrives with a -/// `git clone`, and an executable named in one is a program the machine would run -/// on somebody else's say-so — and the refusal drops the whole bundle. A fixture -/// written in project scope asserts over an empty vector while passing, which is -/// what the drop check below exists to catch. +/// **`root` is a directory outside every workspace, and the declaration is made +/// from the user scope. Neither half is a detail.** Since io-harness 0.74.0 a +/// `plugin.toml` sitting *inside* the discovery root may not contribute a +/// `[[bin]]` at all — it names a program this machine would run, and a file in the +/// workspace root arrives with a `git clone` or is written by the run's own agent — +/// and the refusal drops the whole bundle. Trust follows where the manifest is +/// rather than which file named it, so both halves are required: a user-scope +/// `[[plugin]]` pointing back *into* the workspace is graded as the workspace file +/// it is, and a workspace file pointing *out* has its entry dropped before the +/// manifest is read. What is left is the arrangement an installed bundle actually +/// has, which is the one this surface exists for. +/// +/// A fixture that got either half wrong asserts over an empty vector while +/// passing, which is what the drop check below exists to catch. +/// +/// The path is spelled as a TOML *literal* string: it is absolute, and an absolute +/// Windows path is a run of backslashes a basic string would read as escapes. +/// `tempfile` never puts a `'` in a directory name. fn loaded(root: &Path, bundles: &[(&str, &str)]) -> io_harness::Plugins { let text: String = bundles .iter() .map(|(at, manifest)| { - bundle(root, at, manifest); - format!("[[plugin]]\npath = \"{at}\"\n\n") + let dir = bundle(root, at, manifest); + format!("[[plugin]]\npath = '{}'\n\n", dir.display()) }) .collect(); - std::fs::write(root.join(LOCAL_FILE), text).expect("the configuration"); - let plugins = Config::discover(root) - .expect("the configuration loads") - .plugins(); + let plugins = support::user_scope(&text).config.plugins(); // **A dropped bundle contributes nothing and looks exactly like a bundle that // declared nothing**, so a fixture that silently drops asserts nothing while // passing. io-harness drops a bundle *whole* for a manifest it refuses — an @@ -161,15 +172,16 @@ fn f9_a_declared_program_puts_its_own_directory_on_the_path() { fn a_switched_off_bundle_puts_no_program_on_the_path() { let dir = tempfile::tempdir().expect("a temporary directory"); let root = dir.path(); - bundle(root, "ultraship", SHIPS_A_PROGRAM); - std::fs::write( - root.join(LOCAL_FILE), - "[[plugin]]\npath = \"ultraship\"\nenabled = false\n", - ) - .expect("the configuration"); - let plugins = Config::discover(root) - .expect("the configuration loads") - .plugins(); + let installed = bundle(root, "ultraship", SHIPS_A_PROGRAM); + // Outside the workspace and declared from the user scope, for `loaded`'s + // reason: a `[[bin]]` in a manifest inside the discovery root is refused, and a + // bundle dropped for that is indistinguishable here from one switched off. + let plugins = support::user_scope(&format!( + "[[plugin]]\npath = '{}'\nenabled = false\n", + installed.display() + )) + .config + .plugins(); assert!( !plugins.disabled().is_empty(), "the fixture must actually declare a switched-off bundle, or this test \ diff --git a/tests/configure.rs b/tests/configure.rs index 79af53d..6323616 100644 --- a/tests/configure.rs +++ b/tests/configure.rs @@ -1,18 +1,22 @@ //! F2 and N2 — what is in force, who decided it, and what is never shown. -use std::sync::{Mutex, MutexGuard}; +use std::sync::MutexGuard; use io_cli::configure::{self, Decided}; use io_harness::config::{Config, Scope}; use io_harness::pricing::{Price, PriceTier}; +mod support; + /// `Config::discover` reads `IO_CONFIG` at call time, so two tests setting it at /// once would each see the other's file. Serialised here rather than diagnosed /// later as an intermittent failure on a loaded machine — the same guard /// `tests/wizard.rs` and `tests/contract.rs` already keep for the same reason. +/// +/// Delegated to [`support::env_lock`] rather than declared here: two different +/// mutexes in one binary exclude nothing from each other. fn env_lock() -> MutexGuard<'static, ()> { - static LOCK: Mutex<()> = Mutex::new(()); - LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + support::env_lock() } /// Three scopes that disagree on purpose, so every origin arm has a case. @@ -47,9 +51,20 @@ fn scopes(user: &str, project: &str, local: &str) -> Scopes { impl Scopes { fn config(&self) -> Config { + self.try_config().unwrap() + } + + /// The same read, handing back a refusal instead of unwrapping it. + /// + /// For the tests whose subject is whether io-harness *accepts* a fragment at + /// all. Since io-harness 0.74.0 `Config::from_toml` is no longer usable for + /// that question — it hard-codes `Scope::Project`, where `[[mcp]]`, `[web]`, + /// an absolute `run.templates` and `policy.defaults.read = "allow"` are each + /// refused for being in the wrong file rather than for being wrong. + fn try_config(&self) -> io_harness::Result { let _guard = env_lock(); std::env::set_var("IO_CONFIG", &self.user); - let config = Config::discover(self.root.path()).unwrap(); + let config = Config::discover(self.root.path()); std::env::remove_var("IO_CONFIG"); config } @@ -66,6 +81,117 @@ impl Scopes { } } +/// The sections and values io-harness 0.74.0 refuses from a file in a workspace, +/// each in the file that is refused for it. +/// +/// **Not a list io-cli maintains** — every entry here is driven through +/// `Config::discover` and the assertion is made against **io-harness's own +/// sentence read back from the real refusal**, never against a needle typed here. +/// 0.30.1 shipped a gate whose needle was `"io does not manage"` against a refusal +/// reading `"is not a surface io manages"`; it passed forever and asserted +/// nothing. What is spelled below is only the *input* — what io-harness says about +/// it is read at run time. +const REFUSED_IN_A_WORKSPACE: &[(&str, &str, &str)] = &[ + ( + "a project-scoped provider", + "project", + "[[provider]]\nkind = \"openrouter\"\nmodel = \"anthropic/claude-sonnet-4.5\"\n", + ), + ( + "a project-scoped MCP server", + "project", + "[[mcp]]\nid = \"one\"\ntransport = \"stdio\"\ncommand = \"one\"\n", + ), + ( + "a project-scoped LSP server", + "project", + "[[lsp]]\nid = \"rust\"\ncommand = \"rust-analyzer\"\nextensions = [\"rs\"]\n", + ), + ( + "a hook in the workspace-root local file", + "local", + "[[hook]]\non = [\"finished\"]\nrun = [\"true\"]\n", + ), + ( + "a widened policy default", + "project", + "[policy]\ndefaults = { write = \"allow\" }\n", + ), +]; + +/// **A refusal reads as a refusal, at every door that meets one.** +/// +/// Three doors reach a refused configuration and until 0.35.0 they did not agree. +/// Start-up framed it; the per-turn reload handed back io-harness's bare sentence +/// with no root; the write door rolls back and quotes the harness. So the same +/// `[[mcp]]` in the same file read as a refusal when `io` started and as an +/// unattributed error at the turn boundary that noticed the edit — and the reload +/// door is the one an operator meets *while working*, having just saved the file. +/// +/// The framing had to change too. "could not be read" told an operator their file +/// was broken when it was intact and in the wrong place, which is a different +/// problem with a different fix. +/// +/// **Every assertion is against the sentence io-harness actually produced**, taken +/// from the same refusal the door is being asked about, so this cannot pass +/// against a reworded harness or a needle nobody checked. +/// +/// Sabotage: put `to_string()` back in `configure::reload`. The reload arm fails +/// on the root, which is the fact io-harness cannot supply. +#[test] +fn f2_a_refused_configuration_reads_as_a_refusal_at_every_door() { + for (what, scope, body) in REFUSED_IN_A_WORKSPACE { + let s = match *scope { + "project" => scopes("[run]\nmax_steps = 10\n", body, ""), + _ => scopes("[run]\nmax_steps = 10\n", "", body), + }; + + // The harness's own words, for this exact fixture. Everything below is + // held against this rather than against anything written in this file. + let harness = s + .try_config() + .expect_err(&format!("io-harness refuses {what} in a workspace file")) + .to_string(); + assert!( + harness.contains("IO_CONFIG"), + "{what}: the refusal names where the declaration belongs instead: {harness}", + ); + + // Door one: start-up. + let _guard = env_lock(); + std::env::set_var("IO_CONFIG", &s.user); + let error = Config::discover(s.root.path()).expect_err("the same refusal"); + let started = configure::refusal(s.root.path(), &error); + std::env::remove_var("IO_CONFIG"); + drop(_guard); + + // Door two: the per-turn reload. + let _guard = env_lock(); + std::env::set_var("IO_CONFIG", &s.user); + let reloaded = configure::reload(s.root.path()).expect_err("the same refusal"); + std::env::remove_var("IO_CONFIG"); + drop(_guard); + + for (door, said) in [("start-up", &started), ("reload", &reloaded)] { + assert!( + said.contains(&harness), + "{what}: the {door} door dropped io-harness's own sentence, which is the half \ + that says which key and where it belongs.\n harness: {harness}\n door: {said}", + ); + assert!( + said.contains(&s.root.path().display().to_string()), + "{what}: the {door} door does not name the directory being read, which is the \ + one fact io-harness cannot supply: {said}", + ); + assert!( + !said.contains("could not be read"), + "{what}: the {door} door calls a refusal a failure to read. The file parsed; \ + io-harness declined what it said, and that has a different fix: {said}", + ); + } + } +} + #[test] fn f2_each_scope_names_its_own_file() { let s = scopes( @@ -144,9 +270,15 @@ fn f2_the_winning_scope_is_the_one_reported() { fn f2_a_key_the_catalogue_does_not_know_is_still_shown() { // The property that keeps the surface honest: io-cli's catalogue is its own // list, and a key an operator wrote that is not on it must not be invisible. + // + // `[web]` is in the USER file rather than the project one. io-harness 0.74.0 + // refuses provider-executed web access from a workspace-resident file + // wholesale — a provider-executed search is dialled by the provider, so + // `Act::Net` never sees it — and the claim under test is about io-cli's + // catalogue, not about which file the key arrived in. let s = scopes( - "[run]\nmax_steps = 10\n", - "[web]\nallowed_domains = [\"example.com\"]\n", + "[run]\nmax_steps = 10\n\n[web]\nallowed_domains = [\"example.com\"]\n", + "", "", ); let config = s.config(); @@ -286,7 +418,8 @@ fn names_key(example: &str, key: &str) -> bool { /// /// **The two sides of every assertion come from different places, on purpose.** /// The key names below are written out by hand, here. Whether io-harness accepts -/// them is decided by `Config::from_toml`, which is io-harness's own reader. +/// them is decided by `Config::discover` over a real user-scope file, which is +/// io-harness's own reader on its own path. /// Whether the operator can find them is decided by reading the checked-in file. /// Nothing is derived from the thing it is checking, which is what the five /// vacuous assertions this repository has shipped all had in common. @@ -294,8 +427,8 @@ fn names_key(example: &str, key: &str) -> bool { /// **What deleting a key makes fail.** Cut `max_wait_secs` (or `templates`, /// `headers`, `timeout_secs`, or any of the seven `[toolchain.cargo]` keys) out of /// `docs/config.example.toml` and the `names_key` assertion for it fails by name — -/// the `from_toml` half still passes, because io-harness has not changed. Have -/// io-harness drop a key instead and `from_toml` fails first: every section +/// the parse half still passes, because io-harness has not changed. Have +/// io-harness drop a key instead and the parse fails first: every section /// written below carries `deny_unknown_fields`, so one unknown key refuses the /// WHOLE fragment rather than being ignored, which is the same failure an operator /// would get on their own file. @@ -306,9 +439,14 @@ fn f8_every_documented_harness_key_deserialises() { // (the keys this section names, a file that writes every one of them) // - // Narrowing values only: `from_toml` parses as the PROJECT scope and runs - // `refuse_widening`, so a fragment written with `exec = "allow"` here would - // fail for a reason that has nothing to do with what is being tested. + // **Read at the USER scope, which is the scope `docs/config.example.toml` + // documents.** Through io-harness 0.73 these fragments went through + // `Config::from_toml` and the rule was "narrowing values only". 0.74.0 made + // that unusable: `from_toml` parses as `Scope::Project`, where `[[mcp]]` is + // refused outright, an absolute `run.templates` is refused, and + // `policy.defaults.read = "allow"` joined `PROJECT_WIDENING`. Every one of + // those is a refusal about the file a fragment is in rather than about the + // key, and this test is about the key. let sections: &[(&[&str], &str)] = &[ ( &["read", "write", "exec", "net"], @@ -377,7 +515,7 @@ fn f8_every_documented_harness_key_deserialises() { ]; for (keys, toml) in sections { - if let Err(e) = Config::from_toml(toml) { + if let Err(e) = scopes(toml, "", "").try_config() { panic!( "docs/config.example.toml documents {keys:?} and io-harness refuses the \ section they are in: {e}. The example is now telling operators to write \ @@ -510,8 +648,16 @@ fn f3_a_scope_with_no_file_gets_one() { // --- F4: a project-scoped widening is refused in the harness's own words ------ -/// Every case io-harness refuses in a project-scoped file: the two whole sections -/// and each of `PROJECT_WIDENING`'s five key/value pairs. +/// Widening pairs, spelled the way an operator writes them in a file. +/// +/// **A sample, not the rule.** io-harness 0.74.0's `PROJECT_WIDENING` holds +/// thirteen pairs over twelve keys and it publishes no reader for them, so nothing +/// here is the definition of anything — `configure::widens_workspace` asks the +/// dependency instead of holding a list, and +/// `f2_the_widening_rule_is_asked_of_io_harness_rather_than_copied_from_it` is +/// what keeps it that way. These five exist to drive `configure::write` and read +/// back io-harness's own sentence, which needs a value written into a real file +/// rather than a predicate's answer about one. const WIDENINGS: &[(&str, &str)] = &[ ("policy.defaults.exec", "\"allow\""), ("policy.defaults.net", "\"allow\""), @@ -557,21 +703,55 @@ fn f4_a_project_scoped_widening_is_refused_with_the_harness_s_sentence() { } } +/// **io-harness 0.74.0 moved this line, and the sentence is where it moved to.** +/// +/// Through 0.73 this test asserted the opposite: the rule was about `io.toml` +/// alone and every one of these was legal in the file a repository does not +/// deliver. 0.74.0 extended `refuse_widening` to `io.local.toml` as well, because +/// it sits in the workspace root a run's own agent can write to — a run that can +/// write its own root could otherwise widen its own boundary between one turn and +/// the next. +/// +/// So the local scope is no longer where these are legal, and the only scope left +/// is the user one, which `f2_a_widening_value_is_legal_in_one_scope_and_refused_ +/// in_another` holds. +/// +/// **Asserted on which FILE the refusal names, not merely that it refused.** The +/// project sentence and the local sentence differ in exactly that clause +/// (`config.rs:2429`, `untrusted`), so a needle of "narrow it and never widen it" +/// alone would pass just as well against a write that had gone to `io.toml` — and +/// then this test and `f4_a_project_scoped_widening_is_refused_with_the_harness_s_ +/// sentence` would be one test written twice. #[test] -fn f4_the_same_value_is_accepted_in_the_local_scope() { - // The rule is about the scope, not the value. Every one of these is legal in - // the file a repository does not deliver. +fn f4_the_same_value_is_refused_in_the_local_scope_too() { for (key, value) in WIDENINGS { let s = scopes("[run]\nmax_steps = 10\n", "", ""); let _guard = env_lock(); std::env::set_var("IO_CONFIG", &s.user); - configure::write( + let err = configure::write( s.root.path(), Scope::Local, &[io_cli::edit::Edit::set(*key, *value)], ) - .unwrap_or_else(|e| panic!("{key} = {value} should be legal in io.local.toml: {e}")); + .expect_err(&format!( + "{key} = {value} is refused in io.local.toml since io-harness 0.74.0" + )); + + assert!( + err.contains("a workspace-root `io.local.toml` may narrow it and never widen it"), + "the refusal for {key} is not the local-scope sentence — a write aimed at \ + `io.local.toml` that came back with the `io.toml` wording went to the wrong \ + file: {err}" + ); + assert!( + err.contains(key), + "the refusal does not name the key: {err}" + ); + assert!( + !s.root.path().join("io.local.toml").exists(), + "{key}: a refused write left a local file behind" + ); std::env::remove_var("IO_CONFIG"); } @@ -960,7 +1140,7 @@ fn f1_only_the_expected_exit_status_is_signed() { /// config that stopped parsing. /// /// The third is the sabotage the criterion names — hand-write a fourth effect -/// string and the round trip through `Config::from_toml` refuses it. +/// string and the round trip through io-harness's own reader refuses it. #[test] fn f2_the_effects_are_the_dependency_s_own() { let effects = configure::effects(); @@ -973,10 +1153,17 @@ fn f2_the_effects_are_the_dependency_s_own() { "the menu must be the dependency's enumeration, not io-cli's copy of it" ); assert_eq!(effects, vec!["allow", "ask", "deny"]); + // **In the user scope**, for the reason `f2_the_exec_modes_are_spelled_by_the_ + // dependency` reads at one: io-harness 0.74.0 put `policy.defaults.read` on + // `PROJECT_WIDENING`, so `read = "allow"` in a workspace-resident file is + // refused for the file it is in. The claim here is that every spelling io-cli + // offers is one io-harness's deserializer accepts *somewhere*; which scope + // each is legal in is `f2_a_widening_value_is_legal_in_one_scope_and_refused_ + // in_another`'s question. for word in &effects { let text = format!("[policy]\ndefaults = {{ read = \"{word}\" }}\n"); assert!( - Config::from_toml(&text).is_ok(), + scopes(&text, "", "").try_config().is_ok(), "io-harness refuses the effect io-cli offers: {word}" ); } @@ -1078,10 +1265,57 @@ fn f2_a_widening_value_is_legal_in_one_scope_and_refused_in_another() { "full-access", "[sandbox]\nmode = \"full-access\"\n", ), + // **The eight below are the ones io-cli did not know, and the reason this + // test was rewritten in 0.35.0.** Every one of them was already refused by + // io-harness 0.74.0 and already offered by `/config` and `io config set`, + // and no test failed — because the old version of this loop iterated + // io-cli's own five-pair list, and a list cannot notice what is missing + // from it. They are named individually rather than covered by a helper so + // that the count here is a claim about the dependency's table. + ( + "policy.defaults.read", + "allow", + "[policy]\ndefaults = { read = \"allow\" }\n", + ), + ( + "policy.defaults.write", + "allow", + "[policy]\ndefaults = { write = \"allow\" }\n", + ), + ( + "sandbox.mode", + "workspace-write", + "[sandbox]\nmode = \"workspace-write\"\n", + ), + ( + "sandbox.limits.max_cpu_secs", + "0", + "[sandbox.limits]\nmax_cpu_secs = 0\n", + ), + ( + "sandbox.limits.max_wall_secs", + "0", + "[sandbox.limits]\nmax_wall_secs = 0\n", + ), + ( + "sandbox.limits.max_memory_bytes", + "0", + "[sandbox.limits]\nmax_memory_bytes = 0\n", + ), + ( + "sandbox.limits.max_processes", + "0", + "[sandbox.limits]\nmax_processes = 0\n", + ), + ( + "sandbox.limits.max_open_files", + "0", + "[sandbox.limits]\nmax_open_files = 0\n", + ), ]; for (key, value, project) in widening { assert!( - configure::widens_project(key, value), + configure::widens_workspace(key, value), "{key} = {value} widens and io-cli does not say so" ); // The dependency's own answer, at the scope `from_toml` parses as. @@ -1116,7 +1350,7 @@ fn f2_a_widening_value_is_legal_in_one_scope_and_refused_in_another() { ), ] { assert!( - !configure::widens_project(key, value), + !configure::widens_workspace(key, value), "{key} = {value} narrows and must not be marked" ); assert!( @@ -1124,6 +1358,84 @@ fn f2_a_widening_value_is_legal_in_one_scope_and_refused_in_another() { "io-harness refuses a narrowing value in a committed file: {key}" ); } + + // **A value that fails to deserialize is not a widening, and the distinction + // is what stops this predicate from marking every typo.** `widens_workspace` + // asks the dependency by writing the key into a one-line document, and a + // value the key's type refuses comes back as an error too — so the answer is + // the refusal's own clause and not merely `is_err`. + for (key, value) in [ + ("policy.defaults.exec", "not-an-effect"), + ("sandbox.mode", "sideways"), + ("run.max_steps", "not-a-number"), + ] { + assert!( + !configure::widens_workspace(key, value), + "{key} = {value} is a bad value, not a wider boundary — marking it would send an \ + operator to the user scope to write something that is refused there too" + ); + } +} + +/// **F2, the shape of the answer.** The widening rule is asked of io-harness, not +/// held as a list. +/// +/// **This gate exists because the list is what failed.** Until 0.35.0 +/// `widens_workspace` was `widens_project`, a `matches!` over five hand-copied +/// pairs. io-harness 0.74.0's `PROJECT_WIDENING` holds thirteen pairs over twelve +/// keys, so io-cli's copy had become a strict subset of the rule it claimed to +/// state — and `/config` and `io config set` were offering eight values the +/// destination file refuses, each refusal taking the whole file rather than the +/// key. **No test failed**, because the test iterated io-cli's own list. +/// +/// The loop above now covers all thirteen, which catches *today's* gap. This +/// catches the next one: a release that answers a new divergence by lengthening a +/// list rather than by asking has reintroduced the failure mode, and the loop +/// above cannot see that because a longer correct list still passes it. +/// +/// Sabotage: reinstate the five-pair `matches!`. The loop above fails on eight +/// pairs and this fails on the shape, which is the pair of failures that says the +/// list is both wrong and the wrong idea. +#[test] +fn f2_the_widening_rule_is_asked_of_io_harness_rather_than_copied_from_it() { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/configure.rs"); + // **Normalised at the read boundary, and CI is what taught this file that.** + // Git checks the source out with CRLF on Windows, so `"\n}\n"` below matches + // nothing there and the gate fails on the `expect` rather than on its subject + // — green on macOS and Linux, red on the one platform that sees it. 0.33.0 + // shipped exactly this shape in `tests/docs.rs`'s paragraph splitter and the + // rule it left behind is to normalise where the bytes are read, not per + // helper. This is the first source-text gate written since, and it repeated it. + let text = std::fs::read_to_string(path) + .expect("the module is readable") + .replace("\r\n", "\n"); + let at = text + .find("pub fn widens_workspace") + .expect("`widens_workspace` is the predicate the surfaces consult"); + let body = &text[at..]; + let end = body.find("\n}\n").expect("the function has an end"); + let body = &body[..end]; + + assert!( + body.contains("Config::from_toml"), + "the predicate asks io-harness for its own answer; anything else is a copy that goes \ + stale the next time the dependency tightens, which is exactly what happened at 0.74.0", + ); + for spelled in [ + "policy.defaults.exec", + "policy.defaults.net", + "sandbox.allow_network", + "sandbox.force_floor", + "full-access", + "workspace-write", + ] { + assert!( + !body.contains(spelled), + "`widens_workspace` names `{spelled}` in its body. The pairs belong to io-harness and \ + it publishes no reader for them, so the answer is asked for rather than restated — a \ + key spelled here is a key that has to be remembered when the dependency moves", + ); + } } /// A mode this build does not know is reported, not dropped. diff --git a/tests/contract.rs b/tests/contract.rs index f834243..75afa52 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -1,7 +1,7 @@ //! **F1 and F2** — what a session turn's contract carries, and what it must not. use std::path::PathBuf; -use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; +use std::sync::{Arc, MutexGuard, OnceLock}; use std::time::Duration; use io_cli::contract::{server_notices, session, Capabilities, PROMPT}; @@ -33,9 +33,11 @@ fn nothing() -> Config { /// each other's discovery wrong — intermittently, on a loaded machine, which is /// the most expensive kind of failure to diagnose. The same shape /// `tests/wizard.rs` uses, and for the same reason. +/// +/// Delegated to [`support::env_lock`] rather than declared here: two different +/// mutexes in one binary exclude nothing from each other. fn env_lock() -> MutexGuard<'static, ()> { - static LOCK: Mutex<()> = Mutex::new(()); - LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + support::env_lock() } /// One empty user scope for this whole file, set once. @@ -78,6 +80,49 @@ fn discovered(files: &[(&str, &str)]) -> (tempfile::TempDir, Config) { (dir, config) } +/// As [`discovered`], with the configuration itself in the **user** scope. +/// +/// io-harness 0.74.0 refuses `[[provider]]`, `[[mcp]]`, `[[lsp]]`, `[[hook]]`, +/// provider-executed `[web]` access, ten widening policy values and an absolute +/// `run.skills` from any file that lives inside a workspace — those files arrive +/// with a `git clone`, or sit in a root the run's own agent can write to. +/// [`EVERY_SECTION`] names four of those things, so it cannot be the project file +/// any more. +/// +/// **Not `support::user_scope`**, and the difference is the `files` argument: a +/// contract reads `[instructions]` off disk, relative to the *discovery root* +/// (`config.rs:2673`), so the workspace still has to hold `GUIDE.md` while the +/// configuration lives outside it. `support::user_scope` hands back an empty +/// workspace by design. +/// +/// The user file is deliberately in a second directory. A file that is both the +/// `IO_CONFIG` target and `root/io.toml` is a candidate twice — once as +/// `Scope::User` and once as `Scope::Project` — and the project read is the one +/// that refuses it. +/// +/// The caller holds [`env_lock`] for the call and holds **both** returned +/// directories for as long as the configuration is used. +fn discovered_in_user_scope( + config_toml: &str, + files: &[(&str, &str)], +) -> (tempfile::TempDir, tempfile::TempDir, Config) { + no_user_scope(); + let home = tempfile::tempdir().expect("a directory for the user-scope file"); + let dir = tempfile::tempdir().expect("a workspace"); + for (name, body) in files { + std::fs::write(dir.path().join(name), body).expect("the fixture file"); + } + let path = home.path().join("io.toml"); + std::fs::write(&path, config_toml).expect("the user-scope fixture file"); + + std::env::set_var("IO_CONFIG", &path); + let config = Config::discover(dir.path()); + std::env::remove_var("IO_CONFIG"); + + let config = config.expect("the fixture configuration parses"); + (home, dir, config) +} + /// The operator's own home directory, for the length of one test. /// /// **Every assertion about a contract's `skills` field needs this from 0.15.0**, @@ -1243,7 +1288,7 @@ fn a_turn_is_not_capped_at_twelve_steps() { #[test] fn f1_both_arms_carry_the_same_configuration_field_by_field() { let _guard = env_lock(); - let (dir, config) = discovered(&[("io.toml", EVERY_SECTION), ("GUIDE.md", GUIDE)]); + let (_home, dir, config) = discovered_in_user_scope(EVERY_SECTION, &[("GUIDE.md", GUIDE)]); // The harness's own session, because `exec::contract` reads its root off one // — and the interactive arm is handed that same root, so a difference here @@ -1308,8 +1353,8 @@ fn f1_both_arms_carry_the_same_configuration_field_by_field() { assert_eq!(interactive.lsp, headless.lsp, "[[lsp]]"); assert_eq!( interactive.browser, headless.browser, - "[browser] — absent in both, because io-harness refuses the table in a \ - project-scoped file and this fixture is one", + "[browser] — absent in both, because the fixture declares no `[browser]` \ + table at all", ); // The three that are arm-specific by construction, asserted as differences @@ -1349,7 +1394,7 @@ fn f1_both_arms_carry_the_same_configuration_field_by_field() { #[test] fn f3_every_applicable_section_of_the_file_reaches_a_session_turn() { let _guard = env_lock(); - let (dir, config) = discovered(&[("io.toml", EVERY_SECTION), ("GUIDE.md", GUIDE)]); + let (_home, dir, config) = discovered_in_user_scope(EVERY_SECTION, &[("GUIDE.md", GUIDE)]); let (answerer, _questions) = io_cli::intent::channel(); let contract = session( @@ -1568,8 +1613,7 @@ fn f4_the_step_floor_sits_under_the_file_and_over_the_harness() { #[test] fn f5_servers_in_both_scopes_are_merged_and_a_collision_is_named() { let _guard = env_lock(); - let (dir, config) = discovered(&[( - "io.toml", + let (_home, dir, config) = discovered_in_user_scope( r#" [[mcp]] id = "shared" @@ -1601,7 +1645,8 @@ fn f5_servers_in_both_scopes_are_merged_and_a_collision_is_named() { command = "narrow-lsp" extensions = ["rs"] "#, - )]); + &[], + ); let (stored, complaint) = io_cli::settings::stored(&config); assert!(complaint.is_none(), "{complaint:?}"); @@ -1660,10 +1705,15 @@ fn f5_servers_in_both_scopes_are_merged_and_a_collision_is_named() { ); // A file with servers in one scope only loses nothing and says nothing. - let (quiet, one_scope) = discovered(&[( - "io.toml", + // + // User-scoped like the fixture above, and for the same reason: `[[mcp]]` is + // one of the sections 0.74.0 refuses from a file that lives in a workspace, so + // "one scope" can only mean the operator's own file. There is no longer a + // project scope for a server to be alone in. + let (_quiet_home, quiet, one_scope) = discovered_in_user_scope( "[[mcp]]\nid = \"only\"\ntransport = \"stdio\"\ncommand = \"only\"\n", - )]); + &[], + ); let _ = &quiet; assert!( server_notices(&one_scope, &one_scope.plugins(), &Capabilities::default()).is_empty(), diff --git a/tests/docs.rs b/tests/docs.rs index e6e80d0..2aed12f 100644 --- a/tests/docs.rs +++ b/tests/docs.rs @@ -2428,6 +2428,16 @@ fn f6_the_exit_four_documentation_names_the_approval_carve_out() { /// beforehand — asserted against [`io_cli::exec::asks_nobody_can_answer`], which /// reads `write` and `exec` and never `net`. A page can be edited; that function /// cannot be edited by editing the page. +/// +/// **0.35.0 deletes the exit-code half of this test rather than adjusting it.** +/// It pinned two paragraphs — one here, one on `docs/CONTRACT.md` — saying such a +/// run exits `1`, which is the known defect 0.34.1 documented instead of fixing. +/// 0.35.0 fixes it, so both paragraphs are gone and the assertions that held them +/// fail by design. What replaces them is a count over both headless doors in +/// `tests/exec.rs`, which reads the code out of the seam that chooses it rather +/// than out of a page. The carve-out itself is untouched and still asserted here: +/// the provider endpoint is still authorized before the first step, and there is +/// still nothing to adapt. #[test] fn f6_the_headless_guide_carves_out_the_provider_endpoint() { // The notice an asking posture earns. `net` is deliberately not one of the @@ -2456,9 +2466,6 @@ fn f6_the_headless_guide_carves_out_the_provider_endpoint() { // The one configuration that reaches it, so the carve-out is actionable // rather than a warning about nothing. r#"`act = "net", effect = "ask"`"#, - // And the exit code it produces today, which is not the one the table - // below it gives for a boundary. - "exits `1` and not the `2`", ] { assert!( guide.contains(said), @@ -2466,19 +2473,4 @@ fn f6_the_headless_guide_carves_out_the_provider_endpoint() { refusal claim is back to covering a case it does not cover", ); } - - // And the contract page says it too. - // - // The first draft of this release admitted the wrong exit code in the guide - // and left `docs/CONTRACT.md` publishing `2` for every refusal, so the two - // pages contradicted each other about the number a CI job branches on — with - // the contradiction on the page headed "the contract a CI job depends on". - // An operator who reads one page and not the other is the whole audience for - // this carve-out, so it is asserted on both. - let contract = unwrapped_prose(&read("docs/CONTRACT.md")); - assert!( - contract.contains("One boundary refusal does not reach `2` today"), - "`docs/CONTRACT.md` no longer carries the exit-`2` carve-out, so it now \ - disagrees with the headless guide about the code a refused run exits", - ); } diff --git a/tests/edit.rs b/tests/edit.rs index 0f85688..ddd960b 100644 --- a/tests/edit.rs +++ b/tests/edit.rs @@ -13,6 +13,20 @@ use io_cli::edit::{self, Edit}; +mod support; + +/// Load a written file the way io-harness would load the operator's own. +/// +/// **Never `Config::from_toml` for a file with `[[provider]]` in it.** `from_toml` +/// parses at `Scope::Project`, and io-harness 0.74.0 refuses a provider there — a +/// provider names the endpoint this run's credential is sent to, and `io.toml` +/// arrives with a `git clone`. The moves below are moves of provider entries, so +/// the round trip that proves the move survived has to read at the scope that may +/// declare one. Files with no refused section still go through `from_toml`. +fn loaded(text: &str) -> io_harness::Config { + support::user_scope(text).config.clone() +} + /// A file with every shape an operator's real `io.toml` has and io-cli does not /// model: a header comment, an inline comment on the very key being changed, an /// array of tables, a section io-cli has no type for, and a blank-line rhythm. @@ -373,7 +387,7 @@ max_steps = 30 ); assert!(up.contains("preset = \"groq\""), "key lost"); assert!(up.contains("max_steps = 30"), "a later section moved"); - let config = io_harness::Config::from_toml(&up).expect("the moved file loads"); + let config = loaded(&up); assert_eq!(config.fallback_specs().len(), 1); // And moving it back is the identity, which is the property a one-way @@ -965,7 +979,7 @@ model = \"b\" # groq answers last, and there is no newline after this line"; let down = edit::apply(COMMENTED, &[Edit::move_entry("provider", 0, 1)]).unwrap(); - let config = io_harness::Config::from_toml(&down).expect("the moved file loads"); + let config = loaded(&down); assert_eq!( config.fallback_specs().len(), 1, @@ -993,7 +1007,7 @@ preset = \"groq\" model = \"b\""; let up = edit::apply(TAIL, &[Edit::move_entry("provider", 1, 0)]).unwrap(); - let config = io_harness::Config::from_toml(&up).expect("the moved file loads"); + let config = loaded(&up); assert_eq!(config.fallback_specs().len(), 1, "{up}"); assert!( up.find("groq").unwrap() < up.find("openrouter").unwrap(), diff --git a/tests/exec.rs b/tests/exec.rs index ffe4f1f..7a5aaba 100644 --- a/tests/exec.rs +++ b/tests/exec.rs @@ -15,6 +15,9 @@ //! 0.34.1 F2 — `docs/CONTRACT.md`'s exit-code table is the constants, row by row. //! 0.34.1 F3 — io-cli's own argument refusals exit 1, on the same door. //! +//! 0.35.0 F2 — a typed `Error::Refused` exits 2 on both headless doors, exactly +//! one harness variant does, and a failure to start still exits 1. +//! //! 0.23.0 F8 — `io resume`: the parked runs are listable, one is resumable by id, //! the parked line names the id that addresses the pause, and the two pauses that //! cannot be carried on are refused in words rather than by a failed drive. @@ -750,22 +753,28 @@ fn f7_the_warning_names_only_what_actually_asks() { // A file can ask about writes and not commands, or the reverse. Naming both // when only one asks is the kind of true-sounding sentence that teaches the // wrong thing about the boundary in force. - // `exec = "deny"` rather than `"allow"`: `Config::from_toml` parses at - // PROJECT scope, where a value that widens the boundary is refused outright, - // because a repository you cloned must not be able to grant itself - // permission. Narrowing is always allowed. - let write_only = Config::from_toml( + // **User-scoped, because io-harness 0.74.0 refuses `read = "allow"` from any + // file a workspace can supply** — the key joined `write`, `exec` and `net` on + // `PROJECT_WIDENING` in that release, and `Config::from_toml` *is* the project + // scope, so the four defaults can no longer be stated together there. Written + // out in full rather than trimmed to the three still legal: the subject is + // which of the four `asks_nobody_can_answer` names, and a fixture that left one + // of them to the shipped default would be asserting about a policy it did not + // state. + let write_only = support::user_scope( "[policy.defaults]\nread = \"allow\"\nwrite = \"ask\"\nexec = \"deny\"\nnet = \"deny\"\n", ) - .expect("the configuration parses"); + .config + .clone(); let line = exec::asks_nobody_can_answer(&exec::policy_for(&write_only, None)) .expect("a write that asks still warns"); assert!(line.contains("every write will be denied"), "{line}"); - let exec_only = Config::from_toml( + let exec_only = support::user_scope( "[policy.defaults]\nread = \"allow\"\nwrite = \"deny\"\nexec = \"ask\"\nnet = \"deny\"\n", ) - .expect("the configuration parses"); + .config + .clone(); let line = exec::asks_nobody_can_answer(&exec::policy_for(&exec_only, None)) .expect("a command that asks still warns"); assert!(line.contains("every command will be denied"), "{line}"); @@ -888,10 +897,17 @@ fn f8_the_provider_names_are_spelled_the_way_the_harness_spells_them() { // The same words io-harness's own `ProviderSpec` is tagged with, so one // vocabulary spans the configuration file and the command line. + // + // **Read back through the user scope, because 0.74.0 refuses `[[provider]]` + // from any file inside a workspace** — a provider names the endpoint the run's + // credential is sent to, and it is contacted before the run's first step. The + // lock is taken once around the loop and the fixture form that does not take it + // again is the one called, because `std::sync::Mutex` is not reentrant. + let _guard = support::env_lock(); for name in &names { let toml = format!("[[provider]]\nkind = \"{name}\"\nmodel = \"m\"\n"); assert!( - Config::from_toml(&toml).is_ok(), + support::try_user_scope_locked(&toml, false).is_ok(), "io-harness does not know a provider called `{name}`", ); } @@ -2363,3 +2379,515 @@ fn f2_the_documented_exit_code_table_is_the_constants() { } } } + +/// The seam that chooses an exit code from a harness failure, once per door. +/// +/// **A pair and not a single conversion, because two doors reach this decision.** +/// `io exec` arrives through `exec::turn`, and `io resume` through the four +/// `crate::resume` drivers, whose `Failure` wraps the harness's error. 0.30.0 +/// records exactly what happens to a gate that only one door satisfies: it goes +/// vacuous and nobody notices. Every assertion below is therefore driven over +/// this table and counted, rather than asserted once and assumed for the other. +/// +/// The resume arm goes through `From for Failure` — the same conversion +/// the drivers' own `?` performs — rather than constructing `Failure::Harness` +/// by hand, so it is the production path and not a re-spelling of it. +/// +/// **This is the seam and not the whole journey, and that is a limitation rather +/// than a choice.** A policy can only refuse what a run does, and a run has to +/// reach a provider before it does anything, so no offline test can drive a +/// boundary refusal through the real argv door. The end-to-end arm is +/// live-suite-only. +/// One headless door: what to call it, and the seam it reaches. +type Door = (&'static str, fn(io_harness::Error) -> exec::Ending); + +const DOORS: [Door; 2] = [("io exec", exec_door), ("io resume", resume_door)]; + +/// What `io exec` does with a harness failure: `exec::turn` hands it straight to +/// `Ending`. +fn exec_door(error: io_harness::Error) -> exec::Ending { + exec::Ending::from(error) +} + +/// What `io resume` does with one: the four drivers' `?` wraps it in +/// `resume::Failure` on the way out, and the door unwraps it again. +fn resume_door(error: io_harness::Error) -> exec::Ending { + exec::Ending::from(io_cli::resume::Failure::from(error)) +} + +/// A boundary refusal, shaped the way io-harness raises one when the provider's +/// own endpoint lands in the `ask` tier: an act, a target, and the rule and layer +/// that decided. +fn a_boundary_refusal() -> io_harness::Error { + io_harness::Error::Refused { + act: "net".into(), + target: "openrouter.ai".into(), + rule: Some("*.openrouter.ai".into()), + layer: Some("project".into()), + } +} + +/// **0.35.0 F2 — a typed refusal exits `2`, and it does so at both doors.** +/// +/// The defect 0.34.1 found, documented as a known limitation, and left for a +/// release allowed to change behaviour: both headless doors flattened +/// `io_harness::Error` to a `String` before an exit code was chosen, and +/// `main.rs` has one answer for a `String` — `FAILED`. So the one code that +/// exists to mean "a boundary said no" was unreachable for the one refusal that +/// happens before a run's first step. +/// +/// Sabotage: return `FAILED` from either arm of `From`, or +/// delete the resume door's conversion — the count below names which door lost. +#[test] +fn f2_a_typed_boundary_refusal_exits_refused_at_both_headless_doors() { + assert_eq!( + DOORS.len(), + 2, + "`io exec` and `io resume` both choose an exit code from a harness \ + failure; a table with one door in it proves nothing about the other", + ); + + let mut refused = 0; + for (door, ending_for) in DOORS { + let ending = ending_for(a_boundary_refusal()); + assert_eq!( + ending.code, + exec::REFUSED, + "`{door}` exits {} for a typed `Error::Refused`, and `{}` is what \ + `docs/CONTRACT.md` publishes for a boundary saying no", + ending.code, + exec::REFUSED, + ); + // Not decoration: an `Ending` that reported the right code under some + // other sentence would leave the operator reading about a failure while + // their CI branched on a refusal. + assert!( + ending.to_string().contains("refused by policy"), + "`{door}` no longer carries the harness's own words for the refusal: \ + {ending}", + ); + refused += 1; + } + assert_eq!( + refused, + DOORS.len(), + "one of the two headless doors did not answer REFUSED for a refusal", + ); +} + +/// **0.35.0 F2 — the arm is one variant wide, per neighbour and per door.** +/// +/// The tempting fix is a `_ =>` that catches everything the harness calls a +/// failure, and it would make a configuration fault report as a boundary +/// refusal — the mirror of the defect being removed, and the third time this +/// table would have been given away by a convenient catch-all. +/// +/// These two are the neighbours io-harness itself types apart from a refusal. +/// `Error::Sandbox` is "the sandbox failed to start", separate from `Error::Io` +/// so a caller can tell "the sandbox never ran the code" from "the code ran and +/// failed". `Error::Config` is "configuration was missing or invalid", and +/// `Error::Refused`'s own documentation says it is "typed separately from +/// `Error::Config` so a refusal is distinguishable from a malfunction". +/// +/// The resume door's own refusals are here too. None of them is a boundary: they +/// are io-cli declining to drive a resume at all, which is `FAILED` for the same +/// reason `io exec --policy ask-writes` is — the boundary never got a chance to +/// say anything. +#[test] +fn f2_a_failure_to_start_is_not_a_boundary_refusal_at_either_door() { + for (door, ending_for) in DOORS { + for (name, error) in [ + ( + "Sandbox", + io_harness::Error::Sandbox { + reason: "no sandbox backend on this host".into(), + }, + ), + ( + "Config", + io_harness::Error::Config("no API key for the configured provider".into()), + ), + ] { + let ending = ending_for(error); + assert_eq!( + ending.code, + exec::FAILED, + "`{door}` reports `Error::{name}` as {}, so a run that never \ + started now tells a script a boundary refused it", + ending.code, + ); + } + } + + for (name, failure) in [ + ( + "Interrupted", + io_cli::resume::Failure::Interrupted { run_id: 7 }, + ), + ("Ended", io_cli::resume::Failure::Ended { run_id: 7 }), + ("Unknown", io_cli::resume::Failure::Unknown { run_id: 7 }), + ( + "HeadMoved", + io_cli::resume::Failure::HeadMoved { + session_id: 1, + turn_id: 2, + }, + ), + ] { + assert_eq!( + exec::Ending::from(failure).code, + exec::FAILED, + "`io resume` reports its own `Failure::{name}` as a boundary refusal, \ + and no boundary was asked", + ); + } +} + +/// **0.35.0 F2 — exactly one harness variant is a refusal, counted.** +/// +/// The audit as an assertion rather than as prose. Every variant `Error` declares +/// is put through the seam and the refusals are counted: one, and it is the one +/// named `Refused`. A `contains`-shaped gate would pass under the wildcard this +/// exists to forbid; a count does not. +/// +/// `Error::State` is the only variant absent, and deliberately: it is deprecated +/// since io-harness 0.63.0, nothing in the crate constructs it, and building one +/// would mean taking a `rusqlite` dependency to name a variant that is scheduled +/// for removal. +/// +/// **What this cannot see is a *new* variant.** `Error` is `#[non_exhaustive]`, +/// the list below is written by hand, and `tests/support` — which is where the +/// locked-source readers live — is not this test's to extend. The wildcard makes +/// the safe direction automatic: anything unrecognised is `FAILED`, so a variant +/// a later harness adds cannot become a refusal by accident. The unsafe direction +/// is a harness that adds a *second* refusal, and that is a question for the pin +/// bump that brings it, not something this file can answer. +#[test] +fn f2_exactly_one_harness_error_variant_is_a_boundary_refusal() { + // Built per call rather than once, because `io_harness::Error` is not `Clone` + // and the seam takes it by value. A list built once and reused across the two + // doors would have to rebuild the values from something, and rebuilding them + // from a single kind is how this test went vacuous in its first draft: eleven + // names, one variant, every door passing. + fn every_variant() -> Vec<(&'static str, io_harness::Error)> { + vec![ + ("Io", io_harness::Error::Io(std::io::Error::other("disk"))), + ( + "Storage", + io_harness::Error::Storage { + kind: io_harness::StorageErrorKind::Busy, + message: "another connection holds the write lock".into(), + }, + ), + ( + "Provider", + io_harness::Error::provider_transport("connection refused"), + ), + ("Config", io_harness::Error::Config("no API key".into())), + ( + "Sandbox", + io_harness::Error::Sandbox { + reason: "no backend".into(), + }, + ), + ("Refused", a_boundary_refusal()), + ( + "Mcp", + io_harness::Error::Mcp { + server: "docs".into(), + reason: "the process exited".into(), + }, + ), + ( + "Lsp", + io_harness::Error::Lsp { + server: "rust-analyzer".into(), + reason: "the process exited".into(), + }, + ), + ( + "Browser", + io_harness::Error::Browser { + reason: "no browser on this host".into(), + }, + ), + ( + "Resume", + io_harness::Error::Resume { + reason: "the checkpoint is newer than this binary".into(), + }, + ), + ( + "Conflict", + io_harness::Error::Conflict { + run_id: 7, + owner: "another process".into(), + expires_at: "2026-09-02T00:00:00Z".into(), + }, + ), + ] + } + + assert_eq!( + every_variant().len(), + 11, + "io-harness 0.74.0 declares twelve `Error` variants and this table names \ + eleven of them — every one but the deprecated `State`. A variant added \ + or removed here is a pin bump nobody has audited", + ); + + for (door, ending_for) in DOORS { + let refusals: Vec<&str> = every_variant() + .into_iter() + .map(|(name, error)| (name, ending_for(error).code)) + .filter(|(_, code)| *code == exec::REFUSED) + .map(|(name, _)| name) + .collect(); + assert_eq!( + refusals, + vec!["Refused"], + "`{door}` treats {refusals:?} as boundary refusals. Exactly one \ + variant is one: a `_ =>` arm that swallowed `Sandbox` or `Config` \ + would tell a CI job a policy spoke when nothing did", + ); + } + + // **And the table above is held against the dependency, because a hand-written + // list is exactly what this release came here to remove.** + // + // The wildcard arm covers the *safe* direction: a variant a later harness adds + // is reported `FAILED`, which is conservative. The direction it does not cover + // is a harness adding a **second refusal** variant — io-harness 0.74.0 added + // two new `Error::Refused` shapes in one release, so that is not hypothetical + // — and io-cli would report it as a failure, which is precisely the defect this + // task exists to fix, arriving again through the dependency instead of through + // this crate. A list of eleven cannot notice it; the locked source can. + let declared = support::harness_error_variants(); + let named: Vec = every_variant() + .into_iter() + .map(|(name, _)| name.to_string()) + .collect(); + let missing: Vec<&String> = declared + .iter() + // `State` is deprecated and deliberately not driven; every other variant + // the locked harness declares must be in the table. + .filter(|variant| variant.as_str() != "State" && !named.contains(variant)) + .collect(); + assert!( + missing.is_empty(), + "the locked io-harness declares {missing:?}, which this table does not drive. A new \ + variant is a pin bump nobody has audited: if it is a refusal it must map to REFUSED, and \ + if it is a malfunction the wildcard already has it — but which of the two it is, is a \ + decision and not a default", + ); +} + +// --- the local-address floor, surfaced with the one thing that lifts it --------- + +/// **The layer io-cli matches is the layer io-harness stamps.** +/// +/// io-harness's `FLOOR_LAYER` is `pub(crate)`, so io-cli spells the string. That +/// is one literal and not a table — the shapes the floor refuses are never +/// re-derived here, because 0.30.0 shipped a copy of one of this dependency's +/// address checks and it **failed open** on five of them. What this holds is the +/// spelling, against the locked source, so a rename upstream fails here rather +/// than silently dropping the remedy from every refusal. +/// +/// Sabotage: change either spelling. This fails naming both. +#[test] +fn f11_the_local_address_floor_layer_is_spelled_as_io_harness_spells_it() { + let source = support::harness_source("net.rs"); + let declared = source + .split_once("FLOOR_LAYER: &str = \"") + .and_then(|(_, rest)| rest.split_once('"')) + .map(|(value, _)| value) + .expect("io-harness declares FLOOR_LAYER in src/net.rs"); + + assert_eq!( + declared, + exec::LOCAL_ADDRESS_FLOOR, + "io-harness stamps `{declared}` on a floor refusal and io-cli looks for \ + `{}`, so the remedy reaches nobody", + exec::LOCAL_ADDRESS_FLOOR, + ); +} + +/// **A refusal from the floor carries the one thing that lifts it.** +/// +/// Everyone pointed at a model on their own machine — Ollama, LM Studio, +/// llama.cpp — is refused at run start on this pin, whatever their policy says, +/// and io-harness gives the lift no configuration key on purpose. A bare refusal +/// with no remedy is the single most user-visible break in this release. +/// +/// Asserted at the seam for **both** doors, because a remedy on one of them is a +/// remedy an `io resume` never shows. +#[test] +fn f11_a_floor_refusal_names_the_variable_that_lifts_it_at_both_doors() { + for (door, ending_for) in DOORS { + let ending = ending_for(io_harness::Error::Refused { + act: "net".into(), + target: "127.0.0.1:11434".into(), + rule: Some("loopback".into()), + layer: Some(exec::LOCAL_ADDRESS_FLOOR.to_string()), + }); + + assert_eq!(ending.code, exec::REFUSED, "{door}: the boundary said no"); + assert!( + ending + .to_string() + .contains("IO_HARNESS_ALLOW_LOCAL_ADDRESSES"), + "{door}: a floor refusal with no remedy leaves a local-model operator with nothing \ + to do about it: {ending}", + ); + assert!( + ending.to_string().contains("127.0.0.1:11434"), + "{door}: and it still carries io-harness's own sentence, which names the endpoint: \ + {ending}", + ); + } + + // A refusal from any other layer is left exactly as io-harness wrote it: the + // remedy is about one floor and would be noise, or a lie, anywhere else. + for (door, ending_for) in DOORS { + let ending = ending_for(io_harness::Error::Refused { + act: "write".into(), + target: "/etc/hosts".into(), + rule: Some("*".into()), + layer: Some("builtin-secrets".into()), + }); + assert!( + !ending + .to_string() + .contains("IO_HARNESS_ALLOW_LOCAL_ADDRESSES"), + "{door}: a write refusal is not lifted by a network variable: {ending}", + ); + } +} + +/// **io-cli sets that variable nowhere, and an absence needs a source-text gate.** +/// +/// The lift is the operator's own explicit choice — io-harness gave it no +/// configuration key precisely so a file cannot grant it — and a product whose +/// thesis is that the boundary is legible must not re-grant, from the inside, a +/// permission the harness has just taken away. Even for an operator whose own +/// `io.toml` names a loopback endpoint: that file is a statement about where the +/// model is, not consent to leave the sandbox. +/// +/// 0.30.0's `F15` had no site for exactly this reason — an absence cannot be +/// asserted by driving anything — so this reads the source. +#[test] +fn f11_io_cli_never_sets_the_local_address_variable() { + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut sets = Vec::new(); + + let mut stack = vec![src]; + while let Some(dir) = stack.pop() { + for entry in std::fs::read_dir(&dir).expect("src is readable").flatten() { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + continue; + } + if path.extension().is_none_or(|e| e != "rs") { + continue; + } + let text = std::fs::read_to_string(&path).expect("a source file is readable"); + for (number, line) in text.lines().enumerate() { + let code = line.split_once("//").map_or(line, |(before, _)| before); + if code.contains("set_var") && code.contains("IO_HARNESS_ALLOW_LOCAL_ADDRESSES") { + sets.push(format!("{}:{}", path.display(), number + 1)); + } + } + } + } + + assert!( + sets.is_empty(), + "io-cli sets IO_HARNESS_ALLOW_LOCAL_ADDRESSES at {sets:?}. The harness gave that lift no \ + configuration key on purpose; granting it from inside the product is re-opening a \ + boundary the operator never asked to open", + ); +} + +// --- `[run] templates` reaches the headless door too --------------------------- + +/// **`run.templates` was applied on one door only, and the limits page named the +/// wrong thing as the gap.** +/// +/// `commands::templates` has read the key through its own accessor since the +/// palette learned prompt templates — but only an interactive session called it, +/// so `io exec` ignored a key `docs/config.example.toml` documents. A +/// configuration key that works in the terminal and silently does nothing in CI is +/// the asymmetry this product deleted in 0.14.0, arriving again through a door +/// nobody had checked. +/// +/// **User-scoped, and 0.74.0 is why.** `run.templates` naming an absolute +/// directory is refused from any file inside a workspace: every `*.md` under it is +/// composed into the model's system prompt, so a cloned `io.toml` could put this +/// host's files into the model's context. The operator's own file is the only one +/// that may name a directory outside the workspace. +/// +/// Sabotage: pass `args.goal` through untouched, which is the pre-0.35.0 body. +/// The first arm fails with the template's own name where its text should be. +#[test] +fn f10_a_headless_goal_may_name_a_template_and_a_plain_goal_is_untouched() { + let dir = tempfile::tempdir().expect("a directory of templates"); + std::fs::write( + dir.path().join("review.md"), + "Read every changed line and say what is wrong with it.\n", + ) + .expect("a template is written"); + + let declared = io_cli::edit::spell(&dir.path().display().to_string()); + let scope = support::user_scope(&format!("[run]\ntemplates = {declared}\n")); + + let (rendered, notice) = + exec::goal_for(&scope.config, "/review").expect("the template renders"); + // Trailing newline trimmed: that is `Templates::render`'s doing, not this + // function's, and it is the same text the palette puts in the composer. + assert_eq!( + rendered, "Read every changed line and say what is wrong with it.", + "the headless goal is the template's text, not its name", + ); + assert!(notice.is_none(), "a directory that reads says nothing"); + + // The overwhelmingly common case, which this must not change. + let (plain, _) = + exec::goal_for(&scope.config, "summarise the module").expect("a prompt is not a template"); + assert_eq!( + plain, "summarise the module", + "a goal that does not begin with `/` is a prompt and is passed through", + ); + + // A name no template has is a goal that does not exist, and it is refused + // before a provider is built rather than spent on a call that says so. + let refused = exec::goal_for(&scope.config, "/not-a-template") + .expect_err("a template that is not there cannot be rendered"); + assert!( + refused.contains("not-a-template"), + "the refusal names what was asked for: {refused}", + ); +} + +/// And a configuration naming no templates leaves every goal alone. +/// +/// The arm that stops the expansion being reachable by accident: with no +/// `[run] templates`, `commands::templates` answers `Templates::none()`, and a +/// goal beginning with `/` is refused rather than silently becoming empty. +#[test] +fn f10_a_configuration_with_no_templates_passes_every_plain_goal_through() { + let scope = support::user_scope("[run]\nmax_steps = 10\n"); + + let (plain, notice) = + exec::goal_for(&scope.config, "summarise the module").expect("a prompt needs no templates"); + assert_eq!(plain, "summarise the module"); + assert!( + notice.is_none(), + "a configuration that names no directory has nothing to complain about", + ); + + assert!( + exec::goal_for(&scope.config, "/review").is_err(), + "a template named where none are configured is refused, not rendered empty", + ); +} diff --git a/tests/hooks.rs b/tests/hooks.rs index 0a9681d..865abc4 100644 --- a/tests/hooks.rs +++ b/tests/hooks.rs @@ -19,6 +19,17 @@ //! Fixtures that must be exact about the *absence* of a hook use //! `Config::from_toml`, which reads only the text it is given. //! `Config::discover` also reads a user-scope file where the machine has one. +//! +//! **io-harness 0.74.0 moved the line this file is about.** `[[hook]]` was refused +//! from `io.toml` and permitted in `io.local.toml`; it is now refused from both, +//! because `io.local.toml` is a path in the workspace root and a run's own agent +//! writes paths in the workspace root — one `write_file` of it declared an argv the +//! next `Config::discover` would run, outside the `Policy` and outside the sandbox. +//! The user scope is the one file no workspace can reach and is now the only place +//! a `[[hook]]` may be written, so every fixture below that declares one goes +//! through [`support::user_scope`] rather than writing into the root. + +mod support; use std::path::PathBuf; @@ -39,8 +50,8 @@ fn written(file: &str, text: &str) -> (tempfile::TempDir, PathBuf) { (dir, root) } -/// A hook that watches an event and runs a program. Valid in every scope but the -/// project one. +/// A hook that watches an event and runs a program. Valid in the user scope, and +/// since io-harness 0.74.0 in no other. const EVENT_HOOK: &str = "[[hook]]\non = [\"finished\"]\nrun = [\"true\"]\n"; // --------------------------------------------------------------------------- @@ -51,10 +62,16 @@ const EVENT_HOOK: &str = "[[hook]]\non = [\"finished\"]\nrun = [\"true\"]\n"; /// refusal carries io-harness's own sentence and names the directory it came from. /// /// Both halves matter and neither substitutes for the other. The sentence is the -/// only thing that tells an operator what to do — move the table to -/// `io.local.toml` — and the directory is the only thing that tells them *which* -/// checkout is refusing, which is the whole question when a session will not start -/// in one of six clones on the same machine. +/// only thing that tells an operator what to do — move the table to the user-scope +/// file — and the directory is the only thing that tells them *which* checkout is +/// refusing, which is the whole question when a session will not start in one of +/// six clones on the same machine. +/// +/// **The way out changed in io-harness 0.74.0 and the assertion changed with it.** +/// Until then the sentence named `io.local.toml`; that file is now held to the same +/// rule, so what the refusal names is `$IO_CONFIG` and its two fallbacks. The +/// substring is `IO_CONFIG` because the sentence is spelled per platform — +/// `%IO_CONFIG%` on Windows, `$IO_CONFIG` everywhere else — and both carry it. /// /// Sabotage: reword `configure::refusal` to "your io.toml is invalid", the sort of /// tidy-up that reads as an improvement in review. Under it only this test fails, @@ -62,6 +79,16 @@ const EVENT_HOOK: &str = "[[hook]]\non = [\"finished\"]\nrun = [\"true\"]\n"; /// key named, no rule explained and no file to open. #[test] fn f8_a_project_scoped_hook_is_refused_and_io_cli_names_the_rule_and_the_root() { + // **The one test in this file that reads a scope it did not write, so it is the + // one that has to hold the lock across the read.** `Config::discover` consults + // `$IO_CONFIG` before it reaches the workspace, and the fixtures beside this one + // point that variable at files that are *deliberately* unreadable — a malformed + // `[[hook]]`, a misspelled event name. A discovery that landed inside one of + // those windows would fail on the user scope and this test would assert the + // wrong refusal, on CI, intermittently. Held for the whole body, and the + // fixture form that does not take it again is the one called below. + let _guard = support::env_lock(); + let (_dir, root) = written(PROJECT_FILE, EVENT_HOOK); let error = @@ -71,13 +98,14 @@ fn f8_a_project_scoped_hook_is_refused_and_io_cli_names_the_rule_and_the_root() // io-harness's own words, not a paraphrase of them. assert!( said.contains( - "a project-scoped file may not declare hooks, because a hook runs or writes on \ - this machine" + "a project-scoped file may not declare hooks — a hook runs an argv on this \ + machine, or appends to a path the file itself chose, on an event the file \ + itself picks" ), "io-cli reworded the refusal: {said}", ); assert!( - said.contains(LOCAL_FILE), + said.contains("IO_CONFIG"), "the operator is not told where the table may live: {said}", ); assert!( @@ -86,11 +114,26 @@ fn f8_a_project_scoped_hook_is_refused_and_io_cli_names_the_rule_and_the_root() which one refused: {said}", ); - // The same table one file over is the operator's own machine talking, and it - // loads — so the test above is about the scope rather than about hooks. + // **`io.local.toml` is refused too, and the refusal says a different thing.** + // Both files are inside the workspace root and both are therefore untrusted; + // what separates them is why, and an operator reading the wrong one of the two + // sentences goes looking in the wrong place. Asserted here rather than left + // implicit, because "the project scope is the strict one" is what this test + // said for six releases and it is no longer true. let (_dir, local) = written(LOCAL_FILE, EVENT_HOOK); - let config = Config::discover(&local).expect("a local hook is the operator's own"); - assert!(!config.hooks().is_empty()); + let error = Config::discover(&local) + .expect_err("0.74.0 holds `io.local.toml` to the same rule: the agent can write it"); + let said = error.to_string(); + assert!( + said.contains("a workspace-root `io.local.toml` may not declare hooks"), + "the local-scope refusal is not io-harness's own: {said}", + ); + + // The same table in the one file no workspace can reach is the operator's own + // machine talking, and it loads — so the two refusals above are about the scope + // rather than about hooks. + let scope = support::user_scope_locked(EVENT_HOOK, false); + assert!(!scope.config.hooks().is_empty()); } // --------------------------------------------------------------------------- @@ -131,19 +174,22 @@ fn f9_a_configuration_with_no_hook_attaches_no_hooks_at_all() { ); // And a configuration that says plenty, none of it a hook — so `None` is not - // `None` because there was no file to read. - let (_dir, busy) = written( - LOCAL_FILE, + // `None` because there was no file to read. User-scoped, because `[[mcp]]` is + // a refused section in every scope a workspace can supply since 0.74.0: an MCP + // server is a command, an argv and an environment this process spawns at run + // start. + let scope = support::user_scope( "[run]\nmax_steps = 40\n\n[[mcp]]\nid = \"docs\"\ntransport = \"stdio\"\n\ command = \"mcp-docs\"\n", ); - let config = Config::discover(&busy).expect("the configuration loads"); + let busy = scope.root().to_path_buf(); + let config = &scope.config; assert!(config.hooks().is_empty()); assert!( - io_cli::contract::hooks(&config, &config.plugins(), &busy).is_none(), + io_cli::contract::hooks(config, &config.plugins(), &busy).is_none(), "a configuration with servers and a step cap and no hook was handed a Hooks", ); - let contract = io_cli::contract::configured("go", busy.clone(), &config, &config.plugins()); + let contract = io_cli::contract::configured("go", busy.clone(), config, &config.plugins()); assert!( contract.tool_hooks.is_none(), "the same, on the contract the turn carries", @@ -168,18 +214,18 @@ fn f9_a_configuration_with_no_hook_attaches_no_hooks_at_all() { /// refuse a tool call refuses nothing. #[test] fn f9_a_declared_hook_reaches_both_the_contract_and_the_fan_out() { - let (_dir, root) = written( - LOCAL_FILE, + let scope = support::user_scope( "[[hook]]\nat = \"before_tool\"\ntools = [\"write_file\"]\nrun = [\"true\"]\n\n\ [[hook]]\non = [\"finished\"]\nappend = \"audit.jsonl\"\n", ); - let config = Config::discover(&root).expect("the configuration loads"); + let root = scope.root().to_path_buf(); + let config = &scope.config; - let hooks = io_cli::contract::hooks(&config, &config.plugins(), &root) + let hooks = io_cli::contract::hooks(config, &config.plugins(), &root) .expect("the file declares two hooks"); assert!(!hooks.is_empty()); - let contract = io_cli::contract::configured("go", root.clone(), &config, &config.plugins()); + let contract = io_cli::contract::configured("go", root.clone(), config, &config.plugins()); assert!( contract.tool_hooks.is_some(), "the lifecycle half of the file is accepted and never consulted", @@ -194,20 +240,31 @@ fn f9_a_declared_hook_reaches_both_the_contract_and_the_fan_out() { /// silently in the direction that matters least visibly: `/plugin` still draws the /// bundle's `hooks` row, the manifest still declares them, and not one of them ever /// runs — for a contribution kind whose entire purpose is to run programs. +/// **The bundle sits outside the workspace and is declared from the user scope, +/// which is what an installed bundle actually is.** io-harness 0.74.0 decides what +/// a manifest may contribute by *where the manifest is* rather than by which file +/// named it: a `plugin.toml` inside the workspace root may not carry a `[[hook]]`, +/// an `[[mcp]]` or a `[[bin]]` whatever declared it, because the run's own agent +/// can write that path. A bundle kept elsewhere and named from `$IO_CONFIG` is the +/// one arrangement that still contributes a hook — and it is where `plugin add` +/// puts one. #[test] fn f9_a_hook_a_bundle_contributes_is_a_declared_hook() { - let (_dir, root) = root(); - let bundle = root.join("runner"); + let (_bundles, bundles) = root(); + let bundle = bundles.join("runner"); std::fs::create_dir_all(&bundle).expect("the bundle directory"); std::fs::write( bundle.join(PLUGIN_FILE), "name = \"runner\"\n\n[[hook]]\nat = \"before_tool\"\nrun = [\"true\"]\n", ) .expect("the manifest"); - std::fs::write(root.join(LOCAL_FILE), "[[plugin]]\npath = \"runner\"\n") - .expect("the configuration"); - let config = Config::discover(&root).expect("the configuration loads"); + // A TOML *literal* string, so a Windows path's backslashes are the path rather + // than a run of escapes. A directory name cannot contain a `'` on the platforms + // this ships to, and `tempfile` never puts one there. + let scope = support::user_scope(&format!("[[plugin]]\npath = '{}'\n", bundle.display())); + let root = scope.root().to_path_buf(); + let config = &scope.config; assert!( config.hooks().is_empty(), "the file itself declares a hook, so this test would pass without the merge", @@ -219,11 +276,11 @@ fn f9_a_hook_a_bundle_contributes_is_a_declared_hook() { ); assert!( - io_cli::contract::hooks(&config, &config.plugins(), &root).is_some(), + io_cli::contract::hooks(config, &config.plugins(), &root).is_some(), "the bundle's hook is declared and nothing in this session will run it", ); assert!( - io_cli::contract::configured("go", root.clone(), &config, &config.plugins()) + io_cli::contract::configured("go", root.clone(), config, &config.plugins()) .tool_hooks .is_some(), "the same, on the contract the turn carries", @@ -251,6 +308,14 @@ fn f9_a_hook_a_bundle_contributes_is_a_declared_hook() { /// disagree, and a hook the panel accepts is refused by the run that carries it. #[test] fn a_malformed_hook_is_refused_by_io_harness_and_io_cli_adds_no_check_of_its_own() { + // **User-scoped, or every arm below is refused for the wrong reason.** + // `refuse_widening` runs against the raw table *before* anything deserializes, + // so in any scope a workspace can supply the four malformed tables come back + // with "may not declare hooks" and this test would pass while asserting nothing + // about `Hook::check`. The lock is taken once and the fixture form that does + // not take it again is used, because `std::sync::Mutex` is not reentrant. + let _guard = support::env_lock(); + for (table, phrase) in [ ( "[[hook]]\non = [\"finished\"]\nat = \"before_tool\"\nrun = [\"true\"]\n", @@ -269,8 +334,8 @@ fn a_malformed_hook_is_refused_by_io_harness_and_io_cli_adds_no_check_of_its_own "`tools` filters a lifecycle hook, and this one has no `at`", ), ] { - let (_dir, root) = written(LOCAL_FILE, table); - let error = Config::discover(&root) + let (_dir, root) = root(); + let error = support::try_user_scope_locked(table, false) .err() .unwrap_or_else(|| panic!("io-harness accepted a malformed hook: {table}")); @@ -283,10 +348,21 @@ fn a_malformed_hook_is_refused_by_io_harness_and_io_cli_adds_no_check_of_its_own // io-cli added — a rewording, a hint, a check of its own that fired first — // makes this fail, which is the point: the operator reads the words of // whoever enforced the rule. + // + // **"was not accepted" rather than "could not be read" since 0.35.0**, and + // this test caught the change, which is what it is for. io-harness 0.74.0 + // refuses whole sections from any file inside a workspace, so the + // commonest reason `discover` fails is now a file that is intact and in + // the wrong place — and "could not be read" told that operator their file + // was broken, which is a different problem with a different fix. The + // wording covers a refusal and a genuine parse failure alike without + // io-cli having to classify which it is: `Error::Config` carries a + // sentence and no structure, so classifying would mean a list of the + // harness's clauses that goes stale the next time it tightens. assert_eq!( io_cli::configure::refusal(&root, &error), format!( - "the configuration in {} could not be read:\n{error}", + "the configuration in {} was not accepted:\n{error}", root.display() ), "io-cli put something of its own between the operator and the refusal", @@ -306,12 +382,14 @@ fn a_malformed_hook_is_refused_by_io_harness_and_io_cli_adds_no_check_of_its_own /// lists, that io-cli attaches to the fan-out, and that never runs once. #[test] fn a_hook_naming_an_event_that_does_not_exist_is_refused_rather_than_installed() { - let (_dir, root) = written( - LOCAL_FILE, - "[[hook]]\non = [\"finsihed\"]\nrun = [\"true\"]\n", - ); - let error = Config::discover(&root) - .expect_err("a misspelled event name is a hook that would never fire"); + // One guard across both halves, and the forms that do not take it again: the + // scope has to be the user's or the refusal is the section rule rather than the + // event-name rule, and taking the lock twice on one thread is a deadlock. + let _guard = support::env_lock(); + + let error = + support::try_user_scope_locked("[[hook]]\non = [\"finsihed\"]\nrun = [\"true\"]\n", false) + .expect_err("a misspelled event name is a hook that would never fire"); assert!( error .to_string() @@ -321,9 +399,10 @@ fn a_hook_naming_an_event_that_does_not_exist_is_refused_rather_than_installed() // The correctly spelled one loads, so the assertion above is about the // spelling rather than about the key. - let (_dir, spelled) = written(LOCAL_FILE, EVENT_HOOK); - let config = Config::discover(&spelled).expect("the configuration loads"); - assert!(io_cli::contract::hooks(&config, &config.plugins(), &spelled).is_some()); + let scope = support::user_scope_locked(EVENT_HOOK, false); + let spelled = scope.root().to_path_buf(); + let config = &scope.config; + assert!(io_cli::contract::hooks(config, &config.plugins(), &spelled).is_some()); } // --------------------------------------------------------------------------- @@ -377,8 +456,12 @@ const APPENDING_HOOK: &str = "[[hook]]\non = [\"finished\"]\nappend = \"audit.js /// per launch, for a hook that never fired. #[test] fn a_rootless_caller_builds_no_hooks_and_therefore_writes_no_append_file() { - let (_dir, root) = written(LOCAL_FILE, APPENDING_HOOK); - let config = Config::discover(&root).expect("the configuration loads"); + let scope = support::user_scope(APPENDING_HOOK); + // The workspace the configuration was discovered against, which is deliberately + // empty — so the `audit.jsonl` looked for below can only have been created by + // the `Hooks` this test is about. + let root = scope.root().to_path_buf(); + let config = &scope.config; // The positive control: this is what building a `Hooks` costs. assert!( @@ -386,7 +469,7 @@ fn a_rootless_caller_builds_no_hooks_and_therefore_writes_no_append_file() { "the fixture already had the file, so its appearance below proves nothing", ); assert!( - io_cli::contract::hooks(&config, &config.plugins(), &root).is_some(), + io_cli::contract::hooks(config, &config.plugins(), &root).is_some(), "a declared hook with a root is a Hooks", ); assert!( @@ -398,12 +481,12 @@ fn a_rootless_caller_builds_no_hooks_and_therefore_writes_no_append_file() { // And the same configuration with no root writes nothing, because it builds // nothing. assert!( - io_cli::contract::hooks(&config, &config.plugins(), std::path::Path::new("")).is_none(), + io_cli::contract::hooks(config, &config.plugins(), std::path::Path::new("")).is_none(), "a rootless caller was handed a Hooks, which created `audit.jsonl` in \ whatever directory `io` was launched from", ); assert!( - io_cli::contract::configured("", PathBuf::new(), &config, &config.plugins()) + io_cli::contract::configured("", PathBuf::new(), config, &config.plugins()) .tool_hooks .is_none(), "the road `server_notices` actually takes at startup: an empty root put a \ @@ -440,14 +523,13 @@ fn a_rootless_caller_builds_no_hooks_and_therefore_writes_no_append_file() { /// it fails by leaving the very file it is describing. #[test] fn server_notices_leaves_no_append_file_in_the_directory_io_was_launched_from() { - let (_dir, root) = written( - LOCAL_FILE, - &format!( - "{APPENDING_HOOK}\n[[mcp]]\nid = \"docs\"\ntransport = \"stdio\"\n\ - command = \"mcp-docs\"\n" - ), - ); - let config = Config::discover(&root).expect("the configuration loads"); + // User-scoped on both counts: a `[[hook]]` and an `[[mcp]]` are each refused + // from every file a workspace can supply since io-harness 0.74.0. + let scope = support::user_scope(&format!( + "{APPENDING_HOOK}\n[[mcp]]\nid = \"docs\"\ntransport = \"stdio\"\n\ + command = \"mcp-docs\"\n" + )); + let config = &scope.config; assert!(!config.hooks().is_empty(), "the fixture declares a hook"); // Exactly what `Hooks::new` would `create` for this hook under an empty root. @@ -455,7 +537,7 @@ fn server_notices_leaves_no_append_file_in_the_directory_io_was_launched_from() let before = stray.exists(); let _ = io_cli::contract::server_notices( - &config, + config, &config.plugins(), &io_cli::contract::Capabilities::default(), ); diff --git a/tests/import.rs b/tests/import.rs index 6e9d21a..d371741 100644 --- a/tests/import.rs +++ b/tests/import.rs @@ -9,12 +9,22 @@ //! //! No clocks and no sleeps. Every assertion is about bytes on disk or a value //! parsed out of them. +//! +//! **One thing here does have to touch the environment.** `[[mcp]]` is a +//! user-scope section since io-harness 0.74.0, and the user scope is +//! `$IO_CONFIG` — so a test that lets `/import` write a server without pointing +//! that variable at a file of its own would append `[[mcp]]` entries to the real +//! `~/.io-cli/io.toml` of whoever is running the suite. [`UserConfig`] is that +//! fixture, and it holds the one environment lock this binary has for exactly as +//! long as the variable is set. use std::path::{Path, PathBuf}; use io_cli::import::{apply, detect, files, plan, Destination, Kind, Source}; use io_harness::config::Scope; +mod support; + /// The value that must never reach a file io-cli writes. const SECRET: &str = "sk-live-this-must-never-be-copied"; @@ -32,6 +42,79 @@ const UNSET: &str = "IO_CLI_IMPORT_TEST_TOKEN"; /// server header wants an env reference that resolves. const ALWAYS_SET: &str = "PATH"; +/// The user-scope configuration file an imported `[[mcp]]` entry lands in, with +/// `IO_CONFIG` pointed at it. +/// +/// **io-harness 0.74.0 is why every MCP test in this file holds one.** `[[mcp]]` +/// may no longer be declared by any configuration file under the workspace root, +/// so the destination `/import` writes to is the user scope — and the user scope +/// is `$IO_CONFIG`, which is **process-global**. A fixture that set it and handed +/// back would leak its file into every `Config::discover` running beside it, which +/// is a failure that only appears on CI and only sometimes. +/// +/// So the one environment lock this binary has is held for exactly as long as the +/// variable is set, and released by the same drop that unsets it. Every bare +/// `Config::discover` in this file has to take that same lock across the read — +/// `instructions_land_somewhere_config_discover_reports` is the one that does. +/// +/// Without `IO_CONFIG` this would be the operator's own `~/.io-cli/io.toml`: the +/// test suite would append `[[mcp]]` entries to the machine it runs on. +struct UserConfig { + /// The one environment lock this binary has, held for exactly as long as + /// `IO_CONFIG` is set. `Drop` below runs before either field is dropped, so + /// the variable is unset while the lock is still held. + _lock: std::sync::MutexGuard<'static, ()>, + home: tempfile::TempDir, +} + +impl UserConfig { + fn new() -> Self { + let lock = support::env_lock(); + let home = tempfile::tempdir().expect("a directory for the user-scope file"); + std::env::set_var("IO_CONFIG", home.path().join("io.toml")); + Self { _lock: lock, home } + } + + /// The file itself, which is what `configure::scope_path` resolves for + /// [`Scope::User`] while this lives. + fn path(&self) -> PathBuf { + self.home.path().join("io.toml") + } + + /// The directory holding it, for the assertions that scan bytes rather than + /// values. The imported servers no longer land under the workspace, so a + /// credential assertion that swept only the workspace would now pass by + /// looking in the wrong place. + fn dir(&self) -> &Path { + self.home.path() + } +} + +impl Drop for UserConfig { + fn drop(&mut self) { + std::env::remove_var("IO_CONFIG"); + } +} + +/// The `[[mcp]]` entries **io-harness itself** reads back for a session at `root`. +/// +/// **The oracle is `Config::discover`, not a parse of the bytes io-cli wrote**, +/// and since io-harness 0.74.0 those are two different questions. A `[[mcp]]` +/// entry appended to `root/io.toml` produces a file that parses perfectly and a +/// `Config::discover` that refuses the whole of it — which `configure::write` then +/// rolls back, leaving nothing at all. A helper that read the file would have been +/// asserting "io-cli wrote something" while the product wrote nothing that +/// survived the round trip. This asks the only question that matters: does the +/// harness accept what was written. +/// +/// Requires a live [`UserConfig`], because that is where the entries are. +fn read_back(root: &Path) -> Vec { + io_harness::config::Config::discover(root) + .expect("io-harness reads back what the import wrote") + .mcp_servers() + .to_vec() +} + fn put(path: &Path, text: &str) { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent).expect("a fixture directory"); @@ -82,13 +165,16 @@ fn holds(dir: &Path, needle: &str) -> Option { }) } -/// The `[[mcp]]` entries of a written configuration, as io-harness's own type. +/// The `[[mcp]]` entries **on disk at `path`**, as io-harness's own type. +/// +/// The byte-level companion to [`read_back`], and it answers the one question +/// discovery cannot: what a rolled-back write left behind. A refused write is a +/// `Config::discover` that fails, so the assertion "the rollback left no entry" has +/// to look at the file rather than at a configuration that does not exist. /// -/// **Deserialised, never string-matched.** The question this file has to answer -/// is "will io-harness read back what io-cli wrote", and a substring assertion -/// answers a different one — it would pass on a body that spelled every key right -/// and left off `transport`, which is the one field `#[serde(flatten)]` makes -/// mandatory and easy to forget. +/// **Deserialised, never string-matched.** A substring assertion would pass on a +/// body that spelled every key right and left off `transport`, which is the one +/// field `#[serde(flatten)]` makes mandatory and easy to forget. fn written_servers(path: &Path) -> Vec { let text = std::fs::read_to_string(path).expect("the configuration was written"); let document: toml::Value = toml::from_str(&text).expect("it parses"); @@ -256,6 +342,7 @@ fn a_plan_that_is_declined_writes_nothing_and_leaves_the_home_byte_identical() { #[test] fn a_claude_mcp_entry_round_trips_into_an_mcp_server() { + let user = UserConfig::new(); let home_root = tempfile::tempdir().expect("a fake home"); let workspace = tempfile::tempdir().expect("a workspace"); let home = tempfile::tempdir().expect("io's home"); @@ -286,12 +373,31 @@ fn a_claude_mcp_entry_round_trips_into_an_mcp_server() { .filter(|item| item.kind == Kind::Mcp) .collect(); assert_eq!(items.len(), 1, "{items:?}"); + // **The scope argument was `Project` and the item is `User` anyway.** That is + // the fix rather than a detail of it: io-harness 0.74.0 allows `[[mcp]]` in the + // user scope alone, so threading a caller's scope into these items produced a + // plan whose every entry `configure::write` refused and rolled back. + assert_eq!( + items[0].to, + Destination::Config(Scope::User), + "an `[[mcp]]` entry goes to the one scope io-harness accepts one in", + ); let report = apply(&items, workspace.path()); assert!(report.refused.is_empty(), "{report:?}"); assert_eq!(report.written.len(), 1, "{report:?}"); + assert_eq!( + report.written[0].0, + user.path(), + "the report names the file that was actually written: {report:?}", + ); + assert!( + !workspace.path().join("io.toml").exists(), + "an `io.toml` in the workspace is the file io-harness refuses this section in", + ); - let servers = written_servers(&workspace.path().join("io.toml")); + // Asked of io-harness, not of the bytes: see `read_back`. + let servers = read_back(workspace.path()); assert_eq!(servers.len(), 1); assert_eq!(servers[0].id, "semlith"); let (command, args, env) = stdio(&servers[0]); @@ -306,6 +412,7 @@ fn a_claude_mcp_entry_round_trips_into_an_mcp_server() { #[test] fn a_codex_mcp_table_round_trips_into_an_mcp_server() { + let _user = UserConfig::new(); let home_root = tempfile::tempdir().expect("a fake home"); let workspace = tempfile::tempdir().expect("a workspace"); let home = tempfile::tempdir().expect("io's home"); @@ -343,11 +450,12 @@ fn a_codex_mcp_table_round_trips_into_an_mcp_server() { .filter(|item| item.kind == Kind::Mcp) .collect(); assert_eq!(items.len(), 1); + assert_eq!(items[0].to, Destination::Config(Scope::User)); let report = apply(&items, workspace.path()); assert!(report.refused.is_empty(), "{report:?}"); - let servers = written_servers(&workspace.path().join("io.toml")); + let servers = read_back(workspace.path()); assert_eq!(servers.len(), 1); assert_eq!(servers[0].id, "semlith"); let (command, args, env) = stdio(&servers[0]); @@ -374,6 +482,7 @@ fn a_codex_mcp_table_round_trips_into_an_mcp_server() { /// or drop `edit::spell` from the paths `codex_config` and `env_names` build. #[test] fn a_codex_server_whose_name_must_be_quoted_still_arrives() { + let user = UserConfig::new(); let home_root = tempfile::tempdir().expect("a fake home"); let workspace = tempfile::tempdir().expect("a workspace"); let home = tempfile::tempdir().expect("io's home"); @@ -404,7 +513,7 @@ fn a_codex_server_whose_name_must_be_quoted_still_arrives() { let report = apply(&items, workspace.path()); assert!(report.refused.is_empty(), "{report:?}"); - let servers = written_servers(&workspace.path().join("io.toml")); + let servers = read_back(workspace.path()); assert_eq!(servers.len(), 1); assert_eq!( servers[0].id, "docs.v2", @@ -422,6 +531,14 @@ fn a_codex_server_whose_name_must_be_quoted_still_arrives() { holds(workspace.path(), SECRET).is_none(), "the secret was copied", ); + // **And swept where the entry actually landed.** The workspace sweep above no + // longer covers the written file at all: `[[mcp]]` goes to the user scope now, + // so a check that looked only at the workspace would pass by looking somewhere + // empty. + assert!( + holds(user.dir(), SECRET).is_none(), + "the secret was copied into the user-scope configuration", + ); } // --------------------------------------------------------------------------- @@ -430,6 +547,7 @@ fn a_codex_server_whose_name_must_be_quoted_still_arrives() { #[test] fn a_secret_in_a_servers_environment_reaches_no_file_io_cli_writes() { + let user = UserConfig::new(); let home_root = tempfile::tempdir().expect("a fake home"); let workspace = tempfile::tempdir().expect("a workspace"); let home = tempfile::tempdir().expect("io's home"); @@ -489,6 +607,12 @@ fn a_secret_in_a_servers_environment_reaches_no_file_io_cli_writes() { None, "the secret reached io's home", ); + assert_eq!( + holds(user.dir(), SECRET), + None, + "the secret reached the user-scope configuration, which is where an \ + imported `[[mcp]]` entry now goes", + ); // **And the unresolvable reference is refused at import time rather than at // the operator's next session.** io-harness treats an unset `${env:…}` as a @@ -497,13 +621,28 @@ fn a_secret_in_a_servers_environment_reaches_no_file_io_cli_writes() { // secret has not been exported yet costs a sentence here instead of a dead // session later. Guarded on the variable genuinely being unset, because that // is the premise rather than the claim. + // + // **The count on its own asserted nothing, and this is what it cost.** While + // `/import` was writing `[[mcp]]` into `io.toml`, io-harness 0.74.0 refused + // both entries for the *section* — before substitution was ever evaluated — so + // `refused.len() == 2` was satisfied by a failure that had nothing to do with + // this test's subject. Two refusals is the right count and it is not evidence; + // the sentence is. Each one has to name the variable and say it is not set, or + // this test is passing on the wrong error again. if std::env::var_os(UNSET).is_none() { assert!( report.written.is_empty(), "an unresolvable `${{env:}}` must not be left in the file: {report:?}", ); assert_eq!(report.refused.len(), 2, "{report:?}"); - let at = workspace.path().join("io.toml"); + for line in &report.refused { + assert!( + line.contains(UNSET) && line.contains("is not set"), + "the refusal has to be about the unresolvable reference, not about the file the \ + entry was written into: {line}", + ); + } + let at = user.path(); assert!( !at.exists() || written_servers(&at).is_empty(), "the rollback left a `[[mcp]]` entry behind", @@ -584,8 +723,18 @@ fn instructions_land_somewhere_config_discover_reports() { // The oracle is io-harness's own discovery, not the file being on disk: // `AGENTS.md` is `DEFAULT_INSTRUCTIONS`, so a project with no configuration // at all reads it — and this asserts that it did. - let config = io_harness::config::Config::discover(workspace.path()) - .expect("the workspace configuration is readable"); + // + // **Under the environment lock, and it is not this test's own `IO_CONFIG` that + // makes it necessary.** `Config::discover` reads that variable, the MCP tests + // beside this one set it for as long as their fixture lives, and it is + // process-global — so a discovery here that ran during one of their windows + // would load *their* user-scope file. Holding the one lock this binary has + // across the read is what makes the two windows exclusive. + let config = { + let _guard = support::env_lock(); + io_harness::config::Config::discover(workspace.path()) + } + .expect("the workspace configuration is readable"); let read = config.instructions().join("\n"); assert!( read.contains("Always run the linter before committing."), @@ -965,6 +1114,7 @@ fn sixty_three_present_and_three_incoming_is_reported_and_nothing_is_written() { #[test] fn an_allowlist_is_reported_and_no_policy_is_written() { + let user = UserConfig::new(); let home_root = tempfile::tempdir().expect("a fake home"); let workspace = tempfile::tempdir().expect("a workspace"); let home = tempfile::tempdir().expect("io's home"); @@ -1018,7 +1168,11 @@ fn an_allowlist_is_reported_and_no_policy_is_written() { // Asserted on the PARSED result, not on a substring: a `[policy]` written as // a dotted key would be invisible to a `contains("[policy]")`. - let at = workspace.path().join("io.toml"); + // + // The file is the user scope's, because that is where the `[[mcp]]` entry that + // creates it goes. A `[policy]` check pointed at `workspace/io.toml` would now + // be asserting the absence of a table in a file that does not exist. + let at = user.path(); let text = std::fs::read_to_string(&at).expect("the MCP entry made a file"); let document: toml::Value = toml::from_str(&text).expect("it parses"); assert!( @@ -1026,8 +1180,8 @@ fn an_allowlist_is_reported_and_no_policy_is_written() { "an allowlist was translated into a policy: {text}", ); assert_eq!( - written_servers(&at).len(), + read_back(workspace.path()).len(), 1, - "and the rest of the import still landed", + "and the rest of the import still landed, in a form io-harness reads back", ); } diff --git a/tests/install.rs b/tests/install.rs index 1c59d69..c4f0f2e 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -19,15 +19,21 @@ //! directory empty, and `f1_the_contract_a_turn_is_built_from_names_all_five` //! would then be describing a state the product never reaches. +mod support; + use std::path::{Path, PathBuf}; -use std::sync::{Mutex, MutexGuard, OnceLock}; +use std::sync::MutexGuard; /// `HOME` and `USERPROFILE` are process-wide and every test here rewrites them. +/// +/// **Delegated to [`support::env_lock`] rather than owning a `Mutex` of its own.** +/// Two different mutexes in one binary exclude nothing from each other, so a file +/// that kept a private lock would let an `IO_CONFIG` fixture and a `HOME` fixture +/// run at the same time and each see the other's environment. The name and the +/// signature stay, because every call site here reads correctly either way; only +/// the lock behind them changes. fn env_lock() -> MutexGuard<'static, ()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) + support::env_lock() } /// A home of this test's own, put back when it drops. diff --git a/tests/live.rs b/tests/live.rs index 85c508a..cb01c37 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -2915,14 +2915,19 @@ async fn live_f2_a_parked_question_is_answered_and_the_run_carries_on() { ); } -/// The `[app.io-cli.gates]` section, built in memory rather than through a file. +/// The `[app.io-cli.gates]` section, built through the user scope. /// -/// `Config::from_toml` is public and takes the whole document, so the live arms -/// need no `IO_CONFIG` and therefore no environment lock — which matters here -/// because these tests run in one process and an environment variable is -/// process-wide. +/// **`Config::from_toml` stopped being usable here at io-harness 0.74.0**, and the +/// live suite is where that was found — every offline binary had already been +/// migrated, and this file was not among them because `cargo test` never runs it. +/// `from_toml` hard-codes `Scope::Project`, where a `[[provider]]` is now refused +/// outright, and a rubric judged by a second model has to name one. +/// +/// So it goes through the user scope like every other fixture in this repository, +/// which does mean an environment variable and therefore a lock — taken and +/// released inside `support::user_scope`, which is why no caller here holds one. fn gated(section: &str) -> Config { - Config::from_toml(section).expect("the gates section parses") + support::user_scope(section).config.clone() } /// **0.24.0 F2 — a command criterion that passes makes the run `Success`.** @@ -3638,6 +3643,31 @@ async fn live_f5_an_unreachable_primary_falls_through_to_the_provider_underneath let mut session = Session::open(&store, root).expect("a session"); let policy = workspace_policy(); + // **This test needed io-harness 0.74.0's lift to keep being about its own + // subject, and the live suite is what found that.** The floor refuses a + // loopback endpoint before any connection is attempted, whatever the policy + // says, so the primary failed with `Error::Refused` — which is **not** + // retryable, so the chain stopped instead of falling through. That is correct + // behaviour and worth stating: a boundary refusal that quietly tried the next + // vendor would be the fall-through working around the thing that said no. + // + // There is no way to route around it either. The floor also refuses a name it + // cannot resolve — "a name that will not resolve is not checkable" — so an + // `.invalid` host is a `Refused` too, not a transport failure. To reach a + // *transport* failure at all the address has to be one the floor permits, and + // the only fast, deterministic one is a loopback port with nothing behind it. + // + // So the variable is set here, which is exactly the choice the harness means an + // operator to make deliberately. Set and never unset: these arms share one + // process, and taking it away mid-run would be taking it from whichever + // sibling was mid-request. It changes nothing for them — every other arm talks + // to a public endpoint, which the floor never touched. + // + // `src/` still sets it nowhere, which `f11_io_cli_never_sets_the_local_address_variable` + // asserts: a test choosing it for itself is the operator's choice, and a + // product choosing it for the operator is not. + std::env::set_var("IO_HARNESS_ALLOW_LOCAL_ADDRESSES", "1"); + // A base URL on the loopback with nothing behind it. The failure is a refused // connection, which is `Transport` — retryable, and therefore worth another // vendor by io-harness's own predicate. diff --git a/tests/manage.rs b/tests/manage.rs index 55e5f8c..997a51b 100644 --- a/tests/manage.rs +++ b/tests/manage.rs @@ -14,6 +14,8 @@ use io_cli::manage::{self, ConfigVerb, McpVerb, PluginVerb, Request}; use io_harness::config::Scope; use io_harness::McpTransport; +mod support; + /// The token slice a shell hands `io`, spelled the way a test can read. fn argv(words: &[&str]) -> Vec { words.iter().map(|word| word.to_string()).collect() @@ -23,9 +25,26 @@ fn argv(words: &[&str]) -> Vec { /// once would each see the other's file. The scope test sets it; the `plugin /// remove` tests below resolve a real configuration and would see it set, so they /// take the same lock. Every other test here parses rather than plans. +/// +/// Delegated to [`support::env_lock`] rather than declared here: two different +/// mutexes in one binary exclude nothing from each other, and the `support` +/// fixtures take that one. fn env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); - LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + support::env_lock() +} + +/// Read a written file the way io-harness would read the operator's own. +/// +/// **Never `Config::from_toml` for an `[[mcp]]` file.** `from_toml` parses at +/// `Scope::Project`, and io-harness 0.74.0 refuses MCP servers there — an MCP +/// server is a command, an argv and an environment this process spawns at run +/// start, and `io.toml` arrives with a `git clone`. `mcp add` writes the user +/// scope by default (`decided_scope`'s `unwrap_or(Scope::User)`), so the user +/// scope is where the round trip belongs. +/// +/// Takes [`env_lock`] for itself, so a caller must not already hold it. +fn loaded(text: &str) -> io_harness::Config { + support::user_scope(text).config.clone() } /// The server a request declares, for a test that is about the transport. @@ -386,7 +405,7 @@ fn f6_both_accepted_orderings_write_byte_identical_configuration() { // was asked for — a byte comparison of two identically wrong entries would // pass without this. let text = written(root.path(), &ours); - let config = io_harness::config::Config::from_toml(&text).expect("the entry loads"); + let config = loaded(&text); assert_eq!(config.mcp_servers().len(), 1); assert_eq!(config.mcp_servers()[0].id, "linear-server"); assert!(matches!( @@ -597,41 +616,66 @@ max_retries = 2 assert!(after.contains("max_retries = 2"), "a sibling went: {after}"); } +/// **F4.** A widening is reported at parse time, for **both** files that live in +/// the workspace — and the advice names neither of them. +/// +/// io-harness's `refuse_widening` runs before deserialization, so this is not a +/// rejected setting: it is a configuration file that no longer parses at all. +/// `configure::write` would roll it back and quote the harness; saying it here +/// means the file is never written. +/// +/// **Both halves of this test asserted the defect until 0.35.0, and it was green.** +/// The guard read `Scope::Project` alone, so `--scope local` parsed cleanly and +/// this test asserted that it should — while the refusal on the other arm sent the +/// operator to `--scope local` as the remedy. io-harness 0.74.0 refuses +/// `io.local.toml` for the same reason it refuses `io.toml`: it is not committed, +/// but it sits in the workspace root a run's own agent can write to, so one +/// `write_file` of an unremarkable name was an escalation. io-cli was shipping +/// advice that fails, pinned by an assertion that it should. #[test] -fn f4_a_widening_in_a_project_file_is_reported_rather_than_written() { - // io-harness's `refuse_widening` runs before deserialization, so this is not - // a rejected setting — it is a configuration file that no longer parses at - // all. `configure::write` would roll it back and quote the harness; saying it - // here means the file is never written. - let refusal = manage::parse(&argv(&[ - "config", - "set", - "sandbox.mode", - "full-access", - "--scope", - "project", - ])) - .expect_err("a committed file may not widen what a clone may do"); - assert!(refusal.contains("sandbox.mode"), "{refusal}"); - assert!(refusal.contains("full-access"), "{refusal}"); - assert!(refusal.contains("--scope local"), "{refusal}"); +fn f4_a_widening_in_either_workspace_file_is_reported_rather_than_written() { + for scope in ["project", "local"] { + let refusal = manage::parse(&argv(&[ + "config", + "set", + "sandbox.mode", + "full-access", + "--scope", + scope, + ])) + .unwrap_err(); + assert!(refusal.contains("sandbox.mode"), "{scope}: {refusal}"); + assert!(refusal.contains("full-access"), "{scope}: {refusal}"); + assert!( + refusal.contains("--scope user"), + "{scope}: the user scope is the only destination left, and a refusal that names a \ + file the harness also refuses is worse than one that names none: {refusal}" + ); + assert!( + !refusal.contains("--scope local"), + "{scope}: `io.local.toml` is refused too — this is the sentence the release exists to \ + correct: {refusal}" + ); + } - // The same value in a file that is not shared is the operator's own business. + // The user scope is the operator's own file and is not in a workspace, so the + // same value parses there. The positive control matters: without it this test + // passes against a guard that refuses every scope. let allowed = manage::parse(&argv(&[ "config", "set", "sandbox.mode", "full-access", "--scope", - "local", + "user", ])) - .expect("a local file may say it"); + .expect("the operator's own file may say it"); assert_eq!( allowed, Request::Config(ConfigVerb::Set { key: "sandbox.mode".to_string(), value: "\"full-access\"".to_string(), - scope: Some(Scope::Local), + scope: Some(Scope::User), }) ); } @@ -685,7 +729,11 @@ command = \"mcp-search\" .expect("a write"); assert_eq!(plan.scope, Scope::Project); let gone = io_cli::edit::apply(&text, &plan.edits).expect("the entry goes"); - let config = io_harness::config::Config::from_toml(&gone).expect("the file still loads"); + // Read at the user scope, which is not the scope this file lives at: the + // claim under test is that the removal took the right entry, and 0.74.0 would + // refuse this text as a project file for declaring `[[mcp]]` at all — which is + // a fact about the fixture's location, not about the edit. + let config = loaded(&gone); let left: Vec<&str> = config.mcp_servers().iter().map(|s| s.id.as_str()).collect(); assert_eq!(left, vec!["docs"]); @@ -755,6 +803,61 @@ fn f6_a_plugin_directory_with_no_manifest_is_refused_before_anything_is_written( assert!(after.contains("path = \"bundle\""), "{after}"); } +/// **Installing a bundle that is already declared writes nothing.** +/// +/// **A defect 0.35.0 created, caught before it shipped.** Since the adapter became +/// a copy of what it contributes rather than a pointer into the clone, a `git pull` +/// of a marketplace is no longer visible on its own — **installing again is the +/// update path**. And `pluginview::add` appends unconditionally while +/// `Edit::append` always splices at end of file, so every refresh added a *second* +/// `[[plugin]]` naming the same directory. io-harness drops it with "a plugin with +/// id `x` is already declared and switched on", so nothing broke and nothing was +/// said: the operator's `io.toml` simply grew an ignored entry on every update, +/// quietly, for as long as they kept the bundle current. +/// +/// The adapter itself is regenerated on disk by `marketplace::chosen`, before +/// `plan` decides anything, so a refresh has no edit to make at all. +/// +/// Sabotage: return the `add` edit unconditionally, which is the pre-0.35.0 body. +/// The second plan carries an edit and this fails on the count. +#[test] +fn installing_a_bundle_that_is_already_declared_plans_no_second_entry() { + let root = tempfile::tempdir().expect("a temporary directory"); + let bundle = root.path().join("bundle"); + std::fs::create_dir(&bundle).expect("a directory"); + std::fs::write( + bundle.join(io_cli::pluginview::MANIFEST), + "name = \"demo\"\n", + ) + .expect("a manifest"); + + let request = manage::parse(&argv(&["plugin", "add", "bundle"])).expect("it parses"); + + // The first install, against a configuration that declares nothing. + let first = manage::plan(root.path(), &request, &[]) + .expect("it is a bundle") + .expect("a write"); + assert_eq!( + first.edits.len(), + 1, + "the first install declares it exactly once", + ); + + // And the second, against a configuration that already names this directory — + // which is what `plan` is handed on every real update. + let declared = [("demo".to_string(), bundle.clone())]; + let second = manage::plan(root.path(), &request, &declared) + .expect("it is still a bundle") + .expect("a plan, because the operator still has to be told what a refresh moved"); + assert!( + second.edits.is_empty(), + "a refresh planned {} edit(s). Appending a second `[[plugin]]` for a directory already \ + declared grows the operator's file by an entry io-harness drops, on every update: {:?}", + second.edits.len(), + second.edits, + ); +} + // --- `plugin remove `: the path first, then the declared name ------------- // // `io plugin add ` installs a bundle a marketplace holds and then tells the diff --git a/tests/marketplace.rs b/tests/marketplace.rs index abcf73d..da73973 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -9,6 +9,17 @@ //! under `tests/` can reach without moving `HOME` out from under a suite running in //! parallel, and this crate has shipped untestable driver logic in three releases. //! +//! **The tests that install one do move `IO_CONFIG`, and they hold the one lock +//! while they do.** io-harness 0.74.0 drops a `[[plugin]]` naming a directory +//! outside the workspace from every scope but the operator's own, and a bundle in +//! a marketplace clone is outside every workspace by construction — so a fixture +//! for what `plugin add` writes is a user-scope fixture, and a user-scope fixture +//! is one environment variable for the whole process. `support::env_lock` and +//! `discovered` are how that is kept from reaching the test running beside it; +//! every discovery in this file goes through one or the other. `HOME` is still +//! never moved, which is a different variable and the one this file's fixtures are +//! built to avoid. +//! //! **Nothing here spawns `git` and nothing here reaches a network.** `tests/fetch.rs` //! owns the clone itself, both its endings and the staging directory; what is left //! for this file is everything that happens to a directory once it is on the disk, @@ -21,6 +32,29 @@ use io_cli::manage::{self, MarketVerb, PluginVerb, Request}; use io_cli::marketplace::{self, Market, Went}; use io_harness::config::Scope; +mod support; + +/// `Config::discover`, taken under the one environment lock this binary has. +/// +/// **Every discovery in this file goes through here, and io-harness 0.74.0 is +/// why.** A `[[plugin]]` naming a directory outside the workspace is dropped from +/// every scope but the user's, and a marketplace bundle is *always* outside the +/// workspace — so the tests that install one build a `support::user_scope` +/// fixture, which points `IO_CONFIG` at a file. That variable is process-global. +/// A bare discovery running in that window reads the other test's fixture as well +/// as its own, which is a failure that only ever shows up on a loaded machine — +/// CI — and never here. `tests/plugins.rs` states the same rule for the same +/// reason. +/// +/// The lock is held across the read and no longer: a `Config` is a snapshot of the +/// files it read. A test that takes the lock itself must **not** call this — +/// `std::sync::Mutex` is not reentrant and doing both deadlocks; it holds the +/// guard and calls `support::UserScope::reload` instead. +fn discovered(root: &Path) -> io_harness::Result { + let _guard = support::env_lock(); + io_harness::Config::discover(root) +} + /// The token slice a shell hands `io`, spelled the way a test can read. fn argv(words: &[&str]) -> Vec { words.iter().map(|word| word.to_string()).collect() @@ -549,18 +583,32 @@ fn f2_the_rows_carry_the_manifest_name_and_the_count() { /// anyway, because it holds no configuration and builds no `Edit`. #[test] fn f3_removing_a_marketplace_leaves_every_declaration_alone() { - let work = tempfile::tempdir().expect("a workspace"); + // **The lock is held across the whole test rather than around each read**, and + // `discovered` is therefore not called from here: the fixture below points + // `IO_CONFIG` at a file for as long as this test runs, and every discovery in + // it has to be one that reads that file. + let _guard = support::env_lock(); let store = tempfile::tempdir().expect("a marketplaces directory"); // Resolved, because a temporary directory on macOS is reached through `/var` // and canonicalises to `/private/var`: a fixture that declared the unresolved // path would be comparing two spellings of one directory. - let root = std::fs::canonicalize(work.path()).expect("the workspace resolves"); let store = std::fs::canonicalize(store.path()).expect("the store resolves"); + // **The user scope, because that is the only one this arrangement exists in.** + // io-harness 0.74.0 drops a `[[plugin]]` whose path resolves outside the + // workspace from every scope but the operator's own, and a bundle inside a + // marketplace clone is outside every workspace by construction. It is also the + // scope both doors write: `manage::plan` resolves an unstated `plugin add` to + // `Scope::User` and the `/plugin` pickers name it outright, so a fixture in + // `io.local.toml` was testing an arrangement the product never produces. + let mut scope = support::user_scope_locked("", false); + let root = scope.root().to_path_buf(); // Both bundles contribute something real, so that "it loaded" is a claim about // io-harness having accepted a bundle rather than about a manifest carrying a - // name. A skills directory is the cheapest contribution a local-scope bundle - // may make; it exists, because a bundle naming a directory that does not is a + // name. A skills directory is the cheapest contribution a bundle sitting + // *inside* the workspace may make — the copied one below is one of those, and + // 0.74.0 grades trust by where the manifest is rather than by which file named + // it. It exists on disk, because a bundle naming a directory that does not is a // different failure and would answer this test for the wrong reason. let clone = marketplace::at(&store, &named()); let inside = clone.join("plugins").join("rust"); @@ -575,7 +623,7 @@ fn f3_removing_a_marketplace_leaves_every_declaration_alone() { ); std::fs::create_dir_all(installed.join("skills")).expect("the skills directory"); - let file = root.join("io.local.toml"); + let file = scope.path(); // `{:?}` on the rendered path, which is a TOML basic string with its // backslashes escaped — the same reason `pluginview::quoted` exists, and the // reason a Windows checkout does not write a file that no longer parses. @@ -586,13 +634,13 @@ fn f3_removing_a_marketplace_leaves_every_declaration_alone() { ); std::fs::write(&file, &text).expect("the configuration"); - let config = io_harness::Config::discover(&root).expect("the configuration loads"); - let view = io_cli::pluginview::view(&config.plugins()); - // By id and never by count: `Config::discover` layers the operator's own user - // file over this one, so a developer whose `~/.io-cli` declares a bundle would - // fail a length assertion for a reason that has nothing to do with the - // criterion. `tests/plugins.rs` and `pluginview`'s own test read this the same - // way and for the same reason. + scope.reload().expect("the configuration loads"); + let view = io_cli::pluginview::view(&scope.config.plugins()); + // By id and never by count. The fixture stands in for the user scope while it + // runs, so these buckets hold this test's own bundles — but a length assertion + // read against a machine's real configuration fails for a reason that has + // nothing to do with the criterion, and `tests/plugins.rs` and `pluginview`'s + // own test read this the same way for that reason. for id in ["rust-review", "rust-review-copy"] { assert!( view.plugins.iter().any(|listed| listed.id == id), @@ -633,7 +681,8 @@ fn f3_removing_a_marketplace_leaves_every_declaration_alone() { ); let after = io_cli::pluginview::view( - &io_harness::Config::discover(&root) + &scope + .rediscover() .expect("the configuration still loads") .plugins(), ); @@ -1293,7 +1342,17 @@ fn holding(store: &Path) -> (Vec, PathBuf) { } /// The whole of what a door does for `plugin add `, **in the order it does -/// it**, with `work`'s local file standing in for the operator's configuration. +/// it**, writing into `file` — the operator's own user-scope configuration. +/// +/// **The scope is the user's, at every step, because that is the only scope this +/// install exists in.** `manage::plan` resolves an unstated `plugin add` to +/// `Scope::User`, the `/plugin` pickers name it outright, and io-harness 0.74.0 +/// drops a `[[plugin]]` resolving outside the workspace from every other one — so +/// a helper that disclosed at `Scope::Local` and wrote `io.local.toml` was +/// mirroring a door that does not exist. Both halves have to move together: the +/// scope handed to the disclosure is the scope the entry is about to be written +/// into, which is what makes a bundle carrying an `[[mcp]]` disclose and load +/// rather than disclosing and then dropping. /// /// Resolve the word, ask io-harness what the directory is, and write only if it /// answered — `Err` short-circuits with the file exactly as it was found, which is @@ -1307,19 +1366,19 @@ fn holding(store: &Path) -> (Vec, PathBuf) { /// `crate::home`, and moving `HOME` out from under a suite running in parallel is /// the thing this file's own header refuses to do. The structural gate at the /// bottom is what holds `plan` to this order. -fn install(work: &Path, markets: &[Market], word: &str) -> Result { - let file = work.join(io_harness::config::LOCAL_FILE); +fn install(file: &Path, work: &Path, markets: &[Market], word: &str) -> Result { let homes = Homes::under(work); let chosen = marketplace::chosen(&work.join(word), || markets.to_vec(), word, homes.at())?; if chosen.discloses() { // The read that decides, before the file is opened for writing. marketplace::adapted_disclosure( - Scope::Local, + Scope::User, chosen.dir(), (chosen.from() != chosen.dir()).then(|| chosen.from()), + chosen.copied(), )?; } - let before = std::fs::read_to_string(&file).unwrap_or_default(); + let before = std::fs::read_to_string(file).unwrap_or_default(); let text = io_cli::edit::apply( &before, &[io_cli::pluginview::add(&io_cli::pluginview::declared( @@ -1328,7 +1387,7 @@ fn install(work: &Path, markets: &[Market], word: &str) -> Result Result PathBuf { + let dir = clone.join("plugins").join("rust"); + write( + &dir, + &format!( + "{}/{}", + io_cli::adapt::CLAUDE_DIR, + io_cli::adapt::MANIFEST_FILE + ), + &format!("{{ \"name\": \"{name}\" }}"), + ); + write(&dir, "skills/one.md", "---\nname: one\n---\nthe first\n"); + write(&dir, "commands/go.md", "go\n"); + dir +} + +/// **Installing a bundle again is the update path, and it re-copies rather than +/// merges.** +/// +/// Since 0.35.0 an adapter *ships* the `skills/` and `commands/` a foreign bundle +/// contributes — io-harness 0.74.0 refuses a `skills` or `templates` pointing out +/// of the bundle in every scope, because every `*.md` under one reaches the model's +/// system prompt. So what io holds is a snapshot, and an operator who pulls the +/// clone has changed nothing this session can see until the install runs again. +/// +/// Three things have to hold and each fails alone: the second install rebuilds the +/// adapter rather than adding to it, so a skill the author **withdrew** cannot +/// survive in io's home; the second install claims nothing to take back, so +/// declining it cannot delete an adapter a `[[plugin]]` entry already names; and +/// the operator is told which directories moved, in the disclosure they are already +/// reading, rather than discovering it a week later. +/// +/// Sabotage: have `adapt::generate` write over the adapter instead of rebuilding +/// it. `two.md` still arrives and every other assertion passes; the withdrawn +/// `one.md` is the one that fails, which is the file that would have gone on being +/// read into the prompt. +/// +/// Second sabotage: keep `made: Some(into)` for an adapter that was already there. +/// `unmake` then deletes a live install when the operator answers "leave it" to an +/// update — the entry stays in their file and io-harness drops it on the next turn. +#[test] +fn a_second_install_re_copies_the_adapter_and_says_what_moved() { + let (_dir, root) = clone_dir(); + let homes = Homes::under(&root); + let clone = homes.marketplaces.join("zeroonething").join("ultraship"); + let bundle = foreign(&clone, "rust-review"); + + let markets = marketplace::markets(&homes.marketplaces); + let installed = |markets: &[Market]| { + marketplace::prepared( + markets, + "rust-review", + &homes.marketplaces, + &homes.staging, + &homes.adapters, + ) + }; + + let first = installed(&markets).expect("the foreign bundle adapts"); + assert_eq!( + first.copied, + ["skills", "templates"], + "the adapter reports what it shipped in the generator's own words", + ); + assert_eq!( + first.made.as_deref(), + Some(first.declare.as_path()), + "the first install created the adapter directory, so a decline takes it back", + ); + assert!( + first.declare.join("skills").join("one.md").is_file() + && first.declare.join("templates").join("go.md").is_file(), + "the adapter does not ship the directories it contributes", + ); + + // The author withdraws a skill and publishes another, and the operator pulls. + std::fs::remove_file(bundle.join("skills").join("one.md")).expect("the withdrawal"); + write( + &bundle, + "skills/two.md", + "---\nname: two\n---\nthe second\n", + ); + + let again = installed(&markets).expect("the bundle adapts again"); + assert_eq!( + again.declare, first.declare, + "the update wrote a second adapter beside the first, so the declared entry \ + still names the old one", + ); + assert!( + !first.declare.join("skills").join("one.md").exists(), + "a skill the author withdrew survived the update. An adapter is rebuilt and \ + never merged for exactly this reason: every `*.md` under a contributed \ + `skills/` is read into the model's system prompt on every turn", + ); + assert!( + first.declare.join("skills").join("two.md").is_file(), + "the skill the operator pulled did not reach the session", + ); + assert_eq!( + again.made, None, + "an update claims to have created the adapter, so declining one deletes a \ + bundle the operator installed some other day — out of a configuration file \ + this module never opened", + ); + + // And what the operator is told before they answer. + let said = marketplace::adapted_disclosure( + Scope::User, + &again.declare, + Some(&again.from), + &again.copied, + ) + .expect("io-harness reads the generated manifest") + .said(&io_cli::glyphs::UNICODE) + .join("\n"); + assert!( + said.contains("copied `skills` and `templates`"), + "the disclosure does not name the directories that moved: {said}", + ); + assert!( + said.contains(&again.from.display().to_string()) + && said.contains(&again.declare.display().to_string()), + "both ends are named or the sentence cannot be acted on — the clone an edit \ + would go into, and the directory io is actually reading: {said}", + ); + assert!( + said.contains("installed again"), + "the operator is told the copy is a snapshot but not what refreshes it, \ + which is the whole question they will have: {said}", + ); + + // The control: a bundle that copied nothing says none of this. A row about a + // copy that did not happen is a warning about nothing. + let native = root.join("native"); + manifest(&native, "name = \"native\"\n"); + let quiet = marketplace::adapted_disclosure(Scope::User, &native, Some(&native), &[]) + .expect("io-harness loads the fixture") + .said(&io_cli::glyphs::UNICODE) + .join("\n"); + assert!( + !quiet.contains("copied `"), + "a native bundle is declared where it sits and nothing was copied for it: \ + {quiet}", + ); +} + /// **A repository fetched for an index entry is not a marketplace of its own.** /// /// `marketplace::fetched` clones into `marketplaces/.entries//`, and diff --git a/tests/narrow.rs b/tests/narrow.rs index ebbd13f..258c92b 100644 --- a/tests/narrow.rs +++ b/tests/narrow.rs @@ -1808,6 +1808,88 @@ fn n2_an_effort_level_yields_before_the_standing_facts_at_eighty_columns() { } } +/// **0.35.0 — the probe word never costs a standing fact.** +/// +/// The containment word grew by a word this release: `workspace-write/ +/// macos-sandbox-exec` is thirty-four columns and `… verified` is forty-three, +/// and it lives in the footer's `allowed` group, which is fit **all or nothing**. +/// So the failure mode is not truncation — it is the posture and the containment +/// disappearing **together**, silently, at a width where they used to fit. +/// +/// The invariant is `n2`'s and it has no number in it, for the reason recorded +/// there: which width a field starts costing something depends on the posture +/// string, the glyph set and every counter on the row, so a test that picks one +/// number asserts arithmetic it will get wrong. **Rendering the probe word must +/// never remove a standing fact that was on the row without it.** +/// +/// Sabotage: put the probe word in the `allowed` group as its own span with its +/// own separator, rather than inside the containment string. It costs three more +/// columns and the group starts dropping at a width where it used to hold. +#[test] +fn the_probe_word_never_costs_a_standing_fact_at_any_width() { + use io_cli::status::Status; + + let fixture = |boundary: Option<&str>| { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.policy = Some("read-only".into()); + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + status.planning = true; + status.boundary = boundary.map(str::to_string); + status + }; + + let measured = "macos-sandbox-exec write-outside=refused dial-outside=refused"; + let standing = [ + "read-only", + "workspace-write/macos-sandbox-exec", + "planning", + ]; + + for theme in themes() { + let set = theme.glyphs.name; + let mut ever_held = false; + + for width in [80, 88, 96, 104, 120, 160] { + let bare = text_of(&fixture(None).footer(width, &theme)).join("\n"); + let probed = text_of(&fixture(Some(measured)).footer(width, &theme)).join("\n"); + + for row in probed.lines() { + assert!( + row.chars().count() <= width as usize, + "{set}: a footer row overflowed {width} columns: {row:?}", + ); + } + + for fact in standing { + if bare.contains(fact) { + ever_held = true; + assert!( + probed.contains(fact), + "{set}: at {width} columns, naming what the probe measured took \ + `{fact}` off the row. The `allowed` group is fit all or nothing, so \ + the posture and the containment go together and an operator loses \ + both without being told.\n without: {bare:?}\n with: {probed:?}", + ); + } + } + } + + assert!( + ever_held, + "{set}: no width held a standing fact even without the probe word, so nothing \ + above measured anything", + ); + + // And the word is drawn where there is room, so this gates a field that + // exists rather than one that was never added. + let wide = text_of(&fixture(Some(measured)).footer(200, &theme)).join("\n"); + assert!( + wide.contains("boundary verified"), + "{set}: the probe word is on no footer at any width: {wide:?}", + ); + } +} + /// **0.26.0 N2 — the effort level is drawn in both glyph sets and costs no glyph /// it does not have.** /// diff --git a/tests/palette.rs b/tests/palette.rs index 33f9310..f765550 100644 --- a/tests/palette.rs +++ b/tests/palette.rs @@ -67,14 +67,25 @@ fn written() -> tempfile::TempDir { dir } -/// What `[run] templates = ` parses to. `from_toml` rather than `discover`, -/// so nothing on this machine's disk outside the temporary directory is read. +/// What `[run] templates = ` parses to. +/// +/// **User-scoped, and io-harness 0.74.0 is why.** `run.templates` naming an +/// absolute directory is refused in every scope a workspace can supply — every +/// `*.md` under it is composed into the model's system prompt on each turn, so a +/// cloned `io.toml` could put this host's files into the model's context on the +/// first turn — and `Config::from_toml` *is* the project scope. The directory +/// these fixtures point at is a `tempfile` outside any workspace, which is only +/// nameable at all from the one file no workspace can reach. +/// +/// [`support::user_scope`] discovers against an empty workspace of its own, so +/// nothing on this machine's disk outside the two temporary directories is read. fn configured(dir: &std::path::Path) -> Config { - Config::from_toml(&format!( + support::user_scope(&format!( "[run]\ntemplates = {:?}\n", dir.display().to_string() )) - .expect("io-harness parses its own file") + .config + .clone() } /// Type `text` at the picker, one character at a time, exactly as an operator diff --git a/tests/plugins.rs b/tests/plugins.rs index 2a8a5fa..6761abc 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -11,11 +11,17 @@ //! be asserting that io-cli copied fields correctly, which is not the question: //! the question is whether the turn the operator gets carries the bundle. //! -//! **Nothing here touches the environment.** `Config::discover` reads a user-scope -//! file where one exists, so every fixture that must be exact about the *absence* -//! of something uses `Config::from_toml`, which has no scope but the text; the -//! rest declare their bundles in a temporary root of their own and run in -//! parallel. +//! **One test here touches the environment and the rest do not.** `Config::discover` +//! reads a user-scope file where one exists, so every fixture that must be exact +//! about the *absence* of something uses `Config::from_toml`, which has no scope but +//! the text; the rest declare their bundles in a temporary root of their own and run +//! in parallel. The exception is `f3_the_same_manifest_named_from_the_user_scope_ +//! contributes_all_of_it`, which since io-harness 0.74.0 is the only arrangement in +//! which a bundle may contribute a `[[hook]]`, an `[[mcp]]` or a `[[bin]]` at all — +//! it goes through `support::user_scope`, which takes the one environment lock this +//! binary has. + +mod support; use std::path::{Path, PathBuf}; @@ -52,6 +58,27 @@ fn root() -> (tempfile::TempDir, PathBuf) { (dir, root) } +/// `Config::discover`, taken under the one environment lock this binary has. +/// +/// **Every discovery in this file goes through here, and io-harness 0.74.0 is +/// why.** `f3_the_same_manifest_named_from_the_user_scope_contributes_all_of_it` is +/// the only arrangement left in which a bundle may contribute a `[[hook]]`, an +/// `[[mcp]]` or a `[[bin]]`, and building it means pointing `IO_CONFIG` at a file — +/// which is process-global and therefore shared with every test running beside it. +/// A discovery that happened during that window would load *that* fixture's bundle +/// as well as its own, and the counted assertions below would fail on a bundle they +/// never declared, on CI, intermittently. +/// +/// The lock is held across the read and no longer: a `Config` is a snapshot of the +/// files it read, so nothing after the call can be reached by another test's +/// environment. The `Result` is handed back rather than unwrapped so each call site +/// keeps the sentence it fails with, which is the part that says what the discovery +/// was for. +fn discovered(root: &Path) -> io_harness::Result { + let _guard = support::env_lock(); + Config::discover(root) +} + /// A bundle directory holding `manifest`, with the two directories a manifest can /// name already on disk. fn bundle(root: &Path, at: &str, manifest: &str) -> PathBuf { @@ -123,7 +150,7 @@ fn f1_a_declared_bundle_reaches_the_turn_with_every_name_namespaced() { bundle(&root, "bundles/rust-review", MINIMAL); declaring(&root, PROJECT_FILE, &["bundles/rust-review"]); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); // io-harness's own verdict first: this bundle loaded and nothing was dropped. let plugins = config.plugins(); @@ -245,7 +272,7 @@ fn f2_five_ways_a_bundle_breaks_each_cost_exactly_that_bundle() { // The call returns. That is the first half of the criterion and it is asserted // by there being a line after it. - let config = Config::discover(&root).expect("a broken bundle is not a broken configuration"); + let config = discovered(&root).expect("a broken bundle is not a broken configuration"); let plugins = config.plugins(); assert_eq!( @@ -328,7 +355,7 @@ fn f3_a_bundle_that_names_a_program_is_dropped_whole_from_the_committed_file() { bundle(&root, "runner", &executing(kind)); declaring(&root, PROJECT_FILE, &["runner"]); - let config = Config::discover(&root).expect("a refused bundle is not a refused file"); + let config = discovered(&root).expect("a refused bundle is not a refused file"); let plugins = config.plugins(); assert!( @@ -370,33 +397,49 @@ fn f3_a_bundle_that_names_a_program_is_dropped_whole_from_the_committed_file() { } } -/// **F3, the other half.** The same manifest declared in `io.local.toml` loads, -/// with all seven kinds. +/// **F3, the other half.** The same manifest, kept outside the workspace and named +/// from the user-scope file, contributes all seven kinds. /// /// Without this the rule above is indistinguishable from "a bundle may never /// contribute a hook, a server or an executable", which would make the feature /// useless rather than safe — and a fix that refused everywhere would pass every /// assertion in the test above. /// -/// Sabotage: apply `refuse_executing_contributions` in every scope rather than -/// under `if scope == Scope::Project`. Under it only this test fails, and the -/// product ships a bundle format whose two most useful contributions can never be -/// declared by anyone. +/// **io-harness 0.74.0 moved which arrangement this is.** Until then the pair was +/// `io.toml` against `io.local.toml`, and the local file was on the trusted side. +/// It is not any more: `io.local.toml` is a path in the workspace root and a run's +/// own agent writes paths in the workspace root, so one `write_file` of it declared +/// a bundle whose `plugin.toml` — also written by the agent, also inside the root — +/// carried an argv the next `Config::discover` would run. The rule now asks where +/// the *manifest* is rather than which file named it, so the trusted arrangement +/// takes both halves: a bundle outside the discovery root, declared from the one +/// file no workspace can reach. That is also what an installed bundle is. +/// +/// Sabotage: grade the manifest by the declaring file's scope again — a user-scope +/// `[[plugin]]` naming a directory inside the workspace then loads a manifest the +/// agent wrote. Under it this test still passes and `f3_a_bundle_that_names_a_ +/// program_is_dropped_whole_from_the_committed_file` still passes, because neither +/// is that arrangement; what fails is nothing, which is why the arrangement is +/// spelled out here rather than left to a scope name. #[test] -fn f3_the_same_manifest_declared_locally_contributes_all_of_it() { +fn f3_the_same_manifest_named_from_the_user_scope_contributes_all_of_it() { for kind in ["hook", "mcp", "bin"] { - let (_dir, root) = root(); - bundle(&root, "runner", &executing(kind)); - // The local file, which does not travel with a clone — and no `io.toml` at - // all, so nothing but the scope differs from the test above. - declaring(&root, LOCAL_FILE, &["runner"]); - - let config = Config::discover(&root).expect("the configuration loads"); + // Two roots: the bundle's, which is where the manifest lives, and the + // workspace the configuration is discovered against. They have to be + // different directories or the manifest is inside the workspace again and + // the refusal in the test above applies here too. + let (_bundles, bundles) = root(); + let dir = bundle(&bundles, "runner", &executing(kind)); + // A TOML literal string: the path is absolute, and an absolute Windows path + // is a run of backslashes a basic string would read as escapes. + let scope = support::user_scope(&format!("[[plugin]]\npath = '{}'\n", dir.display())); + let root = scope.root().to_path_buf(); + let config = &scope.config; let plugins = config.plugins(); assert!( plugins.dropped().is_empty(), - "{kind}: dropped from the local scope: {:?}", + "{kind}: dropped from the user scope: {:?}", plugins.dropped(), ); assert_eq!(plugins.names(), vec!["runner"]); @@ -407,7 +450,7 @@ fn f3_the_same_manifest_declared_locally_contributes_all_of_it() { plugin.contributions(), ); - let contract = io_cli::contract::configured("go", root.clone(), &config, &config.plugins()); + let contract = io_cli::contract::configured("go", root.clone(), config, &config.plugins()); assert!( contract.agents.get("runner__reviewer").is_some(), "{kind}: the bundle loaded and its agent did not reach the turn: {:?}", @@ -422,18 +465,19 @@ fn f3_the_same_manifest_declared_locally_contributes_all_of_it() { ), // **Not on the contract, and that is io-harness's decision.** A // `[[bin]]` is handed back resolved and placed nowhere: io-harness - // puts nothing on a `PATH`, so what the local scope has to prove + // puts nothing on a `PATH`, so what the trusted scope has to prove // here is that the entry survived the load with its own name and a // path inside the bundle, which is the pair a run needs to make it - // resolvable at all. + // resolvable at all. Resolved against the **bundle's** root and not + // the workspace, which is now a different directory. "bin" => assert_eq!( plugin.bin(), - vec![("review", root.join("runner/bin/review.mjs"))], + vec![("review", dir.join("bin/review.mjs"))], "the bundle's executable did not come back resolved against its \ own root", ), _ => assert!( - io_cli::contract::hooks(&config, &config.plugins(), &root).is_some(), + io_cli::contract::hooks(config, &config.plugins(), &root).is_some(), "the bundle's hook is declared and nothing will run it", ), } @@ -597,7 +641,7 @@ fn f11_a_bundle_is_switched_off_and_back_on_and_the_entry_survives() { ); std::fs::write(&file, &off).expect("the configuration"); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); let view = pluginview::view(&config.plugins()); let listed = view .plugins @@ -628,7 +672,7 @@ fn f11_a_bundle_is_switched_off_and_back_on_and_the_entry_survives() { assert!(on.contains("enabled = true"), "{on}"); assert!(on.contains("path = \"good\""), "{on}"); std::fs::write(&file, &on).expect("the configuration"); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); assert!( pluginview::view(&config.plugins()) .plugins @@ -676,7 +720,7 @@ fn one_of_each() -> (tempfile::TempDir, PathBuf, Config) { bundle(&root, "good", MINIMAL); empty_bundle(&root, "broken"); declaring(&root, LOCAL_FILE, &["good", "broken"]); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); (dir, root, config) } @@ -1265,7 +1309,7 @@ fn f11_an_added_bundle_loads_through_the_harness() { let text = io_cli::edit::apply("", &[pluginview::add(&written)]).expect("the edit applies"); std::fs::write(root.join(PROJECT_FILE), &text).expect("the project file"); - let config = Config::discover(&root).expect("the written file loads"); + let config = discovered(&root).expect("the written file loads"); let view = pluginview::view(&config.plugins()); // By id, for the reason `listed` below states: `Config::discover` layers the // user file of whoever is running the suite over this root, and a length or a @@ -1379,7 +1423,7 @@ fn f7_a_configuration_declaring_only_a_switched_off_bundle_is_not_empty() { bundle(&root, "bundles/rust-review", MINIMAL); declaring_off(&root, "bundles/rust-review"); - let config = Config::discover(&root).expect("a switched-off bundle is not a broken file"); + let config = discovered(&root).expect("a switched-off bundle is not a broken file"); let plugins = config.plugins(); // io-harness's three buckets, asserted where they are — so a change of @@ -1448,7 +1492,7 @@ fn f7_a_switched_off_bundle_draws_under_its_own_mark() { let (_dir, root) = root(); bundle(&root, "bundles/rust-review", MINIMAL); declaring_off(&root, "bundles/rust-review"); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); let view = pluginview::view(&config.plugins()); for glyphs in [&io_cli::glyphs::UNICODE, &io_cli::glyphs::ASCII] { @@ -1515,7 +1559,7 @@ fn f7_a_switched_off_bundle_reaches_no_turn_while_its_directories_still_read() { let (_dir, root) = root(); bundle(&root, "bundles/rust-review", MINIMAL); declaring_off(&root, "bundles/rust-review"); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); let contract = io_cli::contract::configured("review this", root.clone(), &config, &config.plugins()); @@ -1583,7 +1627,7 @@ fn one_adapted_one_native() -> (tempfile::TempDir, pluginview::View) { bundle(&root, "bundles/rust-review", MINIMAL); bundle(&root, ADAPTED_AT, GENERATED); declaring(&root, LOCAL_FILE, &["bundles/rust-review", ADAPTED_AT]); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); let mut view = pluginview::view(&config.plugins()); view.adapters = Some(adapters_of(&view)); (dir, view) @@ -1715,7 +1759,7 @@ fn an_adapted_bundle_that_is_switched_off_still_says_so_in_its_detail() { let (_dir, root) = root(); bundle(&root, ADAPTED_AT, GENERATED); declaring_off(&root, ADAPTED_AT); - let config = Config::discover(&root).expect("the configuration loads"); + let config = discovered(&root).expect("the configuration loads"); let mut view = pluginview::view(&config.plugins()); view.adapters = Some(adapters_of(&view)); @@ -1755,7 +1799,7 @@ fn n2_nothing_on_disk_moved_means_nothing_is_resolved_again() { let (_guard, root) = root(); bundle(&root, "bundles/rust", MINIMAL); declaring(&root, io_harness::config::LOCAL_FILE, &["bundles/rust"]); - let config = io_harness::Config::discover(&root).expect("the configuration"); + let config = discovered(&root).expect("the configuration"); let resolved = io_cli::resolved::Resolved::load(&config); assert_eq!( @@ -1782,7 +1826,7 @@ fn n3_an_edited_manifest_is_seen() { let (_guard, root) = root(); let dir = bundle(&root, "bundles/rust", MINIMAL); declaring(&root, io_harness::config::LOCAL_FILE, &["bundles/rust"]); - let config = io_harness::Config::discover(&root).expect("the configuration"); + let config = discovered(&root).expect("the configuration"); let resolved = io_cli::resolved::Resolved::load(&config); assert!(!resolved.stale(&config)); @@ -1811,7 +1855,7 @@ fn n3_a_bundle_added_or_removed_is_always_seen() { let (_guard, root) = root(); bundle(&root, "bundles/rust", MINIMAL); declaring(&root, io_harness::config::LOCAL_FILE, &["bundles/rust"]); - let config = io_harness::Config::discover(&root).expect("the configuration"); + let config = discovered(&root).expect("the configuration"); let resolved = io_cli::resolved::Resolved::load(&config); assert!(!resolved.stale(&config)); @@ -1825,7 +1869,7 @@ fn n3_a_bundle_added_or_removed_is_always_seen() { io_harness::config::LOCAL_FILE, &["bundles/rust", "bundles/docs"], ); - let wider = io_harness::Config::discover(&root).expect("the configuration"); + let wider = discovered(&root).expect("the configuration"); assert!( resolved.stale(&wider), "a second bundle was declared and the resolution did not notice — the \ @@ -1849,7 +1893,7 @@ fn n3_two_writes_in_one_second_of_the_same_length_are_the_documented_limit() { let (_guard, root) = root(); let dir = bundle(&root, "bundles/rust", MINIMAL); declaring(&root, io_harness::config::LOCAL_FILE, &["bundles/rust"]); - let config = io_harness::Config::discover(&root).expect("the configuration"); + let config = discovered(&root).expect("the configuration"); let manifest = dir.join(PLUGIN_FILE); let before = std::fs::metadata(&manifest) @@ -1915,7 +1959,7 @@ fn ids_carries_every_declared_bundle_including_the_ones_that_did_not_load() { ) .expect("the configuration"); - let config = Config::discover(&root).expect("the configuration"); + let config = discovered(&root).expect("the configuration"); let pairs = pluginview::ids(&pluginview::view(&config.plugins())); for (id, at) in [ diff --git a/tests/prices.rs b/tests/prices.rs index 1b25cc5..b9b3286 100644 --- a/tests/prices.rs +++ b/tests/prices.rs @@ -10,17 +10,19 @@ //! //! So the end of this file is deliberately not a unit test. It takes the edits //! the module produces, runs them through `io_cli::edit::apply` over a -//! hand-written `io.toml`, hands the result to `io_harness::Config::from_toml`, -//! and asks the resulting `PriceTable` what a call on that model costs. That -//! round trip is the property — every step before it is a step towards it, and a -//! module that got the spelling wrong is a module whose prices exist in a file -//! nothing can read. +//! hand-written `io.toml`, hands the result to io-harness's own loader through +//! [`configured`], and asks the resulting `PriceTable` what a call on that model +//! costs. That round trip is the property — every step before it is a step +//! towards it, and a module that got the spelling wrong is a module whose prices +//! exist in a file nothing can read. use io_cli::edit; use io_cli::prices::{self, Catalogue, DEFAULT_REFERENCE_URL}; use io_harness::pricing::{Price, PriceTable}; use io_harness::{Config, ModelInfo, PriceSource, ProviderSpec}; +mod support; + // --------------------------------------------------------------------------- // Fixtures // --------------------------------------------------------------------------- @@ -35,8 +37,14 @@ use io_harness::{Config, ModelInfo, PriceSource, ProviderSpec}; /// the harness widens it — which is the wrong test failing for the wrong reason. /// A `[[provider]]` table is also exactly what an operator writes, so the fixture /// and the field agree by construction. +/// +/// **Through [`support::user_scope`] and never `Config::from_toml`.** `from_toml` +/// hard-codes `Scope::Project`, and io-harness 0.74.0 refuses `[[provider]]` from +/// a workspace-resident file — a provider names the endpoint a credential is sent +/// to, and `io.toml` arrives with a `git clone`. Every body below declares one, so +/// every one of them has to be read at the one scope that may declare it. fn configured(body: &str) -> Config { - Config::from_toml(body).expect("the fixture is a configuration io-harness accepts") + support::user_scope(body).config.clone() } fn openrouter() -> Config { @@ -425,8 +433,7 @@ fn a_catalogue_the_operator_named_is_taken_whole_and_attributed_to_its_url() { // reach a `/cost` page. let written = edit::apply(FRESH, &catalogue.edits(prices::has_models_section(FRESH))) .expect("the edits produce a file that parses"); - let table = Config::from_toml(&written) - .expect("io-harness accepts it") + let table = configured(&written) .prices() .expect("the file carries a table"); assert_eq!( @@ -639,7 +646,7 @@ fn a_dotted_model_id_survives_the_write_and_prices_a_call() { // The property. io-harness reads the file it would really read, and the table // it builds answers to the id io-harness would really record on a call. - let reloaded = Config::from_toml(&written).expect("io-harness accepts the written file"); + let reloaded = configured(&written); let table = reloaded.prices().expect("the file carries a price table"); assert_eq!(table.as_of(), "2026-08-27", "the date did not move"); assert_eq!( @@ -736,7 +743,7 @@ fn a_first_fill_writes_every_model_into_a_file_that_has_no_prices_section() { "the fill wrote the table more than once:\n{written}", ); - let reloaded = Config::from_toml(&written).expect("io-harness accepts the written file"); + let reloaded = configured(&written); let table = reloaded.prices().expect("the file carries a price table"); assert_eq!(table.as_of(), "2026-08-27"); for (model, price) in &catalogue.rows { @@ -768,8 +775,7 @@ fn a_first_fill_writes_every_model_into_a_file_that_has_no_prices_section() { let written = edit::apply(FRESH, &many.edits(false)) .expect("four hundred models are one section, not four hundred"); assert_eq!(written.matches("[prices.models]").count(), 1); - let table = Config::from_toml(&written) - .expect("io-harness accepts it") + let table = configured(&written) .prices() .expect("the file carries a table"); assert!( @@ -885,8 +891,7 @@ fn a_refresh_writes_one_row_per_model_and_leaves_the_rest_alone() { "2026-08-27", ); let written = edit::apply(EXISTING, &free.edits(true)).expect("a free model is writable"); - let table = Config::from_toml(&written) - .expect("io-harness accepts it") + let table = configured(&written) .prices() .expect("the file carries a table"); assert_eq!( diff --git a/tests/providers.rs b/tests/providers.rs index e286a03..f22749e 100644 --- a/tests/providers.rs +++ b/tests/providers.rs @@ -1,11 +1,13 @@ //! F7, F8, F9 and F10 — the chain, the presets, the live verification, and //! changing a link that already exists. -use std::sync::{Mutex, MutexGuard}; +use std::sync::MutexGuard; use io_cli::providers::{self, At, Credential, Endpoint, Key}; use io_harness::config::{Config, Scope}; +mod support; + const THREE: &str = "\ [[provider]] kind = \"openrouter\" @@ -26,38 +28,53 @@ model = \"llama3.2\" /// `Config::discover` reads `IO_CONFIG` at call time, and the fixtures below set /// it; serialised so two tests cannot see each other's file. +/// +/// Delegated to [`support::env_lock`] rather than declared here: two different +/// mutexes in one binary exclude nothing from each other, and the fixtures below +/// now go through `support`. fn env_lock() -> MutexGuard<'static, ()> { - static LOCK: Mutex<()> = Mutex::new(()); - LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + support::env_lock() } /// A discovered configuration, from a real file. /// -/// `Config::from_toml` is not enough here: the credential column is read from -/// the file the origin names, because io-harness substitutes `${env:…}` while it -/// parses — so a config with no path behind it cannot show an operator which -/// variable they wrote. -struct Fixture { - _dir: tempfile::TempDir, - config: Config, -} - -fn config(toml: &str) -> Fixture { +/// `Config::from_toml` is not enough here for two reasons. The credential column +/// is read from the file the origin names, because io-harness substitutes +/// `${env:…}` while it parses — so a config with no path behind it cannot show an +/// operator which variable they wrote. And since io-harness 0.74.0 `from_toml` +/// parses at `Scope::Project`, which may not declare `[[provider]]` at all: a +/// provider names the endpoint this run's credential is sent to, and `io.toml` +/// arrives with a `git clone`. +/// +/// [`support::user_scope_locked`] is what closes the second: it keeps the file +/// *outside* the discovery root, so it is a `Scope::User` candidate once rather +/// than a `Scope::User` and `Scope::Project` candidate twice. +fn config(toml: &str) -> support::UserScope { let _guard = env_lock(); // The variable the fixture points at has to exist, or the load fails // outright — io-harness resolves an indirection at parse time and refuses an // unset one rather than carrying it through. std::env::set_var("IO_CLI_TEST_GROQ_KEY", "gsk-not-a-real-key"); - let dir = tempfile::tempdir().expect("a temporary directory"); - let path = dir.path().join("io.toml"); - std::fs::write(&path, toml).expect("the fixture is written"); + support::user_scope_locked(toml, false) +} - std::env::set_var("IO_CONFIG", &path); - let config = Config::discover(dir.path()).expect("the fixture parses"); - std::env::remove_var("IO_CONFIG"); +/// The configuration alone, for the round trips that only want to read it back. +fn loaded(text: &str) -> Config { + config(text).config.clone() +} - Fixture { _dir: dir, config } +/// The same read, handing back the refusal instead of panicking on it. +/// +/// The tests that assert io-harness *refuses* something need the refusal to be +/// the one they are about — at project scope every one of these files is refused +/// for declaring a provider, which would make the assertion pass while proving +/// nothing. +fn try_loaded(text: &str) -> io_harness::Result { + let _guard = env_lock(); + std::env::set_var("IO_CLI_TEST_GROQ_KEY", "gsk-not-a-real-key"); + let scope = support::try_user_scope_locked(text, false)?; + Ok(scope.config.clone()) } #[test] @@ -226,10 +243,7 @@ fn f7_removing_the_head_promotes_the_next_link_and_removing_the_only_one_leaves_ ); let after = io_cli::edit::apply(ONLY, &[providers::remove(&at)]).unwrap(); assert_eq!(after, "", "the last entry left bytes behind"); - assert!(Config::from_toml(&after) - .expect("a file with nothing in it is a configuration") - .provider_spec() - .is_none()); + assert!(loaded(&after).provider_spec().is_none()); } #[test] @@ -248,7 +262,7 @@ fn f7_a_compatible_entry_naming_both_bases_or_neither_cannot_be_written() { preset = \"groq\"\nbase_url = \"https://example.com/v1\"\n"; for bad in [neither, both] { assert!( - Config::from_toml(bad).is_err(), + try_loaded(bad).is_err(), "io-harness accepted a `compatible` entry that names both bases or \ neither, so `Endpoint`'s split is no longer load-bearing: {bad}", ); @@ -265,7 +279,7 @@ fn f7_a_compatible_entry_naming_both_bases_or_neither_cannot_be_written() { let written = io_cli::edit::apply("", &[providers::add(endpoint, "a-model", None)]).unwrap(); assert!( - Config::from_toml(&written).is_ok(), + try_loaded(&written).is_ok(), "{endpoint:?} produced an entry io-harness refuses:\n{written}", ); } @@ -330,7 +344,7 @@ fn f7_a_pasted_credential_is_escaped_rather_than_breaking_the_file() { ) .unwrap(); - let config = Config::from_toml(&after).expect("the written file still parses"); + let config = loaded(&after); let Some(io_harness::ProviderSpec::OpenAi { model, api_key }) = config.provider_spec() else { panic!("the file named an openai provider"); }; @@ -720,8 +734,8 @@ fn f10_unsetting_a_credential_deletes_the_line_rather_than_emptying_it() { // And the entry is still a link, which is the half a deletion could break: // `unset` takes one line and leaves the `[[provider]]` around it standing. - let loaded = Config::from_toml(&after).expect("the entry still loads"); - let Some(io_harness::ProviderSpec::OpenAi { model, api_key }) = loaded.provider_spec() else { + let still = loaded(&after); + let Some(io_harness::ProviderSpec::OpenAi { model, api_key }) = still.provider_spec() else { panic!("the entry stopped being an openai provider"); }; assert_eq!(model, "gpt-4o"); @@ -749,8 +763,8 @@ fn f10_unsetting_a_credential_deletes_the_line_rather_than_emptying_it() { ) .unwrap(); assert!(sabotage.contains("api_key = \"\"")); - let sabotaged = - Config::from_toml(&sabotage).expect("an empty key still parses, which is the problem"); + // An empty key still parses, which is the problem. + let sabotaged = loaded(&sabotage); let Some(io_harness::ProviderSpec::OpenAi { api_key, .. }) = sabotaged.provider_spec() else { panic!("an openai provider"); }; diff --git a/tests/scope.rs b/tests/scope.rs new file mode 100644 index 0000000..6af8ddc --- /dev/null +++ b/tests/scope.rs @@ -0,0 +1,149 @@ +//! The configuration scope the rest of the suite's fixtures are built on. +//! +//! io-harness 0.74.0 refuses `[[provider]]`, `[[mcp]]`, `[[lsp]]` and — in +//! `io.local.toml` as well as `io.toml` — `[[hook]]` from any file that lives in +//! a workspace, along with ten widening values and an absolute `run.skills` or +//! `run.templates`. `Scope::User` is the only exemption, because `$IO_CONFIG` is +//! outside every workspace and a run that can write its own root cannot reach it. +//! +//! Every fixture in this suite that declares one of those sections therefore has +//! to be user-scoped, and `support::user_scope` is the one way it is done. This +//! file is that helper's own gate. It exists because the helper is load-bearing +//! for eighteen test binaries and a silent regression in it would not fail +//! honestly — it would make a hundred tests fail somewhere else, for a reason +//! none of them names. + +mod support; + +/// The section 0.74.0 refuses in a workspace file, in the smallest form that +/// still parses. +/// +/// `kind` and `model` and nothing else: `model` is required and `api_key` is not, +/// so the fixture names no credential at all. An earlier draft indirected through +/// `${env:}`, which io-harness resolves at parse time and refuses when unset — it +/// made every test in this file depend on a process-global variable it also had +/// to set, for a field the parser never wanted. +const DECLARES_A_PROVIDER: &str = "\ +[[provider]] +kind = \"openrouter\" +model = \"anthropic/claude-sonnet-4.5\" +"; + +#[test] +fn the_helper_produces_a_configuration_the_harness_accepts() { + let scope = support::user_scope(DECLARES_A_PROVIDER); + + assert!( + scope.config.provider_spec().is_some(), + "a user-scoped file may declare a provider, and the fixture has to prove it did — the \ + assertion is on the parsed spec rather than on the absence of an error, because a \ + configuration that dropped the section would also parse" + ); +} + +#[test] +fn the_user_file_is_not_inside_the_workspace_the_configuration_was_discovered_against() { + let scope = support::user_scope(DECLARES_A_PROVIDER); + + let file = scope.path(); + let root = scope.root(); + + assert!( + !file.starts_with(root), + "the user-scope file is at {} and the discovery root is {} — a file inside the root is a \ + candidate twice, once through IO_CONFIG as Scope::User and once as root/io.toml under \ + Scope::Project, and the project read is the one that refuses it", + file.display(), + root.display() + ); +} + +/// The trap, asserted from the other side. +/// +/// This is the shape every fixture in this repository had before 0.74.0, and the +/// reason a hundred of them went red at once. It is asserted rather than +/// described so that "pointing `IO_CONFIG` at it is enough" cannot quietly become +/// true again in a fixture somebody writes next year. +#[test] +fn the_same_file_inside_the_discovery_root_is_refused_however_io_config_points_at_it() { + let _guard = support::env_lock(); + + { + let dir = tempfile::tempdir().expect("a directory"); + let path = dir.path().join("io.toml"); + std::fs::write(&path, DECLARES_A_PROVIDER).expect("the fixture is written"); + + std::env::set_var("IO_CONFIG", &path); + let discovered = io_harness::Config::discover(dir.path()); + std::env::remove_var("IO_CONFIG"); + + let error = discovered.expect_err( + "a file that is both the IO_CONFIG target and the discovery root's own io.toml is read \ + as Scope::Project as well, and 0.74.0 refuses a provider there", + ); + let said = error.to_string(); + assert!( + said.contains("provider"), + "the refusal names the section it refused: {said}" + ); + } +} + +#[test] +fn a_project_file_beside_a_user_file_is_still_refused() { + // No outer `env_lock` here: `user_scope_with_project` takes it for itself and + // `std::sync::Mutex` is not reentrant, so holding it across the call is a + // deadlock rather than extra safety. + let error = support::user_scope_with_project("[run]\nmax_steps = 40\n", DECLARES_A_PROVIDER) + .expect_err("the project half declares a provider, which is refused"); + + let said = error.to_string(); + assert!( + said.contains("provider"), + "the operator needs to be told which section was refused, not that a file could not be \ + read: {said}" + ); +} + +#[test] +fn the_kept_variant_leaves_io_config_set_and_the_plain_one_does_not() { + // This one asserts over the process environment itself, so it has to hold the + // lock across both halves and use the forms that do not take it again. + let _guard = support::env_lock(); + + { + let _scope = support::user_scope_locked(DECLARES_A_PROVIDER, true); + assert!( + std::env::var_os("IO_CONFIG").is_some(), + "user_scope_kept exists for the product paths that re-discover for themselves" + ); + } + assert!( + std::env::var_os("IO_CONFIG").is_none(), + "and it puts the environment back when the fixture goes out of scope, or the next test in \ + this binary reads a file that has been deleted" + ); + + let _scope = support::user_scope_locked(DECLARES_A_PROVIDER, false); + assert!( + std::env::var_os("IO_CONFIG").is_none(), + "the plain variant unsets it before it returns" + ); +} + +/// A refusal has to be reachable as a value, not only as a panic. +#[test] +fn the_fallible_form_hands_back_the_refusal_rather_than_panicking() { + let _guard = support::env_lock(); + + // `sandbox.force_floor = false` is one of the ten widening values, and it is + // refused in a workspace file — but this fixture is user-scoped, so it is + // accepted here. The refusal below is a different thing entirely: a key no + // `[run]` table has. + let refused = support::try_user_scope_locked("[run]\nnot_a_key = 1\n", false); + + assert!( + refused.is_err(), + "an unknown key is a deserialization failure in any scope" + ); +} diff --git a/tests/servers.rs b/tests/servers.rs index 9cea603..1c38e6e 100644 --- a/tests/servers.rs +++ b/tests/servers.rs @@ -8,9 +8,30 @@ use io_cli::servers::{self, At, Observed, Reached, Server}; use io_harness::config::{Config, Scope}; use io_harness::{EventKind, McpServer, McpTransport}; +mod support; + +/// Read `text` as io-harness would read the operator's own file. +/// +/// **Never `Config::from_toml`.** That parses at `Scope::Project`, and io-harness +/// 0.74.0 refuses `[[mcp]]` from a workspace-resident file — an MCP server is a +/// command, an argv and an environment this process spawns at run start, and +/// `io.toml` arrives with a `git clone`. Every fixture and every written file in +/// this file declares one, and `At::of` already addresses them at `Scope::User`, +/// so reading them at project scope was the test disagreeing with itself. +fn loaded(text: &str) -> Config { + support::user_scope(text).config.clone() +} + +/// The same read, handing back the refusal instead of panicking on it. +fn try_loaded(text: &str) -> io_harness::Result { + let _guard = support::env_lock(); + let scope = support::try_user_scope_locked(text, false)?; + Ok(scope.config.clone()) +} + /// A configuration naming two servers, neither of them reached yet. fn configured() -> Config { - Config::from_toml( + loaded( "\ [[mcp]] id = \"docs\" @@ -23,7 +44,6 @@ transport = \"stdio\" command = \"mcp-search\" ", ) - .expect("the fixture parses") } /// The server reaching the run: an `Mcp` event with no `tool`. Since io-harness @@ -360,7 +380,7 @@ files = [\"AGENTS.md\"] assert!(after.contains(line), "line lost: {line:?}"); } // And the result is still the harness's schema. - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); assert_eq!(config.mcp_servers().len(), 2); } @@ -391,7 +411,7 @@ command = \"mcp-search\" after.contains("command = \"mcp-docs\""), "the first entry was edited instead of the second" ); - let config = Config::from_toml(&after).unwrap(); + let config = loaded(&after); assert_eq!(config.mcp_servers().len(), 2); } @@ -414,7 +434,7 @@ max_steps = 30 let at = At::of(Scope::User, TWO, "docs").expect("the fixture names `docs`"); let after = io_cli::edit::apply(TWO, &[servers::remove(&at)]).unwrap(); - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); let left: Vec<&str> = config.mcp_servers().iter().map(|s| s.id.as_str()).collect(); assert_eq!(left, vec!["search"]); assert!( @@ -456,13 +476,13 @@ args = [\"--verbose\"] fn f6_a_server_id_with_a_quote_in_it_is_escaped_rather_than_breaking_the_file() { let after = io_cli::edit::apply("", &[servers::add(&McpServer::stdio("we\"ird", "cmd"))]).unwrap(); - let config = Config::from_toml(&after).expect("the written file still parses"); + let config = loaded(&after); assert_eq!(config.mcp_servers()[0].id, "we\"ird"); } #[test] fn f6_the_transport_is_shown_as_the_thing_that_reaches_it() { - let config = Config::from_toml( + let config = loaded( "\ [[mcp]] id = \"local\" @@ -474,8 +494,7 @@ id = \"remote\" transport = \"http\" url = \"https://example.com/mcp\" ", - ) - .unwrap(); + ); let list = servers::servers(&config, &Observed::default()); let by = |id: &str| list.iter().find(|s| s.id == id).unwrap().transport.clone(); @@ -594,7 +613,7 @@ fn f6_add_edit_and_remove_change_one_entry_and_no_other_byte() { // And all three results are still the harness's schema, roster and // instructions intact. for text in [&added, &edited, &removed] { - Config::from_toml(text).expect("the written file loads"); + loaded(text); assert!(text.contains("name = \"scout\""), "the roster was dropped"); assert!( text.contains("files = [\"AGENTS.md\"]"), @@ -642,7 +661,7 @@ fn f6_a_server_with_args_and_env_round_trips_as_the_harness_s_own_type() { assert_eq!(edits.len(), 1, "a whole server is one write"); let after = io_cli::edit::apply("", &edits).unwrap(); - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); assert_eq!( config.mcp_servers(), [server].as_slice(), @@ -668,7 +687,7 @@ fn f6_an_http_server_round_trips_with_its_headers() { }; let after = io_cli::edit::apply("", &[servers::add(&server)]).unwrap(); - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); assert_eq!(config.mcp_servers(), [server].as_slice()); // The transport tag, not a `command` borrowed from the other arm. assert!( @@ -693,7 +712,7 @@ fn f6_an_entry_states_nothing_that_is_already_the_default() { ); } // And the round trip is still exact, which is what makes the absence safe. - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); assert_eq!(config.mcp_servers(), [plain].as_slice()); } @@ -739,7 +758,7 @@ fn f6_a_key_no_mcp_entry_carries_is_refused_rather_than_written() { ) .unwrap(); assert!( - Config::from_toml(&smuggled).is_ok(), + try_loaded(&smuggled).is_ok(), "io-harness rejected a misspelled `[[mcp]]` key, so the exemption this \ refusal exists for is gone and the const can go with it", ); @@ -794,10 +813,7 @@ command = "mcp-docs" let at = At::of(Scope::User, ONLY, "docs").expect("the fixture names `docs`"); let after = io_cli::edit::apply(ONLY, &[servers::remove(&at)]).unwrap(); assert_eq!(after, "", "the last server left bytes behind"); - assert!(Config::from_toml(&after) - .expect("a file with nothing in it is a configuration") - .mcp_servers() - .is_empty(),); + assert!(loaded(&after).mcp_servers().is_empty()); const THREE_AND_A_SECTION: &str = r#"[[mcp]] id = "a" @@ -821,7 +837,7 @@ max_steps = 30 assert_eq!(at.index(), 2); let after = io_cli::edit::apply(THREE_AND_A_SECTION, &[servers::remove(&at)]).unwrap(); - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); let left: Vec<&str> = config.mcp_servers().iter().map(|s| s.id.as_str()).collect(); assert_eq!(left, vec!["a", "b"]); assert!( diff --git a/tests/servers_enabled.rs b/tests/servers_enabled.rs index 6678d24..02a2646 100644 --- a/tests/servers_enabled.rs +++ b/tests/servers_enabled.rs @@ -26,6 +26,20 @@ use io_cli::servers::{self, At, Observed, Reached}; use io_harness::config::Scope; use io_harness::{Config, McpProbe, Policy}; +mod support; + +/// Read `text` as io-harness would read the operator's own file. +/// +/// **Never `Config::from_toml`.** That parses at `Scope::Project`, and io-harness +/// 0.74.0 refuses `[[mcp]]` from a workspace-resident file — an MCP server is a +/// command, an argv and an environment this process spawns at run start, and +/// `io.toml` arrives with a `git clone`. Every fixture in this file declares one, +/// and `At::of` below already addresses it at `Scope::User`, so reading it at any +/// other scope was the fixture disagreeing with itself. +fn loaded(text: &str) -> Config { + support::user_scope(text).config.clone() +} + /// One server switched off, one left alone, in the order the file names them. /// /// The bytes as well as the parsed configuration, because half of this file is @@ -51,7 +65,7 @@ command = \"mcp-search\" "; fn mixed() -> Config { - Config::from_toml(MIXED).expect("the fixture parses") + loaded(MIXED) } /// Where `MIXED` declares one of its two servers. @@ -61,7 +75,7 @@ fn at(id: &str) -> At { /// **The state reaches `Server`, and an absent key still means on.** /// -/// Addressed by id rather than by position: `Config::from_toml` reads this one +/// Addressed by id rather than by position: the fixture reads this one /// string, but the rule this file follows is `tests/marketplace.rs`'s — a length /// or an index is an assertion about somebody else's machine as much as about the /// criterion. @@ -198,7 +212,7 @@ fn f10_a_server_is_switched_off_without_being_taken_out_of_the_file() { // rewrite-the-whole-file implementation. assert!(after.contains("command = \"mcp-docs\"")); - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); assert_eq!( config.mcp_servers().len(), 2, @@ -237,7 +251,7 @@ fn f10_a_server_is_switched_off_without_being_taken_out_of_the_file() { fn f10_a_server_switched_off_in_the_file_is_switched_back_on() { let after = io_cli::edit::apply(MIXED, &[servers::switch(&at("docs"), true)]) .expect("the write parses back"); - let config = Config::from_toml(&after).expect("the written file loads"); + let config = loaded(&after); let docs = servers::servers(&config, &Observed::default()) .into_iter() .find(|server| server.id == "docs") diff --git a/tests/status.rs b/tests/status.rs index 87428e0..8093d8b 100644 --- a/tests/status.rs +++ b/tests/status.rs @@ -1692,6 +1692,172 @@ fn reported() -> App { app } +// --- 0.35.0: the boundary that was measured, not the one that was claimed ------ + +/// A probe label in io-harness's own spelling. +/// +/// `BoundaryProbe::trace_label` is +/// `"{backend} write-outside={arm} dial-outside={arm}"` where an arm is `refused`, +/// `landed` or `unmeasured`. Built here from the parts rather than pasted, so a +/// test that means "an arm landed" says so. +fn probe(write: &str, dial: &str) -> String { + format!("macos-sandbox-exec write-outside={write} dial-outside={dial}") +} + +/// **F1 — the probe outcome reaches all three surfaces, or it reaches none.** +/// +/// The status line, the footer and the `/status` page each draw the containment, +/// and 0.12.0 shipped a field that reached two of the three — the comment on +/// `Status::line` still names that as the failure mode. So this asserts over all +/// three in one test rather than three tests, because three tests are three +/// chances to write two of them. +/// +/// Sabotage: have one surface read `Status::containment` directly instead of +/// `Status::contained()`. That surface loses the word and this fails naming it. +#[test] +fn f1_a_measured_boundary_is_named_on_the_line_the_footer_and_the_status_page() { + // A bare `Status` for the two width-bound surfaces, in the shape + // `tests/narrow.rs` uses. `reported()` carries seventeen fields and the + // containment word sits late in the priority order, so it is dropped to fit + // long before a probe word could be — which would make this test fail for a + // reason that has nothing to do with what it asserts. + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + status.boundary = Some(probe("refused", "refused")); + + let line = status.line(200, &DARK).to_string(); + assert!( + line.contains("workspace-write/macos-sandbox-exec"), + "the status line names the backend that answered: {line}", + ); + assert!( + line.contains("boundary verified"), + "and, beside it, what the backend's own probe proved: {line}", + ); + + let footer: String = status + .footer(200, &DARK) + .iter() + .flat_map(|row| row.spans.iter()) + .map(|span| span.content.as_ref()) + .collect(); + assert!( + footer.contains("workspace-write/macos-sandbox-exec"), + "and so does the footer: {footer}", + ); + assert!( + footer.contains("boundary verified"), + "and the footer names the probe too, where there is room for it: {footer}", + ); + + // The page has no width budget, so it gets the full session fixture. + let mut app = reported(); + app.status.boundary = Some(probe("refused", "refused")); + let fixture = fixture(); + let page = committed(&app, &fixture, Some(&fixture.caps), &DARK, ROOMY).join("\n"); + assert!( + page.contains("workspace-write/macos-sandbox-exec verified"), + "and so does /status: {page}", + ); + // And the page carries the measurement itself, which the one-word summary + // cannot: it is the only surface with room to separate an arm that landed + // from an arm that was never attempted. + assert!( + page.contains("write-outside=refused dial-outside=refused"), + "/status prints the label io-harness wrote, not only the verdict: {page}", + ); +} + +/// **F2 — a run nobody probed is not a run that failed a probe.** +/// +/// io-harness writes the `sandbox_events` row only for a contained run, so a run +/// with no containment leaves `Status::boundary` `None`. Drawing `unverified` +/// there would be a claim about a measurement nobody took — the same mistake as +/// rendering a refused host in a way that reads like a visit. +#[test] +fn f2_a_run_that_was_never_contained_draws_no_probe_word_at_all() { + let mut status = Status::new("anthropic/claude-sonnet-4.5"); + status.containment = Some("workspace-write/macos-sandbox-exec".into()); + status.boundary = None; + + let line = status.line(200, &DARK).to_string(); + assert!( + line.contains("workspace-write/macos-sandbox-exec"), + "the backend is still named: {line}", + ); + for word in ["verified", "unverified"] { + assert!( + !line.contains(word), + "no probe was taken, so the line may not say `{word}`: {line}", + ); + } +} + +/// **F3 — `verified` means both arms were attempted and both were refused.** +/// +/// Three arm states and only one combination earns the word. An arm that +/// `landed` is a boundary that did not hold; an arm that is `unmeasured` was not +/// attempted at all. Neither is a proof, and the line says so with the same word +/// because the line cannot afford to distinguish them — `/status` does that. +/// +/// **`contradicted` is deliberately absent, and this is where that is recorded.** +/// `BoundaryProbe::contradicts_claim` is the harness's own notion of a backend +/// that did not do what it claimed, and it needs `claimed_confinement` and +/// `claimed_egress_denial` — neither of which `trace_label` carries. So io-cli +/// cannot tell "this backend claimed confinement and a write landed anyway" from +/// "this backend never claimed that arm", and drawing an alarm on the second +/// would be a false one on the field this release exists to make trustworthy. +/// +/// Sabotage: make `boundary_word` return `verified` unless an arm `landed`. +/// The `unmeasured` rows below fail and nothing else does. +#[test] +fn f3_only_two_refusals_are_a_verified_boundary() { + use io_cli::status::boundary_word; + + assert_eq!(boundary_word(&probe("refused", "refused")), "verified"); + + for (write, dial, why) in [ + ("landed", "refused", "a write outside the roots landed"), + ( + "refused", + "landed", + "a dial outside the allowed egress landed", + ), + ("landed", "landed", "neither arm was refused"), + ("unmeasured", "refused", "the write arm was never attempted"), + ("refused", "unmeasured", "the dial arm was never attempted"), + ("unmeasured", "unmeasured", "neither arm was attempted"), + ] { + assert_eq!( + boundary_word(&probe(write, dial)), + "unverified", + "{why}, so the boundary was not proven", + ); + } +} + +/// **F4 — the word is matched on the key, never on the sentence.** +/// +/// `trace_label` is documented as one stable line and is `key=value`, which is +/// what makes reading it defensible at all. This holds the parse to that: a +/// label that merely contains the word `refused` somewhere — in a backend name, +/// say — is not a refused arm. +#[test] +fn f4_the_probe_is_read_by_its_keys_and_not_by_a_word_appearing_anywhere() { + use io_cli::status::boundary_word; + + assert_eq!( + boundary_word("refused-backend write-outside=landed dial-outside=landed"), + "unverified", + "the word `refused` in a backend name is not an arm that refused", + ); + assert_eq!( + boundary_word("some-backend"), + "unverified", + "a label with no arms at all proves nothing", + ); +} + /// **0.14.0 F10 — `/status` commits the whole state, and every field on it is a /// value io-harness supplied.** /// diff --git a/tests/structure.rs b/tests/structure.rs index 812698f..498c522 100644 --- a/tests/structure.rs +++ b/tests/structure.rs @@ -203,6 +203,73 @@ fn f6_the_migration_report_is_recorded_rather_than_said() { ); } +/// **F6, the last-resort arm.** What nobody said is delivered on the way out of +/// `main`, on every arm rather than on one of them. +/// +/// **This is a defect that shipped, and the shape of the gate is the whole +/// lesson.** `main` holds the report precisely so no early return has to know to +/// deliver it — the comment above `let mut report` says so — and then, until +/// 0.35.0, drained it inside the `Err` arm alone. The wizard's own decline is not +/// an error: answering `io setup` with Esc returns `Ok(exec::OK)`. So an operator +/// upgrading from a pre-0.15.0 install had `io.toml`, `runs.db` and the rest moved +/// into `~/.io-cli`, pressed one key, and was told nothing, with their old +/// directory now empty. The same arm swallowed the line that says a file could +/// **not** be moved because another `io` holds it. +/// +/// **Position is the property, so `contains` cannot express it.** The old code +/// contained this exact loop; it was in the wrong place. The assertion is +/// therefore an ordering — the drain stands before the `match` that splits the two +/// arms — plus a count, because a release that "fixes" this by adding a second +/// loop to the `Ok` arm has reintroduced the thing the comment warns about: the +/// next early return will not know to do it either. +/// +/// Nothing under `tests/` links `src/main.rs`, so a source-text gate is the only +/// assertion available here at all. That is a limitation and it is stated rather +/// than dressed up: this proves where the code sits, not that it runs. +/// +/// Sabotage: move the loop inside the `Err` arm, which is exactly the pre-0.35.0 +/// source. The ordering assertion fails and nothing else in the suite does. +#[test] +fn f6_what_nobody_said_is_delivered_on_every_arm_and_not_only_the_failing_one() { + let text = driver_without_comments(); + + // The needle carries no indentation, deliberately. An earlier draft matched + // `for line in &report {\n eprintln!` — eight spaces — and the sabotage + // below then failed on the `find` rather than on the ordering, because moving + // the loop into an arm indents it to twelve. That is a gate coupled to + // whitespace: it would have gone red on a reformat that changed nothing, and + // it would have reported the wrong reason for a real regression. + let drain = text.find("for line in &report {").expect( + "`main` drains whatever the report still holds to stderr — every arm that had somewhere \ + better to speak has already taken it by `std::mem::take`, so what is left is what nobody \ + said", + ); + assert!( + text[drain..].starts_with("for line in &report {\n") + && text[drain..].contains("eprintln!(\"{line}\");"), + "and it is the stderr drain, not some other loop over the same vector", + ); + + assert_eq!( + text.matches("for line in &report {").count(), + 1, + "exactly one drain. A second loop added to an arm is how this defect returns: the report \ + exists so that no early return has to know to deliver it, and a per-arm drain hands that \ + obligation back to the next arm somebody writes", + ); + + let split = text + .find("match outcome {") + .expect("`main` matches on what `run` returned"); + + assert!( + drain < split, + "the drain is at byte {drain} and the match that splits the arms at {split}: a drain \ + inside an arm is the pre-0.35.0 source, where `io setup` answered with Esc returned \ + `Ok` and told an operator nothing about the install that had just moved underneath them", + ); +} + /// **F14.** Neither arrow key writes a configuration file without a confirmation. /// /// **Counted, not `contains`ed.** `configure::write` is called from two doors and diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 4f9f54a..9ce00b3 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -342,6 +342,65 @@ pub fn harness_run_outcomes() -> Vec { variants } +/// One file of the io-harness source this crate is locked to, as text. +/// +/// The readers below each parse one declaration out of that source; this is for a +/// test that wants a different one. Newlines are normalised, because git checks +/// the registry out with CRLF on Windows and every needle here is written with +/// `\n` — a Windows-only failure in a gate about a constant would be the third +/// instance of that class in this repository. +pub fn harness_source(file: &str) -> String { + std::fs::read_to_string(harness_source_path(file)) + .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. +/// +/// **The unsafe direction of the exit-code seam, and the only one a wildcard does +/// not cover.** `src/exec.rs` maps a harness error to an exit code and exactly one +/// variant — `Refused` — is a boundary refusal worth `REFUSED`; everything else is +/// a malfunction worth `FAILED`, which is what the wildcard arm gives anything a +/// later harness adds. That is the safe direction: a new variant reported as a +/// failure is conservative. +/// +/// The direction that is *not* safe is a harness adding a **second** refusal +/// variant. io-harness 0.74.0 added two new `Error::Refused` *shapes* in one +/// release, so this is not hypothetical — and a second refusal *variant* would be +/// reported by io-cli as `FAILED`, which is precisely the defect 0.35.0 exists to +/// remove, arriving again through the dependency rather than through this crate. +/// A hand-written list of eleven cannot notice it. This can. +pub fn harness_error_variants() -> Vec { + let source = std::fs::read_to_string(harness_source_path("error.rs")) + .expect("io-harness's source is readable from the registry") + .replace("\r\n", "\n"); + let body = source + .split_once("pub enum Error {") + .expect("io-harness declares Error in src/error.rs") + .1; + let body = body.split_once("\n}\n").expect("the enum is closed").0; + + let mut variants = Vec::new(); + for line in body.lines() { + let Some(rest) = line.strip_prefix(" ") else { + continue; + }; + // Doc comments, attributes and the deprecation markers this enum carries + // all fail this, which is the whole filter — a variant is the only thing + // at this indentation that begins with a capital. + if !rest.starts_with(|c: char| c.is_ascii_uppercase()) { + continue; + } + variants.push( + rest.chars() + .take_while(char::is_ascii_alphanumeric) + .collect::(), + ); + } + variants +} + /// Every tool name io-harness declares, in the source this crate is locked to. /// /// The third reader of the dependency's own source, and it exists for the same @@ -716,3 +775,197 @@ impl Provider for Watching { true } } + +/// The one environment lock for a test binary. +/// +/// Every fixture below sets `IO_CONFIG`, which is process-global, so two tests +/// building a configuration at once would each see the other's file. Fourteen +/// test files used to declare a `Mutex` of their own for this; two different +/// mutexes in one binary exclude nothing from each other, so they delegate here +/// and a fixture taking this lock now excludes every environment-touching test +/// beside it rather than only the ones that happened to pick the same mutex. +/// +/// A poisoned lock is taken anyway. The environment is restored by +/// [`UserScope`]'s own drop rather than by unwinding, so a panicking test leaves +/// nothing for the next one to trip over, and refusing to hand out the guard +/// would turn one real failure into every subsequent test failing for a reason +/// that is not its own. +pub fn env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: Mutex<()> = Mutex::new(()); + LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +/// A configuration fixture in the one scope io-harness still trusts. +/// +/// **io-harness 0.74.0 is why this exists.** A workspace-resident configuration +/// file may no longer declare `[[provider]]`, `[[mcp]]`, `[[lsp]]` or — in +/// `io.local.toml` as well as `io.toml` — `[[hook]]`, may not widen the policy in +/// any of ten ways, and may not name an absolute `run.skills` or `run.templates`. +/// The reason is stated in `read_scope`: those files arrive with a `git clone`, +/// or sit in a root the run's own agent can write to, so a single `write_file` of +/// an unremarkable name would otherwise declare an endpoint or a command that the +/// next `Config::discover` acts on, outside the `Policy` and outside the sandbox. +/// +/// `Scope::User` is the sole exemption and the exemption is the whole reason the +/// scope exists — `$IO_CONFIG` is outside every workspace, so a run that can +/// write its own root cannot reach it. +/// +/// **The trap this type exists to close.** The obvious fixture writes `io.toml` +/// into the directory it then hands to `Config::discover`, and points `IO_CONFIG` +/// at that same file. That file is a candidate *twice* — once as `Scope::User` +/// through `IO_CONFIG` and once as `Scope::Project` because it is `root/io.toml` +/// — and the project read refuses it. Pointing `IO_CONFIG` somewhere is not +/// enough; the file has to be somewhere the discovery root does not reach. So +/// this keeps two directories, and the workspace it hands back is empty. +/// +/// **`Config::from_toml` cannot be used for any of this.** It hard-codes +/// `Scope::Project` and calls `refuse_widening` unconditionally, so there is no +/// argument that makes it produce a user-scoped configuration. A fixture that +/// needs one of these sections has to go through discovery, which is why the +/// migration converted the `from_toml` call sites rather than adjusting them. +/// +/// `Debug` because `try_user_scope_locked` returns this in a `Result`, and +/// `Result::expect_err` requires the `Ok` side to be printable — a fixture whose +/// failure case cannot be asserted on with the ordinary vocabulary is a fixture +/// tests work around. +#[derive(Debug)] +pub struct UserScope { + /// Holds the user-scope `io.toml`. Never the discovery root. + home: tempfile::TempDir, + /// The workspace `Config::discover` was pointed at. Deliberately empty. + workspace: tempfile::TempDir, + /// Whether `IO_CONFIG` is still set, so `Drop` knows what to undo. + kept: bool, + pub config: io_harness::Config, +} + +impl UserScope { + /// The workspace root the configuration was discovered against. + pub fn root(&self) -> &std::path::Path { + self.workspace.path() + } + + /// The user-scope configuration file itself. + pub fn path(&self) -> std::path::PathBuf { + self.home.path().join("io.toml") + } + + /// Re-read the file from disk, for a test that has just written to it. + /// + /// The write-then-rediscover shape is what `configure::write` does, and a + /// test asserting over it has to see the same thing the product would. + pub fn reload(&mut self) -> io_harness::Result<()> { + std::env::set_var("IO_CONFIG", self.path()); + let discovered = io_harness::Config::discover(self.workspace.path()); + if !self.kept { + std::env::remove_var("IO_CONFIG"); + } + self.config = discovered?; + Ok(()) + } + + /// Discover again and hand back whatever came out, refusal included. + /// + /// For the tests that assert a refusal's own sentence: they need the `Err`, + /// not a fixture that panics on it. + pub fn rediscover(&self) -> io_harness::Result { + std::env::set_var("IO_CONFIG", self.path()); + let discovered = io_harness::Config::discover(self.workspace.path()); + if !self.kept { + std::env::remove_var("IO_CONFIG"); + } + discovered + } +} + +impl Drop for UserScope { + fn drop(&mut self) { + if self.kept { + std::env::remove_var("IO_CONFIG"); + } + } +} + +/// A user-scoped configuration built from `toml`, with `IO_CONFIG` unset again. +/// +/// Takes [`env_lock`] and releases it before returning, so a caller must not be +/// holding it — `std::sync::Mutex` is not reentrant and doing both deadlocks. +/// Where a test needs the lock held across more than the fixture, take it and +/// call [`user_scope_locked`]. +pub fn user_scope(toml: &str) -> UserScope { + let _guard = env_lock(); + user_scope_locked(toml, false) +} + +/// As [`user_scope`], but `IO_CONFIG` stays set for the fixture's lifetime. +/// +/// For a test that goes on to exercise a product path which resolves the +/// configuration for itself — `configure::write` re-discovers after writing, and +/// would otherwise find the operator's real file. +pub fn user_scope_kept(toml: &str) -> UserScope { + let _guard = env_lock(); + user_scope_locked(toml, true) +} + +/// The body of both, for a caller that already holds [`env_lock`]. +pub fn user_scope_locked(toml: &str, keep: bool) -> UserScope { + match try_user_scope_locked(toml, keep) { + Ok(scope) => scope, + Err(error) => panic!("the user-scope fixture parses: {error}"), + } +} + +/// As [`user_scope_locked`], returning the refusal instead of panicking on it. +/// +/// The tests that assert io-harness's own refusal sentence need this: they are +/// asserting over an `Err`, and a fixture that unwraps has nothing to give them. +pub fn try_user_scope_locked(toml: &str, keep: bool) -> io_harness::Result { + let home = tempfile::tempdir().expect("a directory for the user-scope file"); + let workspace = tempfile::tempdir().expect("a workspace root"); + let path = home.path().join("io.toml"); + std::fs::write(&path, toml).expect("the fixture is written"); + + std::env::set_var("IO_CONFIG", &path); + let discovered = io_harness::Config::discover(workspace.path()); + if !keep { + std::env::remove_var("IO_CONFIG"); + } + + match discovered { + Ok(config) => Ok(UserScope { + home, + workspace, + kept: keep, + config, + }), + Err(error) => { + if keep { + std::env::remove_var("IO_CONFIG"); + } + Err(error) + } + } +} + +/// A user-scope file beside a project-scope one, for the refusal tests. +/// +/// The user file is trusted and the project file is not, which is the whole +/// shape T13 asserts over: a repository that arrives with a `git clone` declaring +/// something only the operator's own file may declare. +pub fn user_scope_with_project( + user: &str, + project: &str, +) -> io_harness::Result { + let _guard = env_lock(); + let home = tempfile::tempdir().expect("a directory for the user-scope file"); + let workspace = tempfile::tempdir().expect("a workspace root"); + let path = home.path().join("io.toml"); + std::fs::write(&path, user).expect("the user fixture is written"); + std::fs::write(workspace.path().join("io.toml"), project) + .expect("the project fixture is written"); + + std::env::set_var("IO_CONFIG", &path); + let discovered = io_harness::Config::discover(workspace.path()); + std::env::remove_var("IO_CONFIG"); + discovered +} diff --git a/tests/wizard.rs b/tests/wizard.rs index 648711c..431e984 100644 --- a/tests/wizard.rs +++ b/tests/wizard.rs @@ -11,7 +11,7 @@ mod support; -use std::sync::{Mutex, MutexGuard, OnceLock}; +use std::sync::{MutexGuard, OnceLock}; use crossterm::event::{Event, KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; use io_cli::keys::Newline; @@ -34,9 +34,25 @@ fn key(code: KeyCode) -> KeyEvent { /// them setting `IO_CONFIG` at once would make each other's `user_path` answer /// wrong — intermittently, on a loaded machine, which is the most expensive kind /// of failure to diagnose. Serialised rather than discovered later. +/// +/// Delegated to [`support::env_lock`] rather than declared here: two different +/// mutexes in one binary exclude nothing from each other, and the `support` +/// fixtures below take that one. fn env_lock() -> MutexGuard<'static, ()> { - static LOCK: Mutex<()> = Mutex::new(()); - LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + support::env_lock() +} + +/// Read a rendered file the way io-harness will actually read it. +/// +/// **Never `Config::from_toml`.** That parses at `Scope::Project`, and since +/// io-harness 0.74.0 a project-scoped file may not declare `[[provider]]` at all +/// — which every file the wizard renders does. The wizard writes the user scope, +/// so that is the scope its output has to be read at. +/// +/// For a caller that already holds [`env_lock`]: `user_scope_locked` does not take +/// the lock itself, and `std::sync::Mutex` is not reentrant. +fn loaded_locked(text: &str) -> Config { + support::user_scope_locked(text, false).config.clone() } /// One config home for this whole file, set once. `settings::user_path` reads the @@ -161,7 +177,7 @@ fn f2_n5_the_wizard_writes_what_it_showed_and_never_shows_the_key() { settings::write(&path, &contents).expect("the file is written"); // --- F2: the file says what the wizard showed. --- - let config = Config::from_toml(&contents).expect("io-harness parses its own file"); + let config = loaded_locked(&contents); let spec = config.provider_spec().expect("a provider was written"); assert!( matches!( @@ -556,11 +572,11 @@ fn f9_the_posture_step_resolves_the_row_the_query_left_visible() { let Progress::Write(_, contents) = progress else { panic!("the confirmation screen should produce a write, got {progress:?}"); }; - // Read the way io-harness reads it. A narrowing posture parses in any scope, - // and row 0 — `Sandboxed workspace`, which is what a filtered index would - // have resolved to — widens, so a wrong row fails here twice over: on this - // parse, and on the effect below. - let config = Config::from_toml(&contents).expect("a narrowing posture parses in any scope"); + // Read the way io-harness reads it: the user scope, which is where the wizard + // writes. Row 0 — `Sandboxed workspace`, which is what a filtered index would + // have resolved to — widens, and the effect assertion below is what a wrong + // row fails on. + let config = loaded_locked(&contents); let policy = config.policy().expect("a policy was written"); assert_eq!( policy.defaults.write, @@ -772,8 +788,10 @@ fn the_rendered_file_is_the_harness_schema_and_nothing_of_our_own() { // The wizard writes the USER scope, where widening is the operator's own // decision and is allowed — so the default "sandboxed workspace" posture is a // correct file that `from_toml` is right to reject. Discovering it through the - // scope it is written to is the faithful check; the narrower postures below - // parse either way. + // scope it is written to is the faithful check. Since 0.74.0 the narrower + // postures no longer parse either way — the `[[provider]]` the wizard renders + // is itself refused at project scope — which is what the assertion below is + // now about. let dir = tempfile::tempdir().expect("a temporary directory"); // The user file lives outside the workspace it is discovered from. Put it at // `/io.toml` and it is ALSO the project-scope candidate, which is read @@ -799,9 +817,20 @@ fn the_rendered_file_is_the_harness_schema_and_nothing_of_our_own() { "the user scope may widen, which is the whole reason the wizard writes there", ); - // A posture that only narrows is legal in any scope, project included. + // A posture that only narrows used to be legal in any scope, project + // included. Since io-harness 0.74.0 the whole rendered file is refused at + // project scope whatever the posture — but for the `[[provider]]` it carries, + // never for the policy. Asserted on WHICH key the refusal names, because "it + // is refused" is now true of both postures and would prove nothing. let narrow = settings::render(&spec, Posture::ReadOnly, "dark").expect("render"); - Config::from_toml(&narrow).expect("a narrowing file parses as a project file too"); + let refusal = Config::from_toml(&narrow) + .expect_err("0.74.0 refuses `[[provider]]` from a project-scoped file") + .to_string(); + assert!( + refusal.contains("key `provider`"), + "a file with no widening in it was refused at project scope for something other than the \ + provider section, so the posture is what the scope objects to after all: {refusal}", + ); // No key written means the provider's own environment variable, which is the // better outcome and not a fallback: a key that is never on disk cannot leak