Test: read SOCKS handshake frames with read_exact

The mock proxy now reassembles greeting and CONNECT across TCP fragments so the ALL_PROXY regression cannot flake on a short read.

AI assistance disclosure: this commit was prepared with AI assistance under maintainer direction.

Co-authored-by: Cursor <cursoragent@cursor.com>

Cargo.lock

@@ -71,6 +71,12 @@ 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"
@@ -1334,6 +1340,17 @@ 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"
@@ -1526,6 +1543,7 @@ dependencies = [
  "rustls-pki-types",
  "serde",
  "serde_json",
+ "socks",
  "url",
  "webpki-roots 0.26.11",
 ]
@@ -1656,6 +1674,28 @@ 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"] }
+ureq = { version = "2", default-features = false, features = ["tls", "json", "socks-proxy"] }
 rustls-native-certs = "0.8"
 webpki-roots = "1"
 tiny_http = "0.12"

crates/context/src/generate_image.rs

@@ -485,6 +485,7 @@ 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,18 +15,30 @@
 //! 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.
-/// Every HTTPS call site builds its agent from this; the plain-HTTP calls
-/// to the live server on localhost do not need it.
+/// `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.
 pub fn agent_builder() -> ureq::AgentBuilder {
-    ureq::AgentBuilder::new().tls_config(tls_config())
+    ureq::AgentBuilder::new()
+        .tls_config(tls_config())
+        .try_proxy_from_env(true)
 }
 
 fn tls_config() -> Arc<ClientConfig> {
@@ -99,6 +111,223 @@ 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,35 +140,25 @@ 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
-    };
-    if sys.is_already_installed(&root, scope).is_none() {
+    if sys.is_already_installed(&root, None).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, None);
     out(io, "Checking for updates...\n");
     let result = (|| -> Result<bool, String> {
         let bundle_dir = bundle::download_and_extract_bundle(&sys)?;
-        let up_to_date = bundle::is_up_to_date(&sys, &root, &providers, &bundle_dir, scope, scope)?;
+        // 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)?;
         util::rm_rf(&bundle_dir);
         Ok(up_to_date)
     })();
     match result {
         Ok(true) => {
-            let v = sys.get_skills_version(&root, scope);
+            let v = sys.get_skills_version(&root, None);
             out(io, &format!("Skills are up to date{}.", version_suffix(&v)));
         }
         Ok(false) => {

crates/skills/tests/drift_ports_tests.rs

@@ -460,133 +460,6 @@ 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");
-    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 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);
-    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 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_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);
-    std::fs::remove_dir_all(&root).ok();
-}
-
 // ─── inferred agent update scope (d2a9efb9) ──────────────────────────────────
 
 #[test]