From 28ff364c3adf938c60fd73c5446e836762111186 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 25 Nov 2025 22:21:05 +0000 Subject: [PATCH] feat: update process ID for event handling (#7261) --- codex-rs/Cargo.lock | 1 + .../app-server-protocol/src/protocol/v2.rs | 2 + .../app-server/src/bespoke_event_handling.rs | 7 + codex-rs/app-server/tests/common/lib.rs | 1 + codex-rs/app-server/tests/common/responses.rs | 39 ++++ .../app-server/tests/suite/v2/turn_start.rs | 129 +++++++++++ codex-rs/core/Cargo.toml | 4 + codex-rs/core/src/tasks/user_shell.rs | 4 + codex-rs/core/src/tools/events.rs | 14 +- .../core/src/tools/handlers/unified_exec.rs | 11 +- codex-rs/core/src/unified_exec/errors.rs | 5 +- codex-rs/core/src/unified_exec/mod.rs | 89 +++++--- .../core/src/unified_exec/session_manager.rs | 213 +++++++++++------- codex-rs/core/tests/suite/unified_exec.rs | 99 ++++---- .../tests/event_processor_with_json_output.rs | 5 + codex-rs/protocol/src/protocol.rs | 8 + codex-rs/tui/src/chatwidget/tests.rs | 5 + 17 files changed, 473 insertions(+), 163 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 34ea8c877..4c87b8dc0 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1144,6 +1144,7 @@ dependencies = [ "codex-apply-patch", "codex-arg0", "codex-async-utils", + "codex-core", "codex-execpolicy", "codex-file-search", "codex-git", diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 3ee502ebd..222f3e4ef 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1020,6 +1020,8 @@ pub enum ThreadItem { command: String, /// The command's working directory. cwd: PathBuf, + /// Identifier for the underlying PTY process (when available). + process_id: Option, status: CommandExecutionStatus, /// A best-effort parsing of the command to understand the action(s) it will perform. /// This returns a list of CommandAction objects because a single shell command may diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 4b4e40d0c..24c74ebe4 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -449,11 +449,13 @@ pub(crate) async fn apply_bespoke_event_handling( .collect::>(); let command = shlex_join(&exec_command_begin_event.command); let cwd = exec_command_begin_event.cwd; + let process_id = exec_command_begin_event.process_id; let item = ThreadItem::CommandExecution { id: item_id, command, cwd, + process_id, status: CommandExecutionStatus::InProgress, command_actions, aggregated_output: None, @@ -486,6 +488,7 @@ pub(crate) async fn apply_bespoke_event_handling( command, cwd, parsed_cmd, + process_id, aggregated_output, exit_code, duration, @@ -514,6 +517,7 @@ pub(crate) async fn apply_bespoke_event_handling( id: call_id, command: shlex_join(&command), cwd, + process_id, status, command_actions, aggregated_output, @@ -649,6 +653,7 @@ async fn complete_command_execution_item( item_id: String, command: String, cwd: PathBuf, + process_id: Option, command_actions: Vec, status: CommandExecutionStatus, outgoing: &OutgoingMessageSender, @@ -657,6 +662,7 @@ async fn complete_command_execution_item( id: item_id, command, cwd, + process_id, status, command_actions, aggregated_output: None, @@ -1015,6 +1021,7 @@ async fn on_command_execution_request_approval_response( item_id.clone(), command.clone(), cwd.clone(), + None, command_actions.clone(), status, outgoing.as_ref(), diff --git a/codex-rs/app-server/tests/common/lib.rs b/codex-rs/app-server/tests/common/lib.rs index 7d3eb5388..6fd54a66d 100644 --- a/codex-rs/app-server/tests/common/lib.rs +++ b/codex-rs/app-server/tests/common/lib.rs @@ -15,6 +15,7 @@ pub use mcp_process::McpProcess; pub use mock_model_server::create_mock_chat_completions_server; pub use mock_model_server::create_mock_chat_completions_server_unchecked; pub use responses::create_apply_patch_sse_response; +pub use responses::create_exec_command_sse_response; pub use responses::create_final_assistant_message_sse_response; pub use responses::create_shell_command_sse_response; pub use rollout::create_fake_rollout; diff --git a/codex-rs/app-server/tests/common/responses.rs b/codex-rs/app-server/tests/common/responses.rs index 0a9183c04..d3d1f40cd 100644 --- a/codex-rs/app-server/tests/common/responses.rs +++ b/codex-rs/app-server/tests/common/responses.rs @@ -94,3 +94,42 @@ pub fn create_apply_patch_sse_response( ); Ok(sse) } + +pub fn create_exec_command_sse_response(call_id: &str) -> anyhow::Result { + let (cmd, args) = if cfg!(windows) { + ("cmd.exe", vec!["/d", "/c", "echo hi"]) + } else { + ("/bin/sh", vec!["-c", "echo hi"]) + }; + let command = std::iter::once(cmd.to_string()) + .chain(args.into_iter().map(str::to_string)) + .collect::>(); + let tool_call_arguments = serde_json::to_string(&json!({ + "cmd": command.join(" "), + "yield_time_ms": 500 + }))?; + let tool_call = json!({ + "choices": [ + { + "delta": { + "tool_calls": [ + { + "id": call_id, + "function": { + "name": "exec_command", + "arguments": tool_call_arguments + } + } + ] + }, + "finish_reason": "tool_calls" + } + ] + }); + + let sse = format!( + "data: {}\n\ndata: DONE\n\n", + serde_json::to_string(&tool_call)? + ); + Ok(sse) +} diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 2c23fedd9..cd35e31ad 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -1,6 +1,7 @@ use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_apply_patch_sse_response; +use app_test_support::create_exec_command_sse_response; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_chat_completions_server; use app_test_support::create_mock_chat_completions_server_unchecked; @@ -907,6 +908,134 @@ async fn turn_start_file_change_approval_decline_v2() -> Result<()> { Ok(()) } +#[tokio::test] +#[cfg_attr(windows, ignore = "process id reporting differs on Windows")] +async fn command_execution_notifications_include_process_id() -> Result<()> { + skip_if_no_network!(Ok(())); + + let responses = vec![ + create_exec_command_sse_response("uexec-1")?, + create_final_assistant_message_sse_response("done")?, + ]; + let server = create_mock_chat_completions_server(responses).await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri(), "never")?; + let config_toml = codex_home.path().join("config.toml"); + let mut config_contents = std::fs::read_to_string(&config_toml)?; + config_contents.push_str( + r#" +[features] +unified_exec = true +"#, + ); + std::fs::write(&config_toml, config_contents)?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "run a command".to_string(), + }], + ..Default::default() + }) + .await?; + let turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + let TurnStartResponse { turn: _turn } = to_response::(turn_resp)?; + + let started_command = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notif = mcp + .read_stream_until_notification_message("item/started") + .await?; + let started: ItemStartedNotification = serde_json::from_value( + notif + .params + .clone() + .expect("item/started should include params"), + )?; + if let ThreadItem::CommandExecution { .. } = started.item { + return Ok::(started.item); + } + } + }) + .await??; + let ThreadItem::CommandExecution { + id, + process_id: started_process_id, + status, + .. + } = started_command + else { + unreachable!("loop ensures we break on command execution items"); + }; + assert_eq!(id, "uexec-1"); + assert_eq!(status, CommandExecutionStatus::InProgress); + let started_process_id = started_process_id.expect("process id should be present"); + + let completed_command = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notif = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let completed: ItemCompletedNotification = serde_json::from_value( + notif + .params + .clone() + .expect("item/completed should include params"), + )?; + if let ThreadItem::CommandExecution { .. } = completed.item { + return Ok::(completed.item); + } + } + }) + .await??; + let ThreadItem::CommandExecution { + id: completed_id, + process_id: completed_process_id, + status: completed_status, + exit_code, + .. + } = completed_command + else { + unreachable!("loop ensures we break on command execution items"); + }; + assert_eq!(completed_id, "uexec-1"); + assert_eq!(completed_status, CommandExecutionStatus::Completed); + assert_eq!(exit_code, Some(0)); + assert_eq!( + completed_process_id.as_deref(), + Some(started_process_id.as_str()) + ); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + // Helper to create a config.toml pointing at the mock model server. fn create_config_toml( codex_home: &Path, diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 705bab57d..7bd2d652e 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -87,6 +87,9 @@ uuid = { workspace = true, features = ["serde", "v4", "v5"] } which = { workspace = true } wildmatch = { workspace = true } +[features] +deterministic_process_ids = [] + [target.'cfg(target_os = "linux")'.dependencies] landlock = { workspace = true } @@ -115,6 +118,7 @@ keyring = { workspace = true, features = ["sync-secret-service"] } assert_cmd = { workspace = true } assert_matches = { workspace = true } codex-arg0 = { workspace = true } +codex-core = { path = ".", features = ["deterministic_process_ids"] } core_test_support = { workspace = true } ctor = { workspace = true } escargot = { workspace = true } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 32e8a2596..ca5243241 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -81,6 +81,7 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: call_id.clone(), + process_id: None, turn_id: turn_context.sub_id.clone(), command: command.clone(), cwd: cwd.clone(), @@ -139,6 +140,7 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, + process_id: None, turn_id: turn_context.sub_id.clone(), command: command.clone(), cwd: cwd.clone(), @@ -161,6 +163,7 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: call_id.clone(), + process_id: None, turn_id: turn_context.sub_id.clone(), command: command.clone(), cwd: cwd.clone(), @@ -205,6 +208,7 @@ impl SessionTask for UserShellCommandTask { turn_context.as_ref(), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, + process_id: None, turn_id: turn_context.sub_id.clone(), command, cwd, diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 37df12d4b..93bce6048 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -65,12 +65,14 @@ pub(crate) async fn emit_exec_command_begin( parsed_cmd: &[ParsedCommand], source: ExecCommandSource, interaction_input: Option, + process_id: Option<&str>, ) { ctx.session .send_event( ctx.turn, EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: ctx.call_id.to_string(), + process_id: process_id.map(str::to_owned), turn_id: ctx.turn.sub_id.clone(), command: command.to_vec(), cwd: cwd.to_path_buf(), @@ -100,6 +102,7 @@ pub(crate) enum ToolEmitter { source: ExecCommandSource, interaction_input: Option, parsed_cmd: Vec, + process_id: Option, }, } @@ -132,6 +135,7 @@ impl ToolEmitter { cwd: PathBuf, source: ExecCommandSource, interaction_input: Option, + process_id: Option, ) -> Self { let parsed_cmd = parse_command(command); Self::UnifiedExec { @@ -140,6 +144,7 @@ impl ToolEmitter { source, interaction_input, parsed_cmd, + process_id, } } @@ -157,7 +162,7 @@ impl ToolEmitter { ) => { emit_exec_stage( ctx, - ExecCommandInput::new(command, cwd.as_path(), parsed_cmd, *source, None), + ExecCommandInput::new(command, cwd.as_path(), parsed_cmd, *source, None, None), stage, ) .await; @@ -229,6 +234,7 @@ impl ToolEmitter { source, interaction_input, parsed_cmd, + process_id, }, stage, ) => { @@ -240,6 +246,7 @@ impl ToolEmitter { parsed_cmd, *source, interaction_input.as_deref(), + process_id.as_deref(), ), stage, ) @@ -319,6 +326,7 @@ struct ExecCommandInput<'a> { parsed_cmd: &'a [ParsedCommand], source: ExecCommandSource, interaction_input: Option<&'a str>, + process_id: Option<&'a str>, } impl<'a> ExecCommandInput<'a> { @@ -328,6 +336,7 @@ impl<'a> ExecCommandInput<'a> { parsed_cmd: &'a [ParsedCommand], source: ExecCommandSource, interaction_input: Option<&'a str>, + process_id: Option<&'a str>, ) -> Self { Self { command, @@ -335,6 +344,7 @@ impl<'a> ExecCommandInput<'a> { parsed_cmd, source, interaction_input, + process_id, } } } @@ -362,6 +372,7 @@ async fn emit_exec_stage( exec_input.parsed_cmd, exec_input.source, exec_input.interaction_input.map(str::to_owned), + exec_input.process_id, ) .await; } @@ -402,6 +413,7 @@ async fn emit_exec_end( ctx.turn, EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: ctx.call_id.to_string(), + process_id: exec_input.process_id.map(str::to_owned), turn_id: ctx.turn.sub_id.clone(), command: exec_input.command.to_vec(), cwd: exec_input.cwd.to_path_buf(), diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 6790b7359..0c4f0a031 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -46,6 +46,7 @@ struct ExecCommandArgs { #[derive(Debug, Deserialize)] struct WriteStdinArgs { + // The model is trained on `session_id`. session_id: i32, #[serde(default)] chars: String, @@ -128,6 +129,7 @@ impl ToolHandler for UnifiedExecHandler { "failed to parse exec_command arguments: {err:?}" )) })?; + let process_id = manager.allocate_process_id().await; let command = get_command(&args); let ExecCommandArgs { @@ -168,6 +170,7 @@ impl ToolHandler for UnifiedExecHandler { cwd.clone(), ExecCommandSource::UnifiedExecStartup, None, + Some(process_id.clone()), ); emitter.emit(event_ctx, ToolEventStage::Begin).await; @@ -175,6 +178,7 @@ impl ToolHandler for UnifiedExecHandler { .exec_command( ExecCommandRequest { command, + process_id, yield_time_ms, max_output_tokens, workdir, @@ -197,7 +201,7 @@ impl ToolHandler for UnifiedExecHandler { manager .write_stdin(WriteStdinRequest { call_id: &call_id, - session_id: args.session_id, + process_id: &args.session_id.to_string(), input: &args.chars, yield_time_ms: args.yield_time_ms, max_output_tokens: args.max_output_tokens, @@ -255,8 +259,9 @@ fn format_response(response: &UnifiedExecResponse) -> String { sections.push(format!("Process exited with code {exit_code}")); } - if let Some(session_id) = response.session_id { - sections.push(format!("Process running with session ID {session_id}")); + if let Some(process_id) = &response.process_id { + // Training still uses "session ID". + sections.push(format!("Process running with session ID {process_id}")); } if let Some(original_token_count) = response.original_token_count { diff --git a/codex-rs/core/src/unified_exec/errors.rs b/codex-rs/core/src/unified_exec/errors.rs index bfbef9357..02031f22f 100644 --- a/codex-rs/core/src/unified_exec/errors.rs +++ b/codex-rs/core/src/unified_exec/errors.rs @@ -5,8 +5,9 @@ use thiserror::Error; pub(crate) enum UnifiedExecError { #[error("Failed to create unified exec session: {message}")] CreateSession { message: String }, - #[error("Unknown session id {session_id}")] - UnknownSessionId { session_id: i32 }, + // Called "session" in the model's training. + #[error("Unknown session id {process_id}")] + UnknownSessionId { process_id: String }, #[error("failed to write to stdin")] WriteToStdin, #[error("missing command line for unified exec request")] diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 1c9194baa..35df80255 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -22,9 +22,9 @@ //! - `session_manager.rs`: orchestration (approvals, sandboxing, reuse) and request handling. use std::collections::HashMap; +use std::collections::HashSet; use std::path::PathBuf; use std::sync::Arc; -use std::sync::atomic::AtomicI32; use std::time::Duration; use rand::Rng; @@ -67,6 +67,7 @@ impl UnifiedExecContext { #[derive(Debug)] pub(crate) struct ExecCommandRequest { pub command: Vec, + pub process_id: String, pub yield_time_ms: u64, pub max_output_tokens: Option, pub workdir: Option, @@ -77,7 +78,7 @@ pub(crate) struct ExecCommandRequest { #[derive(Debug)] pub(crate) struct WriteStdinRequest<'a> { pub call_id: &'a str, - pub session_id: i32, + pub process_id: &'a str, pub input: &'a str, pub yield_time_ms: u64, pub max_output_tokens: Option, @@ -89,7 +90,7 @@ pub(crate) struct UnifiedExecResponse { pub chunk_id: String, pub wall_time: Duration, pub output: String, - pub session_id: Option, + pub process_id: Option, pub exit_code: Option, pub original_token_count: Option, pub session_command: Option>, @@ -97,15 +98,16 @@ pub(crate) struct UnifiedExecResponse { #[derive(Default)] pub(crate) struct UnifiedExecSessionManager { - next_session_id: AtomicI32, - sessions: Mutex>, + sessions: Mutex>, + used_session_ids: Mutex>, } struct SessionEntry { - session: session::UnifiedExecSession, + session: UnifiedExecSession, session_ref: Arc, turn_ref: Arc, call_id: String, + process_id: String, command: Vec, cwd: PathBuf, started_at: tokio::time::Instant, @@ -159,6 +161,11 @@ mod tests { ) -> Result { let context = UnifiedExecContext::new(Arc::clone(session), Arc::clone(turn), "call".to_string()); + let process_id = session + .services + .unified_exec_manager + .allocate_process_id() + .await; session .services @@ -166,6 +173,7 @@ mod tests { .exec_command( ExecCommandRequest { command: vec!["bash".to_string(), "-lc".to_string(), cmd.to_string()], + process_id, yield_time_ms, max_output_tokens: None, workdir: None, @@ -179,7 +187,7 @@ mod tests { async fn write_stdin( session: &Arc, - session_id: i32, + process_id: &str, input: &str, yield_time_ms: u64, ) -> Result { @@ -188,7 +196,7 @@ mod tests { .unified_exec_manager .write_stdin(WriteStdinRequest { call_id: "write-stdin", - session_id, + process_id, input, yield_time_ms, max_output_tokens: None, @@ -221,11 +229,15 @@ mod tests { let (session, turn) = test_session_and_turn(); let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?; - let session_id = open_shell.session_id.expect("expected session_id"); + let process_id = open_shell + .process_id + .as_ref() + .expect("expected process_id") + .as_str(); write_stdin( &session, - session_id, + process_id, "export CODEX_INTERACTIVE_SHELL_VAR=codex\n", 2_500, ) @@ -233,7 +245,7 @@ mod tests { let out_2 = write_stdin( &session, - session_id, + process_id, "echo $CODEX_INTERACTIVE_SHELL_VAR\n", 2_500, ) @@ -253,11 +265,15 @@ mod tests { let (session, turn) = test_session_and_turn(); let shell_a = exec_command(&session, &turn, "bash -i", 2_500).await?; - let session_a = shell_a.session_id.expect("expected session id"); + let session_a = shell_a + .process_id + .as_ref() + .expect("expected process id") + .clone(); write_stdin( &session, - session_a, + session_a.as_str(), "export CODEX_INTERACTIVE_SHELL_VAR=codex\n", 2_500, ) @@ -265,9 +281,10 @@ mod tests { let out_2 = exec_command(&session, &turn, "echo $CODEX_INTERACTIVE_SHELL_VAR", 2_500).await?; + tokio::time::sleep(Duration::from_secs(2)).await; assert!( - out_2.session_id.is_none(), - "short command should not retain a session" + out_2.process_id.is_none(), + "short command should not report a process id if it exits quickly" ); assert!( !out_2.output.contains("codex"), @@ -276,7 +293,11 @@ mod tests { let out_3 = write_stdin( &session, - session_a, + shell_a + .process_id + .as_ref() + .expect("expected process id") + .as_str(), "echo $CODEX_INTERACTIVE_SHELL_VAR\n", 2_500, ) @@ -296,11 +317,15 @@ mod tests { let (session, turn) = test_session_and_turn(); let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?; - let session_id = open_shell.session_id.expect("expected session id"); + let process_id = open_shell + .process_id + .as_ref() + .expect("expected process id") + .as_str(); write_stdin( &session, - session_id, + process_id, "export CODEX_INTERACTIVE_SHELL_VAR=codex\n", 2_500, ) @@ -308,7 +333,7 @@ mod tests { let out_2 = write_stdin( &session, - session_id, + process_id, "sleep 5 && echo $CODEX_INTERACTIVE_SHELL_VAR\n", 10, ) @@ -320,7 +345,7 @@ mod tests { tokio::time::sleep(Duration::from_secs(7)).await; - let out_3 = write_stdin(&session, session_id, "", 100).await?; + let out_3 = write_stdin(&session, process_id, "", 100).await?; assert!( out_3.output.contains("codex"), @@ -337,7 +362,7 @@ mod tests { let result = exec_command(&session, &turn, "echo codex", 120_000).await?; - assert!(result.session_id.is_none()); + assert!(result.process_id.is_some()); assert!(result.output.contains("codex")); Ok(()) @@ -350,8 +375,8 @@ mod tests { let result = exec_command(&session, &turn, "echo codex", 2_500).await?; assert!( - result.session_id.is_none(), - "completed command should not retain session" + result.process_id.is_some(), + "completed command should report a process id" ); assert!(result.output.contains("codex")); @@ -375,31 +400,35 @@ mod tests { let (session, turn) = test_session_and_turn(); let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?; - let session_id = open_shell.session_id.expect("expected session id"); + let process_id = open_shell + .process_id + .as_ref() + .expect("expected process id") + .as_str(); - write_stdin(&session, session_id, "exit\n", 2_500).await?; + write_stdin(&session, process_id, "exit\n", 2_500).await?; tokio::time::sleep(Duration::from_millis(200)).await; - let err = write_stdin(&session, session_id, "", 100) + let err = write_stdin(&session, process_id, "", 100) .await .expect_err("expected unknown session error"); match err { - UnifiedExecError::UnknownSessionId { session_id: err_id } => { - assert_eq!(err_id, session_id); + UnifiedExecError::UnknownSessionId { process_id: err_id } => { + assert_eq!(err_id, process_id, "process id should match request"); } other => panic!("expected UnknownSessionId, got {other:?}"), } assert!( - !session + session .services .unified_exec_manager .sessions .lock() .await - .contains_key(&session_id) + .is_empty() ); Ok(()) diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 515bb607f..df8f54321 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -1,9 +1,9 @@ +use rand::Rng; use std::cmp::Reverse; use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; use std::sync::Arc; - use tokio::sync::Notify; use tokio::sync::mpsc; use tokio::time::Duration; @@ -75,9 +75,38 @@ struct PreparedSessionHandles { turn_ref: Arc, command: Vec, cwd: PathBuf, + process_id: String, } impl UnifiedExecSessionManager { + pub(crate) async fn allocate_process_id(&self) -> String { + loop { + let mut store = self.used_session_ids.lock().await; + + let process_id = if !cfg!(test) && !cfg!(feature = "deterministic_process_ids") { + // production mode → random + rand::rng().random_range(1_000..100_000).to_string() + } else { + // test or deterministic mode + let next = store + .iter() + .filter_map(|s| s.parse::().ok()) + .max() + .map(|m| std::cmp::max(m, 999) + 1) + .unwrap_or(1000); + + next.to_string() + }; + + if store.contains(&process_id) { + continue; + } + + store.insert(process_id.clone()); + return process_id; + } + } + pub(crate) async fn exec_command( &self, request: ExecCommandRequest, @@ -122,14 +151,20 @@ impl UnifiedExecSessionManager { let has_exited = session.has_exited(); let exit_code = session.exit_code(); let chunk_id = generate_chunk_id(); - let session_id = if has_exited { + let process_id = if has_exited { None } else { // Only store session if not exited. - let stored_id = self - .store_session(session, context, &request.command, cwd.clone(), start) - .await; - Some(stored_id) + self.store_session( + session, + context, + &request.command, + cwd.clone(), + start, + request.process_id.clone(), + ) + .await; + Some(request.process_id.clone()) }; let original_token_count = approx_token_count(&text); @@ -138,18 +173,18 @@ impl UnifiedExecSessionManager { chunk_id, wall_time, output, - session_id, + process_id: process_id.clone(), exit_code, original_token_count: Some(original_token_count), session_command: Some(request.command.clone()), }; - if response.session_id.is_some() { + if !has_exited { Self::emit_waiting_status(&context.session, &context.turn, &request.command).await; } // If the command completed during this call, emit an ExecCommandEnd via the emitter. - if response.session_id.is_none() { + if has_exited { let exit = response.exit_code.unwrap_or(-1); Self::emit_exec_end_from_context( context, @@ -158,6 +193,9 @@ impl UnifiedExecSessionManager { response.output.clone(), exit, response.wall_time, + // We always emit the process ID in order to keep consistency between the Begin + // event and the End event. + Some(request.process_id), ) .await; } @@ -169,7 +207,7 @@ impl UnifiedExecSessionManager { &self, request: WriteStdinRequest<'_>, ) -> Result { - let session_id = request.session_id; + let process_id = request.process_id.to_string(); let PreparedSessionHandles { writer_tx, @@ -180,13 +218,15 @@ impl UnifiedExecSessionManager { turn_ref, command: session_command, cwd: session_cwd, - } = self.prepare_session_handles(session_id).await?; + process_id, + } = self.prepare_session_handles(process_id.as_str()).await?; let interaction_emitter = ToolEmitter::unified_exec( &session_command, session_cwd.clone(), ExecCommandSource::UnifiedExecInteraction, (!request.input.is_empty()).then(|| request.input.to_string()), + Some(process_id.clone()), ); let make_event_ctx = || { ToolEventCtx::new( @@ -233,17 +273,21 @@ impl UnifiedExecSessionManager { let original_token_count = approx_token_count(&text); let chunk_id = generate_chunk_id(); - let status = self.refresh_session_state(session_id).await; - let (session_id, exit_code, completion_entry, event_call_id) = match status { - SessionStatus::Alive { exit_code, call_id } => { - (Some(session_id), exit_code, None, call_id) - } + let status = self.refresh_session_state(process_id.as_str()).await; + let (process_id, exit_code, completion_entry, event_call_id) = match status { + SessionStatus::Alive { + exit_code, + call_id, + process_id, + } => (Some(process_id), exit_code, None, call_id), SessionStatus::Exited { exit_code, entry } => { let call_id = entry.call_id.clone(); (None, exit_code, Some(*entry), call_id) } SessionStatus::Unknown => { - return Err(UnifiedExecError::UnknownSessionId { session_id }); + return Err(UnifiedExecError::UnknownSessionId { + process_id: request.process_id.to_string(), + }); } }; @@ -252,7 +296,7 @@ impl UnifiedExecSessionManager { chunk_id, wall_time, output, - session_id, + process_id, exit_code, original_token_count: Some(original_token_count), session_command: Some(session_command.clone()), @@ -273,7 +317,7 @@ impl UnifiedExecSessionManager { ) .await; - if response.session_id.is_some() { + if response.process_id.is_some() { Self::emit_waiting_status(&session_ref, &turn_ref, &session_command).await; } @@ -286,16 +330,17 @@ impl UnifiedExecSessionManager { Ok(response) } - async fn refresh_session_state(&self, session_id: i32) -> SessionStatus { + async fn refresh_session_state(&self, process_id: &str) -> SessionStatus { let mut sessions = self.sessions.lock().await; - let Some(entry) = sessions.get(&session_id) else { + let Some(entry) = sessions.get(process_id) else { return SessionStatus::Unknown; }; let exit_code = entry.session.exit_code(); + let process_id = entry.process_id.clone(); if entry.session.has_exited() { - let Some(entry) = sessions.remove(&session_id) else { + let Some(entry) = sessions.remove(&process_id) else { return SessionStatus::Unknown; }; SessionStatus::Exited { @@ -306,18 +351,21 @@ impl UnifiedExecSessionManager { SessionStatus::Alive { exit_code, call_id: entry.call_id.clone(), + process_id, } } } async fn prepare_session_handles( &self, - session_id: i32, + process_id: &str, ) -> Result { let mut sessions = self.sessions.lock().await; let entry = sessions - .get_mut(&session_id) - .ok_or(UnifiedExecError::UnknownSessionId { session_id })?; + .get_mut(process_id) + .ok_or(UnifiedExecError::UnknownSessionId { + process_id: process_id.to_string(), + })?; entry.last_used = Instant::now(); let OutputHandles { output_buffer, @@ -334,6 +382,7 @@ impl UnifiedExecSessionManager { turn_ref: Arc::clone(&entry.turn_ref), command: entry.command.clone(), cwd: entry.cwd.clone(), + process_id: entry.process_id.clone(), }) } @@ -347,6 +396,7 @@ impl UnifiedExecSessionManager { .map_err(|_| UnifiedExecError::WriteToStdin) } + #[allow(clippy::too_many_arguments)] async fn store_session( &self, session: UnifiedExecSession, @@ -354,15 +404,14 @@ impl UnifiedExecSessionManager { command: &[String], cwd: PathBuf, started_at: Instant, - ) -> i32 { - let session_id = self - .next_session_id - .fetch_add(1, std::sync::atomic::Ordering::SeqCst); + process_id: String, + ) { let entry = SessionEntry { session, session_ref: Arc::clone(&context.session), turn_ref: Arc::clone(&context.turn), call_id: context.call_id.clone(), + process_id: process_id.clone(), command: command.to_vec(), cwd, started_at, @@ -370,8 +419,7 @@ impl UnifiedExecSessionManager { }; let mut sessions = self.sessions.lock().await; Self::prune_sessions_if_needed(&mut sessions); - sessions.insert(session_id, entry); - session_id + sessions.insert(process_id, entry); } async fn emit_exec_end_from_entry( @@ -399,6 +447,7 @@ impl UnifiedExecSessionManager { entry.cwd, ExecCommandSource::UnifiedExecStartup, None, + Some(entry.process_id.clone()), ); emitter .emit(event_ctx, ToolEventStage::Success(output)) @@ -412,6 +461,7 @@ impl UnifiedExecSessionManager { aggregated_output: String, exit_code: i32, duration: Duration, + process_id: Option, ) { let output = ExecToolCallOutput { exit_code, @@ -427,8 +477,13 @@ impl UnifiedExecSessionManager { &context.call_id, None, ); - let emitter = - ToolEmitter::unified_exec(command, cwd, ExecCommandSource::UnifiedExecStartup, None); + let emitter = ToolEmitter::unified_exec( + command, + cwd, + ExecCommandSource::UnifiedExecStartup, + None, + process_id, + ); emitter .emit(event_ctx, ToolEventStage::Success(output)) .await; @@ -574,14 +629,14 @@ impl UnifiedExecSessionManager { collected } - fn prune_sessions_if_needed(sessions: &mut HashMap) { + fn prune_sessions_if_needed(sessions: &mut HashMap) { if sessions.len() < MAX_UNIFIED_EXEC_SESSIONS { return; } - let meta: Vec<(i32, Instant, bool)> = sessions + let meta: Vec<(String, Instant, bool)> = sessions .iter() - .map(|(id, entry)| (*id, entry.last_used, entry.session.has_exited())) + .map(|(id, entry)| (id.clone(), entry.last_used, entry.session.has_exited())) .collect(); if let Some(session_id) = Self::session_id_to_prune_from_meta(&meta) { @@ -590,32 +645,32 @@ impl UnifiedExecSessionManager { } // Centralized pruning policy so we can easily swap strategies later. - fn session_id_to_prune_from_meta(meta: &[(i32, Instant, bool)]) -> Option { + fn session_id_to_prune_from_meta(meta: &[(String, Instant, bool)]) -> Option { if meta.is_empty() { return None; } let mut by_recency = meta.to_vec(); by_recency.sort_by_key(|(_, last_used, _)| Reverse(*last_used)); - let protected: HashSet = by_recency + let protected: HashSet = by_recency .iter() .take(8) - .map(|(session_id, _, _)| *session_id) + .map(|(process_id, _, _)| process_id.clone()) .collect(); let mut lru = meta.to_vec(); lru.sort_by_key(|(_, last_used, _)| *last_used); - if let Some((session_id, _, _)) = lru + if let Some((process_id, _, _)) = lru .iter() - .find(|(session_id, _, exited)| !protected.contains(session_id) && *exited) + .find(|(process_id, _, exited)| !protected.contains(process_id) && *exited) { - return Some(*session_id); + return Some(process_id.clone()); } lru.into_iter() - .find(|(session_id, _, _)| !protected.contains(session_id)) - .map(|(session_id, _, _)| session_id) + .find(|(process_id, _, _)| !protected.contains(process_id)) + .map(|(process_id, _, _)| process_id) } pub(crate) async fn terminate_all_sessions(&self) { @@ -628,6 +683,7 @@ enum SessionStatus { Alive { exit_code: Option, call_id: String, + process_id: String, }, Exited { exit_code: Option, @@ -675,64 +731,67 @@ mod tests { #[test] fn pruning_prefers_exited_sessions_outside_recently_used() { let now = Instant::now(); + let id = |n: i32| n.to_string(); let meta = vec![ - (1, now - Duration::from_secs(40), false), - (2, now - Duration::from_secs(30), true), - (3, now - Duration::from_secs(20), false), - (4, now - Duration::from_secs(19), false), - (5, now - Duration::from_secs(18), false), - (6, now - Duration::from_secs(17), false), - (7, now - Duration::from_secs(16), false), - (8, now - Duration::from_secs(15), false), - (9, now - Duration::from_secs(14), false), - (10, now - Duration::from_secs(13), false), + (id(1), now - Duration::from_secs(40), false), + (id(2), now - Duration::from_secs(30), true), + (id(3), now - Duration::from_secs(20), false), + (id(4), now - Duration::from_secs(19), false), + (id(5), now - Duration::from_secs(18), false), + (id(6), now - Duration::from_secs(17), false), + (id(7), now - Duration::from_secs(16), false), + (id(8), now - Duration::from_secs(15), false), + (id(9), now - Duration::from_secs(14), false), + (id(10), now - Duration::from_secs(13), false), ]; let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta); - assert_eq!(candidate, Some(2)); + assert_eq!(candidate, Some(id(2))); } #[test] fn pruning_falls_back_to_lru_when_no_exited() { let now = Instant::now(); + let id = |n: i32| n.to_string(); let meta = vec![ - (1, now - Duration::from_secs(40), false), - (2, now - Duration::from_secs(30), false), - (3, now - Duration::from_secs(20), false), - (4, now - Duration::from_secs(19), false), - (5, now - Duration::from_secs(18), false), - (6, now - Duration::from_secs(17), false), - (7, now - Duration::from_secs(16), false), - (8, now - Duration::from_secs(15), false), - (9, now - Duration::from_secs(14), false), - (10, now - Duration::from_secs(13), false), + (id(1), now - Duration::from_secs(40), false), + (id(2), now - Duration::from_secs(30), false), + (id(3), now - Duration::from_secs(20), false), + (id(4), now - Duration::from_secs(19), false), + (id(5), now - Duration::from_secs(18), false), + (id(6), now - Duration::from_secs(17), false), + (id(7), now - Duration::from_secs(16), false), + (id(8), now - Duration::from_secs(15), false), + (id(9), now - Duration::from_secs(14), false), + (id(10), now - Duration::from_secs(13), false), ]; let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta); - assert_eq!(candidate, Some(1)); + assert_eq!(candidate, Some(id(1))); } #[test] fn pruning_protects_recent_sessions_even_if_exited() { let now = Instant::now(); + let id = |n: i32| n.to_string(); let meta = vec![ - (1, now - Duration::from_secs(40), false), - (2, now - Duration::from_secs(30), false), - (3, now - Duration::from_secs(20), true), - (4, now - Duration::from_secs(19), false), - (5, now - Duration::from_secs(18), false), - (6, now - Duration::from_secs(17), false), - (7, now - Duration::from_secs(16), false), - (8, now - Duration::from_secs(15), false), - (9, now - Duration::from_secs(14), false), - (10, now - Duration::from_secs(13), true), + (id(1), now - Duration::from_secs(40), false), + (id(2), now - Duration::from_secs(30), false), + (id(3), now - Duration::from_secs(20), true), + (id(4), now - Duration::from_secs(19), false), + (id(5), now - Duration::from_secs(18), false), + (id(6), now - Duration::from_secs(17), false), + (id(7), now - Duration::from_secs(16), false), + (id(8), now - Duration::from_secs(15), false), + (id(9), now - Duration::from_secs(14), false), + (id(10), now - Duration::from_secs(13), true), ]; let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta); // (10) is exited but among the last 8; we should drop the LRU outside that set. - assert_eq!(candidate, Some(1)); + assert_eq!(candidate, Some(id(1))); } } diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 404801658..63964b241 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -44,7 +44,7 @@ fn extract_output_text(item: &Value) -> Option<&str> { struct ParsedUnifiedExecOutput { chunk_id: Option, wall_time_seconds: f64, - session_id: Option, + process_id: Option, exit_code: Option, original_token_count: Option, output: String, @@ -59,7 +59,7 @@ fn parse_unified_exec_output(raw: &str) -> Result { r#"(?:Chunk ID: (?P[^\n]+)\n)?"#, r#"Wall time: (?P-?\d+(?:\.\d+)?) seconds\n"#, r#"(?:Process exited with code (?P-?\d+)\n)?"#, - r#"(?:Process running with session ID (?P-?\d+)\n)?"#, + r#"(?:Process running with session ID (?P-?\d+)\n)?"#, r#"(?:Original token count: (?P\d+)\n)?"#, r#"Output:\n?(?P.*)$"#, )) @@ -92,15 +92,9 @@ fn parse_unified_exec_output(raw: &str) -> Result { }) .transpose()?; - let session_id = captures - .name("session_id") - .map(|value| { - value - .as_str() - .parse::() - .context("failed to parse session id from unified exec output") - }) - .transpose()?; + let process_id = captures + .name("process_id") + .map(|value| value.as_str().to_string()); let original_token_count = captures .name("original_token_count") @@ -121,7 +115,7 @@ fn parse_unified_exec_output(raw: &str) -> Result { Ok(ParsedUnifiedExecOutput { chunk_id, wall_time_seconds, - session_id, + process_id, exit_code, original_token_count, output, @@ -335,7 +329,7 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> { let poll_call_id = "uexec-end-event-poll"; let poll_args = json!({ "chars": "", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 250, }); @@ -493,7 +487,7 @@ async fn unified_exec_emits_output_delta_for_write_stdin() -> Result<()> { let stdin_call_id = "uexec-stdin-delta"; let stdin_args = json!({ "chars": "echo WSTDIN-MARK\\n", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 800, }); @@ -592,7 +586,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> { let stdin_call_id = "uexec-stdin-begin"; let stdin_args = json!({ "chars": "echo hello", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 400, }); @@ -694,7 +688,7 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> let poll_call_id = "uexec-poll-empty"; let poll_args = json!({ "chars": "", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 150, }); @@ -880,8 +874,8 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> { ); assert!( - metadata.session_id.is_none(), - "exec_command for a completed process should not include session_id" + metadata.process_id.is_none(), + "exec_command for a completed process should not include process_id" ); let exit_code = metadata.exit_code.expect("expected exit_code"); @@ -973,7 +967,7 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> { .expect("missing early exit unified_exec output"); assert!( - output.session_id.is_none(), + output.process_id.is_none(), "short-lived process should not keep a session alive" ); assert_eq!( @@ -1023,12 +1017,12 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> { }); let send_args = serde_json::json!({ "chars": "hello unified exec\n", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 500, }); let exit_args = serde_json::json!({ "chars": "\u{0004}", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 500, }); @@ -1099,12 +1093,13 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> { let start_output = outputs .get(start_call_id) .expect("missing start output for exec_command"); - let session_id = start_output - .session_id - .expect("expected session id from exec_command"); + let process_id = start_output + .process_id + .clone() + .expect("expected process id from exec_command"); assert!( - session_id >= 0, - "session_id should be non-negative, got {session_id}" + process_id.len() > 3, + "process_id should be at least 4 digits, got {process_id}" ); assert!( start_output.exit_code.is_none(), @@ -1120,11 +1115,12 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> { "expected echoed output from cat, got {echoed:?}" ); let echoed_session = send_output - .session_id - .expect("write_stdin should return session id while process is running"); + .process_id + .clone() + .expect("write_stdin should return process id while process is running"); assert_eq!( - echoed_session, session_id, - "write_stdin should reuse existing session id" + echoed_session, process_id, + "write_stdin should reuse existing process id" ); assert!( send_output.exit_code.is_none(), @@ -1135,8 +1131,8 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> { .get(exit_call_id) .expect("missing exit metadata output"); assert!( - exit_output.session_id.is_none(), - "session_id should be omitted once the process exits" + exit_output.process_id.is_none(), + "process_id should be omitted once the process exits" ); let exit_code = exit_output .exit_code @@ -1182,14 +1178,14 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<() let echo_call_id = "uexec-end-on-exit-echo"; let echo_args = serde_json::json!({ "chars": "bye-END\n", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 300, }); let exit_call_id = "uexec-end-on-exit"; let exit_args = serde_json::json!({ "chars": "\u{0004}", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 500, }); @@ -1285,7 +1281,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> { let second_call_id = "uexec-stdin"; let second_args = serde_json::json!({ "chars": "hello unified exec\n", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 500, }); @@ -1347,17 +1343,20 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> { let start_output = outputs .get(first_call_id) .expect("missing first unified_exec output"); - let session_id = start_output.session_id.unwrap_or_default(); + let process_id = start_output.process_id.clone().unwrap_or_default(); assert!( - session_id >= 0, - "expected session id in first unified_exec response" + !process_id.is_empty(), + "expected process id in first unified_exec response" ); assert!(start_output.output.is_empty()); let reuse_output = outputs .get(second_call_id) .expect("missing reused unified_exec output"); - assert_eq!(reuse_output.session_id.unwrap_or_default(), session_id); + assert_eq!( + reuse_output.process_id.clone().unwrap_or_default(), + process_id + ); let echoed = reuse_output.output.as_str(); assert!( echoed.contains("hello unified exec"), @@ -1413,7 +1412,7 @@ PY let second_call_id = "uexec-lag-poll"; let second_args = serde_json::json!({ "chars": "", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 2_000, }); @@ -1480,9 +1479,9 @@ PY let start_output = outputs .get(first_call_id) .expect("missing initial unified_exec output"); - let session_id = start_output.session_id.unwrap_or_default(); + let process_id = start_output.process_id.clone().unwrap_or_default(); assert!( - session_id >= 0, + !process_id.is_empty(), "expected session id from initial unified_exec response" ); @@ -1524,7 +1523,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> { let second_call_id = "uexec-poll"; let second_args = serde_json::json!({ "chars": "", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 800, }); @@ -1589,7 +1588,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> { let outputs = collect_tool_outputs(&bodies)?; let first_output = outputs.get(first_call_id).expect("missing timeout output"); - assert_eq!(first_output.session_id, Some(0)); + assert!(first_output.process_id.is_some()); assert!(first_output.output.is_empty()); let poll_output = outputs.get(second_call_id).expect("missing poll output"); @@ -1824,7 +1823,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { let keep_write_call_id = "uexec-prune-keep-write"; let keep_write_args = serde_json::json!({ "chars": "still alive\n", - "session_id": 0, + "session_id": 1000, "yield_time_ms": 500, }); events.push(ev_function_call( @@ -1836,7 +1835,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { let probe_call_id = "uexec-prune-probe"; let probe_args = serde_json::json!({ "chars": "should fail\n", - "session_id": 1, + "session_id": 1001, "yield_time_ms": 500, }); events.push(ev_function_call( @@ -1885,7 +1884,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { .find_map(|req| req.function_call_output_text(keep_call_id)) .expect("missing initial keep session output"); let keep_start_output = parse_unified_exec_output(&keep_start)?; - pretty_assertions::assert_eq!(keep_start_output.session_id, Some(0)); + assert!(keep_start_output.process_id.is_some()); assert!(keep_start_output.exit_code.is_none()); let prune_start = requests @@ -1893,7 +1892,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { .find_map(|req| req.function_call_output_text(prune_call_id)) .expect("missing initial prune session output"); let prune_start_output = parse_unified_exec_output(&prune_start)?; - pretty_assertions::assert_eq!(prune_start_output.session_id, Some(1)); + assert!(prune_start_output.process_id.is_some()); assert!(prune_start_output.exit_code.is_none()); let keep_write = requests @@ -1901,7 +1900,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { .find_map(|req| req.function_call_output_text(keep_write_call_id)) .expect("missing keep write output"); let keep_write_output = parse_unified_exec_output(&keep_write)?; - pretty_assertions::assert_eq!(keep_write_output.session_id, Some(0)); + assert!(keep_write_output.process_id.is_some()); assert!( keep_write_output.output.contains("still alive"), "expected cat session to echo input, got {:?}", @@ -1913,7 +1912,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { .find_map(|req| req.function_call_output_text(probe_call_id)) .expect("missing probe output"); assert!( - pruned_probe.contains("UnknownSessionId") || pruned_probe.contains("Unknown session id"), + pruned_probe.contains("UnknownSessionId") || pruned_probe.contains("Unknown process id"), "expected probe to fail after pruning, got {pruned_probe:?}" ); diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 06f99b96b..7b2d3c18b 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -637,6 +637,7 @@ fn exec_command_end_success_produces_completed_command_item() { "c1", EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: "1".to_string(), + process_id: None, turn_id: "turn-1".to_string(), command: command.clone(), cwd: cwd.clone(), @@ -666,6 +667,7 @@ fn exec_command_end_success_produces_completed_command_item() { "c2", EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "1".to_string(), + process_id: None, turn_id: "turn-1".to_string(), command, cwd, @@ -709,6 +711,7 @@ fn exec_command_end_failure_produces_failed_command_item() { "c1", EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: "2".to_string(), + process_id: None, turn_id: "turn-1".to_string(), command: command.clone(), cwd: cwd.clone(), @@ -737,6 +740,7 @@ fn exec_command_end_failure_produces_failed_command_item() { "c2", EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "2".to_string(), + process_id: None, turn_id: "turn-1".to_string(), command, cwd, @@ -777,6 +781,7 @@ fn exec_command_end_without_begin_is_ignored() { "c1", EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "no-begin".to_string(), + process_id: None, turn_id: "turn-1".to_string(), command: Vec::new(), cwd: PathBuf::from("."), diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2fc47a50c..60c4dffb2 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1328,6 +1328,10 @@ impl Default for ExecCommandSource { pub struct ExecCommandBeginEvent { /// Identifier so this can be paired with the ExecCommandEnd event. pub call_id: String, + /// Identifier for the underlying PTY process (when available). + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub process_id: Option, /// Turn ID that this command belongs to. pub turn_id: String, /// The command to be executed. @@ -1348,6 +1352,10 @@ pub struct ExecCommandBeginEvent { pub struct ExecCommandEndEvent { /// Identifier for the ExecCommandBegin that finished. pub call_id: String, + /// Identifier for the underlying PTY process (when available). + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub process_id: Option, /// Turn ID that this command belongs to. pub turn_id: String, /// The command that was executed. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index fde52a064..2eebdaf23 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -722,6 +722,7 @@ fn begin_exec_with_source( let interaction_input = None; let event = ExecCommandBeginEvent { call_id: call_id.to_string(), + process_id: None, turn_id: "turn-1".to_string(), command, cwd, @@ -760,11 +761,13 @@ fn end_exec( parsed_cmd, source, interaction_input, + process_id, } = begin_event; chat.handle_codex_event(Event { id: call_id.clone(), msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, + process_id, turn_id, command, cwd, @@ -2880,6 +2883,7 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() { id: "c1".into(), msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id: "c1".into(), + process_id: None, turn_id: "turn-1".into(), command: command.clone(), cwd: cwd.clone(), @@ -2892,6 +2896,7 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() { id: "c1".into(), msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "c1".into(), + process_id: None, turn_id: "turn-1".into(), command, cwd,