From ce240716b444ffac5e2e03a1498d9174d9c467a1 Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Mon, 13 Jul 2026 10:11:47 -0400 Subject: [PATCH 1/2] feat: add agent skill installation Embed the fetch skill for offline user and project installs across generic, Codex, Claude, Gemini, and Pi locations. Protect modified copies, serialize writes, and support dry-run and uninstall workflows. --- AGENTS.md | 3 +- README.md | 21 ++ docs/cli-reference.md | 31 ++ skills/fetch/SKILL.md | 1 + src/app.rs | 16 + src/cli.rs | 37 +++ src/cli/completion.rs | 62 ++++ src/flag_registry.rs | 8 + src/lib.rs | 1 + src/skill.rs | 681 ++++++++++++++++++++++++++++++++++++++++++ tests/cli.rs | 174 ++++++++++- 11 files changed, 1033 insertions(+), 2 deletions(-) create mode 100644 src/skill.rs diff --git a/AGENTS.md b/AGENTS.md index 2f89a60..eff28d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,7 +63,7 @@ cargo build --release --locked | Formatting | `src/format`, `src/format/content_type.rs`, `src/image` | MIME-to-formatter policy, streaming formatters, built-in image defaults; external image adapters only for `--image external`/config and must be bounded. | | gRPC/protobuf | `src/grpc`, `src/proto` | Framing/status/reflection, local schema/discovery/conversion/JSON streams. Reuse standard gRPC headers/status/framed-body helpers. | | WebSocket | `src/websocket` | Interactive and non-interactive message loops; custom dialer for DNS/proxy/TLS. | -| Auth/session/update | `src/auth`, `src/session.rs`, `src/update`, `install.sh` | Auth helpers; locked cookie sessions; HTTPS self-update with checksum/archive validation and no origin-specific TLS overrides. | +| Auth/session/update | `src/auth`, `src/session.rs`, `src/update`, `src/skill.rs`, `install.sh` | Auth helpers; locked cookie sessions; HTTPS self-update; offline embedded Agent Skill installation with modification detection. | | Tests | `tests/`, `tests/support/` | Integration tests run the compiled binary; support code is split by domain. `run_fetch` isolates HTTP/3 cache by default. `TestServer`/`H3TestServer` use `mpsc` channel notification (not polling). `wait_for_requests` blocks via `recv_timeout` on the notification channel. | Request flow: CLI parse → config merge → request build (gRPC may load/reflect schema and frame protobuf) → transport execute → response format/output/pager/clipboard. @@ -128,6 +128,7 @@ Request flow: CLI parse → config merge → request build (gRPC may load/reflec - Session saves lock per session, reload latest JSON, merge only local cookie changes, atomically replace, and warn on bounded lock wait. Update locks use `fileutil::FileLock`; background checks are nonblocking. - Self-update metadata/artifact/checksum/redirect URLs require HTTPS except internal test overrides; update networking must not inherit origin-specific TLS/version/Unix-socket config, but may keep proxy/DNS/timeouts/verbosity/custom CA. Artifacts stream with SHA-256; tar extracts streaming, zip uses temp archive; Unix replacement preserves atomic parent-dir sync. - `install.sh` verifies `.sha256`; completion install is opt-in (`--completions` or `FETCH_INSTALL_COMPLETIONS=1`) and must not auto-edit shell startup files by default. +- Agent Skill files live under `skills/fetch/` and are embedded by `src/skill.rs`. User/project installs support generic `.agents/skills/fetch` plus distinct Codex, Claude, Gemini, and Pi directories, record `.fetch-skill.json` in each copy, use atomic file helpers and a parent-directory operation lock, revalidate before writes/deletions, refuse modified installs without `--force`, reject unrelated CLI options rather than silently ignoring them, and leave no lock/directory artifacts when uninstall targets are all missing. Never add network downloads or agent-config edits to skill installation. - Ctrl-C/SIGINT exits 130, including streaming modes. Output downloads keep `*.download` temps under a drop guard. - Rust is pinned to 1.97.0 (`rust-toolchain.toml`); keep `Cargo.toml` rust-version and CI aligned. Windows config search prefers XDG/HOME paths before AppData; Windows mTLS fixtures use RSA certs. - GitHub Actions run fmt/clippy/unit/integration. Release builds archive names for self-updater, Linux GNU uses `cargo-zigbuild` with glibc 2.28 floor, Windows uses static MSVC CRT, archives get SHA-256 sidecars, `FETCH_VERSION` comes from release tag/manual version (local: matching `v*`, then `git describe`, then `v0.0.0-dev`), and `vcs.modified` ignores untracked files. diff --git a/README.md b/README.md index 51f0b56..26bac71 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ inspection, and request timing in one CLI. - **DNS inspection** - Inspect hostname resolution, record families, TTLs, and resolver timing - **Timing waterfall** - Visualize request timing phases (DNS, TCP, TLS, TTFB, transfer) with a waterfall chart - **Configuration** - Global and per-host configuration file support +- **Agent skill** - Bundled offline skill installation for Codex, Claude Code, Gemini CLI, and Pi ## Quick Start @@ -53,6 +54,26 @@ fetch -v -h > `fetch -v --help`) to open the full, colorized command menu with detailed > options, examples, and pager support. +#### Install the agent skill + +```sh +# Use the interoperable ~/.agents/skills/fetch location +fetch --install-skill + +# Or install directly for one agent +fetch --install-skill pi + +# Install all generic and agent-specific locations +fetch --install-skill all + +# Preview a project-local installation +fetch --install-skill all --scope project --dry-run +``` + +The skill is embedded in `fetch`, so installation is offline. The installer +shows destinations before writing, detects modified installations, and does not +edit agent configuration files. See [the CLI reference](docs/cli-reference.md#agent-skill-options). + ## Output Model `fetch` keeps response bodies and metadata separate: the body is written to diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 1140938..2dae42c 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -785,6 +785,37 @@ Update fetch binary in place. Use with `--dry-run` to check for updates without See [Updates](updates.md) for release source, verification, permissions, background auto-update behavior, and cache/lock files. +### Agent skill options + +The fetch Agent Skill is embedded in the binary and can be installed offline. +Choose an agent-specific location or the interoperable `agents` location: + +| Target | User scope | Project scope | +| --- | --- | --- | +| `agents` (default) | `~/.agents/skills/fetch` | `.agents/skills/fetch` | +| `codex` | `~/.codex/skills/fetch` | `.codex/skills/fetch` | +| `claude` | `~/.claude/skills/fetch` | `.claude/skills/fetch` | +| `gemini` | `~/.gemini/skills/fetch` | `.gemini/skills/fetch` | +| `pi` | `~/.pi/agent/skills/fetch` | `.pi/skills/fetch` | + +```sh +fetch --skill # print SKILL.md +fetch --install-skill [agents|codex|claude|gemini|pi|all] +fetch --uninstall-skill [agents|codex|claude|gemini|pi|all] +``` + +User scope is the default; use `--scope project` for the project locations +shown above. Install and uninstall commands show every destination before +changing it and ask for confirmation when attached to a terminal. `--dry-run` +previews changes and `--force` permits replacing or removing a locally modified +installation. No agent configuration files are changed and installation +performs no network requests. + +Each installed copy contains `.fetch-skill.json`, recording the skill version, +fetch version, and hashes used to detect local modifications. `all` means the +five locations listed above; it does not probe and write to additional +directories. + ### `--complete SHELL` Output shell completion scripts. Values: `bash`, `fish`, `zsh`. diff --git a/skills/fetch/SKILL.md b/skills/fetch/SKILL.md index be83bf7..43d55ef 100644 --- a/skills/fetch/SKILL.md +++ b/skills/fetch/SKILL.md @@ -9,6 +9,7 @@ license: MIT compatibility: Requires the fetch executable and network access. metadata: repository: https://github.com/ryanfowler/fetch + skill-version: "1" --- # fetch diff --git a/src/app.rs b/src/app.rs index fe15c20..1128ffc 100644 --- a/src/app.rs +++ b/src/app.rs @@ -283,6 +283,12 @@ async fn run(mut cli: Cli) -> Result { } async fn run_inner(cli: &mut Cli) -> Result { + // Validate skill actions before other metadata dispatch so combinations such + // as `--skill --complete` cannot silently choose whichever branch runs first. + if crate::skill::is_action(cli) { + crate::skill::validate_cli(cli)?; + } + if let Some(shell) = cli.complete.as_deref() { let output = crate::cli::completion::output(shell, &cli.extra_args).map_err(FetchError::Message)?; @@ -292,6 +298,16 @@ async fn run_inner(cli: &mut Cli) -> Result { normalize_extra_args(cli)?; + if crate::skill::is_action(cli) { + return crate::skill::execute(cli); + } + if cli.scope.is_some() { + return Err("flag '--scope' requires a skill action".into()); + } + if cli.force { + return Err("flag '--force' requires a skill action".into()); + } + if cli.help || cli.version || cli.buildinfo { crate::config::apply_best_effort(cli); if cli.help { diff --git a/src/cli.rs b/src/cli.rs index 6c30c3a..68dfa37 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -227,6 +227,9 @@ pub struct Cli { #[arg(long = "dry-run", help = "Print out the request info and exit")] pub dry_run: bool, + #[arg(long, help = "Overwrite a modified skill installation")] + pub force: bool, + #[arg( long = "ech", value_name = "MODE", @@ -336,6 +339,17 @@ pub struct Cli { #[arg(long = "inspect-tls", help = "Inspect the TLS certificate chain")] pub inspect_tls: bool, + #[arg( + long = "install-skill", + value_name = "AGENT", + num_args = 0..=1, + default_missing_value = "auto", + value_parser = ["auto", "agents", "codex", "claude", "gemini", "pi", "all"], + hide_possible_values = true, + help = "Install the bundled agent skill" + )] + pub install_skill: Option, + #[arg( short = 'j', long, @@ -495,6 +509,18 @@ pub struct Cli { #[arg(short = 's', long, help = "Print only errors to stderr")] pub silent: bool, + #[arg( + long, + value_name = "SCOPE", + value_parser = ["user", "project"], + hide_possible_values = true, + help = "Skill installation scope [user, project]" + )] + pub scope: Option, + + #[arg(long, help = "Print the bundled SKILL.md")] + pub skill: bool, + #[arg(long = "sort-headers", help = "Sort displayed headers by name")] pub sort_headers: bool, @@ -523,6 +549,17 @@ pub struct Cli { #[arg(long, help = "Update the fetch binary in place")] pub update: bool, + #[arg( + long = "uninstall-skill", + value_name = "AGENT", + num_args = 0..=1, + default_missing_value = "auto", + value_parser = ["auto", "agents", "codex", "claude", "gemini", "pi", "all"], + hide_possible_values = true, + help = "Uninstall the agent skill" + )] + pub uninstall_skill: Option, + #[arg( short = 'v', long = "verbose", diff --git a/src/cli/completion.rs b/src/cli/completion.rs index 9020419..23c2098 100644 --- a/src/cli/completion.rs +++ b/src/cli/completion.rs @@ -80,6 +80,42 @@ const COMPRESS_VALUES: &[FlagValue] = &[ value: "Disable compression negotiation", }, ]; +const AGENT_VALUES: &[FlagValue] = &[ + FlagValue { + key: "agents", + value: "Open Agent Skills location", + }, + FlagValue { + key: "codex", + value: "Codex", + }, + FlagValue { + key: "claude", + value: "Claude Code", + }, + FlagValue { + key: "gemini", + value: "Gemini CLI", + }, + FlagValue { + key: "pi", + value: "Pi coding agent", + }, + FlagValue { + key: "all", + value: "All compatible locations", + }, +]; +const SCOPE_VALUES: &[FlagValue] = &[ + FlagValue { + key: "user", + value: "Install for the current user", + }, + FlagValue { + key: "project", + value: "Install in the current project", + }, +]; const FORMAT_VALUES: &[FlagValue] = &[ FlagValue { key: "auto", @@ -226,6 +262,7 @@ const FLAGS: &[Flag] = &[ "DNS server IP or DoH URL", ), flag(None, "dry-run", "", "Print out the request info and exit"), + flag(None, "force", "", "Overwrite a modified skill installation"), flag( Some('e'), "edit", @@ -260,6 +297,14 @@ const FLAGS: &[Flag] = &[ "Describe a gRPC service, method, or message", ), flag(None, "grpc-list", "", "List available gRPC services"), + Flag { + short: None, + long: "install-skill", + args: "AGENT", + description: "Install the bundled agent skill", + aliases: &[], + values: AGENT_VALUES, + }, flag( Some('H'), "header", @@ -396,6 +441,15 @@ const FLAGS: &[Flag] = &[ "Use a named session for cookies", ), flag(Some('s'), "silent", "", "Print only errors to stderr"), + Flag { + short: None, + long: "scope", + args: "SCOPE", + description: "Skill installation scope", + aliases: &[], + values: SCOPE_VALUES, + }, + flag(None, "skill", "", "Print the bundled SKILL.md"), flag(None, "sort-headers", "", "Sort displayed headers by name"), flag( Some('t'), @@ -406,6 +460,14 @@ const FLAGS: &[Flag] = &[ flag(Some('T'), "timing", "", "Display a timing waterfall chart"), flag(None, "unix", "PATH", "Make the request over a unix socket"), flag(None, "update", "", "Update the fetch binary in place"), + Flag { + short: None, + long: "uninstall-skill", + args: "AGENT", + description: "Uninstall the agent skill", + aliases: &[], + values: AGENT_VALUES, + }, flag(Some('v'), "verbose", "", "Verbosity of the output"), flag(Some('V'), "version", "", "Print version"), Flag { diff --git a/src/flag_registry.rs b/src/flag_registry.rs index 368a0ad..89e75fd 100644 --- a/src/flag_registry.rs +++ b/src/flag_registry.rs @@ -275,6 +275,14 @@ pub(crate) static FLAGS: &[FlagDef] = &[ // ── convenience iterators ────────────────────────────────────────────── +/// Return registry flag names explicitly set on the CLI. +pub(crate) fn set_flag_names(cli: &Cli) -> impl Iterator + '_ { + FLAGS + .iter() + .filter(move |definition| (definition.is_set)(cli)) + .map(|definition| definition.name) +} + /// Push names of all flags in `category` that are set on `cli` into `ignored`. pub(crate) fn append_ignored_of_category( cli: &Cli, diff --git a/src/lib.rs b/src/lib.rs index 36831a8..1869d3d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,6 +17,7 @@ pub(crate) mod net; pub mod output; pub mod proto; pub mod session; +pub mod skill; pub mod timing; pub mod tls; pub mod update; diff --git a/src/skill.rs b/src/skill.rs new file mode 100644 index 0000000..14b4edd --- /dev/null +++ b/src/skill.rs @@ -0,0 +1,681 @@ +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; +use std::collections::{BTreeMap, BTreeSet}; +use std::fs; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use crate::cli::Cli; +use crate::error::FetchError; +use crate::{core, fileutil}; + +const SKILL_VERSION: &str = "1"; +const METADATA_FILE: &str = ".fetch-skill.json"; + +const FILES: &[(&str, &str)] = &[ + ("SKILL.md", include_str!("../skills/fetch/SKILL.md")), + ( + "references/http.md", + include_str!("../skills/fetch/references/http.md"), + ), + ( + "references/diagnostics.md", + include_str!("../skills/fetch/references/diagnostics.md"), + ), + ( + "references/grpc.md", + include_str!("../skills/fetch/references/grpc.md"), + ), + ( + "references/websocket.md", + include_str!("../skills/fetch/references/websocket.md"), + ), + ( + "evals/evals.json", + include_str!("../skills/fetch/evals/evals.json"), + ), +]; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Scope { + User, + Project, +} + +#[derive(Debug)] +struct Destination { + path: PathBuf, +} + +#[derive(Debug, Deserialize, Serialize)] +struct InstallationMetadata { + skill_version: String, + fetch_version: String, + files: BTreeMap, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum InstallationState { + Missing, + Current, + Outdated, + Modified, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum SkillAction { + Print, + Install, + Uninstall, +} + +impl SkillAction { + fn flag(self) -> &'static str { + match self { + Self::Print => "--skill", + Self::Install => "--install-skill", + Self::Uninstall => "--uninstall-skill", + } + } + + fn mutates(self) -> bool { + matches!(self, Self::Install | Self::Uninstall) + } +} + +pub fn is_action(cli: &Cli) -> bool { + cli.skill || cli.install_skill.is_some() || cli.uninstall_skill.is_some() +} + +pub fn execute(cli: &Cli) -> Result { + validate_cli(cli)?; + if cli.skill { + core::write_stdout(FILES[0].1)?; + return Ok(0); + } + let scope = parse_scope(cli.scope.as_deref())?; + if let Some(agent) = cli.install_skill.as_deref() { + return install(scope, agent, cli.dry_run, cli.force); + } + if let Some(agent) = cli.uninstall_skill.as_deref() { + return uninstall(scope, agent, cli.dry_run, cli.force); + } + unreachable!("skill action was checked by caller") +} + +pub(crate) fn validate_cli(cli: &Cli) -> Result<(), FetchError> { + let actions = [ + (cli.skill, SkillAction::Print), + (cli.install_skill.is_some(), SkillAction::Install), + (cli.uninstall_skill.is_some(), SkillAction::Uninstall), + ]; + let selected: Vec<_> = actions + .into_iter() + .filter_map(|(selected, action)| selected.then_some(action)) + .collect(); + if selected.len() != 1 { + return Err("exactly one skill action must be specified".into()); + } + let action = selected[0]; + + if cli.url.is_some() || !cli.extra_args.is_empty() { + return Err("a URL or positional argument cannot be used with a skill action".into()); + } + if cli.scope.is_some() && !action.mutates() { + return Err(format!("flag '--scope' cannot be used with '{}'", action.flag()).into()); + } + if cli.force && !action.mutates() { + return Err(format!("flag '--force' cannot be used with '{}'", action.flag()).into()); + } + if cli.dry_run && !action.mutates() { + return Err(format!("flag '--dry-run' cannot be used with '{}'", action.flag()).into()); + } + + let top_level_options = [ + (cli.auto_update.is_some(), "--auto-update"), + (cli.buildinfo, "--buildinfo"), + (cli.color.is_some(), "--color"), + (cli.complete.is_some(), "--complete"), + (cli.config.is_some(), "--config"), + (cli.from_curl.is_some(), "--from-curl"), + (cli.help, "--help"), + (cli.inspect_dns, "--inspect-dns"), + (cli.inspect_tls, "--inspect-tls"), + (cli.silent, "--silent"), + (cli.update, "--update"), + (cli.verbose > 0, "--verbose"), + (cli.version, "--version"), + ]; + if let Some((_, flag)) = top_level_options.into_iter().find(|(set, _)| *set) { + return Err(incompatible_option(flag, action)); + } + + if let Some(flag) = crate::flag_registry::set_flag_names(cli).find(|flag| *flag != "--dry-run") + { + return Err(incompatible_option(flag, action)); + } + Ok(()) +} + +fn incompatible_option(flag: &str, action: SkillAction) -> FetchError { + format!("flag '{flag}' cannot be used with '{}'", action.flag()).into() +} + +fn parse_scope(value: Option<&str>) -> Result { + match value.unwrap_or("user") { + "user" => Ok(Scope::User), + "project" => Ok(Scope::Project), + value => Err(FetchError::invalid_value( + "--scope", + value, + "must be one of [user, project]", + )), + } +} + +fn destinations(scope: Scope, agent: &str) -> Result, FetchError> { + let root = match scope { + Scope::User => home_dir()?, + Scope::Project => std::env::current_dir()?, + }; + let destination = |relative: &str| Destination { + path: root.join(relative).join("fetch"), + }; + let agents = || destination(".agents/skills"); + let codex = || destination(".codex/skills"); + let claude = || destination(".claude/skills"); + let gemini = || destination(".gemini/skills"); + let pi = || { + destination(match scope { + Scope::User => ".pi/agent/skills", + Scope::Project => ".pi/skills", + }) + }; + + let result = match agent { + "auto" | "agents" => vec![agents()], + "codex" => vec![codex()], + "claude" => vec![claude()], + "gemini" => vec![gemini()], + "pi" => vec![pi()], + "all" => vec![agents(), codex(), claude(), gemini(), pi()], + value => { + return Err(FetchError::invalid_value( + "--install-skill", + value, + "must be one of [agents, codex, claude, gemini, pi, all]", + )); + } + }; + Ok(result) +} + +fn home_dir() -> Result { + std::env::var_os("HOME") + .or_else(|| std::env::var_os("USERPROFILE")) + .map(PathBuf::from) + .ok_or_else(|| "unable to determine the user home directory".into()) +} + +fn install(scope: Scope, agent: &str, dry_run: bool, force: bool) -> Result { + let destinations = destinations(scope, agent)?; + write_stderr("Skill installation destinations:\n")?; + for destination in &destinations { + write_stderr(format!(" {}\n", destination.path.display()))?; + } + + validate_install_destinations(&destinations, force)?; + if dry_run { + write_stderr("Dry run: no files were written.\n")?; + return Ok(0); + } + if core::stdio().stdin_is_terminal() && !confirm("Install the bundled fetch skill? [y/N] ")? { + write_stderr("Installation cancelled.\n")?; + return Ok(0); + } + + let _locks = acquire_operation_locks(&destinations, true)?; + // The installation may have changed while confirmation was pending or + // while another fetch process held an operation lock. + validate_install_destinations(&destinations, force)?; + for destination in &destinations { + install_directory(&destination.path, force)?; + } + write_stderr(format!( + "Installed fetch skill {} (fetch {}).\n", + SKILL_VERSION, + core::version() + ))?; + Ok(0) +} + +fn uninstall(scope: Scope, agent: &str, dry_run: bool, force: bool) -> Result { + let destinations = destinations(scope, agent)?; + write_stderr("Skill uninstall destinations:\n")?; + for destination in &destinations { + write_stderr(format!(" {}\n", destination.path.display()))?; + ensure_removable(&destination.path, force)?; + } + if dry_run { + write_stderr("Dry run: no files were removed.\n")?; + return Ok(0); + } + if destinations_are_missing(&destinations)? { + write_stderr("Fetch skill is not installed; nothing to remove.\n")?; + return Ok(0); + } + if core::stdio().stdin_is_terminal() && !confirm("Uninstall the fetch skill? [y/N] ")? { + write_stderr("Uninstall cancelled.\n")?; + return Ok(0); + } + let _locks = acquire_operation_locks(&destinations, false)?; + // Revalidate every destination after confirmation while serialized with + // other fetch skill operations. This preflight avoids a known modified + // destination causing a partially completed uninstall. + for destination in &destinations { + ensure_removable(&destination.path, force)?; + } + + // Recheck immediately before each deletion as defense against + // non-cooperating processes and edits. + for destination in destinations { + ensure_removable(&destination.path, force)?; + remove_path(&destination.path)?; + } + write_stderr("Uninstalled fetch skill.\n")?; + Ok(0) +} + +fn destinations_are_missing(destinations: &[Destination]) -> Result { + for destination in destinations { + match fs::symlink_metadata(&destination.path) { + Ok(_) => return Ok(false), + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(error.into()), + } + } + Ok(true) +} + +fn validate_install_destinations( + destinations: &[Destination], + force: bool, +) -> Result<(), FetchError> { + for destination in destinations { + ensure_writable_installation(&destination.path, force)?; + } + Ok(()) +} + +fn acquire_operation_locks( + destinations: &[Destination], + include_missing: bool, +) -> Result, FetchError> { + let mut parents = BTreeSet::new(); + for destination in destinations { + let exists = match fs::symlink_metadata(&destination.path) { + Ok(_) => true, + Err(error) if error.kind() == io::ErrorKind::NotFound => false, + Err(error) => return Err(error.into()), + }; + if include_missing || exists { + parents.insert( + destination + .path + .parent() + .ok_or("invalid skill destination")? + .to_path_buf(), + ); + } + } + + let mut locks = Vec::with_capacity(parents.len()); + for parent in parents { + fs::create_dir_all(&parent)?; + let lock_path = parent.join(".fetch-skill.lock"); + locks.push(fileutil::FileLock::acquire_with_timeout( + lock_path, + Duration::from_secs(5), + || { + let _ = write_stderr("Waiting for another fetch skill operation...\n"); + }, + |timeout| { + FetchError::Message(format!( + "timed out waiting {:.0}s for another fetch skill operation", + timeout.as_secs_f64() + )) + }, + )?); + } + Ok(locks) +} + +fn write_stderr(message: impl AsRef) -> io::Result<()> { + let mut printer = core::Printer::stderr(None); + printer.push_str(message.as_ref()); + printer.flush_to(&mut io::stderr()) +} + +fn confirm(prompt: &str) -> io::Result { + write_stderr(prompt)?; + let mut answer = String::new(); + io::stdin().read_line(&mut answer)?; + Ok(matches!( + answer.trim().to_ascii_lowercase().as_str(), + "y" | "yes" + )) +} + +fn ensure_writable_installation(path: &Path, force: bool) -> Result<(), FetchError> { + if installation_state(path)? == InstallationState::Modified && !force { + return Err(format!( + "refusing to overwrite modified skill installation '{}'; use --force", + path.display() + ) + .into()); + } + Ok(()) +} + +fn ensure_removable(path: &Path, force: bool) -> Result<(), FetchError> { + let Ok(metadata) = fs::symlink_metadata(path) else { + return Ok(()); + }; + if !metadata.file_type().is_symlink() + && installation_state(path)? == InstallationState::Modified + && !force + { + return Err(format!( + "refusing to remove modified skill installation '{}'; use --force", + path.display() + ) + .into()); + } + Ok(()) +} + +fn install_directory(path: &Path, force: bool) -> Result<(), FetchError> { + let state = installation_state(path)?; + ensure_writable_installation(path, force)?; + if let Ok(metadata) = fs::symlink_metadata(path) + && (!metadata.is_dir() || metadata.file_type().is_symlink()) + { + // Older fetch versions could create a managed Claude symlink. Migrate + // an unmodified symlink to a direct agent-specific copy without + // requiring --force; modified or unrelated paths remain protected. + let safe_managed_symlink = + metadata.file_type().is_symlink() && state != InstallationState::Modified; + if !force && !safe_managed_symlink { + return Err(format!("'{}' is not an installation directory", path.display()).into()); + } + remove_path(path)?; + } + + if !path.exists() { + install_fresh_directory(path)?; + } else if force || state == InstallationState::Outdated { + replace_directory(path)?; + } else { + write_installation_files(path)?; + } + Ok(()) +} + +fn install_fresh_directory(path: &Path) -> Result<(), FetchError> { + let parent = path.parent().ok_or("invalid skill destination")?; + fs::create_dir_all(parent)?; + let stage = temporary_path(parent, ".fetch-skill-stage"); + fs::create_dir(&stage)?; + if let Err(error) = write_installation_files(&stage) { + let _ = fs::remove_dir_all(&stage); + return Err(error); + } + fileutil::atomic_replace_file(&stage, path)?; + Ok(()) +} + +fn replace_directory(path: &Path) -> Result<(), FetchError> { + let parent = path.parent().ok_or("invalid skill destination")?; + let stage = temporary_path(parent, ".fetch-skill-stage"); + fs::create_dir(&stage)?; + if let Err(error) = write_installation_files(&stage) { + let _ = fs::remove_dir_all(&stage); + return Err(error); + } + + let backup = temporary_path(parent, ".fetch-skill-backup"); + fs::rename(path, &backup)?; + if let Err(error) = fileutil::atomic_replace_file(&stage, path) { + let _ = fs::rename(&backup, path); + let _ = fs::remove_dir_all(&stage); + return Err(error.into()); + } + fs::remove_dir_all(backup)?; + Ok(()) +} + +fn write_installation_files(path: &Path) -> Result<(), FetchError> { + for (relative, contents) in FILES { + atomic_write(path.join(relative), contents.as_bytes())?; + } + let metadata = InstallationMetadata { + skill_version: SKILL_VERSION.to_string(), + fetch_version: core::version().to_string(), + files: bundled_hashes(), + }; + let mut bytes = serde_json::to_vec_pretty(&metadata) + .map_err(|error| FetchError::Message(error.to_string()))?; + bytes.push(b'\n'); + atomic_write(path.join(METADATA_FILE), &bytes)?; + Ok(()) +} + +fn atomic_write(path: PathBuf, bytes: &[u8]) -> io::Result<()> { + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + fs::create_dir_all(parent)?; + let temp = temporary_path(parent, ".fetch-skill-file"); + let mut options = fs::OpenOptions::new(); + options.create_new(true).write(true); + let mut file = options.open(&temp)?; + file.write_all(bytes)?; + file.sync_all()?; + drop(file); + let result = if path.exists() { + fileutil::atomic_replace_file(&temp, &path) + } else { + fileutil::atomic_write_new_file(&temp, &path) + }; + if result.is_err() { + let _ = fs::remove_file(&temp); + } + result +} + +fn temporary_path(parent: &Path, prefix: &str) -> PathBuf { + use std::time::{SystemTime, UNIX_EPOCH}; + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + parent.join(format!("{prefix}-{}-{nonce}", std::process::id())) +} + +fn remove_path(path: &Path) -> io::Result<()> { + let Ok(metadata) = fs::symlink_metadata(path) else { + return Ok(()); + }; + if metadata.file_type().is_symlink() || metadata.is_file() { + fs::remove_file(path) + } else { + fs::remove_dir_all(path) + } +} + +fn bundled_hashes() -> BTreeMap { + FILES + .iter() + .map(|(path, contents)| ((*path).to_string(), hash(contents.as_bytes()))) + .collect() +} + +fn installation_state(path: &Path) -> Result { + let Ok(path_metadata) = fs::symlink_metadata(path) else { + return Ok(InstallationState::Missing); + }; + if path_metadata.file_type().is_symlink() { + let Ok(target) = fs::canonicalize(path) else { + return Ok(InstallationState::Modified); + }; + return installation_state(&target); + } + if !path_metadata.is_dir() { + return Ok(InstallationState::Modified); + } + + let expected = bundled_hashes(); + let metadata_path = path.join(METADATA_FILE); + let recorded = fs::read(&metadata_path) + .ok() + .and_then(|bytes| serde_json::from_slice::(&bytes).ok()); + let expected_files = recorded + .as_ref() + .map(|metadata| &metadata.files) + .unwrap_or(&expected); + let actual_paths = installed_relative_files(path)?; + let expected_paths: BTreeSet<_> = expected_files.keys().cloned().collect(); + if actual_paths != expected_paths { + return Ok(InstallationState::Modified); + } + for (relative, expected_hash) in expected_files { + let bytes = fs::read(path.join(relative))?; + if hash(&bytes) != *expected_hash { + return Ok(InstallationState::Modified); + } + } + let Some(recorded) = recorded else { + return Ok(if expected_files == &expected { + InstallationState::Current + } else { + InstallationState::Outdated + }); + }; + if recorded.skill_version == SKILL_VERSION && recorded.files == expected { + Ok(InstallationState::Current) + } else { + Ok(InstallationState::Outdated) + } +} + +fn installed_relative_files(root: &Path) -> io::Result> { + fn visit(root: &Path, dir: &Path, result: &mut BTreeSet) -> io::Result<()> { + for entry in fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + let metadata = fs::symlink_metadata(&path)?; + if metadata.is_dir() && !metadata.file_type().is_symlink() { + visit(root, &path, result)?; + } else if path.file_name().and_then(|name| name.to_str()) != Some(METADATA_FILE) { + let relative = path.strip_prefix(root).expect("walk remains below root"); + result.insert(relative.to_string_lossy().replace('\\', "/")); + } + } + Ok(()) + } + let mut result = BTreeSet::new(); + visit(root, root, &mut result)?; + Ok(result) +} + +fn hash(bytes: &[u8]) -> String { + let digest = Sha256::digest(bytes); + digest.iter().map(|byte| format!("{byte:02x}")).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use clap::Parser; + + #[test] + fn skill_actions_reject_ignored_options() { + for args in [ + vec!["fetch", "--skill", "--update"], + vec!["fetch", "--skill", "--scope", "project"], + vec!["fetch", "--skill", "--header", "x-test: true"], + vec!["fetch", "--install-skill", "--method", "POST"], + vec!["fetch", "--uninstall-skill", "--complete", "bash"], + ] { + let cli = Cli::try_parse_from(&args).unwrap(); + let error = validate_cli(&cli).unwrap_err().to_string(); + assert!( + error.contains("cannot be used"), + "unexpected validation for {args:?}: {error}" + ); + } + } + + #[test] + fn skill_actions_accept_only_their_applicable_modifiers() { + for args in [ + vec!["fetch", "--skill"], + vec![ + "fetch", + "--install-skill", + "pi", + "--scope", + "project", + "--dry-run", + "--force", + ], + vec![ + "fetch", + "--uninstall-skill", + "all", + "--scope", + "user", + "--dry-run", + "--force", + ], + ] { + let cli = Cli::try_parse_from(&args).unwrap(); + validate_cli(&cli) + .unwrap_or_else(|error| panic!("unexpected validation for {args:?}: {error}")); + } + } + + #[test] + fn installation_detects_local_modifications() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("fetch"); + install_directory(&path, false).unwrap(); + assert_eq!( + installation_state(&path).unwrap(), + InstallationState::Current + ); + + fs::write(path.join("SKILL.md"), "changed").unwrap(); + assert_eq!( + installation_state(&path).unwrap(), + InstallationState::Modified + ); + assert!(install_directory(&path, false).is_err()); + install_directory(&path, true).unwrap(); + assert_eq!( + installation_state(&path).unwrap(), + InstallationState::Current + ); + } + + #[test] + fn installation_records_versions_and_every_bundled_file() { + let temp = tempfile::tempdir().unwrap(); + let path = temp.path().join("fetch"); + install_directory(&path, false).unwrap(); + let metadata: InstallationMetadata = + serde_json::from_slice(&fs::read(path.join(METADATA_FILE)).unwrap()).unwrap(); + assert_eq!(metadata.skill_version, SKILL_VERSION); + assert_eq!(metadata.fetch_version, core::version()); + assert_eq!(metadata.files, bundled_hashes()); + } +} diff --git a/tests/cli.rs b/tests/cli.rs index 170468f..155c281 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,13 +1,21 @@ mod support; use std::fs; +#[cfg(unix)] +use std::io::Write; use std::path::Path; +#[cfg(unix)] +use std::process::Command; +#[cfg(unix)] +use std::time::Duration; use support::common::{FetchOpts, assert_exit, host_port, run_fetch, run_fetch_opts}; #[cfg(unix)] -use support::common::{assert_no_closed_stdout_panic, run_fetch_with_closed_stdout}; +use support::common::{assert_no_closed_stdout_panic, fetch_bin, run_fetch_with_closed_stdout}; #[cfg(unix)] use support::grpc::{start_reflection_grpc_h2c_server, write_health_descriptor_set}; use support::http::{TestResponse, TestServer}; +#[cfg(unix)] +use support::pty::{configure_pty_child, open_pty, start_pty_capture}; use tempfile::TempDir; #[test] @@ -453,6 +461,170 @@ fn config_error_and_metadata_edges() { } } +#[test] +fn bundled_skill_can_be_printed_and_installed_offline_for_pi() { + let skill = run_fetch(&["--skill"]); + assert_exit(&skill, 0); + assert!(skill.stdout.starts_with("---\nname: fetch\n")); + + let home = TempDir::new().unwrap(); + let home_value = home.path().to_string_lossy().into_owned(); + let installed = run_fetch_opts( + FetchOpts { + env: vec![("HOME".to_string(), home_value)], + ..FetchOpts::default() + }, + &["--install-skill", "pi"], + ); + assert_exit(&installed, 0); + let destination = home.path().join(".pi/agent/skills/fetch"); + assert_eq!( + fs::read_to_string(destination.join("SKILL.md")).unwrap(), + skill.stdout + ); + let metadata = fs::read_to_string(destination.join(".fetch-skill.json")).unwrap(); + assert!(metadata.contains("\"skill_version\": \"1\"")); + assert!(metadata.contains("\"fetch_version\"")); +} + +#[test] +fn skill_install_default_uses_generic_agents_location() { + let home = TempDir::new().unwrap(); + let result = run_fetch_opts( + FetchOpts { + env: vec![( + "HOME".to_string(), + home.path().to_string_lossy().into_owned(), + )], + ..FetchOpts::default() + }, + &["--install-skill"], + ); + assert_exit(&result, 0); + assert!(home.path().join(".agents/skills/fetch/SKILL.md").is_file()); + assert!(!home.path().join(".codex").exists()); + assert!(!home.path().join(".pi").exists()); +} + +#[test] +fn skill_actions_reject_options_that_would_be_ignored() { + for args in [ + &["--skill", "--update"][..], + &["--skill", "--scope", "project"], + &["--skill", "--header", "x-test: true"], + &["--install-skill", "--method", "POST"], + &["--uninstall-skill", "--complete", "bash"], + ] { + let result = run_fetch(args); + assert_exit(&result, 1); + assert!( + result.stderr.contains("cannot be used"), + "unexpected error for {args:?}: {}", + result.stderr + ); + } +} + +#[test] +fn skill_install_dry_run_and_modification_guard_are_safe() { + let home = TempDir::new().unwrap(); + let home_value = home.path().to_string_lossy().into_owned(); + let opts = || FetchOpts { + env: vec![("HOME".to_string(), home_value.clone())], + ..FetchOpts::default() + }; + + let dry_run = run_fetch_opts(opts(), &["--install-skill", "all", "--dry-run"]); + assert_exit(&dry_run, 0); + for path in [ + ".agents/skills/fetch", + ".codex/skills/fetch", + ".claude/skills/fetch", + ".gemini/skills/fetch", + ".pi/agent/skills/fetch", + ] { + assert!(dry_run.stderr.contains(path), "missing destination {path}"); + } + assert!(!home.path().join(".agents").exists()); + + assert_exit(&run_fetch_opts(opts(), &["--install-skill", "all"]), 0); + for path in [ + ".agents/skills/fetch/SKILL.md", + ".codex/skills/fetch/SKILL.md", + ".claude/skills/fetch/SKILL.md", + ".gemini/skills/fetch/SKILL.md", + ".pi/agent/skills/fetch/SKILL.md", + ] { + assert!(home.path().join(path).is_file(), "missing installed {path}"); + } + let skill = home.path().join(".pi/agent/skills/fetch/SKILL.md"); + fs::write(&skill, "locally modified\n").unwrap(); + let guarded = run_fetch_opts(opts(), &["--install-skill", "pi"]); + assert_exit(&guarded, 1); + assert!(guarded.stderr.contains("refusing to overwrite modified")); + + assert_exit( + &run_fetch_opts(opts(), &["--install-skill", "pi", "--force"]), + 0, + ); + assert_ne!(fs::read_to_string(skill).unwrap(), "locally modified\n"); +} + +#[test] +fn uninstalling_missing_project_skill_does_not_create_files() { + let project = TempDir::new().unwrap(); + let result = run_fetch_opts( + FetchOpts { + cwd: Some(project.path().to_path_buf()), + ..FetchOpts::default() + }, + &["--uninstall-skill", "pi", "--scope", "project"], + ); + assert_exit(&result, 0); + assert!(result.stderr.contains(".pi/skills/fetch")); + assert!(result.stderr.contains("nothing to remove")); + assert!( + fs::read_dir(project.path()).unwrap().next().is_none(), + "missing-skill uninstall changed the project directory" + ); +} + +#[cfg(unix)] +#[test] +fn skill_uninstall_rechecks_modifications_after_confirmation() { + let home = TempDir::new().unwrap(); + let home_value = home.path().to_string_lossy().into_owned(); + let installed = run_fetch_opts( + FetchOpts { + env: vec![("HOME".to_string(), home_value.clone())], + ..FetchOpts::default() + }, + &["--install-skill", "pi"], + ); + assert_exit(&installed, 0); + + let pair = open_pty(24, 100, 0, 0); + let capture = start_pty_capture(&pair.master); + let mut command = Command::new(fetch_bin()); + command.args(["--uninstall-skill", "pi"]); + command.env("HOME", &home_value).env("NO_COLOR", ""); + configure_pty_child(&mut command, &pair.slave); + let mut child = command.spawn().unwrap(); + drop(pair.slave); + + capture.wait_for("Uninstall the fetch skill? [y/N]", Duration::from_secs(5)); + let skill = home.path().join(".pi/agent/skills/fetch/SKILL.md"); + fs::write(&skill, "modified while confirmation was pending\n").unwrap(); + let mut input = capture.file.try_clone().unwrap(); + input.write_all(b"y\n").unwrap(); + + let status = child.wait().unwrap(); + assert!(!status.success()); + assert!(skill.exists(), "modified installation was removed"); + assert!(capture.output().contains("refusing to remove modified")); + capture.close(); +} + #[test] fn integration_support_stays_domain_split() { let root = Path::new(env!("CARGO_MANIFEST_DIR")); From e9e41f5c74cadd16f161c7e5d308de2f2e3b010d Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Mon, 13 Jul 2026 10:21:28 -0400 Subject: [PATCH 2/2] test: normalize skill output on Windows --- tests/cli.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/cli.rs b/tests/cli.rs index 155c281..8dc8f16 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -465,7 +465,8 @@ fn config_error_and_metadata_edges() { fn bundled_skill_can_be_printed_and_installed_offline_for_pi() { let skill = run_fetch(&["--skill"]); assert_exit(&skill, 0); - assert!(skill.stdout.starts_with("---\nname: fetch\n")); + let normalized_skill = skill.stdout.replace("\r\n", "\n"); + assert!(normalized_skill.starts_with("---\nname: fetch\n")); let home = TempDir::new().unwrap(); let home_value = home.path().to_string_lossy().into_owned(); @@ -536,6 +537,7 @@ fn skill_install_dry_run_and_modification_guard_are_safe() { let dry_run = run_fetch_opts(opts(), &["--install-skill", "all", "--dry-run"]); assert_exit(&dry_run, 0); + let dry_run_stderr = dry_run.stderr.replace('\\', "/"); for path in [ ".agents/skills/fetch", ".codex/skills/fetch", @@ -543,7 +545,7 @@ fn skill_install_dry_run_and_modification_guard_are_safe() { ".gemini/skills/fetch", ".pi/agent/skills/fetch", ] { - assert!(dry_run.stderr.contains(path), "missing destination {path}"); + assert!(dry_run_stderr.contains(path), "missing destination {path}"); } assert!(!home.path().join(".agents").exists()); @@ -581,8 +583,9 @@ fn uninstalling_missing_project_skill_does_not_create_files() { &["--uninstall-skill", "pi", "--scope", "project"], ); assert_exit(&result, 0); - assert!(result.stderr.contains(".pi/skills/fetch")); - assert!(result.stderr.contains("nothing to remove")); + let stderr = result.stderr.replace('\\', "/"); + assert!(stderr.contains(".pi/skills/fetch")); + assert!(stderr.contains("nothing to remove")); assert!( fs::read_dir(project.path()).unwrap().next().is_none(), "missing-skill uninstall changed the project directory"