diff --git a/.gitignore b/.gitignore index bbf535883..f23cb358b 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ desktop/src-tauri/resources/acp/bin/* !desktop/src-tauri/resources/acp/bin/.gitkeep desktop/src-tauri/resources/acp/node/ desktop/src-tauri/resources/acp/node-runtime.json +desktop/src-tauri/resources/acp/harness-clis.json # sqlx offline query data (generated, not portable) .sqlx/ diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index 16b89fce7..45f943509 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -63,8 +63,12 @@ pub(crate) enum AcpAvailabilityStatus { /// ACP adapter binary missing; underlying CLI may be present. AdapterMissing, /// ACP adapter binary is from the deprecated package (< 1.0). Reinstall required. + /// Retired on current desktops; kept so payloads from older app versions + /// still parse. AdapterOutdated, /// CLI binary missing; ACP adapter may be present. + /// Retired on current desktops (the bundled bridges vendor their own + /// CLI); kept so payloads from older app versions still parse. CliMissing, /// Neither adapter nor CLI found. NotInstalled, diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index f23c14ed1..babbaffe2 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -225,7 +225,9 @@ const overrides = new Map([ // "adapter_outdated" availability retired with the codex version gate (-1 line). // bundled-adapter-doctor-copy: adapterBundled field on // AcpRuntimeCatalogEntry (+2 lines). - ["src/shared/api/types.ts", 1071], + // bundled-cli-probes: "cli_missing" availability retired; +4 doc lines on + // AcpAvailabilityStatus explaining why the state no longer exists. + ["src/shared/api/types.ts", 1075], // readiness-gate: PersonaDialog.tsx threads computeLocalModeGate + // requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog // shows required markers and credential amber rows (parity with @@ -289,7 +291,10 @@ const overrides = new Map([ // claude-code-acp-fallback-retirement: the legacy command moved from the // resolution sweep to identity-only aliases; +5 comment lines documenting // the commands/aliases split that move makes load-bearing. - ["src-tauri/src/managed_agents/discovery.rs", 1259], + // bundled-cli-probes: resolve_probe_binary (bundled-CLI-first probe + // resolution) + classify_runtime/underlying_cli doc rewrites for the + // CliMissing retirement (+8 lines). + ["src-tauri/src/managed_agents/discovery.rs", 1267], // rebase over codex-acp-package-swap: its version-probe tests union with the // doctor-install-reliability nvm/login-shell/semver tests — each side alone // stayed under the 1000 default; the union exceeds it. @@ -304,7 +309,11 @@ const overrides = new Map([ // bundle-acps: version-gate retirement deletes the probe/availability test // sections; ratcheting 1271 -> 1067 to bank the deletions (main's Windows // Doctor test growth keeps this above the 1000 default). - ["src-tauri/src/managed_agents/discovery/tests.rs", 1067], + // bundled-cli-probes: CliMissing test reworked to assert Available when the + // adapter resolves without an underlying CLI (+2 comment lines); main's + // Windows install-command tests inverted to assert the bundled runtimes + // expose no install commands (-13 lines). + ["src-tauri/src/managed_agents/discovery/tests.rs", 1056], // identity-import-keyring: the identity resolution state machine's behavioral // matrix (46 tests over FakeIdentityStore — probe × marker × file cells, // adoption / read-back-corruption / marker-failure arms, recovery-mode diff --git a/desktop/scripts/prepare-acp-tools-resource.sh b/desktop/scripts/prepare-acp-tools-resource.sh index df7f84047..4e9026564 100755 --- a/desktop/scripts/prepare-acp-tools-resource.sh +++ b/desktop/scripts/prepare-acp-tools-resource.sh @@ -59,6 +59,16 @@ node_runtime_manifest="$resource_root/node-runtime.json" rm -f "$node_runtime_manifest" node_runtime_entries=() +# Manifest of the native harness CLIs vendored inside the bundled bridges +# (e.g. `claude` inside the claude-agent-sdk native package, `codex` inside +# @openai/codex). The app resolves auth probes against these pinned binaries +# instead of user installs. Kept OUT of resources/acp/bin on purpose: that +# dir is the highest-priority segment of the agent-spawn PATH, and staging +# `claude`/`codex` there would shadow the user's CLIs inside every session. +harness_cli_manifest="$resource_root/harness-clis.json" +rm -f "$harness_cli_manifest" +harness_cli_entries=() + codesign_if_darwin() { local file="$1" if [[ "$(uname -s)" == "Darwin" ]] && command -v codesign >/dev/null 2>&1; then @@ -66,7 +76,7 @@ codesign_if_darwin() { fi } -while IFS=$'\t' read -r id binary package version node_engine; do +while IFS=$'\t' read -r id binary package version node_engine native_package native_executable; do [[ -n "$id" ]] || continue install_dir="$cache_root/$target/$id/$version/npm" entrypoint="$install_dir/node_modules/$package/dist/index.js" @@ -84,6 +94,20 @@ while IFS=$'\t' read -r id binary package version node_engine; do fi write_node_wrapper "$resource_bin_dir/$binary" "../node/$id/node_modules/$package/dist/index.js" "$node_engine" node_runtime_entries+=("$id"$'\t'"$binary"$'\t'"$node_engine"$'\t'"$(acp_required_node_major "$node_engine")") + # Record the vendored native harness CLI (relative to the acp resource + # root) for the auth-probe manifest. Fail loudly if the lock names one + # that is not in the staged tree — a silent miss would quietly send auth + # probes back to unpinned user installs. + if [[ -n "$native_package" && -n "$native_executable" ]]; then + cli_relpath="node/$id/node_modules/$native_package/$native_executable" + cli_abspath="$resource_root/$cli_relpath" + if [[ ! -f "$cli_abspath" ]]; then + echo "Locked native harness CLI missing from staged tree: $cli_relpath" >&2 + exit 1 + fi + chmod +x "$cli_abspath" + harness_cli_entries+=("$id"$'\t'"$(basename "$native_executable")"$'\t'"$cli_relpath") + fi # Ad-hoc sign every Mach-O in the staged package, not just the main CLIs: # the codex native package also vendors executables like rg and zsh, and # unsigned nested Mach-Os are killed by Gatekeeper. Darwin only, so Linux @@ -104,7 +128,15 @@ for (const entry of data.tools ?? []) { if (entry.source !== "npm") { throw new Error(`Unsupported ACP tool source: ${entry.source}`); } - console.log([entry.id, entry.binary, entry.package, entry.version, entry.nodeEngine ?? ">=22"].join("\t")); + console.log([ + entry.id, + entry.binary, + entry.package, + entry.version, + entry.nodeEngine ?? ">=22", + entry.nativePackage ?? "", + entry.nativeExecutable ?? "", + ].join("\t")); } NODE ) @@ -125,4 +157,20 @@ fs.writeFileSync(manifestFile, `${JSON.stringify({ tools }, null, 2)}\n`); echo "Wrote ACP Node runtime manifest: $node_runtime_manifest" fi +# One manifest entry per vendored native harness CLI, keyed by the bare CLI +# name the app's auth probes use (`claude`, `codex`). Paths are relative to +# the acp resource root (the bin dir's parent). +if ((${#harness_cli_entries[@]} > 0)); then + node -e ' +const fs = require("node:fs"); +const [manifestFile, ...entries] = process.argv.slice(1); +const clis = entries.map((line) => { + const [id, cli, path] = line.split("\t"); + return { id, cli, path }; +}); +fs.writeFileSync(manifestFile, `${JSON.stringify({ clis }, null, 2)}\n`); +' "$harness_cli_manifest" ${harness_cli_entries[@]+"${harness_cli_entries[@]}"} + echo "Wrote ACP harness CLI manifest: $harness_cli_manifest" +fi + echo "Staged ACP tools resource: $resource_bin_dir" diff --git a/desktop/src-tauri/src/managed_agents/acp_tools.rs b/desktop/src-tauri/src/managed_agents/acp_tools.rs index 472e650fa..a328ecb3c 100644 --- a/desktop/src-tauri/src/managed_agents/acp_tools.rs +++ b/desktop/src-tauri/src/managed_agents/acp_tools.rs @@ -26,6 +26,11 @@ const ACP_TOOLS_RESOURCE_DIR: &str = "resources/acp/bin"; /// Node runtime manifest staged by `desktop/scripts/prepare-acp-tools-resource.sh` /// next to the bundled bin dir. const NODE_RUNTIME_MANIFEST_FILE: &str = "node-runtime.json"; +/// Harness CLI manifest staged next to the bin dir: one entry per native CLI +/// vendored inside a bundled bridge (`claude` inside the claude-agent-sdk +/// native package, `codex` inside @openai/codex), with paths relative to the +/// acp resource root. +const HARNESS_CLI_MANIFEST_FILE: &str = "harness-clis.json"; static BUNDLED_ACP_TOOLS_DIR: OnceLock> = OnceLock::new(); @@ -79,6 +84,52 @@ pub(in crate::managed_agents) fn node_runtime_manifest_path(bin_dir: &Path) -> O .map(|dir| dir.join(NODE_RUNTIME_MANIFEST_FILE)) } +/// On-disk shape of `resources/acp/harness-clis.json`. +#[derive(serde::Deserialize)] +struct HarnessCliManifest { + #[serde(default)] + clis: Vec, +} + +#[derive(serde::Deserialize)] +struct HarnessCliEntry { + cli: String, + path: String, +} + +/// Resolve the pinned native harness CLI (`claude`, `codex`) vendored inside +/// a bundled bridge, via the staged `harness-clis.json` manifest. This is the +/// same binary the bridge itself runs, so auth probes against it can never +/// drift from what agent sessions see. Bare command names only, mirroring +/// [`command_in_bundled_dir`]. Deliberately separate from the bin dir: these +/// CLIs must never join the agent-spawn PATH, where they would shadow the +/// user's (possibly newer) installs inside every session. +pub(in crate::managed_agents) fn bundled_harness_cli(cli: &str) -> Option { + let bin_dir = bundled_acp_tools_dir()?; + bundled_harness_cli_in_root(bin_dir.parent()?, cli) +} + +fn bundled_harness_cli_in_root(acp_root: &Path, cli: &str) -> Option { + if command_looks_like_path(cli) { + return None; + } + let manifest = std::fs::read_to_string(acp_root.join(HARNESS_CLI_MANIFEST_FILE)).ok()?; + let manifest: HarnessCliManifest = serde_json::from_str(&manifest).ok()?; + let entry = manifest.clis.into_iter().find(|entry| entry.cli == cli)?; + let relative = PathBuf::from(entry.path); + // Manifest paths are acp-root-relative by contract; anything absolute or + // escaping the root is malformed and must not resolve. + let escapes = relative.is_absolute() + || relative + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)); + if escapes { + return None; + } + let candidate = acp_root.join(relative); + is_executable_file(&candidate).then_some(candidate) +} + fn command_in_dir(dir: &Path, command: &str) -> Option { if command_looks_like_path(command) { return None; @@ -101,7 +152,10 @@ fn bundled_acp_tools_dir_from_parts( #[cfg(test)] mod tests { - use super::{bundled_acp_tools_dir_from_parts, command_in_dir, path_is_in_dir}; + use super::{ + bundled_acp_tools_dir_from_parts, bundled_harness_cli_in_root, command_in_dir, + path_is_in_dir, + }; use std::ffi::OsStr; use std::path::Path; @@ -201,6 +255,74 @@ mod tests { assert!(command_in_dir(temp.path(), "custom/codex-acp").is_none()); } + #[cfg(unix)] + #[test] + fn bundled_harness_cli_resolves_manifest_relative_path() { + use std::fs; + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("temp dir"); + let vendored = temp.path().join("node/claude-acp/node_modules/sdk-native"); + fs::create_dir_all(&vendored).expect("vendored dir"); + let cli = vendored.join("claude"); + fs::write(&cli, "#!/bin/sh\n").expect("write cli"); + fs::set_permissions(&cli, fs::Permissions::from_mode(0o755)).expect("chmod cli"); + fs::write( + temp.path().join("harness-clis.json"), + r#"{"clis":[{"id":"claude-acp","cli":"claude","path":"node/claude-acp/node_modules/sdk-native/claude"}]}"#, + ) + .expect("write manifest"); + + assert_eq!( + bundled_harness_cli_in_root(temp.path(), "claude").as_deref(), + Some(cli.as_path()), + ); + assert!( + bundled_harness_cli_in_root(temp.path(), "codex").is_none(), + "a CLI absent from the manifest must not resolve" + ); + } + + #[test] + fn bundled_harness_cli_without_manifest_is_none() { + let temp = tempfile::tempdir().expect("temp dir"); + assert!(bundled_harness_cli_in_root(temp.path(), "claude").is_none()); + } + + #[cfg(unix)] + #[test] + fn bundled_harness_cli_rejects_escaping_paths_and_path_like_names() { + use std::fs; + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("temp dir"); + let outside = temp.path().join("outside"); + fs::write(&outside, "#!/bin/sh\n").expect("write outside"); + fs::set_permissions(&outside, fs::Permissions::from_mode(0o755)).expect("chmod outside"); + let root = temp.path().join("acp"); + fs::create_dir_all(&root).expect("acp root"); + fs::write( + root.join("harness-clis.json"), + format!( + r#"{{"clis":[{{"id":"a","cli":"escape","path":"../outside"}},{{"id":"b","cli":"absolute","path":"{}"}}]}}"#, + outside.display() + ), + ) + .expect("write manifest"); + + assert!( + bundled_harness_cli_in_root(&root, "escape").is_none(), + "a ..-escaping manifest path must not resolve" + ); + assert!( + bundled_harness_cli_in_root(&root, "absolute").is_none(), + "an absolute manifest path must not resolve" + ); + // Path-like probe names bypass the manifest entirely — they name a + // specific binary and must fall through to regular resolution. + assert!(bundled_harness_cli_in_root(&root, "some/claude").is_none()); + } + #[cfg(unix)] #[test] fn command_in_dir_skips_non_executable_files() { diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 667b8cdf6..dca22ed63 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -21,7 +21,10 @@ pub(crate) struct KnownAcpRuntime { pub mcp_command: Option<&'static str>, /// Whether to enable MCP hook tools (`_Stop`, `_PostCompact`) for this agent. pub mcp_hooks: bool, - /// CLI binary that indicates partial install (e.g. `"claude"` when `claude-agent-acp` is missing). + /// CLI binary whose presence distinguishes `AdapterMissing` from + /// `NotInstalled` when the adapter is absent. `None` for the bundled + /// bridges (claude, codex): they ship their own vendored CLI, so the + /// user's install neither gates availability nor serves auth probes. pub underlying_cli: Option<&'static str>, /// Shell commands to install the runtime CLI itself (run sequentially). pub cli_install_commands: &'static [&'static str], @@ -176,13 +179,13 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ avatar_url: CLAUDE_CODE_AVATAR_URL, mcp_command: None, mcp_hooks: false, - underlying_cli: Some("claude"), - cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://claude.ai/install.ps1 | iex\""], + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], adapter_install_commands: &[], install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", - cli_install_hint: "Install the Claude Code CLI via the official install script.", - adapter_install_hint: "The Claude Code ACP adapter ships with the Buzz desktop app.", + cli_install_hint: "", + adapter_install_hint: "The Claude Code ACP adapter ships with the Buzz desktop app. Reinstall Buzz to restore it.", skill_dir: Some(".claude/skills"), supports_acp_model_switching: false, model_env_var: None, @@ -196,7 +199,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ max_tokens_env_var: None, context_limit_env_var: None, required_normalized_fields: &[], - login_hint: Some("Run the Claude CLI to complete authentication."), + login_hint: Some("Run the Claude CLI to complete authentication (install it first if needed)."), auth_probe_args: Some(&["claude", "auth", "status"]), }, KnownAcpRuntime { @@ -207,13 +210,13 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ avatar_url: CODEX_AVATAR_URL, mcp_command: Some("buzz-dev-mcp"), mcp_hooks: false, - underlying_cli: Some("codex"), - cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://chatgpt.com/codex/install.ps1 | iex\""], + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], adapter_install_commands: &[], install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", - cli_install_hint: "Install the Codex CLI via the official install script.", - adapter_install_hint: "The Codex ACP adapter ships with the Buzz desktop app.", + cli_install_hint: "", + adapter_install_hint: "The Codex ACP adapter ships with the Buzz desktop app. Reinstall Buzz to restore it.", skill_dir: Some(".codex/skills"), supports_acp_model_switching: false, model_env_var: None, @@ -227,7 +230,7 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ max_tokens_env_var: None, context_limit_env_var: None, required_normalized_fields: &[], - login_hint: Some("Run `codex login` to authenticate."), + login_hint: Some("Run `codex login` to authenticate (install the Codex CLI first if needed)."), // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. auth_probe_args: Some(&["codex", "login", "status"]), }, @@ -958,6 +961,16 @@ pub(crate) fn is_npm_global_install(cmd: &str) -> bool { t.starts_with("npm install -g ") || t.starts_with("npm i -g ") } +/// Resolve the binary for a CLI auth probe (`claude`, `codex`): the pinned +/// CLI vendored inside the bundled bridge wins — it is the exact binary agent +/// sessions run and reads the same credential store — falling back to the +/// user's install for builds without staged bundle resources. Not routed +/// through `resolve_command`: probe binaries must stay out of the resolve +/// cache and off the agent-spawn PATH. +pub(crate) fn resolve_probe_binary(cli: &str) -> Option { + super::acp_tools::bundled_harness_cli(cli).or_else(|| resolve_command(cli)) +} + /// Run a CLI auth probe with a 10-second process-level timeout. /// /// Spawns the probe CLI as a child process. Stdout and stderr are drained on @@ -1075,25 +1088,21 @@ pub fn missing_command_message(command: &str, role: &str) -> String { ) } +/// A resolving adapter is available, full stop — whether the runtime is +/// usable beyond that is an auth question (`auth_status`), not an install +/// question. The retired `CliMissing` state gated availability on a user +/// CLI install the bundled bridges no longer need. pub(crate) fn classify_runtime( adapter_result: Option<(&str, PathBuf)>, underlying_cli: Option<&str>, underlying_cli_found: bool, ) -> (AcpAvailabilityStatus, Option, Option) { if let Some((cmd, path)) = adapter_result { - if underlying_cli.is_some() && !underlying_cli_found { - ( - AcpAvailabilityStatus::CliMissing, - Some(cmd.to_string()), - Some(path.display().to_string()), - ) - } else { - ( - AcpAvailabilityStatus::Available, - Some(cmd.to_string()), - Some(path.display().to_string()), - ) - } + ( + AcpAvailabilityStatus::Available, + Some(cmd.to_string()), + Some(path.display().to_string()), + ) } else if underlying_cli.is_some() && underlying_cli_found { (AcpAvailabilityStatus::AdapterMissing, None, None) } else { @@ -1151,7 +1160,6 @@ pub fn discover_acp_runtimes() -> Vec { let adapter_hint = runtime.adapter_install_hint; let install_hint = match availability { AcpAvailabilityStatus::Available => cli_hint.to_string(), - AcpAvailabilityStatus::CliMissing => cli_hint.to_string(), AcpAvailabilityStatus::AdapterMissing => adapter_hint.to_string(), AcpAvailabilityStatus::NotInstalled => { if !cli_hint.is_empty() && !adapter_hint.is_empty() { @@ -1207,8 +1215,8 @@ pub fn discover_acp_runtimes() -> Vec { return None; } let probe_args = partial.runtime.auth_probe_args?; - // Need the resolved binary path for the CLI (e.g. the actual `claude` binary). - let binary_path = resolve_command(probe_args[0])?; + // Probe the bundled CLI when the app ships one, else the user's. + let binary_path = resolve_probe_binary(probe_args[0])?; let probe_args_owned: Vec = probe_args.iter().map(|s| s.to_string()).collect(); let handle = std::thread::spawn(move || { diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index a5aa2e606..f32be9a99 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -176,13 +176,15 @@ fn classifies_not_installed_when_no_underlying_cli() { } #[test] -fn classifies_cli_missing_when_adapter_found_but_cli_absent() { +fn classifies_available_when_adapter_found_even_without_underlying_cli() { + // The retired CliMissing gate must not come back: a resolving adapter is + // available regardless of whether an underlying CLI is on the user PATH. let (status, cmd, path) = classify_runtime( Some(("codex-acp", PathBuf::from("/opt/homebrew/bin/codex-acp"))), Some("codex"), false, ); - assert_eq!(status, AcpAvailabilityStatus::CliMissing); + assert_eq!(status, AcpAvailabilityStatus::Available); assert_eq!(cmd.as_deref(), Some("codex-acp")); assert_eq!(path.as_deref(), Some("/opt/homebrew/bin/codex-acp")); } @@ -868,64 +870,51 @@ fn test_command_basenames_dotted_name_no_extra_candidates() { // ── Phase B: cli_install_commands_for_os ──────────────────────────────────── -/// Claude and Codex have non-empty default cli_install_commands (install.sh). +/// Claude and Codex vendor their CLIs inside the bundled ACP packages — +/// the curl-pipe install commands are retired with the cli_missing gate. #[test] -fn test_claude_and_codex_have_cli_install_commands() { +fn test_claude_and_codex_have_no_cli_install_commands() { let claude = super::known_acp_runtime_exact("claude").unwrap(); let codex = super::known_acp_runtime_exact("codex").unwrap(); assert!( - !claude.cli_install_commands.is_empty(), - "claude must have cli install commands" + claude.cli_install_commands.is_empty(), + "claude CLI ships inside the bundled adapter — must not have cli install commands" ); assert!( - !codex.cli_install_commands.is_empty(), - "codex must have cli install commands" + codex.cli_install_commands.is_empty(), + "codex CLI ships inside the bundled adapter — must not have cli install commands" ); } -/// cli_install_commands_for_os returns a non-empty slice for claude and codex. +/// cli_install_commands_for_os is empty for claude and codex on every platform. #[test] -fn test_cli_install_commands_for_os_non_empty_for_claude_codex() { +fn test_cli_install_commands_for_os_empty_for_claude_codex() { let claude = super::known_acp_runtime_exact("claude").unwrap(); let codex = super::known_acp_runtime_exact("codex").unwrap(); assert!( - !claude.cli_install_commands_for_os().is_empty(), - "claude must have install commands on every platform" + claude.cli_install_commands_for_os().is_empty(), + "claude must not have install commands on any platform" ); assert!( - !codex.cli_install_commands_for_os().is_empty(), - "codex must have install commands on every platform" + codex.cli_install_commands_for_os().is_empty(), + "codex must not have install commands on any platform" ); } -/// On Windows, Claude and Codex select the PowerShell install commands. +/// On Windows, the bundled runtimes still expose no install commands, and +/// goose keeps its platform-neutral commands. #[cfg(windows)] #[test] -fn test_cli_install_commands_for_os_selects_powershell_on_windows() { +fn test_cli_install_commands_for_os_on_windows() { let claude = super::known_acp_runtime_exact("claude").unwrap(); let codex = super::known_acp_runtime_exact("codex").unwrap(); - - // Windows must select the PowerShell commands, not the curl|bash ones. - let claude_cmds = claude.cli_install_commands_for_os(); - let codex_cmds = codex.cli_install_commands_for_os(); - - assert_ne!( - claude_cmds, claude.cli_install_commands, - "Windows must NOT use the default curl|bash commands for claude" - ); - assert_ne!( - codex_cmds, codex.cli_install_commands, - "Windows must NOT use the default curl|bash commands for codex" - ); - - // Verify they are the PowerShell installers. assert!( - claude_cmds.iter().any(|c| c.contains("powershell")), - "claude Windows install must use powershell; got: {claude_cmds:?}" + claude.cli_install_commands_for_os().is_empty(), + "claude ships bundled — no Windows install commands" ); assert!( - codex_cmds.iter().any(|c| c.contains("powershell")), - "codex Windows install must use powershell; got: {codex_cmds:?}" + codex.cli_install_commands_for_os().is_empty(), + "codex ships bundled — no Windows install commands" ); // Goose and buzz-agent must NOT use Windows-specific commands. diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 05f001e24..d8695624d 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -46,7 +46,7 @@ use crate::managed_agents::{ agent_env::baked_build_env, config_bridge::read_goose_file_config, discovery::{ - classify_runtime, find_command, known_acp_runtime, resolve_command, KnownAcpRuntime, + classify_runtime, find_command, known_acp_runtime, resolve_probe_binary, KnownAcpRuntime, }, env_vars::merged_user_env, global_config::GlobalAgentConfig, @@ -502,7 +502,10 @@ fn cli_login_requirements( .iter() .find_map(|cmd| find_command(cmd).map(|path| (*cmd, path))); - // Check whether the underlying CLI itself (e.g. "claude", "codex") is on PATH. + // Check whether the underlying CLI is on PATH — only set for runtimes + // whose CLI is a separate install (not the bundled claude/codex bridges, + // which vendor their own); it distinguishes AdapterMissing from + // NotInstalled below. let underlying_cli_found = runtime .underlying_cli .map(|cli| find_command(cli).is_some()) @@ -513,10 +516,12 @@ fn cli_login_requirements( match availability { AcpAvailabilityStatus::Available => { - // Both adapter and CLI are present — probe login status. - // Resolve via the full login-shell PATH so the probe works in a - // packaged macOS DMG where the GUI PATH lacks npm/homebrew. - let Some(binary_path) = resolve_command(probe_args[0]) else { + // Adapter present — probe login status against the bundled CLI + // when the app ships one (the same pinned binary agent sessions + // run), else the user's install resolved via the full login-shell + // PATH so the probe works in a packaged macOS DMG where the GUI + // PATH lacks npm/homebrew. + let Some(binary_path) = resolve_probe_binary(probe_args[0]) else { // Unexpectedly not resolvable (race or PATH edge case). return vec![Requirement::CliLogin { probe_args: probe_args.iter().map(|s| s.to_string()).collect(), @@ -1041,10 +1046,11 @@ mod tests { } #[test] - fn cli_login_requirements_cli_missing_emits_cli_missing() { + fn cli_login_requirements_probe_runs_even_without_underlying_cli() { // Adapter present (use the running test binary as a portable stand-in), - // underlying CLI absent. - // → CliMissing state → no probe run → CliLogin{CliMissing}. + // underlying CLI absent. The retired CliMissing gate would have skipped + // the probe; now the adapter alone means Available, the probe runs + // (here: exit 0 → logged in) and no requirement is emitted. let exe = present_binary_str(); let rt = make_cli_runtime( static_commands(vec![exe]), // adapter found via absolute path @@ -1052,19 +1058,9 @@ mod tests { ); let reqs = cli_login_requirements(&[exe, "--list"], "install the CLI", &rt); assert!( - !reqs.is_empty(), - "CLI missing must produce a CliLogin requirement" + reqs.is_empty(), + "adapter present must probe login regardless of the user CLI; got {reqs:?}" ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::CliMissing, - "adapter present, CLI absent → CliMissing" - ); - } } #[test] diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 68f9603ac..29a24f0ca 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -529,12 +529,14 @@ pub struct ManagedAgentLogResponse { pub log_path: String, } +/// The retired `CliMissing` variant (adapter present, user CLI absent) is +/// gone: the bundled bridges vendor their own CLI, so a resolving adapter is +/// available regardless of user installs — sign-in state is `AuthStatus`. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum AcpAvailabilityStatus { Available, AdapterMissing, - CliMissing, NotInstalled, } diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 25cfe9af1..b8666728c 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -562,9 +562,7 @@ export function AgentDefinitionDialog({

{selectedRuntime.availability === "adapter_missing" ? `${selectedRuntime.label} CLI is installed but the ACP adapter is missing.` - : selectedRuntime.availability === "cli_missing" - ? `${selectedRuntime.label} ACP adapter is installed but the CLI is missing.` - : `${selectedRuntime.label} is not installed.`}{" "} + : `${selectedRuntime.label} is not installed.`}{" "} Visit Settings > Doctor to set it up.

) : null; diff --git a/desktop/src/features/agents/ui/personaDialogPickers.test.mjs b/desktop/src/features/agents/ui/personaDialogPickers.test.mjs index 5cc49a989..de81a5726 100644 --- a/desktop/src/features/agents/ui/personaDialogPickers.test.mjs +++ b/desktop/src/features/agents/ui/personaDialogPickers.test.mjs @@ -97,7 +97,7 @@ test("getDefaultPersonaRuntime falls back to goose when buzz-agent is unavailabl test("getDefaultPersonaRuntime returns first available when neither buzz-agent nor goose is available", () => { const runtimes = [ makeRuntime("buzz-agent", "adapter_missing"), - makeRuntime("goose", "cli_missing"), + makeRuntime("goose", "not_installed"), makeRuntime("claude"), ]; const result = getDefaultPersonaRuntime(runtimes); @@ -111,7 +111,7 @@ test("getDefaultPersonaRuntime returns null for an empty list", () => { test("getDefaultPersonaRuntime returns null when no runtime is available", () => { const runtimes = [ makeRuntime("buzz-agent", "not_installed"), - makeRuntime("goose", "cli_missing"), + makeRuntime("goose", "adapter_missing"), ]; assert.equal(getDefaultPersonaRuntime(runtimes), null); }); @@ -168,11 +168,7 @@ test("getPersonaModelOptions for buzz-agent with no provider returns default mod // is non-null (so the UI surfaces the reason) for each unavailability reason. test("formatModelDiscoveryErrorStatus returns a non-null status for runtime unavailable errors", () => { - for (const availability of [ - "adapter_missing", - "cli_missing", - "not_installed", - ]) { + for (const availability of ["adapter_missing", "not_installed"]) { const status = formatModelDiscoveryErrorStatus( new Error(`Runtime not available: ${availability}`), "anthropic", diff --git a/desktop/src/features/agents/ui/personaDialogPickers.tsx b/desktop/src/features/agents/ui/personaDialogPickers.tsx index 72a96ca99..c2c9c9e49 100644 --- a/desktop/src/features/agents/ui/personaDialogPickers.tsx +++ b/desktop/src/features/agents/ui/personaDialogPickers.tsx @@ -383,11 +383,9 @@ export function formatRuntimeOptionLabel(runtime: AcpRuntimeCatalogEntry) { const suffix = runtime.availability === "adapter_missing" ? " (adapter missing)" - : runtime.availability === "cli_missing" - ? " (CLI missing)" - : runtime.availability === "not_installed" - ? " (not installed)" - : ""; + : runtime.availability === "not_installed" + ? " (not installed)" + : ""; return `${runtime.label}${suffix}`; } @@ -397,12 +395,10 @@ function runtimeAvailabilitySortRank( switch (availability) { case "available": return 0; - case "cli_missing": - return 1; case "not_installed": - return 2; + return 1; case "adapter_missing": - return 3; + return 2; } } diff --git a/desktop/src/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 355953808..ede6aa250 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -353,19 +353,6 @@ function RuntimeDetails({ runtime }: { runtime: AcpRuntimeCatalogEntry }) { ); } - if (runtime.availability === "cli_missing") { - return ( - <> -

- ACP adapter detected; CLI missing. -

-

- {runtime.installHint} -

- - ); - } - return ( <>

diff --git a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs index cf21b6f6a..2fea5986c 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs +++ b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs @@ -108,7 +108,7 @@ test("validateLinkedAgentRuntimeEdit rejects unavailable linked-agent runtime ch input: updateInput({ runtime: "claude" }), managedAgent: agent(), previousPersona: persona({ runtime: "goose" }), - runtimes: [runtime({ availability: "cli_missing", command: null })], + runtimes: [runtime({ availability: "not_installed", command: null })], }), "Claude Code is not available. Install it before saving this linked agent.", ); diff --git a/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx b/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx index 653e8056e..0d705d568 100644 --- a/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx +++ b/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx @@ -38,8 +38,6 @@ function StatusIcon({ return ; case "adapter_missing": return ; - case "cli_missing": - return ; case "not_installed": return ; } @@ -122,8 +120,6 @@ function InstallActions({ /** * Node.js callout when required, or the install actions when it is not. * Used for both `adapter_missing` and `not_installed` availability states. - * The `cli_missing` branch is intentionally excluded — its install path does - * not involve npm, so no Node.js gate applies. */ function NodeRequiredOrInstall({ hasError, @@ -180,8 +176,7 @@ function RuntimeRow({ "flex min-h-16 items-start gap-3 px-4 py-3 text-sm", runtime.availability === "available" ? "bg-background/60" - : runtime.availability === "adapter_missing" || - runtime.availability === "cli_missing" + : runtime.availability === "adapter_missing" ? "bg-amber-500/5" : "bg-muted/20", )} @@ -218,23 +213,12 @@ function RuntimeRow({

) : null} - {runtime.underlyingCliPath && - runtime.underlyingCliPath !== runtime.binaryPath ? ( -
-

- CLI:{" "} - {runtime.underlyingCliPath} -

- {/* The bundled bridge's resource-dir path is noise — the - "ACP bridge bundled with Buzz." line above covers it. */} - {runtime.adapterBundled ? null : ( -

- ACP adapter:{" "} - {runtime.binaryPath} -

- )} -
- ) : runtime.adapterBundled ? null : ( + {/* The bundled bridge's resource-dir path is noise — the + "ACP bridge bundled with Buzz." line above covers it. The + user-CLI path row is retired with the cli_missing gate: the + bundled bridges vendor their own CLI, so no runtime reports a + separate CLI path anymore. */} + {runtime.adapterBundled ? null : ( <>

{runtime.binaryPath} @@ -285,34 +269,6 @@ function RuntimeRow({ runtime={runtime} /> - ) : runtime.availability === "cli_missing" ? ( - <> -

- {runtime.adapterBundled ? ( - <> - ACP bridge bundled with Buzz, but the {runtime.label} CLI is - not installed. - - ) : ( - <> - ACP adapter found at{" "} - - {runtime.binaryPath ?? "unknown path"} - {" "} - but the {runtime.label} CLI is not installed. - - )} -

-

- {runtime.installHint} -

- - ) : ( <>

diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index bbb6666bc..c8b65ad8b 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -554,10 +554,14 @@ export type GitBashPrerequisite = { installHint: string; }; +/** + * The retired "cli_missing" state (adapter present, user CLI absent) is gone: + * the bundled bridges vendor their own CLI, so a resolving adapter is + * available regardless of user installs — sign-in state lives in AuthStatus. + */ export type AcpAvailabilityStatus = | "available" | "adapter_missing" - | "cli_missing" | "not_installed"; /** Authentication/login status for a CLI-based ACP runtime. */ diff --git a/desktop/src/shared/lib/configNudge.test.mjs b/desktop/src/shared/lib/configNudge.test.mjs index 8e1603e23..c38e95aac 100644 --- a/desktop/src/shared/lib/configNudge.test.mjs +++ b/desktop/src/shared/lib/configNudge.test.mjs @@ -103,6 +103,30 @@ test("extractConfigNudge rejects retired adapter_outdated availability", () => { ); }); +test("extractConfigNudge rejects retired cli_missing availability", () => { + // The cli_missing gate was retired when auth probes moved to the CLIs + // vendored inside the bundled bridges — a user CLI install no longer gates + // availability. Stale nudge JSON emitted by an older app version must not + // parse into a card the current UI has no rendering for. + const payload = { + agent_name: "Fizz", + agent_pubkey: FIZZ_PUBKEY, + requirements: [ + { + surface: "cli_login", + probe_args: ["claude", "auth", "status"], + setup_copy: "install the Claude CLI", + availability: "cli_missing", + }, + ], + }; + assert.equal( + extractConfigNudge(withSentinel("prose", payload)), + null, + "retired cli_missing availability must be rejected by the validator", + ); +}); + test("extractConfigNudge returns null for cli_login without availability", () => { // availability is required — old-format payloads (no availability field) // must not parse so stale nudge JSON from before the Doctor-CTA update diff --git a/desktop/src/shared/lib/configNudge.ts b/desktop/src/shared/lib/configNudge.ts index 901bb2273..5ceb8314d 100644 --- a/desktop/src/shared/lib/configNudge.ts +++ b/desktop/src/shared/lib/configNudge.ts @@ -32,8 +32,7 @@ export type ConfigNudgeRequirement = * Determines which message and CTA the nudge card shows: * - "available" → tooling installed, needs login * - "adapter_missing" → CLI installed but ACP adapter missing - * - "cli_missing" → ACP adapter installed but CLI missing - * - "not_installed" → neither adapter nor CLI found + * - "not_installed" → no adapter found */ availability: AcpAvailabilityStatus; } @@ -139,9 +138,11 @@ function isConfigNudgeRequirement(v: unknown): v is ConfigNudgeRequirement { Array.isArray(r.probe_args) && r.probe_args.every((a) => typeof a === "string") && typeof r.setup_copy === "string" && + // Retired literals ("adapter_outdated", "cli_missing") emitted by + // older app versions are rejected here so stale nudge JSON cannot + // render a card the current UI has no branch for. (r.availability === "available" || r.availability === "adapter_missing" || - r.availability === "cli_missing" || r.availability === "not_installed") ); case "git_bash": diff --git a/desktop/src/shared/ui/config-nudge-attachment.tsx b/desktop/src/shared/ui/config-nudge-attachment.tsx index f21e5a329..b7ebd6ff3 100644 --- a/desktop/src/shared/ui/config-nudge-attachment.tsx +++ b/desktop/src/shared/ui/config-nudge-attachment.tsx @@ -103,8 +103,6 @@ function cliLoginMessage( switch (req.availability) { case "not_installed": return `${harness} isn't installed`; - case "cli_missing": - return `${harness} CLI is missing`; case "adapter_missing": return `${harness} ACP adapter isn't installed`; case "available": diff --git a/desktop/tests/e2e/doctor-cta-screenshots.spec.ts b/desktop/tests/e2e/doctor-cta-screenshots.spec.ts index 9bd273217..e0ab3ffe4 100644 --- a/desktop/tests/e2e/doctor-cta-screenshots.spec.ts +++ b/desktop/tests/e2e/doctor-cta-screenshots.spec.ts @@ -257,44 +257,7 @@ test.describe("doctor CTA nudge card screenshots", () => { }); }); - /** - * 04 — cli_missing state: ACP adapter present but underlying CLI absent. - * Shows "claude CLI is missing" copy. - */ - test("04-cli-login-cli-missing-state", async ({ page }) => { - await installMockBridge(page, { - managedAgents: [ - { - pubkey: AGENT_PUBKEY, - name: AGENT_NAME, - status: "stopped" as const, - channelNames: ["general"], - }, - ], - }); - - await page.goto("/", { waitUntil: "domcontentloaded" }); - - const content = makeNudgeSentinel(AGENT_NAME, AGENT_PUBKEY, [ - { - surface: "cli_login", - probe_args: ["claude"], - setup_copy: "install the Claude CLI", - availability: "cli_missing", - }, - ]); - - await injectNudgeAndNavigate(page, content); - - const card = page.locator("[data-config-nudge]").last(); - await expect(card).toBeVisible({ timeout: 10_000 }); - await expect(card.getByText(/CLI is missing/)).toBeVisible(); - - await card.scrollIntoViewIfNeeded(); - await settleAnimations(page); - - await card.screenshot({ - path: `${SHOTS}/04-cli-login-cli-missing-state.png`, - }); - }); + // The former 04-cli-login-cli-missing-state test retired with the + // cli_missing gate: the validator now rejects that availability literal, + // so no card renders for it (see configNudge.test.mjs). }); diff --git a/desktop/tests/e2e/doctor-states.spec.ts b/desktop/tests/e2e/doctor-states.spec.ts index 5bcb89390..20e56977d 100644 --- a/desktop/tests/e2e/doctor-states.spec.ts +++ b/desktop/tests/e2e/doctor-states.spec.ts @@ -50,6 +50,9 @@ const BUZZ_AGENT_AVAILABLE = { /** * Claude available and logged in — used as a neutral entry when claude is not * the runtime under test, and as the base for the auth states being tested. + * No `underlying_cli_path` and no auto-install: the bundled bridge vendors + * its own CLI, so the backend reports neither for claude since the + * cli_missing gate was retired. */ const CLAUDE_AVAILABLE_LOGGED_IN = { id: "claude", @@ -63,8 +66,8 @@ const CLAUDE_AVAILABLE_LOGGED_IN = { install_hint: "", install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", - can_auto_install: true, - underlying_cli_path: "/usr/local/bin/claude", + can_auto_install: false, + underlying_cli_path: null, node_required: false, auth_status: { status: "logged_in" }, }; @@ -150,7 +153,6 @@ test.describe("Doctor panel state screenshots", () => { availability: "available", command: "codex-acp", binary_path: "/usr/local/bin/codex-acp", - underlying_cli_path: "/usr/local/bin/codex", auth_status: { status: "logged_out" }, login_hint: "Run `codex login` to authenticate.", }, @@ -450,7 +452,8 @@ test.describe("Doctor panel state screenshots", () => { /** * 09 — available runtime whose adapter is the bridge bundled with the app: * the row says "ACP bridge bundled with Buzz" instead of rendering the - * resource-dir path; the user's CLI path still renders. + * resource-dir path, and no CLI path renders — the bundled bridge vendors + * its own CLI, so the user-CLI row retired with the cli_missing gate. */ test("09-bundled-adapter", async ({ page }) => { const bundledPath = @@ -474,7 +477,7 @@ test.describe("Doctor panel state screenshots", () => { const row = page.getByTestId("doctor-runtime-claude"); await expect(row).toBeVisible({ timeout: 10_000 }); await expect(row).toContainText("ACP bridge bundled with Buzz."); - await expect(row).toContainText("/usr/local/bin/claude"); + await expect(row).not.toContainText("CLI:"); await expect(row).not.toContainText(bundledPath); await expect(row).not.toContainText("installed on PATH");