mind-cli 0.47.0

//! Parsing and resolving item refs against a catalog.

use crate::catalog::CatalogItem;
use crate::error::{ItemKind, MindError, Result};
use crate::manifest::InstalledItem;

/// A parsed item ref, before it is matched against a catalog.
#[derive(Debug, Clone)]
pub struct ItemRef {
    /// `None` means "any kind".
    pub kind: Option<ItemKind>,
    pub name: String,
    /// `Some(source_name)` for the `owner/repo#name` form (uses repo as source name).
    pub source: Option<String>,
}

/// A parsed hook target: either a source selector or an item ref.
///
/// The rule is simple: if the raw string contains `#`, it is an item ref
/// (`<source>#<item>`); otherwise it is a source selector. Source selectors may
/// carry glob metacharacters (`*`, `?`, `[..]`) to match multiple sources.
// spec: CLI-194
#[derive(Debug)]
pub enum HookTarget {
    /// A source selector (no `#` in the raw target string). May be a glob.
    Source(String),
    /// An item ref `<source>#<item>` (contains `#`). Parsed via
    /// [`parse_item_ref`]; the source part acts as a filter and may itself be a
    /// glob.
    Item(ItemRef),
}

/// Parse a hook target string, distinguishing a source selector (no `#`) from
/// an `<source>#<item>` item ref (contains `#`). Used by `mind hooks run` and
/// `mind hooks list` to decide what the target addresses.
///
/// This is the `#`-split heuristic only; it does not know about the registry.
/// An item-link instance's own source identity carries a `#<path>` suffix
/// (LNK-4), so its exact identity string would otherwise parse here as an item
/// ref matching nothing. The caller (`hooks_cmd::resolve_hook_target`) checks
/// for an exact match against a registered source identity BEFORE falling
/// back to this function, so that case resolves as a source target (HOOK-105).
/// This function stays a pure string-shape parser.
// spec: CLI-194
pub fn parse_hook_target(target: &str) -> Result<HookTarget> {
    let target = target.trim();
    if target.contains('#') {
        Ok(HookTarget::Item(parse_item_ref(target)?))
    } else {
        Ok(HookTarget::Source(target.to_string()))
    }
}

/// Parse one of: `name`, `skill:name`, `agent:name`, `rule:name`, `owner/repo#name`.
pub fn parse_item_ref(raw: &str) -> Result<ItemRef> {
    let raw = raw.trim();
    let invalid = || MindError::InvalidItemRef {
        name: raw.to_string(),
    };

    // Source-qualified: owner/repo#name (or repo#name). The selector is kept
    // verbatim and matched in `resolve` against the full name or the basename.
    // Split at the LAST '#': an item-link source's own identity carries a
    // '#<path>' suffix (LNK-4), so `<link-source>#<name>` has two.
    if let Some((repo_part, name_part)) = raw.rsplit_once('#') {
        let selector = repo_part.trim();
        if selector.is_empty() {
            return Err(invalid());
        }
        let (kind, name) = split_kind(name_part)?;
        return Ok(ItemRef {
            kind,
            name,
            source: Some(selector.to_string()),
        });
    }

    let (kind, name) = split_kind(raw)?;
    Ok(ItemRef {
        kind,
        name,
        source: None,
    })
}

fn split_kind(raw: &str) -> Result<(Option<ItemKind>, String)> {
    let invalid = || MindError::InvalidItemRef {
        name: raw.to_string(),
    };
    // A pre-colon token is read as a KIND only when it is a reserved kind word
    // (NS-26). Otherwise the WHOLE ref (colon and all) is the effective name with
    // kind=None, so a prefixed effective name like `jk:review` parses as a name and
    // resolves by effective-name match, while `skill:review` stays kind-qualified.
    //
    // spec: NS-74 -- a kind-qualified ref with no source selector is ALSO
    // read as the effective name `<kind>:<name>` by `resolve` and
    // `resolve_installed` (a legacy prefix equal to a kind word); see
    // `alternate_ref`.
    //
    // spec: DSC-112 -- the set of kind words grows (`workflow` joined it with
    // the workflow kind), and a word joining it silently re-reads every
    // existing `<word>:<name>` ref as kind-qualified. Prefixes are guarded at
    // their own ingress (`namespace::validate_prefix`, whose reserved list is
    // append-only and pre-reserves plausible future kind words), plus the
    // warning `catalog::scan_source_at` prints for a registered source whose
    // recorded prefix the list has since caught up with. Nothing is decided
    // here: this function reads what the vocabulary now says.
    if let Some((prefix, name)) = raw.split_once(':')
        && let Some(kind) = ItemKind::parse(prefix)
    {
        // Kind-qualified: the name after the kind word must be non-empty
        // (so `skill:` stays invalid).
        if name.is_empty() {
            return Err(invalid());
        }
        return Ok((Some(kind), name.to_string()));
    }
    if raw.is_empty() {
        return Err(invalid());
    }
    Ok((None, raw.to_string()))
}

/// Whether a source selector matches a full `host/owner/repo` source name. It
/// matches the full name or any trailing path suffix at a component boundary, so
/// `repo`, `owner/repo`, and `host/owner/repo` all select it. A selector resolves
/// uniquely only when one source carries that suffix.
pub fn source_matches(full_name: &str, selector: &str) -> bool {
    full_name == selector || full_name.ends_with(&format!("/{selector}"))
}

/// The full source identity plus each of its trailing-suffix forms at a component
/// boundary (e.g. `host/owner/repo`, `owner/repo`, `repo`). The candidates a
/// selector is matched against in both [`source_matches`] and
/// [`source_matches_glob`].
fn source_suffix_forms(full_name: &str) -> Vec<&str> {
    let mut forms = vec![full_name];
    let mut rest = full_name;
    while let Some(idx) = rest.find('/') {
        rest = &rest[idx + 1..];
        forms.push(rest);
    }
    forms
}

/// Whether a source selector matches a full `host/owner/repo` source name,
/// permitting a glob. When `selector` carries glob metacharacters (CLI-28,
/// CLI-86), it is compiled as a [`glob::Pattern`] and matched as a plain string
/// against the full identity or any trailing-suffix form, so `*` spans any run
/// including `/` (`*agents` matches `github.com/jaemk/agents`). Otherwise it
/// falls back to the exact/unambiguous-suffix semantics of [`source_matches`].
pub fn source_matches_glob(full_name: &str, selector: &str) -> bool {
    if is_glob(selector) {
        match glob::Pattern::new(selector) {
            Ok(pattern) => source_suffix_forms(full_name)
                .iter()
                .any(|form| pattern.matches(form)),
            Err(_) => false,
        }
    } else {
        source_matches(full_name, selector)
    }
}

