Fix home-scoped update checks

Prepared with AI assistance under maintainer authorization.

Cargo.lock

@@ -71,12 +71,6 @@ version = "1.25.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "95832e849adfb21180ccb6826a99da14e5d266ae5c2e668e1602cf234f153797"
 
-[[package]]
-name = "byteorder"
-version = "1.5.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b"
-
 [[package]]
 name = "byteorder-lite"
 version = "0.1.0"
@@ -1340,17 +1334,6 @@ version = "1.16.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b9be42f50aa861c555654aa3a37f52f4b1074bacf4e48fe0ef7fa584e80f1f0f"
 
-[[package]]
-name = "socks"
-version = "0.3.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f0c3dbbd9ae980613c6dd8e28a9407b50509d3803b57624d5dfe8315218cd58b"
-dependencies = [
- "byteorder",
- "libc",
- "winapi",
-]
-
 [[package]]
 name = "stable_deref_trait"
 version = "1.2.1"
@@ -1543,7 +1526,6 @@ dependencies = [
  "rustls-pki-types",
  "serde",
  "serde_json",
- "socks",
  "url",
  "webpki-roots 0.26.11",
 ]
@@ -1674,28 +1656,6 @@ version = "0.1.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a28ac98ddc8b9274cb41bb4d9d4d5c425b6020c50c46f25559911905610b4a88"
 
-[[package]]
-name = "winapi"
-version = "0.3.9"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419"
-dependencies = [
- "winapi-i686-pc-windows-gnu",
- "winapi-x86_64-pc-windows-gnu",
-]
-
-[[package]]
-name = "winapi-i686-pc-windows-gnu"
-version = "0.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
-
-[[package]]
-name = "winapi-x86_64-pc-windows-gnu"
-version = "0.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"
-
 [[package]]
 name = "windows-link"
 version = "0.2.1"

crates/context/Cargo.toml

@@ -20,7 +20,7 @@ regex = { workspace = true }
 once_cell = { workspace = true }
 sha2 = "0.10"
 flate2 = { version = "1", default-features = false, features = ["zlib-rs"] }
-ureq = { version = "2", default-features = false, features = ["tls", "json", "socks-proxy"] }
+ureq = { version = "2", default-features = false, features = ["tls", "json"] }
 rustls-native-certs = "0.8"
 webpki-roots = "1"
 tiny_http = "0.12"

crates/context/src/generate_image.rs

