From 8cd18411510aec7d29a74221b2ff5bb3fc9d3313 Mon Sep 17 00:00:00 2001 From: bharath Date: Mon, 27 Jul 2026 14:15:49 +0530 Subject: [PATCH] fix(desktop): require current codex-acp adapter Signed-off-by: bharath --- .../src-tauri/src/commands/agent_discovery.rs | 40 +++++++++-- .../src-tauri/src/managed_agents/discovery.rs | 67 +++++++++++++------ .../src/managed_agents/discovery/tests.rs | 53 +++++++++++++-- 3 files changed, 129 insertions(+), 31 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 84a00433c0..8f6810161b 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -26,7 +26,8 @@ fn active_installs() -> &'static std::sync::Mutex..` on stdout and exits 0. /// The old 0.16.x adapter (`@zed-industries/codex-acp`) is a Rust binary that does /// not recognise `--version` and exits non-zero. /// -/// Returns the major version on success, `None` on any failure (non-zero exit, +/// Returns the semantic version on success, `None` on any failure (non-zero exit, /// unparseable output, timeout, or missing binary). /// /// The probe is bounded by a 5-second deadline. The child is polled with @@ -1174,16 +1180,16 @@ pub(crate) fn classify_runtime( /// Stdout is redirected to a temporary file rather than a pipe, so forked /// descendants cannot hold EOF open. Reads from a regular file return EOF at its /// current write position regardless of inherited file descriptors, cross-platform. -pub(crate) fn probe_codex_acp_major_version(binary_path: &Path) -> Option { - probe_codex_acp_major_version_with_path( +pub(crate) fn probe_codex_acp_version(binary_path: &Path) -> Option<(u64, u64, u64)> { + probe_codex_acp_version_with_path( binary_path, crate::managed_agents::readiness::cli_probe::augmented_path().as_deref(), ) } -pub(crate) fn probe_codex_acp_major_version_with_path( +pub(crate) fn probe_codex_acp_version_with_path( binary_path: &Path, augmented_path: Option<&str>, -) -> Option { +) -> Option<(u64, u64, u64)> { use std::io::{Read as _, Seek as _, SeekFrom}; use std::time::{Duration, Instant}; const VERSION_PROBE_TIMEOUT: Duration = Duration::from_secs(5); @@ -1239,28 +1245,49 @@ pub(crate) fn probe_codex_acp_major_version_with_path( let stdout = String::from_utf8_lossy(&buf); // Output format: " .." let version_str = stdout.split_whitespace().last()?; - let major_str = version_str.split('.').next()?; - major_str.parse::().ok() + let mut components = version_str.split('.'); + let major = components.next()?.parse::().ok()?; + let minor = components.next()?.parse::().ok()?; + let patch = components.next()?.parse::().ok()?; + if components.next().is_some() { + return None; + } + Some((major, minor, patch)) +} + +/// Compatibility wrappers for callers that only need the major version. +pub(crate) fn probe_codex_acp_major_version(binary_path: &Path) -> Option { + probe_codex_acp_version(binary_path).map(|(major, _, _)| major) +} + +pub(crate) fn probe_codex_acp_major_version_with_path( + binary_path: &Path, + augmented_path: Option<&str>, +) -> Option { + probe_codex_acp_version_with_path(binary_path, augmented_path) + .map(|(major, _, _)| major) } /// Classifies a resolved codex-acp binary path as [`AcpAvailabilityStatus::Available`] /// or [`AcpAvailabilityStatus::AdapterOutdated`]. /// /// The 0.16.x adapter (`@zed-industries/codex-acp`) does not recognise `--version` -/// and exits non-zero — that probe failure yields `AdapterOutdated`. The 1.x adapter -/// (`@agentclientprotocol/codex-acp`) prints its version and exits 0; major ≥ 1 -/// yields `Available`. +/// and exits non-zero — that probe failure yields `AdapterOutdated`. An adapter is +/// available only when it meets [`MIN_CODEX_ACP_VERSION`]. /// /// Used by `discover_acp_runtimes`, `cli_login_requirements`, and /// `install_acp_runtime_blocking` so the version-gate logic is not duplicated. pub(crate) fn codex_adapter_availability(path: &Path) -> AcpAvailabilityStatus { - match probe_codex_acp_major_version(path) { - Some(major) if major >= 1 => AcpAvailabilityStatus::Available, + match probe_codex_acp_version(path) { + Some(version) if version >= MIN_CODEX_ACP_VERSION => { + AcpAvailabilityStatus::Available + } _ => AcpAvailabilityStatus::AdapterOutdated, } } -/// Returns `true` when the codex-acp binary at `path` is outdated (major version < 1) +/// Returns `true` when the codex-acp binary at `path` is below +/// [`MIN_CODEX_ACP_VERSION`] /// or cannot be probed using `augmented_path`. Thin wrapper around /// [`codex_adapter_is_outdated_with_path`]. #[cfg(test)] @@ -1271,15 +1298,16 @@ pub(crate) fn codex_adapter_is_outdated(path: &Path) -> bool { ) } -/// Returns `true` when the codex-acp binary at `path` is outdated (major version < 1) +/// Returns `true` when the codex-acp binary at `path` is below +/// [`MIN_CODEX_ACP_VERSION`] /// or cannot be probed with the supplied PATH. pub(crate) fn codex_adapter_is_outdated_with_path( path: &Path, augmented_path: Option<&str>, ) -> bool { !matches!( - probe_codex_acp_major_version_with_path(path, augmented_path), - Some(major) if major >= 1 + probe_codex_acp_version_with_path(path, augmented_path), + Some(version) if version >= MIN_CODEX_ACP_VERSION ) } @@ -1302,9 +1330,8 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr let (mut availability, command, binary_path) = classify_runtime(adapter_result, runtime.underlying_cli, underlying_cli_found); - // For codex-acp: when the adapter resolves as Available, probe the - // version. An adapter with major version < 1 is treated as outdated — - // the CODEX_CONFIG spawn contract requires 1.x. + // For codex-acp: when the adapter resolves as Available, probe its full + // version. An adapter below MIN_CODEX_ACP_VERSION is treated as outdated. if runtime.id == "codex" && availability == AcpAvailabilityStatus::Available && command.as_deref() == Some("codex-acp") diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 48e8d5479c..bf785cfe83 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -6,7 +6,8 @@ use super::{ codex_adapter_is_outdated, create_time_agent_command_override, default_agent_command, effective_agent_command, find_nvm_default_bin, find_via_login_shell, is_login_shell_path_uninit, is_safe_nvm_tag, managed_agent_avatar_url, normalize_agent_args, - parse_semver_tag, preset_catalog_entry, probe_codex_acp_major_version, record_agent_command, + parse_semver_tag, preset_catalog_entry, probe_codex_acp_major_version, + probe_codex_acp_version, record_agent_command, refresh_login_shell_path, try_record_agent_command, PresetHarness, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, }; @@ -758,13 +759,13 @@ mod managed_path_resolution; fn probe_codex_acp_major_version_parses_1x_output() { use std::os::unix::fs::PermissionsExt; - // Simulate `@agentclientprotocol/codex-acp 1.1.2` output (1.x adapter) + // Simulate a current `@agentclientprotocol/codex-acp` output. let dir = std::env::temp_dir().join(format!("buzz-probe-1x-{}", uuid::Uuid::new_v4())); std::fs::create_dir_all(&dir).expect("create temp dir"); let bin = dir.join("codex-acp"); std::fs::write( &bin, - "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.2'\nexit 0\n", + "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.7'\nexit 0\n", ) .expect("write script"); std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod script"); @@ -775,6 +776,24 @@ fn probe_codex_acp_major_version_parses_1x_output() { assert_eq!(major, Some(1), "1.x adapter must return major version 1"); } +#[cfg(unix)] +#[test] +fn probe_codex_acp_version_parses_full_semver_output() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("temp dir"); + let bin = dir.path().join("codex-acp"); + std::fs::write( + &bin, + "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.7'\nexit 0\n", + ) + .expect("write script"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) + .expect("chmod script"); + + assert_eq!(probe_codex_acp_version(&bin), Some((1, 1, 7))); +} + mod codex_version; #[cfg(unix)] @@ -813,7 +832,7 @@ fn probe_codex_acp_major_version_returns_none_for_missing_binary() { #[cfg(unix)] #[test] -fn codex_adapter_availability_available_for_1x_binary() { +fn codex_adapter_availability_available_for_minimum_supported_binary() { use std::os::unix::fs::PermissionsExt; let dir = std::env::temp_dir().join(format!("buzz-avail-1x-{}", uuid::Uuid::new_v4())); @@ -821,7 +840,7 @@ fn codex_adapter_availability_available_for_1x_binary() { let bin = dir.join("codex-acp"); std::fs::write( &bin, - "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.2'\nexit 0\n", + "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.7'\nexit 0\n", ) .expect("write script"); std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod script"); @@ -832,7 +851,29 @@ fn codex_adapter_availability_available_for_1x_binary() { assert_eq!( status, AcpAvailabilityStatus::Available, - "1.x adapter must classify as Available" + "minimum supported adapter must classify as Available" + ); +} + +#[cfg(unix)] +#[test] +fn codex_adapter_availability_outdated_for_older_1x_binary() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("temp dir"); + let bin = dir.path().join("codex-acp"); + std::fs::write( + &bin, + "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.5'\nexit 0\n", + ) + .expect("write script"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) + .expect("chmod script"); + + assert_eq!( + codex_adapter_availability(&bin), + AcpAvailabilityStatus::AdapterOutdated, + "older 1.x adapter must be offered an upgrade" ); }