agent_first_data/

skill.rs

//! Reusable Agent Skill installer for spore CLIs.
//!
//! A spore that embeds its `SKILL.md` describes itself with a [`SkillSpec`] and calls
//! [`run_skill_admin`] to install, uninstall, or report status of that skill across supported
//! coding agents (Codex, Claude Code, opencode, Hermes).
//!
//! The function performs the filesystem work and returns a typed [`SkillReport`] (the caller
//! serializes it for output) or a [`SkillError`]. It never writes to stdout/stderr itself.
//!
//! Requires the `skill-admin` feature.

use serde::Serialize;
use std::path::{Path, PathBuf};

const SKILL_FILE_NAME: &str = "SKILL.md";
const FNV1A64_OFFSET: u64 = 0xcbf2_9ce4_8422_2325;
const FNV1A64_PRIME: u64 = 0x0000_0100_0000_01b3;

/// A bundled auxiliary file installed alongside `SKILL.md` under the skill
/// directory — for example a `references/` document a skill points its agent to.
///
/// The file is written verbatim; unlike `SKILL.md` it carries no managed marker.
#[derive(Clone, Copy, Debug)]
pub struct SkillAsset<'a> {
    /// Path relative to the skill directory, using `/` separators
    /// (e.g. `references/naming-output.md`). Must be relative with no `.` or `..`
    /// segment, so an asset can never escape the skill directory.
    pub path: &'a str,
    /// Bundled file contents (typically `include_str!`).
    pub contents: &'a str,
}

/// Identity of the skill being managed and the tool that manages it.
///
/// `name` is both the skill directory name and the `name:` front-matter field. `source` is the
/// bundled `SKILL.md` (typically `include_str!`). `title` is a human label for error messages.
/// `marker_slug` seeds the managed-skill marker and the `Generated by <slug> skill install`
/// comment, and is referenced in hints (e.g. `afwidget`). `assets` are auxiliary files bundled
/// under the skill directory (e.g. `references/`); empty for a single-file skill.
#[derive(Clone, Copy, Debug)]
pub struct SkillSpec<'a> {
    /// Skill directory name and front-matter `name` (e.g. `agent-first-widget`).
    pub name: &'a str,
    /// Bundled `SKILL.md` contents.
    pub source: &'a str,
    /// Human-readable skill title for error messages (e.g. `Agent-First Widget`).
    pub title: &'a str,
    /// Short tool slug used in the managed marker, generated-by comment, and hints (e.g. `afwidget`).
    pub marker_slug: &'a str,
    /// Auxiliary files bundled under the skill directory alongside `SKILL.md`
    /// (e.g. `references/`). Installed and removed with the skill; empty `&[]`
    /// for a single-file skill.
    pub assets: &'a [SkillAsset<'a>],
}

/// Which agent target(s) to manage.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum SkillAgentSelection {
    /// Every agent that supports the requested scope.
    All,
    /// Codex.
    Codex,
    /// Claude Code.
    ClaudeCode,
    /// opencode.
    Opencode,
    /// Hermes Agent.
    Hermes,
}

/// A concrete agent a skill is installed for (no `All`).
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum SkillAgent {
    /// Codex (`$CODEX_HOME/skills`, `~/.codex/skills`, or `.codex/skills`).
    Codex,
    /// Claude Code (`~/.claude/skills` or `.claude/skills`).
    ClaudeCode,
    /// opencode (`~/.config/opencode/skills` or `.opencode/skills`).
    Opencode,
    /// Hermes Agent (`$HERMES_HOME/skills`, `~/.hermes/skills`, or `.hermes/skills`).
    Hermes,
}

/// Where to install the skill.
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)]
#[serde(rename_all = "lowercase")]
pub enum SkillScope {
    /// User-level skills directory.
    Personal,
    /// Current workspace's skills directory.
    Workspace,
}

/// Options shared by every skill action.
#[derive(Clone, Debug)]
pub struct SkillOptions {
    /// Agent target selection.
    pub agent: SkillAgentSelection,
    /// Skill scope.
    pub scope: SkillScope,
    /// Explicit skills directory; requires a single concrete `agent`.
    pub skills_dir: Option<String>,
    /// Overwrite or remove a skill that this tool did not manage.
    pub force: bool,
}

/// The skill action to perform.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum SkillAction {
    /// Report whether the skill is installed, valid, managed, and current.
    Status,
    /// Install (or refresh) the skill.
    Install,
    /// Remove a managed skill.
    Uninstall,
}

/// Per-target outcome of `status` / `install`.
#[derive(Clone, Debug, Serialize)]
pub struct SkillTargetStatus {
    /// The agent this target belongs to.
    pub agent: SkillAgent,
    /// The scope this target belongs to.
    pub scope: SkillScope,
    /// Directory that holds skill folders.
    pub skills_dir: PathBuf,
    /// Directory for this skill under `skills_dir`.
    pub skill_dir: PathBuf,
    /// Full path to the target `SKILL.md`.
    pub skill_path: PathBuf,
    /// Whether a skill file exists at `skill_path`.
    pub installed: bool,
    /// Whether the installed file was generated by this tool (or is byte-equal to the bundle).
    pub managed: bool,
    /// Whether the installed file has valid front matter.
    pub valid: bool,
    /// Whether the installed content matches the bundled skill (up to date).
    pub current: bool,
    /// Front-matter validation error, when the installed file is invalid.
    pub validation_error: Option<String>,
}

/// Per-target outcome of `uninstall`.
#[derive(Clone, Debug, Serialize)]
pub struct SkillUninstallStatus {
    /// The agent this target belongs to.
    pub agent: SkillAgent,
    /// The scope this target belongs to.
    pub scope: SkillScope,
    /// Directory that holds skill folders.
    pub skills_dir: PathBuf,
    /// Directory for this skill under `skills_dir`.
    pub skill_dir: PathBuf,
    /// Full path to the target `SKILL.md`.
    pub skill_path: PathBuf,
    /// Whether the skill file itself was removed (false if nothing was installed).
    pub removed: bool,
    /// Bundled asset paths, relative to the skill directory, that were removed
    /// with it. Empty for a single-file skill, and for a target that had
    /// nothing installed.
    pub assets_removed: Vec<String>,
    /// Whether the skill's own directory outlived the uninstall — which happens
    /// when it holds files this tool did not install. They are left untouched,
    /// and saying so is the difference between a clean removal and one that
    /// left something behind.
    pub directory_retained: bool,
}

/// The result of a skill action. Serializes to the protocol shape, carrying a `code`
/// discriminator (`skill_status` / `skill_install` / `skill_uninstall`).
#[derive(Clone, Debug, Serialize)]
#[serde(tag = "code")]
pub enum SkillReport {
    /// `status` result.
    #[serde(rename = "skill_status")]
    Status {
        /// Skill name.
        skill: String,
        /// True when every target is installed.
        installed_all: bool,
        /// True when every target has valid front matter.
        valid_all: bool,
        /// True when every target is up to date with the bundle.
        current_all: bool,
        /// Per-target detail.
        targets: Vec<SkillTargetStatus>,
    },
    /// `install` result.
    #[serde(rename = "skill_install")]
    Install {
        /// Skill name.
        skill: String,
        /// Always true (install succeeded for every target).
        installed: bool,
        /// Per-target detail after writing.
        targets: Vec<SkillTargetStatus>,
        /// Operator hint.
        hint: &'static str,
    },
    /// `uninstall` result.
    #[serde(rename = "skill_uninstall")]
    Uninstall {
        /// Skill name.
        skill: String,
        /// True when at least one file was removed.
        removed_any: bool,
        /// Per-target detail.
        targets: Vec<SkillUninstallStatus>,
    },
}

/// What kind of failure a [`SkillError`] is, so a caller can pick an exit
/// status and a stable code without parsing the message.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum SkillErrorKind {
    /// The options cannot name a target: `--skills-dir` with every agent, or
    /// no usable home directory. Nothing was touched.
    Usage,
    /// The embedding program's [`SkillSpec`] is malformed — a defect in the
    /// program, not in the call.
    InvalidSpec,
    /// A target holds something afdata will not overwrite or remove: an
    /// unmanaged or invalid skill, a symlink, a path outside the skills
    /// directory.
    Refused,
    /// The filesystem failed while reading or writing a target.
    Io,
}

impl SkillErrorKind {
    /// The stable `error.code` for this kind.
    #[must_use]
    pub fn code(self) -> &'static str {
        match self {
            Self::Usage => "skill_usage_error",
            Self::InvalidSpec => "skill_spec_invalid",
            Self::Refused => "skill_refused",
            Self::Io => "skill_io_failed",
        }
    }
}

/// A skill admin failure with an operator-facing message and optional hint.
#[derive(Clone, Debug)]
pub struct SkillError {
    /// What kind of failure this is.
    pub kind: SkillErrorKind,
    /// What went wrong.
    pub message: String,
    /// Optional remediation hint.
    pub hint: Option<String>,
    /// Per-target report captured after a multi-target operation failed.
    pub partial_report: Option<Box<SkillReport>>,
}

impl std::fmt::Display for SkillError {
    fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        formatter.write_str(&self.message)
    }
}

impl std::error::Error for SkillError {}

impl SkillError {
    fn new(kind: SkillErrorKind, message: String, hint: Option<String>) -> Self {
        Self {
            kind,
            message,
            hint,
            partial_report: None,
        }
    }

    fn io(action: &str, err: std::io::Error) -> Self {
        Self {
            kind: SkillErrorKind::Io,
            message: format!("{action} failed: {err}"),
            hint: None,
            partial_report: None,
        }
    }