/// Validate a source selector that may carry glob metacharacters (CLI-28). When
/// `selector` is a glob, compile it once and surface a malformed pattern as
/// [`MindError::InvalidPattern`] so a typo like `[bad` reports a clear
/// invalid-pattern error rather than silently matching nothing (which would
/// surface downstream as `SourceNotFound`). A non-glob selector is always valid.
pub fn validate_source_selector(selector: &str) -> Result<()> {
    if is_glob(selector) {
        glob::Pattern::new(selector).map_err(|source| MindError::InvalidPattern {
            pattern: selector.to_string(),
            source,
        })?;
    }
    Ok(())
}

/// Whether a ref name is a glob pattern (selects many) rather than an exact name.
pub fn is_glob(name: &str) -> bool {
    name.contains(['*', '?', '['])
}

/// Apply the `learn --all` flag (CLI-36): append the `#*` selector so the
/// positional ref is read as a source qualifier selecting every item of that
/// source. Errors `InvalidItemRef` when the ref already carries a `#` selector,
/// since the selector would be doubled.
pub fn all_selector(item: &str) -> Result<String> {
    let item = item.trim();
    if item.contains('#') {
        return Err(MindError::InvalidItemRef {
            name: item.to_string(),
        });
    }
    Ok(format!("{item}#*"))
}

/// Select every catalog item matching `r`: the name as a glob when it contains
/// glob metacharacters, else by exact effective name, with the kind and source
/// qualifier filtering as in [`resolve`]. Used for multi-item `learn`.
pub fn select<'a>(items: &'a [CatalogItem], r: &ItemRef) -> Vec<&'a CatalogItem> {
    let pattern = glob::Pattern::new(&r.name).ok();
    items
        .iter()
        .filter(|it| {
            r.kind.is_none_or(|k| it.kind == k)
                && r.source
                    .as_ref()
                    .is_none_or(|s| source_matches(&it.source, s))
                && match &pattern {
                    Some(p) => p.matches(&it.effective_name()),
                    None => it.effective_name() == r.name,
                }
        })
        .collect()
}

/// NS-74: the second reading of a kind-qualified ref with no source selector.
/// `workflow:review` is the workflow `review`, but it is also the effective name
/// `workflow:review` (a legacy prefix that equals a kind word, DSC-112). `None`
/// when the ref is not kind-qualified or carries a source selector.
fn alternate_ref(r: &ItemRef) -> Option<ItemRef> {
    let kind = r.kind?;
    if r.source.is_some() {
        return None;
    }
    Some(ItemRef {
        kind: None,
        name: format!("{}:{}", kind.as_str(), r.name),
        source: None,
    })
}

/// Find the single catalog item matching `r`, erroring on none or ambiguity.
pub fn resolve<'a>(
    items: &'a [CatalogItem],
    r: &ItemRef,
    sources: usize,
) -> Result<&'a CatalogItem> {
    let matching = |r: &ItemRef| -> Vec<&'a CatalogItem> {
        items
            .iter()
            .filter(|it| {
                r.kind.is_none_or(|k| it.kind == k)
                    && it.effective_name() == r.name
                    && r.source
                        .as_ref()
                        .is_none_or(|s| source_matches(&it.source, s))
            })
            .collect()
    };
    let mut matches = matching(r);
    if let Some(alt) = alternate_ref(r) {
        // spec: NS-74 -- both readings matching is ambiguous; only the
        // effective-name reading matching resolves to it.
        matches.extend(matching(&alt));
    }

    match matches.as_slice() {
        [] => Err(MindError::ItemNotFound {
            query: r.name.clone(),
            sources,
        }),
        [only] => Ok(only),
        many => Err(MindError::AmbiguousItem {
            query: r.name.clone(),
            // spec: DSC-95 -- `MindError::AmbiguousItem`'s `#[error(...)]`
            // Display joins `candidates` verbatim into the rejection message
            // with no sanitizing step of its own, so each entry (both the
            // source name and the item key, both source-controlled) must
            // already be display-safe here. Sanitize each field individually,
            // before composing, per `strip_ansi`'s own caveat: a dangling
            // escape in one field must not be able to swallow the next.
            candidates: many
                .iter()
                .map(|it| {
                    format!(
                        "{}#{}",
                        crate::sanitize::strip_ansi(&it.source),
                        it.key().display()
                    )
                })
                .collect(),
        }),
    }
}

/// Filter a catalog slice to the items named by a set of bare `kind:name` refs
/// (DSC-62). Each ref must carry an explicit kind prefix; a ref that parses
/// without a kind (bare name only) is not matched here. Items are matched by
/// their bare `name` (not effective/prefixed name) and kind.
///
/// Returns the matching subset in catalog order. Does NOT error on unknown refs:
/// the caller (`install_source_items_subset`) is responsible for DSC-63
/// validation before calling this.
pub fn select_by_bare_refs<'a>(
    items: &'a [CatalogItem],
    bare_refs: &[String],
) -> Vec<&'a CatalogItem> {
    // Parse each ref into (kind, bare_name); skip unparseable entries.
    let pairs: Vec<(crate::error::ItemKind, String)> = bare_refs
        .iter()
        .filter_map(|r| {
            let (kind, name) = split_kind(r).ok()?;
            kind.map(|k| (k, name))
        })
        .collect();

    items
        .iter()
        .filter(|it| pairs.iter().any(|(k, n)| it.kind == *k && it.name == *n))
        .collect()
}

/// Whether an installed item matches a parsed ref: its kind (when the ref names
/// one), its effective installed name, and the source qualifier (when given).
/// Used by `recall <item>` and `resolve_installed`, which require exact
/// effective-name matching against the manifest.
pub fn installed_matches(it: &InstalledItem, r: &ItemRef) -> bool {
    r.kind.is_none_or(|k| it.kind == k)
        && it.name == r.name
        && r.source
            .as_ref()
            .is_none_or(|s| source_matches(&it.source, s))
}

