fix(sandbox): stop claiming strict mode is isolation; gate clippy in CI

.github/workflows/ci.yml

@@ -20,10 +20,14 @@ jobs:
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
       - uses: Swatinem/rust-cache@v2
-      - name: Check
-        run: cargo check --all-targets
+      # clippy runs the full `cargo check` pass plus lints, so this replaces the
+      # old bare `cargo check` rather than adding to it — same build, more
+      # coverage, no extra CI time. `-D warnings` is the point: clippy was clean
+      # but ungated, so nothing stopped it drifting back one PR at a time.
+      - name: Lint (clippy, warnings are errors)
+        run: cargo clippy --all-targets --all-features -- -D warnings
       - name: Test
-        run: cargo test
+        run: cargo test --all-features
 
   readme-lint:
     runs-on: ubuntu-latest

src/auth.rs

@@ -254,7 +254,14 @@ fn anthropic_config_dir() -> Option<std::path::PathBuf> {
 /// the credentials directory first means we never execute anything unless the
 /// real CLI has actually been used here.
 fn ant_profile_dir_exists() -> bool {
-    anthropic_config_dir().is_some_and(|d| d.join("credentials").is_dir())
+    profile_dir_exists_at(anthropic_config_dir().as_deref())
+}
+
+/// Pure form of the check, so it can be tested without mutating process-global
+/// env. `set_var` races under the parallel test harness — the whole point of
+/// the `AuthEnv` seam above is to avoid exactly that.
+fn profile_dir_exists_at(config_dir: Option<&std::path::Path>) -> bool {
+    config_dir.is_some_and(|d| d.join("credentials").is_dir())
 }
 
 impl AuthEnv for ProcessAuthEnv {
@@ -472,47 +479,29 @@ mod tests {
     /// startup ran an unrelated build tool and stalled the process long enough
     /// to fail the headless SDK test. Nothing may be executed unless the real
     /// CLI has actually stored a profile here.
+    ///
+    /// Pure over the directory — no `set_var`, so it cannot race other tests.
     #[test]
     fn no_subprocess_when_no_profile_directory_exists() {
         let empty = tempfile::tempdir().unwrap();
-        // SAFETY: single-threaded within this test; the var is restored below.
-        let prev = std::env::var("ANTHROPIC_CONFIG_DIR").ok();
-        unsafe { std::env::set_var("ANTHROPIC_CONFIG_DIR", empty.path()) };
-
         assert!(
-            !ant_profile_dir_exists(),
+            !profile_dir_exists_at(Some(empty.path())),
             "a config dir with no credentials/ must not trigger a spawn"
         );
-        assert!(ProcessAuthEnv.ant_access_token().is_none());
-        assert!(!ProcessAuthEnv.ant_profile_present());
-
-        unsafe {
-            match prev {
-                Some(v) => std::env::set_var("ANTHROPIC_CONFIG_DIR", v),
-                None => std::env::remove_var("ANTHROPIC_CONFIG_DIR"),
-            }
-        }
+        assert!(
+            !profile_dir_exists_at(None),
+            "no config dir at all must not trigger a spawn"
+        );
     }
 
     #[test]
-    fn config_dir_honours_the_env_override() {
+    fn profile_directory_is_detected_when_present() {
         let dir = tempfile::tempdir().unwrap();
-        let prev = std::env::var("ANTHROPIC_CONFIG_DIR").ok();
-        unsafe { std::env::set_var("ANTHROPIC_CONFIG_DIR", dir.path()) };
-
-        assert_eq!(anthropic_config_dir().as_deref(), Some(dir.path()));
         std::fs::create_dir_all(dir.path().join("credentials")).unwrap();
         assert!(
-            ant_profile_dir_exists(),
+            profile_dir_exists_at(Some(dir.path())),
             "credentials/ present ⇒ the real CLI has been used here"
         );
-
-        unsafe {
-            match prev {
-                Some(v) => std::env::set_var("ANTHROPIC_CONFIG_DIR", v),
-                None => std::env::remove_var("ANTHROPIC_CONFIG_DIR"),
-            }
-        }
     }
 
     #[test]

src/autocommit.rs

@@ -402,6 +402,21 @@ pub fn restore_to(
 /// Delete old `refs/rustyclaw/sessions/*` refs, keeping the `keep` newest by
 /// committer date. `keep == 0` disables pruning. Non-fatal: any error is
 /// logged via `tracing::warn!` and the function returns 0.
+/// Choose which shadow refs to delete: keep the `keep` newest, delete the rest.
+///
+/// Extracted so the *direction* is testable. An inversion here deletes the
+/// newest sessions instead of the oldest — silent, unrecoverable data loss —
+/// and the integration test cannot catch it, because `%(committerdate:unix)`
+/// has one-second granularity and sessions created in a loop all tie.
+///
+/// Ties are resolved by whatever order git returned (refname-alphabetical),
+/// which is *not* recency. See the review tracker: prune ordering is unreliable
+/// for sessions created within the same second.
+fn select_refs_to_delete(mut rows: Vec<(i64, String)>, keep: usize) -> Vec<String> {
+    rows.sort_by_key(|e| std::cmp::Reverse(e.0));
+    rows.into_iter().skip(keep).map(|(_, r)| r).collect()
+}
+
 pub fn prune_old_refs(cwd: &Path, keep: u32) -> anyhow::Result<u32> {
     if keep == 0 || !is_git_repo(cwd) {
         return Ok(0);
@@ -420,7 +435,7 @@ pub fn prune_old_refs(cwd: &Path, keep: u32) -> anyhow::Result<u32> {
         return Ok(0);
     }
     let s = String::from_utf8(out.stdout)?;
-    let mut rows: Vec<(i64, String)> = s
+    let rows: Vec<(i64, String)> = s
         .lines()
         .filter_map(|line| {
             let mut it = line.splitn(2, ' ');
@@ -434,12 +449,7 @@ pub fn prune_old_refs(cwd: &Path, keep: u32) -> anyhow::Result<u32> {
         return Ok(0);
     }
 
-    rows.sort_by(|a, b| b.0.cmp(&a.0));
-    let to_delete: Vec<String> = rows
-        .into_iter()
-        .skip(keep as usize)
-        .map(|(_, r)| r)
-        .collect();
+    let to_delete = select_refs_to_delete(rows, keep as usize);
 
     let mut deleted = 0u32;
     for r in &to_delete {
@@ -482,6 +492,48 @@ pub fn snapshot_turn_raw(
     )
 }
 
+#[cfg(test)]
+mod prune_selection_tests {
+    use super::select_refs_to_delete;
+
+    fn rows(pairs: &[(i64, &str)]) -> Vec<(i64, String)> {
+        pairs.iter().map(|(t, r)| (*t, r.to_string())).collect()
+    }
+
+    /// The direction is the whole point: keep the NEWEST, delete the oldest.
+    /// Inverting this deletes the sessions the user most likely wants to undo
+    /// to — silent, unrecoverable loss. The integration test cannot catch an
+    /// inversion because its timestamps all tie at one-second granularity.
+    #[test]
+    fn keeps_the_newest_and_deletes_the_oldest() {
+        let deleted = select_refs_to_delete(
+            rows(&[(100, "old"), (300, "newest"), (200, "mid"), (50, "oldest")]),
+            2,
+        );
+        assert_eq!(deleted, vec!["old".to_string(), "oldest".to_string()]);
+    }
+
+    #[test]
+    fn keeping_more_than_present_deletes_nothing() {
+        assert!(select_refs_to_delete(rows(&[(1, "a"), (2, "b")]), 10).is_empty());
+    }
+
+    #[test]
+    fn keep_zero_deletes_everything() {
+        let deleted = select_refs_to_delete(rows(&[(1, "a"), (2, "b")]), 0);
+        assert_eq!(deleted.len(), 2);
+    }
+
+    /// Documents the known weakness rather than asserting a guarantee we do not
+    /// have: with equal timestamps the survivors are decided by git's output
+    /// order, not by recency.
+    #[test]
+    fn tied_timestamps_fall_back_to_input_order() {
+        let deleted = select_refs_to_delete(rows(&[(7, "a"), (7, "b"), (7, "c")]), 1);
+        assert_eq!(deleted, vec!["b".to_string(), "c".to_string()]);
+    }
+}
+
 #[cfg(test)]
 mod git_detection_tests {
     use super::*;

src/commands/mod.rs

@@ -728,7 +728,7 @@ fn cmd_cost(ctx: &CommandContext) -> CommandAction {
 fn cmd_context(ctx: &CommandContext) -> CommandAction {
     let limit: u64 = 200_000;
     let used = ctx.tokens_in;
-    let pct = if limit > 0 { used * 100 / limit } else { 0 };
+    let pct = (used * 100).checked_div(limit).unwrap_or(0);
     let bar_len = 30usize;
     let filled = (pct as usize * bar_len / 100).min(bar_len);
     let bar: String = "█".repeat(filled) + &"░".repeat(bar_len - filled);
@@ -2608,7 +2608,7 @@ fn cmd_btw(args: &str) -> CommandAction {
 fn cmd_ctx_viz(ctx: &CommandContext) -> CommandAction {
     let limit: u64 = 200_000;
     let used = ctx.tokens_in;
-    let pct = if limit > 0 { used * 100 / limit } else { 0 };
+    let pct = (used * 100).checked_div(limit).unwrap_or(0);
 
     // Build a visual histogram of context usage
     let bar_width = 40usize;

src/deeplink.rs

@@ -137,14 +137,15 @@ fn hex_val(b: u8) -> Option<u8> {
 ///
 /// Requires xdg-utils to be installed (standard on most desktops).
 /// On non-Linux platforms, prints a notice and returns Ok(()).
+#[cfg(not(target_os = "linux"))]
 pub fn register_protocol() -> anyhow::Result<()> {
-    #[cfg(not(target_os = "linux"))]
-    {
-        eprintln!("Deep link protocol registration is only supported on Linux.");
-        return Ok(());
-    }
+    eprintln!("Deep link protocol registration is only supported on Linux.");
+    Ok(())
+}
 
-    #[cfg(target_os = "linux")]
+/// Register the deep link protocol handler on Linux.
+#[cfg(target_os = "linux")]
+pub fn register_protocol() -> anyhow::Result<()> {
     {
         let scheme = protocol_name();
         let binary = std::env::current_exe()

src/hooks.rs

@@ -543,6 +543,9 @@ mod tests {
         }
     }
 
+    /// Only used by the signal-termination test, which is unix-only — so this
+    /// helper is dead code on Windows and trips `-D warnings` there.
+    #[cfg(unix)]
     fn cfg_post(command: &str) -> HooksConfig {
         HooksConfig {
             post_tool_use: vec![entry(command)],