Fix home-scoped update checks

Prepared with AI assistance under maintainer authorization.

crates/skills/src/commands.rs

@@ -140,25 +140,16 @@ 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();
-    // A home-rooted tree is either the user-level install or a project
-    // install under ~. Pick one scope so leftover dirs on the other layout
-    // cannot make a current copy look stale (#824). Inferred scope walks
-    // both, which is what produced the false "Updates available".
-    let scope = if sys.is_home_dir(&root) {
-        if sys.is_already_installed(&root, Some(Scope::User)).is_some() {
-            Some(Scope::User)
-        } else {
-            Some(Scope::Project)
-        }
-    } else {
-        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_impeccable_providers(&root, scope);
+    let providers = sys.find_installed_providers(&root, scope);
     out(io, "Checking for updates...\n");
     let result = (|| -> Result<bool, String> {
         let bundle_dir = bundle::download_and_extract_bundle(&sys)?;

crates/skills/tests/drift_ports_tests.rs

@@ -460,50 +460,15 @@ fn check_accepts_current_copilot_user_agents_in_home_rooted_checkout() {
     std::fs::remove_dir_all(&root).ok();
 }
 
-// ─── check vs update scope parity (#824) ─────────────────────────────────────
-
 #[test]
-fn check_ignores_unrelated_harness_skill_in_home_rooted_tree() {
-    let root = temp_root("check-824-extra-harness");
+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();
     }
-    std::fs::create_dir_all(format!("{home}/.git")).unwrap();
-    let bundle_root = create_fake_universal_bundle(&home, &[".claude", ".cursor"]);
-    let env = base_env(&home, &tmpdir, &bundle_root);
-
-    let r = run_cli(
-        &["install", "-y", "--scope=global", "--no-hooks", "--providers=claude"],
-        &home,
-        &env,
-    );
-    assert_eq!(r.code, 0, "{}\n{}", r.stdout, r.stderr);
-    write(
-        &format!("{home}/.cursor/skills/other/SKILL.md"),
-        "---\nname: other\n---\nUnrelated skill.\n",
-    );
-
-    let r = run_cli(&["check"], &home, &env);
-    assert!(r.stdout.contains("Skills are up to date"), "{}\n{}", r.stdout, r.stderr);
-    assert!(!r.stdout.contains("Updates available"), "{}", r.stdout);
-
-    let r = run_cli(&["update", "--global", "-y", "--no-hooks"], &home, &env);
-    assert!(r.stdout.contains("Skills are up to date"), "{}\n{}", r.stdout, r.stderr);
-    std::fs::remove_dir_all(&root).ok();
-}
-
-#[test]
-fn check_ignores_leftover_pi_project_layout_in_home_rooted_tree() {
-    let root = temp_root("check-824-pi-leftover");
-    let home = format!("{root}/home");
-    let tmpdir = format!("{root}/tmp");
-    for d in [&home, &tmpdir] {
-        std::fs::create_dir_all(d).unwrap();
-    }
-    std::fs::create_dir_all(format!("{home}/.git")).unwrap();
-    let bundle_root = create_fake_universal_bundle(&home, &[".pi"]);
+    let bundle_root = create_fake_universal_bundle(&root, &[".pi"]);
     let env = base_env(&home, &tmpdir, &bundle_root);
 
     let r = run_cli(
@@ -512,78 +477,20 @@ fn check_ignores_leftover_pi_project_layout_in_home_rooted_tree() {
         &env,
     );
     assert_eq!(r.code, 0, "{}\n{}", r.stdout, r.stderr);
-    assert!(std::path::Path::new(&format!("{home}/.pi/agent/skills/impeccable/SKILL.md")).exists());
-    write(
-        &format!("{home}/.pi/skills/other/SKILL.md"),
-        "---\nname: other\n---\nLeftover project-layout skill.\n",
-    );
 
-    let r = run_cli(&["check"], &home, &env);
-    assert!(r.stdout.contains("Skills are up to date"), "{}\n{}", r.stdout, r.stderr);
-    assert!(!r.stdout.contains("Updates available"), "{}", r.stdout);
+    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 r = run_cli(&["update", "--global", "-y", "--no-hooks"], &home, &env);
-    assert!(r.stdout.contains("Skills are up to date"), "{}\n{}", r.stdout, r.stderr);
-    std::fs::remove_dir_all(&root).ok();
-}
+    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);
 
-#[test]
-fn check_sees_home_rooted_project_scope_pi_install() {
-    let root = temp_root("check-824-pi-project");
-    let home = format!("{root}/home");
-    let tmpdir = format!("{root}/tmp");
-    for d in [&home, &tmpdir] {
-        std::fs::create_dir_all(d).unwrap();
-    }
-    std::fs::create_dir_all(format!("{home}/.git")).unwrap();
-    let bundle_root = create_fake_universal_bundle(&home, &[".pi"]);
-    let env = base_env(&home, &tmpdir, &bundle_root);
-
-    let r = run_cli(
-        &["install", "-y", "--scope=project", "--no-hooks", "--providers=pi"],
-        &home,
-        &env,
-    );
-    assert_eq!(r.code, 0, "{}\n{}", r.stdout, r.stderr);
-    assert!(std::path::Path::new(&format!("{home}/.pi/skills/impeccable/SKILL.md")).exists());
-    assert!(!std::path::Path::new(&format!("{home}/.pi/agent/skills/impeccable/SKILL.md")).exists());
-    write(
-        &format!("{home}/.pi/agent/skills/other/SKILL.md"),
-        "---\nname: other\n---\nLeftover user-layout skill.\n",
-    );
-
-    let r = run_cli(&["check"], &home, &env);
-    assert!(!r.stdout.contains("not installed"), "{}\n{}", r.stdout, r.stderr);
-    assert!(r.stdout.contains("Skills are up to date"), "{}\n{}", r.stdout, r.stderr);
-    assert!(!r.stdout.contains("Updates available"), "{}", r.stdout);
-    std::fs::remove_dir_all(&root).ok();
-}
-
-#[test]
-fn check_reports_stale_impeccable_in_home_rooted_tree() {
-    let root = temp_root("check-824-stale");
-    let home = format!("{root}/home");
-    let tmpdir = format!("{root}/tmp");
-    for d in [&home, &tmpdir] {
-        std::fs::create_dir_all(d).unwrap();
-    }
-    std::fs::create_dir_all(format!("{home}/.git")).unwrap();
-    let bundle_root = create_fake_universal_bundle(&home, &[".claude"]);
-    let env = base_env(&home, &tmpdir, &bundle_root);
-
-    let r = run_cli(
-        &["install", "-y", "--scope=global", "--no-hooks", "--providers=claude"],
-        &home,
-        &env,
-    );
-    assert_eq!(r.code, 0, "{}\n{}", r.stdout, r.stderr);
-    write(
-        &format!("{home}/.claude/skills/impeccable/SKILL.md"),
-        "---\nname: impeccable\nversion: 0.0.0-stale\n---\n\nStale copy.\n",
-    );
-
-    let r = run_cli(&["check"], &home, &env);
-    assert!(r.stdout.contains("Updates available"), "{}\n{}", r.stdout, r.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();
 }
 

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=<path>` (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).