/// Like [`installed_matches`] but the name matches as a glob when it contains
/// glob metacharacters (`*`, `?`, `[`), else exactly. Used by `forget` and
/// `upgrade` for multi-item selection.
pub fn installed_matches_glob(it: &InstalledItem, r: &ItemRef) -> bool {
    r.kind.is_none_or(|k| it.kind == k)
        && r.source
            .as_ref()
            .is_none_or(|s| source_matches(&it.source, s))
        && if is_glob(&r.name) {
            glob::Pattern::new(&r.name).is_ok_and(|p| p.matches(&it.name))
        } else {
            it.name == r.name
        }
}

/// Select every installed item matching `r`: the name as a glob when it contains
/// glob metacharacters, else by exact effective name, honoring the kind and
/// source qualifier as in [`installed_matches`]. Used for multi-item `forget`
/// and `upgrade`.
pub fn select_installed<'a>(
    items: &'a std::collections::BTreeMap<String, InstalledItem>,
    r: &ItemRef,
) -> Vec<&'a InstalledItem> {
    items
        .values()
        .filter(|it| installed_matches_glob(it, r))
        .collect()
}

/// Find the installed items matching `r`. Errors `NotInstalled` on no match and
/// `AmbiguousItem` on more than one (e.g. a bare name shared across kinds).
pub fn resolve_installed<'a>(
    items: &'a std::collections::BTreeMap<String, InstalledItem>,
    r: &ItemRef,
) -> Result<&'a InstalledItem> {
    let mut matches: Vec<&InstalledItem> = items
        .values()
        .filter(|it| installed_matches(it, r))
        .collect();
    if let Some(alt) = alternate_ref(r) {
        // spec: NS-74 -- see `resolve`.
        matches.extend(items.values().filter(|it| installed_matches(it, &alt)));
    }
    match matches.as_slice() {
        [] => Err(MindError::NotInstalled {
            name: r.name.clone(),
        }),
        [only] => Ok(only),
        many => Err(MindError::AmbiguousItem {
            query: r.name.clone(),
            // spec: DSC-95 -- `MindError::AmbiguousItem`'s `#[error(...)]`
            // Display joins `candidates` verbatim into the rejection message
            // with no sanitizing step of its own, so each entry (both the
            // source name and the item key, both source-controlled) must
            // already be display-safe here. Sanitize each field individually,
            // before composing, per `strip_ansi`'s own caveat: a dangling
            // escape in one field must not be able to swallow the next.
            candidates: many
                .iter()
                .map(|it| {
                    format!(
                        "{}#{}",
                        crate::sanitize::strip_ansi(&it.source),
                        it.key().display()
                    )
                })
                .collect(),
        }),
    }
}

#[cfg(test)]
mod tests {
    // spec: CLI-1, CLI-2, CLI-3, CLI-4, CLI-5, CLI-31 (item ref parsing, resolution, selection)
    use super::*;
    use std::path::PathBuf;

    // ---- parse_hook_target ----

    // spec: CLI-194
    #[test]
    fn parse_hook_target_no_hash_is_source() {
        // A bare name, an owner/repo pair, and a full host/owner/repo are all
        // source selectors when they contain no `#`.
        let t = parse_hook_target("agents").unwrap();
        assert!(matches!(t, HookTarget::Source(s) if s == "agents"));

        let t = parse_hook_target("owner/repo").unwrap();
        assert!(matches!(t, HookTarget::Source(s) if s == "owner/repo"));

        let t = parse_hook_target("github.com/owner/repo").unwrap();
        assert!(matches!(t, HookTarget::Source(s) if s == "github.com/owner/repo"));
    }

    // spec: CLI-194
    #[test]
    fn parse_hook_target_glob_source() {
        // Glob metacharacters with no `#` remain a source selector.
        let t = parse_hook_target("*").unwrap();
        assert!(matches!(t, HookTarget::Source(s) if s == "*"));

        let t = parse_hook_target("owner/*").unwrap();
        assert!(matches!(t, HookTarget::Source(s) if s == "owner/*"));
    }

    // spec: CLI-194
    #[test]
    fn parse_hook_target_with_hash_is_item() {
        // Bare item ref: source="agents", name="scan", kind=None.
        let t = parse_hook_target("agents#scan").unwrap();
        match t {
            HookTarget::Item(r) => {
                assert_eq!(r.source.as_deref(), Some("agents"));
                assert_eq!(r.name, "scan");
                assert_eq!(r.kind, None);
            }
            HookTarget::Source(_) => panic!("expected Item"),
        }

        // Kind-qualified item ref.
        let t = parse_hook_target("owner/repo#skill:scan").unwrap();
        match t {
            HookTarget::Item(r) => {
                assert_eq!(r.source.as_deref(), Some("owner/repo"));
                assert_eq!(r.kind, Some(ItemKind::Skill));
                assert_eq!(r.name, "scan");
            }
            HookTarget::Source(_) => panic!("expected Item"),
        }

        // Glob item ref.
        let t = parse_hook_target("agents#*").unwrap();
        match t {
            HookTarget::Item(r) => {
                assert_eq!(r.source.as_deref(), Some("agents"));
                assert_eq!(r.name, "*");
            }
            HookTarget::Source(_) => panic!("expected Item"),
        }
    }

    // spec: CLI-194
    #[test]
    fn parse_hook_target_invalid_item_ref_errors() {
        // A hash with an empty repo part is invalid.
        assert!(parse_hook_target("#item").is_err());
        // A hash with a bad kind (empty name after kind) is invalid.
        assert!(parse_hook_target("repo#skill:").is_err());
    }

    fn cat(kind: ItemKind, name: &str, source: &str) -> CatalogItem {
        CatalogItem {
            kind,
            name: name.to_string(),
            source: source.to_string(),
            prefix: None,
            path: PathBuf::new(),
            description: None,
            when_to_use: None,
            link_rel: None,
            bin: None,
            build: None,
            requires: Vec::new(),
            expand: Vec::new(),
            hooks: Vec::new(),
            ignore: None,
        }
    }

    #[test]
    fn parses_bare_name_as_any_kind() {
        let r = parse_item_ref("review").unwrap();
        assert_eq!(r.kind, None);
        assert_eq!(r.name, "review");
        assert_eq!(r.source, None);
    }

    #[test]
    fn parses_kind_prefix() {
        let r = parse_item_ref("skill:review").unwrap();
        assert_eq!(r.kind, Some(ItemKind::Skill));
        assert_eq!(r.name, "review");
    }

