diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index 70b5a8dcb28..c924e26f0fd 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -116,6 +116,8 @@ pub(crate) enum RequirementPayload { }, /// Git for Windows is missing; open Agent runtimes for the installation guide. GitBash, + /// A custom harness command could not be resolved in the current PATH. + MissingBinary { command: String }, } impl RequirementPayload { @@ -190,6 +192,9 @@ impl RequirementPayload { RequirementPayload::GitBash => { "install Git for Windows (open Agent runtimes in Settings to diagnose)".to_string() } + RequirementPayload::MissingBinary { command } => { + format!("install `{command}` or add it to PATH") + } } } } @@ -262,6 +267,10 @@ impl SetupPayload { .requirements .iter() .all(|r| matches!(r, RequirementPayload::CliConfigInvalid { .. })); + let all_missing_binary = self + .requirements + .iter() + .all(|r| matches!(r, RequirementPayload::MissingBinary { .. })); let any_external = self .requirements .iter() @@ -269,6 +278,8 @@ impl SetupPayload { let footer = if has_doctor_requirement { "Open Agent runtimes in Settings, install Git for Windows, then re-check and restart the agent.".to_string() + } else if all_missing_binary { + "Install the missing binary or update PATH, then restart Buzz.".to_string() } else if all_external { // All requirements are external config files — Edit Agent cannot // help. Don't send the user there. @@ -739,6 +750,23 @@ mod tests { )); } + #[test] + fn setup_payload_deserializes_missing_binary_requirement() { + let payload = SetupPayload::from_raw_env_value(Some( + r#"{"agent_name":"Carol","agent_pubkey":"test","requirements":[{"surface":"missing_binary","command":"buzz-pi-acp"}]}"#.to_string(), + )) + .unwrap() + .expect("missing_binary payload must parse"); + assert!(matches!( + payload.requirements.as_slice(), + [RequirementPayload::MissingBinary { command }] if command == "buzz-pi-acp" + )); + let body = payload.nudge_body(); + assert!(body.contains("install `buzz-pi-acp` or add it to PATH")); + assert!(body.contains("restart Buzz")); + assert!(!body.contains("Open Edit Agent")); + } + #[tokio::test] async fn authorized_workflow_nudge_mentions_effective_owner_not_relay_signer() { let agent_keys = nostr::Keys::generate(); diff --git a/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs b/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs index cae0d7e2c94..4ff4453a9ec 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs @@ -8,6 +8,7 @@ //! forced discovery calls before re-probing). use std::collections::HashMap; +use std::path::Path; use std::sync::{Mutex, OnceLock}; use crate::managed_agents::AuthStatus; @@ -52,9 +53,11 @@ pub(crate) fn len() -> usize { /// default path spawns nothing — it reuses the last cached status, falling back /// to `Unknown` for a runtime never probed this session. pub(super) fn resolve_auth_statuses(partials: &mut [super::PartialEntry], force: bool) { - use crate::managed_agents::AcpAvailabilityStatus; + use crate::managed_agents::{readiness::cli_probe, AcpAvailabilityStatus}; if force { + // No agent env here, so only the app env's `CODEX_PATH` applies. + let codex_path = cli_probe::codex_path_env(None); let probe_handles: Vec<(usize, std::thread::JoinHandle)> = partials .iter() .enumerate() @@ -63,14 +66,15 @@ pub(super) fn resolve_auth_statuses(partials: &mut [super::PartialEntry], force: 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 = super::resolve_command(probe_args[0])?; - let probe_args_owned: Vec = - probe_args.iter().map(|s| s.to_string()).collect(); + // Probe the CLI the runtime will actually run (for codex, the + // engine bundled with the resolved adapter). + let adapter_path = partial.entry.binary_path.as_deref().map(Path::new); + let (program, argv) = + cli_probe::probe_command(probe_args, adapter_path, codex_path.as_deref())?; let handle = std::thread::spawn(move || { - let refs: Vec<&str> = probe_args_owned.iter().map(String::as_str).collect(); - super::probe_auth_status(&binary_path, &refs) + let refs: Vec<&str> = argv.iter().map(String::as_str).collect(); + super::probe_auth_status(&program, &refs) }); Some((idx, handle)) }) diff --git a/desktop/src-tauri/src/managed_agents/discovery/catalog.rs b/desktop/src-tauri/src/managed_agents/discovery/catalog.rs index fecf792f214..8f48b396c14 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/catalog.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/catalog.rs @@ -90,7 +90,11 @@ pub(crate) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ avatar_url: CODEX_AVATAR_URL, mcp_command: Some("buzz-dev-mcp"), mcp_hooks: false, - underlying_cli: Some("codex"), + // No global `codex` needed: every supported codex-acp (>= MIN_CODEX_ACP_VERSION) + // depends on @openai/codex and runs that bundled engine unless CODEX_PATH is set. + // Requiring a PATH `codex` marked working installs CliMissing and hid Connect. + // The cli_install_* fields below go unused while this is None. + underlying_cli: None, cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], cli_install_commands_windows: &[windows_install_command!("codex", "https://chatgpt.com/codex/install.ps1")], adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index b68bc84c23f..6b45183e869 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -218,4 +218,20 @@ mod tests { assert!(codex.adapter_install_instructions_url.contains("codex-acp")); assert!(codex.cli_install_hint.contains("Codex CLI")); } + + /// codex-acp runs the Codex engine it bundles (or `CODEX_PATH`), so an + /// installed adapter with no `codex` on PATH is Available, not CliMissing. + #[test] + fn codex_adapter_without_global_cli_is_available() { + let codex = known_acp_runtime_exact("codex").unwrap(); + let (availability, ..) = super::super::classify_runtime( + Some(("codex-acp", std::path::PathBuf::from("codex-acp"))), + codex.underlying_cli, + false, + ); + assert_eq!( + availability, + crate::managed_agents::AcpAvailabilityStatus::Available + ); + } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fc53ff312cc..8e7e077b7fa 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -454,7 +454,7 @@ fn collect_missing_requirements( "complete Claude Code authentication by running the Claude CLI", rt, ), - "codex" => cli_login::requirements(&["codex", "login", "status"], "run `codex login`", rt), + "codex" => cli_login::codex_requirements(rt, &effective.env), _ => vec![], } } @@ -1013,7 +1013,7 @@ mod tests { // Use a not-installed runtime so the requirement is always emitted // regardless of whether codex is on the test machine's PATH. let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], None); - let reqs = cli_login::requirements(&["codex", "login", "status"], "run `codex login`", &rt); + let reqs = cli_login::codex_requirements(&rt, &BTreeMap::new()); // Whether codex is installed or not, the copy (if any) must not mention OPENAI_API_KEY. for req in &reqs { if let Requirement::CliLogin { setup_copy, .. } = req { diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs index 4036d9f2393..b35cd2e6ee7 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs @@ -1,20 +1,44 @@ +use std::collections::BTreeMap; use std::path::Path; use crate::managed_agents::{ - discovery::{ - classify_runtime, codex_adapter_availability, find_command, resolve_command, - KnownAcpRuntime, - }, + discovery::{classify_runtime, codex_adapter_availability, find_command, KnownAcpRuntime}, AcpAvailabilityStatus, }; use super::{cli_probe, Requirement}; -/// Requirements for CLI-login runtimes (claude, codex). +/// Codex requirements, probed with the engine this agent's codex-acp will run +/// (`CODEX_PATH` from `agent_env` or the app env, else the bundled engine). +pub(super) fn codex_requirements( + runtime: &KnownAcpRuntime, + agent_env: &BTreeMap, +) -> Vec { + let codex_path = cli_probe::codex_path_env(Some(agent_env)); + probe_requirements( + &["codex", "login", "status"], + // A global Codex CLI is optional, so lead with the in-app connect flow. + "connect your Codex account in Agent runtimes, or run `codex login` if the Codex CLI is installed", + runtime, + codex_path.as_deref(), + ) +} + +/// Requirements for a CLI-login runtime probed with `probe_args` as given +/// (claude; codex goes through [`codex_requirements`]). pub(super) fn requirements( probe_args: &[&str], setup_copy: &str, runtime: &KnownAcpRuntime, +) -> Vec { + probe_requirements(probe_args, setup_copy, runtime, None) +} + +fn probe_requirements( + probe_args: &[&str], + setup_copy: &str, + runtime: &KnownAcpRuntime, + codex_path: Option<&str>, ) -> Vec { let adapter_result = runtime .commands @@ -39,15 +63,27 @@ pub(super) fn requirements( match availability { AcpAvailabilityStatus::Available => { - let Some(binary_path) = resolve_command(probe_args[0]) else { - return vec![missing_requirement( - probe_args, - setup_copy, - AcpAvailabilityStatus::Available, - )]; + let adapter_path = adapter_path.as_deref().map(Path::new); + let Some((program, argv)) = + cli_probe::probe_command(probe_args, adapter_path, codex_path) + else { + // With `codex_path` set, only a CODEX_PATH that does not + // resolve lands here: codex-acp would fail to spawn it, so + // name that setting rather than asking for a login. + return vec![match codex_path { + Some(path) => Requirement::MissingBinary { + command: format!("CODEX_PATH={path}"), + }, + None => missing_requirement( + probe_args, + setup_copy, + AcpAvailabilityStatus::Available, + ), + }]; }; + let argv: Vec<&str> = argv.iter().map(String::as_str).collect(); let augmented_path = cli_probe::augmented_path(); - match cli_probe::login_probe(&binary_path, probe_args, augmented_path.as_deref()) { + match cli_probe::login_probe(&program, &argv, augmented_path.as_deref()) { cli_probe::ProbeOutcome::LoggedIn => vec![], cli_probe::ProbeOutcome::LoggedOut => vec![missing_requirement( probe_args, diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs index 513da4e2a85..89362594e72 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs @@ -1,6 +1,7 @@ -use std::path::Path; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; -use crate::managed_agents::runtime::build_augmented_path; +use crate::managed_agents::{resolve_command, runtime::build_augmented_path}; /// Build the augmented PATH for CLI probes and other native child processes /// (auth commands, `buzz-acp models` discovery), including nvm's default @@ -49,6 +50,69 @@ pub(crate) enum ProbeOutcome { /// one term. const CONFIG_PARSE_SIGNALS: &[&str] = &["error loading configuration", "unknown variant"]; +/// `CODEX_PATH` as a spawned codex-acp sees it: the agent's env over the app +/// env the spawn inherits. Empty counts as unset, as it does in codex-acp. +pub(crate) fn codex_path_env(agent_env: Option<&BTreeMap>) -> Option { + agent_env + .and_then(|env| env.get("CODEX_PATH").cloned()) + .or_else(|| std::env::var("CODEX_PATH").ok()) + .filter(|path| !path.is_empty()) +} + +/// The program and argv (`argv[0]` is only a label) a login probe runs. +/// +/// Codex is probed with the engine the agent will run, not whichever `codex` +/// is first on PATH: codex-acp spawns `CODEX_PATH` when set, else the +/// `@openai/codex` it bundles. An older global CLI can reject config values +/// the bundled engine accepts, which parks a working agent in setup mode. +/// +/// A set `CODEX_PATH` is strict, as in codex-acp: when it does not resolve +/// this returns `None` rather than falling back to an engine the agent +/// would never run. +pub(crate) fn probe_command( + probe_args: &[&str], + adapter_path: Option<&Path>, + codex_path: Option<&str>, +) -> Option<(PathBuf, Vec)> { + let owned = |args: &[&str]| args.iter().map(|arg| arg.to_string()).collect::>(); + if probe_args[0] == "codex" { + if let Some(path) = codex_path { + return Some((resolve_command(path)?, owned(probe_args))); + } + if let Some(codex_js) = adapter_path.and_then(bundled_codex_js) { + let mut argv = vec!["node".to_string(), codex_js.to_string_lossy().into_owned()]; + argv.extend(owned(&probe_args[1..])); + // Bare `node`, resolved through the probe's augmented PATH exactly + // as the adapter's own shim resolves it. + return Some((PathBuf::from("node"), argv)); + } + } + Some((resolve_command(probe_args[0])?, owned(probe_args))) +} + +/// `@openai/codex/bin/codex.js` as Node resolves it from the codex-acp +/// package: nested under the package first, then hoisted into an ancestor. +fn bundled_codex_js(adapter_path: &Path) -> Option { + // npm's Windows `.cmd` (and sh) shims sit beside `node_modules`; a unix + // bin symlink resolves into the package's `dist/`. + let shim_package: PathBuf = ["node_modules", "@agentclientprotocol", "codex-acp"] + .iter() + .collect(); + let shim_package = adapter_path.parent()?.join(shim_package); + let start = if shim_package.is_dir() { + shim_package + } else { + adapter_path.canonicalize().ok()?.parent()?.to_path_buf() + }; + let codex_js: PathBuf = ["node_modules", "@openai", "codex", "bin", "codex.js"] + .iter() + .collect(); + start + .ancestors() + .map(|dir| dir.join(&codex_js)) + .find(|path| path.is_file()) +} + /// Run the probe at the resolved absolute path so the GUI-PATH gap is /// bypassed. Injects the same augmented PATH used for launched agents so /// script shims with `/usr/bin/env ` shebangs can find runtimes @@ -233,6 +297,62 @@ mod tests { ); } + /// The probe must run the Codex engine codex-acp bundles, not whichever + /// `codex` is first on PATH: an older global CLI rejects config values the + /// bundled engine accepts and parks a working agent in setup mode. + #[test] + fn codex_probe_runs_engine_bundled_with_adapter_not_path_codex() { + use std::path::PathBuf; + + let temp = tempfile::tempdir().expect("temp dir"); + let package: PathBuf = ["node_modules", "@agentclientprotocol", "codex-acp"] + .iter() + .collect(); + let package = temp.path().join(package); + let codex_bin: PathBuf = ["node_modules", "@openai", "codex", "bin"].iter().collect(); + std::fs::create_dir_all(package.join(&codex_bin)).expect("bundled codex dir"); + let codex_js = package.join(&codex_bin).join("codex.js"); + std::fs::write(&codex_js, "").expect("write codex.js"); + let probe = |adapter: &std::path::Path| { + super::probe_command(&["codex", "login", "status"], Some(adapter), None) + .expect("a codex probe command") + }; + let expected = |js: &std::path::Path| { + let js = js.to_string_lossy().into_owned(); + ( + PathBuf::from("node"), + vec!["node".to_string(), js, "login".into(), "status".into()], + ) + }; + + // npm's Windows `.cmd` / sh shim layout: the shim sits beside node_modules. + let shim = temp.path().join("codex-acp.cmd"); + std::fs::write(&shim, "").expect("write shim"); + assert_eq!(probe(&shim), expected(&codex_js)); + + // A set CODEX_PATH is the engine codex-acp runs even when it is broken: + // no silent fallback to the bundled engine. + let missing = temp.path().join("missing-codex").display().to_string(); + assert_eq!( + super::probe_command(&["codex", "login", "status"], Some(&shim), Some(&missing)), + None + ); + + // Unix npm-global layout: bin/codex-acp links into the package's dist/. + #[cfg(unix)] + { + let dist = package.join("dist"); + std::fs::create_dir_all(&dist).expect("dist dir"); + std::fs::write(dist.join("index.js"), "").expect("write index.js"); + let bin = temp.path().join("bin"); + std::fs::create_dir_all(&bin).expect("bin dir"); + std::os::unix::fs::symlink(dist.join("index.js"), bin.join("codex-acp")) + .expect("symlink adapter"); + let codex_js = codex_js.canonicalize().expect("canonical codex.js"); + assert_eq!(probe(&bin.join("codex-acp")), expected(&codex_js)); + } + } + /// Verify that every string in CONFIG_PARSE_SIGNALS is lowercased so the /// case-insensitive match works correctly. #[test]