@@ -485,7 +485,6 @@ mod tests {
     use std::time::Duration;
 
     fn round_trip(edit: bool, override_model: Option<&str>, background: Option<&str>) {
-        let _proxy_lock = crate::http::PROXY_ENV_LOCK.lock().unwrap();
         let server = tiny_http::Server::http("127.0.0.1:0").unwrap();
         let api_base = format!("http://{}", server.server_addr());
         let temp = std::env::temp_dir().join(format!("impeccable-image-{}-{}", std::process::id(), server.server_addr().to_ip().unwrap().port()));

crates/context/src/http.rs

@@ -15,30 +15,18 @@
 //! fails to load, verifies against the bundled roots exactly as before.
 //! `SSL_CERT_FILE` / `SSL_CERT_DIR` stand in for the OS store, as they do
 //! for OpenSSL and curl; the bundled roots stay either way.
-//!
-//! The shared agent builder also honors `ALL_PROXY`, `HTTPS_PROXY`, and
-//! `HTTP_PROXY` (and their lowercase forms) so `update` and `install` work
-//! behind a corporate proxy (#823). Live-mode localhost HTTP does not use
-//! this builder. The `socks-proxy` feature is enabled because ureq 2.x
-//! prefers `ALL_PROXY`, which is often `socks5://`. We opt in on this
-//! builder only, not globally via ureq's `proxy-from-env` feature.
 
 use std::sync::Arc;
 
-#[cfg(test)]
-pub(crate) static PROXY_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
-
 use once_cell::sync::Lazy;
 use ureq::rustls::pki_types::CertificateDer;
 use ureq::rustls::{self, ClientConfig, RootCertStore};
 
-/// `ureq::AgentBuilder::new()` with the engine's trust store installed and
-/// env proxy vars honored. Every HTTPS call site builds its agent from this;
-/// the plain-HTTP calls to the live server on localhost do not use it.
+/// `ureq::AgentBuilder::new()` with the engine's trust store installed.
+/// Every HTTPS call site builds its agent from this; the plain-HTTP calls
+/// to the live server on localhost do not need it.
 pub fn agent_builder() -> ureq::AgentBuilder {
-    ureq::AgentBuilder::new()
-        .tls_config(tls_config())
-        .try_proxy_from_env(true)
+    ureq::AgentBuilder::new().tls_config(tls_config())
 }
 
 fn tls_config() -> Arc<ClientConfig> {
@@ -111,223 +99,6 @@ tB0WGTOG3QIgdJa8gBPU9Y6WsrursItsnUeGTYHKDCZZ6MjlekLFuoc=
     fn agent_builds_from_this_hosts_store() {
         // Runs the real rustls-native-certs load: it must not panic, and the
         // shared config must be accepted by a ureq agent.
-        let _lock = PROXY_ENV_LOCK.lock().unwrap();
         let _agent = agent_builder().build();
     }
-
-    struct ProxyEnvGuard {
-        saved: Vec<(String, Option<String>)>,
-    }
-
-    impl ProxyEnvGuard {
-        fn set(vars: &[(&str, Option<&str>)]) -> Self {
-            let saved = vars
-                .iter()
-                .map(|(key, _)| (key.to_string(), std::env::var(key).ok()))
-                .collect();
-            for (key, value) in vars {
-                match value {
-                    // SAFETY: PROXY_ENV_LOCK serializes every test that
-                    // reads or writes these process-global proxy vars.
-                    Some(v) => unsafe { std::env::set_var(key, v) },
-                    None => unsafe { std::env::remove_var(key) },
-                }
-            }
-            Self { saved }
-        }
-    }
-
-    impl Drop for ProxyEnvGuard {
-        fn drop(&mut self) {
-            for (key, value) in &self.saved {
-                match value {
-                    // SAFETY: same lock as set(); restore before unlock.
-                    Some(v) => unsafe { std::env::set_var(key, v) },
-                    None => unsafe { std::env::remove_var(key) },
-                }
-            }
-        }
-    }
-
-    fn accept_until(
-        listener: std::net::TcpListener,
-        mut handle: impl FnMut(&mut std::net::TcpStream) -> bool,
-    ) {
-        use std::time::Duration;
-        listener
-            .set_nonblocking(true)
-            .expect("nonblocking proxy listener");
-        let deadline = std::time::Instant::now() + Duration::from_secs(5);
-        while std::time::Instant::now() < deadline {
-            let Ok((mut stream, _)) = listener.accept() else {
-                std::thread::sleep(Duration::from_millis(10));
-                continue;
-            };
-            let _ = stream.set_nonblocking(false);
-            let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(2)));
-            let _ = stream.set_write_timeout(Some(std::time::Duration::from_secs(2)));
-            if handle(&mut stream) {
-                return;
-            }
-        }
-    }
-
-    #[test]
-    fn agent_honors_http_proxy_from_env() {
-        use std::io::{Read, Write};
-        use std::net::TcpListener;
-        use std::time::Duration;
-
-        let _lock = PROXY_ENV_LOCK.lock().unwrap();
-
-        let listener = TcpListener::bind("127.0.0.1:0").expect("bind proxy listener");
-        let proxy_addr = listener.local_addr().expect("proxy listener addr");
-
-        let request = std::sync::Arc::new(std::sync::Mutex::new(Vec::<u8>::new()));
-        let request_for_thread = request.clone();
-        let handle = std::thread::spawn(move || {
-            accept_until(listener, |stream| {
-                let mut buf = [0u8; 4096];
-                let n = stream.read(&mut buf).unwrap_or(0);
-                let chunk = &buf[..n];
-                if chunk.is_empty()
-                    || !String::from_utf8_lossy(chunk).contains("proxy-test.invalid")
-                {
-                    return false;
-                }
-                request_for_thread.lock().unwrap().extend_from_slice(chunk);
-                let _ = stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok");
-                true
-            });
-        });
-
-        let proxy_url = format!("http://127.0.0.1:{}", proxy_addr.port());
-        let _env_guard = ProxyEnvGuard::set(&[
-            ("ALL_PROXY", None),
-            ("all_proxy", None),
-            ("HTTPS_PROXY", None),
-            ("https_proxy", None),
-            ("HTTP_PROXY", Some(&proxy_url)),
-            ("http_proxy", Some(&proxy_url)),
-        ]);
-
-        let agent = agent_builder()
-            .timeout(Duration::from_secs(2))
-            .build();
-        let response = agent.get("http://proxy-test.invalid/").call();
-        assert!(response.is_ok(), "expected proxy-routed GET to succeed");
-
-        handle.join().expect("proxy thread");
-
-        let request_bytes = request.lock().unwrap().clone();
-        let request_text = String::from_utf8_lossy(&request_bytes);
-        assert!(
-            request_text.contains("proxy-test.invalid"),
-            "proxy should receive request for target host, got: {request_text:?}"
-        );
-    }
-
-    #[test]
-    fn agent_honors_socks5_all_proxy_from_env() {
-        use std::io::{Read, Write};
-        use std::net::TcpListener;
-        use std::time::Duration;
-
-        fn socks5_then_http(stream: &mut std::net::TcpStream) -> Option<Vec<u8>> {
-            fn read_n(stream: &mut std::net::TcpStream, n: usize) -> Option<Vec<u8>> {
-                let mut buf = vec![0u8; n];
-                stream.read_exact(&mut buf).ok()?;
-                Some(buf)
-            }
-
-            let greet = read_n(stream, 2)?;
-            if greet[0] != 5 {
-                return None;
-            }
-            let _ = read_n(stream, greet[1] as usize)?;
-            stream.write_all(&[0x05, 0x00]).ok()?;
-
-            let req = read_n(stream, 4)?;
-            if req[0] != 5 || req[1] != 1 {
-                return None;
-            }
-            match req[3] {
-                1 => {
-                    let _ = read_n(stream, 6)?;
-                }
-                3 => {
-                    let len = read_n(stream, 1)?;
-                    let _ = read_n(stream, len[0] as usize + 2)?;
-                }
-                4 => {
-                    let _ = read_n(stream, 18)?;
-                }
-                _ => return None,
-            }
-            stream
-                .write_all(&[0x05, 0x00, 0x00, 0x01, 0, 0, 0, 0, 0, 0])
-                .ok()?;
-
-            let mut chunk = Vec::new();
-            let mut buf = [0u8; 4096];
-            loop {
-                let n = stream.read(&mut buf).ok()?;
-                if n == 0 {
-                    break;
-                }
-                chunk.extend_from_slice(&buf[..n]);
-                if String::from_utf8_lossy(&chunk).contains("proxy-test.invalid") {
-                    break;
-                }
-            }
-            if !String::from_utf8_lossy(&chunk).contains("proxy-test.invalid") {
-                return None;
-            }
-            let _ = stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok");
-            Some(chunk)
-        }
-
-        let _lock = PROXY_ENV_LOCK.lock().unwrap();
-
-        let listener = TcpListener::bind("127.0.0.1:0").expect("bind socks listener");
-        let proxy_addr = listener.local_addr().expect("socks listener addr");
-
-        let request = std::sync::Arc::new(std::sync::Mutex::new(Vec::<u8>::new()));
-        let request_for_thread = request.clone();
-        let handle = std::thread::spawn(move || {
-            accept_until(listener, |stream| {
-                if let Some(chunk) = socks5_then_http(stream) {
-                    *request_for_thread.lock().unwrap() = chunk;
-                    true
-                } else {
-                    false
-                }
-            });
-        });
-
-        let proxy_url = format!("socks5://127.0.0.1:{}", proxy_addr.port());
-        let _env_guard = ProxyEnvGuard::set(&[
-            ("ALL_PROXY", Some(&proxy_url)),
-            ("all_proxy", Some(&proxy_url)),
-            ("HTTPS_PROXY", None),
-            ("https_proxy", None),
-            ("HTTP_PROXY", None),
-            ("http_proxy", None),
-        ]);
-
-        let agent = agent_builder()
-            .timeout(Duration::from_secs(2))
-            .build();
-        let response = agent.get("http://proxy-test.invalid/").call();
-        assert!(response.is_ok(), "expected SOCKS5-routed GET to succeed");
-
-        handle.join().expect("socks thread");
-
-        let request_bytes = request.lock().unwrap().clone();
-        let request_text = String::from_utf8_lossy(&request_bytes);
-        assert!(
-            request_text.contains("proxy-test.invalid"),
-            "SOCKS proxy should receive request for target host, got: {request_text:?}"
-        );
-    }
 }

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<bool, String> {
         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) => {

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]

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).