From cb9c18292155da03bb418d4e49943f6d1b036fd8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 16:49:34 +1000 Subject: [PATCH 1/7] fix(doctor): map Claude Code cask updates to brew Apply the builderbot doctor change referenced by Berd commit 49c07563c2148f0f6940b6b14fff7eb6b6c6a6ee so Homebrew cask installs update with brew upgrade claude-code. Signed-off-by: Matt Toohey --- crates/doctor/src/lib.rs | 22 ++++++++++++++++++++++ crates/doctor/src/package_ids.rs | 16 ++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index 3c0192ff5..22e53f2c8 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -1274,6 +1274,28 @@ mod tests { assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); } + /// Claude Code's Homebrew cask should update as the main CLI via brew, not + /// via the npm package used by npm-managed installs. + #[tokio::test] + async fn apply_freshness_brew_main_emits_claude_code_update_command() { + let mut readout = AgentVersionInfo { + install_source: Some(InstallSource::Brew), + ..AgentVersionInfo::default() + }; + let info = freshness::VersionInfo { + installed: Some("2.1.152".into()), + latest: Some("2.1.153".into()), + update_available: Some(true), + command_timeouts: Vec::new(), + }; + apply_freshness(&mut readout, &info, ReadoutSlot::Main, Some("claude-code")); + assert_eq!( + readout.update_command.as_deref(), + Some("brew upgrade claude-code"), + ); + assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); + } + /// Bridge slot with a brew install upgrades via `brew upgrade ` and /// is tagged `UpdateBridge`. #[tokio::test] diff --git a/crates/doctor/src/package_ids.rs b/crates/doctor/src/package_ids.rs index 01cb2f1ed..a79866cc3 100644 --- a/crates/doctor/src/package_ids.rs +++ b/crates/doctor/src/package_ids.rs @@ -88,6 +88,14 @@ pub(crate) const PACKAGE_IDS: &[(&str, &[PackageEntry])] = &[ ( "ai-agent-claude", &[ + // Official Homebrew cask (`brew install --cask claude-code`) for + // the main Claude Code CLI. + ( + InstallSource::Brew, + "claude-code", + LatestSource::Brew, + Role::Main, + ), // Main CLI when installed via npm (e.g. under nvm). The native // curl-pipe install is fingerprinted as `CurlPipe` (no registry // entry here, self-updating) so this only applies when Claude @@ -288,6 +296,14 @@ mod tests { ); } + #[test] + fn claude_main_brew_resolves_to_claude_code_cask() { + assert_eq!( + lookup_package_id("ai-agent-claude", InstallSource::Brew, Role::Main), + Some(("claude-code", LatestSource::Brew)), + ); + } + /// And the bridge readout still resolves to the bridge package even though /// they share an install source. #[test] From b9b6a508b7d513723f27350946342654e765d4dd Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 17:45:45 +1000 Subject: [PATCH 2/7] fix(doctor): preserve Claude Code cask token Derive the Claude Code Homebrew cask token from the resolved Caskroom path when building freshness targets, while allowlisting known Claude casks so arbitrary path text cannot become a brew package id. This keeps claude-code@latest installs using brew info and brew upgrade with claude-code@latest instead of collapsing to claude-code. Signed-off-by: Matt Toohey --- crates/doctor/src/lib.rs | 69 +++++++++++++--- crates/doctor/src/package_ids.rs | 138 +++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 13 deletions(-) diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index 22e53f2c8..410eb9905 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -18,7 +18,7 @@ pub use environment::DoctorEnv; pub use types::{AgentVersionInfo, CheckStatus, DoctorCheck, DoctorReport, FixType}; use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use agents::{check_single_ai_agent, derive_update_command, lookup_fix_command, AI_AGENT_CHECKS}; @@ -30,7 +30,7 @@ use freshness::{ fetch_version_info, is_self_updating, load_cache, save_cache, select_installed_probe, FetchVersionInfoOptions, }; -use package_ids::{lookup_package_id, LatestSource, Role}; +use package_ids::{lookup_package_id_for_binary, LatestSource, Role}; use resolve::resolve_binary_with_diagnostics; use types::{InstallSource, ResolvedBinary}; @@ -379,11 +379,12 @@ fn resolve_package( check_id: &str, source: Option<&InstallSource>, role: Role, + binary_path: Option<&Path>, ) -> (Option, Option) { source .cloned() - .and_then(|src| lookup_package_id(check_id, src, role)) - .map(|(pkg, latest)| (Some(pkg.to_string()), Some(latest))) + .and_then(|src| lookup_package_id_for_binary(check_id, src, role, binary_path)) + .map(|(pkg, latest)| (Some(pkg), Some(latest))) .unwrap_or((None, None)) } @@ -489,24 +490,34 @@ async fn populate_freshness( let is_agent = check.main.is_some() || check.bridge.is_some(); if is_agent { if let (Some(readout), Some(path)) = (&check.main, check.path.as_deref()) { - let (package_id, latest_source) = - resolve_package(&check.id, readout.install_source.as_ref(), Role::Main); + let path = PathBuf::from(path); + let (package_id, latest_source) = resolve_package( + &check.id, + readout.install_source.as_ref(), + Role::Main, + Some(&path), + ); targets.push(FreshnessTarget { id: check.id.clone(), slot: ReadoutSlot::Main, - path: PathBuf::from(path), + path, latest_source, package_id, install_source: readout.install_source.clone(), }); } if let (Some(readout), Some(path)) = (&check.bridge, check.bridge_path.as_deref()) { - let (package_id, latest_source) = - resolve_package(&check.id, readout.install_source.as_ref(), Role::Bridge); + let path = PathBuf::from(path); + let (package_id, latest_source) = resolve_package( + &check.id, + readout.install_source.as_ref(), + Role::Bridge, + Some(&path), + ); targets.push(FreshnessTarget { id: check.id.clone(), slot: ReadoutSlot::Bridge, - path: PathBuf::from(path), + path, latest_source, package_id, install_source: readout.install_source.clone(), @@ -517,12 +528,17 @@ async fn populate_freshness( // install_source the check carries — see check_single_ai_agent). let path_str = check.bridge_path.as_deref().or(check.path.as_deref()); let Some(path_str) = path_str else { continue }; - let (package_id, latest_source) = - resolve_package(&check.id, check.install_source.as_ref(), Role::Any); + let path = PathBuf::from(path_str); + let (package_id, latest_source) = resolve_package( + &check.id, + check.install_source.as_ref(), + Role::Any, + Some(&path), + ); targets.push(FreshnessTarget { id: check.id.clone(), slot: ReadoutSlot::Flat, - path: PathBuf::from(path_str), + path, latest_source, package_id, install_source: check.install_source.clone(), @@ -1296,6 +1312,33 @@ mod tests { assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); } + /// The Homebrew latest-channel cask should keep its token all the way + /// through to the generated update command. + #[tokio::test] + async fn apply_freshness_brew_main_emits_claude_code_latest_update_command() { + let mut readout = AgentVersionInfo { + install_source: Some(InstallSource::Brew), + ..AgentVersionInfo::default() + }; + let info = freshness::VersionInfo { + installed: Some("2.1.152".into()), + latest: Some("2.1.153".into()), + update_available: Some(true), + command_timeouts: Vec::new(), + }; + apply_freshness( + &mut readout, + &info, + ReadoutSlot::Main, + Some("claude-code@latest"), + ); + assert_eq!( + readout.update_command.as_deref(), + Some("brew upgrade claude-code@latest"), + ); + assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); + } + /// Bridge slot with a brew install upgrades via `brew upgrade ` and /// is tagged `UpdateBridge`. #[tokio::test] diff --git a/crates/doctor/src/package_ids.rs b/crates/doctor/src/package_ids.rs index a79866cc3..a388ce884 100644 --- a/crates/doctor/src/package_ids.rs +++ b/crates/doctor/src/package_ids.rs @@ -8,6 +8,8 @@ //! Checks not listed here (or with no matching source) skip the latest-version //! probe. +use std::path::Path; + use crate::types::InstallSource; /// How to fetch the "latest available" version for a package. @@ -238,9 +240,91 @@ pub(crate) fn lookup_package_id( None } +/// Pick the package id and latest-version source for a resolved binary. +/// +/// Most entries are static. Claude Code's Homebrew casks are the exception: +/// both `claude-code` and `claude-code@latest` expose the same `claude` +/// command, so the command name alone cannot distinguish the installed cask +/// channel. When the resolved binary points into Homebrew's Caskroom, preserve +/// the owning Claude cask token, but only for the known allowlisted tokens. +pub(crate) fn lookup_package_id_for_binary( + check_id: &str, + source: InstallSource, + role: Role, + binary_path: Option<&Path>, +) -> Option<(String, LatestSource)> { + if let Some(package_id) = path_package_id_override(check_id, source.clone(), role, binary_path) + { + return Some((package_id.to_string(), LatestSource::Brew)); + } + + lookup_package_id(check_id, source, role) + .map(|(package_id, latest)| (package_id.to_string(), latest)) +} + +fn path_package_id_override( + check_id: &str, + source: InstallSource, + role: Role, + binary_path: Option<&Path>, +) -> Option<&'static str> { + if check_id != "ai-agent-claude" || source != InstallSource::Brew || role != Role::Main { + return None; + } + + let binary_path = binary_path?; + claude_code_cask_token_from_path(binary_path) +} + +fn claude_code_cask_token_from_path(path: &Path) -> Option<&'static str> { + if let Ok(canonical) = path.canonicalize() { + if let Some(token) = claude_code_cask_token_from_caskroom_path(&canonical) { + return Some(token); + } + } + + claude_code_cask_token_from_caskroom_path(path) +} + +fn claude_code_cask_token_from_caskroom_path(path: &Path) -> Option<&'static str> { + let mut components = path.components().filter_map(|c| c.as_os_str().to_str()); + while let Some(component) = components.next() { + if !component.eq_ignore_ascii_case("Caskroom") { + continue; + } + + return allowed_claude_code_cask_token(components.next()?); + } + + None +} + +fn allowed_claude_code_cask_token(token: &str) -> Option<&'static str> { + match token { + "claude-code" => Some("claude-code"), + "claude-code@latest" => Some("claude-code@latest"), + _ => None, + } +} + #[cfg(test)] mod tests { use super::*; + use std::fs; + + fn scratch_dir(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "doctor-package-ids-{name}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(), + )); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir + } #[test] fn lookup_matches_source_with_any_role() { @@ -304,6 +388,60 @@ mod tests { ); } + #[test] + fn claude_main_brew_preserves_latest_cask_token_from_caskroom_path() { + let path = Path::new("/opt/homebrew/Caskroom/claude-code@latest/2.1.153/claude"); + + assert_eq!( + lookup_package_id_for_binary( + "ai-agent-claude", + InstallSource::Brew, + Role::Main, + Some(path), + ), + Some(("claude-code@latest".to_string(), LatestSource::Brew)), + ); + } + + #[cfg(unix)] + #[test] + fn claude_main_brew_preserves_latest_cask_token_through_bin_symlink() { + let root = scratch_dir("claude-cask-symlink"); + let target = root.join("Caskroom/claude-code@latest/2.1.153/claude"); + fs::create_dir_all(target.parent().unwrap()).unwrap(); + fs::write(&target, "#!/bin/sh\n").unwrap(); + fs::create_dir_all(root.join("bin")).unwrap(); + let link = root.join("bin/claude"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + + assert_eq!( + lookup_package_id_for_binary( + "ai-agent-claude", + InstallSource::Brew, + Role::Main, + Some(&link), + ), + Some(("claude-code@latest".to_string(), LatestSource::Brew)), + ); + + let _ = fs::remove_dir_all(root); + } + + #[test] + fn claude_main_brew_ignores_unallowlisted_caskroom_token() { + let path = Path::new("/opt/homebrew/Caskroom/not-claude/1.0.0/claude"); + + assert_eq!( + lookup_package_id_for_binary( + "ai-agent-claude", + InstallSource::Brew, + Role::Main, + Some(path), + ), + Some(("claude-code".to_string(), LatestSource::Brew)), + ); + } + /// And the bridge readout still resolves to the bridge package even though /// they share an install source. #[test] From 346b40c233fc9e6ad2e694540d21a3ef319711a6 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 18:56:40 +1000 Subject: [PATCH 3/7] fix(doctor): force Claude Code npm updates Add npm --force when doctor derives the @anthropic-ai/claude-code global update command so existing claude binaries can be overwritten, while leaving other npm updates and Homebrew cask upgrades unchanged. Signed-off-by: Matt Toohey --- crates/doctor/src/agents.rs | 21 ++++++++++++++++++--- crates/doctor/src/lib.rs | 2 +- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/crates/doctor/src/agents.rs b/crates/doctor/src/agents.rs index cc95d9401..61026d62a 100644 --- a/crates/doctor/src/agents.rs +++ b/crates/doctor/src/agents.rs @@ -160,7 +160,7 @@ pub fn derive_update_command( ) -> Option { let pkg = package_id?; match install_source? { - InstallSource::Npm => Some(format!("npm install -g {pkg}@latest")), + InstallSource::Npm => Some(derive_npm_update_command(pkg)), InstallSource::Brew => Some(format!("brew upgrade {pkg}")), InstallSource::Cargo => Some(format!("cargo install --force {pkg}")), InstallSource::CurlPipe @@ -171,6 +171,13 @@ pub fn derive_update_command( } } +fn derive_npm_update_command(package_id: &str) -> String { + match package_id { + "@anthropic-ai/claude-code" => format!("npm install -g --force {package_id}@latest"), + pkg => format!("npm install -g {pkg}@latest"), + } +} + /// Append `--registry=` to `command` when a registry override is supplied /// and the command is npm-backed. Non-npm commands (curl-pipe installers, auth /// commands, the git-clonefile fix, …) and the `None` registry case return the @@ -915,11 +922,19 @@ mod tests { } #[test] - fn derive_update_command_npm_emits_at_latest() { + fn derive_update_command_claude_npm_includes_force() { assert_eq!( derive_update_command(Some(&InstallSource::Npm), Some("@anthropic-ai/claude-code"),) .as_deref(), - Some("npm install -g @anthropic-ai/claude-code@latest"), + Some("npm install -g --force @anthropic-ai/claude-code@latest"), + ); + } + + #[test] + fn derive_update_command_npm_emits_at_latest() { + assert_eq!( + derive_update_command(Some(&InstallSource::Npm), Some("amp-acp"),).as_deref(), + Some("npm install -g amp-acp@latest"), ); } diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index 410eb9905..509202c58 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -1285,7 +1285,7 @@ mod tests { ); assert_eq!( readout.update_command.as_deref(), - Some("npm install -g @anthropic-ai/claude-code@latest"), + Some("npm install -g --force @anthropic-ai/claude-code@latest"), ); assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); } From a322d44bd7f596f46226231014e711ebd23d5ca6 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 19:09:22 +1000 Subject: [PATCH 4/7] revert(doctor): remove forced Claude Code npm updates Restore doctor-derived npm update commands to the standard npm install -g @latest form instead of special-casing Claude Code with --force. The Homebrew cask source/channel fixes remain intact, so brew-installed Claude Code updates continue to derive brew upgrade commands with the preserved cask token. Signed-off-by: Matt Toohey --- crates/doctor/src/agents.rs | 21 +++------------------ crates/doctor/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/crates/doctor/src/agents.rs b/crates/doctor/src/agents.rs index 61026d62a..cc95d9401 100644 --- a/crates/doctor/src/agents.rs +++ b/crates/doctor/src/agents.rs @@ -160,7 +160,7 @@ pub fn derive_update_command( ) -> Option { let pkg = package_id?; match install_source? { - InstallSource::Npm => Some(derive_npm_update_command(pkg)), + InstallSource::Npm => Some(format!("npm install -g {pkg}@latest")), InstallSource::Brew => Some(format!("brew upgrade {pkg}")), InstallSource::Cargo => Some(format!("cargo install --force {pkg}")), InstallSource::CurlPipe @@ -171,13 +171,6 @@ pub fn derive_update_command( } } -fn derive_npm_update_command(package_id: &str) -> String { - match package_id { - "@anthropic-ai/claude-code" => format!("npm install -g --force {package_id}@latest"), - pkg => format!("npm install -g {pkg}@latest"), - } -} - /// Append `--registry=` to `command` when a registry override is supplied /// and the command is npm-backed. Non-npm commands (curl-pipe installers, auth /// commands, the git-clonefile fix, …) and the `None` registry case return the @@ -922,19 +915,11 @@ mod tests { } #[test] - fn derive_update_command_claude_npm_includes_force() { + fn derive_update_command_npm_emits_at_latest() { assert_eq!( derive_update_command(Some(&InstallSource::Npm), Some("@anthropic-ai/claude-code"),) .as_deref(), - Some("npm install -g --force @anthropic-ai/claude-code@latest"), - ); - } - - #[test] - fn derive_update_command_npm_emits_at_latest() { - assert_eq!( - derive_update_command(Some(&InstallSource::Npm), Some("amp-acp"),).as_deref(), - Some("npm install -g amp-acp@latest"), + Some("npm install -g @anthropic-ai/claude-code@latest"), ); } diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index 509202c58..410eb9905 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -1285,7 +1285,7 @@ mod tests { ); assert_eq!( readout.update_command.as_deref(), - Some("npm install -g --force @anthropic-ai/claude-code@latest"), + Some("npm install -g @anthropic-ai/claude-code@latest"), ); assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); } From a2c48a6b21e8f0ddb58abf728f3c0633bb61a596 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 19:18:40 +1000 Subject: [PATCH 5/7] fix(doctor): prefer Claude cask source over npm globals Signed-off-by: Matt Toohey --- crates/doctor/src/resolve.rs | 116 +++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/crates/doctor/src/resolve.rs b/crates/doctor/src/resolve.rs index 9e39f9d53..1e00fa26b 100644 --- a/crates/doctor/src/resolve.rs +++ b/crates/doctor/src/resolve.rs @@ -406,6 +406,39 @@ fn fingerprint_curl_pipe(path: &Path, home: &Path) -> bool { false } +/// Whether the active binary path is owned by a Homebrew cask. +/// +/// Cask binaries commonly appear as `/bin/` symlinks into +/// `/Caskroom///`. Treat the Caskroom path +/// (or the bin symlink's immediate target) as the owning install source before +/// following the full canonical chain for npm detection: cask wrappers may +/// include node-based internals, but the user updates the active binary through +/// `brew upgrade`, not an unrelated or inactive global npm package. +fn looks_like_homebrew_cask(path: &Path) -> bool { + if path_has_caskroom_component(path) { + return true; + } + + let Ok(target) = std::fs::read_link(path) else { + return false; + }; + let target = if target.is_absolute() { + target + } else { + path.parent() + .map(|parent| parent.join(&target)) + .unwrap_or(target) + }; + + path_has_caskroom_component(&target) +} + +fn path_has_caskroom_component(path: &Path) -> bool { + path.components() + .filter_map(|component| component.as_os_str().to_str()) + .any(|component| component.eq_ignore_ascii_case("Caskroom")) +} + /// Whether the canonicalized binary path lives inside a `node_modules/` /// directory — a clean positive signal for `npm install -g`, regardless of /// which node distribution (brew-shipped, nvm, fnm, asdf, mise) hosts the npm @@ -426,6 +459,14 @@ fn looks_like_npm_global(path: &Path) -> bool { /// Testable inner: same logic as [`detect_install_source`] but takes the home /// directory as a parameter so unit tests can inject a fixed value. fn detect_install_source_with_home(path: &Path, home: Option<&Path>) -> InstallSource { + // Homebrew cask ownership beats npm internals. In a mixed Claude install, + // the active `/opt/homebrew/bin/claude` can be a cask symlink while an + // unrelated npm global package also exists; update planning must follow + // the active Caskroom binary back to Brew. + if looks_like_homebrew_cask(path) { + return InstallSource::Brew; + } + // npm global install (any node distribution). Checked first: the bin entry // is a symlink into `node_modules/`, which may live under a brew prefix // (`npm config get prefix = /opt/homebrew`), so this must win over the @@ -812,6 +853,81 @@ mod tests { let _ = fs::remove_dir_all(&home); } + #[test] + fn homebrew_caskroom_symlink_classifies_as_brew_with_inactive_npm_package() { + #[cfg(unix)] + { + let root = + std::env::temp_dir().join(format!("doctor-cask-mixed-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + + let cask_bin = root.join("Caskroom/claude-code/2.1.152/claude"); + fs::create_dir_all(cask_bin.parent().unwrap()).unwrap(); + File::create(&cask_bin).unwrap(); + fs::create_dir_all(root.join("bin")).unwrap(); + let active = root.join("bin/claude"); + std::os::unix::fs::symlink(&cask_bin, &active).unwrap(); + + // Unrelated global npm package exists under nvm, but it is not the + // active/resolved `claude` binary. + let home = root.join("home"); + let npm_pkg = home + .join(".nvm/versions/node/v23.7.0/lib/node_modules/@anthropic-ai/claude-code/cli"); + fs::create_dir_all(&npm_pkg).unwrap(); + File::create(npm_pkg.join("claude.js")).unwrap(); + fs::create_dir_all(home.join(".nvm/versions/node/v23.7.0/bin")).unwrap(); + std::os::unix::fs::symlink( + npm_pkg.join("claude.js"), + home.join(".nvm/versions/node/v23.7.0/bin/claude"), + ) + .unwrap(); + + assert!(looks_like_homebrew_cask(&active)); + assert_eq!( + detect_install_source_with_home(&active, Some(home.as_path())), + InstallSource::Brew, + ); + + let _ = fs::remove_dir_all(&root); + } + } + + #[test] + fn homebrew_caskroom_ownership_wins_over_node_modules_canonical_target() { + #[cfg(unix)] + { + let root = std::env::temp_dir() + .join(format!("doctor-cask-node-modules-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + + let home = root.join("home"); + let npm_pkg = home + .join(".nvm/versions/node/v23.7.0/lib/node_modules/@anthropic-ai/claude-code/cli"); + fs::create_dir_all(&npm_pkg).unwrap(); + let npm_entry = npm_pkg.join("claude.js"); + File::create(&npm_entry).unwrap(); + + let cask_bin = root.join("Caskroom/claude-code/2.1.152/claude"); + fs::create_dir_all(cask_bin.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&npm_entry, &cask_bin).unwrap(); + fs::create_dir_all(root.join("bin")).unwrap(); + let active = root.join("bin/claude"); + std::os::unix::fs::symlink(&cask_bin, &active).unwrap(); + + assert!(looks_like_homebrew_cask(&active)); + assert!( + looks_like_npm_global(&active), + "canonical node_modules target should not override Caskroom ownership", + ); + assert_eq!( + detect_install_source_with_home(&active, Some(home.as_path())), + InstallSource::Brew, + ); + + let _ = fs::remove_dir_all(&root); + } + } + #[test] fn non_node_modules_path_falls_through_to_existing_detection() { // A plain binary (no symlink, no node_modules) is not npm; the new layer From 989f29de59f51b438cefe6820ba8efc16bfc063a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 20:37:32 +1000 Subject: [PATCH 6/7] fix(doctor): preserve Claude cask token through symlink Check the immediate binary symlink target before canonicalizing so claude-code@latest cask installs that point onward into node_modules still derive brew update commands for the owning cask token. Signed-off-by: Matt Toohey --- crates/doctor/src/package_ids.rs | 54 +++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/doctor/src/package_ids.rs b/crates/doctor/src/package_ids.rs index a388ce884..4cd1c8b26 100644 --- a/crates/doctor/src/package_ids.rs +++ b/crates/doctor/src/package_ids.rs @@ -277,13 +277,34 @@ fn path_package_id_override( } fn claude_code_cask_token_from_path(path: &Path) -> Option<&'static str> { + if let Some(token) = claude_code_cask_token_from_caskroom_path(path) { + return Some(token); + } + + if let Some(target) = immediate_symlink_target(path) { + if let Some(token) = claude_code_cask_token_from_caskroom_path(&target) { + return Some(token); + } + } + if let Ok(canonical) = path.canonicalize() { if let Some(token) = claude_code_cask_token_from_caskroom_path(&canonical) { return Some(token); } } - claude_code_cask_token_from_caskroom_path(path) + None +} + +fn immediate_symlink_target(path: &Path) -> Option { + let target = std::fs::read_link(path).ok()?; + Some(if target.is_absolute() { + target + } else { + path.parent() + .map(|parent| parent.join(&target)) + .unwrap_or(target) + }) } fn claude_code_cask_token_from_caskroom_path(path: &Path) -> Option<&'static str> { @@ -427,6 +448,37 @@ mod tests { let _ = fs::remove_dir_all(root); } + #[cfg(unix)] + #[test] + fn claude_main_brew_preserves_latest_cask_token_from_immediate_symlink_target() { + let root = scratch_dir("claude-cask-immediate-symlink"); + let npm_entry = root.join( + "home/.nvm/versions/node/v23.7.0/lib/node_modules/@anthropic-ai/claude-code/cli/claude.js", + ); + fs::create_dir_all(npm_entry.parent().unwrap()).unwrap(); + fs::write(&npm_entry, "#!/usr/bin/env node\n").unwrap(); + + let cask_bin = root.join("Caskroom/claude-code@latest/2.1.153/claude"); + fs::create_dir_all(cask_bin.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&npm_entry, &cask_bin).unwrap(); + + fs::create_dir_all(root.join("bin")).unwrap(); + let active = root.join("bin/claude"); + std::os::unix::fs::symlink(&cask_bin, &active).unwrap(); + + assert_eq!( + lookup_package_id_for_binary( + "ai-agent-claude", + InstallSource::Brew, + Role::Main, + Some(&active), + ), + Some(("claude-code@latest".to_string(), LatestSource::Brew)), + ); + + let _ = fs::remove_dir_all(root); + } + #[test] fn claude_main_brew_ignores_unallowlisted_caskroom_token() { let path = Path::new("/opt/homebrew/Caskroom/not-claude/1.0.0/claude"); From 86601c446663316dda062de6caef08b819977d7d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 3 Jul 2026 21:02:01 +1000 Subject: [PATCH 7/7] fix(doctor): preserve snapshot PATH for fixes Signed-off-by: Matt Toohey --- crates/doctor/src/lib.rs | 139 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 132 insertions(+), 7 deletions(-) diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index 410eb9905..f4b7992da 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -34,6 +34,8 @@ use package_ids::{lookup_package_id_for_binary, LatestSource, Role}; use resolve::resolve_binary_with_diagnostics; use types::{InstallSource, ResolvedBinary}; +const SNAPSHOT_PATH_ENV: &str = "__BUILDERBOT_DOCTOR_SNAPSHOT_PATH"; + /// Fallback check returned when a spawn_blocking task panics. fn empty_check(id: &str, label: &str) -> DoctorCheck { DoctorCheck { @@ -811,17 +813,19 @@ enum StreamLine { /// caller env snapshot, `path_prefix` keeps the legacy behavior of prepending /// resolved binary dirs plus conservative fallbacks. With a snapshot, doctor /// preserves the snapshot environment and appends missing prefix dirs to its -/// `PATH` so auth probes can still find resolved binaries without replacing -/// the caller's environment. +/// `PATH` so auth probes can still find resolved binaries. Because login shell +/// startup can rewrite `PATH` after process spawn, snapshot callers also carry +/// the merged path in an internal env var and restore it inside the `-c` +/// payload immediately before running the requested command. fn build_shell_command( command: &str, path_prefix: &[PathBuf], env: Option<&DoctorEnv>, ) -> std::process::Command { - let (shell, args) = if std::path::Path::new("/bin/zsh").exists() { - ("/bin/zsh", vec!["-l", "-c", command]) + let shell = if std::path::Path::new("/bin/zsh").exists() { + "/bin/zsh" } else { - ("/bin/bash", vec!["-l", "-c", command]) + "/bin/bash" }; let home = env .and_then(|e| e.get("HOME").map(str::to_string)) @@ -832,8 +836,9 @@ fn build_shell_command( .or_else(|| std::env::var("USER").ok()) .unwrap_or_default(); + let mut shell_command = command.to_string(); let mut cmd = std::process::Command::new(shell); - cmd.args(&args).env_clear(); + cmd.env_clear(); if let Some(env) = env { for (key, value) in &env.vars { @@ -847,7 +852,9 @@ fn build_shell_command( } cmd.env("TERM", "xterm-256color").current_dir(&home); if let Some(path) = merged_snapshot_path(env.get("PATH"), path_prefix) { - cmd.env("PATH", path); + cmd.env("PATH", &path); + cmd.env(SNAPSHOT_PATH_ENV, path); + shell_command = command_with_snapshot_path_restore(command); } } else { cmd.env("HOME", &home) @@ -860,9 +867,17 @@ fn build_shell_command( } } + cmd.arg("-l").arg("-c").arg(shell_command); cmd } +fn command_with_snapshot_path_restore(command: &str) -> String { + let name = SNAPSHOT_PATH_ENV; + format!( + "if [ \"${{{name}+x}}\" = x ]; then PATH=\"${{{name}}}\"; export PATH; unset {name}; fi\n{command}", + ) +} + fn legacy_prefixed_path(path_prefix: &[PathBuf]) -> String { let mut seen: std::collections::HashSet = std::collections::HashSet::new(); let mut path = String::new(); @@ -1087,6 +1102,19 @@ mod tests { } } + fn shell_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\\''")) + } + + fn write_login_path_rewrite_profiles(home: &Path, path: &Path) { + let profile = format!( + "export PATH={}\n", + shell_quote(&format!("{}:/usr/bin:/bin", path.to_string_lossy())), + ); + std::fs::write(home.join(".zprofile"), &profile).unwrap(); + std::fs::write(home.join(".bash_profile"), profile).unwrap(); + } + #[test] fn timeout_diagnostic_ids_are_collision_safe() { let checks = timeout_diagnostic_checks(vec![ @@ -1263,6 +1291,41 @@ mod tests { } } + #[tokio::test] + async fn exec_command_with_env_snapshot_restores_path_after_login_shell_rewrite() { + let tmp = unique_tmp_dir("auth-env-path-login-rewrite"); + let snapshot_bin = tmp.join("nvm/bin"); + let login_rewrite_bin = tmp.join("homebrew/bin"); + let script_name = "doctor-auth-path-probe"; + write_executable(&snapshot_bin.join(script_name), "#!/bin/sh\nexit 0\n"); + write_executable(&login_rewrite_bin.join(script_name), "#!/bin/sh\nexit 42\n"); + write_login_path_rewrite_profiles(&tmp, &login_rewrite_bin); + + let env = DoctorEnv::new(vec![ + ( + "PATH".to_string(), + format!("{}:/usr/bin:/bin", snapshot_bin.to_string_lossy()), + ), + ("HOME".to_string(), tmp.to_string_lossy().to_string()), + ("USER".to_string(), "doctor-test".to_string()), + ("ZDOTDIR".to_string(), tmp.to_string_lossy().to_string()), + ]); + + let outcome = tokio::task::spawn_blocking(move || { + execute_command_with_path_prefix_with_env(script_name, &[], Some(&env)) + }) + .await + .unwrap(); + + let _ = std::fs::remove_dir_all(&tmp); + match outcome { + ExecOutcome::Ok => {} + other => { + panic!("expected snapshot PATH binary to beat login profile rewrite; got {other:?}") + } + } + } + /// `apply_freshness` derives a source-aware update command for a Main slot /// when the npm-installed agent has an actionable update. #[tokio::test] @@ -1508,6 +1571,68 @@ mod tests { ); } + #[tokio::test] + async fn execute_fix_streaming_update_restores_snapshot_path_after_login_shell_rewrite() { + let tmp = unique_tmp_dir("fix-update-env-path-login-rewrite"); + let snapshot_bin = tmp.join("nvm/bin"); + let login_rewrite_bin = tmp.join("homebrew/bin"); + write_executable( + &snapshot_bin.join("npm"), + "#!/bin/sh\n\ + echo \"snapshot-npm $*\"\n", + ); + write_executable( + &login_rewrite_bin.join("npm"), + "#!/bin/sh\n\ + echo \"homebrew-npm $*\"\n\ + exit 42\n", + ); + write_login_path_rewrite_profiles(&tmp, &login_rewrite_bin); + + let env = DoctorEnv::new(vec![ + ( + "PATH".to_string(), + format!("{}:/usr/bin:/bin", snapshot_bin.to_string_lossy()), + ), + ("HOME".to_string(), tmp.to_string_lossy().to_string()), + ("USER".to_string(), "doctor-test".to_string()), + ("ZDOTDIR".to_string(), tmp.to_string_lossy().to_string()), + ]); + let lines: Arc>> = Arc::new(Mutex::new(Vec::new())); + let lines_clone = lines.clone(); + let command = "npm install -g @anthropic-ai/claude-code@latest"; + + let result = execute_fix_streaming_with_env_options( + "ai-agent-claude".to_string(), + FixType::UpdateMain, + ExecuteFixOptions { + command_override: Some(command.to_string()), + npm_registry: None, + env: Some(env), + }, + move |line| { + lines_clone.lock().unwrap().push(line.to_string()); + }, + ) + .await; + + let captured = lines.lock().unwrap().clone(); + let _ = std::fs::remove_dir_all(&tmp); + assert!(result.is_ok(), "snapshot npm update failed: {result:?}"); + assert!( + captured + .iter() + .any(|line| line == "snapshot-npm install -g @anthropic-ai/claude-code@latest"), + "expected update to run through snapshot npm; captured: {captured:?}", + ); + assert!( + captured + .iter() + .all(|line| !line.starts_with("homebrew-npm")), + "login profile PATH rewrite should not select homebrew npm; captured: {captured:?}", + ); + } + /// Without a `command_override`, `UpdateMain` has no static recipe and /// must surface as the standard "Unknown check / fix type" error. #[tokio::test]