    /// A failed atomic installation, which names the step it failed at.
    fn write_failed(action: &str, err: crate::atomic_file::AtomicError) -> Self {
        // Only the closing directory fsync can fail with the new file already
        // in place. Saying so is the difference between "nothing happened" and
        // "it happened but may not survive a crash", and only the second one
        // asks the operator to re-run.
        let hint = err.commit_uncertain().then(|| {
            "the file is installed but its durability is unconfirmed; re-run to be sure".to_string()
        });
        Self {
            kind: SkillErrorKind::Io,
            message: format!("{action} failed: {err}"),
            hint,
            partial_report: None,
        }
    }

    fn with_partial_report(mut self, report: SkillReport) -> Self {
        self.partial_report = Some(Box::new(report));
        self
    }
}

/// Install, uninstall, or report status of `spec`'s skill across the selected agent target(s).
///
/// Returns a typed [`SkillReport`] (caller serializes it for output) or a [`SkillError`].
/// Does not touch stdout/stderr.
pub fn run_skill_admin(
    spec: &SkillSpec,
    action: SkillAction,
    options: &SkillOptions,
) -> Result<SkillReport, SkillError> {
    validate_spec(spec)?;
    match action {
        SkillAction::Status => status(spec, options),
        SkillAction::Install => install(spec, options),
        SkillAction::Uninstall => uninstall(spec, options),
    }
}

fn status(spec: &SkillSpec, options: &SkillOptions) -> Result<SkillReport, SkillError> {
    let targets = resolve_targets(spec, options)?;
    let mut statuses = Vec::with_capacity(targets.len());
    for target in &targets {
        statuses.push(target_status(spec, target)?);
    }
    Ok(SkillReport::Status {
        skill: spec.name.to_string(),
        installed_all: statuses.iter().all(|s| s.installed),
        valid_all: statuses.iter().all(|s| s.valid),
        current_all: statuses.iter().all(|s| s.current),
        targets: statuses,
    })
}

fn install(spec: &SkillSpec, options: &SkillOptions) -> Result<SkillReport, SkillError> {
    validate_skill_text(spec, spec.source, SkillErrorKind::InvalidSpec)?;
    for asset in spec.assets {
        validate_asset_path(asset.path)?;
    }
    let targets = resolve_targets(spec, options)?;
    let content = managed_skill_contents(spec);
    preflight_install_targets(spec, options, &targets)?;
    for target in &targets {
        if let Err(err) = std::fs::create_dir_all(&target.skill_dir)
            .map_err(|e| SkillError::io("create skill dir", e))
        {
            return Err(err.with_partial_report(install_report_lossy(spec, &targets, false)));
        }
    }
    install_targets(spec, &targets, &content)
}

fn preflight_install_targets(
    spec: &SkillSpec,
    options: &SkillOptions,
    targets: &[SkillTarget],
) -> Result<(), SkillError> {
    let mut failures = Vec::new();
    let mut escaped = false;
    for target in targets {
        // Before anything about the file itself: nothing on the way down to it
        // may be a link out of the skills directory.
        if let Err(err) = containment_failure(spec, target) {
            failures.push(err);
            escaped = true;
            continue;
        }
        // An asset path is checked before anything is written, for the same
        // reason the skill file is: a directory found there halfway through
        // leaves `SKILL.md` installed beside a missing reference, and a file the
        // user put there is theirs until a managed `SKILL.md` claims the
        // directory. Byte-equal content is already what install would write.
        let skill_dir_managed = skill_path_file_type(&target.skill_path)?
            .is_some_and(|kind| kind.is_file())
            && is_managed_or_bundled_skill(spec, &target.skill_path)?;
        for asset in spec.assets {
            let dest = asset_target_path(&target.skill_dir, asset.path);
            let Some(kind) = skill_path_file_type(&dest)? else {
                continue;
            };
            if !kind.is_file() && !kind.is_symlink() {
                failures.push(format!(
                    "refusing to overwrite non-regular skill asset at {}",
                    dest.display()
                ));
            } else if !skill_dir_managed
                && !options.force
                && (kind.is_symlink()
                    || std::fs::read(&dest).map_err(|e| SkillError::io("read skill asset", e))?
                        != asset.contents.as_bytes())
            {
                failures.push(format!(
                    "refusing to overwrite unmanaged skill asset at {}",
                    dest.display()
                ));
            }
        }
        if let Some(kind) = skill_path_file_type(&target.skill_path)? {
            if kind.is_symlink() {
                if !options.force {
                    failures.push(format!(
                        "refusing to overwrite symlinked skill at {}",
                        target.skill_path.display()
                    ));
                }
                continue;
            }
            if !kind.is_file() {
                failures.push(format!(
                    "refusing to overwrite non-regular skill at {}",
                    target.skill_path.display()
                ));
                continue;
            }
            if !is_managed_or_bundled_skill(spec, &target.skill_path)? && !options.force {
                failures.push(format!(
                    "refusing to overwrite unmanaged skill at {}",
                    target.skill_path.display()
                ));
            }
        }
    }
    if failures.is_empty() {
        return Ok(());
    }
    // `--force` replaces an unmanaged file inside the skills directory. It is
    // not permission to write outside it, so a containment failure must not be
    // answered with a hint that reads like one.
    let hint = if escaped {
        "--force does not permit writing outside the skills directory"
    } else {
        "pass --force to replace unmanaged files or symlinks"
    };
    Err(SkillError::new(
        SkillErrorKind::Refused,
        failures.join("; "),
        Some(hint.to_string()),
    )
    .with_partial_report(install_report_lossy(spec, targets, false)))
}

fn install_targets(
    spec: &SkillSpec,
    targets: &[SkillTarget],
    content: &str,
) -> Result<SkillReport, SkillError> {
    let mut installed = Vec::with_capacity(targets.len());
    for target in targets {
        if let Err(err) = write_skill_atomic(target, content) {
            return Err(err.with_partial_report(install_report_lossy(spec, targets, false)));
        }
        if let Err(err) = install_target_assets(spec, target) {
            return Err(err.with_partial_report(install_report_lossy(spec, targets, false)));
        }
        if let Err(err) = validate_installed_skill(spec, &target.skill_path) {
            return Err(err.with_partial_report(install_report_lossy(spec, targets, false)));
        }
        match target_status(spec, target) {
            Ok(status) => installed.push(status),
            Err(err) => {
                return Err(err.with_partial_report(install_report_lossy(spec, targets, false)));
            }
        }
    }
    Ok(SkillReport::Install {
        skill: spec.name.to_string(),
        installed: true,
        targets: installed,
        hint: "restart the agent so it reloads installed skills",
    })
}

fn uninstall(spec: &SkillSpec, options: &SkillOptions) -> Result<SkillReport, SkillError> {
    let targets = resolve_targets(spec, options)?;
    preflight_uninstall_targets(spec, options, &targets)?;
    let mut removed = Vec::with_capacity(targets.len());
    for target in &targets {
        let Some(kind) = skill_path_file_type(&target.skill_path)? else {
            removed.push(target_uninstall_status(target, false));
            continue;
        };
        if !kind.is_file() && !kind.is_symlink() {
            let err = SkillError::new(
                SkillErrorKind::Refused,
                format!(
                    "refusing to remove non-regular skill at {}",
                    target.skill_path.display()
                ),
                None,
            );
            return Err(err.with_partial_report(uninstall_report_lossy(spec, &targets, &removed)));
        }
        if let Err(err) = ensure_no_symlinked_dirs(&target.skills_dir, &target.skill_path) {
            return Err(err.with_partial_report(uninstall_report_lossy(spec, &targets, &removed)));
        }
        // Assets go first and `SKILL.md` last, so `SKILL.md` keeps marking the
        // directory as this tool's until everything else is gone. A target that
        // fails partway therefore still reads as installed, and running
        // uninstall again finishes the job instead of skipping a directory whose
        // marker was already removed.
        let assets_removed = match remove_target_assets(spec, target) {
            Ok(assets_removed) => assets_removed,
            Err(err) => {
                return Err(
                    err.with_partial_report(uninstall_report_lossy(spec, &targets, &removed))
                );
            }
        };
        if let Err(err) =
            std::fs::remove_file(&target.skill_path).map_err(|e| SkillError::io("remove skill", e))
        {
            return Err(err.with_partial_report(uninstall_report_lossy(spec, &targets, &removed)));
        }
        let _ = std::fs::remove_dir(&target.skill_dir);
        removed.push(SkillUninstallStatus {
            assets_removed,
            ..target_uninstall_status(target, true)
        });
    }
    Ok(SkillReport::Uninstall {
        skill: spec.name.to_string(),
        removed_any: removed.iter().any(|s| s.removed),
        targets: removed,
    })
}

fn preflight_uninstall_targets(
    spec: &SkillSpec,
    options: &SkillOptions,
    targets: &[SkillTarget],
) -> Result<(), SkillError> {
    let mut failures = Vec::new();
    let mut escaped = false;
    for target in targets {
        if let Err(err) = containment_failure(spec, target) {
            failures.push(err);
            escaped = true;
            continue;
        }
        // A managed asset path now holding a directory is checked here rather
        // than discovered halfway through: by then the skill file is gone and
        // the target is half-removed.
        for asset in spec.assets {
            let dest = asset_target_path(&target.skill_dir, asset.path);
            if let Some(kind) = skill_path_file_type(&dest)?
                && kind.is_dir()
            {
                failures.push(format!(
                    "refusing to remove bundled asset {} because it is a directory",
                    dest.display()
                ));
            }
        }
        let Some(kind) = skill_path_file_type(&target.skill_path)? else {
            continue;
        };
        if kind.is_symlink() {
            if !options.force {
                failures.push(format!(
                    "refusing to remove symlinked skill at {}",
                    target.skill_path.display()
                ));
            }
            continue;
        }
        if !kind.is_file() {
            failures.push(format!(
                "refusing to remove non-regular skill at {}",
                target.skill_path.display()
            ));
            continue;
        }
        if !is_managed_or_bundled_skill(spec, &target.skill_path)? && !options.force {
            failures.push(format!(
                "refusing to remove unmanaged skill at {}",
                target.skill_path.display()
            ));
        }
    }
    if failures.is_empty() {
        return Ok(());
    }
    let hint = if escaped {
        "--force does not permit removing files outside the skills directory".to_string()
    } else {
        format!(
            "only skills generated by {} skill install can be removed without --force",
            spec.marker_slug
        )
    };
    Err(
        SkillError::new(SkillErrorKind::Refused, failures.join("; "), Some(hint))
            .with_partial_report(uninstall_report_lossy(spec, targets, &[])),
    )
}

