diff --git a/AGENTS.md b/AGENTS.md index adda48e..856d20c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,11 +59,17 @@ the only dev-dependency. Adding one is a deliberate, argued act — read the com `git clone` for a marketplace) and nowhere else. - **TOML parsing** is permitted in `src/edit.rs` alone. Every other module that needs a value out of a configuration file goes through `edit::value_at`, `edit::sections` or `edit::array`. - `src/marketplace.rs` reads a stranger's `plugin.toml` that way. + `src/marketplace.rs` reads a stranger's `plugin.toml` that way, and `src/adapt.rs` *writes* one + without ever parsing it. +- **JSON deserialization** is permitted in `src/import.rs` and `src/adapt.rs`, and nowhere else. + The rule is over the parse rather than over the crate name: `src/exec.rs` serializes its own + `--json` event lines and is not a second opinion about anybody's file. The set is compared with + `==` and has its own near-miss test, because a widening makes a gate more permissive and so goes + vacuous without going red. - **No agent loop.** The sweep strips comments first, so a doc comment containing "while" is not a finding; `src/provider.rs` is exempted by path and held to four properties of its own. -**Module map (`src/`, 74 files).** +**Module map (`src/`, 75 files).** - Session surface — `app.rs` (state), `term.rs` (an inline viewport, never the alternate screen), `events.rs` + `triage.rs` (what each io-harness event does; a kind with no @@ -81,7 +87,8 @@ the only dev-dependency. Adding one is a deliberate, argued act — read the com `provider.rs`. - Store — `store.rs`, `export.rs`, `undo.rs`, `rewind.rs`, `sessions.rs`. - Bundles — `skills.rs`, `skillview.rs`, `plugin`-facing `pluginview`, `marketplace.rs`, - `fetch.rs`, `import.rs`. + `fetch.rs`, `import.rs`, `adapt.rs` (the three manifest formats io did not invent, read; and the + `plugin.toml` io generates for one, written and never parsed). - Media and git — `picture.rs`, `attach.rs`, `repo.rs`, `commit.rs`. **Nothing under `tests/` links `src/main.rs`.** A guard written in the driver cannot be tested diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c2367..28b9288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,79 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +## [0.31.0] - 2026-08-30 + +io-cli has operated a marketplace nobody could stock. Every capability bundle +published in the field is a Claude Code plugin or a Codex plugin, and +`plugin.toml` is a format only this project writes — so the surface 0.29.0 and +0.30.0 built, disclosed and gated was reachable only by a bundle you had written +yourself, which is the one case that never needed a marketplace. A survey of the +marketplaces on the author's own machine found five repositories publishing 304 +plugins between them and zero `.toml` files among them. + +From this release io reads the three formats a bundle is actually published in. +The bet is that a plugin's value is in its skills, its templates and its agents +rather than in the syntax of its manifest, and that a tool which reads three +formats is better placed than one that asks 291 strangers to adopt a fourth. + +### Added + +**`.claude-plugin/marketplace.json` is read as a repository's index**, and +`.claude-plugin/plugin.json` and `.codex-plugin/plugin.json` as bundle manifests +wherever the existing walk already looks. The precedence is stated: a +`plugin.toml` at a repository's root suppresses the index and nothing else; where +there is no root manifest the index is the answer and the walk does not also run, +because a union would list bundles the author never published beside the ones +they did; where there is neither, the walk reads each directory natively first and +foreign second. An entry in a shape io does not read is listed with its reason +rather than dropped. + +**An index may place a plugin in another repository**, which 238 of the official +marketplace's 291 entries do, and installing one fetches it at the commit the +index names. `git clone --revision` is not available below git 2.49, so a +commit-pinned shallow fetch is four invocations rather than one. + +**An adapter manifest is generated** under +`~/.io-cli/adapters////plugin.toml`, with absolute paths into +the clone, so io-harness loads the bundle with no change to that crate. The +stranger's checkout is never written to. `/plugin` draws an adapted bundle under +its own mark with the generated manifest's directory on the row, so the +difference between what an author wrote and what io generated is not something +you have to infer, and the file to open when io-harness drops a bundle is named. + +### Changed + +**Skills follow `$IO_CONFIG` and `$IO_CONFIG_HOME`, on both the read and the +write.** They resolved through io-cli's default home while your memory file +resolved through the home actually in force, so pointing either variable +elsewhere moved one and not the other. A skill is something you wrote, and it +belongs where the rest of what you wrote is. The limitation this removes was +recorded in `docs/CONTRACT.md` since 0.30.1. + +**io does not move skills you already have**, and it says so rather than leaving +you to notice. If you have set either variable and your old `~/.io-cli/skills` +still holds anything, io names both directories at startup — a release that moved +only the read would have had `/skills add` writing where nothing looks. + +### Not included, deliberately + +**A Claude Code or Codex plugin's hooks are not carried across.** io-harness's +hooks are argv against its own event tags and deliberately never a shell string, +and it refuses `${env:}`, `${file:}` and `${cmd:}` inside a manifest in every +scope — so a hook in either foreign format is a shell line, an unknown event and a +refused substitution at once. No adapter closes that, and an approximated hook is +a program running on your machine that nobody described accurately. + +So every hook a bundle declares is **shown** before you install it, with its +event, its command unshortened, and the reason it will not run. If you want hooks +under io, the repository's author adds a `plugin.toml`, which then wins its own +root. + +io also does not become compatible with another tool's runtime — the variables it +sets, the directory it runs a hook in, its permission model or its session +lifecycle. io reads what a manifest says and nothing about how another program +would have executed it. + ## [0.30.2] - 2026-08-30 A documentation release. Thirty-one claims across the README, the changelog, the @@ -2667,7 +2740,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.30.2...HEAD +[Unreleased]: https://github.com/initorigin/io-cli/compare/v0.31.0...HEAD +[0.31.0]: https://github.com/initorigin/io-cli/compare/v0.30.2...v0.31.0 [0.30.2]: https://github.com/initorigin/io-cli/compare/v0.30.1...v0.30.2 [0.30.1]: https://github.com/initorigin/io-cli/compare/v0.30.0...v0.30.1 [0.30.0]: https://github.com/initorigin/io-cli/compare/v0.29.0...v0.30.0 diff --git a/Cargo.lock b/Cargo.lock index 297115c..e261fcd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,7 +1850,7 @@ dependencies = [ [[package]] name = "io-cli" -version = "0.30.2" +version = "0.31.0" dependencies = [ "clap", "crossterm", diff --git a/Cargo.toml b/Cargo.toml index 71c6fd7..66f0bfe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "io-cli" -version = "0.30.2" +version = "0.31.0" edition = "2021" # io-harness declares 1.95, set by `libsqlite3-sys` calling the std `cfg_select!` # macro from its build script. Nothing here can be lower than what the harness diff --git a/README.md b/README.md index 2db8d87..2d6bef9 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,7 @@ the limits that capability actually has. | [When a run stops for you](docs/guide/resume.md) | A question, a plan or an interrupted call answered where it was left — and one `io` at a time on one conversation | | [What it costs](docs/guide/accounting.md) | The money and the token split, where a price comes from, and why an unpriced model makes a total a floor | | [Skills](docs/guide/skills.md) | The five shipped, yours, and what a duplicate name costs | -| [Capability bundles](docs/guide/plugins.md) | A directory contributing to six subsystems, marketplaces, and what a bundle is allowed to do being shown before it may do it | +| [Capability bundles](docs/guide/plugins.md) | A directory contributing to six subsystems, marketplaces, installing a Claude Code or Codex plugin, and what a bundle is allowed to do being shown before it may do it | | [Hooks](docs/guide/hooks.md) | Reacting to a run from `io.toml`, and the failure that is quiet | | [The fleet](docs/guide/fleet.md) | Contained turns, the tree `Ctrl+F` shows, and the plan a contained turn proposes | | [Verification gates](docs/guide/verification.md) | The criterion, what it costs per step, and exit `6` | diff --git a/docs/CAPABILITIES.md b/docs/CAPABILITIES.md index b38839a..cc1afb0 100644 --- a/docs/CAPABILITIES.md +++ b/docs/CAPABILITIES.md @@ -26,7 +26,7 @@ the limits that capability actually has. | [When a run stops for you](guide/resume.md) | A question, a plan or an interrupted call decided where it was left; the turn an interrupt makes unresumable; and one `io` at a time on one conversation | | [What it costs](guide/accounting.md) | Money and the token split by run, session, model and day; where a price comes from; and why an unpriced model makes a total a floor | | [Skills](guide/skills.md) | The five this crate ships, adding and removing your own, the claimed name being the frontmatter name, and what a duplicate costs | -| [Capability bundles](guide/plugins.md) | A directory contributing to six subsystems at once, marketplaces, and the disclosure that happens with your configuration file untouched | +| [Capability bundles](guide/plugins.md) | A directory contributing to six subsystems at once, marketplaces, installing a Claude Code or Codex plugin, and the disclosure that happens with your configuration file untouched | | [Hooks](guide/hooks.md) | An audit log, a notification, a formatter or a check that stops the run, declared in `io.toml` — and the failure that is quiet | | [The fleet](guide/fleet.md) | Contained turns spawning children under one shared ceiling, the live tree, and the plan a contained turn proposes before it acts | | [Verification gates](guide/verification.md) | The criterion, the fact that it is evaluated after every step rather than once per turn, per-gate retry, and exit `6` | diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index 9a55367..5c4d857 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -104,8 +104,9 @@ One directory: `~/.io-cli` on Unix, `%USERPROFILE%\.io-cli` on Windows, created | --- | --- | | `io.toml` | The user-scope configuration | | `runs.db` (+ `-wal`, `-shm`) | The session store | -| `skills/` | Skills, with `skills/disabled/` for the ones switched off | -| `marketplaces/` | Cloned marketplaces | +| `skills/` | Skills, with `skills/disabled/` for the ones switched off. Read from and written to the home in force, so `$IO_CONFIG`/`$IO_CONFIG_HOME` moves it with `io.toml` and `IO.md`. io does not move files you already own; it says so at startup if the old directory still holds any | +| `marketplaces/` | Cloned marketplaces, with `marketplaces/.entries/` for a repository an index pointed at | +| `adapters////` | The `plugin.toml` io generates for a Claude Code or Codex bundle | | `.fetching/` | Staging for a clone in flight, renamed into place on success | | `IO.md` | The user-scope memory file | | `.import-offered` | Marks that the import offer was made, so declining is remembered | @@ -116,10 +117,17 @@ deleted that was not already copied. This does nothing at all when `$IO_CONFIG` ## Limits that hold today -**`$IO_CONFIG_HOME` does not move the skills directory.** The skills surfaces resolve through -io-cli's *default* home (`home::path()`) where the memory file resolves through the home actually -in force (`home::in_force()`), so an operator who points `$IO_CONFIG_HOME` elsewhere gets their -memory file there and their skills in `~/.io-cli/skills/`. Known since 0.30.1. +**A marketplace, and an entry inside one, resolve against `github.com` and no other host.** A +marketplace is named `/`; an index entry naming a repository elsewhere has its url +re-derived to that shape, and a url that is not two ordinary path segments on that host is refused +and listed with its reason. The only string io hands `git` is one io built — a url somebody else +wrote reaching a clone is how `ext::sh -c …` becomes a remote shell. All 238 remote entries of +`anthropics/claude-plugins-official` are on that host. + +**A Claude Code or Codex plugin's hooks are read, shown, and translated into nothing.** +io-harness's hooks are argv against its own event tags and never a shell string, and it refuses +`${env:}`, `${file:}` and `${cmd:}` inside a manifest in every scope. This is an impossibility, not +a deferral: the author adds a `plugin.toml` if they want hooks under io. **A turn the operator interrupted cannot be resumed.** It is reported as ended, with `/fork` offered instead. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 209e7cb..c8b3ad9 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -261,7 +261,7 @@ Nine keys live there, and eight tables: | `diff` | `unified` — the default, and what an absent key means — or `minimal`, the changed lines and the `@@` header without the context, for reviewing by file rather than by hunk. | | `glyphs` | `unicode` or `ascii`. Absent asks the locale. | | `plain` | `true` runs every session in plain mode. The same switch as `--plain`, which wins over it. | -| `skills` | a directory of skills for the agent. They appear in the `/` palette by name, and the agent reads them itself. Absent, it is `~/.io-cli/skills`. A leading `~` is your home directory — io-cli expands it before io-harness sees the path, because io-harness substitutes `${env:…}`, `${file:…}` and `${cmd:…}`, and a tilde is none of the three. | +| `skills` | a directory of skills for the agent. They appear in the `/` palette by name, and the agent reads them itself. Absent, it is `skills/` under the home in force — `~/.io-cli/skills` unless `$IO_CONFIG` or `$IO_CONFIG_HOME` moved it. A leading `~` is your home directory — io-cli expands it before io-harness sees the path, because io-harness substitutes `${env:…}`, `${file:…}` and `${cmd:…}`, and a tilde is none of the three. | | `max_parallel_reads` | how many read-only tool calls one turn may run at once. Absent, it is io-harness's own 10; `0` is clamped to 1 rather than meaning none. A `TaskContract` field with no io-harness configuration key of its own, which is why it is named here. | | `spawn_background_after_secs` | how long a spawned child may run before it is backgrounded. Absent, a child is waited for however long it takes. | | `detached_spawns` | whether a spawn may detach at all. Absent, it may. `false` buys a trace with every child's whole life in it, which a detached child gives up. | @@ -289,6 +289,9 @@ configuration file is in it, and so is the run store `runs.db` with the `-wal` and `-shm` SQLite keeps beside it — which is where the agent's durable memory lives too, because that is rows inside the store rather than a file of its own — and the skills directory, which is `~/.io-cli/skills` when `skills` names none. +That directory is the `skills/` of the home in force — the same home your +`io.toml` and `IO.md` are in — so `$IO_CONFIG` or `$IO_CONFIG_HOME` moves all +three together, and io reads and writes skills in whichever one is in force. `~/.io-cli/IO.md` is in it as well: the guidance you want in every project, which `/remember` writes when you pick that scope. That is one directory to copy to a new machine, and one path to put in a bug report — as long as you have named no diff --git a/docs/guide/plugins.md b/docs/guide/plugins.md index 185a126..3e478d3 100644 --- a/docs/guide/plugins.md +++ b/docs/guide/plugins.md @@ -95,7 +95,7 @@ the hand-written reader could not: an inline `hook = [{…}]` array, and a ## Marketplaces **A marketplace is a git repository you name.** It is cloned into your own home -and walked for directories carrying a `plugin.toml`: +and read for the bundles it publishes: ``` /plugin marketplace add zeroonething/ultraship @@ -123,6 +123,69 @@ Installing is the verb you already had: spellings. They are two strangers' repositories, and installing whichever the walk reached first is installing code you did not choose. +### Three manifest formats, and which one wins + +A capability bundle in the field is a Claude Code plugin or a Codex plugin. +`plugin.toml` is a format this project writes and nobody else does, so io reads +all three rather than asking anyone to adopt a fourth: + +| File | Read as | +| --- | --- | +| `plugin.toml` | A bundle, natively | +| `.claude-plugin/marketplace.json` | The repository's own index of what it publishes | +| `.claude-plugin/plugin.json`, `.codex-plugin/plugin.json` | A bundle manifest | + +The precedence is stated rather than discovered: + +- **A `plugin.toml` at the repository's root suppresses the index.** An author who + writes io's own manifest has said what they publish in the format io owns, and a + foreign index does not speak over it. It suppresses the index and nothing else — + a repository with a root manifest and bundles beneath it still lists all of them. +- **Where there is no root `plugin.toml`, the index is the answer** and the + directory walk does not also run. A union would list bundles the author did not + publish beside the ones they did, and you would have no way to tell which was + which. +- **Where there is neither, the walk runs**, reading each directory as a + `plugin.toml` first and a foreign manifest only where it carries none. + +An entry in a shape io does not read is **listed with its reason**, never dropped. + +**An index may place a plugin in another repository**, and 238 of the 291 entries +in `anthropics/claude-plugins-official` do. Those are fetched when you install +one, at the commit the index names where it names one. io re-derives the +repository from the url and rebuilds it: a url that is not `/` on +GitHub is refused, because the only string io hands `git` is one io built. + +**An adapted bundle is marked as adapted in `/plugin`**, under its own mark, with +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. + +**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 +contributes, so io accepts a name that is already an id, folds one that becomes an +id by lowercasing, and refuses everything else. Two entries in one index reaching +the same id are refused naming both. + +### Hooks do not cross, and that is not a gap + +**A Claude Code or Codex plugin's hooks are not carried across.** io-harness's +hooks are argv against its own events, deliberately never a shell string; a hook +in those formats is a shell line, an event io does not know, and a `${…}` +substitution io-harness refuses inside a manifest in every scope. All three at +once, and no adapter closes any of them. + +So io **shows** them instead. Every hook a bundle declares is drawn before you +install it, with its event, its command **unshortened**, and the reason it will +not run. An approximated hook is a program running on your machine that nobody +described accurately, and a shortened one is a command you consented to without +reading. + +If you want hooks under io, the repository's author adds a `plugin.toml` — one +file, which then wins its own root. + **Removing a marketplace removes the clone and nothing else.** A bundle you declared out of it keeps its `[[plugin]]` entry — a cache being emptied is not a reason to undo a decision you made about your configuration. What io owes you diff --git a/src/adapt.rs b/src/adapt.rs new file mode 100644 index 0000000..2091b20 --- /dev/null +++ b/src/adapt.rs @@ -0,0 +1,1212 @@ +//! The manifest formats io-cli did not invent. +//! +//! A capability bundle in the field is a Claude Code plugin or a Codex plugin. +//! `plugin.toml` is a format this project writes and nobody else does, so the +//! marketplace surface [`crate::marketplace`] built could be stocked only by a +//! bundle the operator had written themselves — which is the one case that never +//! needed a marketplace. Five marketplaces publishing 304 plugins were surveyed +//! on one machine while this module was written and not one of them carried a +//! `.toml`. +//! +//! **This module is the only place in the crate that parses JSON that arrived +//! from somewhere else**, beside [`crate::import`], which has read the operator's +//! own Claude and Codex files since 0.21.0. `tests/dependencies.rs` names the two +//! by exact path and fails when a third module deserializes JSON, for the reason +//! the TOML rule already gives in its own words: a second reader of a stranger's +//! file is a second opinion about what that file means. Serializing is not that +//! problem and is not gated — `src/exec.rs` writes io-cli's own event lines and +//! reads nobody. +//! +//! **Three formats, and the precedence between them is the point.** A directory +//! carrying a native `plugin.toml` is read by [`crate::marketplace::manifest`] +//! exactly as it was before this module existed; nothing here can win against it. +//! Where a repository carries `.claude-plugin/marketplace.json`, that file is the +//! author's own statement of what the repository publishes and it is the answer — +//! the directory walk does not also run, because a union of the two would list +//! bundles the author did not publish beside the ones they did and give an +//! operator no way to tell which was which. Where a repository carries neither, a +//! `.claude-plugin/plugin.json` or a `.codex-plugin/plugin.json` is read as a +//! manifest wherever [`crate::pluginview::candidates`] would have looked for a +//! `plugin.toml`. +//! +//! **The dotted-directory skip in that walk is preserved and this module does not +//! weaken it.** `.claude-plugin` is read at a known path relative to a directory +//! the walk already visited; the walk itself still never descends into a dot +//! directory, which is what keeps `.git` out of a marketplace listing. +//! +//! **Everything read here is the same trust class as a subprocess's stderr.** A +//! stranger's description may carry raw newlines or control characters, and on +//! this renderer the scrollback is the transcript — a forged line is read as +//! io-cli's own. Every value except a hook's command goes through +//! [`crate::marketplace::plain`] and [`crate::marketplace::bounded`] on the way +//! out of this module rather than at each surface, so a new surface cannot forget. +//! A hook's command is filtered and **not** bounded, because it is argv an +//! operator is being asked to consent to and a shortened argv is the one thing +//! that surface must never show. +//! +//! **An entry this module cannot read is reported, never skipped.** A marketplace +//! index is written by somebody else and a shape nobody anticipated is found by an +//! operator rather than by a fixture; a listing that silently dropped it would +//! make that failure invisible and unreportable. +//! +//! # Reading a foreign bundle is half of it; [`generate`] is the other half +//! +//! Everything above answers *what is in this repository*. A bundle that is only +//! listed is a bundle nobody can use: io-harness loads a directory with a +//! `plugin.toml` at its root and nothing else, so a Claude Code plugin an operator +//! 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.71.0/src/plugin.rs:268`), 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. +//! +//! **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 +//! about *meaning* — the spelling of a value goes through `toml`'s own serializer +//! here for the same reason [`crate::edit::array`] argues for it, and anything that +//! reads a manifest back reads it through [`crate::edit::value_at`] like every +//! other file. What [`generate`] hands back about the file it wrote comes from +//! io-harness's own accessors and never from io-cli's account of what it spelled. +//! +//! **Translate what maps; disclose what does not.** Four kinds map — `skills/`, +//! `commands/`, `agents/*.md` and `.mcp.json`. A frontmatter key with no slot in +//! an [`AgentDef`], and a hooks file, are reported on [`Adapter::disclosed`] so a +//! surface can name them. Nothing is silently dropped and nothing is approximated: +//! [`Hook`]'s own docs give the argument for why a hook is shown and never +//! translated, and it applies to every key here. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use io_harness::config::Scope; +use io_harness::{AgentDef, McpServer, McpTransport, Plugins, PLUGIN_FILE}; +use serde::Deserialize; + +use crate::marketplace::{bounded, plain}; + +/// The directory a Claude Code plugin keeps its manifests in. +pub const CLAUDE_DIR: &str = ".claude-plugin"; + +/// The directory a Codex plugin keeps its manifest in. +/// +/// A separate constant rather than a list, because the two are not +/// interchangeable: `zeroonething/caveman` carries `.codex` and no +/// `.codex-plugin`, and `zeroonething/ponytail` carries `.codex-plugin`. A reader +/// that treated any dot directory starting with `.codex` as this one would read +/// the first repository's Cursor and Cline rules as a plugin manifest. +pub const CODEX_DIR: &str = ".codex-plugin"; + +/// The index file, inside [`CLAUDE_DIR`]. +pub const INDEX_FILE: &str = "marketplace.json"; + +/// The manifest file, inside either [`CLAUDE_DIR`] or [`CODEX_DIR`]. +pub const MANIFEST_FILE: &str = "plugin.json"; + +/// Where a plugin an index names actually lives. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Source { + /// A path inside the clone. `"./"` is the clone's own root, and + /// `"./plugins/x"` a directory in it. 53 of the official marketplace's 291 + /// entries are this. + Local(String), + /// Another repository, to be fetched at install time. 238 of that index's 291 + /// entries are this. + Remote(Remote), +} + +/// A plugin published in a repository other than the one the index is in. +/// +/// **One shape for both of the tags the field uses.** An index spells this +/// `{"source": "url", …}` 153 times and `{"source": "git-subdir", …}` 85 times, +/// and the two carry the same keys — a `url`, an optional `path` into it, an +/// optional `ref`, an optional `sha`. Reading them through one type keyed on the +/// presence of `url` rather than on the tag is deliberate: a second reader for a +/// second name is the second-implementation defect this product has paid for in +/// `servers::edit`, in `providers::edit` and in the guided browser that built a +/// command string by hand. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Remote { + /// The repository to clone. + pub url: String, + /// The directory inside it the bundle is rooted at, where the entry names one. + pub path: Option, + /// A tag or branch. Spelled `ref` in the file, which is a Rust keyword. + pub reference: Option, + /// A commit to pin to. Every one of the official index's 238 remote entries + /// carries one; `superpowers-marketplace`'s ten carry none, so it is optional. + pub sha: Option, +} + +/// One plugin an index names. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Entry { + /// The author's own name for it, and the word an operator types at + /// `plugin add`. Filtered and bounded. + pub name: String, + /// Filtered and bounded, or `None` where the entry carries none. + pub description: Option, + /// Displayed and never resolved — io-harness resolves no versions and this + /// release does not become the layer that does. + pub version: Option, + /// Where the plugin is. + pub source: Source, +} + +/// A repository's own statement of what it publishes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Index { + /// The entries this module could read, in the order the file lists them. + pub entries: Vec, + /// One line per entry it could not, naming what was wrong. Reported rather + /// than dropped: see the module docs. + pub unreadable: Vec, +} + +/// One bundle manifest, in either of the two foreign formats. +/// +/// Every field is optional because every field is optional in the wild — a +/// `plugin.json` naming nothing is a file this module reports rather than one it +/// refuses, exactly as [`crate::marketplace::manifest`] treats a `plugin.toml` +/// that names nothing. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Manifest { + /// Filtered and bounded. + pub name: Option, + /// Filtered and bounded. + pub description: Option, + /// Filtered and bounded. + pub version: Option, + /// A relative path to a skills directory, where the manifest names one. Codex + /// manifests write `"./skills/"`; Claude manifests generally leave it out and + /// the directory is found by its conventional name. + pub skills: Option, + /// A relative path to a hooks file, where the manifest names one. + pub hooks: Option, + /// Which directory it was read from, so a surface can say whether it was the + /// Claude manifest or the Codex one without guessing. + pub from: PathBuf, +} + +/// One hook a foreign manifest declares. +/// +/// Read to be **shown** and never to be translated. io-harness's `Hook.run` is +/// argv and deliberately never a shell string, its `on` takes the harness's own +/// event tags, and 0.71.0 refuses `${env:}`, `${file:}` and `${cmd:}` inside a +/// manifest in every scope. A Claude hook is +/// `"\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" session-start"` — a shell +/// string, an unknown event and a refused substitution, all three at once. No +/// adapter closes that; an approximated hook is a program running on the +/// operator's machine that nobody described accurately. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Hook { + /// The event the foreign format names, verbatim. + pub event: String, + /// The command it would run, filtered of control characters and **not** + /// bounded. See the module docs. + pub command: String, +} + +/// The index a clone publishes, or `None` where it publishes none. +#[must_use] +pub fn index_at(clone: &Path) -> Option { + let file = clone.join(CLAUDE_DIR).join(INDEX_FILE); + let text = std::fs::read_to_string(&file).ok()?; + let wire: WireIndex = serde_json::from_str(&text).ok()?; + + let mut entries = Vec::new(); + let mut unreadable = Vec::new(); + for slot in wire.plugins { + match slot { + Slot::Read(entry) => match read_entry(entry) { + Ok(entry) => entries.push(entry), + Err(said) => unreadable.push(said), + }, + Slot::Unread(bad) => unreadable.push(bad.said()), + } + } + Some(Index { + entries, + unreadable, + }) +} + +/// The foreign manifest in `dir`, Claude's preferred over Codex's. +/// +/// **Claude first because it is the format more repositories publish**, not +/// because it says more: the two carry the same keys for everything this module +/// reads. Where a repository publishes both, they agree in every case surveyed, +/// and a rule that picked one is better than a merge that could produce a bundle +/// neither file describes. +#[must_use] +pub fn manifest_at(dir: &Path) -> Option { + for holder in [CLAUDE_DIR, CODEX_DIR] { + let file = dir.join(holder).join(MANIFEST_FILE); + let Ok(text) = std::fs::read_to_string(&file) else { + continue; + }; + let Ok(wire) = serde_json::from_str::(&text) else { + continue; + }; + return Some(Manifest { + name: value(wire.name), + description: value(wire.description), + version: value(wire.version), + skills: value(wire.skills), + hooks: value(wire.hooks), + from: file, + }); + } + None +} + +/// Every hook a hooks file declares, in the order it declares them. +/// +/// An empty vector for a file that is not there, does not parse, or declares +/// none: this answers a disclosure surface, and a bundle with no hooks and a +/// bundle whose hooks file is malformed both cross the same amount of nothing. +/// What must not happen is a hook that exists and is not drawn, and every shape +/// below that yields rows yields one row per hook. +#[must_use] +pub fn hooks_in(file: &Path) -> Vec { + let Ok(text) = std::fs::read_to_string(file) else { + return Vec::new(); + }; + let Ok(wire) = serde_json::from_str::(&text) else { + return Vec::new(); + }; + let mut found = Vec::new(); + for (event, matchers) in wire.hooks { + for matcher in matchers { + for hook in matcher.hooks { + let Some(command) = hook.command else { + continue; + }; + found.push(Hook { + // **The event is bounded and the command is not**, and the + // asymmetry is the point rather than an oversight: an event is + // a label, and a label a stranger made a megabyte long is a + // row that buries the one above it. A command is argv the + // operator is being told will not run, and a shortened argv on + // a consent surface is the one thing it must never show. + event: bounded(&plain(&event)), + command: plain(&command), + }); + } + } + } + found +} + +/// An index entry's name as a plugin id io-harness accepts, or `None` where no +/// such id is derivable from it. +/// +/// **An id is not a label, and that is why this refuses far more than it maps.** +/// It is the word an operator types at `plugin add`, and io-harness namespaces +/// every name a bundle contributes with it — a bundle whose id is `rust-review` +/// contributes `rust-review__reviewer`. A mapping that dropped or invented +/// characters would hand a person an id they never saw in the index and cannot +/// guess from it: `My.Plugin` quietly installed as `my-plugin` is one name in the +/// author's file, a second in the listing, and a third thing entirely in every +/// namespaced name the bundle goes on to contribute. +/// +/// So the line is drawn at what a reader undoes by eye: +/// +/// - A name that is already an id is the id, unchanged. +/// - A name that becomes one under an ASCII case fold alone is folded. `Rust` and +/// `rust` are the same word to anyone reading them, and the fold is the one +/// transformation a person performs without being told it happened. +/// - Everything else refuses — a dot, a space, an underscore, a slash, a leading +/// hyphen, a non-ASCII letter, or a name longer than +/// [`io_harness::MAX_ID`]. [`crate::marketplace`] reports it by name. +/// +/// Refusing costs the index's author one renamed key. Mangling costs every +/// operator of that marketplace a name they cannot type. +/// +/// The rule is restated here rather than called because io-harness's own check is +/// private and answers about a `plugin.toml` that does not exist yet — an index +/// entry has no file to be refused against. [`io_harness::MAX_ID`] is taken from +/// the crate rather than copied, so the one part of the rule that can move is not +/// spelled twice. +#[must_use] +pub fn normalised(name: &str) -> Option { + // ASCII, not Unicode: a Unicode fold can turn one character into several + // (`İ` folds to `i` plus a combining dot), which is exactly the invented + // character this refuses. Byte length is the id's length because an id that + // passes the character rule is ASCII, and it is what io-harness measures. + let id = name.to_ascii_lowercase(); + let usable = !id.is_empty() + && id.len() <= io_harness::MAX_ID + && id.starts_with(|glyph: char| glyph.is_ascii_lowercase() || glyph.is_ascii_digit()) + && id + .chars() + .all(|glyph| glyph.is_ascii_lowercase() || glyph.is_ascii_digit() || glyph == '-'); + usable.then_some(id) +} + +// --------------------------------------------------------------------------- +// Writing the manifest io-harness reads +// --------------------------------------------------------------------------- + +/// The conventional directory a bundle keeps its skills in. +/// +/// A Codex manifest names it (`"./skills/"`) and a Claude one generally does not, +/// so [`Manifest::skills`] is preferred where it is there and this is the answer +/// where it is not — which is every Claude bundle surveyed. +pub const SKILLS_DIR: &str = "skills"; + +/// The conventional directory a bundle keeps its slash commands in. +/// +/// 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.71.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. +pub const COMMANDS_DIR: &str = "commands"; + +/// The conventional directory a bundle keeps its agent definitions in. +pub const AGENTS_DIR: &str = "agents"; + +/// The MCP server file a Claude Code bundle publishes at its root. +pub const MCP_FILE: &str = ".mcp.json"; + +/// The one substitution a bundle writes that io can honestly expand. +/// +/// It means the bundle's own root, which is the directory being adapted — so the +/// answer is already known and nothing of the host is read to produce it. Every +/// other `${...}` is refused by name; see [`expanded`]. +pub const PLUGIN_ROOT: &str = "${CLAUDE_PLUGIN_ROOT}"; + +/// One generated manifest: where it is, what io-harness reads out of it, and what +/// the bundle carried that it does not. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Adapter { + /// The `plugin.toml` that was written. + pub manifest: PathBuf, + /// What the bundle contributes, in io-harness's own words and its own fixed + /// order. + /// + /// [`io_harness::Plugin::contributions`] on the file that was just written — + /// not io-cli's account of what it spelled. The two can only agree if the + /// manifest loaded, and it is read back through io-harness precisely so that + /// a surface reporting "skills, templates, agents, mcp" is reporting a fact + /// rather than an intention. + pub contributes: Vec, + /// One line per thing the bundle carried that a `plugin.toml` has no slot for. + /// + /// 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 adapter directory for one bundle, under an adapters root. +/// +/// Three levels — `//` — which is [`crate::marketplace::at`]'s +/// own two-level layout with the bundle's own name under it, because one clone +/// publishes many bundles and a `plugin.toml` is recognised by sitting at a +/// directory's root. +/// +/// The root is an argument rather than a call into [`crate::home::adapters`], for +/// the reason `src/marketplace.rs`'s docs give for the same shape: a decision that +/// lives behind [`crate::home`] cannot be reached by a test without moving the +/// operator's home out from under a suite running in parallel. +#[must_use] +pub fn at(root: &Path, owner: &str, repo: &str, name: &str) -> PathBuf { + root.join(owner).join(repo).join(name) +} + +/// Write the `plugin.toml` that makes `bundle` loadable, and report what it says. +/// +/// `bundle` is a directory inside a clone and is **only read**; `into` is where +/// the manifest goes, and it is regenerated rather than edited, so an adapter that +/// is already there is replaced. `name` is the id the manifest declares and the +/// word every name the bundle contributes is namespaced by — it goes through +/// [`normalised`], which refuses far more than it maps and says why. +/// +/// # The four kinds, and the one rule that decides the paths +/// +/// `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.71.0/src/plugin.rs:268`) 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. +/// +/// # Validated through io-harness, on the bytes that were written +/// +/// [`io_harness::Plugins::inspect`] is the loader +/// [`Config::plugins`](io_harness::Config::plugins) runs, reached without a +/// `[[plugin]]` entry — the id grammar, the trust rule, the narrowing rule and the +/// substitution refusal, all of them, on the file that is on disk. A manifest it +/// refuses is **removed before this returns**, so nothing an operator could go on +/// to declare is left behind: the whole point of adapting a bundle is that +/// declaring it is then safe. +/// +/// [`Scope::User`] because that is where an adapter is declared from. A bundle +/// that contributes an `[[mcp]]` may not be named by a project-scoped `io.toml` +/// at all, and validating under a scope the adapter will never be declared from +/// would refuse manifests that work. +/// +/// # Errors +/// +/// 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. +pub fn generate(bundle: &Path, name: &str, into: &Path) -> Result { + let id = normalised(name).ok_or_else(|| { + format!( + "{name:?} is not a usable plugin id, so no adapter is written. An id is 1 to {} \ + characters of `a-z`, `0-9` and `-`, starting with a letter or a digit — it is the \ + word every name this bundle contributes is namespaced by, and io-cli refuses to \ + invent one, because a bundle installed under a name nobody saw in the index cannot \ + be typed by the operator who found it.", + io_harness::MAX_ID, + ) + })?; + + // Canonical, because the manifest is written elsewhere and every path in it + // has to still mean this directory when io-harness joins it. A relative + // `bundle`, or one reached through a symlinked temporary directory, would + // otherwise be resolved against whatever the process's working directory + // happened to be at load time rather than at generation time. + let root = std::fs::canonicalize(bundle).map_err(|e| { + format!( + "{}: {e}. A bundle is a directory in a clone that is already on disk.", + bundle.display() + ) + })?; + + let mut lines = vec![format!("name = {}", spelled(&id))]; + let mut disclosed: Vec = Vec::new(); + + let declared = manifest_at(&root); + if let Some(read) = &declared { + if let Some(said) = &read.description { + lines.push(format!( + "description = {}", + spelled(&expanded(said, &root, "description")?) + )); + } + if let Some(version) = &read.version { + lines.push(format!( + "version = {}", + spelled(&expanded(version, &root, "version")?) + )); + } + if let Some(hooks) = &read.hooks { + // The one kind that is stated and never translated. `Hook`'s own docs + // carry the argument: a Claude hook is a shell string, an event + // io-harness does not have, and a refused substitution, all at once, + // and an approximated hook is a program running on the operator's + // machine that nobody described accurately. + disclosed.push(format!( + "the bundle declares hooks in `{hooks}` — a hook names a program this machine \ + would run, and io shows one rather than translating it", + )); + } + } + + let skills = directory( + &root, + declared.as_ref().and_then(|r| r.skills.as_deref()), + SKILLS_DIR, + ); + if let Some(dir) = &skills { + lines.push(format!("skills = {}", spelled(&shown(dir)))); + } + if let Some(dir) = directory(&root, None, COMMANDS_DIR) { + lines.push(format!("templates = {}", spelled(&shown(&dir)))); + } + + let (agents, said) = agents_in(&root.join(AGENTS_DIR), &root)?; + disclosed.extend(said); + for agent in &agents { + lines.push(format!("\n[[agent]]\n{}", item(agent, "an agent")?)); + } + + for server in servers_in(&root.join(MCP_FILE), &root)? { + lines.push(format!("\n[[mcp]]\n{}", item(&server, "an MCP server")?)); + } + + let text = format!("{}\n", lines.join("\n")); + + // **The backstop, and it is not decoration.** Every value above went through + // `expanded`, which refuses a substitution by the field it was written in — + // this catches the case that check has not been wired to yet. io-harness + // refuses a `${` anywhere in a manifest, in every scope + // (`io-harness-0.71.0/src/plugin.rs:774`), and the refusal takes the whole + // bundle rather than the one key: a single missed value would cost the + // operator every capability the bundle carries, reported against a file they + // did not write. Nothing is written when this fires. + if let Some(found) = substitution(&text) { + return Err(format!( + "the manifest io would write for `{id}` still carries `{found}`, and io-harness \ + refuses a substitution inside a plugin.toml in every scope — the refusal takes the \ + whole bundle, not the one key. Nothing was written.", + )); + } + + crate::home::create(into).map_err(|e| format!("{}: {e}", into.display()))?; + let manifest = into.join(PLUGIN_FILE); + std::fs::write(&manifest, text.as_bytes()) + .map_err(|e| format!("{}: {e}", manifest.display()))?; + + let plugin = match Plugins::inspect(Scope::User, into) { + Ok(plugin) => plugin, + Err(refused) => { + // Removed rather than left for an operator to find: a manifest + // io-harness refuses is one that would be reported on + // `Plugins::dropped` against a path they never chose, and a directory + // holding a broken adapter reads exactly like one holding a working + // adapter until something declares it. + let _ = std::fs::remove_file(&manifest); + return Err(refused.to_string()); + } + }; + + Ok(Adapter { + manifest, + contributes: plugin + .contributions() + .into_iter() + .map(str::to_string) + .collect(), + disclosed, + }) +} + +/// 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. +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. + let Some(at) = inside(root, candidate) else { + continue; + }; + if at.is_dir() { + return Some(std::fs::canonicalize(&at).unwrap_or(at)); + } + } + None +} + +/// `said` resolved against `root`, or `None` where it addresses anything outside. +/// +/// Plain names only. A `..`, an absolute path, a root or a prefix component is not +/// dropped-and-continued but refuses the whole value, because a manifest that +/// names `../../..` has not named a directory in this bundle at all and quietly +/// reading `` instead would be io choosing a directory the author did not. +/// `.` is passed over, since `"./skills/"` is how both foreign formats spell it. +fn inside(root: &Path, said: &str) -> Option { + let mut at = root.to_path_buf(); + for part in Path::new(said).components() { + match part { + std::path::Component::Normal(name) => at.push(name), + std::path::Component::CurDir => {} + _ => return None, + } + } + Some(at) +} + +/// One path, as a manifest spells it. +/// +/// `to_string_lossy` rather than a refusal on a non-UTF-8 path: the value is +/// about to go through [`spelled`], which escapes whatever it is handed, and a +/// bundle unreachable because one byte of its path is not UTF-8 is a worse answer +/// than one whose path was written with the replacement character in it — which +/// io-harness will then report by name when the directory is not there. +fn shown(path: &Path) -> String { + path.to_string_lossy().into_owned() +} + +/// One string value, spelled the way TOML spells it. +/// +/// **The escaping goes through `toml`'s own serializer and never through a format +/// 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. +/// +/// `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 +/// Rust's grammar rather than TOML's: it writes an escaped apostrophe, which TOML +/// rejects outright, and it spells a control character in Rust's own brace form +/// where TOML wants a `u` and four fixed hex digits with no braces at all. Either +/// produces a manifest io-harness refuses, out of a `description` field a +/// stranger filled in. +fn spelled(value: &str) -> String { + toml::Value::String(value.to_string()).to_string() +} + +/// The `key = value` lines of one array-of-tables entry. +/// +/// **The bytes come out of io-harness's own type through serde**, which is +/// `src/import.rs`'s argument for the same call and it holds harder here: +/// [`McpServer`] is `#[serde(flatten)]` over a `#[serde(tag = "transport")]` enum, +/// so the discriminant sits flat beside `id` and a hand-written body that forgot +/// it loads with `missing field transport`. Serializing the value io-harness +/// itself deserializes is what makes "what was written" and "what will be read" +/// one question. +/// +/// Each value is rendered in `toml::Value`'s own inline form, which is why an +/// `env` or a `headers` map comes out as an inline table: an `[[mcp]]` block +/// cannot carry an `[mcp.env]` header after it without that header attaching to +/// the wrong entry. +fn item(value: &impl serde::Serialize, what: &str) -> Result { + let value = + toml::Value::try_from(value).map_err(|e| format!("{what} will not serialise: {e}"))?; + let table = value + .as_table() + .ok_or_else(|| format!("{what} is not a table"))?; + Ok(table + .iter() + .map(|(key, value)| format!("{key} = {value}")) + .collect::>() + .join("\n")) +} + +/// The first `${...}` in `text`, as it is written, or `None`. +fn substitution(text: &str) -> Option { + let at = text.find("${")?; + let rest = &text[at..]; + Some(rest.find('}').map_or(rest, |end| &rest[..=end]).to_string()) +} + +/// `raw` with [`PLUGIN_ROOT`] expanded, or a refusal naming `field`. +/// +/// **One substitution is expandable and every other is a refusal.** +/// `${CLAUDE_PLUGIN_ROOT}` means the bundle's root, which is the directory being +/// 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.71.0/src/plugin.rs:774`), because a +/// bundle is a third party's directory even when the file naming it is the +/// operator's own. +/// +/// A passthrough is not an option and neither is dropping the value. The refusal +/// io-harness makes takes the **whole bundle** — one unexpanded `${` costs the +/// operator every skill, template, agent and server the bundle carries, reported +/// against a file they never wrote. Refusing here costs them the same bundle and +/// names the field, which is the difference between something they can act on and +/// something they cannot. +fn expanded(raw: &str, root: &Path, field: &str) -> Result { + let said = raw.replace(PLUGIN_ROOT, &shown(root)); + match substitution(&said) { + None => Ok(said), + Some(found) => Err(format!( + "{field}: `{found}` is a substitution io cannot expand. `{PLUGIN_ROOT}` means the \ + bundle's own root and is written out; anything else would have io-harness read this \ + machine's environment or its files, or run a program on it, for a directory nobody \ + has agreed to — so it refuses one inside a plugin.toml in every scope, and the \ + refusal takes the whole bundle. Write the value out in the bundle, or declare this \ + server in your own io.toml where a substitution is resolved.", + )), + } +} + +/// Every agent `dir` defines, and one line for every frontmatter key with no slot +/// in an [`AgentDef`]. +/// +/// **What maps is translated and what does not is disclosed.** io-harness's +/// definition carries a name, a role, a model, a step cap, an effort and three +/// narrowing switches; a Claude agent's frontmatter carries a name, a +/// description, a tool list and a colour. One key means the same thing in both — +/// `name`, the word a `spawn_agent` asks for — and the file's **body** is the +/// role, because the markdown after the closing fence already *is* the child's +/// system prompt, which is what `AgentDef::role` is prepended to. +/// +/// Everything else is reported by name rather than approximated, and `model` is +/// the one worth saying out loud: a Claude agent writes `sonnet`, `opus` or +/// `inherit`, which are one vendor's family aliases, while `AgentDef::model` is a +/// model id a provider published. Writing one into the other invents a fact and +/// the run pays for it — a call asking a provider for a model it has never heard +/// of — so it is disclosed, and the definition asks for whatever the run's +/// provider was built with. +/// +/// A directory that is not there yields nothing and is not an error: a bundle +/// with no agents and a bundle whose `agents/` is missing contribute the same +/// nothing, and the manifest simply carries no `[[agent]]`. +fn agents_in(dir: &Path, root: &Path) -> Result<(Vec, Vec), String> { + let Ok(entries) = std::fs::read_dir(dir) else { + return Ok((Vec::new(), Vec::new())); + }; + // Sorted, so a manifest generated twice from one bundle is byte for byte the + // same file: `read_dir` answers in whatever order the filesystem holds, and a + // regenerated adapter that differs from the one before it is a diff an + // operator has to read to find out nothing changed. + let mut files: Vec = entries + .flatten() + .map(|entry| entry.path()) + .filter(|path| { + path.extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("md")) + }) + .collect(); + files.sort(); + + let mut defs = Vec::new(); + let mut disclosed = Vec::new(); + for file in files { + let called = file.file_name().map_or_else( + || file.display().to_string(), + |name| name.to_string_lossy().into_owned(), + ); + let Ok(text) = std::fs::read_to_string(&file) else { + disclosed.push(format!( + "{called} could not be read and contributes no agent" + )); + continue; + }; + let Some((keys, body)) = front_matter(&text) else { + disclosed.push(format!( + "{called} opens with no `---` frontmatter, so it names no agent and contributes \ + none", + )); + continue; + }; + + // The file's own stem where the frontmatter names nothing. An agent + // definition with no name cannot be spawned, and the filename is already + // what a reader of the bundle calls it. + let name = keys + .iter() + .find(|(key, _)| key == "name") + .and_then(|(_, said)| value(Some(said.clone()))) + .or_else(|| { + file.file_stem() + .map(|stem| stem.to_string_lossy().into_owned()) + }) + .filter(|name| !name.is_empty()); + let Some(name) = name else { + disclosed.push(format!( + "{called} yields no agent name and contributes no agent" + )); + continue; + }; + + let mut def = AgentDef::new(name); + // **Not filtered through `plain`, and that is the one place this differs + // from every other value out of a stranger's bundle.** `plain` turns a + // control character into a space because those values are drawn on a + // terminal where the scrollback is the transcript; a role is never drawn, + // it is prepended to a child's system prompt, and flattening its newlines + // would rewrite the prompt the bundle's author wrote. + let role = body.trim(); + if !role.is_empty() { + def = def.with_role(expanded(role, root, &called)?); + } + defs.push(def); + + for (key, _) in keys.iter().filter(|(key, _)| key != "name") { + disclosed.push(format!( + "{called}: `{key}` has no slot in an io-harness agent definition and is not \ + translated", + )); + } + } + Ok((defs, disclosed)) +} + +/// The `---` frontmatter block's keys in file order, and the body after it. +/// +/// **Deliberately not a YAML parser, and this crate does not gain one.** The +/// dependency set is ten names asserted in both directions by +/// `tests/dependencies.rs`, and an eleventh to read one key out of a fenced block +/// would be a parser for a format io-cli neither writes nor validates. +/// +/// What it reads: a file opening with `---` alone on its first line, up to the +/// next `---` alone on a line; a `key: value` at column zero where the key is +/// `A-Za-z0-9_-`; and a line indented under one of those, folded onto the value +/// before it with a single space — which is what a `>` or `|` block scalar looks +/// like, and how every surveyed `description` is written. +/// +/// **What it does not read, stated rather than implied:** nested mappings, +/// sequences (`- item`), flow collections (`[a, b]`), anchors, aliases, tags, +/// comments, multiple documents, and YAML's own quoting and escapes beyond one +/// pair of matching surrounding quotes. +/// +/// A value this reads wrongly cannot become a capability the bundle did not +/// declare, and that is what makes the shortcut safe rather than merely small: +/// the only key acted on is `name`, and every other key is reported by name and +/// translated into nothing at all. +fn front_matter(text: &str) -> Option<(Vec<(String, String)>, &str)> { + let rest = text + .strip_prefix("---\n") + .or_else(|| text.strip_prefix("---\r\n"))?; + let (block, body) = fenced(rest)?; + + let mut keys: Vec<(String, String)> = Vec::new(); + for line in block.lines() { + if line.trim().is_empty() { + continue; + } + if line.starts_with(' ') || line.starts_with('\t') { + if let Some((_, said)) = keys.last_mut() { + if !said.is_empty() { + said.push(' '); + } + said.push_str(line.trim()); + } + continue; + } + let Some((key, said)) = line.split_once(':') else { + continue; + }; + let key = key.trim(); + if key.is_empty() + || !key + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'-') + { + continue; + } + // `>` and `|` open a block scalar whose text is on the indented lines + // below, which the branch above folds onto this value; the chomping + // indicators `-` and `+` may follow either of them. + let said = said.trim().trim_start_matches(['>', '|', '-', '+']).trim(); + keys.push((key.to_string(), unquoted(said).to_string())); + } + Some((keys, body)) +} + +/// The text before the next `---` alone on a line, and the text after that line. +fn fenced(text: &str) -> Option<(&str, &str)> { + let mut at = 0usize; + for line in text.split_inclusive('\n') { + if line.trim_end() == "---" { + return Some((&text[..at], &text[at + line.len()..])); + } + at += line.len(); + } + None +} + +/// One pair of matching surrounding quotes removed, and nothing else. +fn unquoted(said: &str) -> &str { + for quote in ['"', '\''] { + if said.len() >= 2 && said.starts_with(quote) && said.ends_with(quote) { + return &said[1..said.len() - 1]; + } + } + said +} + +/// Every MCP server a `.mcp.json` declares, as io-harness's own type. +/// +/// **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.71.0/src/mcp.rs:289`), 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 +/// contributes and half of it is not what the operator was shown. +/// +/// A file that is not there yields nothing and is not an error, for +/// [`agents_in`]'s reason. A file that is there and is not JSON **is** an error: +/// the bundle says it publishes servers, and answering "none" would be io-cli +/// deciding a malformed file means the same as an absent one. +fn servers_in(file: &Path, root: &Path) -> Result, String> { + let Ok(text) = std::fs::read_to_string(file) else { + return Ok(Vec::new()); + }; + let read: WireServers = serde_json::from_str(&text).map_err(|e| { + format!( + "{}: {e}. The bundle declares MCP servers in a file io cannot read, so nothing was \ + written.", + file.display() + ) + })?; + + let mut out = Vec::new(); + for (id, wire) in read.servers { + let field = |key: &str| format!("`{MCP_FILE}` server `{id}`, key `{key}`"); + // A `type` the file leaves out is stdio, which is what every local server + // relies on: the key is written only for the two remote transports. A + // file that leaves it out and names a `url` means the remote one, and + // reading that as stdio would refuse it for having no `command`. + let kind = match (wire.kind.as_deref(), wire.url.is_some()) { + (Some(said), _) => said.to_string(), + (None, true) => "http".to_string(), + (None, false) => "stdio".to_string(), + }; + let server = match kind.as_str() { + "stdio" => { + let command = wire.command.ok_or_else(|| { + format!( + "`{MCP_FILE}` server `{id}` is a stdio server naming no `command`, so \ + there is no program for io-harness to start and nothing was written. \ + Give it a `command`, or a `type` and a `url` for a remote server." + ) + })?; + let args = wire + .args + .iter() + .map(|arg| expanded(arg, root, &field("args"))) + .collect::, String>>()?; + let mut built = + McpServer::stdio(id.as_str(), expanded(&command, root, &field("command"))?) + .with_args(args); + if let McpTransport::Stdio { env, .. } = &mut built.transport { + for (name, said) in &wire.env { + env.insert( + name.clone(), + expanded(said, root, &field(&format!("env.{name}")))?, + ); + } + } + built + } + "http" | "streamable-http" | "streamableHttp" => { + let url = wire.url.ok_or_else(|| { + format!( + "`{MCP_FILE}` server `{id}` is an HTTP server naming no `url`, so there \ + is nowhere for io-harness to dial and nothing was written." + ) + })?; + let mut built = McpServer::http(id.as_str(), expanded(&url, root, &field("url"))?); + if let McpTransport::Http { headers, .. } = &mut built.transport { + for (name, said) in &wire.headers { + headers.insert( + name.clone(), + expanded(said, root, &field(&format!("headers.{name}")))?, + ); + } + } + built + } + other => { + return Err(format!( + "`{MCP_FILE}` server `{id}` asks for the `{other}` transport, and io-harness \ + speaks stdio and streamable HTTP and nothing else — so no `[[mcp]]` entry \ + can be written for it and nothing was written at all. A bundle installed \ + with one server missing is a capability the operator was shown and did not \ + get. Ask the bundle's author for a streamable-HTTP endpoint, or declare the \ + server yourself in your own io.toml." + )); + } + }; + out.push(server); + } + Ok(out) +} + +/// A `.mcp.json`, as it is written. +#[derive(Deserialize)] +struct WireServers { + /// Sorted by the map, so the generated manifest lists servers in one order + /// whatever order the file wrote them in — see [`agents_in`] for why that + /// matters. + #[serde(rename = "mcpServers", default)] + servers: BTreeMap, +} + +/// One server in it, every field optional and every unknown key ignored. +/// +/// [`WireManifest`]'s rule on the other file, for its reason: reading somebody +/// else's file forgives, and the surveyed files carry `disabled`, `description` +/// and vendor keys this translates nothing from. +#[derive(Deserialize)] +struct WireServer { + #[serde(rename = "type", default)] + kind: Option, + #[serde(default)] + command: Option, + #[serde(default)] + args: Vec, + #[serde(default)] + env: BTreeMap, + #[serde(default)] + url: Option, + #[serde(default)] + headers: BTreeMap, +} + +/// A read value, filtered, bounded, and `None` where it is blank. +/// +/// The same collapse [`crate::marketplace`]'s own reader makes and for the same +/// reason: a key present and empty names a bundle no better than a key that is +/// absent, and one case is easier to draw than two. +fn value(raw: Option) -> Option { + let said = plain(raw?.trim()); + let said = said.trim(); + if said.is_empty() { + return None; + } + Some(bounded(said)) +} + +/// One wire entry turned into an [`Entry`], or the sentence saying why not. +fn read_entry(wire: WireEntry) -> Result { + let name = value(Some(wire.name.clone())) + .ok_or_else(|| format!("an entry named {:?} carries no usable name", wire.name))?; + let source = match wire.source { + WireSource::Local(said) => { + let said = plain(said.trim()); + let said = said.trim(); + if said.is_empty() { + return Err(format!("{name} names an empty source path")); + } + Source::Local(bounded(said)) + } + WireSource::Remote { + url, + path, + reference, + sha, + } => Source::Remote(Remote { + url: value(Some(url)).ok_or_else(|| format!("{name} names an empty url"))?, + path: value(path), + reference: value(reference), + sha: value(sha), + }), + }; + Ok(Entry { + name, + description: value(wire.description), + version: value(wire.version), + source, + }) +} + +/// The index file as it is written. +/// +/// `plugins` defaults so a file carrying only `name` and `owner` reads as an index +/// holding nothing rather than as a file that does not parse — which is the honest +/// answer, and the one a repository mid-edit produces. +#[derive(Deserialize)] +struct WireIndex { + #[serde(default)] + plugins: Vec, +} + +/// One element of `plugins`, whether or not this module can read it. +/// +/// **Untagged, with a fallback that matches any object**, so one entry in a shape +/// nobody anticipated costs that entry and not the whole index. `serde`'s untagged +/// enum tries the variants in order, so [`WireEntry`] is attempted first and +/// [`Unread`] — every field optional — accepts whatever is left. +#[derive(Deserialize)] +#[serde(untagged)] +enum Slot { + Read(WireEntry), + Unread(Unread), +} + +/// An entry this module could not read, kept only for the name to report it by. +#[derive(Deserialize)] +struct Unread { + #[serde(default)] + name: Option, +} + +impl Unread { + /// The line a listing draws for it. + fn said(&self) -> String { + match self.name.as_deref().map(plain) { + Some(name) if !name.trim().is_empty() => { + format!("{} names a source io does not read", bounded(name.trim()),) + } + _ => "an entry with no name names a source io does not read".to_string(), + } + } +} + +/// One readable entry, as it is written. +#[derive(Deserialize)] +struct WireEntry { + name: String, + #[serde(default)] + description: Option, + #[serde(default)] + version: Option, + source: WireSource, +} + +/// The two spellings of `source`. +/// +/// The object arm names no `source` tag of its own on purpose: `url` is what +/// decides the arm, and both `"url"` and `"git-subdir"` carry one. Keying on the +/// tag instead would refuse a third name for a shape this already reads correctly. +#[derive(Deserialize)] +#[serde(untagged)] +enum WireSource { + Local(String), + Remote { + url: String, + #[serde(default)] + path: Option, + #[serde(rename = "ref", default)] + reference: Option, + #[serde(default)] + sha: Option, + }, +} + +/// A bundle manifest, as it is written, in either foreign format. +/// +/// Every field optional and every unknown key ignored. The surveyed manifests +/// carry `author`, `homepage`, `repository`, `license`, `keywords`, `interface`, +/// `lspServers` and more; a reader that refused an unknown key would refuse +/// almost every real file. This is the opposite of the rule for the manifest io +/// **writes**, which io-harness parses under `deny_unknown_fields` — reading +/// somebody else's file forgives, writing one does not. +#[derive(Deserialize)] +struct WireManifest { + #[serde(default)] + name: Option, + #[serde(default)] + description: Option, + #[serde(default)] + version: Option, + #[serde(default)] + skills: Option, + #[serde(default)] + hooks: Option, +} + +/// A hooks file, as it is written. +#[derive(Deserialize)] +struct WireHooks { + #[serde(default)] + hooks: std::collections::BTreeMap>, +} + +/// One matcher group under an event. +#[derive(Deserialize)] +struct WireMatcher { + #[serde(default)] + hooks: Vec, +} + +/// One hook inside a matcher group. +#[derive(Deserialize)] +struct WireHook { + #[serde(default)] + command: Option, +} diff --git a/src/contract.rs b/src/contract.rs index 509aaee..e167268 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -515,7 +515,7 @@ pub fn skills_dir(config: &Config, capabilities: &Capabilities, root: PathBuf) - resolve_skills(contract).skills } -/// `~/.io-cli/skills`, when there is something there to discover. +/// `skills/` under the home in force, when there is something there to discover. /// /// **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 @@ -524,10 +524,19 @@ pub fn skills_dir(config: &Config, capabilities: &Capabilities, root: PathBuf) - /// 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. -/// Filtered, an operator with no `~/.io-cli/skills` gets a contract with no -/// skills directory, which is exactly the contract they got before this release. +/// Filtered, an operator with no such directory gets a contract with no skills +/// directory, which is exactly the contract they got before this release. +/// +/// **The home is [`crate::home::in_force`]'s, not [`crate::home::path`]'s.** An +/// operator who set `$IO_CONFIG` or `$IO_CONFIG_HOME` moved the directory their +/// own authored content lives in, and a skill is authored content — a file that +/// person wrote — in the same class as the memory note `crate::memory` anchors on +/// the same home for the same reason (`src/memory.rs`), not a cache io-cli +/// fetched on their behalf. Reading io-cli's default home instead would offer a +/// directory beside a configuration file the session is not using and leave the +/// skills sitting beside the one it is unread. fn default_skills() -> Option { - let dir = crate::home::path()?.join("skills"); + let dir = crate::home::authored()?.join("skills"); dir.is_dir().then_some(dir) } @@ -640,7 +649,8 @@ pub fn server_notices(config: &Config, caps: &Capabilities) -> Vec { /// /// **0.15.0 adds one thing an operator did not ask for, and it is conditional on /// the operator's own disk rather than on their file.** A contract that no key -/// named a skills directory for carries `~/.io-cli/skills` — but only where that +/// named a skills directory for carries the `skills/` directory under the home in +/// force — but only where that /// directory exists, because `Skills::discover` fails the run on one that does /// not — `Error::Config("skills directory … does not exist")`, propagated by /// `TaskContract::discover_skills` before the first completion. An operator who @@ -697,6 +707,6 @@ pub fn session( // applied by `Config::apply_to` inside [`configured`] and `[app.io-cli] // skills` two lines up, so this is where a `~` either of them carries becomes // a home directory and where a session that named neither picks up - // `~/.io-cli/skills`. + // the `skills/` directory under the home in force. resolve_skills(contract) } diff --git a/src/fetch.rs b/src/fetch.rs index 256274b..6631410 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -88,7 +88,7 @@ pub const PROGRAM: &str = "git"; /// been typed by somebody is how `ext::sh -c …` becomes a remote shell, and /// [`resolve`] refuses everything that is not two ordinary path segments precisely /// so that this string is the only host that can ever be reached. -const HOST: &str = "https://github.com/"; +pub const HOST: &str = "https://github.com/"; /// The variable that stops git asking for a credential on a terminal io-cli owns. /// @@ -353,6 +353,164 @@ pub fn argv(url: &str, into: &Path) -> Vec { ] } +/// Where in a repository's history a fetch should land. +/// +/// A marketplace itself is always [`Pin::Head`]: the operator asked for a +/// repository by name and there is nothing to pin them to. A bundle an index +/// places in another repository is whatever that index named, and the official +/// index names a `sha` on all 238 of its remote entries. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Pin { + /// The default branch, shallow. What a marketplace add has always done. + Head, + /// A tag or branch name, shallow. One invocation, because `git clone` takes + /// `--branch`. + Ref(String), + /// A commit. **Not one invocation**, and that is a property of git rather + /// than a choice: `git clone --revision` landed in 2.49, the release machine + /// runs 2.41, and io-cli states no git floor. See [`steps`]. + Commit(String), +} + +impl Pin { + /// What an index named, or `None` where what it named is not usable. + /// + /// **A `sha` and a `ref` are argv elements out of a stranger's file, so each + /// is judged before it can become one.** A value beginning with `-` becomes an + /// *option* to a program that parses options, whatever the argv looked like + /// when it left here — the same reason [`resolve`] refuses such a segment, and + /// the reason this returns `None` rather than trimming anything. + /// + /// A directory name that says which pin this is. + /// + /// **A fetched repository is keyed on it**, because `clone_at` answers + /// `Already` on an existing destination before it looks at the pin and nothing + /// records which commit a directory holds. Two index entries naming one + /// repository at two commits are two directories, so the second cannot be + /// handed the first one's tree. + /// + /// Safe as a path component by construction rather than by filtering: a commit + /// is hex, a ref has already passed [`segment`], and the unpinned case is a + /// constant. Nothing an index wrote reaches this unjudged. + #[must_use] + pub fn spelling(&self) -> String { + match self { + Self::Head => String::from("head"), + Self::Ref(reference) => { + let mut out = String::from("ref-"); + out.push_str(reference); + out + } + Self::Commit(commit) => commit.clone(), + } + } + + /// A commit is preferred over a tag where an entry carries both, because a tag + /// is a name its author can move and a commit is not, and an index that names + /// both has named the commit it means. + #[must_use] + pub fn named(reference: Option<&str>, commit: Option<&str>) -> Option { + if let Some(commit) = commit { + return commit_like(commit).then(|| Self::Commit(commit.to_string())); + } + match reference { + Some(reference) => segment(reference).then(|| Self::Ref(reference.to_string())), + None => Some(Self::Head), + } + } +} + +/// Whether `text` is a commit this module will put in an argv. +/// +/// Hex and nothing else, between an abbreviation git will still resolve and the +/// longest hash it produces. Stated as what is allowed, the direction [`segment`] +/// is stated in and for its reason. +fn commit_like(text: &str) -> bool { + text.len() >= 7 && text.len() <= 64 && text.chars().all(|glyph| glyph.is_ascii_hexdigit()) +} + +/// The `/` an index entry's clone URL points at, on the one host. +/// +/// **The URL a stranger wrote is never the URL that reaches git.** `HOST`'s own +/// documentation states the ceiling this keeps: the only string that can reach the +/// program is one built here out of that constant, because an argument somebody +/// else chose is how `ext::sh -c …` becomes a remote shell. So an entry's `url` is +/// taken apart, judged by [`resolve`]'s rules like any other name, and thrown +/// away — [`url`] rebuilds it. Anything not on that host, or not exactly two +/// ordinary path segments, is `None`, and the caller lists it with its reason +/// rather than dropping it. +/// +/// The ceiling costs nothing that exists: all 238 remote entries of +/// `anthropics/claude-plugins-official` and all ten of +/// `obra/superpowers-marketplace` are two segments on this host. +#[must_use] +pub fn from_url(text: &str) -> Option { + resolve(text.trim().strip_prefix(HOST)?) +} + +/// Every argv a fetch at `at` needs, in order, each already complete. +/// +/// **One list for one invocation and four for a commit, and the length is the +/// honest part.** `git clone --revision ` would make the pinned case a single +/// call, and it is not available: the option landed in git 2.49 and this product +/// names no git floor, so the portable route is an empty repository, a remote, a +/// shallow fetch of the one commit, and a detached checkout of what came back. +/// GitHub serves a fetch of a reachable commit, which is what makes the third step +/// work at depth one. +/// +/// Every element is owned and handed through whole, exactly as [`argv`]'s are: +/// nothing here is quoted, escaped, joined, or spliced into a line for something +/// else to take apart again. `FETCH_HEAD` rather than the commit is what the +/// checkout names, so the only place the commit appears is the fetch that asked +/// for it. +#[must_use] +pub fn steps(url: &str, into: &Path, at: &Pin) -> Vec> { + let dir = into.as_os_str().to_os_string(); + match at { + Pin::Head => vec![argv(url, into)], + Pin::Ref(reference) => vec![vec![ + OsString::from("clone"), + OsString::from("--depth"), + OsString::from("1"), + OsString::from("--branch"), + OsString::from(reference), + OsString::from(url), + dir, + ]], + Pin::Commit(commit) => vec![ + vec![ + OsString::from("init"), + OsString::from("--quiet"), + dir.clone(), + ], + vec![ + OsString::from("-C"), + dir.clone(), + OsString::from("remote"), + OsString::from("add"), + OsString::from("origin"), + OsString::from(url), + ], + vec![ + OsString::from("-C"), + dir.clone(), + OsString::from("fetch"), + OsString::from("--depth"), + OsString::from("1"), + OsString::from("origin"), + OsString::from(commit), + ], + vec![ + OsString::from("-C"), + dir, + OsString::from("checkout"), + OsString::from("--detach"), + OsString::from("FETCH_HEAD"), + ], + ], + } +} + /// Clone `url` into `into`, assembling it at `staging` first. /// /// Both paths are the caller's, which is what lets `tests/fetch.rs` drive every @@ -367,6 +525,36 @@ pub fn argv(url: &str, into: &Path) -> Vec { /// again afterwards whatever happened, so the only thing that can survive this /// call is a complete tree at `into`. pub fn clone(url: &str, into: &Path, staging: &Path) -> Fetched { + clone_at(url, into, staging, &Pin::Head) +} + +/// One spawn of [`PROGRAM`], with the streams the viewport depends on. +/// +/// **The crate's only spawn site, and it takes a finished argv.** Every element +/// has already been built by [`steps`], which is pure and asserted; this function +/// adds nothing to the list and cannot, because it never sees the parts. All three +/// streams are spelled out even though `output()` already wires them this way — the +/// wiring is the property that keeps the viewport safe, the child gets no tty and +/// writes nothing to the screen, and a reader checking that should not have to know +/// a default. +fn run(argv: Vec) -> std::io::Result { + Command::new(PROGRAM) + .args(argv) + .env(NO_PROMPT, "0") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() +} + +/// Clone `url` into `into` at `at`, assembling it at `staging` first. +/// +/// [`clone`]'s general form. The staging-and-rename discipline is the same and is +/// the reason a pinned fetch is safe to build out of four invocations: nothing +/// reaches `into` until every step has succeeded, so a process killed between the +/// fetch and the checkout leaves a directory under `staging` that the next run +/// removes, never a half-pinned tree under a path the listing walks. +pub fn clone_at(url: &str, into: &Path, staging: &Path, at: &Pin) -> Fetched { if into.exists() { return Fetched::Already(into.to_path_buf()); } @@ -382,21 +570,32 @@ pub fn clone(url: &str, into: &Path, staging: &Path) -> Fetched { // fails for a reason that has nothing to do with the marketplace. let _ = std::fs::remove_dir_all(staging); - // All three streams are spelled out even though `output()` already wires them - // this way. The wiring is the property that keeps the viewport safe — the - // child gets no tty and writes nothing to the screen — and a reader checking - // that should not have to know a default. - let output = Command::new(PROGRAM) - .args(argv(url, staging)) - .env(NO_PROMPT, "0") - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .output(); + // **The first step that fails ends the fetch.** A pinned checkout is four + // invocations and each depends on the one before it, so carrying on after a + // failure would run `checkout` against a repository that has fetched nothing + // and report the second, less useful error. `Pin::Head` is one step and this + // loop is exactly what the code above it always did. + let mut failure = None; + for argv in steps(url, staging, at) { + match run(argv) { + Err(error) => { + failure = Some(Fetched::unstartable(&error)); + break; + } + Ok(output) if output.status.success() => {} + Ok(output) => { + failure = Some(Fetched::Failed { + status: output.status.code(), + stderr: readable(&output.stderr), + }); + break; + } + } + } - let fetched = match output { - Err(error) => Fetched::unstartable(&error), - Ok(output) if output.status.success() => match settle(staging, into) { + let fetched = match failure { + Some(failure) => failure, + None => match settle(staging, into) { Ok(()) => Fetched::Cloned(into.to_path_buf()), // A clone that worked and a move that did not is still a failure with // nothing at the destination, which is the only shape this module @@ -406,10 +605,6 @@ pub fn clone(url: &str, into: &Path, staging: &Path) -> Fetched { stderr: error.to_string(), }, }, - Ok(output) => Fetched::Failed { - status: output.status.code(), - stderr: readable(&output.stderr), - }, }; // Unconditional, and a no-op on the path that succeeded — the rename already diff --git a/src/home.rs b/src/home.rs index ba5e631..ab5258a 100644 --- a/src/home.rs +++ b/src/home.rs @@ -54,6 +54,25 @@ const SKILLS: &str = "skills"; /// document. const MARKETPLACES: &str = "marketplaces"; +/// Where the `plugin.toml` io writes for a foreign bundle is kept. +/// +/// A bundle in the field is a Claude Code or a Codex plugin and carries no +/// `plugin.toml` at all; [`crate::adapt::generate`] writes one that io-harness +/// loads, pointing at the clone. **The generated file is io's own and is never +/// written inside the clone** — a marketplace is a stranger's checkout and +/// `src/marketplace.rs` keeps it untouched — so it needs a directory of its own, +/// and this is it. +/// +/// Three levels under it, `//`, which is [`MARKETPLACES`]'s +/// own two-level layout with the bundle's own name under it: one clone publishes +/// many bundles, and a `plugin.toml` is recognised by sitting at a directory's +/// root, so two bundles cannot share one. +/// +/// Not created by [`adopt`], for [`STAGING`]'s reason: nothing is here until a +/// bundle has been adapted, and a home carrying an empty `adapters` would be one +/// more directory for an operator to wonder about. +const ADAPTERS: &str = "adapters"; + /// Where a clone is assembled before it becomes a marketplace. /// /// **Dot-named, and deliberately outside [`MARKETPLACES`].** What is in here is by @@ -206,12 +225,13 @@ pub fn path() -> Option { /// Where marketplaces are cloned to, whether or not the directory exists yet. /// -/// Derived from [`path`] rather than from [`in_force`], the way -/// [`crate::contract::skills_dir`] derives the default skills directory -/// (`src/contract.rs:528`): a marketplace is io-cli's own cache of other people's -/// repositories, and an operator who pointed `$IO_CONFIG_HOME` somewhere else -/// moved their *configuration*. Following that variable here would put the clones -/// wherever the configuration is and leave the ones already fetched invisible. +/// Derived from [`path`] rather than from [`in_force`], which is the opposite of +/// what the default skills directory does: a marketplace is io-cli's own cache of +/// other people's repositories, not something the operator wrote, and authored +/// content follows the home in force while a cache stays with the crate that +/// filled it. An operator who pointed `$IO_CONFIG_HOME` somewhere else moved their +/// *configuration*; following that variable here would put the clones wherever the +/// configuration is and leave the ones already fetched invisible. /// /// **The directory is not promised to exist.** [`adopt`] returns `None` without /// creating anything whenever either variable is set, so every caller creates it @@ -221,6 +241,28 @@ pub fn marketplaces() -> Option { Some(path()?.join(MARKETPLACES)) } +/// Where a foreign bundle's generated `plugin.toml` is written, whether or not the +/// directory exists yet. +/// +/// Derived from [`path`] rather than from [`in_force`], for [`marketplaces`]'s +/// reason and it is the same reason one level on: an adapter is **io's own +/// generated file** — a translation of a clone that this crate writes, that +/// nobody authors and that is regenerated rather than edited — so it belongs with +/// the crate's own home the way a cache does, not with the configuration the +/// operator moved. An operator who pointed `$IO_CONFIG_HOME` somewhere else moved +/// their *configuration*; following that variable here would put the adapters +/// wherever the configuration is and leave every one already generated invisible, +/// while the `[[plugin]]` entries naming them still pointed at the old path. +/// +/// **The directory is not promised to exist**, exactly as [`marketplaces`] is +/// not: [`adopt`] creates nothing at all when the operator has named their own +/// location, so every caller makes it rather than assuming — which +/// [`crate::adapt::generate`] does on the way past. +#[must_use] +pub fn adapters() -> Option { + Some(path()?.join(ADAPTERS)) +} + /// Where a clone is assembled before it is renamed into [`marketplaces`]. /// /// Same home, so the rename that finishes a fetch is a same-filesystem rename with @@ -275,6 +317,27 @@ pub fn origin() -> Origin { /// [`io_harness::config::user_path`] rather than from [`path`]: under `$IO_CONFIG` /// the file is somewhere io-cli did not choose, and a row reporting the home this /// crate *would* have picked would be wrong in exactly the case the row exists for. +/// The directory an operator's **own** content lives in. +/// +/// **One answer for reading and for writing, and 0.31.0 exists partly because +/// they were two.** A skill is something the operator wrote, like a memory note: +/// it belongs beside the configuration they are actually using, not beside the one +/// io-cli would have picked. But a directory that only the *read* followed would +/// be worse than either — `/skills add` would write where nothing looks, and an +/// operator who set `$IO_CONFIG_HOME` would watch skills they had installed stop +/// reaching the model with nothing said. So every surface that reads or writes +/// authored content asks this, and there is no second resolution to disagree with. +/// +/// [`in_force`] with [`path`] as the fallback, because `user_path` answers `None` +/// on a machine with no home at all and io-cli's own default is still the better +/// guess than nothing. Deliberately **not** what [`marketplaces`] or [`adapters`] +/// use: those hold other people's repositories and io's own generated files, which +/// are a cache and belong with the crate. +#[must_use] +pub fn authored() -> Option { + in_force().map(|(dir, _)| dir).or_else(path) +} + #[must_use] pub fn in_force() -> Option<(PathBuf, Origin)> { let dir = io_harness::config::user_path()?.parent()?.to_path_buf(); diff --git a/src/lib.rs b/src/lib.rs index 6fc39b1..8f30cc2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,6 +57,7 @@ //! //! [io-harness]: https://docs.rs/io-harness +pub mod adapt; pub mod app; pub mod approval; pub mod attach; diff --git a/src/main.rs b/src/main.rs index 1f0628a..161abd7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -107,7 +107,7 @@ async fn run(report: &mut Vec) -> Result { // says so below, and `/skills` lists the directory in force rather than this // one. if adopted.is_some() { - if let Some(home) = io_cli::home::path() { + if let Some(home) = io_cli::home::authored() { report.extend(io_cli::skills::install(&home)); } } @@ -414,7 +414,7 @@ async fn drive( // makes every row read as the operator's, which is the safe direction. It // never reaches a path that is joined to, so this cannot resolve anything // against the working directory. - let home = io_cli::home::path().unwrap_or_default(); + let home = io_cli::home::authored().unwrap_or_default(); let bundles = bundle_skills(&config); let (skills, complaint) = commands::skills(&home, skills_dir.as_deref(), &bundles); if let Some(complaint) = complaint { @@ -424,12 +424,22 @@ async fn drive( // read**, said once, here, where the resolved directory is finally known. // Silence would leave an operator with a startup line saying five skills were // installed and a model that has never heard of them. - if let (Some(home), Some(in_force)) = (io_cli::home::path(), skills_dir.as_deref()) { - let ours = io_cli::skills::dir(&home); - if ours.is_dir() && ours != in_force { + // + // **Guarded on the two homes and never on the resolved directory, and 0.31.0 + // is why.** The resolved directory is `None` whenever the one in force does + // not exist yet — which is the ordinary state for an operator who has just + // set `$IO_CONFIG_HOME` — so a guard on it stayed silent in exactly the case + // it was written for: skills sitting in io-cli's default home, unread, with + // nothing said. Comparing the homes themselves fires there. + if let (Some(default), Some(authored)) = (io_cli::home::path(), io_cli::home::authored()) { + let ours = io_cli::skills::dir(&default); + let now = io_cli::skills::dir(&authored); + if ours != now && ours.is_dir() { notices.push(format!( - "the skills in force are in {} — io's own are in {} and are not being read", - in_force.display(), + "skills are read from and written to {} now, and {} still holds some — \ + `$IO_CONFIG` or `$IO_CONFIG_HOME` moved where io keeps what you wrote, \ + and io does not move files you own", + now.display(), ours.display(), )); } @@ -555,7 +565,7 @@ fn skills_view( capabilities: &io_cli::contract::Capabilities, root: &std::path::Path, ) -> io_cli::skillview::View { - let Some(home) = io_cli::home::path() else { + let Some(home) = io_cli::home::authored() else { return io_cli::skillview::View::default(); }; let bundles = bundle_skills(config); @@ -1113,7 +1123,7 @@ async fn loop_over Result>( paint(screen, &mut app)?; if !io_cli::home::import_offered() { - if let Some(home) = io_cli::home::path() { + if let Some(home) = io_cli::home::authored() { let found = io_cli::import::detect( &io_cli::home::expand(std::path::Path::new("~")), session.root(), @@ -1494,7 +1504,7 @@ async fn loop_over Result>( .filter(|(_, on)| **on) .map(|(item, _)| item.clone()) .collect(); - match (chosen.is_empty(), io_cli::home::path()) { + match (chosen.is_empty(), io_cli::home::authored()) { (true, _) => app.record( Tone::Muted, "nothing was imported and nothing was written; \ @@ -2011,7 +2021,12 @@ async fn loop_over Result>( // `index`, for `Pick::PluginRemove`'s reason: the outer // `index` is the row that was chosen and the entry's own // position in the file is a different number entirely. - Pick::PluginInstall { id, scope, edits } => { + Pick::PluginInstall { + id, + scope, + edits, + made, + } => { if io_cli::store::acts(index) { // **The write happens here and nowhere earlier.** // `edits` is `manage::plan`'s own single @@ -2062,6 +2077,17 @@ async fn loop_over Result>( // `Plugins::inspect` validates a directory with // nothing on disk naming it, so there is now nothing // to leave behind and nothing to undo. + // **And the adapter goes with the decline.** For a + // Claude Code or Codex bundle io had to write a + // manifest before io-harness could read the bundle + // at all, so there *is* something to take back + // here even though the operator's configuration + // was never opened. `marketplace::unmake` is the + // one place that decision lives, and `Esc` on this + // same picker calls it too — the standard way out + // of a modal must not be the one that leaves + // something behind. + io_cli::marketplace::unmake(made.as_deref()); app.record( Tone::Muted, format!( @@ -2362,15 +2388,15 @@ async fn loop_over Result>( // than in the confirmation's own arm because the // clone still exists at this point, so the entries // inside it can still be found. - let inside = io_cli::marketplace::dependents( - &io_cli::pluginview::view(&config), - &market.root, - ); // `record` and never `say`: the footer is one slot // and is gone on the next keystroke, and this is a // consequence the operator has to still be able to // read after they have answered. - if let Some(said) = io_cli::marketplace::warning(&inside) { + if let Some(said) = io_cli::marketplace::removal_cost( + &io_cli::pluginview::view(&config), + &market.root, + io_cli::home::adapters().as_deref(), + ) { app.record(Tone::Warning, said); } descended = Some(( @@ -2543,7 +2569,7 @@ async fn loop_over Result>( // under it. Leave it and `/` goes on // offering a skill whose file is gone. skills = io_cli::commands::skills( - &io_cli::home::path().unwrap_or_default(), + &io_cli::home::authored().unwrap_or_default(), io_cli::contract::skills_dir( &config, &capabilities, @@ -2631,7 +2657,7 @@ async fn loop_over Result>( // harness refuses by name — or hides one // that has just come back. skills = io_cli::commands::skills( - &io_cli::home::path().unwrap_or_default(), + &io_cli::home::authored().unwrap_or_default(), io_cli::contract::skills_dir( &config, &capabilities, @@ -4059,7 +4085,21 @@ async fn loop_over Result>( // case that asked for it. picker = descended; } - Outcome::Cancelled => picker = None, + Outcome::Cancelled => { + // **`Esc` is a decline, and one picker has something to take + // back.** An install disclosure is drawn over an adapter io + // already wrote — io-harness has no loader that takes a + // foreign manifest, so there is nothing to disclose until the + // generated one exists. The "leave it" row removes it; leaving + // `Esc` out would make the standard way out of every other + // modal in this product the one that leaves a directory nobody + // agreed to under the operator's home, listed by no surface + // and removed by no verb. + if let Some((_, Pick::PluginInstall { made, .. })) = &picker { + io_cli::marketplace::unmake(made.as_deref()); + } + picker = None; + } Outcome::Idle => {} } paint_picker(screen, &mut app, picker.as_mut())?; @@ -4357,11 +4397,11 @@ async fn loop_over Result>( // `Pick::Marketplace`, which asks the same question // before it offers the same removal. if let Some(clone) = io_cli::fetch::at(named) { - let inside = io_cli::marketplace::dependents( + if let Some(warned) = io_cli::marketplace::removal_cost( &io_cli::pluginview::view(&config), &clone, - ); - if let Some(warned) = io_cli::marketplace::warning(&inside) { + io_cli::home::adapters().as_deref(), + ) { app.record(Tone::Warning, warned); } } @@ -4378,7 +4418,7 @@ async fn loop_over Result>( // documents as working. The acts live in `manage_main`, // which a session never calls. Ok((io_cli::manage::Request::Skill(verb), None)) => { - match io_cli::home::path() { + match io_cli::home::authored() { None => app.record( Tone::Refused, "there is no home on this machine to keep skills in" @@ -4522,6 +4562,17 @@ async fn loop_over Result>( for line in disclosure.said(&app.theme.glyphs) { app.record(Tone::Muted, line); } + // **Named individually, with the command unshortened.** + // A hook in a Claude Code or Codex bundle cannot cross + // — see `marketplace::NOT_CARRIED` for the three + // reasons — and this is the surface where an operator + // would otherwise reasonably assume it does. `Refused` + // rather than `Muted`, because the rest of the list is + // what the bundle *will* do and these are what it will + // not. + for line in &disclosure.withheld { + app.record(Tone::Refused, line.clone()); + } picker = Some(( Picker::new( format!("Install {}?", disclosure.id), @@ -4545,6 +4596,7 @@ async fn loop_over Result>( id: disclosure.id.clone(), scope: plan.scope, edits: plan.edits.clone(), + made: plan.made.clone(), }, )); } @@ -4972,7 +5024,7 @@ async fn loop_over Result>( // `Esc` has still *seen* what io found. The picker then turns // items on one at a time; nothing is written until the last row is // chosen, which is why a cancelled import is never a partial one. - Action::Import => match io_cli::home::path() { + Action::Import => match io_cli::home::authored() { None => app.record( Tone::Refused, "io has no home directory of its own, so there is nowhere to import into" @@ -6345,7 +6397,7 @@ async fn loop_over Result>( // rejected once — a line that repeats for the life of the // file it is about stops being read. skills = commands::skills( - &io_cli::home::path().unwrap_or_default(), + &io_cli::home::authored().unwrap_or_default(), io_cli::contract::skills_dir( &config, &capabilities, @@ -8656,6 +8708,15 @@ enum Pick { id: String, scope: io_harness::config::Scope, edits: Vec, + /// The adapter directory io generated so that io-harness could read this + /// bundle at all, removed when the operator declines. + /// + /// `None` for a native bundle, which needed nothing generated. A Claude + /// Code or Codex bundle has no manifest io-harness reads, so there is + /// literally nothing to disclose until io has written one — the file is + /// io's own, in io's own home, and the operator's configuration is + /// untouched either way. + made: Option, }, /// The second half of removing a bundle: which entry, and are you sure. /// @@ -8895,7 +8956,7 @@ async fn manage_main( // the acts live in `skillview`, which the keystroke calls too, and neither // door words an outcome for itself. io_cli::manage::Request::Skill(verb) => { - let Some(home) = io_cli::home::path() else { + let Some(home) = io_cli::home::authored() else { return Err( "there is no home on this machine to keep skills in; set `IO_CONFIG_HOME`" .to_string(), @@ -8981,6 +9042,14 @@ async fn manage_main( "{} — io-harness read, parsed and trust-checked this bundle. It contributes:", disclosure.id, ); + // The hooks that will not cross, named individually and unshortened, + // before the lines saying what the bundle *will* do. On this door the + // typed command is the consent, so this is the operator's only chance to + // read it — and a bundle installed for its hooks with its hooks silently + // absent is the one outcome this release must not produce. + for line in &disclosure.withheld { + eprintln!("{line}"); + } for line in disclosure.said(&io_cli::glyphs::ASCII) { eprintln!("{line}"); } @@ -9065,9 +9134,11 @@ fn marketplace_main( // never touched — that is F3 — so this is the only place the operator // is told what stops loading. if let Some(clone) = io_cli::fetch::at(named) { - let inside = - io_cli::marketplace::dependents(&io_cli::pluginview::view(config), &clone); - if let Some(warned) = io_cli::marketplace::warning(&inside) { + if let Some(warned) = io_cli::marketplace::removal_cost( + &io_cli::pluginview::view(config), + &clone, + io_cli::home::adapters().as_deref(), + ) { eprintln!("{warned}"); } } diff --git a/src/manage.rs b/src/manage.rs index 1ee3663..1dd8ac4 100644 --- a/src/manage.rs +++ b/src/manage.rs @@ -249,6 +249,17 @@ pub enum ConfigVerb { /// one, and that is a property worth keeping rather than a shape worth widening. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Plan { + /// A directory io created while building this plan, which a **declined** + /// install must take back. + /// + /// `Some` for exactly one request: `plugin add ` resolving to a Claude + /// Code or Codex bundle, where the generated manifest has to exist before + /// io-harness can read the bundle and there is therefore something to disclose + /// at all. The operator's configuration is never opened either way — this is + /// io's own file, in io's own home — and [`declined`] is the one place it is + /// removed, so two doors cannot disagree about whether saying no leaves + /// something behind. + pub made: Option, /// The scope whose file is written. pub scope: Scope, /// What to write, applied together or not at all. @@ -667,6 +678,7 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { scope: *scope, edits: vec![crate::servers::add(server)], disclosure: None, + made: None, }, Request::Mcp(McpVerb::Edit { id, key, value }) => { let at = declared_server(root, id)?; @@ -685,6 +697,7 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { scope: at.scope, edits: vec![edit], disclosure: None, + made: None, } } Request::Mcp(McpVerb::Remove { id }) => { @@ -693,6 +706,7 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { scope: at.scope, edits: vec![crate::servers::remove(&at)], disclosure: None, + made: None, } } // **The entry is found and then edited, never removed and re-added.** @@ -711,10 +725,21 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { // // `display()` gives back the word as typed: `path` is a `PathBuf` made // out of one token and is never rendered from components. + // **The three homes an install may need, resolved here because this is + // the door.** A remote entry is cloned, and a Claude Code or Codex + // bundle gets a manifest generated for it; neither directory exists + // when the word is typed. `marketplace::chosen` takes them rather than + // reading `crate::home` itself, which is that module's standing rule. + let homes = crate::marketplace::Homes { + marketplaces: &crate::home::marketplaces().ok_or(crate::marketplace::NOWHERE)?, + staging: &crate::home::staging().ok_or(crate::marketplace::NOWHERE)?, + adapters: &crate::home::adapters().ok_or(crate::marketplace::NOWHERE)?, + }; let chosen = crate::marketplace::chosen( &resolve(root, path), || crate::marketplace::installed().unwrap_or_default(), &path.display().to_string(), + homes, )?; let written = crate::pluginview::declared(root, chosen.dir()); // **Which reading won decides whether anything is disclosed, and that @@ -733,11 +758,26 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { // user-scope file and is refused whole in a committed one, and an // install that inspected at the wrong scope would disclose a bundle // that then dropped, or refuse one that would have loaded. + // + // **`adapted_disclosure` and not `disclosure`, and the second argument + // is the point.** For an adapted bundle the directory io-harness is + // asked to load is the generated manifest's, which carries no hooks — + // they do not cross. The author's own directory is where the hooks it + // declares are, and naming them is the whole of what io owes an + // operator who would otherwise reasonably assume they run. let disclosure = chosen .discloses() - .then(|| crate::marketplace::disclosure(*scope, chosen.dir())) - .transpose()?; + .then(|| { + crate::marketplace::adapted_disclosure( + *scope, + chosen.dir(), + (chosen.from() != chosen.dir()).then(|| chosen.from()), + ) + }) + .transpose() + .inspect_err(|_| crate::marketplace::unmake(chosen.made()))?; Plan { + made: chosen.made().map(Path::to_path_buf), scope: *scope, // One entry, switched on, written once. Through 0.29.0 a // marketplace bundle was written `enabled = false` first and @@ -762,12 +802,14 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { scope, edits: vec![crate::pluginview::remove(index)], disclosure: None, + made: None, } } Request::Config(ConfigVerb::Set { key, value, scope }) => Plan { scope: decided_scope(root, key, *scope), edits: vec![Edit::set(key.clone(), value.clone())], disclosure: None, + made: None, }, Request::Config(ConfigVerb::Unset { key, scope }) => Plan { scope: decided_scope(root, key, *scope), @@ -779,6 +821,7 @@ pub fn plan(root: &Path, request: &Request) -> Result, String> { // operator's entire block. See `Edit::unset`. edits: vec![Edit::unset(key.clone())], disclosure: None, + made: None, }, // **A marketplace verb plans no write, and it is here beside the reads // rather than given a shape of its own.** `add` and `remove` are not @@ -824,6 +867,7 @@ fn switched(root: &Path, id: &str, on: bool) -> Result { scope: at.scope, edits: vec![crate::servers::switch(&at, on)], disclosure: None, + made: None, }) } diff --git a/src/marketplace.rs b/src/marketplace.rs index d776f36..64a5692 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -166,12 +166,40 @@ pub const HERE_MARK: &str = crate::pluginview::LOADED_MARK; #[derive(Debug, Clone, PartialEq, Eq)] pub struct Bundle { /// The directory the manifest sits in. + /// + /// For a bundle an index places in **another** repository this is the clone + /// the index itself is in, because the bundle's own directory does not exist + /// until the install fetches it. [`Bundle::source`] is what says which of the + /// two this is, and nothing draws a label off this field for an index bundle — + /// an index always names its entries, so [`Bundle::label`] never reaches the + /// directory case for one. pub dir: PathBuf, /// The manifest's `name`, unquoted, or `None` where it carries none this /// module could read. pub name: Option, /// The manifest's `description`, unquoted, or `None`. pub description: Option, + /// Whether the author wrote a manifest io reads, or io generated one. + pub origin: Origin, + /// Where an index said the bundle is, and `None` for a bundle found by the + /// walk — which is by definition already at [`Bundle::dir`]. + pub source: Option, +} + +/// Whether a bundle's manifest is the author's or io's. +/// +/// **Drawn rather than inferred.** An adapted bundle works through a +/// `plugin.toml` io generated from somebody else's `plugin.json`, and the +/// difference between what an author wrote and what io made of it must never be +/// something an operator has to work out — least of all when a bundle is dropped +/// and the file to look at is the generated one. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Origin { + /// The directory carries a [`MANIFEST`]. Read exactly as it was before io-cli + /// read any foreign format, and it wins its own directory. + Native, + /// Read from a Claude Code or Codex manifest. io writes the `plugin.toml`. + Adapted, } impl Bundle { @@ -203,17 +231,33 @@ impl Bundle { /// the operator has no other way to tell. #[must_use] pub fn line(&self) -> String { + // The file named is the one io actually read, which since 0.31.0 is not + // always a `plugin.toml`. A sentence telling an operator to look in a file + // that is not there is worse than the missing description it is reporting. + let read = self.read_from(); match (&self.name, &self.description) { (Some(_), Some(said)) => said.clone(), - (Some(_), None) => "its plugin.toml carries no description".to_string(), + (Some(_), None) => format!("its {read} carries no description"), (None, Some(said)) => { - let mut out = String::from("its plugin.toml does not name it; "); + let mut out = format!("its {read} does not name it; "); out.push_str(said); out } - (None, None) => { - "its plugin.toml does not name it, and carries no description".to_string() - } + (None, None) => format!("its {read} does not name it, and carries no description"), + } + } + + /// The name of the manifest file this bundle's two keys came out of. + #[must_use] + pub fn read_from(&self) -> &'static str { + // **An index entry's two keys came out of the index**, not out of any + // `plugin.json`, and in the common case that file does not exist at the + // path the sentence would send an operator to. `source` is what says which + // read this was: the walk sets it `None`, `from_entry` sets it `Some`. + match (self.origin, self.source.is_some()) { + (Origin::Native, _) => MANIFEST, + (Origin::Adapted, true) => crate::adapt::INDEX_FILE, + (Origin::Adapted, false) => crate::adapt::MANIFEST_FILE, } } } @@ -251,7 +295,16 @@ impl Market { #[must_use] pub fn held(&self) -> String { match self.bundles.len() { - 0 => format!("no directory in it carries a {MANIFEST}"), + // Three formats are read now, so the sentence names the shape rather + // than one filename. Before 0.31.0 this said "no directory in it + // carries a plugin.toml", which was true and was the answer for every + // marketplace in the field — the release exists because of it. + 0 => format!( + "nothing in it is a bundle — no {MANIFEST}, no {}/{}, no {}", + crate::adapt::CLAUDE_DIR, + crate::adapt::INDEX_FILE, + crate::adapt::MANIFEST_FILE, + ), 1 => "1 bundle".to_string(), many => format!("{many} bundles"), } @@ -288,9 +341,156 @@ pub fn manifest(dir: &Path) -> Option { dir: dir.to_path_buf(), name: declared(&text, "name"), description: declared(&text, "description"), + origin: Origin::Native, + source: None, }) } +/// One directory's foreign manifest as a [`Bundle`], or `None` where it carries +/// none. +/// +/// [`manifest`]'s counterpart for the two formats io-cli did not invent, and it is +/// only ever consulted where that function has already answered `None` — a native +/// `plugin.toml` wins its own directory, which is F2. +#[must_use] +pub fn adapted(dir: &Path) -> Option { + let read = crate::adapt::manifest_at(dir)?; + Some(Bundle { + dir: dir.to_path_buf(), + name: read.name, + description: read.description, + origin: Origin::Adapted, + source: None, + }) +} + +/// One index entry as a [`Bundle`], under the id [`published`] derived for it. +/// +/// The name is the index's, never the directory's, and that is a decision rather +/// than a convenience: the index is the author's own statement of what the +/// repository publishes, it is the word an operator types at `plugin add`, and an +/// entry whose source is another repository has no local directory to take a name +/// from until the install has already happened. +/// +/// **It is the id and not the entry's own spelling**, for [`Bundle::label`]'s +/// reason: the label is the word the listing draws and therefore the word +/// [`locate`] is asked for, and io-harness namespaces the bundle's contributions +/// with the id. An entry spelled `Rust-Review` drawn under that spelling would +/// name a bundle no install could resolve. The two differ only by a case fold — +/// [`crate::adapt::normalised`] refuses anything wider — so the row still reads +/// as the entry an operator found in the index. +fn from_entry(clone: &Path, id: String, entry: crate::adapt::Entry) -> Bundle { + let dir = match &entry.source { + crate::adapt::Source::Local(said) => local_dir(clone, said), + crate::adapt::Source::Remote(_) => clone.to_path_buf(), + }; + Bundle { + dir, + name: Some(id), + description: entry.description, + origin: Origin::Adapted, + source: Some(entry.source), + } +} + +/// The bundles an index publishes, and one line for every entry it does not. +/// +/// **One reader answering both, because [`holdings`] and [`unreadable`] must not +/// disagree about which entry is which.** An entry counted as a bundle by one and +/// reported as a refusal by the other is a row an operator can see, cannot +/// install, and is told nothing about. The two stay separate at the surface — a +/// refusal is not a bundle and a listing must not offer it as one — and share the +/// decision underneath. +/// +/// Two refusals are made here on top of what [`crate::adapt`] itself could not +/// read, and neither is a drop: +/// +/// 1. A name [`crate::adapt::normalised`] cannot make an id is refused **by +/// name**. It joins the reader's own unreadable lines because the rule there +/// already is "reported, never skipped" — an entry that vanished would leave +/// an operator comparing a listing against a file to work out which of the two +/// is lying. +/// 2. Two entries reaching one id are refused **naming both**, in [`locate`]'s +/// words for the same class of problem: one name, two different repositories' +/// code, and whichever was found first is code the operator did not choose. +/// Neither is offered, and that is where this parts from `locate` — `locate` +/// can offer a qualifier that tells two marketplaces apart, and inside one +/// index there is no second spelling to offer. The index's author is the only +/// party who can resolve it. +fn published(clone: &Path, index: crate::adapt::Index) -> (Vec, Vec) { + let mut said = index.unreadable; + let mut kept: Vec<(String, String, Bundle)> = Vec::new(); + for entry in index.entries { + let written = entry.name.clone(); + let Some(id) = crate::adapt::normalised(&written) else { + said.push(format!( + "{written} is not a usable plugin id and io does not invent one — an id is \ + what you type at `plugin add` and what namespaces every name the bundle \ + contributes, so it must be 1 to {} characters of `a-z`, `0-9` and `-`, \ + starting with a letter or a digit", + io_harness::MAX_ID, + )); + continue; + }; + kept.push((id.clone(), written, from_entry(clone, id, entry))); + } + + // The ids more than one entry reached, in the order the index first names + // them, so two reads of one file report in one order. Quadratic over the + // entries and deliberately so: the largest index in the field carries 291 of + // them, and a map keyed by id would still have to be walked in the file's + // order to report in it. + let mut clashing: Vec = Vec::new(); + for (id, _, _) in &kept { + if !clashing.contains(id) && kept.iter().filter(|held| held.0 == *id).count() > 1 { + clashing.push(id.clone()); + } + } + for id in &clashing { + let names: Vec = kept + .iter() + .filter(|held| held.0 == *id) + .map(|held| format!("`{}`", held.1)) + .collect(); + let count = names.len(); + // ` and ` is `locate`'s own joiner for its own list of spellings. A second + // joiner for the same class of sentence is a second phrasing an operator + // has to learn twice. + let spellings = names.join(" and "); + said.push(format!( + "{count} entries in this index are the plugin id `{id}` — {spellings} — and \ + installing whichever was found first would install code you did not choose; \ + none of them is offered until the index names them apart" + )); + } + + let bundles = kept + .into_iter() + .filter(|held| !clashing.contains(&held.0)) + .map(|held| held.2) + .collect(); + (bundles, said) +} + +/// A `"./"` or `"./plugins/x"` source resolved against the clone. +/// +/// **Every component that is not a plain name is dropped**, so a source of +/// `"../../etc"` or an absolute path cannot address anything outside the clone. +/// A marketplace index is written by a stranger and this is the one value in it +/// that becomes a path io reads from; the directory it names is inside the clone +/// or the entry addresses the clone's own root. +fn local_dir(clone: &Path, said: &str) -> PathBuf { + let mut dir = clone.to_path_buf(); + for part in Path::new(said).components() { + if let std::path::Component::Normal(name) = part { + if name != "." { + dir.push(name); + } + } + } + dir +} + /// One top-level key of a manifest, unquoted, filtered and bounded. /// /// [`crate::edit::value_at`] answers in the value's **source bytes**, so a string @@ -331,7 +531,7 @@ const LONGEST: usize = 200; /// Plain dots rather than [`Glyphs::ellipsis`]: this is a property of the data and /// runs before any renderer is chosen, and a value that arrives at both glyph sets /// already carrying a `…` is a value that is wrong in one of them. -fn bounded(value: &str) -> String { +pub(crate) fn bounded(value: &str) -> String { if value.chars().count() <= LONGEST { return value.to_string(); } @@ -450,13 +650,39 @@ fn escapes(text: &str) -> String { /// Every bundle inside a clone, the clone's own root included. /// +/// **Three formats, and the precedence between them is the whole of this +/// function.** +/// +/// 1. A native `plugin.toml` at the clone's own root **suppresses the index**. A +/// repository that has written io's own manifest has said what it publishes in +/// the format this crate owns, and a foreign index must not speak over it. That +/// is what lets the author of a Claude or Codex bundle take back control by +/// adding one file, and [`crate::adapt`]'s generator never writes inside a +/// clone precisely so this stays the author's decision. It suppresses the index +/// and nothing else: the walk still runs, so a repository that carries a root +/// manifest **and** bundles in subdirectories lists all of them exactly as it +/// did before this crate read any foreign format. +/// 2. Otherwise `.claude-plugin/marketplace.json`, where the clone carries one. +/// **The walk does not also run**, and F3 asserts the count rather than the +/// membership for exactly that reason: a union would list bundles the author +/// did not publish beside the ones they did, with no way for an operator to +/// tell which was which. The index is the author's own statement, and it is the +/// file both Claude Code and Codex read. Not every entry becomes a bundle — +/// [`published`] holds back the ones that name no usable plugin id, and +/// [`unreadable`] is where each of those says so by name. +/// 3. Otherwise the walk, with each directory read as a native manifest first and +/// a foreign one only where it carries none. +/// /// **The walk is [`crate::pluginview::candidates`]'s**, not a second one written /// here: it already skips `target`, `node_modules` and every dotted directory — /// `.git`, which every clone has, most of all — and it already orders by depth and /// then by path so two calls on one machine answer the same way. A repository /// laying its bundles out in a way that walk cannot see is a repository `/plugin /// add` could not see either, and one walk that is sometimes too shallow is better -/// than two that disagree about which. +/// than two that disagree about which. Reading `.claude-plugin/plugin.json` at a +/// directory the walk visited does **not** weaken its dotted-directory skip: the +/// path is known and relative to a directory already admitted, and the walk itself +/// still never descends into one. /// /// The root is checked separately because that function only ever looks at a /// directory's *children*, and a marketplace that is itself one bundle — a single @@ -464,12 +690,42 @@ fn escapes(text: &str) -> String { /// entirely. #[must_use] pub fn holdings(clone: &Path) -> Vec { - let mut dirs = Vec::new(); + if !clone.join(MANIFEST).is_file() { + if let Some(index) = crate::adapt::index_at(clone) { + return published(clone, index).0; + } + } + // `visited` and not `candidates`: the latter is this filtered by "carries a + // plugin.toml", and a directory holding only a `.claude-plugin/plugin.json` + // carries none — so iterating `candidates` looking for a foreign manifest + // would find one exactly never. + let mut dirs = vec![clone.to_path_buf()]; + dirs.extend(crate::pluginview::visited(clone)); + dirs.iter() + .filter_map(|dir| manifest(dir).or_else(|| adapted(dir))) + .collect() +} + +/// Every entry of an index that yields no bundle, for the surface that lists it. +/// +/// Three kinds of line, and one list because an operator reading a listing has one +/// question — which entries are not here, and why: an entry [`crate::adapt`] could +/// not read at all, a name that is no plugin id, and a set of names that reach one +/// id. [`published`] writes all three. +/// +/// Separate from [`holdings`] rather than a field on it, because a refusal is not +/// a bundle and a listing that mixed the two would offer a row that cannot be +/// installed. Empty for a clone with no index, which is the same answer as an +/// index that read cleanly — the distinction that matters is whether there is +/// something to report, and there is not. +#[must_use] +pub fn unreadable(clone: &Path) -> Vec { if clone.join(MANIFEST).is_file() { - dirs.push(clone.to_path_buf()); + return Vec::new(); } - dirs.extend(crate::pluginview::candidates(clone)); - dirs.iter().filter_map(|dir| manifest(dir)).collect() + crate::adapt::index_at(clone) + .map(|index| published(clone, index).1) + .unwrap_or_default() } /// Every marketplace under `root`, ordered by owner and then by repository. @@ -524,6 +780,97 @@ fn name_of(path: &Path) -> Option { Some(path.file_name()?.to_string_lossy().into_owned()) } +/// Where a bundle an index placed in another repository is cloned to. +/// +/// **Dot-named, and inside the marketplaces tree on purpose.** [`markets`] skips +/// anything dot-named at both of its two levels, so a repository fetched for one +/// entry can never be counted as a marketplace of its own — which it is not: the +/// operator added one marketplace and this is a directory that marketplace's index +/// pointed at. Inside the tree rather than beside it so that +/// `plugin marketplace remove` takes it away with everything else the marketplace +/// brought down. +const ENTRIES: &str = ".entries"; + +/// Where the repositories one marketplace's index pointed at are kept. +/// +/// `/.entries//` — the marketplace's own name, so +/// [`discard`] removes an index's fetched repositories with the index itself +/// rather than leaving them in a directory no surface lists. Dot-named, so +/// [`markets`] cannot count one as a marketplace the operator added. +fn entries(marketplaces: &Path, market: &Named) -> PathBuf { + at(marketplaces.join(ENTRIES).as_path(), market) +} + +/// The directory a bundle is actually in, fetching it where it is not here yet. +/// +/// **A local source is already on the disk and this does nothing at all.** 53 of +/// the official index's 291 entries are that shape. The other 238 name another +/// repository, and this is where that repository is brought down — through +/// [`crate::fetch`], which owns every spawn, at the commit or the tag the index +/// named, with the URL re-derived from [`crate::fetch::from_url`] so the string +/// reaching git is one io-cli built. +/// +/// The `path` an entry names is joined **component by component, plain names +/// only**, exactly as a local source is: it is a value out of a stranger's file +/// and the directory it addresses is inside the clone or the entry addresses the +/// clone's root. +pub fn fetched( + bundle: &Bundle, + market: &Named, + marketplaces: &Path, + staging: &Path, +) -> Result { + let Some(crate::adapt::Source::Remote(remote)) = &bundle.source else { + return Ok(bundle.dir.clone()); + }; + let named = crate::fetch::from_url(&remote.url).ok_or_else(|| { + let mut out = String::from("io reads a marketplace entry only from "); + out.push_str(crate::fetch::HOST); + out.push_str("/, and "); + out.push_str(&bounded(&plain(&remote.url))); + out.push_str(" is not that, so it is not fetched"); + out + })?; + let pin = crate::fetch::Pin::named(remote.reference.as_deref(), remote.sha.as_deref()) + .ok_or_else(|| { + let mut out = String::from("the index pins "); + out.push_str(&bundle.label()); + out.push_str(" to something io will not put on a command line, so it is not fetched"); + out + })?; + + // **Keyed on the marketplace that named it and on the pin, not on the + // repository alone**, and both halves are defects this would otherwise have. + // + // `clone_at` answers `Already` the instant the destination exists, before the + // pin is consulted, and nothing records which commit a directory holds. Two + // entries naming one repository at two commits — which is ordinary; 85 of the + // official index's entries are `git-subdir` into a handful of repositories — + // would give the second entry the first one's code, adapted from a tree the + // index did not name for it, with the disclosure truthfully describing the + // wrong commit. The pin in the path makes `Already` mean *this* commit. + // + // Under the marketplace's own `/` so that `discard` can take the + // entries away with the clone that named them. Keyed the other way they would + // outlive every removal in a directory nothing lists. + let into = at(&entries(marketplaces, market), &named).join(pin.spelling()); + match crate::fetch::clone_at(&crate::fetch::url(&named), &into, staging, &pin) { + crate::fetch::Fetched::Cloned(dir) | crate::fetch::Fetched::Already(dir) => { + Ok(match &remote.path { + Some(said) => local_dir(&dir, said), + None => dir, + }) + } + // `sentence` answers `None` only for `Cloned`, which the arm above takes, + // so this is the failure's own words and never a default. The fallback is + // written all the same rather than unwrapped: an ending added upstream + // would otherwise turn a refusal into a panic on the consent surface. + other => Err(other + .sentence() + .unwrap_or_else(|| String::from("the fetch did not finish and said nothing"))), + } +} + /// What io-cli says when it has no home of its own to keep a marketplace in. /// /// A constant because two surfaces say it: the act itself, and the panel that has @@ -661,6 +1008,14 @@ pub fn discard(root: &Path, named: &Named) -> Outcome { said, }; } + // **The repositories this marketplace's index pointed at go with it.** They + // were fetched only because this index named them, they sit under + // `.entries//` keyed on this marketplace for exactly that reason, + // and nothing lists them — a removal that left them behind would leave clones + // on the disk the operator has no surface to find or delete. Best effort and + // before the clone, so the sentence below is not written unless the clone + // itself went. + let _ = std::fs::remove_dir_all(entries(root, named)); match std::fs::remove_dir_all(&clone) { Ok(()) => { let mut said = String::from("the clone is gone: "); @@ -715,6 +1070,57 @@ pub fn dependents(view: &crate::pluginview::View, clone: &Path) -> Vec .collect() } +/// The adapter directories that stop working when `clone` goes. +/// +/// **[`dependents`] cannot see these, and that is a property of the design rather +/// than an oversight.** A bundle installed from a `plugin.toml` is declared at a +/// path *inside* the clone, so removing the clone is visibly removing it. A bundle +/// installed from a Claude Code or Codex manifest is declared at a path under +/// `~/.io-cli/adapters`, which is not inside the clone at all — the generated +/// manifest merely *points* there. Removing the marketplace would leave that entry +/// declared, loading a manifest whose every path has just been deleted, and +/// `dependents` would have said nothing. +/// +/// So the link is read where it actually is: the generated manifest's own +/// `skills` and `templates` values, through [`crate::edit::value_at`], which is +/// this crate's only permitted TOML reader. An adapter naming a path inside the +/// clone is an adapter this removal orphans. +/// +/// **The `[[plugin]]` entry is not touched and neither is the adapter directory.** +/// 0.29.0's rule is that a cache being emptied does not undo a configuration +/// decision, and an adapter is io's own file rather than the operator's — but it +/// is still not this verb's to delete, because the entry that names it survives. +/// What is owed is the consequence, said before anything goes. +#[must_use] +pub fn orphaned(view: &crate::pluginview::View, clone: &Path, adapters: &Path) -> Vec { + let real = resolved(clone); + let under = resolved(adapters); + view.plugins + .iter() + .map(|listed| listed.root.clone()) + .chain(view.refused.iter().map(|refused| refused.path.clone())) + .filter(|root| resolved(root).starts_with(&under)) + .filter(|root| points_into(root, &real)) + .collect() +} + +/// Whether the adapter at `root` names a path inside `clone`. +/// +/// Both of the generated manifest's directory keys are checked, not one: a bundle +/// publishing only `commands/` has a `templates` and no `skills`, and a rule that +/// read `skills` alone would call that adapter unaffected by a removal that guts +/// it. +fn points_into(root: &Path, clone: &Path) -> bool { + let Ok(text) = std::fs::read_to_string(root.join(MANIFEST)) else { + return false; + }; + ["skills", "templates"].iter().any(|key| { + crate::edit::value_at(&text, key) + .map(|raw| unquote(raw.trim())) + .is_some_and(|said| resolved(Path::new(&said)).starts_with(clone)) + }) +} + /// A path with its symlinks followed, or the path itself where it cannot be. /// /// The fallback is the honest one: a path that does not exist cannot be resolved, @@ -730,27 +1136,67 @@ fn resolved(path: &Path) -> PathBuf { /// One function so that the two doors cannot word this differently, and `None` /// rather than an empty string so that neither door has to decide whether to draw /// it — an empty warning drawn as a row is a row an operator reads as a warning. +/// What removing `clone` costs, in one sentence, or `None` when it costs nothing. +/// +/// **The one call a door makes**, so the three surfaces that offer this removal +/// cannot ask different questions. Before 0.31.0 each of them called +/// [`dependents`] and then [`warning`]; a release that added a second way for a +/// bundle to depend on a clone would have had to be remembered at all three, and +/// `src/main.rs` is linked by nothing under `tests/`, so forgetting one would have +/// been invisible to every gate. +/// +/// `adapters` is an `Option` because [`crate::home::adapters`] is: an operator +/// with no home directory has no adapters, and a `None` there is not an error, it +/// is an empty list. +#[must_use] +pub fn removal_cost( + view: &crate::pluginview::View, + clone: &Path, + adapters: Option<&Path>, +) -> Option { + let inside = dependents(view, clone); + let orphans = adapters + .map(|at| orphaned(view, clone, at)) + .unwrap_or_default(); + warning(&inside, &orphans) +} + +/// The line that says what a removal is about to cost, given both lists. #[must_use] -pub fn warning(inside: &[PathBuf]) -> Option { - if inside.is_empty() { +pub fn warning(inside: &[PathBuf], orphans: &[PathBuf]) -> Option { + if inside.is_empty() && orphans.is_empty() { return None; } - let mut said = if inside.len() == 1 { - String::from("1 declared bundle lives inside it") + // **Counted together and listed together.** The two lists differ in where the + // declaration points and in nothing an operator cares about: both are bundles + // that stop loading, both keep their `[[plugin]]` entry, and both are named so + // the operator can decide before anything goes. Two sentences would ask them + // to work out which kind theirs was in order to read the consequence. + let all: Vec<&PathBuf> = inside.iter().chain(orphans.iter()).collect(); + let mut said = if all.len() == 1 { + String::from("1 declared bundle stops loading") } else { - format!("{} declared bundles live inside it", inside.len()) + format!("{} declared bundles stop loading", all.len()) }; said.push_str( ": the `[[plugin]]` entries are left exactly as they are, and io-harness will \ report them as missing from the next turn — ", ); said.push_str( - &inside - .iter() + &all.iter() .map(|path| path.display().to_string()) .collect::>() .join(", "), ); + if !orphans.is_empty() { + // Named as adapters rather than left to look like ordinary directories: an + // operator who goes looking finds a `plugin.toml` io wrote, in a directory + // they never made, and the sentence is the only thing that explains it. + said.push_str( + ". The ones under `adapters/` are manifests io generated; they are not \ + removed, and they stop loading because the clone they name is going", + ); + } Some(said) } @@ -798,6 +1244,22 @@ fn asked(query: &str) -> (&str, Option<&str>) { /// what makes `@/` keep resolving to a clone's own root bundle /// when that root shares its label with something deeper. pub fn locate(markets: &[Market], query: &str) -> Result { + located(markets, query).map(|(_, bundle)| bundle.dir.clone()) +} + +/// [`locate`], keeping the marketplace and the bundle rather than the path alone. +/// +/// **The install needs all three and a `PathBuf` throws two of them away.** An +/// adapted bundle's directory is not the directory a `[[plugin]]` entry names — +/// the entry names the generated manifest, which lives under +/// `///`, and the owner and the repository come from +/// the [`Market`] while the name comes from the [`Bundle`]. A remote entry's +/// directory does not exist at all until [`fetched`] has run, and what says so is +/// [`Bundle::source`]. +/// +/// One matcher and two returns, so `plugin add` and every listing agree about +/// which bundle a word names. [`locate`] is this, narrowed. +pub fn located<'a>(markets: &'a [Market], query: &str) -> Result<(&'a Market, &'a Bundle), String> { let (name, qualifier) = asked(query); let hits: Vec<(&Market, &Bundle)> = markets .iter() @@ -822,7 +1284,7 @@ pub fn locate(markets: &[Market], query: &str) -> Result { }; let hits = if precise.len() == 1 { precise } else { hits }; match hits.as_slice() { - [(_, bundle)] => Ok(bundle.dir.clone()), + [(market, bundle)] => Ok((market, bundle)), [] => Err(unheld(markets, query)), several => { let spellings = several @@ -961,15 +1423,126 @@ pub fn chosen( dir: &Path, markets: impl FnOnce() -> Vec, text: &str, + at: Homes<'_>, ) -> Result { match crate::pluginview::refusal(dir) { None => Ok(Chosen::Path(dir.to_path_buf())), - Some(refused) => locate(&markets(), text) + Some(refused) => prepared(&markets(), text, at.marketplaces, at.staging, at.adapters) .map(Chosen::Held) .map_err(|missing| format!("{refused} — {missing}")), } } +/// The three directories an install may need, passed in rather than looked up. +/// +/// One argument instead of three, because [`chosen`] already takes three and a +/// fourth, fifth and sixth positional `&Path` is a call nobody can read. Grouped +/// rather than resolved inside for this module's standing reason: a decision +/// behind [`crate::home`] is a decision nothing under `tests/` can reach without +/// moving the operator's home out from under a suite running in parallel. +#[derive(Debug, Clone, Copy)] +pub struct Homes<'a> { + /// Where marketplaces are cloned to. + pub marketplaces: &'a Path, + /// Where a clone is assembled before it is renamed into place. + pub staging: &'a Path, + /// Where io writes the manifests it generates. + pub adapters: &'a Path, +} + +/// A bundle brought to the point where a `[[plugin]]` entry can name it. +/// +/// Three directories rather than one, because for an adapted bundle they are three +/// different places and every surface downstream needs a different one. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Prepared { + /// What the `[[plugin]]` entry names. The bundle's own directory for a native + /// bundle; the generated manifest's directory for an adapted one. + pub declare: PathBuf, + /// 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. + pub made: Option, +} + +/// Resolve a word to a bundle and bring it to the point of being installable. +/// +/// **This is the step 0.29.0 and 0.30.0 did not need and 0.31.0 cannot do +/// without.** Until this release every bundle a marketplace held already carried +/// a `plugin.toml`, so resolving a name to a directory *was* the install. A bundle +/// published as a Claude Code or Codex plugin carries no manifest io-harness +/// reads, and one in another repository is not on the disk at all, so the word has +/// to be resolved, fetched and adapted before there is anything an entry can name. +/// +/// Native bundles are untouched by every line of it: [`fetched`] returns their own +/// directory unchanged and the generation is skipped, so `plugin add` on a +/// `plugin.toml` marketplace does exactly what it did in 0.30.2. +/// +/// **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 +/// foreign manifest — there is nothing to read until the manifest io generates +/// exists. What the operator's *configuration* never sees is any of it: +/// `Prepared::made` is what a declined install removes, and the scope file is not +/// opened either way. +pub fn prepared( + markets: &[Market], + query: &str, + marketplaces: &Path, + staging: &Path, + adapters: &Path, +) -> Result { + let (market, bundle) = located(markets, query)?; + let from = fetched(bundle, &market.named, marketplaces, staging)?; + // **The disk decides, not how the bundle was listed.** `from_entry` stamps + // `Origin::Adapted` on every index entry, because an index is a foreign format + // — but the directory it points at may still carry io's own manifest, and a + // remote entry's repository is not even read until the fetch above has run. A + // rule keyed on `origin` would generate an adapter over an author's real + // `plugin.toml`, silently dropping the `[[hook]]`, `[[mcp]]` and `[[agent]]` + // blocks the generator has no source for. `holdings` says a native manifest + // wins its own directory; this is that same sentence on the install path. + if from.join(MANIFEST).is_file() { + return Ok(Prepared { + declare: from.clone(), + from, + made: None, + }); + } + // The **normalised id**, not the label. It is what the generated manifest + // declares and what io-harness namespaces by, so a directory named anything + // else would put two spellings of one bundle on the disk — and on a + // case-insensitive filesystem two bundles labelled `Foo` and `foo` would share + // one adapter directory and the second install would overwrite the first. + let name = crate::adapt::normalised(&bundle.label()).ok_or_else(|| { + format!( + "`{}` is not a usable plugin id, so io cannot name a directory for it", + bundle.label() + ) + })?; + let into = crate::adapt::at(adapters, &market.named.owner, &market.named.repo, &name); + crate::adapt::generate(&from, &name, &into)?; + Ok(Prepared { + declare: into.clone(), + from, + made: Some(into), + }) +} + +/// Take back what an install created, because the operator said no. +/// +/// A no-op for a native bundle, which created nothing. Best effort by design: the +/// operator has already declined and the entry was never written, so a directory +/// that cannot be removed is a stale cache rather than anything they consented to, +/// and a refusal reported *after* a refusal is noise about io's own housekeeping. +pub fn unmake(made: Option<&Path>) { + if let Some(dir) = made { + let _ = std::fs::remove_dir_all(dir); + } +} + /// Which reading of the word won, and the directory it named. /// /// **The reading is the disclosure rule, and it is recorded here so that nothing @@ -987,16 +1560,41 @@ pub fn chosen( pub enum Chosen { /// A directory on this machine, named by the operator. Path(PathBuf), - /// A bundle a marketplace holds, resolved by name. - Held(PathBuf), + /// A bundle a marketplace holds, resolved by name, fetched where it was + /// elsewhere and adapted where its manifest is not one io-harness reads. + Held(Prepared), } impl Chosen { - /// The directory either reading named. + /// The directory a `[[plugin]]` entry names. #[must_use] pub fn dir(&self) -> &Path { match self { - Self::Path(dir) | Self::Held(dir) => dir, + Self::Path(dir) => dir, + Self::Held(prepared) => &prepared.declare, + } + } + + /// The bundle's **own** directory, which for an adapted bundle is not + /// [`Chosen::dir`]. + /// + /// What the disclosure reads hooks from: the generated manifest carries none, + /// so asking io-harness what the bundle contributes cannot answer what its + /// author declared. + #[must_use] + pub fn from(&self) -> &Path { + match self { + Self::Path(dir) => dir, + Self::Held(prepared) => &prepared.from, + } + } + + /// What a declined install must take back, if anything was created. + #[must_use] + pub fn made(&self) -> Option<&Path> { + match self { + Self::Path(_) => None, + Self::Held(prepared) => prepared.made.as_deref(), } } @@ -1028,6 +1626,21 @@ pub struct Disclosure { /// the TUI writes into a terminal whose set the operator chose, and the argv /// form writes down a pipe in ASCII. [`Disclosure::said`] is the fold. pub rows: Vec, + /// One line per hook the bundle declares that io will **not** carry across, + /// each with its event, its command unshortened, and the reason. + /// + /// **Empty for a native bundle, and not because the field does not apply.** A + /// `plugin.toml` declares its hooks to io-harness, which runs them; there is + /// nothing being withheld and nothing to say. This list is only ever the hooks + /// of a Claude Code or Codex bundle, which cannot cross — see + /// [`crate::adapt::Hook`] for why no adapter closes that gap. + /// + /// A separate field rather than more [`Disclosure::rows`], so the count of + /// withheld hooks is a number a test can assert. One row per hook, and F11 + /// asserts the count rather than a `contains`, because one row satisfies a + /// `contains` forever and a hook that exists and is not drawn is the failure + /// this exists to prevent. + pub withheld: Vec, } impl Disclosure { @@ -1103,6 +1716,38 @@ 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) +} + +/// The sentence every withheld hook carries, spelled once. +/// +/// **Three reasons and not one, because an operator who is told "unsupported" +/// cannot tell whether to wait for a release or to stop expecting it.** +/// io-harness's `Hook.run` is argv and deliberately never a shell string; its `on` +/// takes the harness's own event tags; and 0.71.0 refuses `${env:}`, `${file:}` +/// and `${cmd:}` inside a manifest in every scope. A Claude hook is a shell +/// string, an unknown event and a refused substitution at once, so no adapter +/// closes it — and an approximated hook is a program running on this machine that +/// nobody described accurately. +pub const NOT_CARRIED: &str = "io does not run this — a hook here is a shell line for another \ + tool's events, and io-harness takes argv against its own"; + +/// [`disclosure`], plus the hooks a foreign bundle declares and io will not carry. +/// +/// `from` is the bundle's **own** directory — the stranger's checkout — where +/// `dir` is what io-harness is asked to load, which for an adapted bundle is the +/// generated manifest's directory. The two differ precisely because the adapter +/// carries no hooks: asking io-harness what the bundle contributes therefore +/// cannot answer what it *declared*, and reading the author's own file is the only +/// way to name what is being left behind. +/// +/// `None` for a native bundle, where the two are the same directory and nothing is +/// withheld. +pub fn adapted_disclosure( + scope: Scope, + dir: &Path, + from: Option<&Path>, +) -> 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`. @@ -1110,9 +1755,59 @@ pub fn disclosure(scope: Scope, dir: &Path) -> Result { Ok(Disclosure { id: listed.id.clone(), rows: crate::pluginview::detail(&listed, u16::MAX, &crate::glyphs::ASCII), + withheld: from.map(withheld_hooks).unwrap_or_default(), }) } +/// One line per hook the bundle at `from` declares, with its command unshortened. +/// +/// **The command is filtered and never bounded**, which is the one place this +/// module's `LONGEST` deliberately does not apply: it is argv the operator is +/// being asked to consent to being *absent*, and a shortened argv on a consent +/// surface is the single thing that surface must never show. +fn withheld_hooks(from: &Path) -> Vec { + let mut found = Vec::new(); + for at in hook_files(from) { + for hook in crate::adapt::hooks_in(&at) { + let mut said = hook.event.clone(); + said.push_str(" — "); + said.push_str(&hook.command); + said.push_str(" — "); + said.push_str(NOT_CARRIED); + found.push(said); + } + } + found +} + +/// The hooks files a foreign bundle declares or keeps where they are conventional. +/// +/// **Two places, and the manifest's own answer comes first.** A Codex manifest +/// names `"hooks": "./hooks/hooks.json"`, and a Claude bundle generally names +/// nothing and keeps the file at that same conventional path — so looking only at +/// the manifest would miss most bundles and looking only at the convention would +/// miss a bundle that moved it. The conventional path is not added twice when the +/// manifest already named it. +/// +/// The declared path is joined **component by component, plain names only**, the +/// way every other value out of a stranger's file is: it decides which file io +/// opens, and `../../.ssh/id_ed25519` must address nothing. +/// +/// Path discovery only. The reading is [`crate::adapt`]'s, which is the module +/// permitted to parse JSON. +fn hook_files(from: &Path) -> Vec { + let mut found = Vec::new(); + if let Some(said) = crate::adapt::manifest_at(from).and_then(|read| read.hooks) { + found.push(local_dir(from, &said)); + } + let conventional = from.join("hooks").join("hooks.json"); + if !found.contains(&conventional) { + found.push(conventional); + } + found.retain(|path| path.is_file()); + found +} + /// Every bundle whose name or description carries `text`, in [`markets`]' order. /// /// **Across every marketplace, and F5's named sabotage is stopping at the first diff --git a/src/pluginview.rs b/src/pluginview.rs index e5d3f21..67cf6f3 100644 --- a/src/pluginview.rs +++ b/src/pluginview.rs @@ -99,10 +99,19 @@ //! Nothing here opens a `plugin.toml`. Every fact comes from //! [`io_harness::Config::plugins`], which re-reads each declared bundle from disk //! itself; a second reader in this crate would be a second opinion about what a -//! manifest means, and `tests/dependencies.rs` forbids one by path. Like -//! [`crate::servers`] and [`crate::skillview`], this is a data model and pure -//! functions: the driver in `src/main.rs` owns the keyboard and applies what -//! [`add`] and [`remove`] return. +//! manifest means, and `tests/dependencies.rs` forbids one by path. +//! +//! **One fact on this surface is not io-harness's**: whether io wrote a bundle's +//! `plugin.toml` itself. io-harness loads a generated manifest exactly as it loads +//! an authored one and has no word for the difference, so the answer is a path — +//! the directory [`crate::home::adapters`] names, resolved once in [`view`] and +//! carried on [`View::adapters`] — and it is still not a second read of any file. +//! See [`Listed::adapted`]. +//! +//! Like [`crate::servers`] and [`crate::skillview`], this is a data model and +//! functions over it: [`view`] is where the machine is read, once, and everything +//! that draws is pure over what it returned. The driver in `src/main.rs` owns the +//! keyboard and applies what [`add`] and [`remove`] return. use std::path::{Path, PathBuf}; @@ -132,6 +141,25 @@ pub const REFUSED_MARK: &str = "!"; /// draw, since a switched-off bundle is nothing for the operator to fix. pub const DISABLED_MARK: &str = "-"; +/// The mark on a bundle whose `plugin.toml` io wrote. See [`LOADED_MARK`]. +/// +/// `~` against `+`, `-` and `!`: one ASCII character in both glyph sets, on no +/// other row this surface draws, and shaped like none of the three — no stroke of +/// it is a stroke of theirs, so a column of marks stays readable at a glance +/// rather than resolving into two similar characters. +/// +/// **It takes the column from the state marks rather than sitting beside them**, +/// because a row carries one mark and this is the fact an operator cannot recover +/// any other way. A bundle switched off already says "switched off" as the first +/// field of its detail — see [`rows`] — and a bundle that loaded is in the list +/// with its contributions leading the row, so the state survives the column being +/// spent here. Where the manifest came from does not survive anywhere else: an +/// adapted bundle looks exactly like a native one on every other field, and the +/// file to open when io-harness refuses it or when its contributions are not what +/// the clone promised is the generated one, which nothing else on this surface +/// names. +pub const ADAPTED_MARK: &str = "~"; + /// The narrowest a bundle's root may be drawn at, before its separator. /// /// Twenty cells is `...bundles/rust-review` — the last segments and the mark @@ -201,6 +229,46 @@ pub struct Listed { pub layers: Vec, } +impl Listed { + /// Whether io wrote this bundle's manifest rather than the bundle's author. + /// + /// **A path prefix and nothing else.** [`crate::adapt::generate`] writes every + /// manifest it generates under one directory — + /// `////plugin.toml`, the layout + /// [`crate::adapt::at`] states — and writes generated manifests nowhere else, + /// so a root inside `adapters` came from there and a root outside it did not. + /// Nothing is read off the disk to answer this: the module opens no + /// `plugin.toml` (see the module docs), and a marker inside the file would in + /// any case be a claim about the bytes rather than about where they live. + /// + /// **`adapters` is io-cli's own home and does not follow `$IO_CONFIG`.** + /// [`crate::home::adapters`] derives from the operator's home directory rather + /// than from the configuration in force, exactly as the marketplaces do, so an + /// operator who points `$IO_CONFIG` or `$IO_CONFIG_HOME` at an unusual place + /// has moved their *configuration* and not the generated manifests — the + /// prefix still matches and the mark still draws. What does change the answer + /// is an operator who copies a generated `plugin.toml` out of that directory + /// and declares the copy: it reads as native, which is the true answer, because + /// the copy is a file they now own and io will not regenerate it. + /// + /// A method rather than a field, because [`copy_out`] is called from surfaces + /// that have no adapters root to hand — `src/marketplace.rs` copies out a + /// bundle it inspected inside a clone — and a field would have to be threaded + /// through every one of them to be true anywhere. + #[must_use] + pub fn adapted(&self, adapters: &Path) -> bool { + // **Both sides resolved, and `src/marketplace.rs`'s `orphaned` argues why + // at length.** A temporary directory on macOS is reached through a + // symlink — `/var/folders/…` and `/private/var/folders/…` are one + // directory — and `starts_with` compares components. Comparing raw here + // and resolved there would have `removal_cost` name a bundle as an + // orphaned adapter while its own row drew the native mark: two surfaces, + // two answers, one bundle. + let resolve = |path: &Path| std::fs::canonicalize(path).unwrap_or_else(|_| path.to_owned()); + resolve(&self.root).starts_with(resolve(adapters)) + } +} + /// One bundle that was declared and did not load. /// /// Named for what happened rather than mirroring io-harness's `Dropped`: from @@ -237,6 +305,19 @@ pub struct View { pub plugins: Vec, /// The bundles that did not, with the reason each one did not. pub refused: Vec, + /// Where io writes the manifests it generates, on this machine. + /// + /// Carried on the view rather than passed to [`rows`], because it is a fact + /// about the machine and not about the terminal: every caller of `rows` and + /// every caller of [`detail`] would otherwise have to know it, including the + /// ones drawing a bundle that is not on this list at all. + /// + /// `None` where the operator's home directory cannot be determined, which is + /// [`crate::home::adapters`]'s own shape and is read here as *nothing is + /// adapted*: with no adapters root there is no directory a generated manifest + /// could be under, so no row is marked and every bundle draws as the native + /// one it must be. + pub adapters: Option, } impl View { @@ -390,6 +471,11 @@ pub fn view(config: &io_harness::Config) -> View { error: dropped.error.clone(), }) .collect(), + // Resolved once, here, where the view is assembled. The alternative is + // every drawing function calling [`crate::home::adapters`] itself, which + // would make three pure functions read this machine's environment and + // leave a test unable to say what any of them saw. + adapters: crate::home::adapters(), } } @@ -407,11 +493,18 @@ pub fn view(config: &io_harness::Config) -> View { /// /// **A bundle declared `enabled = false` is inside `view.plugins` and so is inside /// that first range**, drawn under [`DISABLED_MARK`] with the state leading its -/// detail. Three marks and still two ranges: the switched-off bundles are a state +/// detail. Four marks and still two ranges: the switched-off bundles are a state /// the operator chose, not a second failure list, and giving them a range of their /// own would have added a third arm to every index calculation on this surface for /// nothing. /// +/// **The fourth mark is [`ADAPTED_MARK`], and it takes the column from the other +/// two rather than adding a range of its own.** A bundle whose manifest io +/// generated is an ordinary declared bundle — it loads, it can be switched off, it +/// is indexed exactly where it sits — so the only thing that changes is which +/// character the row wears. See [`ADAPTED_MARK`] for why that character is worth +/// more there than `+` or `-` is, and [`Listed::adapted`] for what decides it. +/// /// # The narrow form, and what gives way in it /// /// The id is the label. The contributions are unconditional, because they are the @@ -423,6 +516,11 @@ pub fn view(config: &io_harness::Config) -> View { /// contributed and /// as much description as fits; widen the terminal and the path arrives. /// +/// **On an adapted bundle that trade is reversed**: the root's floor is reserved +/// ahead of the description and the description gives way instead. The row's mark +/// says io wrote the manifest and the row is then the only place that says where +/// it wrote it — see [`ADAPTED_MARK`]. +/// /// A refused row never carries the root, and not because it did not fit: /// io-harness's sentence already opens with the path, so appending it would draw /// the same string twice on the row with the least room to spare. @@ -435,8 +533,10 @@ pub fn rows(view: &View, width: u16, glyphs: &Glyphs) -> Vec { let floor = separator_width + glyphs.ellipsis.chars().count(); let mut out = Vec::with_capacity(view.plugins.len() + view.refused.len()); + let adapters = view.adapters.as_deref(); for plugin in &view.plugins { + let adapted = adapters.is_some_and(|root| plugin.adapted(root)); // Commas inside a field, the glyph set's separator between fields: this // is one fact — the list of kinds — and punctuating it with the same run // that divides the row would read as five fields. @@ -473,9 +573,21 @@ pub fn rows(view: &View, width: u16, glyphs: &Glyphs) -> Vec { .saturating_sub(plugin.id.chars().count()) .saturating_sub(detail.chars().count()); + // **On an adapted bundle the root is not the field that gives way.** It is + // the directory holding the manifest io wrote, which is the file to open + // when io-harness refuses the bundle or when what it contributed is not + // what the clone promised — and nothing else on this surface names it. The + // description competing with it is a line io copied out of somebody else's + // metadata, so the root's floor is reserved before the description is + // fitted rather than taken from whatever the description leaves. + let reserved = if adapted { + separator_width + ROOT_FLOOR + } else { + 0 + }; if let Some(description) = &plugin.description { - if left > floor { - let described = fit(description, left - separator_width, glyphs); + if left > floor + reserved { + let described = fit(description, left - separator_width - reserved, glyphs); left -= separator_width + described.chars().count(); detail.push_str(separator); detail.push_str(&described); @@ -493,7 +605,9 @@ pub fn rows(view: &View, width: u16, glyphs: &Glyphs) -> Vec { } out.push(Row::marked( - if plugin.enabled { + if adapted { + ADAPTED_MARK + } else if plugin.enabled { LOADED_MARK } else { DISABLED_MARK @@ -774,6 +888,33 @@ pub fn refusal(dir: &Path) -> Option { /// the one that would make this walk expensive. #[must_use] pub fn candidates(root: &Path) -> Vec { + visited(root) + .into_iter() + .filter(|path| path.join(MANIFEST).is_file()) + .collect() +} + +/// Every directory below `root` the bundle walk looks at, nearest first. +/// +/// **[`candidates`] is this filtered by "carries a [`MANIFEST`]", and separating +/// the two is what 0.31.0 needed rather than a second walk.** A directory holding +/// only a `.claude-plugin/plugin.json` carries no `plugin.toml`, so it was never +/// in `candidates`'s answer at all — a marketplace reader that iterated that list +/// looking for foreign manifests would have found none, ever, and the walk it +/// wanted was the frontier rather than the result. +/// +/// The skips are unchanged and they are the point: `target`, `node_modules` and +/// **every dotted directory** are never entered, so `.git` — which every clone has +/// — is not walked here and cannot contribute a bundle. Reading +/// `.claude-plugin/plugin.json` at a directory this function returned is a known +/// path relative to an already-admitted directory and is not this walk descending +/// into a dot directory; `tests/marketplace.rs` asserts the distinction. +/// +/// Ordered by depth and then by path so the answer is stable between two calls on +/// one machine, which is what makes the row an operator picked yesterday the row +/// in the same place today. +#[must_use] +pub fn visited(root: &Path) -> Vec { let mut found = Vec::new(); let mut frontier = vec![(root.to_path_buf(), 0usize)]; while let Some((dir, depth)) = frontier.pop() { @@ -790,9 +931,7 @@ pub fn candidates(root: &Path) -> Vec { if name.starts_with('.') || name == "target" || name == "node_modules" { continue; } - if path.join(MANIFEST).is_file() { - found.push(path.clone()); - } + found.push(path.clone()); if depth + 1 < DEPTH { frontier.push((path, depth + 1)); } @@ -986,6 +1125,10 @@ mod tests { View { plugins: vec![plugin], refused: Vec::new(), + // No adapters root, so every bundle here is native and the state marks + // are what the rows wear. The adapted case is asserted in + // `tests/plugins.rs`, against a bundle whose root really is under one. + adapters: None, } } @@ -1073,6 +1216,7 @@ mod tests { path: PathBuf::from("/repo/bundles/empty"), error: "/repo/bundles/empty: no plugin.toml".to_string(), }], + adapters: None, }; for glyphs in [&UNICODE, &ASCII] { let rows = rows(&view, 200, glyphs); @@ -1174,6 +1318,7 @@ mod tests { path: PathBuf::from("/repo/bundles/empty"), error: sentence.to_string(), }], + adapters: None, }; for glyphs in [&UNICODE, &ASCII] { let rows = rows(&view, 200, glyphs); @@ -1204,6 +1349,7 @@ mod tests { path: PathBuf::from("/repo/bundles/x"), error: sentence.to_string(), }], + adapters: None, }; for glyphs in [&UNICODE, &ASCII] { let rows = rows(&view, 80, glyphs); diff --git a/src/skillview.rs b/src/skillview.rs index 74ca820..3b75d3f 100644 --- a/src/skillview.rs +++ b/src/skillview.rs @@ -259,7 +259,7 @@ pub struct View { /// Every skill in `dir` and in `dir/disabled`. /// /// **`dir` is the directory the RUN reads, and it is not always io-cli's own.** -/// `[run] skills` and `[app.io-cli] skills` both beat the `~/.io-cli/skills` +/// `[run] skills` and `[app.io-cli] skills` both beat the default `skills/` /// default, so a surface that walked the home would list five files the model is /// never offered and hide the ones it is — which would make this module's whole /// premise false. The caller resolves it through `crate::contract::skills_dir`, @@ -328,7 +328,7 @@ pub fn view(home: &Path, dir: &Path, bundles: &[(String, PathBuf)]) -> View { /// [`Origin::Bundle`] by construction. /// /// This is not a rare path. `crate::contract::skills_dir` answers `None` whenever -/// the operator has never made `~/.io-cli/skills`, which is the ordinary state of +/// the operator has never made the default `skills/` directory, which is the ordinary state of /// a fresh install — and it is exactly the state in which every skill the model /// is offered came from a bundle. Returning an empty view here would blank the /// surface precisely when it is the only listing there is. diff --git a/tests/adapt.rs b/tests/adapt.rs new file mode 100644 index 0000000..e3af4d4 --- /dev/null +++ b/tests/adapt.rs @@ -0,0 +1,771 @@ +//! The three manifest formats io-cli did not invent, read. +//! +//! Every fixture here is written by the test. **A real marketplace is never +//! cloned**: the release's `preferred_tools` says so, and a gate that reached the +//! network would be a gate that fails when GitHub does. +//! +//! The shapes asserted on were taken from five real marketplaces on one machine — +//! `zeroonething/ultraship`, `zeroonething/caveman`, `zeroonething/ponytail`, +//! `obra/superpowers-marketplace` and `anthropics/claude-plugins-official`, 304 +//! plugins between them — rather than from either format's documentation. Where a +//! fixture below looks oddly specific, that is why. +//! +//! **F7, F8 and F9 assert on the manifest io writes, and every claim about it goes +//! through io-harness.** `Plugins::inspect` and `Config::plugins` are the loader +//! that reads a `plugin.toml` in the field, and `Plugin`'s own accessors are what +//! answer what a bundle contributed. A test that read back the string io-cli +//! spelled would assert that this crate can quote its own output, which is not the +//! question: the question is whether io-harness loads it. + +use std::path::{Path, PathBuf}; + +use io_cli::adapt::{self, Source}; +use io_harness::config::{Config, Scope, LOCAL_FILE}; +use io_harness::{Plugins, PLUGIN_FILE}; + +/// A clone directory, empty. +fn clone() -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().expect("a clone directory"); + let root = dir.path().to_path_buf(); + (dir, root) +} + +/// Write `text` to `root/rel`, making the directories on the way. +fn file(root: &Path, rel: &str, text: &str) -> PathBuf { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().expect("a parent")).expect("the directories"); + std::fs::write(&path, text).expect("the file"); + path +} + +/// The index shape `zeroonething/ultraship` publishes — one plugin at the root. +const ONE_AT_THE_ROOT: &str = r#"{ + "name": "ultraship", + "description": "Ship at inference speed.", + "owner": { "name": "Aakash Pawar (zeroonething)" }, + "plugins": [ + { + "name": "ultraship", + "description": "Turn vague ideas into releasable software.", + "version": "2.3.0", + "source": "./", + "author": { "name": "Aakash Pawar (zeroonething)" } + } + ] +}"#; + +#[test] +fn f1_an_index_naming_one_plugin_at_the_root_reads_as_one_entry() { + let (_dir, root) = clone(); + file(&root, ".claude-plugin/marketplace.json", ONE_AT_THE_ROOT); + + let index = adapt::index_at(&root).expect("the index is read"); + + assert_eq!(index.entries.len(), 1, "one entry, counted and not matched"); + assert!( + index.unreadable.is_empty(), + "nothing in this file is unreadable; found {:?}", + index.unreadable, + ); + let entry = &index.entries[0]; + assert_eq!(entry.name, "ultraship"); + assert_eq!(entry.version.as_deref(), Some("2.3.0")); + assert_eq!( + entry.source, + Source::Local("./".to_string()), + "the root spelling is a local source and not a remote one", + ); +} + +#[test] +fn a_clone_publishing_no_index_answers_none_rather_than_an_empty_one() { + let (_dir, root) = clone(); + assert!( + adapt::index_at(&root).is_none(), + "no index and an index holding nothing are two different answers, and the \ + precedence rule turns on which of them this is", + ); +} + +#[test] +fn f5_the_two_remote_tags_are_one_shape_and_both_keep_their_sha() { + let (_dir, root) = clone(); + file( + &root, + ".claude-plugin/marketplace.json", + r#"{ + "plugins": [ + { + "name": "api-security-testing", + "source": { + "source": "git-subdir", + "url": "https://github.com/42Crunch-AI/claude-plugins.git", + "path": "plugins/api-security-testing", + "ref": "v1.5.5", + "sha": "30287f5e3f122a646d1ac5ca3ab96e130c52a3ad" + } + }, + { + "name": "agentforce-adlc", + "source": { + "source": "url", + "url": "https://github.com/SalesforceAIResearch/agentforce-adlc.git", + "sha": "d16d14ac7f817336e21bf9392cf51b6cac6194d8" + } + }, + { + "name": "superpowers", + "source": { + "source": "url", + "url": "https://github.com/obra/superpowers.git" + } + } + ] +}"#, + ); + + let index = adapt::index_at(&root).expect("the index is read"); + assert_eq!(index.entries.len(), 3, "three entries, counted"); + + let Source::Remote(subdir) = &index.entries[0].source else { + panic!("a git-subdir entry is a remote source"); + }; + assert_eq!(subdir.path.as_deref(), Some("plugins/api-security-testing")); + assert_eq!(subdir.reference.as_deref(), Some("v1.5.5")); + assert_eq!( + subdir.sha.as_deref(), + Some("30287f5e3f122a646d1ac5ca3ab96e130c52a3ad"), + ); + + let Source::Remote(url) = &index.entries[1].source else { + panic!("a url entry is a remote source"); + }; + assert_eq!(url.path, None, "a url entry need not name a path"); + assert_eq!( + url.sha.as_deref(), + Some("d16d14ac7f817336e21bf9392cf51b6cac6194d8"), + ); + + let Source::Remote(bare) = &index.entries[2].source else { + panic!("a url entry with no sha is still a remote source"); + }; + assert_eq!( + (bare.sha.as_deref(), bare.reference.as_deref()), + (None, None), + "`superpowers-marketplace` publishes ten of these; the sha is optional in \ + the field even though the official index sets it on all 238 of its remote \ + entries", + ); +} + +#[test] +fn an_entry_in_a_shape_io_does_not_read_is_reported_and_the_rest_survive() { + let (_dir, root) = clone(); + file( + &root, + ".claude-plugin/marketplace.json", + r#"{ + "plugins": [ + { "name": "good", "source": "./plugins/good" }, + { "name": "strange", "source": { "source": "smoke-signal", "channel": 4 } }, + { "name": "also-good", "source": "./plugins/also-good" } + ] +}"#, + ); + + let index = adapt::index_at(&root).expect("the index is read"); + + assert_eq!( + index.entries.len(), + 2, + "one bad entry costs that entry and not the file", + ); + assert_eq!( + index.unreadable.len(), + 1, + "and it is reported rather than dropped — an operator finds a shape no \ + fixture anticipated, and a silent skip makes that unreportable", + ); + assert!( + index.unreadable[0].contains("strange"), + "the report names the entry: {:?}", + index.unreadable[0], + ); +} + +#[test] +fn f4_a_codex_manifest_is_read_where_a_claude_one_is_absent() { + let (_dir, root) = clone(); + file( + &root, + ".codex-plugin/plugin.json", + r#"{ + "name": "ultraship", + "version": "2.3.0", + "description": "Ship at inference speed.", + "skills": "./skills/", + "hooks": "./hooks/hooks.json", + "interface": { "displayName": "UltraShip", "capabilities": ["Instructions"] } +}"#, + ); + + let read = adapt::manifest_at(&root).expect("the Codex manifest is read"); + assert_eq!(read.name.as_deref(), Some("ultraship")); + assert_eq!(read.skills.as_deref(), Some("./skills/")); + assert_eq!(read.hooks.as_deref(), Some("./hooks/hooks.json")); + assert!( + read.from.ends_with(".codex-plugin/plugin.json"), + "the manifest says which file it came from so a surface need not guess: {}", + read.from.display(), + ); +} + +#[test] +fn a_claude_manifest_wins_where_a_repository_publishes_both() { + let (_dir, root) = clone(); + file( + &root, + ".claude-plugin/plugin.json", + r#"{ "name": "claude-said" }"#, + ); + file( + &root, + ".codex-plugin/plugin.json", + r#"{ "name": "codex-said" }"#, + ); + + let read = adapt::manifest_at(&root).expect("a manifest is read"); + assert_eq!( + read.name.as_deref(), + Some("claude-said"), + "one of them wins by a stated rule; a merge could produce a bundle neither \ + file describes", + ); +} + +#[test] +fn a_manifest_with_an_unknown_key_still_reads() { + let (_dir, root) = clone(); + file( + &root, + ".claude-plugin/plugin.json", + r#"{ "name": "lsp", "lspServers": { "gopls": { "command": "gopls" } } }"#, + ); + + assert_eq!( + adapt::manifest_at(&root).and_then(|read| read.name), + Some("lsp".to_string()), + "reading somebody else's file forgives an unknown key; the official index \ + carries `lspServers`, `strict`, `tags` and `keywords` among others, and a \ + reader that refused one would refuse almost every real manifest", + ); +} + +#[test] +fn f11_every_hook_is_read_and_its_command_is_never_shortened() { + let (_dir, root) = clone(); + let long = format!( + "\"${{CLAUDE_PLUGIN_ROOT}}/hooks/{}.cmd\" run", + "x".repeat(400) + ); + let hooks = file( + &root, + "hooks/hooks.json", + &format!( + r#"{{ + "hooks": {{ + "SessionStart": [ + {{ "matcher": "startup|clear", "hooks": [ + {{ "type": "command", "command": "\"${{CLAUDE_PLUGIN_ROOT}}/hooks/run-hook.cmd\" session-start" }}, + {{ "type": "command", "command": {long:?} }} + ] }} + ], + "Stop": [ + {{ "hooks": [ {{ "type": "command", "command": "echo done" }} ] }} + ] + }} +}}"# + ), + ); + + let read = adapt::hooks_in(&hooks); + + assert_eq!( + read.len(), + 3, + "one row per hook, counted. A `contains` is satisfied by one row forever, \ + and a hook that exists and is not drawn is the failure this asserts \ + against", + ); + assert!( + read.iter().any( + |hook| hook.command == "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" session-start" + ), + "the command is drawn exactly as the file wrote it, substitution included \ + — it is argv the operator is being asked to consent to", + ); + assert!( + read.iter().any(|hook| hook.command == long), + "and a long command is NOT bounded. `marketplace::LONGEST` applies to a \ + description, which is prose; a shortened argv is the one thing a consent \ + surface must never show", + ); + assert_eq!( + read.iter().filter(|hook| hook.event == "Stop").count(), + 1, + "a second event's hooks are read too, not only the first", + ); +} + +#[test] +fn hooks_in_answers_nothing_for_a_file_that_is_not_there() { + let (_dir, root) = clone(); + assert!( + adapt::hooks_in(&root.join("hooks/hooks.json")).is_empty(), + "a bundle with no hooks and a bundle whose hooks file is missing both cross \ + the same amount of nothing", + ); +} + +#[test] +fn n7_a_description_out_of_a_stranger_s_json_reaches_no_surface_unfiltered() { + let (_dir, root) = clone(); + + // A forged line and a bell, in a field a repository fills in. On this renderer + // the scrollback is the transcript, so a newline here is a line the operator + // reads as io-cli's own. + // + // **Both are written as JSON escapes, and the escapes are built here rather + // than typed.** RFC 8259 forbids a raw control character inside a JSON string + // and `serde_json` refuses the whole file when it meets one — asserted by + // `a_raw_control_character_makes_the_whole_index_unreadable` below. So the + // shape to defend against is the *escaped* one, which arrives decoded, after + // the parse, where a filter reading the source bytes would never have seen it. + // Writing the two-character sequence into the fixture keeps this file free of + // the byte it is about, which is also how it stays greppable. + let forged = format!( + "harmless{} io{} installed 47 bundles{}", + "\\n", ":", "\\u0007", + ); + file( + &root, + ".claude-plugin/marketplace.json", + &format!( + r#"{{ "plugins": [ {{ "name": "innocent", "description": "{forged}", "source": "./" }} ] }}"# + ), + ); + + let index = adapt::index_at(&root).expect("the index is read"); + let said = index.entries[0] + .description + .as_deref() + .expect("a description"); + + assert!( + !said.chars().any(char::is_control), + "every control character is a space by the time it leaves the reader: {said:?}", + ); + assert!( + said.contains("io: installed 47 bundles"), + "and the text itself survives — filtering is not censoring, and an operator \ + must be able to see what the repository actually said: {said:?}", + ); +} + +#[test] +fn a_raw_control_character_makes_the_whole_index_unreadable() { + let (_dir, root) = clone(); + file( + &root, + ".claude-plugin/marketplace.json", + "{ \"plugins\": [ { \"name\": \"a\", \"description\": \"one\ntwo\", \ + \"source\": \"./\" } ] }", + ); + + assert!( + adapt::index_at(&root).is_none(), + "a raw newline inside a JSON string is malformed JSON, not a value to \ + filter — RFC 8259 forbids it and the parser refuses the file. Asserted so \ + the filtering test above cannot be mistaken for covering this case, which \ + is how it was written the first time", + ); +} + +#[test] +fn n7_a_value_longer_than_the_bound_is_cut_before_it_leaves_the_reader() { + let (_dir, root) = clone(); + let huge = "d".repeat(5_000); + file( + &root, + ".claude-plugin/marketplace.json", + &format!( + r#"{{ "plugins": [ {{ "name": "big", "description": {huge:?}, "source": "./" }} ] }}"# + ), + ); + + let index = adapt::index_at(&root).expect("the index is read"); + let said = index.entries[0] + .description + .as_deref() + .expect("a description"); + + assert!( + said.chars().count() < 300, + "a description is one finished line of a picker row, and nothing stops a \ + repository putting a megabyte on it; got {} characters", + said.chars().count(), + ); +} + +// --------------------------------------------------------------------------- +// F7, F8, F9 — the manifest io writes +// --------------------------------------------------------------------------- + +/// The agent frontmatter shape a real bundle publishes: a name, a folded +/// `description`, a model that is one vendor's family alias, and a tool list. +/// +/// Taken from `zeroonething/caveman`'s `agents/cavecrew-builder.md`. Three of the +/// four keys have no slot in an io-harness agent definition, which is the whole +/// point of the fixture — one is translated and three are disclosed. +const REVIEWER: &str = "---\n\ + name: reviewer\n\ + description: >\n\ + \x20 Reads a diff and says what is wrong with it.\n\ + model: sonnet\n\ + tools: Read, Grep\n\ + ---\n\ + \n\ + You look for what is missing.\n"; + +/// A stdio server, which is what every local `.mcp.json` entry is: no `type` key, +/// a `command`, and arguments. +const STDIO_SERVER: &str = + r#"{ "mcpServers": { "github": { "command": "github-mcp-server", "args": ["stdio"] } } }"#; + +/// A bundle carrying one of each kind that maps, and nothing that does not. +fn four_kinds(root: &Path) -> PathBuf { + let bundle = root.join("bundle"); + file( + &bundle, + ".claude-plugin/plugin.json", + r#"{ "name": "rust-review", "description": "Everything our Rust reviews need." }"#, + ); + file( + &bundle, + "skills/review/SKILL.md", + "# review\n\nHow we review.\n", + ); + file(&bundle, "commands/ship.md", "Ship $ARGUMENTS.\n"); + file(&bundle, "agents/reviewer.md", REVIEWER); + file(&bundle, ".mcp.json", STDIO_SERVER); + bundle +} + +/// 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 { + 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") + .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. +#[test] +fn f7_a_four_kind_bundle_becomes_a_manifest_io_harness_loads_with_nothing_dropped() { + let (_dir, root) = clone(); + let bundle = four_kinds(&root); + let into = adapt::at( + &root.join("adapters"), + "zeroonething", + "rust-review", + "rust-review", + ); + + let written = adapt::generate(&bundle, "rust-review", &into).expect("the adapter is written"); + + assert_eq!( + written.manifest, + root.join("adapters/zeroonething/rust-review/rust-review") + .join(PLUGIN_FILE), + "the destination is `////plugin.toml`, and nothing \ + is written inside the clone — a file io wrote in a stranger's checkout is a \ + dirty tree at their next `git pull`", + ); + assert!( + !bundle.join(PLUGIN_FILE).exists(), + "and the clone is untouched", + ); + + let plugins = loaded(&root, &into); + assert!( + plugins.dropped().is_empty(), + "io-harness refused the manifest io wrote: {:?}", + plugins.dropped(), + ); + let plugin = plugins + .get("rust-review") + .expect("loaded under the id io declared"); + + // Every assertion below is io-harness's own accessor on the file it has just + // read. A test that compared the generated text against a string would be + // 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", + ); + assert_eq!( + plugin.templates_dir(), + Some(std::fs::canonicalize(bundle.join("commands")).expect("the commands directory")), + "`commands/` and a templates directory are one file in two vocabularies — \ + markdown, optional frontmatter, `$ARGUMENTS` — so the directory is named and \ + nothing is copied", + ); + assert_eq!( + plugin.agents().len(), + 1, + "one agent, counted. A `contains` over the manifest text is satisfied by one \ + `[[agent]]` forever, and an agent that exists and is not declared is the \ + failure this asserts against", + ); + assert_eq!(plugin.agents()[0].name, "rust-review__reviewer"); + assert_eq!( + plugin.agents()[0].role.as_deref(), + Some("You look for what is missing."), + "the markdown after the closing fence IS the child's system prompt, which is \ + what a role is prepended to", + ); + assert_eq!(plugin.mcp_servers().len(), 1, "one MCP server, counted"); + assert_eq!(plugin.mcp_servers()[0].id, "rust-review__github"); + + assert_eq!( + written.contributes, + ["skills", "templates", "agents", "mcp"], + "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.disclosed.len(), + 3, + "`description`, `model` and `tools` have no slot in an io-harness agent \ + definition, and each is reported so a surface can name it rather than being \ + dropped in silence: {:?}", + written.disclosed, + ); + assert!( + written + .disclosed + .iter() + .any(|said| said.contains("`model`")), + "`sonnet` is one vendor's family alias and `AgentDef::model` is a model id a \ + provider published; translating one into the other invents a fact the run pays \ + for: {:?}", + written.disclosed, + ); +} + +/// **F7, the control.** A bundle carrying none of the four kinds contributes none +/// of them. +/// +/// Without this, every count in F7 is satisfied by a generator that wrote a `name` +/// and stopped: `dropped()` would still be empty, `get(id)` would still find the +/// plugin, and the four assertions would be measuring a fixture rather than a +/// translation. This fixture differs from F7's in exactly the four directories. +#[test] +fn f7_a_bundle_carrying_none_of_the_four_kinds_contributes_nothing() { + let (_dir, root) = clone(); + let bundle = root.join("bundle"); + file( + &bundle, + ".claude-plugin/plugin.json", + r#"{ "name": "bare" }"#, + ); + let into = root.join("adapters/bare"); + + adapt::generate(&bundle, "bare", &into).expect("a bundle carrying nothing still adapts"); + + let plugins = loaded(&root, &into); + let plugin = plugins + .get("bare") + .expect("loaded under the id io declared"); + assert!( + plugin.contributions().is_empty(), + "the four kinds are read off the disk, not written unconditionally: {:?}", + plugin.contributions(), + ); + assert_eq!(plugin.skills_dir(), None); + assert_eq!(plugin.templates_dir(), None); + assert!(plugin.agents().is_empty()); + assert!(plugin.mcp_servers().is_empty()); +} + +/// **F8.** The generated manifest goes back through io-harness's own parser, and a +/// key io-cli invents fails the build rather than one operator's configuration. +/// +/// Sabotage: add a `commands` key to the generator beside `templates`. Under it +/// this test fails and nothing else does — and in the field the cost is not one +/// setting that does nothing, it is `Plugins::dropped` carrying the whole bundle +/// with an "unknown field" the operator cannot act on, in a file io-cli wrote and +/// they have never opened. +#[test] +fn f8_the_generated_manifest_round_trips_and_an_invented_key_refuses_the_bundle() { + 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 manifest = into.join(PLUGIN_FILE); + let written = std::fs::read_to_string(&manifest).expect("the manifest io wrote"); + + // `inspect` is the loader `Config::plugins` runs, reached without a + // `[[plugin]]` entry — every check, on the file that is on disk, with none of + // a configuration's other scopes in the way. + let plugin = Plugins::inspect(Scope::User, &into).expect("the manifest io wrote parses"); + assert_eq!(plugin.id(), "rust-review"); + + // **The half that makes the assertion above worth making.** io-harness parses a + // manifest under `deny_unknown_fields`, and this proves it on the exact bytes + // io writes. `commands` is the near miss worth choosing: it is the foreign + // format's own word for what io-harness calls `templates`, so it is the key + // this generator is most likely to grow by accident. + // + // **Prepended, not appended, and that is load-bearing.** The written manifest + // ends in an `[[mcp]]` block, and a bare key after an array-of-tables header + // belongs to that table rather than to the root — where `McpServer`'s + // `#[serde(flatten)]` absorbs it in silence. A test that appended would pass + // for a reason that has nothing to do with the rule it is about. + std::fs::write(&manifest, format!("commands = \"commands\"\n{written}")) + .expect("the manifest with one invented key"); + + let refused = Plugins::inspect(Scope::User, &into) + .expect_err("a key io-harness has no field for refuses the manifest and is not ignored"); + assert!( + refused.to_string().contains("commands"), + "and the refusal names the key, which is what makes it actionable: {refused}", + ); +} + +/// 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#"{ + "mcpServers": { + "local": { + "command": "node", + "args": ["${CLAUDE_PLUGIN_ROOT}/server/index.js"], + "env": { "ROOT": "${CLAUDE_PLUGIN_ROOT}" } + } + } +}"#; + +/// **F9.** `${CLAUDE_PLUGIN_ROOT}` comes out as an absolute path, and the +/// generated bytes carry no `${` at all. +/// +/// Sabotage: pass the value through unchanged. The manifest still looks right to +/// anyone reading it, and io-harness refuses it — and the refusal takes the whole +/// bundle in every scope, so the operator loses every skill, template and agent +/// the bundle carries over one argument in a file they did not write. +#[test] +fn f9_the_one_expandable_substitution_is_written_out_and_no_dollar_brace_survives() { + let (_dir, root) = clone(); + let bundle = root.join("bundle"); + file(&bundle, ".mcp.json", PLUGIN_ROOT_SERVER); + let into = root.join("adapters/local"); + + adapt::generate(&bundle, "local", &into).expect("the adapter is written"); + + // Scanned in the bytes, because the bytes are what io-harness refuses. What + // the generator meant to expand is not the question. + let text = std::fs::read_to_string(into.join(PLUGIN_FILE)).expect("the manifest"); + assert!( + !text.contains("${"), + "io-harness refuses a substitution inside a plugin.toml in every scope and the \ + refusal takes the whole bundle, so the generated bytes carry none: {text}", + ); + + // And the path is the one io-harness will actually reach, asked of io-harness. + let plugin = Plugins::inspect(Scope::User, &into).expect("the manifest parses"); + assert_eq!(plugin.mcp_servers().len(), 1, "one server, counted"); + let io_harness::McpTransport::Stdio { args, env, .. } = &plugin.mcp_servers()[0].transport + else { + panic!("a server naming a `command` is a stdio server"); + }; + assert_eq!(args.len(), 1, "one argument, counted"); + assert!( + Path::new(&args[0]).is_absolute(), + "the substitution means the bundle's root, and a manifest in io's own home \ + reaches the clone only by an absolute path: {}", + args[0], + ); + assert!( + args[0].ends_with("server/index.js") || args[0].ends_with("server\\index.js"), + "and the rest of the argument survives the expansion rather than being replaced \ + by it: {}", + args[0], + ); + let expected = std::fs::canonicalize(&bundle) + .expect("the bundle") + .to_string_lossy() + .into_owned(); + assert_eq!( + env.get("ROOT"), + Some(&expected), + "every field is expanded and not only the one this fixture was written for", + ); +} + +/// **F9, the other half.** A substitution io cannot honestly expand is a refusal +/// naming the server, and nothing is left on disk. +#[test] +fn f9_a_substitution_io_cannot_expand_refuses_and_names_the_server() { + let (_dir, root) = clone(); + let bundle = root.join("bundle"); + file( + &bundle, + ".mcp.json", + r#"{ "mcpServers": { "github": { + "command": "github-mcp-server", + "env": { "GITHUB_TOKEN": "${GITHUB_TOKEN}" } } } }"#, + ); + let into = root.join("adapters/github"); + + let refused = adapt::generate(&bundle, "github", &into) + .expect_err("a substitution io cannot honestly expand is never passed through"); + + assert!( + refused.contains("server `github`"), + "the refusal names the server rather than the bundle: a person fixing this has \ + to know which entry of the file it is about, and `github-mcp-server` would \ + satisfy a looser check forever: {refused}", + ); + assert!( + refused.contains("env.GITHUB_TOKEN"), + "and the field it is in: {refused}", + ); + assert!( + !into.join(PLUGIN_FILE).exists(), + "and nothing is left behind. A manifest io-harness would refuse must never reach \ + a directory an operator could go on to declare — a broken adapter on disk reads \ + exactly like a working one until something loads it", + ); +} diff --git a/tests/contract.rs b/tests/contract.rs index f271519..fc7ea27 100644 --- a/tests/contract.rs +++ b/tests/contract.rs @@ -93,16 +93,32 @@ fn discovered(files: &[(&str, &str)]) -> (tempfile::TempDir, Config) { /// home it was started with. struct HomeFixture { dir: tempfile::TempDir, - previous: [(&'static str, Option); 2], + previous: [(&'static str, Option); 4], } impl HomeFixture { + /// **The fixture names the home in force as well as the operator's home, and + /// that is what `io` itself does.** `contract::default_skills` anchors on + /// [`io_cli::home::in_force`] since 0.31.0, and in the binary + /// `home::adopt()` runs immediately above the first `Config::discover` and + /// sets `IO_CONFIG_HOME` to `~/.io-cli`. A fixture that set only `HOME` would + /// therefore be testing a state the product never reaches — and, worse, would + /// inherit whatever `no_user_scope` had already pointed the variable at, so + /// the default this file asserts on would be a directory in some other + /// fixture's temporary tree. + /// + /// `IO_CONFIG` names a file outright and beats `IO_CONFIG_HOME`, so an + /// inherited one has to go or the fixture is not the fixture. Every variable + /// touched here is restored on drop, this one included. fn new() -> Self { let dir = tempfile::tempdir().expect("a home directory"); - let previous = ["HOME", "USERPROFILE"].map(|var| (var, std::env::var_os(var))); - for (var, _) in &previous { - std::env::set_var(var, dir.path()); - } + let home = dir.path().to_path_buf(); + let previous = ["HOME", "USERPROFILE", "IO_CONFIG_HOME", "IO_CONFIG"] + .map(|var| (var, std::env::var_os(var))); + std::env::set_var("HOME", &home); + std::env::set_var("USERPROFILE", &home); + std::env::set_var("IO_CONFIG_HOME", home.join(".io-cli")); + std::env::remove_var("IO_CONFIG"); Self { dir, previous } } @@ -1924,3 +1940,62 @@ fn n3_this_release_adds_no_configuration_key() { something to configure", ); } + +/// **F16 — the default skills directory sits under the home in force, beside the +/// memory note.** +/// +/// A skill is something the operator wrote, so it belongs wherever they put the +/// rest of what they wrote: an `$IO_CONFIG_HOME` pointed somewhere else moved +/// `io.toml` and `IO.md` there, and a skills default that stayed with io-cli's own +/// default home would read a directory beside a configuration file this session is +/// not using. +/// +/// **Both halves are asserted here, in one test, because that is the only thing +/// that keeps them together.** A test for the skills half alone permits +/// `contract::default_skills` and `memory::path` to answer about two different +/// directories, which is the state this release exists to end. +/// +/// The fixture makes the *wrong* answer available on purpose: `HomeFixture` +/// creates `~/.io-cli/skills` under a home that is not the one in force, so a +/// regression to `home::path` fails loudly with a path rather than quietly with a +/// `None` that could mean anything. +/// +/// Sabotage: put `home::path()` back in `contract::default_skills` — under which +/// this fails on the first assertion, naming the home it followed. +/// +/// The lock is held for the whole of it. `IO_CONFIG_HOME` is left set on the way +/// out, the way `tests/memory.rs` leaves it: the directory it names is gone by +/// then, so a later `Config::discover` finds no user scope, which is what every +/// other fixture in this file wants anyway. +#[test] +fn f16_the_skills_default_follows_the_home_in_force() { + let _guard = env_lock(); + let user = io_harness::config::Scope::User; + + // io-cli's default home, with skills in it — the answer that must NOT win. + let default_home = HomeFixture::new(); + let unread = default_home.skills(); + + // And the home actually in force, which is somewhere else entirely. + let dir = tempfile::tempdir().expect("the home in force"); + // `IO_CONFIG` names a file outright and beats `IO_CONFIG_HOME`, so a developer + // who has one exported would otherwise decide this test. + std::env::remove_var(io_harness::config::CONFIG_VAR); + std::env::set_var(io_harness::config::CONFIG_HOME_VAR, dir.path()); + let skills = dir.path().join("skills"); + std::fs::create_dir_all(&skills).expect("the skills directory in force"); + + assert_eq!( + io_cli::contract::skills_dir(¬hing(), &Capabilities::default(), root()), + Some(skills), + "a contract that names no skills directory takes the one under the home in \ + force, not the {} nobody pointed this session at", + unread.display(), + ); + assert_eq!( + io_cli::memory::path(&root(), user), + Some(dir.path().join(io_cli::memory::file_name(user))), + "and the operator's memory note answers about the same directory, which is \ + the point: authored content has one home, not two", + ); +} diff --git a/tests/dependencies.rs b/tests/dependencies.rs index 472072b..7fa3760 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -813,6 +813,37 @@ fn f10_the_fetch_spawns_git_and_builds_no_argument_out_of_a_string() { behavioural assertions in tests/fetch.rs are what must be re-argued \ first.", ); + // **0.31.0 pins a second builder, and this is where that gate was re-argued + // rather than relaxed.** An index entry may name a commit, and a shallow clone + // checked out at one is not a single `git clone`: `--revision` landed in git + // 2.49, this product names no git floor, so the portable route is four + // invocations. The literal that used to be asserted here — `.args(argv(` at + // the spawn — cannot survive that, because the spawn now takes a finished list + // from a loop rather than calling one builder inline. + // + // What replaces it is stronger rather than weaker, and the difference is worth + // stating because 0.31.0's own risk list names "relaxing F10 to fit both + // shapes" as a failure mode. Before: one builder's signature pinned, and the + // spawn asserted to call it. After: **two** builders' signatures pinned, and + // the spawn asserted to take a `Vec` **parameter** — so it cannot + // add an element at the call site even in principle, which the inline form + // could have done and was only prevented from doing by `!contains(".arg(")`. + // The count of spawns is unchanged at one, and every other absence below still + // holds. + assert!( + code.contains("pub fn steps(url: &str, into: &Path, at: &Pin) -> Vec>"), + "the pinned-fetch builder no longer takes a URL, a destination and a pin \ + and nothing else. It is the second place an argv element could be \ + interpolated from something a stranger's index wrote, so its signature is \ + pinned for the reason `argv`'s is.", + ); + assert!( + code.contains("fn run(argv: Vec)"), + "the one spawn no longer takes a finished argv as a parameter. That is the \ + property that makes the assertions above load-bearing: the spawn never \ + sees the parts, so every element it passes was built by a pure function \ + `tests/fetch.rs` asserts the output of.", + ); // The backstop, and it is a backstop: this module builds every string it // produces with `String::push_str`, so an interpolation written in its own // idiom would pass this and die on the assertion above instead. Kept because @@ -832,9 +863,16 @@ fn f10_the_fetch_spawns_git_and_builds_no_argument_out_of_a_string() { hides from both.", ); assert!( - code.contains(".args(argv("), - "the spawn no longer takes its arguments from `argv`, so tests/fetch.rs is \ - asserting the shape of a function nothing runs", + code.contains(".args(argv)"), + "the spawn no longer takes its arguments from the finished list handed to \ + it, so tests/fetch.rs is asserting the shape of a function nothing runs", + ); + assert_eq!( + code.matches(".args(").count(), + 1, + "src/fetch.rs passes arguments in more than one place. Every element goes \ + through a pure builder and reaches the program through the single `run`, \ + so a second `.args(` is a second argv nothing asserts the shape of.", ); } @@ -1036,6 +1074,154 @@ fn f7_the_configuration_is_read_through_the_harness_and_never_parsed_here() { } } +/// The modules permitted to turn somebody else's JSON into a value. +/// +/// **Two, and they are files rather than modules.** `src/import.rs` has read the +/// operator's own `~/.claude.json` and Codex settings since 0.21.0; +/// `src/adapt.rs` reads the three foreign plugin manifest formats. Sorted here +/// for the reason `spawning_modules` is sorted: `read_dir` answers in whatever +/// order the filesystem holds, and a test that passes on one machine and fails on +/// another is worse than no test. +fn json_reading_modules() -> Vec { + let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut permitted = vec![src.join("adapt.rs"), src.join("import.rs")]; + permitted.sort(); + permitted +} + +/// Is `found` **exactly** the set permitted to deserialize JSON? +/// +/// Named for the reason `only_the_permitted_spawn` is named, and it is the same +/// reason: the sabotage arm for this criterion is "widen the set to a substring +/// or stem match", which makes the gate *more* permissive — so nothing fails and +/// it goes vacuous without going red, and the call site cannot observe it because +/// the call site's own list is the thing being widened. +fn only_the_permitted_json_reader(found: &[PathBuf]) -> bool { + found == json_reading_modules().as_slice() +} + +/// **N1 — a second reader of a stranger's file is a second opinion about it, and +/// JSON must not spread before it is confined.** +/// +/// This is `f7_the_configuration_is_read_through_the_harness_and_never_parsed_ +/// here`'s rule applied to the other format, and it is written in the same +/// release as the first line that reads a stranger's JSON rather than after it. +/// The TOML rule was learned expensively: `src/edit.rs` is the one exemption and +/// it is held to properties, because a module that decides what somebody else's +/// file *means* is a second answer to a question that already has one. +/// +/// **Deserialization only, and the asymmetry is the whole argument.** The TOML +/// rule forbids `toml::from_str` — the parse — and says why in its own words at +/// `f7`'s comment: a `from_str` into a shape of our own is the second opinion. +/// Writing is not that. `src/exec.rs` builds `--json` event lines with +/// `serde_json::to_string` and `serde_json::json!`, reading nobody's file and +/// deciding nothing about anybody's format, and a gate that banned it would have +/// to exempt that module for a property which is not the one being protected — +/// an exemption granted for the wrong reason is how a permission list starts +/// widening itself. +#[test] +fn n1_json_is_deserialized_in_the_permitted_modules_and_nowhere_else() { + let permitted = json_reading_modules(); + let mut readers = Vec::new(); + + for (path, text) in sources() { + // Comments stripped, for the reason the TOML sweep strips them and the + // reason 0.16.0, 0.19.0, 0.25.0 and 0.26.0 each paid for: a gate that + // reads prose forbids a file from explaining itself, and this module's + // own documentation has to name the call it is permitted to make. + let code = code_of(&text); + + // Spelling the name around is the same act as writing it, exactly as it + // is for a spawn. `use serde_json as j` puts a parse in a file where the + // literal below never appears, so the aliased forms are forbidden + // everywhere — the permitted modules included, which therefore write the + // call out in full and are found by the sweep like anything else. + for evasion in [ + "use serde_json as ", + "use serde_json::{", + "use serde_json::*", + ] { + assert!( + !code.contains(evasion), + "{} imports serde_json under another spelling; a JSON parse is \ + written out in full or it is not written", + path.display(), + ); + } + + if [ + "serde_json::from_str", + "serde_json::from_slice", + "serde_json::from_reader", + ] + .iter() + .any(|call| code.contains(call)) + { + readers.push(path); + } + } + + readers.sort(); + assert!( + only_the_permitted_json_reader(&readers), + "JSON is deserialized somewhere other than the two modules permitted to do \ + it. Found {readers:?}; exactly these are permitted, by exact path: \ + {permitted:?} — one for the operator's own Claude and Codex files, one for \ + the foreign plugin manifests. A third is a second opinion about what \ + somebody else's file means, which is the defect the TOML rule beside this \ + one exists to prevent.", + ); +} + +/// **N1's second half — the permitted set is exact paths, and widening it is an +/// edit somebody makes on purpose.** +/// +/// The sibling arm — a third file parsing JSON — is covered by the sweep above. +/// This covers the arm that sweep cannot see, because a gate that has been +/// widened refuses nothing and therefore fails nothing. Each near-miss shares a +/// stem, a stem prefix or a module directory with a permitted path and is a +/// different file. +#[test] +fn n1_the_permitted_json_set_is_exact_paths_and_never_a_substring() { + let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let permitted = json_reading_modules(); + + // The control. Without it a predicate answering `false` for everything would + // satisfy every assertion below while refusing the real modules too. + assert!( + only_the_permitted_json_reader(&permitted), + "the permitted set does not admit itself, so nothing below means anything", + ); + + for near in [ + // Its stem opens with a permitted stem, which a `file_stem` prefix match + // admits. + src.join("adapter.rs"), + // A permitted stem, whole, followed by a separator of its own — what an + // extension-dropped text match admits. + src.join("adapt_hooks.rs"), + // A module directory named for a permitted file, which is what + // `p.starts_with(q.with_extension(""))` admits. + src.join("adapt").join("mod.rs"), + // And the same shapes against the other permitted module, so the property + // is asserted for both rather than for the new one only. + src.join("importer.rs"), + src.join("import").join("mod.rs"), + ] { + let mut widened = permitted.clone(); + widened.push(near.clone()); + widened.sort(); + assert!( + !only_the_permitted_json_reader(&widened), + "{} is admitted beside the permitted modules. The set is compared by \ + exact path for exactly this reason: a substring, stem or prefix match \ + is a permission list that widens itself, and a third module would then \ + read a stranger's JSON while this file went on passing.", + near.display(), + ); + } +} + #[test] fn n1_the_binary_is_named_io_and_there_is_exactly_one() { let manifest = manifest(); diff --git a/tests/fetch.rs b/tests/fetch.rs index 5a308a4..202fac2 100644 --- a/tests/fetch.rs +++ b/tests/fetch.rs @@ -389,3 +389,234 @@ fn f10_a_clone_that_works_is_renamed_into_place_and_the_staging_goes() { what it now has", ); } + +// --------------------------------------------------------------------------- +// 0.31.0 — F5 and F6. An index entry may live in another repository and may name +// the commit it means, and neither may weaken the shape asserted above. +// --------------------------------------------------------------------------- + +/// Every element of `argv` as a `String`, for reading in an assertion. +fn said(argv: &[OsString]) -> Vec { + argv.iter() + .map(|part| part.to_string_lossy().into_owned()) + .collect() +} + +/// **F6 — the five-element shape is what an unpinned fetch still runs.** +/// +/// `f10_the_program_is_the_literal_git_and_the_argv_is_five_owned_elements` above +/// asserts `argv` itself and is untouched by this release. This asserts the other +/// half of F6: that the pinned work did not quietly re-route the ordinary +/// marketplace add through some more general shape. A marketplace is added by +/// name, there is nothing to pin it to, and it must still be one shallow clone. +/// +/// Sabotage: give `Pin::Head` the four-step form. This fails on the step count, +/// and the assertion above it fails on the elements. +#[test] +fn f6_an_unpinned_fetch_is_still_one_shallow_clone_of_five_elements() { + let steps = fetch::steps( + "https://example.invalid/x.git", + Path::new("/tmp/x"), + &fetch::Pin::Head, + ); + + assert_eq!(steps.len(), 1, "one invocation, counted"); + assert_eq!( + said(&steps[0]), + vec![ + "clone".to_string(), + "--depth".to_string(), + "1".to_string(), + "https://example.invalid/x.git".to_string(), + "/tmp/x".to_string(), + ], + "and it is byte for byte what `argv` builds — the shape 0.29.0 argued for", + ); + assert_eq!( + steps[0], + fetch::argv("https://example.invalid/x.git", Path::new("/tmp/x")), + "asserted against `argv` itself as well as against the literal, so the two \ + cannot drift apart without one of these two assertions saying so", + ); +} + +/// **F5/F6 — a commit is a sequence, and the sequence is its own shape.** +/// +/// Four invocations rather than one, and that is git's constraint rather than a +/// choice: `git clone --revision` landed in 2.49 and this product names no git +/// floor. The five-element assertion above is **not** widened to accommodate it — +/// this is a different builder input with its own test, which is exactly what F6 +/// requires. +#[test] +fn f5_a_commit_pinned_fetch_names_the_sha_in_its_fetch_and_checks_out_fetch_head() { + let at = fetch::Pin::named(None, Some("30287f5e3f122a646d1ac5ca3ab96e130c52a3ad")) + .expect("a real sha out of the official index"); + let steps = fetch::steps( + "https://github.com/42Crunch-AI/claude-plugins.git", + Path::new("/tmp/x"), + &at, + ); + + assert_eq!( + steps.len(), + 4, + "init, remote add, a shallow fetch of the commit, a detached checkout", + ); + assert_eq!( + steps + .iter() + .filter( + |step| said(step).contains(&"30287f5e3f122a646d1ac5ca3ab96e130c52a3ad".to_string()) + ) + .count(), + 1, + "the sha appears in exactly one invocation — the fetch that asks for it. \ + Counted rather than `contains`ed, because a sha in two places is a sha in \ + a checkout that should be naming FETCH_HEAD", + ); + assert_eq!( + said(&steps[2]), + vec![ + "-C".to_string(), + "/tmp/x".to_string(), + "fetch".to_string(), + "--depth".to_string(), + "1".to_string(), + "origin".to_string(), + "30287f5e3f122a646d1ac5ca3ab96e130c52a3ad".to_string(), + ], + "shallow, so a pin costs no more history than an unpinned add does", + ); + assert!( + said(&steps[3]).contains(&"FETCH_HEAD".to_string()), + "the checkout names what came back rather than the commit again", + ); +} + +/// **F5 — a tag is one invocation, because `git clone` takes `--branch`.** +#[test] +fn f5_a_ref_pinned_fetch_is_one_clone_carrying_the_tag() { + let at = fetch::Pin::named(Some("v1.5.5"), None).expect("a real tag out of the official index"); + let steps = fetch::steps("https://github.com/x/y.git", Path::new("/tmp/x"), &at); + + assert_eq!(steps.len(), 1, "one invocation"); + assert_eq!( + said(&steps[0]), + vec![ + "clone".to_string(), + "--depth".to_string(), + "1".to_string(), + "--branch".to_string(), + "v1.5.5".to_string(), + "https://github.com/x/y.git".to_string(), + "/tmp/x".to_string(), + ], + ); +} + +/// **A commit beats a tag where an index names both, and it is not a preference.** +/// +/// A tag is a name its author can move afterwards; a commit is not. An index that +/// names both has named the commit it means, and 85 of the official index's +/// entries carry both. +#[test] +fn f5_a_commit_beats_a_tag_where_an_entry_names_both() { + assert_eq!( + fetch::Pin::named( + Some("v1.5.5"), + Some("30287f5e3f122a646d1ac5ca3ab96e130c52a3ad") + ), + Some(fetch::Pin::Commit( + "30287f5e3f122a646d1ac5ca3ab96e130c52a3ad".to_string() + )), + ); +} + +/// **A pin out of a stranger's file is judged before it becomes an argument.** +/// +/// The same rule `resolve` states for a name and for the same reason: a value +/// beginning with `-` is an *option* to a program that parses options, whatever +/// the argv looked like when it left the builder. +#[test] +fn f5_a_pin_that_could_become_an_argument_is_refused() { + for bad in [ + "--upload-pack=touch /tmp/pwned", + "-x", + "not-hex-at-all", + "abc", + ] { + assert_eq!( + fetch::Pin::named(None, Some(bad)), + None, + "{bad:?} is offered as a commit and must be refused rather than trimmed", + ); + } + for bad in ["--upload-pack=x", "-b", "", "..", "a/b"] { + assert_eq!( + fetch::Pin::named(Some(bad), None), + None, + "{bad:?} is offered as a ref and must be refused", + ); + } + assert_eq!( + fetch::Pin::named(None, None), + Some(fetch::Pin::Head), + "the control: an entry naming neither is the unpinned case and is not a \ + refusal. Without this a function answering None for everything would \ + satisfy every assertion above", + ); +} + +/// **The URL a stranger wrote is never the URL that reaches git.** +/// +/// `HOST`'s own documentation states the ceiling: the only string that can reach +/// the program is one this module built out of that constant, because an argument +/// somebody else chose is how `ext::sh -c …` becomes a remote shell. So an index +/// entry's `url` is taken apart, judged by `resolve`'s rules, and thrown away. +/// +/// The ceiling costs nothing that exists: all 238 remote entries of +/// `anthropics/claude-plugins-official` and all ten of +/// `obra/superpowers-marketplace` are two ordinary segments on this host. +#[test] +fn f5_a_remote_entrys_url_is_re_derived_and_never_passed_through() { + assert_eq!( + fetch::from_url("https://github.com/obra/superpowers.git"), + Some(Named { + owner: "obra".to_string(), + repo: "superpowers".to_string(), + }), + "the control — a real entry from a real index resolves", + ); + assert_eq!( + fetch::url(&fetch::from_url("https://github.com/obra/superpowers.git").expect("a name")), + "https://github.com/obra/superpowers.git", + "and what reaches git is rebuilt from the module's own constant, which for \ + a well-formed entry is the same string — the point is that it is a \ + different string's twin rather than the same string", + ); + + for refused in [ + // Another forge. io-cli's marketplace ceiling is one host and an entry + // inside a marketplace must not reach further than the marketplace could. + "https://gitlab.com/x/y.git", + // A host that merely begins with the permitted one. + "https://github.com.evil.test/x/y.git", + // Scheme downgrades and transports that are not fetches at all. + "http://github.com/x/y.git", + "git@github.com:x/y.git", + "ext::sh -c touch% /tmp/pwned", + // On the right host and still not two ordinary segments. + "https://github.com/x/y/z.git", + "https://github.com/x", + "https://github.com/-x/y.git", + "https://github.com/../y.git", + ] { + assert_eq!( + fetch::from_url(refused), + None, + "{refused:?} reached the name resolver. Every one of these is a url an \ + index could carry, and the module's stated invariant is that git \ + receives no string a stranger chose", + ); + } +} diff --git a/tests/glyphs.rs b/tests/glyphs.rs index fc47e52..5534bb8 100644 --- a/tests/glyphs.rs +++ b/tests/glyphs.rs @@ -896,6 +896,67 @@ fn memory_fixture() -> ( ) } +/// Every mark `/plugin` draws, taken off rows the surface itself produced. +/// +/// A `View` written out here rather than a configuration on disk, because what the +/// gate needs is one row of every kind in one list — loaded, switched off, refused +/// and adapted — and the adapted one is a root under the adapters directory, which +/// is a fact about a path rather than anything a fixture has to create. +/// +/// The ASCII set, because that is the set the gate is about. The marks carry no +/// set of their own — see `pluginview::LOADED_MARK` — and asserting them off the +/// substituted rendering is what says so. +fn plugin_marks() -> Vec<&'static str> { + let adapters = std::path::PathBuf::from("/home/someone/.io-cli/adapters"); + let bundle = |id: &str, root: std::path::PathBuf| io_cli::pluginview::Listed { + id: id.to_string(), + enabled: true, + description: Some("Everything our Rust reviews need.".to_string()), + version: None, + root, + contributions: vec!["skills"], + skills: None, + templates: None, + agents: Vec::new(), + servers: Vec::new(), + hooks: Vec::new(), + layers: Vec::new(), + }; + let mut off = bundle( + "tools-v1", + std::path::PathBuf::from("/home/someone/code/bundles/tools-v1"), + ); + off.enabled = false; + let view = io_cli::pluginview::View { + plugins: vec![ + bundle( + "rust-review", + std::path::PathBuf::from("/home/someone/code/bundles/rust-review"), + ), + off, + // `///`, which is the layout + // `adapt::at` writes and the only thing `Listed::adapted` reads. + bundle( + "claude-review", + adapters + .join("zeroonething") + .join("ultraship") + .join("claude-review"), + ), + ], + refused: vec![io_cli::pluginview::Refused { + id: "empty".to_string(), + path: std::path::PathBuf::from("/home/someone/code/bundles/empty"), + error: "no plugin.toml".to_string(), + }], + adapters: Some(adapters), + }; + io_cli::pluginview::rows(&view, 200, &ASCII) + .iter() + .filter_map(|row| row.mark) + .collect() +} + /// **N3 — the memory page draws in ASCII, at eighty columns and at thirty.** /// /// The sabotage arm is a pinned note marked with a glyph outside ASCII: the sweep @@ -907,9 +968,17 @@ fn memory_fixture() -> ( /// what is left after that, dropping it entirely when there is no room — so the /// mark, which is the whole of what says *pinned* and *not read*, is the one thing /// that cannot be squeezed out. +/// +/// The pairing block at the end is the registry for **every** mark this product +/// draws in a picker's mark column, `/plugin`'s four included: what is asserted of +/// a mark — one ASCII cell, drawn on a row a surface really produces, and different +/// from the marks it sits beside — is a property of the column rather than of the +/// memory page, and a second gate beside this one is a second place for the next +/// mark to be left out of. #[test] fn n3_the_memory_page_draws_in_ascii_and_fits_a_narrow_terminal() { use io_cli::commands::{memory_page, LOOSE_MARK, PINNED_MARK, READ_MARK, UNREAD_MARK}; + use io_cli::pluginview::{ADAPTED_MARK, DISABLED_MARK, LOADED_MARK, REFUSED_MARK}; let (files, entries) = memory_fixture(); let theme = ascii(); @@ -947,7 +1016,27 @@ fn n3_the_memory_page_draws_in_ascii_and_fits_a_narrow_terminal() { // mark to a space would pass the sweep above and destroy the page. let (rows, _) = memory_page(&files, &entries, false, &ASCII); let marks: Vec<&str> = rows.iter().filter_map(|row| row.mark).collect(); - for mark in [READ_MARK, UNREAD_MARK, PINNED_MARK, LOOSE_MARK] { + // `/plugin`'s own four, off the rows `pluginview::rows` drew for a list holding + // one bundle of every kind. A mark that is declared and that no surface ever + // puts on a row fails here rather than shipping as a constant nobody sees. + // **Each mark is looked for on the surface that draws it, not in the union.** + // `LOADED_MARK` is `+` and so is `READ_MARK`; `DISABLED_MARK` is `-` and so + // are `LOOSE_MARK` and `UNREAD_MARK`. Searching one merged set would find + // `/plugin`'s two commonest marks in the *memory page's* rows and pass however + // `/plugin` drew them — make every arm of its mark selector return + // `ADAPTED_MARK` and a union check notices nothing. Paired with its own + // surface, that sabotage fails here. + let drawn = plugin_marks(); + for (mark, on) in [ + (READ_MARK, &marks), + (UNREAD_MARK, &marks), + (PINNED_MARK, &marks), + (LOOSE_MARK, &marks), + (LOADED_MARK, &drawn), + (DISABLED_MARK, &drawn), + (REFUSED_MARK, &drawn), + (ADAPTED_MARK, &drawn), + ] { assert!(mark.is_ascii(), "{mark:?} is not ASCII"); assert_eq!( mark.chars().count(), @@ -955,8 +1044,8 @@ fn n3_the_memory_page_draws_in_ascii_and_fits_a_narrow_terminal() { "{mark:?} is not one cell; a mark of an odd width shifts the column", ); assert!( - marks.contains(&mark), - "{mark:?} is on no row of the fixture" + on.contains(&mark), + "{mark:?} is on no row of the surface that is supposed to draw it", ); } assert_ne!( @@ -964,6 +1053,19 @@ fn n3_the_memory_page_draws_in_ascii_and_fits_a_narrow_terminal() { "a pinned note and a loose one have to look different or the mark says nothing", ); assert_ne!(READ_MARK, UNREAD_MARK); + // **All four of `/plugin`'s, pairwise**, and not only the new one against the + // old three: they share one column on one list, so any two of them being the + // same character makes the column say less than an operator reads off it. + let plugin = [LOADED_MARK, DISABLED_MARK, REFUSED_MARK, ADAPTED_MARK]; + for (at, mark) in plugin.iter().enumerate() { + for other in &plugin[at + 1..] { + assert_ne!( + mark, other, + "two of `/plugin`'s marks are the same character, so a row wearing \ + it says two things at once", + ); + } + } } /// N3 — the two pickers `/remember` and `/memory` open, and every sentence they diff --git a/tests/install.rs b/tests/install.rs index bfe9864..5af7dbd 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -33,16 +33,34 @@ fn env_lock() -> MutexGuard<'static, ()> { /// A home of this test's own, put back when it drops. struct HomeFixture { dir: tempfile::TempDir, - previous: [(&'static str, Option); 2], + previous: [(&'static str, Option); 4], } impl HomeFixture { + /// **The fixture names the home in force as well as the operator's home, + /// because that is what `io` itself does.** `contract::default_skills` anchors + /// on `home::in_force` since 0.31.0 — a skill is authored content and belongs + /// wherever the operator put the rest of what they wrote — and in the binary + /// `home::adopt` runs immediately above the one `Config::discover` either arm + /// reaches and sets `IO_CONFIG_HOME` to `~/.io-cli`. + /// + /// A fixture that set only `HOME` would therefore be asserting about a state + /// the product never reaches, and would inherit whatever another test file had + /// last pointed the variable at — which is exactly how this test failed on the + /// first full-suite run after that change, with `configured` answering `None` + /// for a directory it had just installed five skills into. + /// + /// `IO_CONFIG` names a file outright and beats `IO_CONFIG_HOME`, so an + /// inherited one has to go. Every variable touched here is restored on drop. fn new() -> Self { let dir = tempfile::tempdir().expect("a home directory"); - let previous = ["HOME", "USERPROFILE"].map(|var| (var, std::env::var_os(var))); - for (var, _) in &previous { - std::env::set_var(var, dir.path()); - } + let home = dir.path().to_path_buf(); + let previous = ["HOME", "USERPROFILE", "IO_CONFIG_HOME", "IO_CONFIG"] + .map(|var| (var, std::env::var_os(var))); + std::env::set_var("HOME", &home); + std::env::set_var("USERPROFILE", &home); + std::env::set_var("IO_CONFIG_HOME", home.join(".io-cli")); + std::env::remove_var("IO_CONFIG"); Self { dir, previous } } diff --git a/tests/marketplace.rs b/tests/marketplace.rs index 0763d18..8a7127e 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -26,6 +26,36 @@ fn argv(words: &[&str]) -> Vec { words.iter().map(|word| word.to_string()).collect() } +/// The three directories an install may need, all inside one temporary tree. +/// +/// Owned here and borrowed at the call, because `marketplace::Homes` is three +/// borrows: a test that built it from temporaries inline would be building it out +/// of values that die at the semicolon. Held as a type of its own so a test says +/// *which* tree it is pointing io at, rather than repeating three joins. +struct Homes { + marketplaces: PathBuf, + staging: PathBuf, + adapters: PathBuf, +} + +impl Homes { + fn under(root: &Path) -> Self { + Self { + marketplaces: root.join("marketplaces"), + staging: root.join("staging"), + adapters: root.join("adapters"), + } + } + + fn at(&self) -> marketplace::Homes<'_> { + marketplace::Homes { + marketplaces: &self.marketplaces, + staging: &self.staging, + adapters: &self.adapters, + } + } +} + /// The name used throughout, spelled once. fn named() -> Named { Named { @@ -577,13 +607,16 @@ fn f3_removing_a_marketplace_leaves_every_declaration_alone() { 1, "exactly the bundle declared inside the clone depends on it: {depends:?}", ); - let warned = marketplace::warning(&depends).expect("a dependent bundle is warned about"); + // 0.31.0 gave `warning` a second list — the adapters a removal orphans, whose + // declarations point into the clone from outside it. This call passes an empty + // one, which is what a marketplace of native bundles costs. + let warned = marketplace::warning(&depends, &[]).expect("a dependent bundle is warned about"); assert!( warned.contains("left exactly as they are"), "the warning must say the entries survive: {warned}", ); assert!( - marketplace::warning(&[]).is_none(), + marketplace::warning(&[], &[]).is_none(), "an empty warning drawn as a row is a row an operator reads as a warning", ); @@ -841,8 +874,9 @@ fn f4_a_bundle_is_installed_by_name_and_install_is_add() { // which is the case a rule keyed on the word's shape gets wrong. let here = work.path().join("shared"); manifest(&here, "name = \"shared\"\n"); - let read_as = - marketplace::chosen(&here, || markets.clone(), "shared").expect("a real path is a path"); + let homes = Homes::under(work.path()); + let read_as = marketplace::chosen(&here, || markets.clone(), "shared", homes.at()) + .expect("a real path is a path"); assert_eq!( read_as, marketplace::Chosen::Path(here), @@ -853,7 +887,7 @@ fn f4_a_bundle_is_installed_by_name_and_install_is_add() { // know which was meant and reporting one half hides the other. let plain = work.path().join("nope"); std::fs::create_dir_all(&plain).expect("a directory that is not a bundle"); - let refusal = marketplace::chosen(&plain, || markets.clone(), "nope") + let refusal = marketplace::chosen(&plain, || markets.clone(), "nope", homes.at()) .expect_err("neither reading of `nope` holds"); assert!( refusal.contains(io_cli::pluginview::MANIFEST), @@ -1273,10 +1307,15 @@ fn holding(store: &Path) -> (Vec, PathBuf) { /// 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); - let chosen = marketplace::chosen(&work.join(word), || markets.to_vec(), word)?; + 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::disclosure(Scope::Local, chosen.dir())?; + marketplace::adapted_disclosure( + Scope::Local, + chosen.dir(), + (chosen.from() != chosen.dir()).then(|| chosen.from()), + )?; } let before = std::fs::read_to_string(&file).unwrap_or_default(); let text = io_cli::edit::apply( @@ -1314,13 +1353,24 @@ fn f6_the_install_discloses_the_harness_s_own_parse_before_it_writes() { // The name reading, decided against the disk by the one function that decides // it. The word names no directory here, so it can only be a name. + let homes = Homes::under(work.path()); let chosen = marketplace::chosen( &work.path().join("rust-review"), || markets.clone(), "rust-review", + homes.at(), ) .expect("the marketplace holds it"); - assert_eq!(chosen, marketplace::Chosen::Held(dir.clone())); + assert_eq!( + chosen, + marketplace::Chosen::Held(marketplace::Prepared { + declare: dir.clone(), + from: dir.clone(), + made: None, + }), + "a native bundle is declared at its own directory, io generates nothing \ + for it, and a declined install therefore has nothing to take back", + ); assert!( chosen.discloses(), "a bundle out of a marketplace was read as a directory the operator wrote, \ @@ -1867,12 +1917,21 @@ fn f16_and_f17_have_one_reader_and_one_validator() { unnecessary in 0.30.0 — and it was the last thing that wrote to an \ operator's configuration before they had consented to anything", ); + // **The needle moved with the call and the count did not, which is the point.** + // 0.31.0 replaced `disclosure` with `adapted_disclosure` at this one site — the + // second argument is the bundle's own directory, so a foreign bundle's hooks + // can be named while io-harness reads the generated manifest. The property + // being gated is unchanged: **one** validation stands between a stranger's + // directory and the operator's configuration file. Both spellings are counted + // together, so restoring the old call beside the new one fails here rather + // than passing because the needle only knew one of them. + let validations = planner.matches("marketplace::disclosure(").count() + + planner.matches("marketplace::adapted_disclosure(").count(); assert_eq!( - planner.matches("marketplace::disclosure(").count(), - 1, - "the validation that gates the write has {} call sites in src/manage.rs, not \ - 1 — a second is a second answer about whether a bundle may be declared", - planner.matches("marketplace::disclosure(").count(), + validations, 1, + "the validation that gates the write has {validations} call sites in \ + src/manage.rs, not 1 — a second is a second answer about whether a bundle \ + may be declared", ); // The hand reader is gone from the one file whose job was reading somebody @@ -1892,3 +1951,821 @@ fn f16_and_f17_have_one_reader_and_one_validator() { "the hook rows do not come from `Plugin::hooks()`", ); } + +// --------------------------------------------------------------------------- +// 0.31.0 — the three manifest formats, and the precedence between them. +// +// `tests/adapt.rs` asserts what the reader makes of each file. These assert what +// `marketplace::holdings` makes of a whole clone, which is the surface every +// listing and every install actually goes through. +// --------------------------------------------------------------------------- + +/// A clone directory, empty. +fn clone_dir() -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().expect("a clone directory"); + let root = dir.path().to_path_buf(); + (dir, root) +} + +/// Write `text` to `root/rel`, making the directories on the way. +fn write(root: &Path, rel: &str, text: &str) { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().expect("a parent")).expect("the directories"); + std::fs::write(&path, text).expect("the file"); +} + +#[test] +fn f1_a_clone_laid_out_like_ultraship_holds_one_bundle_where_it_held_none() { + let (_dir, root) = clone_dir(); + // `zeroonething/ultraship` as it actually is: no `.toml` anywhere, an index + // naming one plugin at the clone's own root, and ten skill directories. + write( + &root, + ".claude-plugin/marketplace.json", + r#"{ + "name": "ultraship", + "owner": { "name": "Aakash Pawar (zeroonething)" }, + "plugins": [ + { "name": "ultraship", "description": "Ship at inference speed.", "source": "./" } + ] +}"#, + ); + write( + &root, + ".claude-plugin/plugin.json", + r#"{ "name": "ultraship" }"#, + ); + for skill in ["brainstorm", "plan", "develop", "iterate", "complete"] { + write(&root, &format!("skills/{skill}/SKILL.md"), "# a skill\n"); + } + + let held = marketplace::holdings(&root); + + assert_eq!( + held.len(), + 1, + "exactly one bundle. Before 0.31.0 this returned none and the surface said \ + so — `plugin marketplace add zeroonething/ultraship` cloned the repository \ + and then answered that no directory in it carried a plugin.toml, which is \ + the defect this release exists to fix", + ); + assert_eq!(held[0].label(), "ultraship"); + assert_eq!( + held[0].origin, + marketplace::Origin::Adapted, + "and it says it is adapted; the difference between what an author wrote and \ + what io generated is never something an operator has to infer", + ); +} + +#[test] +fn f2_a_native_manifest_wins_its_own_directory_and_the_bundle_is_one() { + let (_dir, root) = clone_dir(); + manifest( + &root, + "name = \"native\"\ndescription = \"the author's own\"\n", + ); + write( + &root, + ".claude-plugin/plugin.json", + r#"{ "name": "foreign", "description": "somebody else's format" }"#, + ); + + let held = marketplace::holdings(&root); + + assert_eq!( + held.len(), + 1, + "two bundles for one directory is the failure this asserts against", + ); + assert_eq!(held[0].label(), "native", "the plugin.toml is the answer"); + assert_eq!(held[0].origin, marketplace::Origin::Native); +} + +#[test] +fn f2_a_root_plugin_toml_suppresses_the_index_and_leaves_the_walk_alone() { + let (_dir, root) = clone_dir(); + manifest(&root, "name = \"native-root\"\n"); + // An index naming a plugin that does not exist on disk. If it were read, the + // count below would be its length rather than what the walk finds. + write( + &root, + ".claude-plugin/marketplace.json", + r#"{ "plugins": [ { "name": "from-the-index", "source": "./" } ] }"#, + ); + manifest(&root.join("plugins").join("child"), "name = \"child\"\n"); + + let held = marketplace::holdings(&root); + let mut labels: Vec = held.iter().map(marketplace::Bundle::label).collect(); + labels.sort(); + + assert_eq!( + labels, + vec!["child".to_string(), "native-root".to_string()], + "an author who writes io's own manifest at the root has said what they \ + publish in the format this crate owns, and a foreign index must not speak \ + over it — but suppressing the index is all it does. A repository carrying \ + a root manifest and bundles beneath it lists all of them, exactly as it \ + did before this crate read any foreign format", + ); + assert!( + marketplace::unreadable(&root).is_empty(), + "and the suppressed index reports nothing either", + ); +} + +#[test] +fn f3_the_index_is_the_answer_and_the_walk_does_not_also_run() { + let (_dir, root) = clone_dir(); + write( + &root, + ".claude-plugin/marketplace.json", + r#"{ + "plugins": [ + { "name": "first", "source": "./plugins/first" }, + { "name": "second", "source": "./plugins/second" } + ] +}"#, + ); + // A third directory carrying a real `plugin.toml` the index does not name. + manifest(&root.join("plugins").join("third"), "name = \"third\"\n"); + + let held = marketplace::holdings(&root); + let labels: Vec = held.iter().map(marketplace::Bundle::label).collect(); + + assert_eq!( + held.len(), + 2, + "counted, not matched. A union of the index and the walk would list bundles \ + the author did not publish beside the ones they did, and an operator would \ + have no way to tell which was which — so the assertion is the count, and \ + `contains` would pass over exactly the defect it is written for", + ); + assert_eq!(labels, vec!["first".to_string(), "second".to_string()]); + assert!( + !labels.contains(&"third".to_string()), + "the walk did not also run", + ); +} + +#[test] +fn f4_two_codex_manifests_at_two_directories_the_walk_visits_are_both_found() { + let (_dir, root) = clone_dir(); + write( + &root, + "plugins/alpha/.codex-plugin/plugin.json", + r#"{ "name": "alpha" }"#, + ); + write( + &root, + "plugins/beta/.codex-plugin/plugin.json", + r#"{ "name": "beta" }"#, + ); + + let held = marketplace::holdings(&root); + let mut labels: Vec = held.iter().map(marketplace::Bundle::label).collect(); + labels.sort(); + + assert_eq!( + labels, + vec!["alpha".to_string(), "beta".to_string()], + "no index and no plugin.toml, so the walk runs and reads the foreign \ + manifest at each directory it already visits", + ); + assert!( + held.iter() + .all(|bundle| bundle.origin == marketplace::Origin::Adapted), + "both adapted", + ); +} + +#[test] +fn the_walk_still_never_descends_into_a_dot_directory() { + let (_dir, root) = clone_dir(); + // A bundle inside `.git` is what every clone would otherwise offer, and a + // `.claude-plugin` directory of its own is the shape that makes reading a + // known path relative to an admitted directory look like descending into one. + write( + &root, + ".git/modules/x/.claude-plugin/plugin.json", + r#"{ "name": "should-not-appear" }"#, + ); + write( + &root, + "plugins/real/.claude-plugin/plugin.json", + r#"{ "name": "real" }"#, + ); + + let labels: Vec = marketplace::holdings(&root) + .iter() + .map(marketplace::Bundle::label) + .collect(); + + assert_eq!( + labels, + vec!["real".to_string()], + "reading `.claude-plugin` at a known path relative to a directory the walk \ + already admitted is not the walk descending into a dot directory, and this \ + is the assertion that keeps the two apart", + ); +} + +#[test] +fn a_repository_that_is_no_bundle_at_all_says_so_without_naming_one_file() { + let (_dir, root) = clone_dir(); + write(&root, "README.md", "# not a bundle\n"); + + let market = Market { + named: named(), + root: root.clone(), + bundles: marketplace::holdings(&root), + }; + + let held = market.held(); + assert!( + held.contains(io_cli::pluginview::MANIFEST) + && held.contains(io_cli::adapt::INDEX_FILE) + && held.contains(io_cli::adapt::MANIFEST_FILE), + "three formats are read now, so the sentence names all three rather than \ + the one filename that was the answer for every marketplace in the field: \ + {held:?}", + ); +} + +// --------------------------------------------------------------------------- +// F10 — an index entry's name is a plugin id or it is refused by name. +// +// io-harness's `check_id` requires `[a-z0-9][a-z0-9-]{0,31}`, and `MAX_ID` is the +// 32. An index is somebody else's file and nothing in it was written against that +// rule, so the question is what io does with a name that is not one — and the +// answer these assert is: an ASCII case fold, and otherwise a refusal naming the +// entry. Mangling is what they are written to fail on. +// --------------------------------------------------------------------------- + +#[test] +fn f10_a_name_that_is_no_plugin_id_is_refused_by_name_and_never_mangled() { + let (_dir, root) = clone_dir(); + // Every character legal, and one too many of them — the length half of the + // rule has its own entry, because a build that dropped `MAX_ID` entirely + // would otherwise pass every assertion below. + let too_long = "marketplace-plugin-with-a-very-long-name"; + assert!( + too_long.len() > io_harness::MAX_ID, + "the fixture has to be over the bound to test it: {}", + too_long.len(), + ); + write( + &root, + ".claude-plugin/marketplace.json", + &format!( + r#"{{ + "plugins": [ + {{ "name": "My.Plugin", "source": "./plugins/mine" }}, + {{ "name": "{too_long}", "source": "./plugins/long" }}, + {{ "name": "ordinary", "source": "./plugins/ordinary" }} + ] +}}"# + ), + ); + + let labels: Vec = marketplace::holdings(&root) + .iter() + .map(marketplace::Bundle::label) + .collect(); + let said = marketplace::unreadable(&root); + + assert_eq!( + labels, + vec!["ordinary".to_string()], + "the whole list, not a `contains`: a mangled `my-plugin` and a truncated \ + id are both extra rows, and only the exact list fails on them. An id is \ + what an operator types at `plugin add` and what io-harness namespaces \ + every contribution with, so inventing one hands a person a name they \ + never saw in the index", + ); + assert_eq!( + said.len(), + 2, + "refused, never dropped — one line per entry that yields no bundle, and \ + two of the three yield none: {said:?}", + ); + assert_eq!( + said.iter() + .filter(|line| line.contains("My.Plugin")) + .count(), + 1, + "and the refusal names the entry as the index spells it, which is the only \ + word the operator can search their own file for: {said:?}", + ); + assert_eq!( + said.iter().filter(|line| line.contains(too_long)).count(), + 1, + "the name past MAX_ID is refused by name too, not silently cut to fit: \ + {said:?}", + ); +} + +#[test] +fn f10_two_entries_that_reach_one_id_are_refused_naming_both() { + let (_dir, root) = clone_dir(); + // Two spellings of one word, and a third entry that clashes with nothing. The + // third is what makes the count below decide something: without it, "one + // bundle" and "no bundles" would both be consistent with a rule that withheld + // only one of the pair. + write( + &root, + ".claude-plugin/marketplace.json", + r#"{ + "plugins": [ + { "name": "Reviewer", "source": "./plugins/one" }, + { "name": "REVIEWER", "source": "./plugins/two" }, + { "name": "formatter", "source": "./plugins/three" } + ] +}"#, + ); + + let labels: Vec = marketplace::holdings(&root) + .iter() + .map(marketplace::Bundle::label) + .collect(); + let said = marketplace::unreadable(&root); + + assert_eq!( + labels, + vec!["formatter".to_string()], + "neither half of the clash is offered. Installing whichever was found \ + first installs one repository's code under a name the operator believed \ + meant the other, and inside one index there is no qualifier to tell them \ + apart the way `locate` offers one for two marketplaces", + ); + assert_eq!( + said.len(), + 1, + "one refusal for the pair, not one each: {said:?}", + ); + // `Reviewer` and `REVIEWER` are each a substring of nothing else in the + // sentence — not of one another, and not of the id `reviewer` the two reach. + // A refusal that named only the id, or only the first entry, fails both. + assert!( + said[0].contains("Reviewer"), + "the refusal names the first entry as written: {said:?}", + ); + assert!( + said[0].contains("REVIEWER"), + "and the second, which is the half a first-match rule would never mention: \ + {said:?}", + ); +} + +#[test] +fn f10_a_name_that_is_already_an_id_is_untouched_and_a_case_fold_still_installs() { + let (_dir, root) = clone_dir(); + write( + &root, + ".claude-plugin/marketplace.json", + r#"{ + "plugins": [ + { "name": "rust-review", "source": "./plugins/one" }, + { "name": "Codex-Bridge", "source": "./plugins/two" } + ] +}"#, + ); + + let labels: Vec = marketplace::holdings(&root) + .iter() + .map(marketplace::Bundle::label) + .collect(); + + assert_eq!( + labels, + vec!["rust-review".to_string(), "codex-bridge".to_string()], + "the control the two refusals above are worthless without: a rule that \ + refused every name would satisfy both of them. A name that is already an \ + id comes through as the index wrote it, and a name that becomes one under \ + an ASCII case fold alone is folded — the one transformation a reader \ + undoes by eye", + ); + assert!( + marketplace::unreadable(&root).is_empty(), + "and nothing is reported, because nothing was withheld", + ); +} + +// --------------------------------------------------------------------------- +// F5 — an entry the index placed in another repository, brought down. +// +// The clone itself is `tests/fetch.rs`'s and the argv shapes are asserted there. +// What is asserted here is the decision `marketplace::fetched` makes before any +// of that: which entries need no fetch at all, and which are refused before a +// program is started rather than after. +// --------------------------------------------------------------------------- + +/// A bundle as an index entry produces one, with `source` supplied. +fn entry_bundle(dir: &Path, source: Option) -> marketplace::Bundle { + marketplace::Bundle { + dir: dir.to_path_buf(), + name: Some("reviewer".to_string()), + description: None, + origin: marketplace::Origin::Adapted, + source, + } +} + +/// **F13 — a removal names the adapters it orphans, and takes no entry away.** +/// +/// The failure this is written for is silence. `dependents` finds a declared +/// bundle whose *path is inside* the clone, which is every native install and no +/// adapted one: an adapted bundle is declared under `~/.io-cli/adapters` and only +/// its generated manifest points into the clone. So before this release the +/// warning for a marketplace of adapted bundles would have been `None`, and the +/// operator would have been told nothing at all. +/// +/// Sabotage: have `orphaned` return an empty vector. `dependents` still finds the +/// native bundle, `warning` still returns `Some`, and only the count and the +/// adapter's own path fail — which is why both are asserted rather than the +/// sentence merely being non-empty. +#[test] +fn f13_a_removal_names_the_adapters_it_orphans_and_leaves_every_entry() { + let (_dir, root) = clone_dir(); + let clone = root + .join("marketplaces") + .join("zeroonething") + .join("ultraship"); + let adapters = root.join("adapters"); + + // One native bundle inside the clone, and one adapter outside it whose + // generated manifest points back in. Both are declared; both stop loading. + manifest(&clone.join("native"), "name = \"native\"\n"); + std::fs::create_dir_all(clone.join("skills")).expect("the skills directory"); + let adapter = adapters + .join("zeroonething") + .join("ultraship") + .join("adapted"); + manifest( + &adapter, + &format!( + "name = \"adapted\"\nskills = {}\n", + io_cli::edit::spell(&clone.join("skills").to_string_lossy()), + ), + ); + // And a second adapter pointing at a different clone, which this removal must + // NOT name — without it, a rule that listed every adapter would pass. + let elsewhere = adapters.join("someone").join("else").join("untouched"); + manifest( + &elsewhere, + &format!( + "name = \"untouched\"\nskills = {}\n", + io_cli::edit::spell(&root.join("other").to_string_lossy()), + ), + ); + std::fs::create_dir_all(root.join("other")).expect("the other directory"); + + let scope = root.join("io.toml"); + let declared = format!( + "[[plugin]]\npath = {}\n\n[[plugin]]\npath = {}\n\n[[plugin]]\npath = {}\n", + io_cli::edit::spell(&clone.join("native").to_string_lossy()), + io_cli::edit::spell(&adapter.to_string_lossy()), + io_cli::edit::spell(&elsewhere.to_string_lossy()), + ); + std::fs::write(&scope, &declared).expect("the scope file"); + let config = io_harness::Config::from_toml(&declared).expect("the declarations parse"); + let view = io_cli::pluginview::view(&config); + + let orphans = marketplace::orphaned(&view, &clone, &adapters); + assert_eq!( + orphans.len(), + 1, + "exactly the adapter naming this clone — counted, so an implementation \ + listing every adapter fails here: {orphans:?}", + ); + assert_eq!(orphans[0], adapter, "and it is that adapter, by path"); + + let said = marketplace::removal_cost(&view, &clone, Some(&adapters)) + .expect("two declared bundles stop loading"); + assert!( + said.contains("2 declared bundles"), + "the native bundle and the adapted one are counted together, because the \ + difference between them is not something an operator asked about: {said}", + ); + assert!( + said.contains(&adapter.display().to_string()), + "the adapter is named, so the operator can find the file io wrote: {said}", + ); + assert!( + !said.contains(&elsewhere.display().to_string()), + "and the adapter for another clone is not: {said}", + ); + assert!( + said.contains("adapters/"), + "the sentence says what those paths are — an operator who goes looking \ + finds a plugin.toml io wrote in a directory they never made: {said}", + ); + + assert_eq!( + std::fs::read_to_string(&scope).expect("the scope file survives"), + declared, + "and not one byte of the configuration changed. Naming a consequence is \ + not acting on it, and a cache being emptied does not undo a decision the \ + operator made", + ); +} + +/// **N8 — a 291-entry index costs one file read, asserted as a shape and never +/// as a clock.** +/// +/// **`tests/timing.rs` forbids a test from measuring elapsed time, and it is +/// right.** The first draft of this test timed `holdings` against a two-second +/// ceiling; that gate caught it on the first full-suite run. A wall-clock +/// assertion is flaky on a loaded machine and tells you nothing about *why* a +/// path is fast, so what is asserted here is the cost's shape and the number is +/// recorded in the release record instead. +/// +/// The shape: the index path opens **one** file and descends into nothing, where +/// the walk it replaces is a recursive `read_dir` to `DEPTH`. A real bundle sits +/// three directories down and must not appear — if the walk had run, it would +/// have found it. That is a stronger statement than a duration, because it fails +/// for the reason a regression would actually have. +/// +/// The official marketplace's 291 entries are the first real input either path +/// has had at that size, which is why this is a recorded criterion rather than an +/// assumption. +#[test] +fn n8_an_index_of_291_entries_is_one_read_and_no_walk() { + let (_dir, root) = clone_dir(); + let entries: Vec = (0..291) + .map(|n| format!(r#"{{ "name": "plugin-{n}", "source": "./plugins/p{n}" }}"#)) + .collect(); + write( + &root, + ".claude-plugin/marketplace.json", + &format!(r#"{{ "plugins": [ {} ] }}"#, entries.join(",\n")), + ); + // A real bundle the index does not name, deep enough that a walk would have + // had to descend to reach it. + manifest( + &root.join("vendor").join("nested").join("deep"), + "name = \"only-a-walk-finds-me\"\n", + ); + + let held = marketplace::holdings(&root); + + assert_eq!( + held.len(), + 291, + "every entry the index names, and only those — counted", + ); + assert!( + !held + .iter() + .any(|bundle| bundle.label() == "only-a-walk-finds-me"), + "the walk did not run, which is where the cost of the old path was", + ); +} + +/// **F11 — every hook is disclosed, with its command unshortened and its reason.** +/// +/// The count is the assertion. A `contains` is satisfied by one row forever, and +/// the failure this is written for is a bundle whose second hook is not drawn — +/// which is exactly what a surface built by hand around "the hook" produces. +/// +/// The fixture's second command is 400 characters. `marketplace::LONGEST` bounds a +/// description because a description is prose a repository fills in; it must not +/// bound this, because this is argv an operator is being told will **not** run, +/// and a shortened argv on a consent surface is the one thing it must never show. +#[test] +fn f11_every_hook_a_foreign_bundle_declares_is_disclosed_with_its_reason() { + let (_dir, root) = clone_dir(); + let bundle = root.join("bundle"); + // A real `plugin.toml`, so io-harness loads the directory and the disclosure + // has something to be *beside*. The hooks below are the author's own file and + // are not in it — which is the whole point: asking io-harness what the bundle + // contributes cannot answer what it declared. + manifest( + &bundle, + "name = \"hooked\"\ndescription = \"a bundle with hooks\"\n", + ); + let long = "x".repeat(400); + write( + &bundle, + "hooks/hooks.json", + &format!( + r#"{{ "hooks": {{ + "SessionStart": [ {{ "hooks": [ + {{ "type": "command", "command": "\"${{CLAUDE_PLUGIN_ROOT}}/hooks/run.cmd\" start" }}, + {{ "type": "command", "command": "{long}" }} + ] }} ], + "Stop": [ {{ "hooks": [ {{ "type": "command", "command": "echo done" }} ] }} ] + }} }}"# + ), + ); + + let said = marketplace::adapted_disclosure(Scope::User, &bundle, Some(&bundle)) + .expect("io-harness loads the fixture"); + + assert_eq!( + said.withheld.len(), + 3, + "one line per hook, counted — three declared, three disclosed", + ); + assert_eq!( + said.withheld + .iter() + .filter(|line| line.contains(&long)) + .count(), + 1, + "the 400-character command is drawn whole. `LONGEST` bounds a description \ + and must not bound argv", + ); + assert!( + said.withheld + .iter() + .any(|line| line.contains("${CLAUDE_PLUGIN_ROOT}")), + "and the substitution is shown as the author wrote it, because that is what \ + would have run", + ); + assert_eq!( + said.withheld + .iter() + .filter(|line| line.contains(marketplace::NOT_CARRIED)) + .count(), + 3, + "every line carries the reason, not just the first — an operator reading \ + the third row must not have to infer it from the first", + ); + assert_eq!( + said.withheld + .iter() + .filter(|line| line.starts_with("Stop")) + .count(), + 1, + "a second event's hooks are disclosed too", + ); +} + +#[test] +fn f11_a_native_bundle_withholds_nothing_and_says_nothing() { + let (_dir, root) = clone_dir(); + let bundle = root.join("bundle"); + manifest(&bundle, "name = \"native\"\n"); + + let said = marketplace::disclosure(Scope::User, &bundle).expect("io-harness loads the fixture"); + + assert!( + said.withheld.is_empty(), + "a plugin.toml declares its hooks to io-harness, which runs them. Nothing \ + is being withheld and a line saying so would be a warning about nothing: \ + {:?}", + said.withheld, + ); + assert!( + !said.rows.is_empty(), + "the control — the disclosure itself still has something to say, so the \ + assertion above is not passing because the whole thing came back empty", + ); +} + +/// **A repository fetched for an index entry is not a marketplace of its own.** +/// +/// `marketplace::fetched` clones into `marketplaces/.entries//`, and +/// the dot is load-bearing rather than cosmetic: `markets` skips a dot-named +/// directory at both of its two levels, so what an entry pulled down can never be +/// counted as a marketplace the operator added. Inside the tree rather than beside +/// it so `plugin marketplace remove` still takes it away. +/// +/// Sabotage: name the directory `entries`. `markets` then answers two, and the +/// second is a repository nobody asked for, listed under an owner nobody typed. +#[test] +fn a_repository_fetched_for_an_entry_is_not_listed_as_a_marketplace() { + let (_dir, root) = clone_dir(); + manifest( + &root.join("zeroonething").join("ultraship"), + "name = \"real\"\n", + ); + // What a remote entry's fetch leaves behind, laid out exactly as `fetched` + // writes it. + manifest( + &root + .join(".entries") + .join("someone") + .join("their-plugin") + .join("bundle"), + "name = \"pulled-down-for-an-entry\"\n", + ); + + let found = marketplace::markets(&root); + + assert_eq!( + found.len(), + 1, + "one marketplace — the one the operator added. Counted, because a second \ + entry here is a repository they never named: {:?}", + found + .iter() + .map(marketplace::Market::name) + .collect::>(), + ); + assert_eq!(found[0].name(), "zeroonething/ultraship"); +} + +#[test] +fn f5_a_local_source_is_already_here_and_starts_no_program() { + let (_dir, root) = clone_dir(); + let here = root.join("plugins").join("reviewer"); + + assert_eq!( + marketplace::fetched( + &entry_bundle( + &here, + Some(io_cli::adapt::Source::Local( + "./plugins/reviewer".to_string() + )) + ), + &named(), + &root.join("marketplaces"), + &root.join("staging"), + ), + Ok(here.clone()), + "53 of the official index's 291 entries are this shape, and none of them \ + needs git at all", + ); + assert_eq!( + marketplace::fetched( + &entry_bundle(&here, None), + &named(), + &root.join("marketplaces"), + &root.join("staging") + ), + Ok(here), + "and a bundle the walk found carries no source, which is the same answer", + ); + assert!( + !root.join("staging").exists() && !root.join("marketplaces").exists(), + "neither path was so much as created — nothing was fetched", + ); +} + +#[test] +fn f5_a_remote_entry_on_another_host_is_refused_before_anything_is_started() { + let (_dir, root) = clone_dir(); + + for url in [ + "https://gitlab.com/x/y.git", + "https://github.com.evil.test/x/y.git", + "ext::sh -c touch% /tmp/pwned", + "https://github.com/x/y/z.git", + ] { + let bundle = entry_bundle( + &root, + Some(io_cli::adapt::Source::Remote(io_cli::adapt::Remote { + url: url.to_string(), + path: None, + reference: None, + sha: None, + })), + ); + let said = marketplace::fetched( + &bundle, + &named(), + &root.join("marketplaces"), + &root.join("staging"), + ) + .expect_err("a url io does not read is a refusal"); + assert!( + said.contains(io_cli::fetch::HOST), + "the refusal says what io does read, so it is actionable rather than a \ + dead end: {said:?}", + ); + } + assert!( + !root.join("staging").exists(), + "and no clone was staged for any of them — the refusal is before the \ + program, not after it", + ); +} + +#[test] +fn f5_a_pin_that_could_become_an_argument_refuses_the_whole_entry() { + let (_dir, root) = clone_dir(); + let bundle = entry_bundle( + &root, + Some(io_cli::adapt::Source::Remote(io_cli::adapt::Remote { + url: "https://github.com/x/y.git".to_string(), + path: None, + reference: Some("--upload-pack=touch /tmp/pwned".to_string()), + sha: None, + })), + ); + + let said = marketplace::fetched( + &bundle, + &named(), + &root.join("marketplaces"), + &root.join("staging"), + ) + .expect_err("a pin that is an option is a refusal"); + assert!( + said.contains("reviewer"), + "the refusal names the bundle it withheld: {said:?}", + ); + assert!(!root.join("staging").exists(), "and nothing was started",); +} diff --git a/tests/plugins.rs b/tests/plugins.rs index 940d4ff..9bcbcab 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -1526,3 +1526,193 @@ fn f7_a_switched_off_bundle_reaches_no_turn_while_its_directories_still_read() { pane can say what switching it on would bring", ); } + +// --------------------------------------------------------------------------- +// F14 — a bundle whose manifest io wrote is drawn as one +// --------------------------------------------------------------------------- + +/// Where a generated manifest sits, under the fixture's own adapters root. +/// +/// `///`, which is the layout `adapt::at` writes and +/// the only thing `Listed::adapted` reads. A real adapter is written under +/// `home::adapters()`; the prefix is what decides the answer, so a fixture that +/// reproduces the shape reproduces the case without moving anybody's home. +const ADAPTED_AT: &str = "adapters/zeroonething/ultraship/claude-review"; + +/// The bundle id the generated manifest declares. +const ADAPTED: &str = "claude-review"; + +/// A manifest of the shape `adapt::generate` writes: a name, the two directories it +/// found in the clone, and a description long enough to compete with the root for +/// what an eighty-column row has left. +/// +/// The description is deliberately not the bundle's name, so a row's detail +/// containing `claude-review` can only have got it from the path. +const GENERATED: &str = r#" +name = "claude-review" +description = "A code review bundle published for another agent, adapted by io." +skills = "skills" +templates = "templates" +"#; + +/// A root declaring one adapted bundle and one native one, in that order. +fn one_adapted_one_native() -> (tempfile::TempDir, pluginview::View) { + let (dir, root) = root(); + 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 mut view = pluginview::view(&config); + view.adapters = Some(adapters_of(&view)); + (dir, view) +} + +/// The adapters root **as io-harness resolved it**, taken back off the bundle's own +/// root rather than rebuilt from the fixture's directory. +/// +/// A temporary directory is reached through a symlink on macOS — `/var/folders/…` +/// and `/private/var/folders/…` are the same directory — and `Path::starts_with` +/// compares components, so a prefix assembled here from `root()` would not be a +/// prefix of the path the harness read and every row would draw as native for a +/// reason that has nothing to do with the criterion. +fn adapters_of(view: &pluginview::View) -> PathBuf { + listed(view, ADAPTED) + .root + .ancestors() + .nth(3) + .expect("`///` has three directories above it") + .to_path_buf() +} + +/// **F14.** An adapted bundle draws under its own mark and the native bundle +/// listed beside it does not. +/// +/// Asserted as the list of labels wearing `ADAPTED_MARK` rather than as a +/// `contains`, because both failures this gate exists for produce a list that still +/// contains the right row: the mark drawn on nothing gives an empty list, and the +/// mark drawn on everything gives a list of two. A fixture holding one bundle of +/// each kind is what makes the second one visible at all. +/// +/// Sabotage: drop the `adapted` arm from the mark in `pluginview::rows`. The +/// adapted bundle draws `+` like any other, an operator has no way to tell a +/// manifest io wrote from one a person did, and the generated file they must open +/// when io-harness refuses the bundle is named nowhere. +#[test] +fn f14_an_adapted_bundle_draws_its_own_mark_and_a_native_one_beside_it_does_not() { + let (_dir, view) = one_adapted_one_native(); + + for glyphs in [&io_cli::glyphs::UNICODE, &io_cli::glyphs::ASCII] { + // Wide enough that nothing is shortened, so what is read off the row is + // the mark rather than what survived the fitter. + let rows = pluginview::rows(&view, 400, glyphs); + let marked: Vec<&str> = rows + .iter() + .filter(|row| row.mark == Some(pluginview::ADAPTED_MARK)) + .map(|row| row.label.as_str()) + .collect(); + assert_eq!( + marked, + vec![ADAPTED], + "{}: the adapted mark is on {} of the drawn rows rather than on the one \ + bundle io wrote a manifest for; drawn: {:?}", + glyphs.name, + marked.len(), + rows.iter() + .map(|row| (row.mark, row.label.clone())) + .collect::>(), + ); + + let native = rows + .iter() + .find(|row| row.label == "rust-review") + .expect("the native bundle drew a row"); + assert_eq!( + native.mark, + Some(pluginview::LOADED_MARK), + "{}: a bundle whose author wrote its `plugin.toml` is drawn as adapted", + glyphs.name, + ); + } +} + +/// **F14, the half a mark cannot carry.** An adapted row names the directory the +/// generated manifest is in, at the width the row is actually drawn at. +/// +/// One character says *io wrote this manifest*; it does not say **where**, and the +/// generated file is under a directory the operator never typed. Eighty columns +/// rather than four hundred, because a fact that is only true on a wide terminal is +/// not one an operator can rely on: the root has a floor and a reserved place ahead +/// of the description, which is a line io copied out of somebody else's metadata. +/// +/// Sabotage: drop the reservation in `pluginview::rows` and fit the description +/// against the whole of what is left. This description is longer than the room, so +/// it takes all of it, the root falls below its floor and is dropped whole, and the +/// row that carries the mark stops carrying the path the mark is about. +#[test] +fn f14_an_adapted_row_names_the_directory_the_generated_manifest_is_in() { + let (_dir, view) = one_adapted_one_native(); + + for glyphs in [&io_cli::glyphs::UNICODE, &io_cli::glyphs::ASCII] { + let rows = pluginview::rows(&view, 80, glyphs); + let row = rows + .iter() + .find(|row| row.label == ADAPTED) + .expect("the adapted bundle drew a row"); + let detail = row.detail.clone().expect("a listed bundle has a detail"); + assert!( + detail.contains(ADAPTED), + "{}: the row wears the adapted mark and does not say which directory \ + the generated manifest is in: {detail}", + glyphs.name, + ); + // The picker's own budget: marker, label, gap, detail. A row that names the + // path by overrunning the terminal has not named it. + assert!( + row.label.chars().count() + detail.chars().count() + 4 <= 80, + "{}: the row does not fit eighty columns: {detail}", + glyphs.name, + ); + } +} + +/// **The one thing `ADAPTED_MARK` taking the column costs, asserted rather than +/// argued.** +/// +/// A row carries one mark, so an adapted bundle that is also **switched off** +/// draws `~` and not `-`. `ADAPTED_MARK`'s own rustdoc says that is safe because a +/// switched-off bundle already leads its detail with the words — and a sentence in +/// a doc comment is not a gate. This is the gate: the state has to survive the +/// column being spent, or the trade the constant argues for is one that lost an +/// operator a fact. +/// +/// Sabotage: stop leading a switched-off row's detail with its state. Every other +/// bundle in the product keeps its `-`, so nothing else in the suite goes red — +/// only the row where the mark was spent on something else. +#[test] +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 mut view = pluginview::view(&config); + view.adapters = Some(adapters_of(&view)); + + let rows = pluginview::rows(&view, 200, &io_cli::glyphs::ASCII); + let row = rows + .iter() + .find(|row| row.label == ADAPTED) + .expect("the switched-off adapted bundle is listed at all"); + + assert_eq!( + row.mark, + Some(pluginview::ADAPTED_MARK), + "the adapted mark takes the column from the state mark — that is the \ + decision this test exists to hold to its own terms", + ); + let detail = row.detail.as_deref().unwrap_or_default(); + assert!( + detail.starts_with("switched off"), + "so the state must lead the detail, or spending the column lost it: \ + {detail:?}", + ); +}