    #[test]
    fn parses_source_qualified() {
        let r = parse_item_ref("james/agents#agent:dev").unwrap();
        assert_eq!(r.source.as_deref(), Some("james/agents"));
        assert_eq!(r.kind, Some(ItemKind::Agent));
        assert_eq!(r.name, "dev");
    }

    // spec: NS-26
    #[test]
    fn colon_token_is_kind_only_when_reserved_word() {
        // A non-kind pre-colon token: the WHOLE ref is the effective name, kind=None.
        // This lets a prefixed effective name like `jk:review` be used as a ref.
        let r = parse_item_ref("jk:review").unwrap();
        assert_eq!(r.kind, None);
        assert_eq!(r.name, "jk:review");
        assert_eq!(r.source, None);

        // A reserved kind word stays kind-qualified.
        let s = parse_item_ref("skill:review").unwrap();
        assert_eq!(s.kind, Some(ItemKind::Skill));
        assert_eq!(s.name, "review");

        // Resolve by effective name: an item whose effective name is `jk:review`
        // (built with prefix=None so this is independent of the separator constant)
        // is found by the kindless effective-name ref.
        let items = vec![cat(ItemKind::Skill, "jk:review", "agents")];
        let found = resolve(&items, &parse_item_ref("jk:review").unwrap(), 1).unwrap();
        assert_eq!(found.effective_name(), "jk:review");

        // Source-qualified, kindless: the post-`#` token still parses as a name.
        let q = parse_item_ref("owner/repo#jk:review").unwrap();
        assert_eq!(q.source.as_deref(), Some("owner/repo"));
        assert_eq!(q.kind, None);
        assert_eq!(q.name, "jk:review");
    }

    #[test]
    fn source_selector_matches_full_name_or_trailing_suffix() {
        let full = "github.com/james/agents";
        assert!(source_matches(full, "github.com/james/agents"));
        assert!(source_matches(full, "james/agents"));
        assert!(source_matches(full, "agents"));
        // Not a component-boundary suffix.
        assert!(!source_matches(full, "james"));
        assert!(!source_matches(full, "ts"));
        assert!(!source_matches(full, "bob/agents"));
    }

    // STO-64: `source_matches` treats `@` as an ordinary character in the
    // suffix it compares (it only splits on `/`), so an `<repo>@<alias>`-style
    // selector like `unmeld foo@bar` still matches by trailing-suffix, exactly
    // as any other selector does -- this function's own semantics are
    // unchanged by the CLI-204/STO-64 tightening. What DID change is at the
    // identity-construction layer (`source::validate_identity_part`): before
    // STO-64, `host/owner/foo@bar` could be produced two ways -- a repo
    // literally named `foo@bar` (unaliased), or repo `foo` melded `--as bar`
    // -- so the string a selector matches against was not reliably tied to
    // one meaning. `repo` can no longer contain `@`, so this suffix form is
    // now producible only via an identity alias, making a selector like
    // `foo@bar` an unambiguous reference to that aliased instance. Driven
    // through real `Source` construction (`parse_spec` + `apply_alias`), not
    // a hand-built string, so the identity is built exactly as production
    // builds it.
    // spec: STO-64
    #[test]
    fn at_suffixed_selector_matches_only_the_aliased_instance_and_is_unambiguous() {
        let mut aliased = crate::source::parse_spec("https://forge.example.com/owner/foo")
            .expect("plain repo must parse");
        aliased.apply_alias(Some("bar".into()));
        assert_eq!(aliased.name, "forge.example.com/owner/foo@bar");

        // The selector `foo@bar` (as `unmeld foo@bar` would parse it) matches
        // the aliased instance's full name by trailing-suffix, same as any
        // other bare-name selector.
        assert!(source_matches(&aliased.name, "foo@bar"));
        // A different alias does not match.
        let mut other = crate::source::parse_spec("https://forge.example.com/owner/foo").unwrap();
        other.apply_alias(Some("baz".into()));
        assert!(!source_matches(&other.name, "foo@bar"));

        // STO-64 pins that no OTHER construction path can produce this same
        // suffix: a repo literally named `foo@bar` is refused at parse time
        // (see `source::tests::at_and_hash_are_rejected_in_repo`), so
        // `foo@bar` can only ever resolve to the identity-aliased instance,
        // never collide with an unaliased repo of that literal name.
        assert!(matches!(
            crate::source::parse_spec("https://forge.example.com/owner/foo@bar"),
            Err(crate::error::MindError::UnsafeRepoSpec { .. })
        ));
    }

    // spec: CLI-28, CLI-86
    #[test]
    fn source_glob_matches_full_id_and_suffix_forms() {
        let full = "github.com/jaemk/agents";
        // `*` spans `/`, matching against the full identity as a plain string.
        assert!(source_matches_glob(full, "*agents"));
        assert!(source_matches_glob(full, "github.com/*/agents"));
        assert!(source_matches_glob(full, "*"));
        // Matched against a trailing-suffix form (`agents`).
        assert!(source_matches_glob(full, "ag*"));
        assert!(source_matches_glob(full, "jaemk/*"));
        // `?` and `[..]` metacharacters are honored.
        assert!(source_matches_glob(full, "agent?"));
        assert!(source_matches_glob(full, "[ab]gents"));
    }

    // spec: CLI-28, CLI-86
    #[test]
    fn source_glob_matching_nothing_is_false() {
        let full = "github.com/jaemk/agents";
        assert!(!source_matches_glob(full, "*foo"));
        assert!(!source_matches_glob(full, "skills*"));
    }

    // spec: CLI-28, CLI-86
    #[test]
    fn source_glob_non_glob_falls_back_to_exact_or_suffix() {
        let full = "github.com/jaemk/agents";
        // No glob metacharacters: exact/suffix semantics of `source_matches`.
        assert!(source_matches_glob(full, "agents"));
        assert!(source_matches_glob(full, "jaemk/agents"));
        assert!(source_matches_glob(full, "github.com/jaemk/agents"));
        // A non-component-boundary substring still does not match without a glob.
        assert!(!source_matches_glob(full, "jaemk"));
        assert!(!source_matches_glob(full, "ts"));
    }

