From a7f909911e42d4056b0feb87b6975a9a2d9b7470 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 14 Jul 2026 14:15:04 +1000 Subject: [PATCH 1/3] feat(doctor): drop claude/codex main CLIs for ACP bridge passthroughs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bundled ACP bridges vendor the complete harness CLIs — @agentclientprotocol/claude-agent-acp ships the full Claude Code binary and @agentclientprotocol/codex-acp ships the full codex binary — and expose them via passthrough subcommands that share the user's credential store. Doctor therefore no longer needs the raw `claude`/`codex` CLIs; the bridge is the only binary these checks resolve. - agents.rs: set `main_command: None` for ai-agent-claude/-codex; probe auth via `claude-agent-acp --cli auth status|login` and `codex-acp cli login status|login`; repoint install fixes at the bridge npm packages (also fixing the stale `@zed-industries/codex-acp` scope). This also fixes auth reading "Unknown" with no offered fix when the bridge is bundled but no main CLI is installed. - Delete the dead main-CLI machinery: brew/npm package ids for `claude-code`/`codex`, the claude-code cask-token detector, and the claude curl-install footprints. - Add `InstallSource::Bundled` and `RunChecksOptions::bundled_tools_dir` so doctor itself stamps bundled readouts (install source + `bundled` flag) and suppresses registry update commands for them, replacing Staged's ad-hoc `mark_bundled_bridges` prefix matching. - Freshness for bundled claude/codex probes the vendored harness version through the passthrough (`--cli --version` / `cli --version`) so update badges surface the Claude Code / codex version, not the bridge's. - Staged: pass the bundled tools dir into doctor, drop `mark_bundled_bridges`, and generalize the "ACP bundled with Staged" label to any bundled readout. - update-acp-tools-lock.mjs: smoke-check each bridge's CLI passthrough before writing the lock (`--skip-smoke` to bypass), so a future bridge release can't silently break doctor's probes. Verified: cargo clippy -D warnings (root workspace + Staged), cargo test -p doctor (115 passed), cargo test for Staged (490 passed), cargo fmt --check, frontend typecheck/tests/formatting. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- apps/staged/scripts/update-acp-tools-lock.mjs | 89 ++++- apps/staged/src-tauri/src/doctor.rs | 105 ++--- apps/staged/src/lib/commands.ts | 5 +- .../lib/features/doctor/DoctorCheckRow.svelte | 6 +- crates/doctor/src/agents.rs | 241 +++++++++--- crates/doctor/src/freshness.rs | 58 +-- crates/doctor/src/lib.rs | 179 ++++++--- crates/doctor/src/package_ids.rs | 365 +++--------------- crates/doctor/src/resolve.rs | 21 +- crates/doctor/src/types.rs | 13 +- 10 files changed, 529 insertions(+), 553 deletions(-) diff --git a/apps/staged/scripts/update-acp-tools-lock.mjs b/apps/staged/scripts/update-acp-tools-lock.mjs index c17fa02b..1b06eb21 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,12 @@ 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. const TOOL_SPECS = [ { id: "claude-acp", @@ -28,6 +35,7 @@ const TOOL_SPECS = [ dependencyPackage: "@anthropic-ai/claude-agent-sdk", nativePackageKey: "claudeAgentSdk", includeClaudeCodeVersion: true, + passthroughArgs: ["--cli", "--version"], }, { id: "codex-acp", @@ -35,6 +43,7 @@ const TOOL_SPECS = [ package: CODEX_ACP_PACKAGE, dependencyPackage: "@openai/codex", nativePackageKey: "openaiCodex", + passthroughArgs: ["cli", "--version"], }, ]; @@ -95,13 +104,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 +129,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 +148,10 @@ function parseArgs(argv) { lockFile = path.resolve(value); continue; } + if (arg === "--skip-smoke") { + skipSmoke = true; + continue; + } throw new Error(`Unknown argument: ${arg}`); } @@ -141,7 +161,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 +361,59 @@ 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 a +// version-shaped token 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 must fail here before it +// gets pinned. +async function smokeCheckPassthrough(tool, version) { + const invocation = `${tool.binary} ${tool.passthroughArgs.join(" ")}`; + 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 (!/\d+\.\d+\.\d+/.test(output)) { + throw new Error( + `Passthrough smoke check failed for ${tool.package}@${version}: ` + + `\`${invocation}\` printed no version: ${output || "(empty output)"}`, + ); + } + 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 +423,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.version); + } + } + } 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} + ACP bundled with Staged + {:else} + {check.path} + {/if} {@render updateBadge('main', check.main)} {/if} {#if check.bridgePath} diff --git a/crates/doctor/src/agents.rs b/crates/doctor/src/agents.rs index dd6925e3..fb90476f 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,42 @@ 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, + bundled_version_args: Some(&["cli", "--version"]), }, AgentCheckInfo { id: "ai-agent-pi", @@ -100,6 +115,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 +130,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 +144,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 +160,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 +203,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 +304,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 +777,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 +802,94 @@ 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"), + ); + assert_eq!(codex.bundled_version_args, Some(&["cli", "--version"][..])); + } + + #[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", "--version"][..]), + ); + // 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 +916,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 +935,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 +950,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 +992,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 +1038,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 +1046,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 +1077,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 +1159,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 +1188,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, } From 9f5cfec2852ee04eff3ed049ae88dedc91b40c63 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 14 Jul 2026 14:41:33 +1000 Subject: [PATCH 2/3] fix(doctor): shorten bundled-tool label to "Bundled with Staged" The bundled readout now covers whole harness CLIs vendored inside the bridges, not just the ACP binary, so the "ACP bundled with Staged" label in DoctorCheckRow undersold what ships. Drop the "ACP" prefix in both the main and bridge path slots. Verified: svelte-check + tsc (0 errors), vitest (431 passed), prettier --check on the edited file. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- apps/staged/src/lib/features/doctor/DoctorCheckRow.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/staged/src/lib/features/doctor/DoctorCheckRow.svelte b/apps/staged/src/lib/features/doctor/DoctorCheckRow.svelte index 069f5823..da0735b9 100644 --- a/apps/staged/src/lib/features/doctor/DoctorCheckRow.svelte +++ b/apps/staged/src/lib/features/doctor/DoctorCheckRow.svelte @@ -169,7 +169,7 @@ {check.message} {#if check.path} {#if check.main?.bundled} - ACP bundled with Staged + Bundled with Staged {:else} {check.path} {/if} @@ -177,7 +177,7 @@ {/if} {#if check.bridgePath} {#if check.bridge?.bundled} - ACP bundled with Staged + Bundled with Staged {:else} {check.bridgePath} {/if} From 064dc0ffe5f38d1d681bec3254da33c4d0cf8560 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 14 Jul 2026 15:21:56 +1000 Subject: [PATCH 3/3] fix(doctor): probe bundled codex version with cli -V, not cli --version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The codex-acp entrypoint checks process.argv.includes("--version") before dispatching the `cli` passthrough, so `codex-acp cli --version` prints the bridge's own version (1.1.2) instead of the vendored @openai/codex version (0.144.1) and the bundled freshness readout compared the wrong number. Verified against a real codex-acp@1.1.2 install: `cli --version` → "@agentclientprotocol/codex-acp 1.1.2", while clap's short flag `cli -V` escapes the literal-match interception and prints "codex-cli 0.144.1". claude-agent-acp is unaffected — `--cli --version` already prints "2.1.205 (Claude Code)". - agents.rs: switch the codex bundled_version_args to ["cli", "-V"]. - update-acp-tools-lock.mjs: same probe swap, and harden the smoke check to require the locked vendored-harness version (claudeCodeVersion for claude, dependencyVersion for codex) in the probe output instead of any semver token, so a future bridge release that intercepts the probe again fails the lock update instead of silently shipping a wrong freshness readout. Confirmed the hardened check passes with the new probes and rejects the old codex probe with a pointed diagnostic. Verified: cargo test -p doctor (115 passed), Staged cargo test (490 passed), cargo clippy -p doctor -D warnings, cargo fmt --check, and a live run of update-acp-tools-lock.mjs against a temp lock file (both smoke checks pass; the old codex probe variant fails). Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- apps/staged/scripts/update-acp-tools-lock.mjs | 32 +++++++++++++------ crates/doctor/src/agents.rs | 11 +++++-- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/apps/staged/scripts/update-acp-tools-lock.mjs b/apps/staged/scripts/update-acp-tools-lock.mjs index 1b06eb21..c0d3c30a 100755 --- a/apps/staged/scripts/update-acp-tools-lock.mjs +++ b/apps/staged/scripts/update-acp-tools-lock.mjs @@ -27,6 +27,10 @@ const CODEX_ACP_PACKAGE = "@agentclientprotocol/codex-acp"; // (`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", @@ -43,7 +47,7 @@ const TOOL_SPECS = [ package: CODEX_ACP_PACKAGE, dependencyPackage: "@openai/codex", nativePackageKey: "openaiCodex", - passthroughArgs: ["cli", "--version"], + passthroughArgs: ["cli", "-V"], }, ]; @@ -362,13 +366,17 @@ 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 a -// version-shaped token 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 must fail here before it -// gets pinned. -async function smokeCheckPassthrough(tool, version) { +// 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( @@ -400,10 +408,14 @@ async function smokeCheckPassthrough(tool, version) { `would break on this release. ${error instanceof Error ? error.message : String(error)}`, ); } - if (!/\d+\.\d+\.\d+/.test(output)) { + if (!output.includes(harnessVersion)) { throw new Error( `Passthrough smoke check failed for ${tool.package}@${version}: ` + - `\`${invocation}\` printed no version: ${output || "(empty output)"}`, + `\`${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]}`); @@ -429,7 +441,7 @@ async function main() { for (const tool of TOOL_SPECS) { const locked = tools.find((entry) => entry.id === tool.id); if (locked) { - await smokeCheckPassthrough(tool, locked.version); + await smokeCheckPassthrough(tool, locked); } } } diff --git a/crates/doctor/src/agents.rs b/crates/doctor/src/agents.rs index fb90476f..1ed3b11a 100644 --- a/crates/doctor/src/agents.rs +++ b/crates/doctor/src/agents.rs @@ -101,7 +101,10 @@ pub const AI_AGENT_CHECKS: &[AgentCheckInfo] = &[ auth_command: Some("codex-acp cli login"), auth_status_command: Some("codex-acp cli login status"), install_source_override: None, - bundled_version_args: Some(&["cli", "--version"]), + // `-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", @@ -864,7 +867,9 @@ mod tests { codex.install_command, Some("npm install -g @agentclientprotocol/codex-acp"), ); - assert_eq!(codex.bundled_version_args, Some(&["cli", "--version"][..])); + // `--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] @@ -875,7 +880,7 @@ mod tests { ); assert_eq!( bundled_version_probe_args("ai-agent-codex", Some(&InstallSource::Bundled)), - Some(&["cli", "--version"][..]), + Some(&["cli", "-V"][..]), ); // Registry installs keep their registry-consistent probes. assert_eq!(