diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 0a2d17103b..f126ca69fb 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -1753,6 +1753,7 @@ impl ThreadRequestProcessor { let terminals = thread .list_background_terminals() .await + .map_err(invalid_request)? .into_iter() .map(|terminal| ThreadBackgroundTerminal { item_id: terminal.item_id, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 3b8d972d76..aa60c2cc1e 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -404,7 +404,7 @@ impl CodexThread { self.codex.agent_status().await } - pub async fn list_background_terminals(&self) -> Vec { + pub async fn list_background_terminals(&self) -> Result, String> { self.codex.session.list_background_terminals().await } diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index 61f6980156..42c8668b7b 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -785,7 +785,9 @@ impl Session { .await; } - pub(crate) async fn list_background_terminals(&self) -> Vec { + pub(crate) async fn list_background_terminals( + &self, + ) -> Result, String> { self.services.unified_exec_manager.list_processes().await } diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 0fd317de22..331fc3d535 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -162,7 +162,8 @@ struct ProcessEntry { exit_watcher: Option>, call_id: String, process_id: i32, - cwd: AbsolutePathBuf, + cwd_uri: PathUri, + path_convention: PathConvention, initial_exec_command_active: Arc, hook_command: String, tty: bool, diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index 284b02ddf3..698aa35202 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -129,7 +129,8 @@ async fn exec_command_with_tty( exit_watcher: None, call_id: context.call_id.clone(), process_id, - cwd: cwd.clone(), + cwd_uri: PathUri::from_abs_path(&cwd), + path_convention: PathConvention::native(), initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(true)), hook_command: cmd.to_string(), tty, @@ -366,7 +367,10 @@ async fn unified_exec_persists_across_requests() -> anyhow::Result<()> { .await?; let process_id = open_shell.process_id.expect("expected process_id"); assert_eq!( - session.list_background_terminals().await, + session + .list_background_terminals() + .await + .expect("local terminal cwd should be listable"), vec![BackgroundTerminalInfo { item_id: "call".to_string(), process_id: process_id.to_string(), @@ -399,7 +403,49 @@ async fn unified_exec_persists_across_requests() -> anyhow::Result<()> { assert!(session.terminate_background_terminal(process_id).await); assert!(!session.terminate_background_terminal(process_id).await); - assert!(session.list_background_terminals().await.is_empty()); + assert!( + session + .list_background_terminals() + .await + .expect("local terminal cwd should be listable") + .is_empty() + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn background_terminal_listing_rejects_foreign_cwd_conventions() -> anyhow::Result<()> { + skip_if_sandbox!(Ok(())); + + let (session, turn) = test_session_and_turn().await; + let open_shell = exec_command( + &session, &turn, "bash -i", /*yield_time_ms*/ 2_500, /*workdir*/ None, + ) + .await?; + let process_id = open_shell.process_id.expect("expected process_id"); + { + let mut store = session + .services + .unified_exec_manager + .process_store + .lock() + .await; + let entry = store + .processes + .get_mut(&process_id) + .expect("background process should be stored"); + entry.cwd_uri = PathUri::parse("file:///C:/workspace")?; + entry.path_convention = PathConvention::Windows; + } + + assert_eq!( + session.list_background_terminals().await, + Err(format!( + "background terminal process {process_id} uses Windows cwd paths, which cannot be listed on a POSIX host" + )) + ); + assert!(session.terminate_background_terminal(process_id).await); Ok(()) } @@ -676,7 +722,8 @@ async fn terminating_initial_exec_command_rechecks_initial_response_state() -> a exit_watcher: None, call_id: "call".to_string(), process_id, - cwd, + cwd_uri: PathUri::from_abs_path(&cwd), + path_convention: PathConvention::native(), initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(true)), hook_command: "sleep 60".to_string(), tty: true, @@ -750,7 +797,8 @@ async fn terminating_during_stdin_poll_returns_exited_response() -> anyhow::Resu exit_watcher: None, call_id: "call".to_string(), process_id, - cwd, + cwd_uri: PathUri::from_abs_path(&cwd), + path_convention: PathConvention::native(), initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(false)), hook_command: "sleep 60".to_string(), tty: true, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index cb7d690c74..64cbed9613 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -480,7 +480,6 @@ impl UnifiedExecProcessManager { context, &request.command, request.hook_command.clone(), - cwd.clone(), request.cwd_uri.clone(), request.path_convention, start, @@ -893,7 +892,6 @@ impl UnifiedExecProcessManager { context: &UnifiedExecContext, command: &[String], hook_command: String, - cwd: AbsolutePathBuf, cwd_uri: PathUri, path_convention: PathConvention, started_at: Instant, @@ -909,7 +907,7 @@ impl UnifiedExecProcessManager { Arc::clone(&context.turn), context.call_id.clone(), command.to_vec(), - cwd_uri, + cwd_uri.clone(), path_convention, process_id, transcript, @@ -920,7 +918,8 @@ impl UnifiedExecProcessManager { exit_watcher: Some(exit_watcher), call_id: context.call_id.clone(), process_id, - cwd: cwd.clone(), + cwd_uri, + path_convention, initial_exec_command_active, hook_command, tty, @@ -1454,7 +1453,7 @@ impl UnifiedExecProcessManager { } } - pub(crate) async fn list_processes(&self) -> Vec { + pub(crate) async fn list_processes(&self) -> Result, String> { let store = self.process_store.lock().await; let mut entries = store .processes @@ -1464,11 +1463,27 @@ impl UnifiedExecProcessManager { entries.sort_by_key(|entry| entry.process_id); entries .into_iter() - .map(|entry| BackgroundTerminalInfo { - item_id: entry.call_id.clone(), - process_id: entry.process_id.to_string(), - command: entry.hook_command.clone(), - cwd: entry.cwd.clone(), + .map(|entry| { + if entry.path_convention != PathConvention::native() { + return Err(format!( + "background terminal process {} uses {} cwd paths, which cannot be listed on a {} host", + entry.process_id, + entry.path_convention, + PathConvention::native(), + )); + } + let cwd = entry.cwd_uri.to_abs_path().map_err(|error| { + format!( + "background terminal process {} cwd `{}` cannot be represented on this host: {error}", + entry.process_id, entry.cwd_uri, + ) + })?; + Ok(BackgroundTerminalInfo { + item_id: entry.call_id.clone(), + process_id: entry.process_id.to_string(), + command: entry.hook_command.clone(), + cwd, + }) }) .collect() }