    // spec: CLI-28, CLI-86
    #[test]
    fn validate_source_selector_accepts_valid_glob() {
        // A well-formed glob compiles, so validation passes.
        assert!(validate_source_selector("*agents").is_ok());
        assert!(validate_source_selector("github.com/*/agents").is_ok());
        assert!(validate_source_selector("[ab]gents").is_ok());
        assert!(validate_source_selector("agent?").is_ok());
    }

    // spec: CLI-28, CLI-86
    #[test]
    fn validate_source_selector_rejects_malformed_glob() {
        // `[bad` opens a character class that is never closed -- glob compilation
        // fails, and validation must surface InvalidPattern (not silently pass and
        // later read as SourceNotFound).
        let err = validate_source_selector("[bad").unwrap_err();
        assert!(
            matches!(err, MindError::InvalidPattern { ref pattern, .. } if pattern == "[bad"),
            "expected InvalidPattern carrying the offending pattern, got {err:?}"
        );
        // The user-facing message names the failure mode.
        assert!(
            err.to_string().contains("not a valid glob selector"),
            "message should explain the invalid glob: {err}"
        );
    }

    // spec: CLI-86
    #[test]
    fn validate_source_selector_passes_non_glob() {
        // No glob metacharacters: nothing to compile, always Ok even for a name
        // that would carry an unbalanced bracket meaning only as a literal.
        assert!(validate_source_selector("agents").is_ok());
        assert!(validate_source_selector("github.com/jaemk/agents").is_ok());
        assert!(validate_source_selector("").is_ok());
    }

    #[test]
    fn rejects_bad_refs() {
        for bad in ["", "skill:"] {
            assert!(parse_item_ref(bad).is_err(), "expected error for {bad:?}");
        }
    }

    #[test]
    fn resolves_unique_match() {
        let items = vec![cat(ItemKind::Skill, "review", "agents")];
        let r = parse_item_ref("review").unwrap();
        assert_eq!(resolve(&items, &r, 1).unwrap().name, "review");
    }

    #[test]
    fn errors_on_no_match() {
        let items = vec![cat(ItemKind::Skill, "review", "agents")];
        let r = parse_item_ref("nope").unwrap();
        assert!(matches!(
            resolve(&items, &r, 1),
            Err(MindError::ItemNotFound { .. })
        ));
    }

    #[test]
    fn errors_on_ambiguous_match() {
        let items = vec![
            cat(ItemKind::Skill, "review", "agents"),
            cat(ItemKind::Skill, "review", "other"),
        ];
        let r = parse_item_ref("review").unwrap();
        assert!(matches!(
            resolve(&items, &r, 2),
            Err(MindError::AmbiguousItem { .. })
        ));
    }

    /// DSC-95: `AmbiguousItem`'s `candidates` (built from `it.source` and
    /// `it.key()`, both source-controlled) must reach the caller already
    /// sanitized -- `MindError::AmbiguousItem`'s `#[error(...)]` Display has
    /// no sanitizing step of its own, so a raw ESC byte in either half of
    /// the composed `source#kind:name` candidate would otherwise ride
    /// straight to a terminal via `main.rs`'s `eprintln!("error: {err}")`.
    /// The candidate string is built from TWO independently sanitized
    /// fields (`strip_ansi(&it.source)` and `it.key().display()`), so an ESC
    /// is planted in BOTH the source AND the item name here: sanitizing only
    /// the source half (e.g. reverting `it.key().display()` back to
    /// `it.key().as_str()`) would still compile and would still pass a test
    /// that only puts the ESC in `source`.
    ///
    /// Asserts against the `candidates` field directly, not the full
    /// `Display` message: `query` echoes the ref the caller passed in
    /// verbatim (a CLI-typed string, not source-controlled catalog content)
    /// and is a separate, out-of-scope concern from the candidate list this
    /// test targets.
    // spec: DSC-95
    #[test]
    fn ambiguous_item_candidates_are_sanitized_for_display() {
        let items = vec![
            cat(ItemKind::Skill, "evil\x1b[32mname", "evil\x1b[31msource"),
            cat(ItemKind::Skill, "evil\x1b[32mname", "other"),
        ];
        let r = parse_item_ref("evil\x1b[32mname").unwrap();
        let err = resolve(&items, &r, 2).unwrap_err();
        match err {
            MindError::AmbiguousItem { candidates, .. } => {
                assert!(
                    candidates.iter().all(|c| !c.contains('\x1b')),
                    "candidates must not carry a raw ESC byte: {candidates:?}"
                );
                // The sanitized source half (only the first item's source
                // carries the ESC) and the sanitized item name half (every
                // candidate carries it, since both items share the name)
                // must each still be readable post-sanitization.
                assert!(
                    candidates.iter().any(|c| c.contains("evilsource")),
                    "must still name the sanitized source: {candidates:?}"
                );
                assert!(
                    candidates.iter().all(|c| c.contains("evilname")),
                    "must still name the sanitized item name in every \
                     candidate: {candidates:?}"
                );
            }
            other => panic!("expected AmbiguousItem, got {other:?}"),
        }
    }

    #[test]
    fn kind_prefix_disambiguates() {
        let items = vec![
            cat(ItemKind::Skill, "x", "a"),
            cat(ItemKind::Agent, "x", "a"),
        ];
        let r = parse_item_ref("agent:x").unwrap();
        assert_eq!(resolve(&items, &r, 1).unwrap().kind, ItemKind::Agent);
    }

    fn inst(kind: ItemKind, name: &str, source: &str) -> InstalledItem {
        InstalledItem {
            kind,
            name: name.to_string(),
            bare_name: name.to_string(),
            source: source.to_string(),
            commit: String::new(),
            hash: String::new(),
            store: String::new(),
            links: Vec::new(),
            description: None,
            install_hooks: Vec::new(),
            dropped_requires: Vec::new(),
            profiles: Vec::new(),
            explicit: false,
            version: None,
        }
    }

    fn manifest(items: Vec<InstalledItem>) -> std::collections::BTreeMap<String, InstalledItem> {
        items.into_iter().map(|it| (it.key().into(), it)).collect()
    }

