diff --git a/apps/staged/scripts/update-acp-tools-lock.mjs b/apps/staged/scripts/update-acp-tools-lock.mjs index c17fa02b..c0d3c30a 100755 --- a/apps/staged/scripts/update-acp-tools-lock.mjs +++ b/apps/staged/scripts/update-acp-tools-lock.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node import { execFile } from "node:child_process"; -import { mkdir, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import process from "node:process"; import { promisify } from "node:util"; @@ -20,6 +21,16 @@ const SUPPORTED_TARGETS = [ // Zed package. const CODEX_ACP_PACKAGE = "@agentclientprotocol/codex-acp"; +// `passthroughArgs` is the bridge's CLI-passthrough invocation that prints the +// vendored harness CLI's version. Doctor probes auth (and, for bundled +// installs, freshness) through these same passthrough subcommands +// (`claude-agent-acp --cli …`, `codex-acp cli …`), so a bridge release that +// drops or renames them must fail the smoke check here instead of silently +// breaking every doctor probe in the field. +// +// codex-acp uses `-V` because its entrypoint intercepts a literal `--version` +// anywhere in argv and prints the bridge's own version; only codex's clap +// short flag reaches the vendored binary. const TOOL_SPECS = [ { id: "claude-acp", @@ -28,6 +39,7 @@ const TOOL_SPECS = [ dependencyPackage: "@anthropic-ai/claude-agent-sdk", nativePackageKey: "claudeAgentSdk", includeClaudeCodeVersion: true, + passthroughArgs: ["--cli", "--version"], }, { id: "codex-acp", @@ -35,6 +47,7 @@ const TOOL_SPECS = [ package: CODEX_ACP_PACKAGE, dependencyPackage: "@openai/codex", nativePackageKey: "openaiCodex", + passthroughArgs: ["cli", "-V"], }, ]; @@ -95,13 +108,19 @@ const npmViewCache = new Map(); const execFileAsync = promisify(execFile); function usage() { - console.log(`Usage: scripts/update-acp-tools-lock.mjs [--target ]... [--lock-file ] + console.log(`Usage: scripts/update-acp-tools-lock.mjs [--target ]... [--lock-file ] [--skip-smoke] Queries npm for the latest release of each supported ACP bridge tool and writes acp-tools.lock.json. Fails loudly when a package or one of its per-target native dependencies cannot be resolved — never silently pins an older version. +Before writing the lock, each tool's CLI passthrough (the subcommand doctor's +auth/version probes rely on) is smoke-checked against the resolved release by +installing it into a temp prefix and running it on the current platform. +--skip-smoke bypasses this (e.g. on hosts that cannot execute the vendored +binaries). + Supported targets: ${SUPPORTED_TARGETS.join("\n ")} @@ -114,6 +133,7 @@ Environment: function parseArgs(argv) { const targets = []; let lockFile = process.env.ACP_TOOLS_LOCK_FILE ?? defaultLockFile; + let skipSmoke = false; for (let i = 0; i < argv.length; i += 1) { const arg = argv[i]; if (arg === "-h" || arg === "--help") { @@ -132,6 +152,10 @@ function parseArgs(argv) { lockFile = path.resolve(value); continue; } + if (arg === "--skip-smoke") { + skipSmoke = true; + continue; + } throw new Error(`Unknown argument: ${arg}`); } @@ -141,7 +165,7 @@ function parseArgs(argv) { throw new Error(`Unsupported target '${target}'`); } } - return { targets: selectedTargets, lockFile }; + return { targets: selectedTargets, lockFile, skipSmoke }; } async function npmView(spec, fields) { @@ -341,8 +365,67 @@ async function lockToolForTarget(tool, target) { }; } +// Install the resolved release into a temp npm prefix and run the bridge's +// CLI passthrough on the current platform, requiring exit 0 and the locked +// vendored-harness version in the output. Guards the interface doctor's +// probes depend on: the passthrough flags are bridge behavior, not a +// documented stable contract, so a release that breaks them — or one whose +// entrypoint starts answering the probe with the bridge's own version +// instead of the vendored CLI's — must fail here before it gets pinned. +async function smokeCheckPassthrough(tool, locked) { + const invocation = `${tool.binary} ${tool.passthroughArgs.join(" ")}`; + const version = locked.version; + // The version doctor surfaces: the vendored harness CLI's, not the bridge's. + const harnessVersion = locked.claudeCodeVersion ?? locked.dependencyVersion; + const prefix = await mkdtemp(path.join(os.tmpdir(), "acp-tools-smoke-")); + try { + await execFileAsync( + "npm", + [ + "install", + "--prefix", + prefix, + "--no-fund", + "--no-audit", + "--loglevel=error", + `${tool.package}@${version}`, + ], + { maxBuffer: 10 * 1024 * 1024, timeout: 10 * 60 * 1000 }, + ); + const binary = path.join(prefix, "node_modules", ".bin", tool.binary); + let output; + try { + const { stdout, stderr } = await execFileAsync( + binary, + tool.passthroughArgs, + { timeout: 60 * 1000 }, + ); + output = `${stdout}\n${stderr}`.trim(); + } catch (error) { + throw new Error( + `Passthrough smoke check failed for ${tool.package}@${version}: ` + + `\`${invocation}\` did not exit 0 — doctor's auth/version probes ` + + `would break on this release. ${error instanceof Error ? error.message : String(error)}`, + ); + } + if (!output.includes(harnessVersion)) { + throw new Error( + `Passthrough smoke check failed for ${tool.package}@${version}: ` + + `\`${invocation}\` did not print the vendored ` + + `${tool.dependencyPackage} version ${harnessVersion}: ` + + `${output || "(empty output)"}. If the output shows the bridge's ` + + `own version, its entrypoint is intercepting the probe before the ` + + `passthrough dispatch and doctor's freshness readout would be wrong.`, + ); + } + console.log(`Smoke-checked \`${invocation}\`: ${output.split("\n")[0]}`); + } finally { + await rm(prefix, { recursive: true, force: true }); + } +} + async function main() { - const { targets, lockFile } = parseArgs(process.argv.slice(2)); + const { targets, lockFile, skipSmoke } = parseArgs(process.argv.slice(2)); const tools = []; for (const tool of TOOL_SPECS) { for (const target of targets) { @@ -352,6 +435,16 @@ async function main() { tools.sort((left, right) => `${left.id}:${left.target}`.localeCompare(`${right.id}:${right.target}`), ); + if (skipSmoke) { + console.log("Skipping passthrough smoke checks (--skip-smoke)"); + } else { + for (const tool of TOOL_SPECS) { + const locked = tools.find((entry) => entry.id === tool.id); + if (locked) { + await smokeCheckPassthrough(tool, locked); + } + } + } await mkdir(path.dirname(lockFile), { recursive: true }); await writeFile(lockFile, `${JSON.stringify({ tools }, null, 2)}\n`); console.log(`Updated ${path.relative(process.cwd(), lockFile)}`); diff --git a/apps/staged/src-tauri/src/doctor.rs b/apps/staged/src-tauri/src/doctor.rs index af9a96ef..a4620ff7 100644 --- a/apps/staged/src-tauri/src/doctor.rs +++ b/apps/staged/src-tauri/src/doctor.rs @@ -26,7 +26,11 @@ async fn doctor_env_vars(bundled_dir: Option<&Path>) -> Vec<(String, String)> { env_vars } -fn run_checks_options(check_freshness: bool, env_vars: Vec<(String, String)>) -> RunChecksOptions { +fn run_checks_options( + check_freshness: bool, + env_vars: Vec<(String, String)>, + bundled_dir: Option, +) -> RunChecksOptions { RunChecksOptions { check_freshness, offline: false, @@ -34,6 +38,11 @@ fn run_checks_options(check_freshness: bool, env_vars: Vec<(String, String)>) -> // from public npm/brew/crates.io, not an internal mirror. npm_registry: None, env: None, + // Doctor labels binaries resolved from this dir as bundled (install + // source + readout flag) and suppresses registry update fixes for + // them — versions are pinned by acp-tools.lock.json and ship with + // Staged updates. + bundled_tools_dir: bundled_dir, } .with_env_snapshot(env_vars) } @@ -74,41 +83,26 @@ pub async fn run_doctor_freshness(app_handle: tauri::AppHandle) -> DoctorReport } /// Run the doctor crate's checks plus Staged-local ones (currently the -/// bundled ACP Node.js runtime check) over one shared env snapshot. +/// bundled ACP Node.js runtime check) over one shared env snapshot. Bundled +/// readouts are labeled by the doctor crate itself via +/// `RunChecksOptions::bundled_tools_dir`. async fn run_doctor_report(app_handle: &tauri::AppHandle, check_freshness: bool) -> DoctorReport { let bundled_dir = crate::acp_tools::resolve_bundled_acp_tools_dir(app_handle); let env_vars = doctor_env_vars(bundled_dir.as_deref()).await; let (mut report, node_runtime) = tokio::join!( - doctor::run_checks_with_options(run_checks_options(check_freshness, env_vars.clone())), + doctor::run_checks_with_options(run_checks_options( + check_freshness, + env_vars.clone(), + bundled_dir.clone(), + )), run_node_runtime_check(&env_vars, bundled_dir.as_deref()), ); if let Some(check) = node_runtime { report.checks.push(check); } - mark_bundled_bridges(&mut report, bundled_dir.as_deref()); report } -/// Stamp `bundled` on bridge readouts whose resolved path lives inside the -/// app's bundled ACP tools dir. The doctor crate resolves those bridges via -/// the PATH prefix `apply_bundled_tools_env` injects, so a prefix match -/// against the same dir identifies them; the UI then presents them as -/// bundled instead of showing an app-internal resource path. -fn mark_bundled_bridges(report: &mut DoctorReport, bundled_dir: Option<&Path>) { - let Some(dir) = bundled_dir else { return }; - for check in &mut report.checks { - let in_bundled_dir = check - .bridge_path - .as_deref() - .is_some_and(|p| Path::new(p).starts_with(dir)); - if in_bundled_dir { - if let Some(bridge) = check.bridge.as_mut() { - bridge.bundled = Some(true); - } - } - } -} - /// Run a fix for a doctor check, identified by check ID and fix type. /// /// The actual shell command is looked up from the static check definitions — @@ -146,7 +140,8 @@ pub async fn run_doctor_update( ) -> Result<(), String> { let bundled_dir = crate::acp_tools::resolve_bundled_acp_tools_dir(&app_handle); let env_vars = doctor_env_vars(bundled_dir.as_deref()).await; - let expected = expected_update_command(&check_id, &fix_type, env_vars.clone()).await?; + let expected = + expected_update_command(&check_id, &fix_type, env_vars.clone(), bundled_dir).await?; if expected != command { return Err(format!( "Update command mismatch for {check_id}: refusing to run a command \ @@ -166,13 +161,17 @@ pub async fn run_doctor_update( } /// Re-run freshness and return the authoritative update command for the given -/// check + slot, or an error if no actionable update is derivable. +/// check + slot, or an error if no actionable update is derivable. Passes the +/// bundled dir through so bundled readouts derive no update command here, +/// exactly as in the report the UI rendered. async fn expected_update_command( check_id: &str, fix_type: &FixType, env_vars: Vec<(String, String)>, + bundled_dir: Option, ) -> Result { - let report = doctor::run_checks_with_options(run_checks_options(true, env_vars)).await; + let report = + doctor::run_checks_with_options(run_checks_options(true, env_vars, bundled_dir)).await; let check = report .checks @@ -654,51 +653,15 @@ mod tests { assert_eq!(parse_node_major(""), None); } - fn agent_check_with_bridge(id: &str, bridge_path: &str) -> DoctorCheck { - let mut check = - node_runtime_doctor_check(CheckStatus::Pass, "Installed".to_string(), None, None); - check.id = id.to_string(); - check.bridge_path = Some(bridge_path.to_string()); - check.bridge = Some(AgentVersionInfo::default()); - check - } - - #[test] - fn mark_bundled_bridges_stamps_only_bridges_inside_bundled_dir() { - let mut report = DoctorReport { - checks: vec![ - agent_check_with_bridge( - "ai-agent-claude", - "/bundle/resources/acp/bin/claude-agent-acp", - ), - agent_check_with_bridge("ai-agent-pi", "/Users/me/.npm-global/bin/pi-acp"), - ], - }; - - mark_bundled_bridges(&mut report, Some(Path::new("/bundle/resources/acp/bin"))); - - assert_eq!( - report.checks[0].bridge.as_ref().unwrap().bundled, - Some(true), - ); - assert_eq!( - report.checks[1].bridge.as_ref().unwrap().bundled, - None, - "a user-installed bridge outside the bundled dir must not be stamped", - ); - } - + /// Bundled-readout labeling lives in the doctor crate now; Staged's job is + /// only to hand the resolved bundled dir into the run options. #[test] - fn mark_bundled_bridges_without_bundled_dir_is_a_no_op() { - let mut report = DoctorReport { - checks: vec![agent_check_with_bridge( - "ai-agent-claude", - "/bundle/resources/acp/bin/claude-agent-acp", - )], - }; - - mark_bundled_bridges(&mut report, None); + fn run_checks_options_carries_bundled_tools_dir() { + let dir = PathBuf::from("/bundle/resources/acp/bin"); + let opts = run_checks_options(false, Vec::new(), Some(dir.clone())); + assert_eq!(opts.bundled_tools_dir, Some(dir)); - assert_eq!(report.checks[0].bridge.as_ref().unwrap().bundled, None); + let opts = run_checks_options(false, Vec::new(), None); + assert!(opts.bundled_tools_dir.is_none()); } } diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 1d40f8dd..15577707 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -1294,7 +1294,7 @@ export function squashCommits(branchId: string, provider?: string): Promise {check.message} {#if check.path} - {check.path} + {#if check.main?.bundled} + Bundled with Staged + {:else} + {check.path} + {/if} {@render updateBadge('main', check.main)} {/if} {#if check.bridgePath} {#if check.bridge?.bundled} - ACP bundled with Staged + Bundled with Staged {:else} {check.bridgePath} {/if} diff --git a/crates/doctor/src/agents.rs b/crates/doctor/src/agents.rs index dd6925e3..1ed3b11a 100644 --- a/crates/doctor/src/agents.rs +++ b/crates/doctor/src/agents.rs @@ -38,11 +38,18 @@ pub struct AgentCheckInfo { pub auth_status_command: Option<&'static str>, /// Per-tool override declaring this agent's install source when generic path /// and filesystem-fingerprint heuristics fall through to - /// [`InstallSource::Unknown`]. Lets a curl/native-only agent (Claude native, - /// Cursor, Amp) report its true source instead of `Unknown`. Applied only on + /// [`InstallSource::Unknown`]. Lets a curl/native-only agent (Cursor, Amp) + /// report its true source instead of `Unknown`. Applied only on /// `Unknown`; a positively-detected source (Brew/Npm/…) and a missing binary /// (`None`) are left untouched. pub install_source_override: Option, + /// CLI args that print the vendored agent CLI's version through the ACP + /// bridge's passthrough subcommand (e.g. `--cli --version` for + /// `claude-agent-acp`). Used by the freshness pass for + /// [`InstallSource::Bundled`] readouts, where the bridge version is pinned + /// by the embedding app's lock and the version worth surfacing is the + /// vendored harness CLI's (e.g. Claude Code 2.1.x). + pub bundled_version_args: Option<&'static [&'static str]>, } /// All AI agents we check for individually. @@ -59,34 +66,45 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ auth_command: None, auth_status_command: None, install_source_override: None, + bundled_version_args: None, }, + // The claude-agent-acp bridge vendors the complete Claude Code CLI and + // forwards `--cli ` to it, sharing the user's credential store + // (macOS Keychain / ~/.claude). The bridge is therefore the only binary + // this check needs — no separate `claude` main CLI is probed. AgentCheckInfo { id: "ai-agent-claude", label: "Claude Code", commands: &["claude-agent-acp"], - main_command: Some("claude"), - install_url: Some("https://code.claude.com/docs/en/overview"), - install_command: Some("curl -fsSL https://claude.ai/install.sh | bash"), - bridge_install_url: Some("https://github.com/zed-industries/claude-agent-acp#installation"), - bridge_install_command: Some("npm install -g @agentclientprotocol/claude-agent-acp"), - auth_command: Some("claude auth login"), - auth_status_command: Some("claude auth status"), - // Main `claude` is a curl/native install; the bridge is npm and is - // detected positively, so this only takes effect on the main-only path. - install_source_override: Some(InstallSource::CurlPipe), + main_command: None, + install_url: Some("https://www.npmjs.com/package/@agentclientprotocol/claude-agent-acp"), + install_command: Some("npm install -g @agentclientprotocol/claude-agent-acp"), + bridge_install_url: None, + bridge_install_command: None, + auth_command: Some("claude-agent-acp --cli auth login"), + auth_status_command: Some("claude-agent-acp --cli auth status"), + install_source_override: None, + bundled_version_args: Some(&["--cli", "--version"]), }, + // The codex-acp bridge vendors the full `codex` binary and forwards + // `cli ` to it, sharing the user's ~/.codex/auth.json — same + // single-binary shape as Claude above. AgentCheckInfo { id: "ai-agent-codex", label: "Codex", commands: &["codex-acp"], - main_command: Some("codex"), - install_url: Some("https://github.com/openai/codex#quickstart"), - install_command: Some("brew install --cask codex"), - bridge_install_url: Some("https://github.com/zed-industries/codex-acp#installation"), - bridge_install_command: Some("npm install -g @zed-industries/codex-acp"), - auth_command: Some("codex login"), - auth_status_command: Some("codex login status"), + main_command: None, + install_url: Some("https://www.npmjs.com/package/@agentclientprotocol/codex-acp"), + install_command: Some("npm install -g @agentclientprotocol/codex-acp"), + bridge_install_url: None, + bridge_install_command: None, + auth_command: Some("codex-acp cli login"), + auth_status_command: Some("codex-acp cli login status"), install_source_override: None, + // `-V`, not `--version`: the bridge entrypoint intercepts a literal + // `--version` anywhere in argv and prints its own version, so only + // codex's clap short flag reaches the vendored binary. + bundled_version_args: Some(&["cli", "-V"]), }, AgentCheckInfo { id: "ai-agent-pi", @@ -100,6 +118,7 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ auth_command: None, auth_status_command: None, install_source_override: None, + bundled_version_args: None, }, AgentCheckInfo { id: "ai-agent-amp", @@ -114,6 +133,7 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ auth_status_command: Some("amp usage"), // Main `amp` curl installer; bridge `amp-acp` is npm (detected positively). install_source_override: Some(InstallSource::CurlPipe), + bundled_version_args: None, }, AgentCheckInfo { id: "ai-agent-copilot", @@ -127,6 +147,7 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ auth_command: Some("copilot login"), auth_status_command: None, install_source_override: None, + bundled_version_args: None, }, AgentCheckInfo { id: "ai-agent-cursor", @@ -142,13 +163,34 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ // Cursor has only a curl/native installer and no ACP bridge, so the // resolved binary is the curl install — the override's primary use case. install_source_override: Some(InstallSource::CurlPipe), + bundled_version_args: None, }, ]; +/// Version-probe args for a bundled agent readout: the bridge-passthrough +/// invocation that prints the vendored harness CLI's version (e.g. Claude +/// Code 2.1.x instead of the bridge package's own version). `None` for +/// non-bundled sources — registry installs keep their registry-consistent +/// probes so `update_available` compares like with like — and for agents +/// without a passthrough. +pub(crate) fn bundled_version_probe_args( + check_id: &str, + install_source: Option<&InstallSource>, +) -> Option<&'static [&'static str]> { + if !matches!(install_source, Some(InstallSource::Bundled)) { + return None; + } + AI_AGENT_CHECKS + .iter() + .find(|info| info.id == check_id) + .and_then(|info| info.bundled_version_args) +} + /// Derive a source-aware update command from a readout's install source and -/// package id. Returns `None` for self-updating sources (`CurlPipe`), sources -/// with no canonical update recipe (`Mise`/`Asdf`/`Unknown`/`System`), or when -/// the package id is unknown. +/// package id. Returns `None` for self-updating sources (`CurlPipe`), +/// app-managed installs (`Bundled` — updates ship with the embedding app), +/// sources with no canonical update recipe (`Mise`/`Asdf`/`Unknown`/`System`), +/// or when the package id is unknown. /// /// The caller is responsible for gating on `update_available == Some(true)` — /// this function only knows how to update, not whether to. `apply_npm_registry` @@ -164,6 +206,7 @@ pub fn derive_update_command( InstallSource::Brew => Some(format!("brew upgrade {pkg}")), InstallSource::Cargo => Some(format!("cargo install --force {pkg}")), InstallSource::CurlPipe + | InstallSource::Bundled | InstallSource::Mise | InstallSource::Asdf | InstallSource::Unknown @@ -264,8 +307,11 @@ fn apply_install_source_override( /// `Some` only when the binary was resolved (i.e. a source was detected); the /// version fields stay `None` until the freshness pass fills them in. A binary /// that wasn't found (`None`) yields `None` — no empty readout is fabricated. +/// A `Bundled` source also stamps the readout's `bundled` flag so the UI can +/// label the binary without re-deriving the app-bundle prefix match. fn version_readout(source: Option) -> Option { source.map(|src| AgentVersionInfo { + bundled: (src == InstallSource::Bundled).then_some(true), install_source: Some(src), ..AgentVersionInfo::default() }) @@ -734,14 +780,14 @@ mod tests { /// When only the agent CLI resolves (no bridge), the main readout carries /// the CLI's source — upgraded from `Unknown` via the per-agent override — - /// and the bridge readout is `None`. Claude's main-only path returns before + /// and the bridge readout is `None`. Amp's main-only path returns before /// any auth probe, so this runs without shelling out. #[test] fn agent_check_main_only_applies_override_and_omits_bridge() { let bridge_missing = resolved(None, None); - let main = resolved(Some("/h/.local/bin/claude"), Some(InstallSource::Unknown)); + let main = resolved(Some("/h/.local/bin/amp"), Some(InstallSource::Unknown)); let check = check_single_ai_agent( - agent("ai-agent-claude"), + agent("ai-agent-amp"), true, std::slice::from_ref(&bridge_missing), Some(&main), @@ -759,6 +805,96 @@ mod tests { assert_eq!(check.install_source, Some(InstallSource::CurlPipe)); } + /// A binary resolved from the embedding app's bundled tools dir carries + /// `InstallSource::Bundled` and the readout's `bundled` flag, so the UI can + /// label it without re-deriving the prefix match. Uses Copilot — the same + /// single-binary shape as claude/codex (`main_command: None`, so the + /// readout lands under `main`) but with no auth-status probe, so the check + /// runs without shelling out. + #[test] + fn agent_check_bundled_source_stamps_bundled_readout() { + let bridge = resolved( + Some("/Applications/App.app/resources/acp/bin/copilot"), + Some(InstallSource::Bundled), + ); + let check = check_single_ai_agent( + agent("ai-agent-copilot"), + true, + std::slice::from_ref(&bridge), + None, + None, + None, + ); + + let m = check.main.expect("main readout present"); + assert_eq!(m.install_source, Some(InstallSource::Bundled)); + assert_eq!(m.bundled, Some(true)); + assert_eq!(check.install_source, Some(InstallSource::Bundled)); + } + + /// The claude/codex checks are single-binary: the ACP bridge vendors the + /// full harness CLI, so no separate main command is resolved and all + /// auth/install commands go through the bridge passthrough. + #[test] + fn claude_and_codex_probe_through_bridge_passthrough() { + let claude = agent("ai-agent-claude"); + assert_eq!(claude.main_command, None); + assert_eq!( + claude.auth_status_command, + Some("claude-agent-acp --cli auth status"), + ); + assert_eq!( + claude.auth_command, + Some("claude-agent-acp --cli auth login") + ); + assert_eq!( + claude.install_command, + Some("npm install -g @agentclientprotocol/claude-agent-acp"), + ); + assert_eq!( + claude.bundled_version_args, + Some(&["--cli", "--version"][..]) + ); + + let codex = agent("ai-agent-codex"); + assert_eq!(codex.main_command, None); + assert_eq!( + codex.auth_status_command, + Some("codex-acp cli login status") + ); + assert_eq!(codex.auth_command, Some("codex-acp cli login")); + assert_eq!( + codex.install_command, + Some("npm install -g @agentclientprotocol/codex-acp"), + ); + // `--version` would be swallowed by the bridge's own version handler; + // clap's `-V` passes through to the vendored codex. + assert_eq!(codex.bundled_version_args, Some(&["cli", "-V"][..])); + } + + #[test] + fn bundled_version_probe_args_apply_only_to_bundled_sources() { + assert_eq!( + bundled_version_probe_args("ai-agent-claude", Some(&InstallSource::Bundled)), + Some(&["--cli", "--version"][..]), + ); + assert_eq!( + bundled_version_probe_args("ai-agent-codex", Some(&InstallSource::Bundled)), + Some(&["cli", "-V"][..]), + ); + // Registry installs keep their registry-consistent probes. + assert_eq!( + bundled_version_probe_args("ai-agent-claude", Some(&InstallSource::Npm)), + None, + ); + assert_eq!(bundled_version_probe_args("ai-agent-claude", None), None); + // Bundled agents without a passthrough fall back to the default probe. + assert_eq!( + bundled_version_probe_args("ai-agent-goose", Some(&InstallSource::Bundled)), + None, + ); + } + /// An agent with no separate bridge reports its single binary under `main`, /// leaving `bridge` as `None`. Copilot has an auth command but no /// auth-status command, so it reports NotApplicable without shelling out. @@ -785,18 +921,17 @@ mod tests { let tmp = unique_tmp_dir("auth-env-vs-hermit"); let snapshot_bin = tmp.join("snapshot/bin"); let hermit_bin = tmp.join("hermit/bin"); - let claude = snapshot_bin.join("claude"); let bridge = snapshot_bin.join("claude-agent-acp"); + // The auth probe is the bridge passthrough (`--cli auth status`). write_executable( - &claude, + &bridge, "#!/bin/sh\n\ test \"$DOCTOR_AGENT_ENV\" = snapshot || exit 43\n\ - test \"$1\" = auth || exit 44\n\ - test \"$2\" = status || exit 45\n\ + test \"$1\" = --cli || exit 44\n\ + test \"$2\" = auth || exit 45\n\ + test \"$3\" = status || exit 46\n\ exit 0\n", ); - write_executable(&bridge, "#!/bin/sh\nexit 0\n"); - write_executable(&hermit_bin.join("claude"), "#!/bin/sh\nexit 42\n"); write_executable(&hermit_bin.join("claude-agent-acp"), "#!/bin/sh\nexit 42\n"); write_login_path_rewrite_profiles(&tmp, &hermit_bin); @@ -805,11 +940,6 @@ mod tests { search_output: "BRIDGE-SNAPSHOT-SEARCH".to_string(), install_source: Some(InstallSource::Npm), }; - let main_resolved = ResolvedBinary { - path: Some(claude.clone()), - search_output: "MAIN-SNAPSHOT-SEARCH".to_string(), - install_source: Some(InstallSource::CurlPipe), - }; let env = DoctorEnv::new(vec![ ("DOCTOR_AGENT_ENV".to_string(), "snapshot".to_string()), ( @@ -825,21 +955,19 @@ mod tests { agent("ai-agent-claude"), true, std::slice::from_ref(&bridge_resolved), - Some(&main_resolved), + None, None, Some(&env), ); assert_eq!(check.status, CheckStatus::Pass); assert_eq!(check.auth_status, Some(AuthStatus::Authenticated)); + // Single-binary shape: the bridge reports under `path`/`main`. assert_eq!( check.path.as_deref(), - Some(claude.to_string_lossy().as_ref()) - ); - assert_eq!( - check.bridge_path.as_deref(), Some(bridge.to_string_lossy().as_ref()) ); + assert!(check.bridge_path.is_none()); let raw = check.raw_output.unwrap_or_default(); assert!( !raw.contains(&hermit_bin.to_string_lossy().to_string()), @@ -869,7 +997,11 @@ mod tests { fn auth_fix_lookup_returns_agent_auth_command() { assert_eq!( lookup_fix_command("ai-agent-claude", &FixType::Auth).as_deref(), - Some("claude auth login"), + Some("claude-agent-acp --cli auth login"), + ); + assert_eq!( + lookup_fix_command("ai-agent-codex", &FixType::Auth).as_deref(), + Some("codex-acp cli login"), ); assert_eq!( lookup_fix_command("ai-agent-copilot", &FixType::Auth).as_deref(), @@ -911,7 +1043,7 @@ mod tests { #[test] fn curl_native_agents_declare_curl_pipe_override() { - for id in ["ai-agent-claude", "ai-agent-amp", "ai-agent-cursor"] { + for id in ["ai-agent-amp", "ai-agent-cursor"] { let info = AI_AGENT_CHECKS.iter().find(|i| i.id == id).unwrap(); assert_eq!( info.install_source_override, @@ -919,12 +1051,11 @@ mod tests { "{id} should declare a CurlPipe override", ); } - // Registry-installed agents declare no override. - let codex = AI_AGENT_CHECKS - .iter() - .find(|i| i.id == "ai-agent-codex") - .unwrap(); - assert_eq!(codex.install_source_override, None); + // Bridge-only and registry-installed agents declare no override. + for id in ["ai-agent-claude", "ai-agent-codex"] { + let info = AI_AGENT_CHECKS.iter().find(|i| i.id == id).unwrap(); + assert_eq!(info.install_source_override, None, "{id}"); + } } #[test] @@ -951,8 +1082,11 @@ mod tests { curl, ); assert_eq!( - apply_npm_registry("claude auth login", Some("https://artifactory/npm")), - "claude auth login", + apply_npm_registry( + "claude-agent-acp --cli auth login", + Some("https://artifactory/npm"), + ), + "claude-agent-acp --cli auth login", ); } @@ -1030,9 +1164,12 @@ mod tests { #[test] 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 @anthropic-ai/claude-code@latest"), + derive_update_command( + Some(&InstallSource::Npm), + Some("@agentclientprotocol/claude-agent-acp"), + ) + .as_deref(), + Some("npm install -g @agentclientprotocol/claude-agent-acp@latest"), ); } @@ -1056,6 +1193,7 @@ mod tests { fn derive_update_command_self_updating_and_opaque_sources_return_none() { for src in [ InstallSource::CurlPipe, + InstallSource::Bundled, InstallSource::Mise, InstallSource::Asdf, InstallSource::Unknown, diff --git a/crates/doctor/src/freshness.rs b/crates/doctor/src/freshness.rs index a0d3277b..7cea4514 100644 --- a/crates/doctor/src/freshness.rs +++ b/crates/doctor/src/freshness.rs @@ -346,14 +346,19 @@ fn installed_version( } } -/// Whether a tool keeps itself up to date and therefore should never raise an -/// "update available" nag. Curl/native installers (Claude native, Cursor, -/// Amp-curl) self-update; those installs are fingerprinted as -/// [`InstallSource::CurlPipe`] (directly or via a per-agent override). Registry -/// installs (brew/npm/cargo) are user-managed and remain actionable, which is -/// why a brew/npm install of the same agent is *not* treated as self-updating. +/// Whether a tool's freshness is managed outside the user's hands and +/// therefore should never raise an "update available" nag. Curl/native +/// installers (Cursor, Amp-curl) self-update; those installs are +/// fingerprinted as [`InstallSource::CurlPipe`] (directly or via a per-agent +/// override). [`InstallSource::Bundled`] binaries are pinned by the embedding +/// app's lock and updated with the app. Registry installs (brew/npm/cargo) +/// are user-managed and remain actionable, which is why a brew/npm install of +/// the same agent is *not* treated as self-updating. pub(crate) fn is_self_updating(source: Option<&InstallSource>) -> bool { - matches!(source, Some(InstallSource::CurlPipe)) + matches!( + source, + Some(InstallSource::CurlPipe | InstallSource::Bundled) + ) } /// Dispatch a latest-version probe per source. @@ -761,8 +766,9 @@ mod tests { } #[test] - fn self_updating_only_for_curl_pipe() { + fn self_updating_only_for_curl_pipe_and_bundled() { assert!(is_self_updating(Some(&InstallSource::CurlPipe))); + assert!(is_self_updating(Some(&InstallSource::Bundled))); assert!(!is_self_updating(Some(&InstallSource::Npm))); assert!(!is_self_updating(Some(&InstallSource::Brew))); assert!(!is_self_updating(None)); @@ -899,48 +905,20 @@ mod tests { #[test] fn package_json_resolves_scoped_package() { let root = scratch_dir("pj-scoped"); - let pkg = root.join("node_modules/@zed-industries/codex-acp"); + let pkg = root.join("node_modules/@agentclientprotocol/codex-acp"); std::fs::create_dir_all(pkg.join("dist")).unwrap(); let index = pkg.join("dist/index.js"); std::fs::write(&index, "// node script\n").unwrap(); std::fs::write( pkg.join("package.json"), - br#"{"name": "@zed-industries/codex-acp", "version": "0.7.1"}"#, + br#"{"name": "@agentclientprotocol/codex-acp", "version": "1.1.2"}"#, ) .unwrap(); assert_eq!( - installed_version_from_package_json(&index, Some("@zed-industries/codex-acp")) + installed_version_from_package_json(&index, Some("@agentclientprotocol/codex-acp")) .as_deref(), - Some("0.7.1"), - ); - } - - /// Exercises the Claude main CLI's new npm package id end-to-end at the - /// freshness layer: when claude is npm-installed under nvm, its main - /// readout walks up to `@anthropic-ai/claude-code`'s `package.json`. - #[test] - fn package_json_resolves_claude_main_npm_layout() { - let root = scratch_dir("pj-claude-main"); - let pkg = root.join("node_modules/@anthropic-ai/claude-code"); - std::fs::create_dir_all(pkg.join("cli")).unwrap(); - let entry = pkg.join("cli/cli.js"); - std::fs::write(&entry, "// node script\n").unwrap(); - std::fs::write( - pkg.join("package.json"), - br#"{"name": "@anthropic-ai/claude-code", "version": "2.1.0"}"#, - ) - .unwrap(); - - // npm leaves a `claude` symlink in `bin/`. - std::fs::create_dir_all(root.join("bin")).unwrap(); - let bin = root.join("bin/claude"); - #[cfg(unix)] - std::os::unix::fs::symlink(&entry, &bin).unwrap(); - - assert_eq!( - installed_version_from_package_json(&bin, Some("@anthropic-ai/claude-code")).as_deref(), - Some("2.1.0"), + Some("1.1.2"), ); } diff --git a/crates/doctor/src/lib.rs b/crates/doctor/src/lib.rs index f4b7992d..d51109a3 100644 --- a/crates/doctor/src/lib.rs +++ b/crates/doctor/src/lib.rs @@ -21,16 +21,19 @@ use std::collections::{HashMap, HashSet}; 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}; +use agents::{ + bundled_version_probe_args, check_single_ai_agent, derive_update_command, lookup_fix_command, + AI_AGENT_CHECKS, +}; use checks::{check_clonefile, check_gh, check_gh_auth, check_git, check_git_lfs}; use command::{ format_duration, run_command_with_timeout, CommandError, CommandTimeout, DEFAULT_PROBE_TIMEOUT, }; use freshness::{ fetch_version_info, is_self_updating, load_cache, save_cache, select_installed_probe, - FetchVersionInfoOptions, + FetchVersionInfoOptions, InstalledProbe, }; -use package_ids::{lookup_package_id_for_binary, LatestSource, Role}; +use package_ids::{lookup_package_id, LatestSource, Role}; use resolve::resolve_binary_with_diagnostics; use types::{InstallSource, ResolvedBinary}; @@ -82,6 +85,14 @@ pub struct RunChecksOptions { /// resolution, checks, freshness probes, and fixes all see the same shell /// environment. `None` preserves the previous per-call-site behavior. pub env: Option, + /// Directory holding agent binaries bundled inside the embedding app + /// (e.g. Staged's `resources/acp/bin`). Binaries that resolve from inside + /// this dir are labeled [`InstallSource::Bundled`] and their readouts get + /// `bundled: Some(true)` — versions are pinned by the app's lock and ship + /// with app updates, so no registry install/update fix is offered. The + /// caller remains responsible for putting the dir on the probe PATH (via + /// `env`); this option only affects labeling. + pub bundled_tools_dir: Option, } impl RunChecksOptions { @@ -105,10 +116,11 @@ pub async fn run_checks_with_options(opts: RunChecksOptions) -> DoctorReport { offline, npm_registry, env, + bundled_tools_dir, } = opts; let env = env.map(Arc::new); let npm_registry = npm_registry.as_deref(); - let report = collect_base_report(npm_registry, env.clone()).await; + let report = collect_base_report(npm_registry, env.clone(), bundled_tools_dir.as_deref()).await; if check_freshness { populate_freshness(report, offline, npm_registry, env).await @@ -120,6 +132,7 @@ pub async fn run_checks_with_options(opts: RunChecksOptions) -> DoctorReport { async fn collect_base_report( npm_registry: Option<&str>, env: Option>, + bundled_tools_dir: Option<&Path>, ) -> DoctorReport { let mut binary_names: Vec<&'static str> = vec!["git", "gh", "git-lfs"]; for info in AI_AGENT_CHECKS { @@ -155,6 +168,10 @@ async fn collect_base_report( } } + if let Some(dir) = bundled_tools_dir { + apply_bundled_install_source(&mut resolved, dir); + } + let fallback = ResolvedBinary { path: None, search_output: "resolution task panicked".to_string(), @@ -257,6 +274,20 @@ async fn collect_base_report( DoctorReport { checks } } +/// Re-label binaries that resolved from inside the embedding app's bundled +/// tools dir as [`InstallSource::Bundled`]. Runs before the checks consume the +/// resolution results, so the label flows into readouts (which also stamp +/// `bundled: Some(true)`), the flat `install_source`, and the freshness pass — +/// where `Bundled` has no registry entry and therefore never yields an update +/// nag or a registry fix command. +fn apply_bundled_install_source(resolved: &mut HashMap<&str, ResolvedBinary>, dir: &Path) { + for rb in resolved.values_mut() { + if rb.path.as_deref().is_some_and(|p| p.starts_with(dir)) { + rb.install_source = Some(InstallSource::Bundled); + } + } +} + fn timeout_diagnostic_checks(timeouts: Vec) -> Vec { let mut seen_timeouts = HashSet::new(); let mut used_ids = HashSet::new(); @@ -381,12 +412,11 @@ 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_for_binary(check_id, src, role, binary_path)) - .map(|(pkg, latest)| (Some(pkg), Some(latest))) + .and_then(|src| lookup_package_id(check_id, src, role)) + .map(|(pkg, latest)| (Some(pkg.to_string()), Some(latest))) .unwrap_or((None, None)) } @@ -493,12 +523,8 @@ async fn populate_freshness( if is_agent { if let (Some(readout), Some(path)) = (&check.main, check.path.as_deref()) { let path = PathBuf::from(path); - let (package_id, latest_source) = resolve_package( - &check.id, - readout.install_source.as_ref(), - Role::Main, - Some(&path), - ); + let (package_id, latest_source) = + resolve_package(&check.id, readout.install_source.as_ref(), Role::Main); targets.push(FreshnessTarget { id: check.id.clone(), slot: ReadoutSlot::Main, @@ -506,16 +532,19 @@ async fn populate_freshness( latest_source, package_id, install_source: readout.install_source.clone(), + // Bundled bridges probe the vendored harness CLI's version + // through the bridge passthrough (e.g. Claude Code 2.1.x), + // not the bridge package's own pinned version. + version_args: bundled_version_probe_args( + &check.id, + readout.install_source.as_ref(), + ), }); } if let (Some(readout), Some(path)) = (&check.bridge, check.bridge_path.as_deref()) { let path = PathBuf::from(path); - let (package_id, latest_source) = resolve_package( - &check.id, - readout.install_source.as_ref(), - Role::Bridge, - Some(&path), - ); + let (package_id, latest_source) = + resolve_package(&check.id, readout.install_source.as_ref(), Role::Bridge); targets.push(FreshnessTarget { id: check.id.clone(), slot: ReadoutSlot::Bridge, @@ -523,6 +552,7 @@ async fn populate_freshness( latest_source, package_id, install_source: readout.install_source.clone(), + version_args: None, }); } } else { @@ -531,12 +561,8 @@ async fn populate_freshness( let path_str = check.bridge_path.as_deref().or(check.path.as_deref()); let Some(path_str) = path_str else { continue }; let path = PathBuf::from(path_str); - let (package_id, latest_source) = resolve_package( - &check.id, - check.install_source.as_ref(), - Role::Any, - Some(&path), - ); + let (package_id, latest_source) = + resolve_package(&check.id, check.install_source.as_ref(), Role::Any); targets.push(FreshnessTarget { id: check.id.clone(), slot: ReadoutSlot::Flat, @@ -544,6 +570,7 @@ async fn populate_freshness( latest_source, package_id, install_source: check.install_source.clone(), + version_args: None, }); } } @@ -553,9 +580,14 @@ async fn populate_freshness( let npm_registry = npm_registry.clone(); let env = env.clone(); async move { - // npm-distributed bridges don't honor `--version`; read their - // installed version straight from the owning `package.json`. - let probe = select_installed_probe(t.install_source.as_ref(), t.package_id.as_deref()); + // Bundled bridges probe through their explicit passthrough args. + // Otherwise: npm-distributed bridges don't honor `--version`, so + // their installed version is read straight from the owning + // `package.json`; everything else runs ` --version`. + let probe = match t.version_args { + Some(args) => InstalledProbe::Cli(args), + None => select_installed_probe(t.install_source.as_ref(), t.package_id.as_deref()), + }; let info = fetch_version_info( t.latest_source, t.package_id.as_deref(), @@ -642,6 +674,9 @@ struct FreshnessTarget { latest_source: Option, package_id: Option, install_source: Option, + /// Explicit installed-version probe args (bridge passthrough for bundled + /// installs). `None` selects the source-derived default probe. + version_args: Option<&'static [&'static str]>, } /// Options for executing a doctor fix command. @@ -1344,62 +1379,97 @@ mod tests { &mut readout, &info, ReadoutSlot::Main, - Some("@anthropic-ai/claude-code"), + Some("@agentclientprotocol/claude-agent-acp"), ); assert_eq!( readout.update_command.as_deref(), - Some("npm install -g @anthropic-ai/claude-code@latest"), + Some("npm install -g @agentclientprotocol/claude-agent-acp@latest"), ); 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. + /// A brew-installed main CLI updates via `brew upgrade `. #[tokio::test] - async fn apply_freshness_brew_main_emits_claude_code_update_command() { + async fn apply_freshness_brew_main_emits_update_main_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()), + installed: Some("0.1.0".into()), + latest: Some("0.2.0".into()), update_available: Some(true), command_timeouts: Vec::new(), }; - apply_freshness(&mut readout, &info, ReadoutSlot::Main, Some("claude-code")); + apply_freshness(&mut readout, &info, ReadoutSlot::Main, Some("ampcode")); assert_eq!( readout.update_command.as_deref(), - Some("brew upgrade claude-code"), + Some("brew upgrade ampcode"), ); 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. + /// Bundled readouts report the probed version but never an update nag or + /// command — the binary is pinned by the embedding app's lock and updates + /// ship with the app. #[tokio::test] - async fn apply_freshness_brew_main_emits_claude_code_latest_update_command() { + async fn apply_freshness_bundled_suppresses_update() { let mut readout = AgentVersionInfo { - install_source: Some(InstallSource::Brew), + bundled: Some(true), + install_source: Some(InstallSource::Bundled), ..AgentVersionInfo::default() }; let info = freshness::VersionInfo { - installed: Some("2.1.152".into()), - latest: Some("2.1.153".into()), + installed: Some("2.1.205".into()), + latest: Some("2.2.0".into()), update_available: Some(true), command_timeouts: Vec::new(), }; - apply_freshness( - &mut readout, - &info, - ReadoutSlot::Main, - Some("claude-code@latest"), + apply_freshness(&mut readout, &info, ReadoutSlot::Main, None); + assert_eq!(readout.installed_version.as_deref(), Some("2.1.205")); + assert!(readout.update_available.is_none()); + assert!(readout.update_command.is_none()); + assert!(readout.update_fix_type.is_none()); + } + + /// Binaries resolved from inside the bundled tools dir are re-labeled + /// `Bundled`; everything else keeps its detected source. + #[test] + fn apply_bundled_install_source_relabels_only_bundled_paths() { + let mut resolved: HashMap<&str, ResolvedBinary> = HashMap::new(); + resolved.insert( + "codex-acp", + ResolvedBinary { + path: Some(PathBuf::from("/bundle/resources/acp/bin/codex-acp")), + search_output: String::new(), + install_source: Some(InstallSource::Unknown), + }, + ); + resolved.insert( + "pi-acp", + ResolvedBinary { + path: Some(PathBuf::from("/Users/me/.npm-global/bin/pi-acp")), + search_output: String::new(), + install_source: Some(InstallSource::Npm), + }, ); + resolved.insert( + "goose", + ResolvedBinary { + path: None, + search_output: String::new(), + install_source: None, + }, + ); + + apply_bundled_install_source(&mut resolved, Path::new("/bundle/resources/acp/bin")); + assert_eq!( - readout.update_command.as_deref(), - Some("brew upgrade claude-code@latest"), + resolved["codex-acp"].install_source, + Some(InstallSource::Bundled), ); - assert_eq!(readout.update_fix_type, Some(FixType::UpdateMain)); + assert_eq!(resolved["pi-acp"].install_source, Some(InstallSource::Npm)); + assert_eq!(resolved["goose"].install_source, None); } /// Bridge slot with a brew install upgrades via `brew upgrade ` and @@ -1600,7 +1670,7 @@ mod tests { ]); 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 command = "npm install -g @agentclientprotocol/claude-agent-acp@latest"; let result = execute_fix_streaming_with_env_options( "ai-agent-claude".to_string(), @@ -1620,9 +1690,8 @@ mod tests { 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"), + captured.iter().any(|line| line + == "snapshot-npm install -g @agentclientprotocol/claude-agent-acp@latest"), "expected update to run through snapshot npm; captured: {captured:?}", ); assert!( diff --git a/crates/doctor/src/package_ids.rs b/crates/doctor/src/package_ids.rs index 4cd1c8b2..2ca96156 100644 --- a/crates/doctor/src/package_ids.rs +++ b/crates/doctor/src/package_ids.rs @@ -8,8 +8,6 @@ //! 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. @@ -30,12 +28,11 @@ pub(crate) enum LatestSource { GitHubReleases, } -/// Which binary an entry describes. An AI-agent check fronts up to two distinct -/// binaries — the agent's own CLI (`Main`) and its ACP bridge (`Bridge`). When -/// both are installed from the same registry (e.g. both via npm, as with -/// Claude) the install source alone is ambiguous and the role is what -/// disambiguates them. Non-agent checks (and agents whose two binaries already -/// have distinct install sources) use `Any`. +/// Which binary an entry describes. An AI-agent check can front two distinct +/// binaries — the agent's own CLI (`Main`) and its ACP bridge (`Bridge`) — +/// and the role keeps their entries from answering each other's lookups when +/// they could share an install source. Non-agent checks and single-binary +/// agent checks use `Any`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum Role { Main, @@ -62,11 +59,9 @@ type PackageEntry = (InstallSource, &'static str, LatestSource, Role); /// Static table of `check_id -> &[PackageEntry]`. /// /// A single check can have multiple entries when the same agent ships through -/// different registries (e.g. brew for the main binary, npm for the ACP bridge) -/// or when both binaries share an install source but have distinct package ids -/// (Claude: `@anthropic-ai/claude-code` for the main CLI, `@agentclientprotocol -/// /claude-agent-acp` for the bridge — both npm). `lookup_package_id` picks the -/// first entry whose `(install_source, role)` matches the query. +/// different registries (e.g. Amp: brew for the main binary, npm for the ACP +/// bridge). `lookup_package_id` picks the first entry whose +/// `(install_source, role)` matches the query. pub(crate) const PACKAGE_IDS: &[(&str, &[PackageEntry])] = &[ ( "git", @@ -87,62 +82,28 @@ pub(crate) const PACKAGE_IDS: &[(&str, &[PackageEntry])] = &[ ), // ai-agent-goose: brew tap exists but no canonical formula yet — skip. // TODO: revisit when block/goose lands a stable brew formula. + // + // ai-agent-claude / ai-agent-codex: the ACP bridge is the only binary the + // check fronts (it vendors the full harness CLI), so the single npm entry + // is untagged. Bundled installs have no registry entry — their versions + // are pinned by the embedding app's lock. ( "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 - // landed via `npm i -g @anthropic-ai/claude-code`. - ( - InstallSource::Npm, - "@anthropic-ai/claude-code", - LatestSource::Npm, - Role::Main, - ), - // ACP bridge — separate npm package. - ( - InstallSource::Npm, - "@agentclientprotocol/claude-agent-acp", - LatestSource::Npm, - Role::Bridge, - ), - ], - // TODO: the main `claude` native (CurlPipe) install has no registry - // entry — its latest is published via the native installer's channel - // manifest, which we don't parse yet. Claude native is self-updating - // (see `freshness::is_self_updating`), so it stays report-only for now. + &[( + InstallSource::Npm, + "@agentclientprotocol/claude-agent-acp", + LatestSource::Npm, + Role::Any, + )], ), ( "ai-agent-codex", - &[ - // Bridge ships via npm; main CLI via brew or npm. Role tags - // disambiguate the two npm packages (bridge vs main). - ( - InstallSource::Npm, - "@zed-industries/codex-acp", - LatestSource::Npm, - Role::Bridge, - ), - (InstallSource::Brew, "codex", LatestSource::Brew, Role::Main), - // Main CLI when installed via npm. WARNING: the unscoped `codex` - // package on npm is an unrelated 2012 project; only the scoped - // `@openai/codex` is OpenAI's CLI. - ( - InstallSource::Npm, - "@openai/codex", - LatestSource::Npm, - Role::Main, - ), - ], + &[( + InstallSource::Npm, + "@agentclientprotocol/codex-acp", + LatestSource::Npm, + Role::Any, + )], ), ( "ai-agent-pi", @@ -240,112 +201,9 @@ 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 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); - } - } - - 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> { - 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() { @@ -372,18 +230,6 @@ mod tests { ); } - #[test] - fn codex_has_role_tagged_entries() { - assert_eq!( - lookup_package_id("ai-agent-codex", InstallSource::Npm, Role::Bridge), - Some(("@zed-industries/codex-acp", LatestSource::Npm)), - ); - assert_eq!( - lookup_package_id("ai-agent-codex", InstallSource::Brew, Role::Main), - Some(("codex", LatestSource::Brew)), - ); - } - #[test] fn cursor_curl_pipe_resolves_to_github_releases() { let (_, latest) = lookup_package_id("ai-agent-cursor", InstallSource::CurlPipe, Role::Any) @@ -391,126 +237,43 @@ mod tests { assert_eq!(latest, LatestSource::GitHubReleases); } - /// The whole point of the role split: claude's main CLI under npm must - /// resolve to `@anthropic-ai/claude-code`, not the bridge package. - #[test] - fn claude_main_npm_resolves_to_main_package() { - assert_eq!( - lookup_package_id("ai-agent-claude", InstallSource::Npm, Role::Main), - Some(("@anthropic-ai/claude-code", LatestSource::Npm)), - ); - } - - #[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)), - ); - } - + /// Claude's single binary is the ACP bridge, reported under the main slot: + /// its untagged entry must answer both Main and Any queries with the bridge + /// npm package. #[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); - } - - #[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"); - - assert_eq!( - lookup_package_id_for_binary( - "ai-agent-claude", - InstallSource::Brew, - Role::Main, - Some(path), - ), - Some(("claude-code".to_string(), LatestSource::Brew)), - ); + fn claude_npm_resolves_to_bridge_package_for_main_and_any() { + for role in [Role::Main, Role::Any] { + assert_eq!( + lookup_package_id("ai-agent-claude", InstallSource::Npm, role), + Some(("@agentclientprotocol/claude-agent-acp", LatestSource::Npm)), + ); + } } - /// And the bridge readout still resolves to the bridge package even though - /// they share an install source. + /// Same single-binary shape for codex — and the package id must be the + /// maintained `@agentclientprotocol` scope, not the retired + /// `@zed-industries` one. #[test] - fn claude_bridge_npm_resolves_to_bridge_package() { - assert_eq!( - lookup_package_id("ai-agent-claude", InstallSource::Npm, Role::Bridge), - Some(("@agentclientprotocol/claude-agent-acp", LatestSource::Npm)), - ); + fn codex_npm_resolves_to_bridge_package_for_main_and_any() { + for role in [Role::Main, Role::Any] { + assert_eq!( + lookup_package_id("ai-agent-codex", InstallSource::Npm, role), + Some(("@agentclientprotocol/codex-acp", LatestSource::Npm)), + ); + } } - /// A `Role::Any` query on a role-tagged check returns the first matching - /// entry — used by non-agent (flat) lookups and as a permissive fallback. + /// Bundled installs are pinned by the embedding app's lock and have no + /// registry entry — no latest-version probe, no update nag. #[test] - fn claude_npm_with_any_role_returns_first_match() { - // The Main entry appears first in the table; Any should hit it. - let (pkg, _) = lookup_package_id("ai-agent-claude", InstallSource::Npm, Role::Any).unwrap(); - assert_eq!(pkg, "@anthropic-ai/claude-code"); + fn bundled_installs_have_no_registry_entry() { + for id in ["ai-agent-claude", "ai-agent-codex"] { + assert_eq!( + lookup_package_id(id, InstallSource::Bundled, Role::Any), + None, + "{id}", + ); + } } /// `Role::Any` entries match any query role — confirms copilot's single @@ -566,22 +329,4 @@ mod tests { Some(("@github/copilot", LatestSource::Npm)), ); } - - #[test] - fn codex_npm_main_resolves_to_openai_codex() { - assert_eq!( - lookup_package_id("ai-agent-codex", InstallSource::Npm, Role::Main), - Some(("@openai/codex", LatestSource::Npm)), - ); - } - - /// Guard that adding the Main npm entry didn't shadow the existing Bridge - /// entry — the role-tagged lookup must still pick the bridge package. - #[test] - fn codex_npm_bridge_unchanged() { - assert_eq!( - lookup_package_id("ai-agent-codex", InstallSource::Npm, Role::Bridge), - Some(("@zed-industries/codex-acp", LatestSource::Npm)), - ); - } } diff --git a/crates/doctor/src/resolve.rs b/crates/doctor/src/resolve.rs index 88ac8b57..dcb9d082 100644 --- a/crates/doctor/src/resolve.rs +++ b/crates/doctor/src/resolve.rs @@ -285,8 +285,8 @@ fn resolved_binary( /// (mirroring the dirs in [`npm_search_dirs`]), and the System dirs. When those /// fall through to [`InstallSource::Unknown`] for a binary in a user-local bin /// dir, a cheap filesystem fingerprint (see [`fingerprint_curl_pipe`]) is -/// attempted to recognise curl/native installers (Claude native, Cursor, Amp), -/// using the caller snapshot's `HOME` when one is supplied. +/// attempted to recognise curl/native installers (Cursor, Amp), using the +/// caller snapshot's `HOME` when one is supplied. fn detect_install_source_with_env(path: &Path, env: Option<&DoctorEnv>) -> InstallSource { let home = env .and_then(|env| env.get("HOME").map(PathBuf::from)) @@ -439,11 +439,6 @@ struct CurlInstallerFootprint { /// listed so a bare `~/.local/bin/` with no installer footprint stays /// [`InstallSource::Unknown`]. const CURL_INSTALLER_FOOTPRINTS: &[CurlInstallerFootprint] = &[ - // Claude native installer — claude.ai/install.sh. - CurlInstallerFootprint { - binary: "claude", - markers: &[".local/share/claude", ".claude/local", ".claude/bin"], - }, // Cursor CLI installer — cursor.com/install. CurlInstallerFootprint { binary: "cursor-agent", @@ -463,7 +458,7 @@ const CURL_INSTALLER_FOOTPRINTS: &[CurlInstallerFootprint] = &[ /// 1. A known installer footprint marker (see [`CURL_INSTALLER_FOOTPRINTS`]) /// exists under `$HOME` and the binary name matches that installer. /// 2. The bin entry is a symlink into a *versioned* install dir under `$HOME` -/// (the layout Cursor's and Claude's native installers use: +/// (the layout Cursor's native installer uses: /// `~/.local/bin/` → `…/versions//`). /// /// No subprocess or network access — only `read_link`/`exists`/`canonicalize`. @@ -1198,10 +1193,10 @@ mod tests { fn fingerprint_curl_pipe_matches_known_installer_marker() { let home = std::env::temp_dir().join(format!("doctor-fp-marker-{}", std::process::id())); let _ = fs::remove_dir_all(&home); - // Claude native installer: ~/.local/bin/claude + ~/.local/share/claude. + // Amp installer: ~/.local/bin/amp + ~/.local/share/amp. fs::create_dir_all(home.join(".local/bin")).unwrap(); - fs::create_dir_all(home.join(".local/share/claude")).unwrap(); - let bin = home.join(".local/bin/claude"); + fs::create_dir_all(home.join(".local/share/amp")).unwrap(); + let bin = home.join(".local/bin/amp"); File::create(&bin).unwrap(); assert!(fingerprint_curl_pipe(&bin, &home)); @@ -1247,8 +1242,8 @@ mod tests { let home = std::env::temp_dir().join(format!("doctor-fp-outside-{}", std::process::id())); let _ = fs::remove_dir_all(&home); // Marker exists, but the binary is elsewhere — must not fingerprint. - fs::create_dir_all(home.join(".local/share/claude")).unwrap(); - let bin = PathBuf::from("/tmp/elsewhere/claude"); + fs::create_dir_all(home.join(".local/share/amp")).unwrap(); + let bin = PathBuf::from("/tmp/elsewhere/amp"); assert!(!fingerprint_curl_pipe(&bin, &home)); let _ = fs::remove_dir_all(&home); diff --git a/crates/doctor/src/types.rs b/crates/doctor/src/types.rs index 7c39aaec..0aa18fbe 100644 --- a/crates/doctor/src/types.rs +++ b/crates/doctor/src/types.rs @@ -59,6 +59,11 @@ pub enum InstallSource { Asdf, CurlPipe, System, + /// Shipped inside the embedding app's bundle (resolved from + /// [`crate::RunChecksOptions::bundled_tools_dir`] rather than a user + /// install). Versions are pinned by the app and updated with it, so + /// bundled readouts never get an update nag or a registry fix command. + Bundled, Unknown, } @@ -95,9 +100,11 @@ pub struct AgentVersionInfo { /// slot. Always paired with `update_command`: both `Some` or both `None`. pub update_fix_type: Option, /// Whether this binary ships bundled with the embedding app (resolved from - /// the app's bundled tools dir rather than a user install). The crate never - /// populates this — it has no notion of an app bundle; the embedding app - /// stamps it after checks run so the UI can present the binary as bundled. + /// the app's bundled tools dir rather than a user install). Populated by + /// the crate when the caller supplies + /// [`crate::RunChecksOptions::bundled_tools_dir`] and the readout's binary + /// resolved from inside it; the UI can then present the binary as bundled + /// instead of showing an app-internal resource path. #[serde(default)] pub bundled: Option, }