struct SkillTarget {
    agent: SkillAgent,
    scope: SkillScope,
    skills_dir: PathBuf,
    skill_dir: PathBuf,
    skill_path: PathBuf,
}

fn resolve_targets(
    spec: &SkillSpec,
    options: &SkillOptions,
) -> Result<Vec<SkillTarget>, SkillError> {
    if options.skills_dir.is_some() && options.agent == SkillAgentSelection::All {
        return Err(SkillError::new(
            SkillErrorKind::Usage,
            "--skills-dir requires a single --agent".to_string(),
            Some("custom skills directories are ambiguous when --agent all is used".to_string()),
        ));
    }
    match (options.agent, options.scope) {
        (SkillAgentSelection::All, SkillScope::Personal) => Ok(vec![
            resolve_target(spec, SkillAgent::Codex, SkillScope::Personal, None)?,
            resolve_target(spec, SkillAgent::ClaudeCode, SkillScope::Personal, None)?,
            resolve_target(spec, SkillAgent::Opencode, SkillScope::Personal, None)?,
            resolve_target(spec, SkillAgent::Hermes, SkillScope::Personal, None)?,
        ]),
        (SkillAgentSelection::All, SkillScope::Workspace) => Ok(vec![
            resolve_target(spec, SkillAgent::Codex, SkillScope::Workspace, None)?,
            resolve_target(spec, SkillAgent::ClaudeCode, SkillScope::Workspace, None)?,
            resolve_target(spec, SkillAgent::Opencode, SkillScope::Workspace, None)?,
            resolve_target(spec, SkillAgent::Hermes, SkillScope::Workspace, None)?,
        ]),
        (SkillAgentSelection::Codex, SkillScope::Workspace) => Ok(vec![resolve_target(
            spec,
            SkillAgent::Codex,
            SkillScope::Workspace,
            options.skills_dir.as_deref(),
        )?]),
        (SkillAgentSelection::Codex, SkillScope::Personal) => Ok(vec![resolve_target(
            spec,
            SkillAgent::Codex,
            SkillScope::Personal,
            options.skills_dir.as_deref(),
        )?]),
        (SkillAgentSelection::ClaudeCode, scope) => Ok(vec![resolve_target(
            spec,
            SkillAgent::ClaudeCode,
            scope,
            options.skills_dir.as_deref(),
        )?]),
        (SkillAgentSelection::Opencode, scope) => Ok(vec![resolve_target(
            spec,
            SkillAgent::Opencode,
            scope,
            options.skills_dir.as_deref(),
        )?]),
        (SkillAgentSelection::Hermes, scope) => Ok(vec![resolve_target(
            spec,
            SkillAgent::Hermes,
            scope,
            options.skills_dir.as_deref(),
        )?]),
    }
}

fn resolve_target(
    spec: &SkillSpec,
    agent: SkillAgent,
    scope: SkillScope,
    skills_dir: Option<&str>,
) -> Result<SkillTarget, SkillError> {
    let skills_dir = match skills_dir {
        Some(dir) => expand_tilde(dir)?,
        None => default_skills_dir(agent, scope)?,
    };
    let skill_dir = skills_dir.join(spec.name);
    let skill_path = skill_dir.join(SKILL_FILE_NAME);
    Ok(SkillTarget {
        agent,
        scope,
        skills_dir,
        skill_dir,
        skill_path,
    })
}

fn default_skills_dir(agent: SkillAgent, scope: SkillScope) -> Result<PathBuf, SkillError> {
    match (agent, scope) {
        (SkillAgent::Codex, SkillScope::Personal) => {
            if let Some(codex_home) = env_dir("CODEX_HOME") {
                Ok(codex_home.join("skills"))
            } else {
                Ok(home_dir()?.join(".codex").join("skills"))
            }
        }
        (SkillAgent::Codex, SkillScope::Workspace) => workspace_skills_dir(".codex"),
        (SkillAgent::ClaudeCode, SkillScope::Personal) => {
            Ok(home_dir()?.join(".claude").join("skills"))
        }
        (SkillAgent::ClaudeCode, SkillScope::Workspace) => workspace_skills_dir(".claude"),
        (SkillAgent::Opencode, SkillScope::Personal) => {
            if let Some(xdg) = env_dir("XDG_CONFIG_HOME") {
                Ok(xdg.join("opencode").join("skills"))
            } else {
                Ok(home_dir()?.join(".config").join("opencode").join("skills"))
            }
        }
        (SkillAgent::Opencode, SkillScope::Workspace) => workspace_skills_dir(".opencode"),
        (SkillAgent::Hermes, SkillScope::Personal) => {
            if let Some(hermes_home) = env_dir("HERMES_HOME") {
                Ok(hermes_home.join("skills"))
            } else {
                Ok(home_dir()?.join(".hermes").join("skills"))
            }
        }
        (SkillAgent::Hermes, SkillScope::Workspace) => workspace_skills_dir(".hermes"),
    }
}

fn workspace_skills_dir(agent_dir: &str) -> Result<PathBuf, SkillError> {
    std::env::current_dir()
        .map(|dir| dir.join(agent_dir).join("skills"))
        .map_err(|e| SkillError::io("resolve current directory", e))
}

fn target_status(spec: &SkillSpec, target: &SkillTarget) -> Result<SkillTargetStatus, SkillError> {
    // Reported rather than raised: one target reachable only through a link
    // must not cost the operator the status of every other target. Nothing is
    // read through it, so this says what is wrong without following it.
    if let Err(message) = containment_failure(spec, target) {
        return Ok(SkillTargetStatus {
            agent: target.agent,
            scope: target.scope,
            skills_dir: target.skills_dir.clone(),
            skill_dir: target.skill_dir.clone(),
            skill_path: target.skill_path.clone(),
            installed: false,
            managed: false,
            valid: false,
            current: false,
            validation_error: Some(message),
        });
    }
    let Some(kind) = skill_path_file_type(&target.skill_path)? else {
        return Ok(SkillTargetStatus {
            agent: target.agent,
            scope: target.scope,
            skills_dir: target.skills_dir.clone(),
            skill_dir: target.skill_dir.clone(),
            skill_path: target.skill_path.clone(),
            installed: false,
            managed: false,
            valid: false,
            current: false,
            validation_error: None,
        });
    };
    let installed = true;
    let mut valid = false;
    let mut current = false;
    let mut validation_error = None;
    let mut managed = false;
    if kind.is_symlink() {
        validation_error = Some("target SKILL.md is a symlink; refusing to follow it".to_string());
    } else if kind.is_file() {
        let text = std::fs::read_to_string(&target.skill_path)
            .map_err(|e| SkillError::io("read skill", e))?;
        managed = skill_text_is_managed_or_bundled(spec, &text);
        current = normalized_content_hash(spec, &text) == source_hash(spec)
            && assets_current(spec, &target.skill_dir);
        match validate_skill_text(spec, &text, SkillErrorKind::Refused) {
            Ok(()) => valid = true,
            Err(err) => validation_error = Some(err.message),
        }
    } else {
        validation_error = Some("target SKILL.md is not a regular file".to_string());
    }
    Ok(SkillTargetStatus {
        agent: target.agent,
        scope: target.scope,
        skills_dir: target.skills_dir.clone(),
        skill_dir: target.skill_dir.clone(),
        skill_path: target.skill_path.clone(),
        installed,
        managed,
        valid,
        current,
        validation_error,
    })
}

fn target_uninstall_status(target: &SkillTarget, removed: bool) -> SkillUninstallStatus {
    SkillUninstallStatus {
        agent: target.agent,
        scope: target.scope,
        skills_dir: target.skills_dir.clone(),
        skill_dir: target.skill_dir.clone(),
        skill_path: target.skill_path.clone(),
        removed,
        assets_removed: Vec::new(),
        directory_retained: std::fs::symlink_metadata(&target.skill_dir).is_ok(),
    }
}

fn target_status_lossy(spec: &SkillSpec, target: &SkillTarget) -> SkillTargetStatus {
    target_status(spec, target).unwrap_or_else(|err| {
        let installed = std::fs::symlink_metadata(&target.skill_path).is_ok();
        SkillTargetStatus {
            agent: target.agent,
            scope: target.scope,
            skills_dir: target.skills_dir.clone(),
            skill_dir: target.skill_dir.clone(),
            skill_path: target.skill_path.clone(),
            installed,
            managed: false,
            valid: false,
            current: false,
            validation_error: Some(err.message),
        }
    })
}

fn install_report_lossy(spec: &SkillSpec, targets: &[SkillTarget], installed: bool) -> SkillReport {
    SkillReport::Install {
        skill: spec.name.to_string(),
        installed,
        targets: targets
            .iter()
            .map(|target| target_status_lossy(spec, target))
            .collect(),
        hint: "restart the agent so it reloads installed skills",
    }
}

