diff --git a/desktop/src-tauri/src/managed_agents/backend.rs b/desktop/src-tauri/src/managed_agents/backend.rs index c6c96c73647..9bd2c3c8a68 100644 --- a/desktop/src-tauri/src/managed_agents/backend.rs +++ b/desktop/src-tauri/src/managed_agents/backend.rs @@ -1,4 +1,5 @@ use super::discovery::{command_search::command_discovery_dirs, resolve_command}; +use super::process_spawn::spawn_with_busy_retry as spawn_provider; use sha2::{Digest, Sha256}; use std::io::{BufReader, Read, Write}; use std::path::{Path, PathBuf}; @@ -88,11 +89,10 @@ pub fn invoke_provider( cmd.current_dir(home); } crate::util::configure_no_window(&mut cmd); - let mut child = cmd - .stdin(std::process::Stdio::piped()) + cmd.stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() + .stderr(std::process::Stdio::piped()); + let mut child = spawn_provider(&mut cmd) .map_err(|e| format!("failed to spawn {}: {e}", binary.display()))?; // Write request and close stdin immediately so the provider sees EOF. diff --git a/desktop/src-tauri/src/managed_agents/backend_tests.rs b/desktop/src-tauri/src/managed_agents/backend_tests.rs index d467f75dda1..b8af06e1c20 100644 --- a/desktop/src-tauri/src/managed_agents/backend_tests.rs +++ b/desktop/src-tauri/src/managed_agents/backend_tests.rs @@ -142,6 +142,59 @@ fn write_test_provider(path: &Path, body: &str) { std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)).unwrap(); } +#[cfg(target_os = "linux")] +#[test] +fn provider_spawn_waits_for_a_transient_writer() { + let directory = tempfile::tempdir().unwrap(); + let provider = directory.path().join("provider"); + write_test_provider(&provider, "exit 0"); + let writer = std::fs::OpenOptions::new() + .write(true) + .open(&provider) + .unwrap(); + assert_eq!( + std::process::Command::new(&provider) + .spawn() + .unwrap_err() + .kind(), + std::io::ErrorKind::ExecutableFileBusy, + ); + let closer = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(20)); + drop(writer); + }); + + let mut child = spawn_provider(&mut std::process::Command::new(&provider)) + .expect("launch after the inherited writer closes"); + assert!(child.wait().unwrap().success()); + closer.join().unwrap(); +} + +#[cfg(target_os = "linux")] +#[test] +fn provider_spawn_returns_persistent_write_conflicts() { + let directory = tempfile::tempdir().unwrap(); + let provider = directory.path().join("provider"); + write_test_provider(&provider, "exit 0"); + let _writer = std::fs::OpenOptions::new() + .write(true) + .open(&provider) + .unwrap(); + + let error = spawn_provider(&mut std::process::Command::new(&provider)).unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::ExecutableFileBusy); +} + +#[test] +fn provider_spawn_preserves_other_launch_errors() { + let directory = tempfile::tempdir().unwrap(); + let error = spawn_provider(&mut std::process::Command::new( + directory.path().join("missing"), + )) + .unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::NotFound); +} + #[cfg(unix)] #[test] fn provider_deploy_negotiates_and_deploys_the_same_staged_bytes() { diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 8a5f422a25a..b9ca77d58ef 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -840,14 +840,13 @@ pub(crate) fn probe_codex_acp_version_with_path( command.env("PATH", path); } crate::util::configure_no_window(&mut command); - let mut child = command + let deadline = Instant::now() + VERSION_PROBE_TIMEOUT; + command .stdout(tmp.try_clone().ok()?) - .stderr(std::process::Stdio::null()) - .spawn() - .ok()?; + .stderr(std::process::Stdio::null()); + let mut child = super::process_spawn::spawn_with_busy_retry(&mut command).ok()?; // Poll until the deadline rather than blocking on stdout EOF. - let deadline = Instant::now() + VERSION_PROBE_TIMEOUT; let exit_status = loop { match child.try_wait() { Ok(Some(status)) => break status, 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 19053865cbe..817d104041f 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 @@ -73,3 +73,56 @@ fn codex_adapter_availability_outdated_for_older_1x_binary() { assert!(codex_adapter_is_outdated(&bin)); } } + +#[cfg(target_os = "linux")] +#[test] +fn probe_codex_acp_version_waits_for_a_transient_executable_writer() { + use std::os::unix::fs::PermissionsExt; + let temp = tempfile::tempdir().expect("temp dir"); + let binary = temp.path().join("codex-acp"); + std::fs::write( + &binary, + "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.2'\n", + ) + .expect("write adapter"); + std::fs::set_permissions(&binary, std::fs::Permissions::from_mode(0o755)) + .expect("chmod adapter"); + let writer = std::fs::OpenOptions::new() + .write(true) + .open(&binary) + .unwrap(); + assert_eq!( + std::process::Command::new(&binary) + .spawn() + .unwrap_err() + .kind(), + std::io::ErrorKind::ExecutableFileBusy + ); + let closer = std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_millis(20)); + drop(writer); + }); + let version = probe_codex_acp_version_with_path(&binary, None); + closer.join().unwrap(); + assert_eq!(version, Some((1, 1, 2))); +} + +#[cfg(target_os = "linux")] +#[test] +fn probe_codex_acp_version_rejects_a_persistent_executable_writer() { + use std::os::unix::fs::PermissionsExt; + let temp = tempfile::tempdir().expect("temp dir"); + let binary = temp.path().join("codex-acp"); + std::fs::write( + &binary, + "#!/bin/sh\necho '@agentclientprotocol/codex-acp 1.1.2'\n", + ) + .expect("write adapter"); + std::fs::set_permissions(&binary, std::fs::Permissions::from_mode(0o755)) + .expect("chmod adapter"); + let _writer = std::fs::OpenOptions::new() + .write(true) + .open(&binary) + .unwrap(); + assert_eq!(probe_codex_acp_version_with_path(&binary, None), None); +} diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index ad030769de0..7accae3b0f6 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -32,6 +32,7 @@ pub(crate) mod persona_events; mod personas; #[cfg(windows)] mod process_lifecycle; +mod process_spawn; pub(crate) mod readiness; pub(crate) mod reconcile; mod relay_mesh; diff --git a/desktop/src-tauri/src/managed_agents/process_spawn.rs b/desktop/src-tauri/src/managed_agents/process_spawn.rs new file mode 100644 index 00000000000..8209b2966c6 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/process_spawn.rs @@ -0,0 +1,16 @@ +pub(super) fn spawn_with_busy_retry( + cmd: &mut std::process::Command, +) -> std::io::Result { + // A concurrent fork can inherit the staging writer until that child execs, + // even after this thread closes it. Retry only that failed exec: the command + // never started. Persistent writers still fail after 100 ms. + for _ in 0..10 { + match cmd.spawn() { + Err(error) if error.kind() == std::io::ErrorKind::ExecutableFileBusy => { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + result => return result, + } + } + cmd.spawn() +}