mur_common/skill/

loader.rs

//! Single-pass skill loader: lists global + per-agent skills,
//! resolves trust level, checks drift, returns one flat Vec.

use crate::skill::types::TrustLevel;
use crate::skill::{SkillManifest, content_hash_for_trust, local};
use crate::trust::skills::SkillTrustStore;
use std::path::Path;

/// Validate that a skill name contains only safe identifier characters.
///
/// Skill names are interpolated into XML-like `<skill-instruction source="…">`
/// attributes.  Restricting the character set at load time means injection is
/// blocked at the source rather than relying solely on escaping at emit time.
pub fn is_valid_skill_name(name: &str) -> bool {
    !name.is_empty()
        && name.len() <= 64
        // Reserved path components: a skill name is joined into
        // `<mur_home>/skills/<name>`, so `.`/`..` must never be accepted.
        && name != "."
        && name != ".."
        // The character set already excludes `/` and `\`, which keeps a name to
        // a single path component (no traversal into sibling/parent dirs).
        && name
            .chars()
            .all(|c| c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | '-'))
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum SkillScope {
    Global,
    Agent,
}

/// Outcome of resolving a `profile.yaml` skill ref (e.g. `skills/<name>`)
/// against an agent's home directory.
///
/// Distinguishing `Missing` from `Malformed` matters: a ref written without
/// installing the backing files (issue #717) is a *missing* skill — telling
/// the user it "no longer parses" points them at the wrong root cause.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum SkillRefStatus {
    /// The ref resolves to a manifest that parses and validates.
    Loadable,
    /// No file exists at the resolved manifest path.
    Missing { path: std::path::PathBuf },
    /// A file exists but does not parse/validate as a skill manifest.
    Malformed {
        path: std::path::PathBuf,
        error: String,
    },
    /// The ref itself cannot name a file — it holds whitespace, which a skill
    /// id never does. Seen in the wild as several refs concatenated into one
    /// `profile.yaml` list item (`skills/a - skills/b - skills/b`), growing by
    /// a segment each time something appended to the string instead of pushing
    /// a new item.
    ///
    /// Separate from `Missing` for the same reason `Missing` is separate from
    /// `Malformed`: the advice differs. Nothing is missing here — the backing
    /// skills are installed — so "install it" sends people to run a command
    /// that cannot help. The entry has to be split or removed.
    CorruptRef { reason: String },
}

/// Resolve a `profile.yaml` skill ref to its backing manifest file and report
/// whether it is loadable, missing, or malformed.
///
/// Resolution mirrors the runtime loader's layout rules: modern refs point at
/// a *directory* (`skills/<name>`) holding `skill.yaml`; legacy refs may point
/// directly at a `.yaml`/`.yml`/`.md` file. This is the single source of truth
/// for ref resolution — the Hub loadability badge and the creation-time
/// validation in mur-core both call it.
pub fn skill_ref_status(agent_home: &Path, rel_ref: &str) -> SkillRefStatus {
    let joined = agent_home.join(rel_ref);
    let ext = joined
        .extension()
        .and_then(|e| e.to_str())
        .unwrap_or("")
        .to_ascii_lowercase();
    let file = if joined.is_dir() || !matches!(ext.as_str(), "yaml" | "yml" | "md" | "markdown") {
        // Modern directory layout: the ref names the skill dir; the manifest
        // lives inside it. Also used when the dir is absent, so the Missing
        // path names the exact manifest we expected to find.
        joined.join("skill.yaml")
    } else {
        joined
    };
    if !file.is_file() {
        // Only reclassify once resolution has already failed, and only when the
        // ref holds whitespace. Anything that resolves today keeps resolving —
        // this can never turn a working ref into an error.
        if rel_ref.split_whitespace().count() > 1 {
            return SkillRefStatus::CorruptRef {
                reason: "a skill ref cannot contain whitespace; this entry looks like several \
                         refs concatenated — split it into separate list items, or remove it"
                    .to_string(),
            };
        }
        return SkillRefStatus::Missing { path: file };
    }
    let text = match std::fs::read_to_string(&file) {
        Ok(t) => t,
        Err(e) => {
            return SkillRefStatus::Malformed {
                path: file,
                error: format!("unreadable: {e}"),
            };
        }
    };
    let ext = file
        .extension()
        .and_then(|e| e.to_str())
        .unwrap_or("")
        .to_ascii_lowercase();
    let parsed = match ext.as_str() {
        "yaml" | "yml" => crate::skill::parse_canonical(&text),
        "md" | "markdown" => crate::skill::parse_markdown(&text)
            .or_else(|_| crate::skill::parse_legacy_markdown(&text)),
        other => {
            return SkillRefStatus::Malformed {
                path: file,
                error: format!("unsupported manifest extension '.{other}'"),
            };
        }
    };
    match parsed {
        Ok(m) => match crate::skill::validate(&m) {
            Ok(()) => SkillRefStatus::Loadable,
            Err(e) => SkillRefStatus::Malformed {
                path: file,
                error: format!("invalid manifest: {e}"),
            },
        },
        Err(e) => SkillRefStatus::Malformed {
            path: file,
            error: format!("parse failed: {e}"),
        },
    }
}