fn uninstall_report_lossy(
    spec: &SkillSpec,
    targets: &[SkillTarget],
    removed: &[SkillUninstallStatus],
) -> SkillReport {
    let mut statuses = Vec::with_capacity(targets.len());
    for target in targets {
        if let Some(status) = removed.iter().find(|status| {
            status.agent == target.agent
                && status.scope == target.scope
                && status.skill_path == target.skill_path
        }) {
            statuses.push(status.clone());
        } else {
            statuses.push(target_uninstall_status(target, false));
        }
    }
    SkillReport::Uninstall {
        skill: spec.name.to_string(),
        removed_any: statuses.iter().any(|status| status.removed),
        targets: statuses,
    }
}

fn generated_by(spec: &SkillSpec) -> String {
    format!("Generated by {} skill install", spec.marker_slug)
}

fn text_hash(text: &str) -> String {
    let mut hash = FNV1A64_OFFSET;
    for byte in text.as_bytes() {
        hash ^= u64::from(*byte);
        hash = hash.wrapping_mul(FNV1A64_PRIME);
    }
    format!("{hash:016x}")
}

fn source_hash(spec: &SkillSpec) -> String {
    normalized_content_hash(spec, spec.source)
}

fn normalized_content_hash(spec: &SkillSpec, text: &str) -> String {
    text_hash(&normalize_skill_text(spec, text))
}

fn managed_marker_block(spec: &SkillSpec) -> String {
    let slug = spec.marker_slug;
    format!(
        "<!--\n{}\n{}-managed-skill: true\n{}-managed-skill-name: {}\n{}-managed-skill-owner: {}\n{}-managed-skill-content-hash-fnv1a64: {}\n-->",
        generated_by(spec),
        slug,
        slug,
        spec.name,
        slug,
        slug,
        slug,
        source_hash(spec)
    )
}

fn managed_skill_contents(spec: &SkillSpec) -> String {
    let block = managed_marker_block(spec);
    let mut lines = spec.source.lines();
    let mut output = String::new();
    let mut inserted = false;
    if let Some(first) = lines.next() {
        output.push_str(first);
        output.push('\n');
    }
    for line in lines {
        output.push_str(line);
        output.push('\n');
        if !inserted && line.trim() == "---" {
            output.push_str(&block);
            output.push_str("\n\n");
            inserted = true;
        }
    }
    if !inserted {
        output.push_str(&block);
        output.push('\n');
    }
    output
}

fn validate_installed_skill(spec: &SkillSpec, path: &Path) -> Result<(), SkillError> {
    let text =
        std::fs::read_to_string(path).map_err(|e| SkillError::io("read installed skill", e))?;
    // The source was validated before writing, so a rejection here means the
    // file on disk is not what was written.
    validate_skill_text(spec, &text, SkillErrorKind::Io)
}

fn validate_skill_text(
    spec: &SkillSpec,
    text: &str,
    kind: SkillErrorKind,
) -> Result<(), SkillError> {
    crate::skill::validate_skill_named(text, spec.name).map_err(|err| {
        SkillError::new(
            kind,
            format!("invalid {} skill front matter: {err}", spec.title),
            Some(format!(
                "make SKILL.md metadata conform to the Agent Skills specification and set name to {}",
                spec.name
            )),
        )
    })?;
    Ok(())
}

fn is_managed_or_bundled_skill(spec: &SkillSpec, path: &Path) -> Result<bool, SkillError> {
    let Some(kind) = skill_path_file_type(path)? else {
        return Ok(false);
    };
    if kind.is_symlink() {
        return Err(SkillError::new(
            SkillErrorKind::Refused,
            format!("refusing to inspect symlinked skill at {}", path.display()),
            Some("pass --force to replace or remove the symlink itself".to_string()),
        ));
    }
    let text = std::fs::read_to_string(path).map_err(|e| SkillError::io("read skill", e))?;
    Ok(skill_text_is_managed_or_bundled(spec, &text))
}

fn skill_text_is_managed_or_bundled(spec: &SkillSpec, text: &str) -> bool {
    skill_text_has_managed_identity(spec, text)
        || normalize_skill_text(spec, text) == normalize_skill_text(spec, spec.source)
}

fn skill_text_has_managed_identity(spec: &SkillSpec, text: &str) -> bool {
    for block in html_comment_blocks(text) {
        if managed_marker_block_has_identity(spec, &block) {
            return true;
        }
    }
    false
}

fn managed_marker_block_has_identity(spec: &SkillSpec, block: &str) -> bool {
    let slug = spec.marker_slug;
    let generated_by_line = generated_by(spec);
    let managed_line = format!("{slug}-managed-skill: true");
    let name_line = format!("{slug}-managed-skill-name: {}", spec.name);
    let owner_line = format!("{slug}-managed-skill-owner: {slug}");
    let mut has_generated_by = false;
    let mut has_managed = false;
    let mut has_name = false;
    let mut has_owner = false;
    for line in block.replace("\r\n", "\n").lines() {
        let trimmed = line.trim();
        has_generated_by |= trimmed == generated_by_line;
        has_managed |= trimmed == managed_line;
        has_name |= trimmed == name_line;
        has_owner |= trimmed == owner_line;
    }
    has_generated_by && has_managed && has_name && has_owner
}

fn html_comment_blocks(text: &str) -> Vec<String> {
    let normalized = text.replace("\r\n", "\n");
    let mut blocks = Vec::new();
    let mut lines = normalized.lines();
    while let Some(line) = lines.next() {
        if line.trim() != "<!--" {
            continue;
        }
        let mut block = vec![line.to_string()];
        for next in lines.by_ref() {
            block.push(next.to_string());
            if next.trim() == "-->" {
                blocks.push(block.join("\n"));
                break;
            }
        }
    }
    blocks
}

fn strip_managed_marker_blocks(spec: &SkillSpec, text: &str) -> String {
    let normalized = text.replace("\r\n", "\n");
    let mut output = Vec::new();
    let mut lines = normalized.lines();
    while let Some(line) = lines.next() {
        if line.trim() != "<!--" {
            output.push(line.to_string());
            continue;
        }
        let mut block = vec![line.to_string()];
        let mut closed = false;
        for next in lines.by_ref() {
            block.push(next.to_string());
            if next.trim() == "-->" {
                closed = true;
                break;
            }
        }
        if closed {
            let block_text = block.join("\n");
            if managed_marker_block_has_identity(spec, &block_text) {
                continue;
            }
        }
        output.extend(block);
    }
    output.join("\n")
}

fn normalize_skill_text(spec: &SkillSpec, text: &str) -> String {
    let text = strip_managed_marker_blocks(spec, text);
    // Drop the managed-marker blocks, then collapse runs of blank lines to one so that the blank
    // line `managed_skill_contents` inserts after the marker block does not make a managed install
    // compare unequal to the bundled source.
    let mut out: Vec<&str> = Vec::new();
    for line in text.lines() {
        let trimmed = line.trim();
        if trimmed.is_empty() && out.last().is_some_and(|prev| prev.trim().is_empty()) {
            continue;
        }
        out.push(line);
    }
    out.join("\n").trim().to_string()
}

fn validate_spec(spec: &SkillSpec) -> Result<(), SkillError> {
    validate_slug("skill name", spec.name)?;
    validate_slug("marker slug", spec.marker_slug)?;
    validate_skill_text(spec, spec.source, SkillErrorKind::InvalidSpec)
}

fn validate_slug(field: &str, value: &str) -> Result<(), SkillError> {
    if slug_is_valid(value) {
        return Ok(());
    }
    Err(SkillError::new(
        SkillErrorKind::InvalidSpec,
        format!(
            "invalid {field} {value:?}: expected a lowercase slug matching [a-z0-9][a-z0-9-]*[a-z0-9]"
        ),
        Some("use lowercase ASCII letters, digits, and single hyphen-separated words".to_string()),
    ))
}

fn slug_is_valid(value: &str) -> bool {
    let bytes = value.as_bytes();
    if bytes.is_empty() {
        return false;
    }
    fn is_lower_alnum(byte: u8) -> bool {
        byte.is_ascii_lowercase() || byte.is_ascii_digit()
    }
    if !is_lower_alnum(bytes[0]) || !is_lower_alnum(bytes[bytes.len() - 1]) {
        return false;
    }
    bytes
        .iter()
        .all(|byte| is_lower_alnum(*byte) || *byte == b'-')
}

fn skill_path_file_type(path: &Path) -> Result<Option<std::fs::FileType>, SkillError> {
    match std::fs::symlink_metadata(path) {
        Ok(metadata) => Ok(Some(metadata.file_type())),
        Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None),
        Err(err) => Err(SkillError::io("inspect skill path", err)),
    }
}

/// Refuse to reach `path` through a symbolic link somewhere below `root`.
///
/// Everything a skill install writes goes to a fixed name under a skills root
/// the caller named, and the final file's own type is judged separately — a
/// symlink there is replaced, never followed. What nothing else judges is the
/// directories in between. `create_dir_all` walks straight through a link
/// without complaint, so a workspace that ships `.claude/skills/<name>` as a
/// link to another directory would silently redirect a fixed-name write, and
/// the matching uninstall would delete files wherever it pointed.
///
/// `--force` does not relax this. It permits replacing an unmanaged file
/// *inside* the root; it is not permission to write outside it.
///
/// `root` itself is deliberately not checked: the caller chose it, and a home
/// directory that is a link onto another volume is ordinary.
///
/// This is a check, so a link swapped in between it and the write that follows
/// is still possible. Closing that window needs directory-handle-relative
/// opens, which Rust's standard library does not expose; an attacker able to
/// win that race can already write inside the tree being installed into.
fn ensure_no_symlinked_dirs(root: &Path, path: &Path) -> Result<(), SkillError> {
    let Ok(relative) = path.strip_prefix(root) else {
        return Err(SkillError::new(
            SkillErrorKind::Refused,
            format!(
                "{} is not inside the skills directory {}",
                path.display(),
                root.display()
            ),
            Some("pass --skills-dir to install where you mean to".to_string()),
        ));
    };
    let mut current = root.to_path_buf();
    let mut components = relative.components().peekable();
    while let Some(component) = components.next() {
        current.push(component);
        // The last component is the file itself, and installing over it is a
        // rename that replaces a link rather than following it.
        if components.peek().is_none() {
            break;
        }
        match std::fs::symlink_metadata(&current) {
            Ok(metadata) if metadata.file_type().is_symlink() => {
                return Err(SkillError::new(
                    SkillErrorKind::Refused,
                    format!(
                        "refusing to reach {} through the symlinked directory {}",
                        path.display(),
                        current.display()
                    ),
                    Some(
                        "replace the link with a real directory, or pass --skills-dir".to_string(),
                    ),
                ));
            }
            Ok(_) => {}
            // Nothing exists below here yet, so nothing below here is a link.
            Err(err) if err.kind() == std::io::ErrorKind::NotFound => break,
            Err(err) => return Err(SkillError::io("inspect skill path", err)),
        }
    }
    Ok(())
}