    /// DSC-95: `resolve_installed`'s `AmbiguousItem` candidates must reach the
    /// caller already sanitized, mirroring `ambiguous_item_candidates_are_sanitized_for_display`
    /// for the catalog-side `resolve` above. As there, the ESC is planted in
    /// BOTH the source and the item name so both halves of the composed
    /// candidate string are exercised -- an ESC in `source` alone would not
    /// catch a reversion of the item-name half back to `as_str()`. Asserts
    /// against `candidates` directly (see the sibling test's doc for why
    /// `query` is out of scope here).
    // spec: DSC-95
    #[test]
    fn resolve_installed_ambiguous_item_candidates_are_sanitized_for_display() {
        let m = manifest(vec![
            inst(ItemKind::Skill, "evil\x1b[32mname", "evil\x1b[31msource"),
            inst(ItemKind::Agent, "evil\x1b[32mname", "other"),
        ]);
        let bare = parse_item_ref("evil\x1b[32mname").unwrap();
        let err = resolve_installed(&m, &bare).unwrap_err();
        match err {
            MindError::AmbiguousItem { candidates, .. } => {
                assert!(
                    candidates.iter().all(|c| !c.contains('\x1b')),
                    "candidates must not carry a raw ESC byte: {candidates:?}"
                );
                // As in the catalog-side sibling test: only the first item's
                // source carries the ESC, but both items share the ESC-laden
                // name, so the sanitized source half must appear in some
                // candidate and the sanitized name half in all of them.
                assert!(
                    candidates.iter().any(|c| c.contains("evilsource")),
                    "must still name the sanitized source: {candidates:?}"
                );
                assert!(
                    candidates.iter().all(|c| c.contains("evilname")),
                    "must still name the sanitized item name in every \
                     candidate: {candidates:?}"
                );
            }
            other => panic!("expected AmbiguousItem, got {other:?}"),
        }
    }

    /// NS-74: `workflow:review` is the workflow `review` AND the effective name
    /// `workflow:review` (a legacy prefix equal to a kind word). Both matching
    /// is ambiguous; only the alternate matching resolves to it; only the
    /// primary matching is unchanged.
    #[test]
    fn kind_qualified_ref_also_reads_as_an_effective_name_in_resolve() {
        // spec: NS-74
        let mut aliased = cat(ItemKind::Skill, "review", "h/o/a");
        aliased.prefix = Some("workflow".to_string());
        let wf = cat(ItemKind::Workflow, "review", "h/o/b");
        let r = parse_item_ref("workflow:review").unwrap();

        match resolve(&[aliased.clone(), wf.clone()], &r, 2) {
            Err(MindError::AmbiguousItem { candidates, .. }) => {
                assert_eq!(candidates.len(), 2, "{candidates:?}");
            }
            other => panic!("expected AmbiguousItem, got {other:?}"),
        }
        let only_alt = [aliased];
        assert_eq!(
            resolve(&only_alt, &r, 1).unwrap().kind,
            ItemKind::Skill,
            "only the effective-name reading matches"
        );
        let only_primary = [wf];
        assert_eq!(
            resolve(&only_primary, &r, 1).unwrap().kind,
            ItemKind::Workflow
        );
        // A source selector suppresses the alternate reading.
        let mut aliased2 = cat(ItemKind::Skill, "review", "h/o/a");
        aliased2.prefix = Some("workflow".to_string());
        let scoped = parse_item_ref("h/o/a#workflow:review").unwrap();
        assert!(matches!(
            resolve(&[aliased2], &scoped, 1),
            Err(MindError::ItemNotFound { .. })
        ));
    }

    #[test]
    fn kind_qualified_ref_also_reads_as_an_effective_name_in_resolve_installed() {
        // spec: NS-74
        let m = manifest(vec![
            inst(ItemKind::Skill, "workflow:review", "h/o/a"),
            inst(ItemKind::Workflow, "review", "h/o/b"),
        ]);
        let r = parse_item_ref("workflow:review").unwrap();
        assert!(matches!(
            resolve_installed(&m, &r),
            Err(MindError::AmbiguousItem { .. })
        ));
        let only_alt = manifest(vec![inst(ItemKind::Skill, "workflow:review", "h/o/a")]);
        assert_eq!(
            resolve_installed(&only_alt, &r).unwrap().kind,
            ItemKind::Skill
        );
        let only_primary = manifest(vec![inst(ItemKind::Workflow, "review", "h/o/b")]);
        assert_eq!(
            resolve_installed(&only_primary, &r).unwrap().kind,
            ItemKind::Workflow
        );
    }

    #[test]
    fn alternate_reading_unions_candidates_and_is_suppressed_by_a_source_selector() {
        // spec: NS-74
        let mut aliased = cat(ItemKind::Skill, "review", "h/o/a");
        aliased.prefix = Some("workflow".to_string());
        let wf = cat(ItemKind::Workflow, "review", "h/o/b");
        let r = parse_item_ref("workflow:review").unwrap();
        match resolve(&[aliased, wf], &r, 2) {
            Err(MindError::AmbiguousItem { candidates, .. }) => {
                assert!(
                    candidates.iter().any(|c| c.contains("workflow:review"))
                        && candidates.iter().any(|c| c == "h/o/b#workflow:review"),
                    "union must name the aliased skill and the workflow: {candidates:?}"
                );
            }
            other => panic!("expected AmbiguousItem, got {other:?}"),
        }
        // Two aliased items and no workflow: the alternate itself is ambiguous.
        let mut a1 = cat(ItemKind::Skill, "review", "h/o/a");
        a1.prefix = Some("workflow".to_string());
        let mut a2 = cat(ItemKind::Skill, "review", "h/o/c");
        a2.prefix = Some("workflow".to_string());
        assert!(matches!(
            resolve(&[a1, a2], &r, 2),
            Err(MindError::AmbiguousItem { .. })
        ));
        // A bare (kind-less) ref never gets an alternate.
        assert!(alternate_ref(&parse_item_ref("review").unwrap()).is_none());
        // Installed: a source selector suppresses the alternate reading.
        let m = manifest(vec![inst(ItemKind::Skill, "workflow:review", "h/o/a")]);
        let scoped = parse_item_ref("h/o/a#workflow:review").unwrap();
        assert!(matches!(
            resolve_installed(&m, &scoped),
            Err(MindError::NotInstalled { .. })
        ));
        // Neither reading matches.
        assert!(matches!(
            resolve_installed(&m, &parse_item_ref("agent:review").unwrap()),
            Err(MindError::NotInstalled { .. })
        ));
    }

