From ea267953848df29f455f3a1aff9d0a3bec5e4443 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 11:34:58 +0530 Subject: [PATCH 01/12] feat(adapt): read Claude Code and Codex plugin manifests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `src/adapt.rs`, the crate's reader for the three manifest formats a capability bundle is actually published in: `.claude-plugin/marketplace.json` as a repository's index, and `.claude-plugin/plugin.json` or `.codex-plugin/plugin.json` as a bundle manifest. Every value it returns has been through `marketplace::plain` and `marketplace::bounded`, except a hook's command, which is filtered and deliberately not shortened because it is argv an operator is asked to consent to. The two remote source tags — `url` and `git-subdir` — are read as one shape keyed on the presence of a `url` rather than on the tag. They carry the same keys in the field, and a second reader for a second name is the shape this crate has paid for before. An entry in a shape the reader does not know costs that entry and not the file: the index reports it with its reason rather than dropping it, because a shape no fixture anticipated is found by an operator and a silent skip makes that unreportable. `tests/dependencies.rs` gains the JSON rule in the same commit as the first line that reads a stranger's JSON, rather than after it. It permits `serde_json::from_str`, `from_slice` and `from_reader` at an exact set of two paths compared with `==` — `src/import.rs`, which has read the operator's own Claude and Codex files since 0.21.0, and `src/adapt.rs` — with a near-miss test so that widening the set to a stem or substring match goes red instead of going vacuously green. Serialization is not gated: the TOML rule beside it forbids the parse and says why, and `src/exec.rs` writing its own `--json` event lines is not a second opinion about anybody's file. `contract::default_skills` now anchors on `home::in_force` rather than `home::path`, so the skills directory follows `$IO_CONFIG_HOME` exactly as the operator's memory note already did. A skill is authored content, not a cache, and this is the release that writes absolute paths into a home — two answers to which home would have been written into a second surface and then described. `home::marketplaces`'s rustdoc cited the old behaviour as its precedent and now states its own reason instead. `HomeFixture` names the home in force as well as the operator's home, which is what `home::adopt` does in the binary. --- Cargo.toml | 2 +- src/adapt.rs | 430 ++++++++++++++++++++++++++++++++++++++++++ src/contract.rs | 17 +- src/home.rs | 13 +- src/lib.rs | 1 + src/marketplace.rs | 2 +- tests/adapt.rs | 409 +++++++++++++++++++++++++++++++++++++++ tests/contract.rs | 85 ++++++++- tests/dependencies.rs | 144 ++++++++++++++ 9 files changed, 1086 insertions(+), 17 deletions(-) create mode 100644 src/adapt.rs create mode 100644 tests/adapt.rs 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/src/adapt.rs b/src/adapt.rs new file mode 100644 index 0000000..65ab782 --- /dev/null +++ b/src/adapt.rs @@ -0,0 +1,430 @@ +//! 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. + +use std::path::{Path, PathBuf}; + +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 { + // Filtered, never bounded. The event is the file's own word + // and is bounded, because it is a label rather than argv. + event: bounded(&plain(&event)), + command: plain(&command), + }); + } + } + } + found +} + +/// 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..e111a57 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::in_force()?.0.join("skills"); dir.is_dir().then_some(dir) } diff --git a/src/home.rs b/src/home.rs index ba5e631..2393972 100644 --- a/src/home.rs +++ b/src/home.rs @@ -206,12 +206,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 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/marketplace.rs b/src/marketplace.rs index d776f36..b3c5360 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -331,7 +331,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(); } diff --git a/tests/adapt.rs b/tests/adapt.rs new file mode 100644 index 0000000..226addd --- /dev/null +++ b/tests/adapt.rs @@ -0,0 +1,409 @@ +//! 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. + +use std::path::{Path, PathBuf}; + +use io_cli::adapt::{self, Source}; + +/// 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(), + ); +} 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..6fc1e89 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -1036,6 +1036,150 @@ 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(); From 9e8b25b296d9dfc99e85493a61b82c0512baba18 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 11:36:44 +0530 Subject: [PATCH 02/12] chore: lockfile for the 0.31.0 version bump --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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", From 05f636043ff1db76359e826a4c0559b2470b6b39 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 11:41:57 +0530 Subject: [PATCH 03/12] feat(marketplace): stock a marketplace from the formats the field publishes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `marketplace::holdings` reads three manifest formats with a stated precedence. A native `plugin.toml` at the clone's root suppresses a foreign index and nothing else, so an author who writes io's own manifest has said what they publish and a stranger's index cannot speak over it — while a repository carrying a root manifest and bundles beneath it lists all of them exactly as it did before. Where there is no root manifest, `.claude-plugin/marketplace.json` is the answer and the walk does not also run: a union would list bundles the author did not publish beside the ones they did, with no way to tell which was which, so the test counts rather than matching. Where there is neither, the walk reads each directory as a native manifest first and a foreign one only where it carries none. `pluginview::candidates` is now `pluginview::visited` filtered by "carries a manifest", which is what the walk always was. The split is load-bearing rather than tidying: a directory holding only a `.claude-plugin/plugin.json` carries no `plugin.toml`, so it was never in `candidates`'s answer, and a reader iterating that list for foreign manifests would have found one exactly never. The skips are unchanged, `.git` included, and a test asserts that reading a known path inside an already-admitted directory is not the walk descending into a dotted one. `Bundle` gains an origin and the source an index named. A bundle whose source is another repository has no local directory until an install fetches it, so its `dir` is the clone the index is in and the source is what says so. A local source is resolved by dropping every component that is not a plain name, so a source of `../../etc` addresses nothing outside the clone. `Bundle::line` names the file io actually read rather than always saying `plugin.toml`, and `Market::held`'s zero case names all three shapes — it used to say "no directory in it carries a plugin.toml", which was true and was the answer for every marketplace in the field. --- src/marketplace.rs | 183 +++++++++++++++++++++++++++++++-- src/pluginview.rs | 31 +++++- tests/marketplace.rs | 233 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 433 insertions(+), 14 deletions(-) diff --git a/src/marketplace.rs b/src/marketplace.rs index b3c5360..f383dd0 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,28 @@ 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 { + match self.origin { + Origin::Native => MANIFEST, + Origin::Adapted => crate::adapt::MANIFEST_FILE, } } } @@ -251,7 +290,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 +336,69 @@ 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`]. +/// +/// 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. +fn from_entry(clone: &Path, 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(entry.name), + description: entry.description, + origin: Origin::Adapted, + source: Some(entry.source), + } +} + +/// 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 @@ -450,13 +558,37 @@ 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. +/// 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 +596,41 @@ 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 index + .entries + .into_iter() + .map(|entry| from_entry(clone, entry)) + .collect(); + } + } + // `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() +} + +/// What an index said it could not read, for the surface that lists it. +/// +/// 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| index.unreadable) + .unwrap_or_default() } /// Every marketplace under `root`, ordered by owner and then by repository. diff --git a/src/pluginview.rs b/src/pluginview.rs index e5d3f21..3bc7d87 100644 --- a/src/pluginview.rs +++ b/src/pluginview.rs @@ -774,6 +774,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 +817,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)); } diff --git a/tests/marketplace.rs b/tests/marketplace.rs index 0763d18..f7fc018 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -1892,3 +1892,236 @@ 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:?}", + ); +} From 8772fc31a615c77d1bae213ca9acc67e76b0ca21 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 11:55:48 +0530 Subject: [PATCH 04/12] feat(fetch): pin a remote bundle to what its index named MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An index entry may place a plugin in another repository, and 238 of the official marketplace's 291 entries do. Every one of them names a commit, so a fetch that ignored it would install something other than what the index pinned. `git clone --revision` landed in git 2.49 and this product names no git floor, so a commit-pinned shallow fetch is four invocations — init, remote add, a shallow fetch of the commit, a detached checkout of what came back — and a tag-pinned one is a single `clone --branch`. `fetch::steps` builds all three shapes and `fetch::argv` is unchanged, so an ordinary marketplace add still runs the same five elements it has since 0.29.0, asserted both against the literal and against `argv` itself. The URL a stranger wrote never reaches git. `HOST`'s own documentation states that the only string reaching the program is one this module built from 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 by `fetch::from_url`, judged by `resolve`'s existing rules, and thrown away; `url` rebuilds it. A url on another host, or one that is not two ordinary path segments, is refused. All 238 remote entries of the official index and all ten of superpowers-marketplace are two segments on the permitted host, so the ceiling refuses nothing that exists. A `sha` and a `ref` are argv elements out of the same file and are judged the same way: a value beginning with `-` is an option to a program that parses options. `tests/dependencies.rs`'s F10 gate is re-argued rather than relaxed. The spawn can no longer call one builder inline, because a pinned fetch is a loop over a list of finished argvs, so `.args(argv(` is replaced by a single `run` taking a `Vec` parameter — which is stronger: the spawn never sees the parts and cannot add an element even in principle. Two builder signatures are pinned where one was, the spawn count stays at one, and `.args(` is now asserted to appear exactly once. --- src/fetch.rs | 207 ++++++++++++++++++++++++++++++++++++---- tests/dependencies.rs | 44 ++++++++- tests/fetch.rs | 217 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 447 insertions(+), 21 deletions(-) diff --git a/src/fetch.rs b/src/fetch.rs index 256274b..0ce0b4d 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -353,6 +353,140 @@ 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 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) => { + let mut one = vec![ + OsString::from("clone"), + OsString::from("--depth"), + OsString::from("1"), + OsString::from("--branch"), + OsString::from(reference), + OsString::from(url), + dir, + ]; + one.shrink_to_fit(); + vec![one] + } + 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 +501,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 +546,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 +581,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/tests/dependencies.rs b/tests/dependencies.rs index 6fc1e89..5444710 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.", ); } diff --git a/tests/fetch.rs b/tests/fetch.rs index 5a308a4..63f7877 100644 --- a/tests/fetch.rs +++ b/tests/fetch.rs @@ -389,3 +389,220 @@ 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", + ); + } +} From 52cfc472f09a5145868018efb1089508f633b388 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 12:01:00 +0530 Subject: [PATCH 05/12] feat(marketplace): derive a bundle id, refuse a mangled one, fetch a remote entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An index entry's name becomes the id an operator types at `plugin add` and the prefix io-harness namespaces every contribution with, so `adapt::normalised` accepts an ASCII case fold and nothing else. `My.Plugin` is refused by name rather than installed as `my-plugin`: a mapping that dropped or invented a character would give a person one name in the author's file, a second in the listing and a third inside every namespaced contribution. Two entries in one index reaching the same id are refused naming both, in the sentence shape `locate` already uses for a bare name two marketplaces carry — parting from it only where the two genuinely differ, since inside one index there is no qualifier that tells them apart and only the index's author can resolve it. The rule is stated against `io_harness::MAX_ID` rather than a copied number. `marketplace::fetched` brings down an entry the index placed elsewhere. A local source is already on the disk and starts no program, which is 53 of the official index's 291 entries. The rest clone into a dot-named directory inside the marketplaces tree, so `markets` cannot count one as a marketplace of its own and `plugin marketplace remove` still takes it away. A url on another host, or one that is not two ordinary path segments, is refused before anything is started and the refusal says what io does read; a `ref` or `sha` that could become an option refuses the whole entry the same way. `fetch::HOST` is public so the refusal can name it rather than spelling it a second time. --- src/adapt.rs | 53 +++++++- src/fetch.rs | 30 ++--- src/marketplace.rs | 174 ++++++++++++++++++++++++-- tests/dependencies.rs | 10 +- tests/fetch.rs | 22 +++- tests/marketplace.rs | 275 +++++++++++++++++++++++++++++++++++++++++- 6 files changed, 525 insertions(+), 39 deletions(-) diff --git a/src/adapt.rs b/src/adapt.rs index 65ab782..cbc6c65 100644 --- a/src/adapt.rs +++ b/src/adapt.rs @@ -264,6 +264,52 @@ pub fn hooks_in(file: &Path) -> Vec { 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) +} + /// A read value, filtered, bounded, and `None` where it is blank. /// /// The same collapse [`crate::marketplace`]'s own reader makes and for the same @@ -346,10 +392,9 @@ 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()), - ), + 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(), } } diff --git a/src/fetch.rs b/src/fetch.rs index 0ce0b4d..97ddd32 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. /// @@ -444,21 +444,21 @@ 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) => { - let mut one = vec![ - OsString::from("clone"), - OsString::from("--depth"), - OsString::from("1"), - OsString::from("--branch"), - OsString::from(reference), - OsString::from(url), - dir, - ]; - one.shrink_to_fit(); - vec![one] - } + 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("init"), + OsString::from("--quiet"), + dir.clone(), + ], vec![ OsString::from("-C"), dir.clone(), diff --git a/src/marketplace.rs b/src/marketplace.rs index f383dd0..634aac3 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -359,27 +359,114 @@ pub fn adapted(dir: &Path) -> Option { }) } -/// One index entry as a [`Bundle`]. +/// 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. -fn from_entry(clone: &Path, entry: crate::adapt::Entry) -> Bundle { +/// +/// **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(entry.name), + 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 @@ -575,7 +662,9 @@ fn escapes(text: &str) -> String { /// 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. +/// 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. /// @@ -598,11 +687,7 @@ fn escapes(text: &str) -> String { pub fn holdings(clone: &Path) -> Vec { if !clone.join(MANIFEST).is_file() { if let Some(index) = crate::adapt::index_at(clone) { - return index - .entries - .into_iter() - .map(|entry| from_entry(clone, entry)) - .collect(); + return published(clone, index).0; } } // `visited` and not `candidates`: the latter is this filtered by "carries a @@ -616,7 +701,12 @@ pub fn holdings(clone: &Path) -> Vec { .collect() } -/// What an index said it could not read, for the surface that lists it. +/// 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 @@ -629,7 +719,7 @@ pub fn unreadable(clone: &Path) -> Vec { return Vec::new(); } crate::adapt::index_at(clone) - .map(|index| index.unreadable) + .map(|index| published(clone, index).1) .unwrap_or_default() } @@ -685,6 +775,68 @@ 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"; + +/// 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, 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 + })?; + + let into = at(marketplaces.join(ENTRIES).as_path(), &named); + 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 diff --git a/tests/dependencies.rs b/tests/dependencies.rs index 5444710..7fa3760 100644 --- a/tests/dependencies.rs +++ b/tests/dependencies.rs @@ -1149,9 +1149,13 @@ fn n1_json_is_deserialized_in_the_permitted_modules_and_nowhere_else() { ); } - if ["serde_json::from_str", "serde_json::from_slice", "serde_json::from_reader"] - .iter() - .any(|call| code.contains(call)) + if [ + "serde_json::from_str", + "serde_json::from_slice", + "serde_json::from_reader", + ] + .iter() + .any(|call| code.contains(call)) { readers.push(path); } diff --git a/tests/fetch.rs b/tests/fetch.rs index 63f7877..202fac2 100644 --- a/tests/fetch.rs +++ b/tests/fetch.rs @@ -414,7 +414,11 @@ fn said(argv: &[OsString]) -> Vec { /// 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); + 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!( @@ -461,7 +465,9 @@ fn f5_a_commit_pinned_fetch_names_the_sha_in_its_fetch_and_checks_out_fetch_head assert_eq!( steps .iter() - .filter(|step| said(step).contains(&"30287f5e3f122a646d1ac5ca3ab96e130c52a3ad".to_string())) + .filter( + |step| said(step).contains(&"30287f5e3f122a646d1ac5ca3ab96e130c52a3ad".to_string()) + ) .count(), 1, "the sha appears in exactly one invocation — the fetch that asks for it. \ @@ -516,7 +522,10 @@ fn f5_a_ref_pinned_fetch_is_one_clone_carrying_the_tag() { #[test] fn f5_a_commit_beats_a_tag_where_an_entry_names_both() { assert_eq!( - fetch::Pin::named(Some("v1.5.5"), Some("30287f5e3f122a646d1ac5ca3ab96e130c52a3ad")), + fetch::Pin::named( + Some("v1.5.5"), + Some("30287f5e3f122a646d1ac5ca3ab96e130c52a3ad") + ), Some(fetch::Pin::Commit( "30287f5e3f122a646d1ac5ca3ab96e130c52a3ad".to_string() )), @@ -530,7 +539,12 @@ fn f5_a_commit_beats_a_tag_where_an_entry_names_both() { /// 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"] { + for bad in [ + "--upload-pack=touch /tmp/pwned", + "-x", + "not-hex-at-all", + "abc", + ] { assert_eq!( fetch::Pin::named(None, Some(bad)), None, diff --git a/tests/marketplace.rs b/tests/marketplace.rs index f7fc018..175a46e 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -1931,7 +1931,11 @@ fn f1_a_clone_laid_out_like_ultraship_holds_one_bundle_where_it_held_none() { ] }"#, ); - write(&root, ".claude-plugin/plugin.json", r#"{ "name": "ultraship" }"#); + 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"); } @@ -1958,7 +1962,10 @@ fn f1_a_clone_laid_out_like_ultraship_holds_one_bundle_where_it_held_none() { #[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"); + manifest( + &root, + "name = \"native\"\ndescription = \"the author's own\"\n", + ); write( &root, ".claude-plugin/plugin.json", @@ -2125,3 +2132,267 @@ fn a_repository_that_is_no_bundle_at_all_says_so_without_naming_one_file() { {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, + } +} + +#[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()))), + &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), &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, &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, &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", + ); +} From 74a5de4c62979f4134ba5aa5b36d0590a0745ab7 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 12:20:30 +0530 Subject: [PATCH 06/12] feat(adapt): generate a manifest io-harness loads, and disclose what cannot cross MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `adapt::generate` writes `~/.io-cli/adapters////plugin.toml` from a Claude Code or Codex bundle, with absolute canonical paths into the clone — `Plugin::skills_dir` is `self.root.join(d)` and an absolute join discards the root, so io-harness loads it as an ordinary bundle with no change to that crate. The stranger's checkout is never written to, and a test asserts the clone still carries no `plugin.toml`. Every claim the tests make about the generated file goes through io-harness: `Plugins::inspect` and `Config::plugins` load it, and `Plugin`'s own accessors answer what it contributed. Reading back the string io-cli spelled would assert that this crate can quote its own output, which is not the question. The manifest is written and never parsed. Scalars go through `toml::Value`'s own serializer rather than `format!("{:?}")`, which escapes for Rust's grammar: an escaped apostrophe TOML rejects outright, Rust's brace form for control characters where TOML wants four fixed hex digits, and a Windows path's `\U` opening an escape inside a basic string. `src/edit.rs` remains the only module permitted to parse TOML. Generation is validated through io-harness before the operator is asked anything, and a manifest it refuses is removed rather than left on disk — a manifest io-harness rejects takes the whole configuration down, not just the bundle. Every `${…}` is resolved or refused at generation time: `${CLAUDE_PLUGIN_ROOT}` expands to the canonical bundle root, anything else refuses naming the field it was in, and the assembled bytes are scanned for `${` as a backstop. Agent frontmatter is read by a small hand-rolled reader that says in its own rustdoc that it is deliberately not a YAML parser and lists what it does not support. Only `name` is acted on; the markdown body becomes `AgentDef::role`. `model` is disclosed rather than translated — `sonnet` and `opus` are one vendor's family aliases while `AgentDef::model` is a published model id, and translating would invent a fact the run pays for. `marketplace::adapted_disclosure` draws every hook a foreign bundle declares with its event, its command unshortened and the reason it will not run. The hooks are read from the author's own file rather than from io-harness, because the generated manifest carries none — asking io-harness what a bundle contributes cannot answer what it declared. `LONGEST` bounds a description and deliberately does not bound argv: a shortened command on a consent surface is the one thing that surface must never show. A native bundle withholds nothing and says nothing. --- src/adapt.rs | 704 +++++++++++++++++++++++++++++++++++++++++++ src/home.rs | 41 +++ src/marketplace.rs | 97 ++++++ tests/adapt.rs | 362 ++++++++++++++++++++++ tests/marketplace.rs | 176 ++++++++++- 5 files changed, 1374 insertions(+), 6 deletions(-) diff --git a/src/adapt.rs b/src/adapt.rs index cbc6c65..d53f2e4 100644 --- a/src/adapt.rs +++ b/src/adapt.rs @@ -48,9 +48,46 @@ //! 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}; @@ -310,6 +347,673 @@ pub fn normalised(name: &str) -> Option { 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)) { + let at = root.join(candidate); + if at.is_dir() { + return Some(std::fs::canonicalize(&at).unwrap_or(at)); + } + } + None +} + +/// 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 diff --git a/src/home.rs b/src/home.rs index 2393972..c7ec44d 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 @@ -222,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 diff --git a/src/marketplace.rs b/src/marketplace.rs index 634aac3..641ec01 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -1341,6 +1341,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 { @@ -1416,6 +1431,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`. @@ -1423,9 +1470,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(|from| withheld_hooks(from)).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/tests/adapt.rs b/tests/adapt.rs index 226addd..e3af4d4 100644 --- a/tests/adapt.rs +++ b/tests/adapt.rs @@ -9,10 +9,19 @@ //! `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) { @@ -407,3 +416,356 @@ fn n7_a_value_longer_than_the_bound_is_cut_before_it_leaves_the_reader() { 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/marketplace.rs b/tests/marketplace.rs index 175a46e..0135b6b 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -2312,6 +2312,164 @@ fn entry_bundle(dir: &Path, source: Option) -> marketplac } } +/// **N8 — a 291-entry index costs one file read, and the shape is what is +/// asserted.** +/// +/// A wall-clock threshold is a flaky test on a loaded machine, so the property is +/// stated structurally and the number is only a ceiling wide enough that only a +/// catastrophic regression reaches it. The shape: the index path opens **one** +/// file and descends into nothing, where the walk it replaces is a recursive +/// `read_dir` to `DEPTH`. That is asserted by putting a real bundle three +/// directories down and requiring it not to appear — if the walk had run, it +/// would 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 began = std::time::Instant::now(); + let held = marketplace::holdings(&root); + let took = began.elapsed(); + + 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", + ); + assert!( + took < std::time::Duration::from_secs(2), + "reading one file and building 291 rows took {took:?}. This ceiling is \ + deliberately far above any real measurement — it is here to catch a \ + regression that reintroduced a walk or a per-entry filesystem call, not \ + to assert a performance number on a loaded machine", + ); +} + +/// **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", + ); +} + #[test] fn f5_a_local_source_is_already_here_and_starts_no_program() { let (_dir, root) = clone_dir(); @@ -2319,7 +2477,12 @@ fn f5_a_local_source_is_already_here_and_starts_no_program() { assert_eq!( marketplace::fetched( - &entry_bundle(&here, Some(io_cli::adapt::Source::Local("./plugins/reviewer".to_string()))), + &entry_bundle( + &here, + Some(io_cli::adapt::Source::Local( + "./plugins/reviewer".to_string() + )) + ), &root.join("marketplaces"), &root.join("staging"), ), @@ -2328,7 +2491,11 @@ fn f5_a_local_source_is_already_here_and_starts_no_program() { needs git at all", ); assert_eq!( - marketplace::fetched(&entry_bundle(&here, None), &root.join("marketplaces"), &root.join("staging")), + marketplace::fetched( + &entry_bundle(&here, None), + &root.join("marketplaces"), + &root.join("staging") + ), Ok(here), "and a bundle the walk found carries no source, which is the same answer", ); @@ -2391,8 +2558,5 @@ fn f5_a_pin_that_could_become_an_argument_refuses_the_whole_entry() { said.contains("reviewer"), "the refusal names the bundle it withheld: {said:?}", ); - assert!( - !root.join("staging").exists(), - "and nothing was started", - ); + assert!(!root.join("staging").exists(), "and nothing was started",); } From 772710a27de6bb950ded917d33934579a51ec75b Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 12:20:30 +0530 Subject: [PATCH 07/12] docs: the three manifest formats, the adapter path, and what does not cross --- CHANGELOG.md | 69 ++++++++++++++++++++++++++++++++++++++++++- README.md | 2 +- docs/CAPABILITIES.md | 2 +- docs/CONTRACT.md | 8 ++--- docs/guide/plugins.md | 64 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 135 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c2367..6a86949 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,72 @@ 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. An adapted bundle is marked as adapted +wherever it is drawn, so the difference between what an author wrote and what io +generated is never something you have to infer. + +### Changed + +**The skills directory now follows `$IO_CONFIG_HOME`.** It resolved through +io-cli's default home while your memory file resolved through the home actually in +force, so pointing that 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. + +### 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 +2733,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/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..1312f25 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -105,7 +105,8 @@ 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 | +| `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,11 +117,6 @@ 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 turn the operator interrupted cannot be resumed.** It is reported as ended, with `/fork` offered instead. diff --git a/docs/guide/plugins.md b/docs/guide/plugins.md index 185a126..94cc638 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,68 @@ 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`, in the listing and in +the disclosure — and io writes the generated manifest 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 From 9c5a59b970ea8cd22dfbfe2bd6ff72403bddd341 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 12:27:43 +0530 Subject: [PATCH 08/12] feat(marketplace): name the adapters a removal orphans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 a marketplace of adapted bundles would have been removed with the operator told nothing at all. `orphaned` reads the link where it actually is: the generated manifest's own `skills` and `templates` values, through `edit::value_at`, which is this crate's only permitted TOML reader. Both keys are checked rather than one, because a bundle publishing only `commands/` has a `templates` and no `skills` and a rule reading `skills` alone would call that adapter unaffected by a removal that guts it. Neither the `[[plugin]]` entry nor the adapter directory is touched. A cache being emptied does not undo a configuration decision, and an adapter is io's own file but is still named by an entry that survives. What is owed is the consequence, said before anything goes — and the two kinds are counted and listed together, because the difference between them is not something the operator asked about. `removal_cost` is now the one call each of the three doors makes. Each of them previously called `dependents` and then `warning`, so a release adding a second way for a bundle to depend on a clone had to be remembered in three places — and `src/main.rs` is linked by nothing under `tests/`, so forgetting one would have been invisible to every gate. --- src/main.rs | 24 +++++----- src/marketplace.rs | 105 +++++++++++++++++++++++++++++++++++++++--- tests/marketplace.rs | 106 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 215 insertions(+), 20 deletions(-) diff --git a/src/main.rs b/src/main.rs index 1f0628a..c99d7d2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2362,15 +2362,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(( @@ -4357,11 +4357,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); } } @@ -9065,9 +9065,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/marketplace.rs b/src/marketplace.rs index 641ec01..1288278 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -1028,6 +1028,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, @@ -1043,27 +1094,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) } diff --git a/tests/marketplace.rs b/tests/marketplace.rs index 0135b6b..a73259f 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -577,13 +577,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", ); @@ -2312,6 +2315,105 @@ fn entry_bundle(dir: &Path, source: Option) -> marketplac } } +/// **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, and the shape is what is /// asserted.** /// From 76e396a22615cf3c1887dfa06043dfbe656d4b32 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 12:40:44 +0530 Subject: [PATCH 09/12] feat(pluginview): draw an adapted bundle under its own mark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ADAPTED_MARK` is `~`, one ASCII cell in both glyph sets, sharing no stroke with `+`, `-` or `!` so a column of marks stays readable. It takes the column from the state marks rather than sitting beside them, because a row carries one mark and where the manifest came from is the fact an operator cannot recover any other way — a switched-off bundle already leads its detail with the words, and a test now holds that claim to its own terms rather than leaving it in a doc comment. `Listed::adapted` is a path prefix against the adapters root, which `View` now carries. `detail` is deliberately not changed: its signature has no adapters root and its three call sites are outside this change, and `/plugin`'s listing is what the criterion is about. An adapted row reserves the floor for its root ahead of the description, so the directory holding the generated manifest is drawn rather than dropped when the description is long — that file is what an operator opens when io-harness refuses the bundle or its contributions are not what the clone promised, and nothing else on the surface names it. The mark is registered in `tests/glyphs.rs`'s existing pairing gate rather than in a second one beside it, which meant giving that gate a `/plugin` fixture: its presence check reads the rows of the page it renders, and the memory page can never carry a plugin mark. No slash command was added. `COMMANDS.len()` is 36 and group occupancy is [7, 9, 10, 10], both unchanged and both still asserted. --- AGENTS.md | 13 ++- docs/CONTRACT.md | 12 +++ src/pluginview.rs | 129 +++++++++++++++++++++++++++-- tests/glyphs.rs | 99 +++++++++++++++++++++- tests/marketplace.rs | 75 +++++++++++++---- tests/plugins.rs | 190 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 487 insertions(+), 31 deletions(-) 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/docs/CONTRACT.md b/docs/CONTRACT.md index 1312f25..28bccec 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -117,6 +117,18 @@ deleted that was not already copied. This does nothing at all when `$IO_CONFIG` ## Limits that hold today +**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/src/pluginview.rs b/src/pluginview.rs index 3bc7d87..9fd8c72 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,38 @@ 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 { + self.root.starts_with(adapters) + } +} + /// One bundle that was declared and did not load. /// /// Named for what happened rather than mirroring io-harness's `Dropped`: from @@ -237,6 +297,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 +463,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 +485,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 +508,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 +525,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 +565,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 +597,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 @@ -1011,6 +1117,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, } } @@ -1098,6 +1208,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); @@ -1199,6 +1310,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); @@ -1229,6 +1341,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/tests/glyphs.rs b/tests/glyphs.rs index fc47e52..1c9b7c6 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(); @@ -946,8 +1015,21 @@ fn n3_the_memory_page_draws_in_ascii_and_fits_a_narrow_terminal() { // The marks survive the substitution and still mark. A set that mapped every // 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] { + let mut marks: Vec<&str> = rows.iter().filter_map(|row| row.mark).collect(); + // `/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. + marks.extend(plugin_marks()); + for mark in [ + READ_MARK, + UNREAD_MARK, + PINNED_MARK, + LOOSE_MARK, + LOADED_MARK, + DISABLED_MARK, + REFUSED_MARK, + ADAPTED_MARK, + ] { assert!(mark.is_ascii(), "{mark:?} is not ASCII"); assert_eq!( mark.chars().count(), @@ -964,6 +1046,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/marketplace.rs b/tests/marketplace.rs index a73259f..52510ac 100644 --- a/tests/marketplace.rs +++ b/tests/marketplace.rs @@ -2414,16 +2414,21 @@ fn f13_a_removal_names_the_adapters_it_orphans_and_leaves_every_entry() { ); } -/// **N8 — a 291-entry index costs one file read, and the shape is what is -/// asserted.** +/// **N8 — a 291-entry index costs one file read, asserted as a shape and never +/// as a clock.** /// -/// A wall-clock threshold is a flaky test on a loaded machine, so the property is -/// stated structurally and the number is only a ceiling wide enough that only a -/// catastrophic regression reaches it. The shape: the index path opens **one** -/// file and descends into nothing, where the walk it replaces is a recursive -/// `read_dir` to `DEPTH`. That is asserted by putting a real bundle three -/// directories down and requiring it not to appear — if the walk had run, it -/// would have. +/// **`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 @@ -2446,9 +2451,7 @@ fn n8_an_index_of_291_entries_is_one_read_and_no_walk() { "name = \"only-a-walk-finds-me\"\n", ); - let began = std::time::Instant::now(); let held = marketplace::holdings(&root); - let took = began.elapsed(); assert_eq!( held.len(), @@ -2461,13 +2464,6 @@ fn n8_an_index_of_291_entries_is_one_read_and_no_walk() { .any(|bundle| bundle.label() == "only-a-walk-finds-me"), "the walk did not run, which is where the cost of the old path was", ); - assert!( - took < std::time::Duration::from_secs(2), - "reading one file and building 291 rows took {took:?}. This ceiling is \ - deliberately far above any real measurement — it is here to catch a \ - regression that reintroduced a walk or a per-entry filesystem call, not \ - to assert a performance number on a loaded machine", - ); } /// **F11 — every hook is disclosed, with its command unshortened and its reason.** @@ -2572,6 +2568,49 @@ fn f11_a_native_bundle_withholds_nothing_and_says_nothing() { ); } +/// **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(); 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:?}", + ); +} From 4eb1426acc43cdec9a8866600ec49213dca47b3d Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 12:48:30 +0530 Subject: [PATCH 10/12] fix(install): the skills fixture names the home in force, as the binary does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/install.rs`'s fixture set `HOME` and left `IO_CONFIG_HOME` wherever another test file had pointed it, so after `contract::default_skills` moved onto `home::in_force` it asserted about a state the product never reaches — `configured` answered `None` for a directory it had just installed five skills into. `home::adopt` sets that variable above the one `Config::discover` either arm reaches, so the fixture now does too, and restores it on drop. Found by the full suite rather than by any single file, which is the argument for running it after a wave of parallel work. Also drops a redundant closure clippy refuses under `-D warnings`. --- src/marketplace.rs | 2 +- tests/install.rs | 28 +++++++++++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/marketplace.rs b/src/marketplace.rs index 1288278..a3f587b 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -1561,7 +1561,7 @@ pub fn adapted_disclosure( Ok(Disclosure { id: listed.id.clone(), rows: crate::pluginview::detail(&listed, u16::MAX, &crate::glyphs::ASCII), - withheld: from.map(|from| withheld_hooks(from)).unwrap_or_default(), + withheld: from.map(withheld_hooks).unwrap_or_default(), }) } 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 } } From f1ca6cde20ac82703a682bacffe63e336e31223a Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 13:07:42 +0530 Subject: [PATCH 11/12] fix: install what the marketplace now lists, and one home for what you wrote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The whole 0.31.0 pipeline had no caller on the install path.** `plugin marketplace add` listed a Claude Code bundle and `plugin add` then refused it — `chosen` resolved the name to the clone directory and `pluginview::refusal` found no `plugin.toml` there, because the adapter is generated and nothing generated it. Every row the release added was listable and uninstallable. Found by running the built binary against a real repository, which is the gate 0.30.1 exists for. `marketplace::prepared` is the missing step: resolve the word to a bundle, fetch it where the index placed it elsewhere, and generate the adapter where the manifest is not one io-harness reads. Native bundles are untouched by every line of it — and nativeness is decided by the disk rather than by how the bundle was listed, because an index entry may still point at a directory carrying io's own manifest and generating over it would drop the author's hooks, layers and agents. An entry clone is keyed on the marketplace that named it **and on the pin**. `clone_at` answers `Already` before it consults the pin and nothing records which commit a directory holds, so two entries naming one repository at two commits gave the second the first one's code with a disclosure truthfully describing the wrong tree. Keyed under the marketplace, `marketplace remove` now takes the fetched repositories with it instead of leaving clones no surface lists. **A manifest's `skills` could name any directory on the machine.** `Path::join` replaces the base on an absolute argument and keeps `..`, so `"skills": "../../.."` in a stranger's `plugin.json` would write the operator's home into a generated manifest and `Skills::discover` would read it into the prompt. Plain components only, the filter every other stranger-supplied path already went through. **Skills reads had moved to the home in force and writes had not.** An operator with `$IO_CONFIG_HOME` set would have had `/skills add` write where nothing looked, and skills they already had would have stopped reaching the model with nothing said — the notice that existed to catch it was guarded on a directory that does not exist in exactly that case. `home::authored` is now the single answer for every surface that reads or writes what the operator wrote, and the notice compares the two homes rather than the resolved directory. Also: an index-derived bundle no longer tells the operator to look in a `plugin.json` that does not exist; the adapter directory uses the normalised id rather than the label, so two bundles differing only in case cannot share one; and `Listed::adapted` resolves both sides like `orphaned` does, so the mark and the removal warning cannot disagree on a machine where the home is a symlink. --- src/adapt.rs | 39 +++++++- src/contract.rs | 2 +- src/fetch.rs | 24 +++++ src/home.rs | 21 +++++ src/main.rs | 88 ++++++++++++++---- src/manage.rs | 48 +++++++++- src/marketplace.rs | 216 ++++++++++++++++++++++++++++++++++++++++--- src/pluginview.rs | 10 +- tests/marketplace.rs | 98 +++++++++++++++++--- 9 files changed, 496 insertions(+), 50 deletions(-) diff --git a/src/adapt.rs b/src/adapt.rs index d53f2e4..2091b20 100644 --- a/src/adapt.rs +++ b/src/adapt.rs @@ -290,8 +290,12 @@ pub fn hooks_in(file: &Path) -> Vec { continue; }; found.push(Hook { - // Filtered, never bounded. The event is the file's own word - // and is bounded, because it is a label rather than argv. + // **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), }); @@ -592,7 +596,17 @@ pub fn generate(bundle: &Path, name: &str, into: &Path) -> Result, conventional: &str) -> Option { for candidate in named.into_iter().chain(std::iter::once(conventional)) { - let at = root.join(candidate); + // **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)); } @@ -600,6 +614,25 @@ fn directory(root: &Path, named: Option<&str>, conventional: &str) -> Option` 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 diff --git a/src/contract.rs b/src/contract.rs index e111a57..e71dff4 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -536,7 +536,7 @@ pub fn skills_dir(config: &Config, capabilities: &Capabilities, root: PathBuf) - /// 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::in_force()?.0.join("skills"); + let dir = crate::home::authored()?.join("skills"); dir.is_dir().then_some(dir) } diff --git a/src/fetch.rs b/src/fetch.rs index 97ddd32..6631410 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -381,6 +381,30 @@ impl Pin { /// 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. diff --git a/src/home.rs b/src/home.rs index c7ec44d..ab5258a 100644 --- a/src/home.rs +++ b/src/home.rs @@ -317,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/main.rs b/src/main.rs index c99d7d2..68fb11d 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,16 @@ 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. `manage::declined` is the one + // place that decision lives, so this door and the + // argv door cannot disagree about whether saying + // no leaves something behind. + io_cli::marketplace::unmake(made.as_deref()); app.record( Tone::Muted, format!( @@ -2543,7 +2568,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 +2656,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, @@ -4378,7 +4403,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 +4547,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 +4581,7 @@ async fn loop_over Result>( id: disclosure.id.clone(), scope: plan.scope, edits: plan.edits.clone(), + made: plan.made.clone(), }, )); } @@ -4972,7 +5009,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 +6382,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 +8693,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 +8941,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 +9027,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}"); } 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 a3f587b..64a5692 100644 --- a/src/marketplace.rs +++ b/src/marketplace.rs @@ -250,9 +250,14 @@ impl Bundle { /// The name of the manifest file this bundle's two keys came out of. #[must_use] pub fn read_from(&self) -> &'static str { - match self.origin { - Origin::Native => MANIFEST, - Origin::Adapted => crate::adapt::MANIFEST_FILE, + // **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, } } } @@ -786,6 +791,16 @@ fn name_of(path: &Path) -> Option { /// 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 @@ -799,7 +814,12 @@ const ENTRIES: &str = ".entries"; /// 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, marketplaces: &Path, staging: &Path) -> Result { +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()); }; @@ -819,7 +839,21 @@ pub fn fetched(bundle: &Bundle, marketplaces: &Path, staging: &Path) -> Result