/// The containment message for a target whose skill file or bundled assets
/// cannot be reached without following a link out of the skills directory, or
/// `Ok(())` when every one of them stays inside it.
fn containment_failure(spec: &SkillSpec, target: &SkillTarget) -> Result<(), String> {
    let mut paths = vec![target.skill_path.clone()];
    paths.extend(
        spec.assets
            .iter()
            .map(|asset| asset_target_path(&target.skill_dir, asset.path)),
    );
    for path in paths {
        if let Err(err) = ensure_no_symlinked_dirs(&target.skills_dir, &path) {
            return Err(err.message);
        }
    }
    Ok(())
}

fn write_skill_atomic(target: &SkillTarget, content: &str) -> Result<(), SkillError> {
    ensure_no_symlinked_dirs(&target.skills_dir, &target.skill_path)?;
    write_file_atomic(&target.skill_path, content, "install skill file")
}

/// Atomically install `content` at `path` through the crate's one atomic file
/// installation, so a reader never sees a partial file, a pre-existing symlink
/// is replaced rather than followed, and the new directory entry is synced
/// before this reports success.
///
/// An existing regular file keeps its own permissions; a file created here is
/// world-readable, which is what a skill an agent must load has to be.
fn write_file_atomic(path: &Path, content: &str, action: &str) -> Result<(), SkillError> {
    let preserved = match std::fs::symlink_metadata(path) {
        Ok(metadata) if metadata.file_type().is_file() => Some(metadata.permissions()),
        _ => None,
    };
    let unix_mode = if preserved.is_some() {
        None
    } else {
        Some(0o644)
    };
    crate::atomic_file::install(
        path,
        crate::atomic_file::AtomicInstall::replacing(content.as_bytes())
            .with_permissions(preserved)
            .with_unix_mode(unix_mode),
    )
    .map_err(|err| SkillError::write_failed(action, err))
}

/// Reject an asset path that is absolute or carries a `.`/`..`/empty segment, so
/// a bundled asset can only ever land inside the skill directory.
fn validate_asset_path(path: &str) -> Result<(), SkillError> {
    let bad = path.is_empty()
        || Path::new(path).is_absolute()
        || path
            .split(['/', '\\'])
            .any(|seg| seg.is_empty() || seg == "." || seg == "..");
    if bad {
        return Err(SkillError::new(
            SkillErrorKind::InvalidSpec,
            format!("invalid skill asset path: {path}"),
            Some(
                "asset paths must be relative to the skill directory with no `.`/`..` segments"
                    .to_string(),
            ),
        ));
    }
    Ok(())
}

/// Resolve an asset's `/`-separated relative path to an absolute path under
/// `skill_dir`, one segment at a time so it works on every platform.
fn asset_target_path(skill_dir: &Path, rel_path: &str) -> PathBuf {
    let mut out = skill_dir.to_path_buf();
    for seg in rel_path.split(['/', '\\']) {
        out.push(seg);
    }
    out
}

/// Write every bundled asset under the target skill directory, creating parent
/// directories as needed. Paths are assumed already validated by `install`.
fn install_target_assets(spec: &SkillSpec, target: &SkillTarget) -> Result<(), SkillError> {
    for asset in spec.assets {
        let dest = asset_target_path(&target.skill_dir, asset.path);
        // Re-checked here rather than only in the preflight: the asset tree is
        // several directories deep, and this is the last moment before the
        // write that could land outside the root.
        ensure_no_symlinked_dirs(&target.skills_dir, &dest)?;
        if let Some(parent) = dest.parent() {
            std::fs::create_dir_all(parent)
                .map_err(|e| SkillError::io("create skill asset dir", e))?;
        }
        write_file_atomic(&dest, asset.contents, "install skill asset")?;
    }
    Ok(())
}

/// True when every bundled asset is present on disk and byte-equal to the
/// bundle. A single-file skill (`assets == []`) is trivially current.
fn assets_current(spec: &SkillSpec, skill_dir: &Path) -> bool {
    spec.assets.iter().all(|asset| {
        let dest = asset_target_path(skill_dir, asset.path);
        std::fs::read_to_string(&dest)
            .map(|text| text == asset.contents)
            .unwrap_or(false)
    })
}

/// Remove the bundled asset files, then their now-empty parent directories,
/// deepest first.
///
/// The two halves are not the same promise, and used to be reported as if they
/// were. Every managed *file* must go: one that survives is a stale reference
/// the next `status` cannot see, because that reads `SKILL.md` — which is
/// already gone — and calls the target uninstalled. So a failed `remove_file`
/// is returned, not swallowed. A *directory* that refuses to go is the normal
/// case rather than a fault: `remove_dir` fails exactly when the user put
/// something of their own in it, and their file is not this tool's to delete.
/// That outcome is reported instead of raising.
fn remove_target_assets(spec: &SkillSpec, target: &SkillTarget) -> Result<Vec<String>, SkillError> {
    let mut removed = Vec::new();
    let mut dirs: Vec<PathBuf> = Vec::new();
    for asset in spec.assets {
        let dest = asset_target_path(&target.skill_dir, asset.path);
        ensure_no_symlinked_dirs(&target.skills_dir, &dest)?;
        match std::fs::remove_file(&dest) {
            Ok(()) => removed.push(asset.path.to_string()),
            // Already absent: the end state this asked for.
            Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
            Err(err) => {
                return Err(SkillError::io(
                    &format!("remove bundled skill asset {}", asset.path),
                    err,
                ));
            }
        }
        let mut dir = dest.parent().map(Path::to_path_buf);
        while let Some(current) = dir {
            if current == target.skill_dir || !current.starts_with(&target.skill_dir) {
                break;
            }
            if !dirs.contains(&current) {
                dirs.push(current.clone());
            }
            dir = current.parent().map(Path::to_path_buf);
        }
    }
    dirs.sort_by_key(|d| std::cmp::Reverse(d.components().count()));
    for dir in dirs {
        let _ = std::fs::remove_dir(&dir);
    }
    Ok(removed)
}

/// An environment variable naming a directory, ignored when empty or
/// relative: either would put a personal skill under the current directory.
fn env_dir(name: &str) -> Option<PathBuf> {
    std::env::var_os(name)
        .map(PathBuf::from)
        .filter(|path| path.is_absolute())
}

fn home_dir() -> Result<PathBuf, SkillError> {
    env_dir("HOME")
        .or_else(|| env_dir("USERPROFILE"))
        .ok_or_else(|| {
            SkillError::new(
                SkillErrorKind::Usage,
                "cannot determine home directory".to_string(),
                Some("pass --skills-dir explicitly".to_string()),
            )
        })
}