    #[test]
    fn installed_lookup_honors_kind_and_source_qualifier() {
        // spec: CLI-40, CLI-63, CLI-71
        let m = manifest(vec![
            inst(ItemKind::Skill, "review", "github.com/james/agents"),
            inst(ItemKind::Agent, "review", "github.com/james/agents"),
        ]);
        // A bare name shared across kinds is ambiguous.
        let bare = parse_item_ref("review").unwrap();
        assert!(matches!(
            resolve_installed(&m, &bare),
            Err(MindError::AmbiguousItem { .. })
        ));
        // A kind prefix disambiguates.
        let skill = parse_item_ref("skill:review").unwrap();
        assert_eq!(resolve_installed(&m, &skill).unwrap().kind, ItemKind::Skill);
        // A source qualifier that does not match yields NotInstalled.
        let wrong = parse_item_ref("other/repo#skill:review").unwrap();
        assert!(matches!(
            resolve_installed(&m, &wrong),
            Err(MindError::NotInstalled { .. })
        ));
        // A matching source qualifier resolves.
        let right = parse_item_ref("james/agents#skill:review").unwrap();
        assert_eq!(resolve_installed(&m, &right).unwrap().kind, ItemKind::Skill);
    }

    #[test]
    fn select_installed_matches_glob_kind_and_source() {
        // spec: CLI-41
        // The manifest is keyed by `kind:effective_name`, so names are distinct.
        let m = manifest(vec![
            inst(ItemKind::Skill, "review", "github.com/james/agents"),
            inst(ItemKind::Skill, "release", "github.com/james/agents"),
            inst(ItemKind::Agent, "dev", "github.com/james/agents"),
            inst(ItemKind::Skill, "audit", "github.com/bob/agents"),
        ]);
        // Glob over all skills (across both sources).
        assert_eq!(
            select_installed(&m, &parse_item_ref("skill:*").unwrap()).len(),
            3
        );
        // Prefix glob.
        assert_eq!(
            select_installed(&m, &parse_item_ref("rele*").unwrap()).len(),
            1
        );
        // Everything.
        assert_eq!(select_installed(&m, &parse_item_ref("*").unwrap()).len(), 4);
        // Source-scoped glob.
        assert_eq!(
            select_installed(&m, &parse_item_ref("bob/agents#*").unwrap()).len(),
            1
        );
        // Exact name (no glob) still matches by equality.
        assert_eq!(
            select_installed(&m, &parse_item_ref("review").unwrap()).len(),
            1
        );
    }

    // spec: CLI-65
    #[test]
    fn installed_matches_glob_bare_star_matches_all() {
        let items = [
            inst(ItemKind::Skill, "review", "github.com/james/agents"),
            inst(ItemKind::Agent, "dev", "github.com/james/agents"),
            inst(ItemKind::Rule, "style", "github.com/bob/agents"),
        ];
        let r = parse_item_ref("*").unwrap();
        assert!(items.iter().all(|it| installed_matches_glob(it, &r)));
    }

    // spec: CLI-65
    #[test]
    fn installed_matches_glob_kind_prefix_narrows() {
        let skill = inst(ItemKind::Skill, "review", "github.com/james/agents");
        let agent = inst(ItemKind::Agent, "dev", "github.com/james/agents");
        let r = parse_item_ref("skill:*").unwrap();
        assert!(installed_matches_glob(&skill, &r));
        assert!(!installed_matches_glob(&agent, &r));
    }

    // spec: CLI-65
    #[test]
    fn installed_matches_glob_source_qualifier_narrows() {
        let james = inst(ItemKind::Skill, "review", "github.com/james/agents");
        let bob = inst(ItemKind::Skill, "audit", "github.com/bob/agents");
        let r = parse_item_ref("james/agents#*").unwrap();
        assert!(installed_matches_glob(&james, &r));
        assert!(!installed_matches_glob(&bob, &r));
    }

    // spec: CLI-65
    #[test]
    fn installed_matches_glob_exact_name_matches_only_that_item() {
        let review = inst(ItemKind::Skill, "review", "github.com/james/agents");
        let release = inst(ItemKind::Skill, "release", "github.com/james/agents");
        let r = parse_item_ref("review").unwrap();
        assert!(installed_matches_glob(&review, &r));
        assert!(!installed_matches_glob(&release, &r));
    }

    // spec: CLI-65
    #[test]
    fn installed_matches_glob_non_matching_glob_is_false() {
        let review = inst(ItemKind::Skill, "review", "github.com/james/agents");
        let r = parse_item_ref("xyz*").unwrap();
        assert!(!installed_matches_glob(&review, &r));
    }

    #[test]
    fn all_selector_appends_glob_and_rejects_hash() {
        // spec: CLI-36
        assert_eq!(
            all_selector("local/dev/agents").unwrap(),
            "local/dev/agents#*"
        );
        assert_eq!(all_selector("agents").unwrap(), "agents#*");
        // Whitespace is trimmed before the suffix is appended.
        assert_eq!(all_selector("  agents  ").unwrap(), "agents#*");
        // A ref that already names an item (carries `#`) is rejected.
        assert!(matches!(
            all_selector("agents#review"),
            Err(MindError::InvalidItemRef { .. })
        ));
        assert!(matches!(
            all_selector("agents#*"),
            Err(MindError::InvalidItemRef { .. })
        ));
    }

    #[test]
    fn detects_glob_patterns() {
        assert!(is_glob("*"));
        assert!(is_glob("review*"));
        assert!(is_glob("skill:*"));
        assert!(!is_glob("review"));
    }

    // ----- DSC-62 / DSC-63: select_by_bare_refs subset filtering -----

    #[test]
    fn select_by_bare_refs_matches_kind_and_bare_name() {
        // spec: DSC-62 — a bare kind:name ref selects exactly the item of that
        // kind and bare name; an unlisted item is excluded.
        let items = vec![
            cat(ItemKind::Skill, "review", "a"),
            cat(ItemKind::Agent, "dev", "a"),
            cat(ItemKind::Rule, "style", "a"),
        ];
        let refs = vec!["skill:review".to_string(), "agent:dev".to_string()];
        let picked = select_by_bare_refs(&items, &refs);
        assert_eq!(picked.len(), 2);
        assert!(
            picked
                .iter()
                .any(|it| it.kind == ItemKind::Skill && it.name == "review")
        );
        assert!(
            picked
                .iter()
                .any(|it| it.kind == ItemKind::Agent && it.name == "dev")
        );
        assert!(
            !picked.iter().any(|it| it.name == "style"),
            "an unlisted item must not be selected"
        );
    }

