diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 6e44481d57..5b781f63f6 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -346,7 +346,10 @@ const overrides = new Map([ // entries.push block collapsed into the helper. // +6: legacy Goose Windows install dir (%USERPROFILE%\goose) probed in // common_binary_paths so pre-#2680 standalone installs are discoverable. - ["src-tauri/src/managed_agents/discovery.rs", 1841], + // +19: codex-acp minimum-version gate — MIN_CODEX_ACP_VERSION plus the strict + // three-component parse in probe_codex_acp_version, so an outdated 1.x adapter + // is offered a reinstall instead of classifying as Available on major alone. + ["src-tauri/src/managed_agents/discovery.rs", 1860], // BYOH — save_custom_harness_to_dir (backup-swap atomic write) + save_and_warm / // delete_and_warm (persist-mutex serialization for concurrent-safe registry // refresh, B-6). Also: id/collision/load/registry tests (from the file base) + @@ -391,7 +394,11 @@ const overrides = new Map([ // Available both-present AND adapter-present/CLI-absent — the selectability // regression guard), bound to an injectable resolver so the tests stay // PATH-independent. - ["src-tauri/src/managed_agents/discovery/tests.rs", 1871], + // +51: codex-acp minimum-version gate — probe_codex_acp_version assertions carry + // the full (major, minor, patch) triple instead of a bare major, plus + // below-the-floor and uncomparable-version (partial / prerelease) classification + // regressions for the fail-closed parse. + ["src-tauri/src/managed_agents/discovery/tests.rs", 1922], // 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 @@ -633,7 +640,9 @@ const overrides = new Map([ // with backoff, output truncation) extracted to agent_discovery/install_exec.rs // alongside its tests, matching the managed_node.rs / post_install_verification.rs // split. The entries above describe the file's history, not its current shape. - ["src-tauri/src/commands/agent_discovery.rs", 1808], + // +27: codex-acp minimum-version gate — test_plan_adapter_install_updates_older_ + // 1x_codex_binary pins that a 1.x adapter below the floor still plans a reinstall. + ["src-tauri/src/commands/agent_discovery.rs", 1835], // draft-persistence predicate: submit-time `loadDraft` check + inline comment // + deps-array entry in submitMessage closes the never-persisted-boundary // defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 76f8596caf..d6429e0454 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -25,7 +25,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, -/// unparseable output, timeout, or missing binary). +/// Returns the `(major, minor, patch)` triple on success, `None` on any failure +/// (non-zero exit, unparseable output, timeout, or missing binary). +/// +/// The parse is deliberately strict: exactly three numeric dot-separated components. +/// Partial versions (`1.2`) and prerelease tags (`1.2.0-rc1`) return `None` and so +/// classify as [`AcpAvailabilityStatus::AdapterOutdated`] — failing closed offers a +/// reinstall rather than running an adapter whose version cannot be compared. /// /// The probe is bounded by a 5-second deadline. The child is polled with /// [`std::process::Child::try_wait`] (the repo's standard deadline pattern) and @@ -1180,16 +1195,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); @@ -1245,30 +1260,35 @@ 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)) } /// 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 its version is at least [`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) -/// or cannot be probed using `augmented_path`. Thin wrapper around -/// [`codex_adapter_is_outdated_with_path`]. +/// 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)] pub(crate) fn codex_adapter_is_outdated(path: &Path) -> bool { codex_adapter_is_outdated_with_path( @@ -1277,15 +1297,15 @@ pub(crate) fn codex_adapter_is_outdated(path: &Path) -> bool { ) } -/// Returns `true` when the codex-acp binary at `path` is outdated (major version < 1) -/// or cannot be probed with the supplied PATH. +/// 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 ) } @@ -1308,9 +1328,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..8761346b1f 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -6,7 +6,7 @@ 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_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, }; @@ -749,37 +749,41 @@ fn apply_agent_command_update_concrete_pin_keeps_materialized_runtime() { assert_eq!(record_agent_command(&record, &personas), "codex-acp"); } -// ── probe_codex_acp_major_version ───────────────────────────────────────────── +// ── probe_codex_acp_version ─────────────────────────────────────────────────── mod managed_path_resolution; #[cfg(unix)] #[test] -fn probe_codex_acp_major_version_parses_1x_output() { +fn probe_codex_acp_version_parses_full_semver_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"); - let major = probe_codex_acp_major_version(&bin); + let version = probe_codex_acp_version(&bin); let _ = std::fs::remove_dir_all(dir); - assert_eq!(major, Some(1), "1.x adapter must return major version 1"); + assert_eq!( + version, + Some((1, 1, 7)), + "adapter output must parse to its full semantic version" + ); } mod codex_version; #[cfg(unix)] #[test] -fn probe_codex_acp_major_version_returns_none_for_nonzero_exit() { +fn probe_codex_acp_version_returns_none_for_nonzero_exit() { use std::os::unix::fs::PermissionsExt; // Simulate old 0.16.x adapter: `--version` is unrecognised, exits non-zero @@ -789,21 +793,21 @@ fn probe_codex_acp_major_version_returns_none_for_nonzero_exit() { std::fs::write(&bin, "#!/bin/sh\nexit 1\n").expect("write script"); std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod script"); - let major = probe_codex_acp_major_version(&bin); + let version = probe_codex_acp_version(&bin); let _ = std::fs::remove_dir_all(dir); assert_eq!( - major, None, + version, None, "old 0.16.x adapter (non-zero exit) must return None" ); } #[cfg(unix)] #[test] -fn probe_codex_acp_major_version_returns_none_for_missing_binary() { +fn probe_codex_acp_version_returns_none_for_missing_binary() { let path = std::path::Path::new("/nonexistent/path/codex-acp-does-not-exist"); - let major = probe_codex_acp_major_version(path); - assert_eq!(major, None, "missing binary must return None"); + let version = probe_codex_acp_version(path); + assert_eq!(version, None, "missing binary must return None"); } // ── codex_adapter_availability / codex_adapter_is_outdated ─────────────────── @@ -813,7 +817,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 +825,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 +836,7 @@ 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" ); } @@ -858,6 +862,53 @@ fn codex_adapter_availability_outdated_for_0x_binary() { ); } +#[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, + "a 1.x adapter below the floor must be offered an upgrade" + ); +} + +/// The strict three-component parse fails closed: a version Buzz cannot compare +/// against the floor is treated as outdated rather than assumed current. +#[cfg(unix)] +#[test] +fn codex_adapter_availability_outdated_for_uncomparable_version() { + use std::os::unix::fs::PermissionsExt; + + for version in ["1.2", "1.2.0-rc1"] { + let dir = tempfile::tempdir().expect("temp dir"); + let bin = dir.path().join("codex-acp"); + std::fs::write( + &bin, + format!("#!/bin/sh\necho '@agentclientprotocol/codex-acp {version}'\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, + "version {version} is not comparable to the floor and must fail closed" + ); + } +} + #[cfg(unix)] #[test] fn codex_adapter_availability_outdated_for_missing_binary() { @@ -876,7 +927,7 @@ fn codex_adapter_availability_outdated_for_missing_binary() { #[cfg(unix)] #[test] -fn probe_codex_acp_major_version_returns_none_for_hung_direct_child() { +fn probe_codex_acp_version_returns_none_for_hung_direct_child() { use std::os::unix::fs::PermissionsExt; use std::time::Instant; @@ -894,12 +945,12 @@ fn probe_codex_acp_major_version_returns_none_for_hung_direct_child() { std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod script"); let start = Instant::now(); - let major = probe_codex_acp_major_version(&bin); + let version = probe_codex_acp_version(&bin); let elapsed = start.elapsed(); let _ = std::fs::remove_dir_all(dir); assert_eq!( - major, None, + version, None, "hung binary must return None (timeout kills child)" ); // The timeout is 5 s; give a 10 s margin for parallel pre-push suites. @@ -911,7 +962,7 @@ fn probe_codex_acp_major_version_returns_none_for_hung_direct_child() { #[cfg(unix)] #[test] -fn probe_codex_acp_major_version_returns_version_when_descendant_holds_pipe_open() { +fn probe_codex_acp_version_returns_version_when_descendant_holds_pipe_open() { use std::os::unix::fs::PermissionsExt; use std::time::Instant; @@ -936,7 +987,7 @@ fn probe_codex_acp_major_version_returns_version_when_descendant_holds_pipe_open std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod script"); let start = Instant::now(); - let major = probe_codex_acp_major_version(&bin); + let version = probe_codex_acp_version(&bin); let elapsed = start.elapsed(); let _ = std::fs::remove_dir_all(dir); @@ -947,9 +998,9 @@ fn probe_codex_acp_major_version_returns_version_when_descendant_holds_pipe_open "probe must not block on descendant pipe; elapsed: {elapsed:?}" ); assert_eq!( - major, - Some(1), - "1.x version must be parsed even when descendant holds pipe open" + version, + Some((1, 1, 2)), + "version must be parsed even when descendant holds pipe open" ); } diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/codex_version.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/codex_version.rs index 5886a43990..82bfd27f32 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/codex_version.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/codex_version.rs @@ -1,8 +1,8 @@ -use super::super::probe_codex_acp_major_version_with_path; +use super::super::probe_codex_acp_version_with_path; #[cfg(unix)] #[test] -fn probe_codex_acp_major_version_uses_augmented_path_for_env_shebang_interpreter() { +fn probe_codex_acp_version_uses_augmented_path_for_env_shebang_interpreter() { use std::fs; use std::os::unix::fs::PermissionsExt; let temp = tempfile::tempdir().expect("temp dir"); @@ -31,7 +31,7 @@ fn probe_codex_acp_major_version_uses_augmented_path_for_env_shebang_interpreter .to_string_lossy() .into_owned(); assert_eq!( - probe_codex_acp_major_version_with_path(&shim_path, Some(&scrubbed_path)), + probe_codex_acp_version_with_path(&shim_path, Some(&scrubbed_path)), None, "with a scrubbed PATH, /usr/bin/env should not find node" ); @@ -41,8 +41,8 @@ fn probe_codex_acp_major_version_uses_augmented_path_for_env_shebang_interpreter .to_string_lossy() .into_owned(); assert_eq!( - probe_codex_acp_major_version_with_path(&shim_path, Some(&augmented_path)), - Some(1), + probe_codex_acp_version_with_path(&shim_path, Some(&augmented_path)), + Some((1, 1, 2)), "the injected augmented PATH should allow /usr/bin/env to find node" ); }