/` 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 { @@ -974,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: "); @@ -1202,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() @@ -1226,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 @@ -1365,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 @@ -1391,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(), } } diff --git a/src/pluginview.rs b/src/pluginview.rs index 9fd8c72..67cf6f3 100644 --- a/src/pluginview.rs +++ b/src/pluginview.rs @@ -257,7 +257,15 @@ impl Listed { /// through every one of them to be true anywhere. #[must_use] pub fn adapted(&self, adapters: &Path) -> bool { - self.root.starts_with(adapters) + // **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)) } } diff --git a/tests/marketplace.rs b/tests/marketplace.rs index 52510ac..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 { @@ -844,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), @@ -856,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), @@ -1276,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( @@ -1317,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, \ @@ -1870,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 @@ -2624,6 +2680,7 @@ fn f5_a_local_source_is_already_here_and_starts_no_program() { "./plugins/reviewer".to_string() )) ), + &named(), &root.join("marketplaces"), &root.join("staging"), ), @@ -2634,6 +2691,7 @@ fn f5_a_local_source_is_already_here_and_starts_no_program() { assert_eq!( marketplace::fetched( &entry_bundle(&here, None), + &named(), &root.join("marketplaces"), &root.join("staging") ), @@ -2665,8 +2723,13 @@ fn f5_a_remote_entry_on_another_host_is_refused_before_anything_is_started() { sha: None, })), ); - let said = marketplace::fetched(&bundle, &root.join("marketplaces"), &root.join("staging")) - .expect_err("a url io does not read is a refusal"); + 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 \ @@ -2693,8 +2756,13 @@ fn f5_a_pin_that_could_become_an_argument_refuses_the_whole_entry() { })), ); - let said = marketplace::fetched(&bundle, &root.join("marketplaces"), &root.join("staging")) - .expect_err("a pin that is an option is a refusal"); + 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:?}", From 888e155978be8a345b8866ef6f3b31b3e8701ca9 Mon Sep 17 00:00:00 2001 From: aakashpawar1999 Date: Sun, 30 Aug 2026 13:13:25 +0530 Subject: [PATCH 12/12] fix: Esc is a decline, the mark gate is load-bearing, and the prose matches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Esc` on the install disclosure dropped the picker and the adapter with it. It is the standard way out of every other modal in this product, so it was the one that left a directory nobody agreed to under the operator's home, listed by no surface and removed by no verb. It now takes the same path the "leave it" row does. The glyphs mark registry looked for each mark in the union of two surfaces' rows, and `LOADED_MARK` is `+` like `READ_MARK` while `DISABLED_MARK` is `-` like `LOOSE_MARK` and `UNREAD_MARK` — so `/plugin`'s two commonest marks were found in the *memory page's* rows and the gate passed however `/plugin` drew them. Each mark is now looked for on the surface that draws it, proved by making every arm of the mark selector return `ADAPTED_MARK`: that sabotage passed before and fails now. Three shipped claims the code did not support. "Marked as adapted wherever it is drawn" was true of `/plugin`'s rows and not of the marketplace listing or the disclosure — the prose now says where the mark is and what it is for. The skills-directory paragraphs in the configuration guide and `docs/CONTRACT.md` said `~/.io-cli/skills` flatly, which stopped being the whole truth when skills moved onto the home in force; they now say both, and the CHANGELOG says the write moved with the read and that io does not move files you already own. Four rustdoc comments naming `~/.io-cli/skills` as the default were left behind by the same change and now describe the home in force. --- CHANGELOG.md | 23 +++++++++++++++-------- docs/CONTRACT.md | 2 +- docs/guide/configuration.md | 5 ++++- docs/guide/plugins.md | 5 +++-- src/contract.rs | 5 +++-- src/main.rs | 25 ++++++++++++++++++++----- src/skillview.rs | 4 ++-- tests/glyphs.rs | 33 ++++++++++++++++++++------------- 8 files changed, 68 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a86949..28b9288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,17 +41,24 @@ 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. An adapted bundle is marked as adapted -wherever it is drawn, so the difference between what an author wrote and what io -generated is never something you have to infer. +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 -**The skills directory now follows `$IO_CONFIG_HOME`.** It resolved through -io-cli's default home while your memory file resolved through the home actually in -force, so pointing that 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. +**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 diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index 28bccec..5c4d857 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -104,7 +104,7 @@ 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 | +| `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 | 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 94cc638..3e478d3 100644 --- a/docs/guide/plugins.md +++ b/docs/guide/plugins.md @@ -156,8 +156,9 @@ 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`, in the listing and in -the disclosure — and io writes the generated manifest under +**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. diff --git a/src/contract.rs b/src/contract.rs index e71dff4..e167268 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -649,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 @@ -706,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/main.rs b/src/main.rs index 68fb11d..161abd7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2082,10 +2082,11 @@ async fn loop_over Result>( // 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. `manage::declined` is the one - // place that decision lives, so this door and the - // argv door cannot disagree about whether saying - // no leaves something behind. + // 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, @@ -4084,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())?; 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/glyphs.rs b/tests/glyphs.rs index 1c9b7c6..5534bb8 100644 --- a/tests/glyphs.rs +++ b/tests/glyphs.rs @@ -1015,20 +1015,27 @@ fn n3_the_memory_page_draws_in_ascii_and_fits_a_narrow_terminal() { // The marks survive the substitution and still mark. A set that mapped every // mark to a space would pass the sweep above and destroy the page. let (rows, _) = memory_page(&files, &entries, false, &ASCII); - let mut marks: Vec<&str> = rows.iter().filter_map(|row| row.mark).collect(); + let marks: Vec<&str> = rows.iter().filter_map(|row| row.mark).collect(); // `/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. - marks.extend(plugin_marks()); - for mark in [ - READ_MARK, - UNREAD_MARK, - PINNED_MARK, - LOOSE_MARK, - LOADED_MARK, - DISABLED_MARK, - REFUSED_MARK, - ADAPTED_MARK, + // **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!( @@ -1037,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!(