#[derive(Debug, Clone)]
pub struct LoadedSkill {
    pub name: String,
    pub manifest: SkillManifest,
    pub trust: TrustLevel,
    pub scope: SkillScope,
    pub content_hash: String,
    /// Absolute install directory of this skill (holds skill.yaml + any bundle).
    pub dir: std::path::PathBuf,
}

pub fn load_all(mur_home: &Path, agent_name: &str) -> Vec<LoadedSkill> {
    let trust = load_trust_migrated(mur_home);
    let mut out: Vec<LoadedSkill> = Vec::new();
    let mut seen_names: std::collections::HashSet<String> = Default::default();

    // Per-agent first (wins on name collision)
    if let Ok(names) = local::list_installed_agent(mur_home, agent_name) {
        for name in names {
            // Skip non-skill dirs (e.g. a fleet run-ledger `fleet:<name>/`
            // written under skills/ by the DAG executor's record_run — it holds
            // events.jsonl, not skill.yaml). Without this, its colon name trips
            // is_valid_skill_name in load_one and spams a warning every load.
            if !crate::skill::store::agent_skill_dir(mur_home, agent_name)
                .join(&name)
                .join("skill.yaml")
                .is_file()
            {
                continue;
            }
            if let Some(mut loaded) =
                load_one(mur_home, &name, SkillScope::Agent, &trust, |m, n| {
                    local::load_installed_agent(m, agent_name, n)
                })
            {
                loaded.dir = crate::skill::store::agent_skill_dir(mur_home, agent_name).join(&name);
                seen_names.insert(loaded.name.clone());
                out.push(loaded);
            }
        }
    }
    // Federated knowledge cache next (daemon-assembled snapshot of global
    // skills at or above the lifecycle floor; federation P0). A cache entry
    // wins over a same-named global — it IS that global skill, scope-filtered
    // — but never over a per-agent install.
    let cache_dir = mur_home
        .join("agents")
        .join(agent_name)
        .join("knowledge_cache");
    if let Ok(entries) = std::fs::read_dir(&cache_dir) {
        let mut names: Vec<String> = entries
            .filter_map(|e| e.ok())
            .filter(|e| e.path().join("skill.yaml").is_file())
            .filter_map(|e| e.file_name().to_str().map(String::from))
            .collect();
        names.sort(); // deterministic load order
        for name in names {
            if seen_names.contains(&name) {
                continue;
            }
            let dir = cache_dir.join(&name);
            let dir_for_loader = dir.clone();
            if let Some(mut loaded) = load_one(
                mur_home,
                &name,
                SkillScope::Global,
                &trust,
                move |_m, _n| crate::skill::read_from_dir(&dir_for_loader),
            ) {
                loaded.dir = dir;
                seen_names.insert(loaded.name.clone());
                out.push(loaded);
            }
        }
    }

    if let Ok(names) = local::list_installed(mur_home) {
        for name in names {
            if seen_names.contains(&name) {
                continue;
            }
            // Skip non-skill dirs (see the agent loop above) — a manifest-less
            // dir is a ledger/data dir, not a skill.
            if !crate::skill::store::global_skill_dir(mur_home, &name)
                .join("skill.yaml")
                .is_file()
            {
                continue;
            }
            if let Some(mut loaded) = load_one(
                mur_home,
                &name,
                SkillScope::Global,
                &trust,
                local::load_installed,
            ) {
                loaded.dir = crate::skill::store::global_skill_dir(mur_home, &name);
                out.push(loaded);
            }
        }
    }
    out
}