fn expand_tilde(input: &str) -> Result<PathBuf, SkillError> {
    if input == "~" {
        return home_dir();
    }
    if let Some(rest) = input.strip_prefix("~/") {
        return Ok(home_dir()?.join(rest));
    }
    Ok(PathBuf::from(input))
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::time::{SystemTime, UNIX_EPOCH};

    const SKILL_SOURCE: &str =
        "---\nname: agent-first-test\ndescription: test skill\n---\n\n# Body\n\nrules.\n";

    fn spec() -> SkillSpec<'static> {
        SkillSpec {
            name: "agent-first-test",
            source: SKILL_SOURCE,
            title: "Agent-First Test",
            marker_slug: "aftest",
            assets: &[],
        }
    }

    fn managed_skill_with_body(body: &str) -> String {
        format!(
            "---\nname: agent-first-test\ndescription: test skill\n---\n{}\n\n{body}",
            managed_marker_block(&spec())
        )
    }

    fn temp_skills_dir(name: &str) -> PathBuf {
        let suffix = SystemTime::now()
            .duration_since(UNIX_EPOCH)
            .map(|d| d.as_nanos())
            .unwrap_or(0);
        std::env::temp_dir().join(format!(
            "afdata_skill_{name}_{}_{}",
            std::process::id(),
            suffix
        ))
    }

    fn options(agent: SkillAgentSelection, dir: &Path, force: bool) -> SkillOptions {
        SkillOptions {
            agent,
            scope: SkillScope::Personal,
            skills_dir: Some(dir.to_string_lossy().to_string()),
            force,
        }
    }

    fn custom_target(agent: SkillAgent, dir: &Path) -> SkillTarget {
        let skill_dir = dir.join("agent-first-test");
        SkillTarget {
            agent,
            scope: SkillScope::Personal,
            skills_dir: dir.to_path_buf(),
            skill_path: skill_dir.join(SKILL_FILE_NAME),
            skill_dir,
        }
    }

    #[test]
    fn validates_bundled_frontmatter() {
        assert!(crate::skill::validate_skill_named(SKILL_SOURCE, "agent-first-test").is_ok());
    }

    #[test]
    fn rejects_unquoted_colon_space() {
        let bad = "---\nname: x\ndescription: broken: yaml\n---\n";
        assert!(crate::skill::validate_skill(bad).is_err());
    }

    fn install_status_uninstall_for(agent: SkillAgentSelection, expect: SkillAgent, tag: &str) {
        let dir = temp_skills_dir(tag);
        let opts = options(agent, &dir, false);
        let skill_path = dir.join("agent-first-test").join(SKILL_FILE_NAME);

        let installed = run_skill_admin(&spec(), SkillAction::Install, &opts);
        assert!(installed.is_ok());
        assert!(skill_path.is_file());
        let text = std::fs::read_to_string(&skill_path).unwrap_or_default();
        assert!(text.contains(&managed_marker_block(&spec())));
        assert!(text.contains("aftest-managed-skill-name: agent-first-test"));
        assert!(text.contains("aftest-managed-skill-owner: aftest"));
        assert!(text.contains("aftest-managed-skill-content-hash-fnv1a64:"));
        assert!(!text.contains("aftest-managed-skill-source-hash-fnv1a64:"));

        let status = run_skill_admin(&spec(), SkillAction::Status, &opts);
        assert!(status.is_ok());
        if let Ok(SkillReport::Status {
            installed_all,
            valid_all,
            current_all,
            targets,
            ..
        }) = status
        {
            assert!(installed_all);
            assert!(valid_all);
            assert!(current_all);
            assert_eq!(targets.first().map(|t| t.agent), Some(expect));
            assert_eq!(targets.first().map(|t| t.current), Some(true));
        }

        let removed = run_skill_admin(&spec(), SkillAction::Uninstall, &opts);
        assert!(removed.is_ok());
        assert!(!skill_path.exists());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn install_status_uninstall_codex() {
        install_status_uninstall_for(SkillAgentSelection::Codex, SkillAgent::Codex, "codex");
    }

    #[test]
    fn install_status_uninstall_claude_code() {
        install_status_uninstall_for(
            SkillAgentSelection::ClaudeCode,
            SkillAgent::ClaudeCode,
            "claude",
        );
    }

    #[test]
    fn install_status_uninstall_opencode() {
        install_status_uninstall_for(
            SkillAgentSelection::Opencode,
            SkillAgent::Opencode,
            "opencode",
        );
    }

    #[test]
    fn install_status_uninstall_hermes() {
        install_status_uninstall_for(SkillAgentSelection::Hermes, SkillAgent::Hermes, "hermes");
    }

    fn spec_with_assets() -> SkillSpec<'static> {
        const ASSETS: &[SkillAsset] = &[
            SkillAsset {
                path: "references/guide.md",
                contents: "# guide\n",
            },
            SkillAsset {
                path: "references/registry.json",
                contents: "{\"ok\":true}\n",
            },
        ];
        SkillSpec {
            name: "agent-first-test",
            source: SKILL_SOURCE,
            title: "Agent-First Test",
            marker_slug: "aftest",
            assets: ASSETS,
        }
    }

    #[test]
    fn install_writes_and_uninstall_removes_bundled_assets() {
        let dir = temp_skills_dir("assets");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        let skill_dir = dir.join("agent-first-test");
        let guide = skill_dir.join("references").join("guide.md");
        let registry = skill_dir.join("references").join("registry.json");

        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());
        assert_eq!(
            std::fs::read_to_string(&guide).unwrap_or_default(),
            "# guide\n"
        );
        assert_eq!(
            std::fs::read_to_string(&registry).unwrap_or_default(),
            "{\"ok\":true}\n"
        );

        // A SKILL.md-only install (a bundled asset missing) must report
        // not-current so a plain re-install repopulates it — the exact
        // regression this fixes.
        std::fs::remove_file(&guide).unwrap();
        match run_skill_admin(&spec_with_assets(), SkillAction::Status, &opts) {
            Ok(SkillReport::Status { current_all, .. }) => {
                assert!(
                    !current_all,
                    "a missing asset must make the skill not-current"
                );
            }
            other => panic!("unexpected status: {other:?}"),
        }
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());
        assert!(guide.is_file(), "re-install must restore the missing asset");

        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Uninstall, &opts).is_ok());
        assert!(!skill_dir.join(SKILL_FILE_NAME).exists());
        assert!(!guide.exists(), "uninstall must remove bundled assets");
        assert!(
            !skill_dir.join("references").exists(),
            "uninstall must remove now-empty asset directories"
        );
        assert!(
            !skill_dir.exists(),
            "uninstall must remove the skill directory"
        );
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn install_rejects_escaping_asset_path() {
        const ESCAPE: &[SkillAsset] = &[SkillAsset {
            path: "../evil.md",
            contents: "x",
        }];
        let dir = temp_skills_dir("assets-escape");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        let spec = SkillSpec {
            name: "agent-first-test",
            source: SKILL_SOURCE,
            title: "Agent-First Test",
            marker_slug: "aftest",
            assets: ESCAPE,
        };
        assert!(
            run_skill_admin(&spec, SkillAction::Install, &opts).is_err(),
            "an asset path escaping the skill dir must be rejected"
        );
        assert!(!dir.join("evil.md").exists());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn status_reports_stale_install_as_not_current() {
        let dir = temp_skills_dir("stale");
        let opts = options(SkillAgentSelection::Opencode, &dir, false);
        let skill_dir = dir.join("agent-first-test");
        let skill_path = skill_dir.join(SKILL_FILE_NAME);
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        // A managed marker but stale body: valid + managed, but not current.
        let stale = managed_skill_with_body("# Body\n\nOLD rules.\n");
        assert!(std::fs::write(&skill_path, stale).is_ok());

        let status = run_skill_admin(&spec(), SkillAction::Status, &opts);
        if let Ok(SkillReport::Status {
            current_all,
            targets,
            ..
        }) = status
        {
            assert!(!current_all);
            if let Some(t) = targets.first() {
                assert!(t.installed);
                assert!(t.valid);
                assert!(t.managed);
                assert!(!t.current);
            }
        }

        // Reinstall makes it current again.
        assert!(run_skill_admin(&spec(), SkillAction::Install, &opts).is_ok());
        let refreshed = std::fs::read_to_string(&skill_path).unwrap_or_default();
        assert!(refreshed.contains(&managed_marker_block(&spec())));
        assert!(!refreshed.contains("<!-- aftest-managed-skill: true -->"));
        if let Ok(SkillReport::Status { targets, .. }) =
            run_skill_admin(&spec(), SkillAction::Status, &opts)
        {
            assert_eq!(targets.first().map(|t| t.current), Some(true));
        }
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn random_text_with_marker_words_is_not_managed() {
        let dir = temp_skills_dir("marker-words");
        let opts = options(SkillAgentSelection::Opencode, &dir, false);
        let skill_dir = dir.join("agent-first-test");
        let skill_path = skill_dir.join(SKILL_FILE_NAME);
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        let random = format!(
            "---\nname: agent-first-test\ndescription: test skill\n---\n\nThis mentions {} and {} but is not a generated block.\n",
            generated_by(&spec()),
            "aftest-managed-skill: true"
        );
        assert!(std::fs::write(&skill_path, random).is_ok());

        if let Ok(SkillReport::Status { targets, .. }) =
            run_skill_admin(&spec(), SkillAction::Status, &opts)
        {
            assert_eq!(targets.first().map(|t| t.managed), Some(false));
        }
        assert!(run_skill_admin(&spec(), SkillAction::Install, &opts).is_err());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn old_marker_format_is_not_managed() {
        let dir = temp_skills_dir("old-marker");
        let opts = options(SkillAgentSelection::Opencode, &dir, false);
        let skill_dir = dir.join("agent-first-test");
        let skill_path = skill_dir.join(SKILL_FILE_NAME);
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        let old_marker = concat!(
            "---\n",
            "name: agent-first-test\n",
            "description: test skill\n",
            "---\n",
            "<!--\n",
            "Generated by aftest skill install\n",
            "aftest-managed-skill: true\n",
            "aftest-managed-skill-name: agent-first-test\n",
            "aftest-managed-skill-source-hash-fnv1a64: deadbeef\n",
            "-->\n",
            "\n",
            "# Body\n",
            "\n",
            "rules.\n"
        );
        assert!(std::fs::write(&skill_path, old_marker).is_ok());

        if let Ok(SkillReport::Status { targets, .. }) =
            run_skill_admin(&spec(), SkillAction::Status, &opts)
            && let Some(t) = targets.first()
        {
            assert!(t.installed);
            assert!(t.valid);
            assert!(!t.managed);
            assert!(!t.current);
        }
        assert!(run_skill_admin(&spec(), SkillAction::Install, &opts).is_err());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn install_preflight_reports_all_targets_without_writing() {
        let dir = temp_skills_dir("install-preflight");
        let codex = custom_target(SkillAgent::Codex, &dir.join("codex"));
        let opencode = custom_target(SkillAgent::Opencode, &dir.join("opencode"));
        assert!(std::fs::create_dir_all(&opencode.skill_dir).is_ok());
        assert!(
            std::fs::write(
                &opencode.skill_path,
                "---\nname: custom\ndescription: custom\n---\n"
            )
            .is_ok()
        );
        let opts = SkillOptions {
            agent: SkillAgentSelection::All,
            scope: SkillScope::Personal,
            skills_dir: None,
            force: false,
        };

        let result = preflight_install_targets(&spec(), &opts, &[codex, opencode]);
        assert!(result.is_err());
        let Err(err) = result else {
            return;
        };
        assert!(
            err.message
                .contains("refusing to overwrite unmanaged skill")
        );
        assert!(!dir.join("codex").join("agent-first-test").exists());
        let partial_report = err.partial_report.map(|report| *report);
        assert!(matches!(partial_report, Some(SkillReport::Install { .. })));
        let Some(SkillReport::Install {
            installed, targets, ..
        }) = partial_report
        else {
            return;
        };
        assert!(!installed);
        assert_eq!(targets.len(), 2);
        assert_eq!(targets.first().map(|target| target.installed), Some(false));
        assert_eq!(targets.get(1).map(|target| target.installed), Some(true));
        assert_eq!(targets.get(1).map(|target| target.managed), Some(false));
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn uninstall_preflight_reports_all_targets_without_removing() {
        let dir = temp_skills_dir("uninstall-preflight");
        let codex = custom_target(SkillAgent::Codex, &dir.join("codex"));
        let opencode = custom_target(SkillAgent::Opencode, &dir.join("opencode"));
        assert!(std::fs::create_dir_all(&codex.skill_dir).is_ok());
        assert!(std::fs::create_dir_all(&opencode.skill_dir).is_ok());
        assert!(std::fs::write(&codex.skill_path, managed_skill_contents(&spec())).is_ok());
        assert!(
            std::fs::write(
                &opencode.skill_path,
                "---\nname: custom\ndescription: custom\n---\n"
            )
            .is_ok()
        );
        let opts = SkillOptions {
            agent: SkillAgentSelection::All,
            scope: SkillScope::Personal,
            skills_dir: None,
            force: false,
        };

        let result = preflight_uninstall_targets(&spec(), &opts, &[codex, opencode]);
        assert!(result.is_err());
        let Err(err) = result else {
            return;
        };
        assert!(err.message.contains("refusing to remove unmanaged skill"));
        assert!(
            dir.join("codex")
                .join("agent-first-test")
                .join(SKILL_FILE_NAME)
                .exists()
        );
        let partial_report = err.partial_report.map(|report| *report);
        assert!(matches!(
            partial_report,
            Some(SkillReport::Uninstall { .. })
        ));
        let Some(SkillReport::Uninstall {
            removed_any,
            targets,
            ..
        }) = partial_report
        else {
            return;
        };
        assert!(!removed_any);
        assert_eq!(targets.len(), 2);
        assert!(targets.iter().all(|target| !target.removed));
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn install_and_uninstall_refuse_unmanaged() {
        let dir = temp_skills_dir("unmanaged");
        let skill_dir = dir.join("agent-first-test");
        let skill_path = skill_dir.join(SKILL_FILE_NAME);
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        assert!(
            std::fs::write(&skill_path, "---\nname: custom\ndescription: custom\n---\n").is_ok()
        );
        let opts = options(SkillAgentSelection::Codex, &dir, false);

        assert!(run_skill_admin(&spec(), SkillAction::Install, &opts).is_err());
        assert!(run_skill_admin(&spec(), SkillAction::Uninstall, &opts).is_err());
        assert!(skill_path.exists());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn invalid_spec_slugs_are_rejected_before_path_resolution() {
        for name in ["", "../x", "x/y", ".hidden", "bad_name", "Bad"] {
            let bad = SkillSpec {
                name,
                source: SKILL_SOURCE,
                title: "Bad",
                marker_slug: "aftest",
                assets: &[],
            };
            let opts = options(SkillAgentSelection::Codex, Path::new("/tmp/afdata"), false);
            assert!(
                run_skill_admin(&bad, SkillAction::Status, &opts).is_err(),
                "{name:?}"
            );
        }

        let bad_marker = SkillSpec {
            name: "agent-first-test",
            source: SKILL_SOURCE,
            title: "Bad",
            marker_slug: "../aftest",
            assets: &[],
        };
        let opts = options(SkillAgentSelection::Codex, Path::new("/tmp/afdata"), false);
        assert!(run_skill_admin(&bad_marker, SkillAction::Status, &opts).is_err());
    }

    #[test]
    fn frontmatter_name_must_match_spec_name() {
        let bad = SkillSpec {
            name: "agent-first-test",
            source: "---\nname: other-skill\ndescription: test skill\n---\n",
            title: "Bad",
            marker_slug: "aftest",
            assets: &[],
        };
        let dir = temp_skills_dir("frontmatter-name");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        assert!(run_skill_admin(&bad, SkillAction::Install, &opts).is_err());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[cfg(unix)]
    #[test]
    fn symlink_target_is_rejected_by_default_and_force_does_not_follow() {
        use std::os::unix::fs::symlink;

        let dir = temp_skills_dir("symlink-install");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        let force_opts = options(SkillAgentSelection::Codex, &dir, true);
        let skill_dir = dir.join("agent-first-test");
        let skill_path = skill_dir.join(SKILL_FILE_NAME);
        let external = dir.join("external.md");
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        assert!(std::fs::write(&external, "external").is_ok());
        assert!(symlink(&external, &skill_path).is_ok());

        assert!(run_skill_admin(&spec(), SkillAction::Install, &opts).is_err());
        assert_eq!(
            std::fs::read_to_string(&external).unwrap_or_default(),
            "external"
        );
        assert!(run_skill_admin(&spec(), SkillAction::Uninstall, &opts).is_err());
        assert!(skill_path.is_symlink());

        assert!(run_skill_admin(&spec(), SkillAction::Install, &force_opts).is_ok());
        assert_eq!(
            std::fs::read_to_string(&external).unwrap_or_default(),
            "external"
        );
        assert!(skill_path.is_file());
        assert!(!skill_path.is_symlink());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[cfg(unix)]
    #[test]
    fn a_symlinked_skill_directory_never_receives_a_write() {
        use std::os::unix::fs::symlink;

        let dir = temp_skills_dir("symlink-skill-dir");
        let outside = temp_skills_dir("symlink-skill-dir-outside");
        assert!(std::fs::create_dir_all(&dir).is_ok());
        assert!(std::fs::create_dir_all(&outside).is_ok());
        let sentinel = outside.join(SKILL_FILE_NAME);
        assert!(std::fs::write(&sentinel, "sentinel").is_ok());
        // The shape a hostile checkout ships: the skill's own directory is a
        // link somewhere else, and every path below it is a fixed name.
        assert!(symlink(&outside, dir.join("agent-first-test")).is_ok());

        for force in [false, true] {
            let opts = options(SkillAgentSelection::Codex, &dir, force);
            let err = run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts)
                .expect_err("install through a symlinked skill directory must fail");
            assert!(
                err.message.contains("symlinked directory"),
                "unexpected message: {}",
                err.message
            );
            assert_eq!(
                err.hint.as_deref(),
                Some("--force does not permit writing outside the skills directory"),
                "--force must not read as permission to escape the root"
            );
            assert!(
                run_skill_admin(&spec_with_assets(), SkillAction::Uninstall, &opts).is_err(),
                "uninstall must refuse the same path"
            );
        }

        assert_eq!(
            std::fs::read_to_string(&sentinel).unwrap_or_default(),
            "sentinel",
            "the file outside the skills directory must be untouched"
        );
        assert!(!outside.join("references").exists());
        let _ = std::fs::remove_dir_all(dir);
        let _ = std::fs::remove_dir_all(outside);
    }

    #[cfg(unix)]
    #[test]
    fn a_symlinked_asset_directory_never_receives_a_write() {
        use std::os::unix::fs::symlink;

        let dir = temp_skills_dir("symlink-asset-dir");
        let outside = temp_skills_dir("symlink-asset-dir-outside");
        let skill_dir = dir.join("agent-first-test");
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        assert!(std::fs::create_dir_all(&outside).is_ok());
        let sentinel = outside.join("guide.md");
        assert!(std::fs::write(&sentinel, "sentinel").is_ok());
        // One level deeper than the skill directory: the bundled asset tree's
        // own parent is the link.
        assert!(symlink(&outside, skill_dir.join("references")).is_ok());

        let opts = options(SkillAgentSelection::Codex, &dir, true);
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_err());

        assert_eq!(
            std::fs::read_to_string(&sentinel).unwrap_or_default(),
            "sentinel"
        );
        assert!(!skill_dir.join(SKILL_FILE_NAME).exists());
        let _ = std::fs::remove_dir_all(dir);
        let _ = std::fs::remove_dir_all(outside);
    }

    #[cfg(unix)]
    #[test]
    fn status_names_a_symlinked_skill_directory_instead_of_reading_through_it() {
        use std::os::unix::fs::symlink;

        let dir = temp_skills_dir("symlink-status");
        let outside = temp_skills_dir("symlink-status-outside");
        assert!(std::fs::create_dir_all(&dir).is_ok());
        assert!(std::fs::create_dir_all(&outside).is_ok());
        assert!(std::fs::write(outside.join(SKILL_FILE_NAME), "sentinel").is_ok());
        assert!(symlink(&outside, dir.join("agent-first-test")).is_ok());

        let opts = options(SkillAgentSelection::Codex, &dir, false);
        let report = run_skill_admin(&spec(), SkillAction::Status, &opts)
            .expect("status must report every target rather than fail outright");
        let SkillReport::Status { targets, .. } = report else {
            panic!("expected a status report");
        };
        let target = targets.first().expect("one target");
        assert!(!target.installed, "nothing may be read through the link");
        assert!(
            target
                .validation_error
                .as_deref()
                .unwrap_or_default()
                .contains("symlinked directory")
        );
        let _ = std::fs::remove_dir_all(dir);
        let _ = std::fs::remove_dir_all(outside);
    }

    #[test]
    fn uninstall_reports_the_assets_it_removed_and_a_directory_the_user_kept() {
        let dir = temp_skills_dir("uninstall-report");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());
        // A file this tool did not install, in the directory it is about to
        // try to remove.
        let mine = dir.join("agent-first-test").join("notes.md");
        assert!(std::fs::write(&mine, "mine").is_ok());

        let report = run_skill_admin(&spec_with_assets(), SkillAction::Uninstall, &opts)
            .expect("uninstall must succeed");
        let SkillReport::Uninstall { targets, .. } = report else {
            panic!("expected an uninstall report");
        };
        let target = targets.first().expect("one target");
        assert!(target.removed);
        assert_eq!(
            target.assets_removed,
            vec![
                "references/guide.md".to_string(),
                "references/registry.json".to_string()
            ]
        );
        assert!(
            target.directory_retained,
            "a directory holding the user's own file survives, and the report says so"
        );
        assert_eq!(std::fs::read_to_string(&mine).unwrap_or_default(), "mine");
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn install_leaves_a_users_own_reference_alone_without_force() {
        let dir = temp_skills_dir("unmanaged-asset");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        let skill_dir = dir.join("agent-first-test");
        let guide = skill_dir.join("references").join("guide.md");
        std::fs::create_dir_all(guide.parent().unwrap()).unwrap();
        std::fs::write(&guide, "my notes\n").unwrap();

        let err = run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts)
            .expect_err("a file the user put there is not this tool's to replace");
        assert!(
            err.message.contains("unmanaged skill asset"),
            "{}",
            err.message
        );
        assert_eq!(std::fs::read_to_string(&guide).unwrap(), "my notes\n");
        assert!(!skill_dir.join(SKILL_FILE_NAME).exists());

        // Content identical to the bundle is already what install would write.
        std::fs::write(&guide, "# guide\n").unwrap();
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());

        // Once a managed SKILL.md owns the directory, a changed asset is a
        // stale copy to refresh, not the user's file.
        std::fs::write(&guide, "older bundle\n").unwrap();
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());
        assert_eq!(std::fs::read_to_string(&guide).unwrap(), "# guide\n");

        std::fs::remove_file(skill_dir.join(SKILL_FILE_NAME)).unwrap();
        std::fs::write(&guide, "my notes\n").unwrap();
        let forced = options(SkillAgentSelection::Codex, &dir, true);
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &forced).is_ok());
        assert_eq!(std::fs::read_to_string(&guide).unwrap(), "# guide\n");
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn install_refuses_a_directory_at_an_asset_path_before_writing_anything() {
        let dir = temp_skills_dir("asset-is-dir");
        let opts = options(SkillAgentSelection::Codex, &dir, true);
        let skill_dir = dir.join("agent-first-test");
        std::fs::create_dir_all(skill_dir.join("references").join("guide.md")).unwrap();

        let err = run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts)
            .expect_err("a directory cannot be replaced by an asset file");
        assert!(
            err.message.contains("non-regular skill asset"),
            "{}",
            err.message
        );
        assert!(!skill_dir.join(SKILL_FILE_NAME).exists());
        let _ = std::fs::remove_dir_all(dir);
    }

    /// `SKILL.md` is the marker uninstall goes by, so it has to be the last
    /// thing removed: a run that fails on an asset must leave a target the
    /// next run still recognises and can finish.
    #[cfg(unix)]
    #[test]
    fn an_uninstall_that_fails_partway_can_be_run_again() {
        use std::os::unix::fs::PermissionsExt;
        let dir = temp_skills_dir("uninstall-retry");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        let skill_dir = dir.join("agent-first-test");
        let references = skill_dir.join("references");
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());

        std::fs::set_permissions(&references, std::fs::Permissions::from_mode(0o555)).unwrap();
        let failed = run_skill_admin(&spec_with_assets(), SkillAction::Uninstall, &opts);
        std::fs::set_permissions(&references, std::fs::Permissions::from_mode(0o755)).unwrap();
        assert!(failed.is_err());
        assert!(skill_dir.join(SKILL_FILE_NAME).is_file());

        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Uninstall, &opts).is_ok());
        assert!(!references.join("guide.md").exists());
        assert!(!skill_dir.exists());
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn uninstall_reports_a_bundled_asset_it_could_not_remove() {
        let dir = temp_skills_dir("uninstall-stuck-asset");
        let opts = options(SkillAgentSelection::Codex, &dir, false);
        assert!(run_skill_admin(&spec_with_assets(), SkillAction::Install, &opts).is_ok());
        // A directory where a managed file belongs: `remove_file` cannot take
        // it, and the old code reported the target removed anyway.
        let asset = dir
            .join("agent-first-test")
            .join("references")
            .join("guide.md");
        assert!(std::fs::remove_file(&asset).is_ok());
        assert!(std::fs::create_dir(&asset).is_ok());
        assert!(std::fs::write(asset.join("kept.md"), "kept").is_ok());

        let err = run_skill_admin(&spec_with_assets(), SkillAction::Uninstall, &opts)
            .expect_err("a managed asset that cannot be removed is not a clean uninstall");
        // The spec spells a bundled asset with `/` because that is one fixed
        // name for every platform; the message names the file the operator has
        // to go and look at, which is spelled the local way.
        let named = Path::new("references").join("guide.md");
        assert!(
            err.message.contains(&named.display().to_string()),
            "unexpected message: {}",
            err.message
        );
        assert!(
            asset.join("kept.md").exists(),
            "the user's file must survive the refusal"
        );
        let _ = std::fs::remove_dir_all(dir);
    }

    #[cfg(unix)]
    #[test]
    fn force_uninstall_removes_symlink_without_following() {
        use std::os::unix::fs::symlink;

        let dir = temp_skills_dir("symlink-uninstall");
        let force_opts = options(SkillAgentSelection::Codex, &dir, true);
        let skill_dir = dir.join("agent-first-test");
        let skill_path = skill_dir.join(SKILL_FILE_NAME);
        let external = dir.join("external.md");
        assert!(std::fs::create_dir_all(&skill_dir).is_ok());
        assert!(std::fs::write(&external, "external").is_ok());
        assert!(symlink(&external, &skill_path).is_ok());

        assert!(run_skill_admin(&spec(), SkillAction::Uninstall, &force_opts).is_ok());
        assert!(!skill_path.exists());
        assert_eq!(
            std::fs::read_to_string(&external).unwrap_or_default(),
            "external"
        );
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn serializes_to_protocol_shape() {
        let dir = temp_skills_dir("serialize");
        let opts = options(SkillAgentSelection::Opencode, &dir, false);
        if let Ok(report) = run_skill_admin(&spec(), SkillAction::Install, &opts) {
            let value = serde_json::to_value(&report).unwrap_or(serde_json::Value::Null);
            assert_eq!(value["code"], "skill_install");
            assert_eq!(value["installed"], true);
            assert_eq!(value["targets"][0]["agent"], "opencode");
            assert_eq!(value["targets"][0]["current"], true);
            assert_eq!(
                value["targets"][0]["skill_dir"],
                serde_json::json!(dir.join("agent-first-test").to_string_lossy().to_string())
            );
        }
        let _ = std::fs::remove_dir_all(dir);
    }

    #[test]
    fn all_personal_resolves_four_targets() {
        let opts = SkillOptions {
            agent: SkillAgentSelection::All,
            scope: SkillScope::Personal,
            skills_dir: None,
            force: false,
        };
        let targets = resolve_targets(&spec(), &opts);
        assert!(targets.is_ok());
        if let Ok(targets) = targets {
            assert_eq!(targets.len(), 4);
            assert_eq!(targets[0].agent, SkillAgent::Codex);
            assert_eq!(targets[1].agent, SkillAgent::ClaudeCode);
            assert_eq!(targets[2].agent, SkillAgent::Opencode);
            assert_eq!(targets[3].agent, SkillAgent::Hermes);
        }
    }

    #[test]
    fn all_workspace_resolves_four_targets() {
        let opts = SkillOptions {
            agent: SkillAgentSelection::All,
            scope: SkillScope::Workspace,
            skills_dir: None,
            force: false,
        };
        let targets = resolve_targets(&spec(), &opts);
        assert!(targets.is_ok());
        if let Ok(targets) = targets {
            assert_eq!(targets.len(), 4);
            assert_eq!(targets[0].agent, SkillAgent::Codex);
            assert_eq!(targets[0].scope, SkillScope::Workspace);
            assert_eq!(targets[1].agent, SkillAgent::ClaudeCode);
            assert_eq!(targets[1].scope, SkillScope::Workspace);
            assert_eq!(targets[2].agent, SkillAgent::Opencode);
            assert_eq!(targets[2].scope, SkillScope::Workspace);
            assert_eq!(targets[3].agent, SkillAgent::Hermes);
            assert_eq!(targets[3].scope, SkillScope::Workspace);
        }
    }

    #[test]
    fn codex_workspace_scope_uses_codex_skills_dir() {
        let opts = SkillOptions {
            agent: SkillAgentSelection::Codex,
            scope: SkillScope::Workspace,
            skills_dir: None,
            force: false,
        };
        let targets = resolve_targets(&spec(), &opts);
        assert!(targets.is_ok());
        if let Ok(targets) = targets {
            assert_eq!(targets.len(), 1);
            assert_eq!(targets[0].agent, SkillAgent::Codex);
            assert_eq!(targets[0].scope, SkillScope::Workspace);
            assert!(targets[0].skills_dir.ends_with(".codex/skills"));
        }
    }

    #[test]
    fn hermes_workspace_scope_uses_hermes_skills_dir() {
        let opts = SkillOptions {
            agent: SkillAgentSelection::Hermes,
            scope: SkillScope::Workspace,
            skills_dir: None,
            force: false,
        };
        let targets = resolve_targets(&spec(), &opts);
        assert!(targets.is_ok());
        if let Ok(targets) = targets {
            assert_eq!(targets.len(), 1);
            assert_eq!(targets[0].agent, SkillAgent::Hermes);
            assert_eq!(targets[0].scope, SkillScope::Workspace);
            assert!(targets[0].skills_dir.ends_with(".hermes/skills"));
        }
    }
}