feat(doctor): drop claude/codex main CLIs for ACP bridge passthroughs

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,13 +39,15 @@ const TOOL_SPECS = [
     dependencyPackage: "@anthropic-ai/claude-agent-sdk",
     nativePackageKey: "claudeAgentSdk",
     includeClaudeCodeVersion: true,
+    passthroughArgs: ["--cli", "--version"],
   },
   {
     id: "codex-acp",
     binary: "codex-acp",
     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 <triple>]... [--lock-file <path>]
+  console.log(`Usage: scripts/update-acp-tools-lock.mjs [--target <triple>]... [--lock-file <path>] [--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)}`);

apps/staged/src-tauri/src/doctor.rs

@@ -26,14 +26,23 @@ 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<PathBuf>,
+) -> RunChecksOptions {
     RunChecksOptions {
         check_freshness,
         offline: false,
         // Use the default public registries — Staged installs these agents
         // 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<PathBuf>,
 ) -> Result<String, String> {
-    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());
     }
 }

apps/staged/src/lib/commands.ts

@@ -1294,7 +1294,7 @@ export function squashCommits(branchId: string, provider?: string): Promise<stri
 export type DoctorFixType = 'command' | 'bridge' | 'auth' | 'updateMain' | 'updateBridge';
 
 export type DoctorInstallSource =
-  'brew' | 'npm' | 'cargo' | 'mise' | 'asdf' | 'curlPipe' | 'system' | 'unknown';
+  'brew' | 'npm' | 'cargo' | 'mise' | 'asdf' | 'curlPipe' | 'system' | 'bundled' | 'unknown';
 
 /**
  * Version + install-source readout for one binary behind an agent check.
@@ -1316,7 +1316,8 @@ export interface AgentVersionInfo {
   /** 'updateMain' or 'updateBridge', matching this readout's slot. */
   updateFixType: 'updateMain' | 'updateBridge' | null;
   /** True when this binary ships bundled with Staged (resolved from the app's
-   *  bundled ACP tools dir rather than a user install). */
+   *  bundled ACP tools dir rather than a user install). Stamped by the doctor
+   *  crate alongside installSource === 'bundled'. */
   bundled: boolean | null;
 }
 

apps/staged/src/lib/features/doctor/DoctorCheckRow.svelte

@@ -168,12 +168,16 @@
     </span>
     <span class="check-message">{check.message}</span>
     {#if check.path}
-      <span class="check-path">{check.path}</span>
+      {#if check.main?.bundled}
+        <span class="check-path">Bundled with Staged</span>
+      {:else}
+        <span class="check-path">{check.path}</span>
+      {/if}
       {@render updateBadge('main', check.main)}
     {/if}
     {#if check.bridgePath}
       {#if check.bridge?.bundled}
-        <span class="check-path">ACP bundled with Staged</span>
+        <span class="check-path">Bundled with Staged</span>
       {:else}
         <span class="check-path">{check.bridgePath}</span>
       {/if}