/// Load the trust store, re-keying it into the trust-hash domain on first use.
///
/// The migration runs here rather than in a separate command because this is
/// the one path every agent start goes through, so a store never stays stale
/// long enough for the loss to be noticed. It is a no-op once the schema is
/// current, and it is fail-soft in both directions: if the re-key cannot be
/// saved the in-memory store is still correct for this run, and if a skill is
/// missing from disk its entry is kept untouched.
fn load_trust_migrated(mur_home: &Path) -> SkillTrustStore {
    let mut trust = SkillTrustStore::load(mur_home).unwrap_or_default();
    let Some(rekeyed) =
        trust.migrate_to_trust_hash(|name| local::load_installed(mur_home, name).ok())
    else {
        return trust; // already current — the common case, no write
    };
    {
        match trust.save(mur_home) {
            Ok(()) => tracing::info!(
                rekeyed,
                "skill trust store migrated to the trust-hash domain"
            ),
            // Not fatal: the in-memory store is already correct, so this run
            // resolves trust properly and the next start retries the write.
            Err(e) => tracing::warn!(error = %e, "could not persist trust-store migration"),
        }
    }
    trust
}

fn load_one<F>(
    mur_home: &Path,
    name: &str,
    scope: SkillScope,
    trust: &SkillTrustStore,
    loader: F,
) -> Option<LoadedSkill>
where
    F: FnOnce(&Path, &str) -> Result<SkillManifest, crate::skill::StoreError>,
{
    // Validate name before loading: only safe identifier characters allowed.
    // Skill names are interpolated into XML attributes; an unvalidated name
    // containing `"` or `>` could break the attribute boundary even after
    // escaping if the validator itself is bypassed.
    if !is_valid_skill_name(name) {
        tracing::warn!(
            skill = %name,
            "skill name contains invalid characters (expected [A-Za-z0-9_.-]{{1,64}}); skipping"
        );
        return None;
    }

    let manifest = match loader(mur_home, name) {
        Ok(m) => m,
        Err(e) => {
            tracing::warn!(skill = %name, error = %e, "skill load failed; skipping");
            return None;
        }
    };
    // `content_hash_for_trust`, not `content_sha256`: this is the trust-store
    // key, and the trust hash excludes `transfer_chain` / `evolution_log` so a
    // transfer or a generation increment does not silently re-key an already
    // trusted skill. Using the plain content hash here is what made every
    // transfer- and fleet-import-installed skill (which key by the trust hash)
    // miss its entry and load as Sandboxed regardless of its recorded level.
    let hash = match content_hash_for_trust(&manifest) {
        Ok(h) => h,
        Err(e) => {
            tracing::warn!(skill = %name, error = %e, "skill hash failed; skipping");
            return None;
        }
    };
    // No separate drift check: the entry is KEYED by the content hash, so
    // finding one is already proof the content matches the pinned bytes. The
    // check that used to sit here compared `content_sha256(&manifest)` against
    // a hash derived from the same manifest — a value against itself, which
    // could never report drift. Worse, once the key became the trust hash the
    // two would differ by construction and every skill would refuse to load.
    let entry = trust.entries.get(&hash);
    if let Some(pinned) = entry {
        if trust.is_revoked(&hash) {
            tracing::warn!(skill = %name, "skill hash revoked; skipping");
            return None;
        }
        Some(LoadedSkill {
            name: name.into(),
            manifest,
            trust: pinned.level,
            scope,
            content_hash: hash,
            dir: std::path::PathBuf::new(), // overwritten by load_all
        })
    } else {
        // Unpinned = first-load Sandboxed.
        Some(LoadedSkill {
            name: name.into(),
            manifest,
            trust: TrustLevel::Sandboxed,
            scope,
            content_hash: hash,
            dir: std::path::PathBuf::new(), // overwritten by load_all
        })
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::skill::{parse_canonical, write_to_dir};
    use tempfile::tempdir;

    /// The bug this domain unification fixes, stated as a test.
    ///
    /// A skill whose `evolution_log` grows — which happens during ordinary use —
    /// keeps the same `content_hash_for_trust` but gets a NEW `content_sha256`.
    /// The loader used to key on the latter, so the trust entry written at
    /// install stopped matching and the skill silently dropped to `Sandboxed`.
    #[test]
    fn a_generation_increment_does_not_lose_the_recorded_trust_level() {
        use crate::trust::skills::{SkillTrustStore, TrustEntry};
        let dir = tempdir().unwrap();
        let home = dir.path();
        let mut m = make("evolving");
        let sdir = home
            .join("agents")
            .join("a1")
            .join("skills")
            .join("evolving");
        write_to_dir(&sdir, &m).unwrap();

        // Trust recorded at install time, keyed by the trust hash.
        let key = crate::skill::content_hash_for_trust(&m).unwrap();
        let mut trust = SkillTrustStore::default();
        trust.insert(
            key.clone(),
            TrustEntry {
                name: "evolving".into(),
                version: m.version.clone(),
                level: TrustLevel::Trusted,
                installed_at: "2026-08-19T00:00:00Z".into(),
                ..Default::default()
            },
        );
        trust.save(home).unwrap();

        // The skill evolves in place: plain content hash moves, trust hash does not.
        let before_plain = crate::skill::content_sha256(&m).unwrap();
        m.evolution_log
            .push(crate::skill::evolution::EvolutionEvent::initial_human(
                "t", "1.0.0",
            ));
        write_to_dir(&sdir, &m).unwrap();
        let after_plain = crate::skill::content_sha256(&m).unwrap();
        assert_ne!(
            before_plain, after_plain,
            "precondition: an evolution entry must move the plain content hash"
        );
        assert_eq!(
            key,
            crate::skill::content_hash_for_trust(&m).unwrap(),
            "precondition: the trust hash must be stable across a generation increment"
        );

        let loaded = load_all(home, "a1");
        let s = loaded.iter().find(|s| s.name == "evolving").unwrap();
        assert_eq!(
            s.trust,
            TrustLevel::Trusted,
            "the recorded trust level was lost when the skill evolved"
        );
    }

    /// A v1 store (keys from `content_sha256`) is re-keyed on first load, so an
    /// existing install keeps its trust level across the upgrade instead of
    /// silently reverting to Sandboxed.
    #[test]
    fn a_legacy_store_is_migrated_on_load() {
        use crate::trust::skills::SkillTrustStore;
        let dir = tempdir().unwrap();
        let home = dir.path();
        // The two domains differ only once a skill carries an evolution log or a
        // transfer chain — for a pristine skill they are the same hash, which is
        // why this migration touches far fewer entries than it might appear to.
        let mut m = make("legacy");
        m.evolution_log
            .push(crate::skill::evolution::EvolutionEvent::initial_human(
                "t", "1.0.0",
            ));
        write_to_dir(&home.join("skills").join("legacy"), &m).unwrap();

        // v1: keyed by the plain content hash, and no schema field on disk.
        let legacy_key = crate::skill::content_sha256(&m).unwrap();
        let trust_key = crate::skill::content_hash_for_trust(&m).unwrap();
        assert_ne!(
            legacy_key, trust_key,
            "precondition: the two domains must differ for this to be a migration"
        );
        let json = format!(
            r#"{{"entries":{{"{legacy_key}":{{"name":"legacy","version":"{}","level":"trusted","installed_at":"2026-08-19T00:00:00Z"}}}},"revoked":[]}}"#,
            m.version
        );
        std::fs::create_dir_all(home.join("trust")).unwrap();
        std::fs::write(SkillTrustStore::path(home), json).unwrap();

        let loaded = load_all(home, "a1");
        let s = loaded.iter().find(|s| s.name == "legacy").unwrap();
        assert_eq!(
            s.trust,
            TrustLevel::Trusted,
            "a v1 entry must survive the domain change"
        );

        // ...and the migration is persisted, so it runs once.
        let reloaded = SkillTrustStore::load(home).unwrap();
        assert_eq!(reloaded.schema, crate::trust::skills::TRUST_STORE_SCHEMA);
        assert!(reloaded.entries.contains_key(&trust_key));
        assert!(!reloaded.entries.contains_key(&legacy_key));
    }

    #[test]
    fn load_all_sets_agent_skill_dir() {
        let dir = tempdir().unwrap();
        let home = dir.path();
        let sdir = home.join("agents").join("a1").join("skills").join("demo");
        write_to_dir(&sdir, &make("demo")).unwrap();

        let loaded = load_all(home, "a1");
        let demo = loaded.iter().find(|s| s.name == "demo").unwrap();
        assert_eq!(demo.dir, sdir);
    }

    fn make(name: &str) -> SkillManifest {
        make_desc(name, "test")
    }

    fn make_desc(name: &str, desc: &str) -> SkillManifest {
        parse_canonical(&format!(
            r#"name: {name}
version: 1.0.0
publisher: human:t
description: {desc}
category: context
content:
  abstract: hi
  context: body
"#
        ))
        .unwrap()
    }

    #[test]
    fn knowledge_cache_skill_loads() {
        let dir = tempdir().unwrap();
        let home = dir.path();
        let cdir = home
            .join("agents/a1/knowledge_cache")
            .join("federated-skill");
        write_to_dir(&cdir, &make("federated-skill")).unwrap();

        let loaded = load_all(home, "a1");
        let hit = loaded
            .iter()
            .find(|s| s.name == "federated-skill")
            .expect("cached skill must be visible to the loader");
        assert_eq!(hit.dir, cdir);
    }

    #[test]
    fn agent_local_wins_over_cache_wins_over_global() {
        let dir = tempdir().unwrap();
        let home = dir.path();
        write_to_dir(
            &home.join("agents/a1/skills/dup"),
            &make_desc("dup", "agent-local"),
        )
        .unwrap();
        write_to_dir(
            &home.join("agents/a1/knowledge_cache/dup"),
            &make_desc("dup", "cache"),
        )
        .unwrap();
        write_to_dir(&home.join("skills/dup"), &make_desc("dup", "global")).unwrap();

        let loaded = load_all(home, "a1");
        let dups: Vec<_> = loaded.iter().filter(|s| s.name == "dup").collect();
        assert_eq!(dups.len(), 1, "name collision must resolve to ONE copy");
        assert_eq!(dups[0].manifest.description, "agent-local");

        // Remove the per-agent copy: the cache copy takes over, not the global.
        std::fs::remove_dir_all(home.join("agents/a1/skills/dup")).unwrap();
        let loaded = load_all(home, "a1");
        let dup = loaded.iter().find(|s| s.name == "dup").unwrap();
        assert_eq!(dup.manifest.description, "cache");
    }

    #[test]
    fn empty_mur_home_returns_empty() {
        let dir = tempdir().unwrap();
        let loaded = load_all(dir.path(), "alice");
        assert!(loaded.is_empty());
    }

    #[test]
    fn load_all_skips_non_skill_dirs() {
        let dir = tempdir().unwrap();
        let home = dir.path();
        // A real global skill (has skill.yaml)…
        write_to_dir(&home.join("skills").join("real"), &make("real")).unwrap();
        // …and a non-skill dir under skills/ (only events.jsonl, no skill.yaml) —
        // e.g. a fleet run-ledger. Uses a portable name here: the real ledger id
        // is `fleet:<name>`, but a colon is an illegal filename on Windows, so
        // the test fixture would fail to even create it. The skip logic keys on
        // the absent skill.yaml, not the name.
        let ledger = home.join("skills").join("not-a-skill");
        std::fs::create_dir_all(&ledger).unwrap();
        std::fs::write(ledger.join("events.jsonl"), "{}\n").unwrap();

        let loaded = load_all(home, "a1");
        let names: Vec<_> = loaded.iter().map(|s| s.name.as_str()).collect();
        assert_eq!(
            names,
            vec!["real"],
            "ledger dir must not be loaded as a skill"
        );
    }

    #[test]
    fn is_valid_skill_name_rejects_traversal_and_reserved() {
        // Legit names.
        assert!(is_valid_skill_name("web-search"));
        assert!(is_valid_skill_name("my.skill_v2"));
        // Reserved path components.
        assert!(!is_valid_skill_name("."));
        assert!(!is_valid_skill_name(".."));
        // Path separators (the dangerous traversal form) and absolutes.
        assert!(!is_valid_skill_name("../agents/victim/skills/evil"));
        assert!(!is_valid_skill_name("a/b"));
        assert!(!is_valid_skill_name("a\\b"));
        assert!(!is_valid_skill_name("/etc/passwd"));
        // Bounds.
        assert!(!is_valid_skill_name(""));
        assert!(!is_valid_skill_name(&"x".repeat(65)));
    }

    #[test]
    fn global_skill_returns_sandboxed_when_no_trust_entry() {
        let dir = tempdir().unwrap();
        write_to_dir(&dir.path().join("skills").join("demo"), &make("demo")).unwrap();
        let loaded = load_all(dir.path(), "alice");
        assert_eq!(loaded.len(), 1);
        assert_eq!(loaded[0].name, "demo");
        assert_eq!(loaded[0].trust, TrustLevel::Sandboxed);
        assert_eq!(loaded[0].scope, SkillScope::Global);
    }

    #[test]
    fn agent_overrides_global_by_name() {
        let dir = tempdir().unwrap();
        // Both global and agent have "shared"
        write_to_dir(&dir.path().join("skills").join("shared"), &make("shared")).unwrap();
        write_to_dir(
            &dir.path()
                .join("agents")
                .join("alice")
                .join("skills")
                .join("shared"),
            &make("shared"),
        )
        .unwrap();
        let loaded = load_all(dir.path(), "alice");
        let shared: Vec<_> = loaded.iter().filter(|s| s.name == "shared").collect();
        assert_eq!(shared.len(), 1);
        assert_eq!(shared[0].scope, SkillScope::Agent);
    }

    // ── skill_ref_status (#717): missing vs malformed ────────────────────

    #[test]
    fn skill_ref_status_loadable_for_installed_dir_skill() {
        let home = tempdir().unwrap();
        write_to_dir(&home.path().join("skills").join("demo"), &make("demo")).unwrap();
        assert_eq!(
            skill_ref_status(home.path(), "skills/demo"),
            SkillRefStatus::Loadable
        );
    }

    #[test]
    fn skill_ref_status_absent_ref_is_missing_with_manifest_path() {
        let home = tempdir().unwrap();
        match skill_ref_status(home.path(), "skills/executing-plans") {
            SkillRefStatus::Missing { path } => {
                // The reported path names the exact manifest we expected.
                assert!(path.ends_with("skills/executing-plans/skill.yaml"));
            }
            other => panic!("expected Missing, got {other:?}"),
        }
    }

    /// Observed on a live concierge: one `profile.yaml` item holding ten refs
    /// run together, growing by a segment every time something appended to the
    /// string instead of pushing a new item. Reported as `missing`, which sent
    /// the user to install skills that were already installed.
    #[test]
    fn skill_ref_status_concatenated_refs_are_corrupt_not_missing() {
        let home = tempdir().unwrap();
        let ref_ = "skills/pm-spec-handoff - skills/brainstorming - skills/brainstorming";
        match skill_ref_status(home.path(), ref_) {
            SkillRefStatus::CorruptRef { reason } => {
                assert!(reason.contains("whitespace"), "unhelpful reason: {reason}");
            }
            other => panic!("expected CorruptRef, got {other:?}"),
        }
    }

    /// The regression that matters most: a plain uninstalled ref must keep
    /// reporting `Missing`, because there "install it" is the right advice.
    #[test]
    fn skill_ref_status_plain_absent_ref_stays_missing() {
        let home = tempdir().unwrap();
        assert!(matches!(
            skill_ref_status(home.path(), "skills/never-installed"),
            SkillRefStatus::Missing { .. }
        ));
    }

    #[test]
    fn skill_ref_status_garbage_yaml_is_malformed() {
        let home = tempdir().unwrap();
        let sdir = home.path().join("skills").join("broken");
        std::fs::create_dir_all(&sdir).unwrap();
        std::fs::write(sdir.join("skill.yaml"), "{{{ not: [valid").unwrap();
        assert!(matches!(
            skill_ref_status(home.path(), "skills/broken"),
            SkillRefStatus::Malformed { .. }
        ));
    }

    #[test]
    fn skill_ref_status_legacy_md_file_resolves_directly() {
        let home = tempdir().unwrap();
        let sdir = home.path().join("skills");
        std::fs::create_dir_all(&sdir).unwrap();
        // Absent legacy .md ref → Missing at the file itself (no /skill.yaml).
        match skill_ref_status(home.path(), "skills/old.md") {
            SkillRefStatus::Missing { path } => assert!(path.ends_with("skills/old.md")),
            other => panic!("expected Missing, got {other:?}"),
        }
        // Present but unparseable legacy .md → Malformed.
        std::fs::write(sdir.join("old.md"), "no frontmatter here").unwrap();
        assert!(matches!(
            skill_ref_status(home.path(), "skills/old.md"),
            SkillRefStatus::Malformed { .. }
        ));
    }
}