diff --git a/crates/skills/src/commands.rs b/crates/skills/src/commands.rs index 6e3fdfc99..bd9d6b659 100644 --- a/crates/skills/src/commands.rs +++ b/crates/skills/src/commands.rs @@ -140,25 +140,26 @@ fn locale_compare(a: &str, b: &str) -> std::cmp::Ordering { fn check(io: &mut Io) -> R<()> { let (sys, _) = ctx(io); let root = sys.find_project_root(); - if sys.is_already_installed(&root, None).is_none() { + // A home-rooted check is the user-level equivalent of `update --global`. + // Keep both verbs on the canonical provider paths so stale legacy paths + // (for example ~/.pi/skills) cannot make only `check` report drift. + let scope = if sys.is_home_dir(&root) { Some(Scope::User) } else { None }; + if sys.is_already_installed(&root, scope).is_none() { out(io, "Impeccable is not installed in this project."); out(io, "Run `npx impeccable install` to install."); return Err(Flow::Exit(0)); } - let providers = sys.find_installed_providers(&root, None); + let providers = sys.find_installed_providers(&root, scope); out(io, "Checking for updates...\n"); let result = (|| -> Result { let bundle_dir = bundle::download_and_extract_bundle(&sys)?; - // JS: agentScope 'user' for a home-rooted checkout (d2a9efb9), so - // check() judges agent freshness against the user agent dirs. - let agent_scope = if sys.is_home_dir(&root) { Some(Scope::User) } else { None }; - let up_to_date = bundle::is_up_to_date(&sys, &root, &providers, &bundle_dir, None, agent_scope)?; + let up_to_date = bundle::is_up_to_date(&sys, &root, &providers, &bundle_dir, scope, scope)?; util::rm_rf(&bundle_dir); Ok(up_to_date) })(); match result { Ok(true) => { - let v = sys.get_skills_version(&root, None); + let v = sys.get_skills_version(&root, scope); out(io, &format!("Skills are up to date{}.", version_suffix(&v))); } Ok(false) => { diff --git a/crates/skills/tests/drift_ports_tests.rs b/crates/skills/tests/drift_ports_tests.rs index dfd8b7049..133c755be 100644 --- a/crates/skills/tests/drift_ports_tests.rs +++ b/crates/skills/tests/drift_ports_tests.rs @@ -460,6 +460,40 @@ fn check_accepts_current_copilot_user_agents_in_home_rooted_checkout() { std::fs::remove_dir_all(&root).ok(); } +#[test] +fn check_ignores_stale_legacy_pi_skills_when_the_user_install_is_current() { + let root = temp_root("pi-check-home-scope"); + let home = format!("{root}/home"); + let tmpdir = format!("{root}/tmp"); + for d in [&home, &tmpdir] { + std::fs::create_dir_all(d).unwrap(); + } + let bundle_root = create_fake_universal_bundle(&root, &[".pi"]); + let env = base_env(&home, &tmpdir, &bundle_root); + + let r = run_cli( + &["install", "-y", "--scope=global", "--no-hooks", "--providers=pi"], + &home, + &env, + ); + assert_eq!(r.code, 0, "{}\n{}", r.stdout, r.stderr); + + let canonical = format!("{home}/.pi/agent/skills/impeccable"); + let legacy = format!("{home}/.pi/skills/impeccable"); + std::fs::create_dir_all(format!("{home}/.pi/skills")).unwrap(); + std::fs::create_dir_all(&legacy).unwrap(); + write(&format!("{legacy}/SKILL.md"), "---\nname: impeccable\nversion: 1.0.0-stale\n---\n"); + assert!(std::path::Path::new(&canonical).exists()); + + let update = run_cli(&["update", "--global", "-y", "--no-hooks"], &home, &env); + assert!(update.stdout.contains("Skills are up to date"), "{}\n{}", update.stdout, update.stderr); + + let check = run_cli(&["check"], &home, &env); + assert!(check.stdout.contains("Skills are up to date"), "{}\n{}", check.stdout, check.stderr); + assert!(!check.stdout.contains("Updates available"), "{}", check.stdout); + std::fs::remove_dir_all(&root).ok(); +} + // ─── inferred agent update scope (d2a9efb9) ────────────────────────────────── #[test] diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index a2e53526b..87e50ca73 100644 --- a/docs/CLI-CONTRACT.md +++ b/docs/CLI-CONTRACT.md @@ -383,7 +383,7 @@ retain their local-development trust behavior. See [bundle signing](BUNDLE-SIGNI Already installed (and not `--force`): `Impeccable skills are already installed (found in ${provider}/).`; compares tree hashes (`sha256` of file content with `\.(claude|cursor|...)\/skills\/` normalized to `.PROVIDER/skills/`); if differs → refresh + `Updated ${n} skill(s) to v${v}.`; missing hooks repaired; else `Skills are up to date (v${v}).` + `Run with --force to reinstall.`; offline → `Could not check for skill updates: ${msg}` + `Existing skills were left unchanged.`; ends `Done!` or the above; `exit 0`. Version read from `^version:\s*(.+)$` in installed `impeccable/SKILL.md`. - **update flags**: `-y|--yes`, `--force`, `--no-hooks`, scope flags as above (unknown → `Unknown update scope: ${v}. Use --project or --user.`). Resolves project vs user installs holding an `impeccable`/`*-impeccable`/`teach-impeccable` skill; none → `No impeccable skill folders found in this project or at the user level.` + `Run \`npx impeccable install\` to install first.`, exit 1; both → prompt `Update which? [project]/user: ` (non-TTY defaults project). Prints `Updating the ${label} install: ${root} (${providers})`, linked providers note, `Checking for updates...`; up to date → `Skills are up to date (vX).` [+hooks] + `Nothing else to do.`, exit 0; else `Found skills in: ...`, prompt `Update skills in N provider folder(s)? (Y/n) ` (n/no → `Aborted.` exit 0), refresh, `Updated N skill(s) to vX.`, `Done!`. - **link**: `--source=` (default `.impeccable`), `--providers`, `--force`, `-y`. Source must contain `dist/universal/` or provider `*/skills` dirs, else `Could not find compiled skills in ${src}. Expected dist/universal/ or provider skill folders.` Prompts `Link impeccable skills into N folder(s)? (Y/n) `; creates relative dir symlinks; existing non-link skipped with warning unless `--force`; output `Linked impeccable into: ... (N linked, N already linked, N skipped).` + submodule hint. -- **check**: not installed → `Impeccable is not installed in this project.` + `Run \`npx impeccable install\` to install.` exit 0; else `Checking for updates...\n` then `Skills are up to date (vX).` or `Updates available.` + `Run \`npx impeccable update\` to update.`; failure → `Could not check for updates: ${msg}` exit 1. +- **check**: not installed → `Impeccable is not installed in this project.` + `Run \`npx impeccable install\` to install.` exit 0; else `Checking for updates...\n` then `Skills are up to date (vX).` or `Updates available.` + `Run \`npx impeccable update\` to update.`; failure → `Could not check for updates: ${msg}` exit 1. A home-rooted check uses user scope, matching `update --global`: provider-specific canonical global paths are compared, while stale legacy duplicates such as `~/.pi/skills` do not create false update notices. - Prompts: non-TTY `ask()` reads answers line-by-line from stdin (fd 0) after echoing the question; TTY SIGINT → `PromptAbortError` (`code IMPECCABLE_PROMPT_ABORT`) → cli.js prints `\nAborted.` exit 130. ANSI (`\x1b[36m` accent, `\x1b[1m` bold, `\x1b[2m` dim, `\x1b[32m` good) only when stdout is TTY, `NO_COLOR` unset, `TERM !== 'dumb'`. - Tests: `tests/skills-cli.test.js`, `tests/cli-remote-e2e` (opt-in).