    #[test]
    fn select_by_bare_refs_distinguishes_kind_for_same_bare_name() {
        // spec: DSC-63 — refs carry an explicit kind, so two items sharing a bare
        // name across kinds are not conflated: skill:x selects only the skill.
        let items = vec![
            cat(ItemKind::Skill, "x", "a"),
            cat(ItemKind::Agent, "x", "a"),
        ];
        let only_skill = select_by_bare_refs(&items, &["skill:x".to_string()]);
        assert_eq!(only_skill.len(), 1);
        assert_eq!(only_skill[0].kind, ItemKind::Skill);

        let only_agent = select_by_bare_refs(&items, &["agent:x".to_string()]);
        assert_eq!(only_agent.len(), 1);
        assert_eq!(only_agent[0].kind, ItemKind::Agent);

        // Both refs select both items.
        let both = select_by_bare_refs(&items, &["skill:x".to_string(), "agent:x".to_string()]);
        assert_eq!(both.len(), 2);
    }

    #[test]
    fn select_by_bare_refs_matches_by_bare_not_effective_name() {
        // spec: DSC-63 — matching is against the BARE name, so a prefixed item is
        // still selected by its bare ref (the prefix is an install-time transform).
        let mut item = cat(ItemKind::Skill, "review", "a");
        item.prefix = Some("pfx".to_string());
        assert_eq!(item.effective_name(), "pfx:review");
        let items = vec![item];

        // The bare ref selects it.
        let by_bare = select_by_bare_refs(&items, &["skill:review".to_string()]);
        assert_eq!(by_bare.len(), 1, "bare ref must select the prefixed item");

        // A ref written with the prefix does NOT match (refs are bare in source truth).
        let by_prefixed = select_by_bare_refs(&items, &["skill:pfx:review".to_string()]);
        assert!(
            by_prefixed.is_empty(),
            "a prefixed-name ref must not match; refs are bare names"
        );
    }

    #[test]
    fn select_by_bare_refs_skips_kindless_and_malformed_refs() {
        // spec: DSC-62 — a ref with no explicit kind (bare name only) is not
        // matched here, and a malformed ref is skipped rather than panicking.
        let items = vec![
            cat(ItemKind::Skill, "review", "a"),
            cat(ItemKind::Agent, "dev", "a"),
        ];
        // Bare name (no kind) matches nothing.
        assert!(
            select_by_bare_refs(&items, &["review".to_string()]).is_empty(),
            "a kindless ref must not select anything"
        );
        // A non-kind-word prefix is kindless (parses to kind=None), so the
        // kindless filter drops it; an empty name after a kind word is malformed.
        // Both are skipped.
        assert!(select_by_bare_refs(&items, &["bogus:review".to_string()]).is_empty());
        assert!(select_by_bare_refs(&items, &["skill:".to_string()]).is_empty());
        assert!(select_by_bare_refs(&items, &["".to_string()]).is_empty());
    }

    #[test]
    fn select_by_bare_refs_ref_matching_nothing_yields_empty() {
        // spec: DSC-62 — a well-formed ref naming an item that is not present
        // selects nothing (validation of unknown refs is the caller's job).
        let items = vec![cat(ItemKind::Skill, "review", "a")];
        assert!(
            select_by_bare_refs(&items, &["skill:absent".to_string()]).is_empty(),
            "a ref matching no item yields an empty subset"
        );
        // An empty ref list selects nothing.
        assert!(select_by_bare_refs(&items, &[]).is_empty());
    }

    #[test]
    fn select_by_bare_refs_duplicate_refs_do_not_duplicate_items() {
        // spec: DSC-62 — the result is the matching subset in catalog order; a
        // duplicate ref does not yield the same item twice (iteration is over the
        // catalog, filtered by the ref set).
        let items = vec![
            cat(ItemKind::Skill, "review", "a"),
            cat(ItemKind::Agent, "dev", "a"),
        ];
        let refs = vec![
            "skill:review".to_string(),
            "skill:review".to_string(),
            "skill:review".to_string(),
        ];
        let picked = select_by_bare_refs(&items, &refs);
        assert_eq!(
            picked.len(),
            1,
            "a duplicated ref must not duplicate the item"
        );
        assert_eq!(picked[0].name, "review");
    }

    #[test]
    fn select_by_bare_refs_preserves_catalog_order() {
        // spec: DSC-62 — the subset is returned in catalog order, independent of
        // the order refs are listed in install-items.
        let items = vec![
            cat(ItemKind::Skill, "review", "a"),
            cat(ItemKind::Agent, "dev", "a"),
            cat(ItemKind::Rule, "style", "a"),
        ];
        // Refs in reverse order; result follows catalog order.
        let refs = vec![
            "rule:style".to_string(),
            "agent:dev".to_string(),
            "skill:review".to_string(),
        ];
        let picked = select_by_bare_refs(&items, &refs);
        assert_eq!(picked.len(), 3);
        assert_eq!(picked[0].name, "review");
        assert_eq!(picked[1].name, "dev");
        assert_eq!(picked[2].name, "style");
    }

    #[test]
    fn select_matches_glob_kind_and_source() {
        let items = vec![
            cat(ItemKind::Skill, "review", "a"),
            cat(ItemKind::Skill, "release", "a"),
            cat(ItemKind::Agent, "dev", "a"),
            cat(ItemKind::Skill, "review", "b"),
        ];
        // Glob over all skills.
        assert_eq!(select(&items, &parse_item_ref("skill:*").unwrap()).len(), 3);
        // Prefix glob.
        assert_eq!(select(&items, &parse_item_ref("rele*").unwrap()).len(), 1);
        // Everything.
        assert_eq!(select(&items, &parse_item_ref("*").unwrap()).len(), 4);
        // Source-scoped glob.
        assert_eq!(select(&items, &parse_item_ref("a#*").unwrap()).len(), 3);
        // Source + kind + glob compose: skills of source a.
        assert_eq!(
            select(&items, &parse_item_ref("a#skill:*").unwrap()).len(),
            2
        );
        // Exact name (no glob) still matches by equality.
        assert_eq!(select(&items, &parse_item_ref("dev").unwrap()).len(), 1);
    }
}