From 7e2165f3944663c75cb840ca26a8f510f38fb7da Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Thu, 20 Nov 2025 17:02:37 -0800 Subject: [PATCH 01/13] [app-server] update doc with codex error info (#6941) Document new codex error info. Also fixed the name from `codex_error_code` to `codex_error_info`. --- .../app-server-protocol/src/protocol/v2.rs | 7 ++++-- codex-rs/app-server/README.md | 23 +++++++++++++++++++ .../app-server/src/bespoke_event_handling.rs | 22 +++++++++--------- codex-rs/core/src/codex.rs | 6 ++--- codex-rs/core/src/error.rs | 4 ++-- .../tests/event_processor_with_json_output.rs | 6 ++--- codex-rs/protocol/src/protocol.rs | 4 ++-- codex-rs/tui/src/chatwidget/tests.rs | 2 +- 8 files changed, 50 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 6ecf0f9256..1f8463a60a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -50,6 +50,9 @@ macro_rules! v2_enum_from_core { } /// This translation layer make sure that we expose codex error code in camel case. +/// +/// When an upstream HTTP status is available (for example, from the Responses API or a provider), +/// it is forwarded in `httpStatusCode` on the relevant `codexErrorInfo` variant. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -615,7 +618,7 @@ pub struct Turn { #[error("{message}")] pub struct TurnError { pub message: String, - pub codex_error_code: Option, + pub codex_error_info: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -1253,7 +1256,7 @@ mod tests { } #[test] - fn codex_error_code_serializes_http_status_code_in_camel_case() { + fn codex_error_info_serializes_http_status_code_in_camel_case() { let value = CodexErrorInfo::ResponseTooManyFailedAttempts { http_status_code: Some(401), }; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 35653099aa..d5ac8a57d0 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -339,6 +339,29 @@ Event notifications are the server-initiated event stream for thread lifecycles, The app-server streams JSON-RPC notifications while a turn is running. Each turn starts with `turn/started` (initial `turn`) and ends with `turn/completed` (final `turn` plus token `usage`), and clients subscribe to the events they care about, rendering each item incrementally as updates arrive. The per-item lifecycle is always: `item/started` → zero or more item-specific deltas → `item/completed`. +- `turn/started` — `{ turn }` with the turn id, empty `items`, and `status: "inProgress"`. +- `turn/completed` — `{ turn }` where `turn.status` is `completed`, `interrupted`, or `failed`; failures carry `{ error: { message, codexErrorInfo? } }`. + +Today both notifications carry an empty `items` array even when item events were streamed; rely on `item/*` notifications for the canonical item list until this is fixed. + +#### Errors +`error` event is emitted whenever the server hits an error mid-turn (for example, upstream model errors or quota limits). Carries the same `{ error: { message, codexErrorInfo? } }` payload as `turn.status: "failed"` and may precede that terminal notification. + + `codexErrorInfo` maps to the `CodexErrorInfo` enum. Common values: + - `ContextWindowExceeded` + - `UsageLimitExceeded` + - `HttpConnectionFailed { httpStatusCode? }`: upstream HTTP failures including 4xx/5xx + - `ResponseStreamConnectionFailed { httpStatusCode? }`: failure to connect to the response SSE stream + - `ResponseStreamDisconnected { httpStatusCode? }`: disconnect of the response SSE stream in the middle of a turn before completion + - `ResponseTooManyFailedAttempts { httpStatusCode? }` + - `BadRequest` + - `Unauthorized` + - `SandboxError` + - `InternalServerError` + - `Other`: all unclassified errors + +When an upstream HTTP status is available (for example, from the Responses API or a provider), it is forwarded in `httpStatusCode` on the relevant `codexErrorInfo` variant. + #### Thread items `ThreadItem` is the tagged union carried in turn responses and `item/*` notifications. Currently we support events for the following items: diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 69346d2e31..387aabc32d 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -264,7 +264,7 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::Error(ev) => { let turn_error = TurnError { message: ev.message, - codex_error_code: ev.codex_error_code.map(V2CodexErrorInfo::from), + codex_error_info: ev.codex_error_info.map(V2CodexErrorInfo::from), }; handle_error(conversation_id, turn_error.clone(), &turn_summary_store).await; outgoing @@ -278,7 +278,7 @@ pub(crate) async fn apply_bespoke_event_handling( // but we notify the client. let turn_error = TurnError { message: ev.message, - codex_error_code: ev.codex_error_code.map(V2CodexErrorInfo::from), + codex_error_info: ev.codex_error_info.map(V2CodexErrorInfo::from), }; outgoing .send_server_notification(ServerNotification::Error(ErrorNotification { @@ -899,7 +899,7 @@ mod tests { conversation_id, TurnError { message: "boom".to_string(), - codex_error_code: Some(V2CodexErrorInfo::InternalServerError), + codex_error_info: Some(V2CodexErrorInfo::InternalServerError), }, &turn_summary_store, ) @@ -910,7 +910,7 @@ mod tests { turn_summary.last_error, Some(TurnError { message: "boom".to_string(), - codex_error_code: Some(V2CodexErrorInfo::InternalServerError), + codex_error_info: Some(V2CodexErrorInfo::InternalServerError), }) ); Ok(()) @@ -956,7 +956,7 @@ mod tests { conversation_id, TurnError { message: "oops".to_string(), - codex_error_code: None, + codex_error_info: None, }, &turn_summary_store, ) @@ -996,7 +996,7 @@ mod tests { conversation_id, TurnError { message: "bad".to_string(), - codex_error_code: Some(V2CodexErrorInfo::Other), + codex_error_info: Some(V2CodexErrorInfo::Other), }, &turn_summary_store, ) @@ -1024,7 +1024,7 @@ mod tests { TurnStatus::Failed { error: TurnError { message: "bad".to_string(), - codex_error_code: Some(V2CodexErrorInfo::Other), + codex_error_info: Some(V2CodexErrorInfo::Other), } } ); @@ -1079,7 +1079,7 @@ mod tests { conversation_a, TurnError { message: "a1".to_string(), - codex_error_code: Some(V2CodexErrorInfo::BadRequest), + codex_error_info: Some(V2CodexErrorInfo::BadRequest), }, &turn_summary_store, ) @@ -1098,7 +1098,7 @@ mod tests { conversation_b, TurnError { message: "b1".to_string(), - codex_error_code: None, + codex_error_info: None, }, &turn_summary_store, ) @@ -1134,7 +1134,7 @@ mod tests { TurnStatus::Failed { error: TurnError { message: "a1".to_string(), - codex_error_code: Some(V2CodexErrorInfo::BadRequest), + codex_error_info: Some(V2CodexErrorInfo::BadRequest), } } ); @@ -1155,7 +1155,7 @@ mod tests { TurnStatus::Failed { error: TurnError { message: "b1".to_string(), - codex_error_code: None, + codex_error_info: None, } } ); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 098db0c4fe..a04b05cb38 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1191,12 +1191,12 @@ impl Session { message: impl Into, codex_error: CodexErr, ) { - let codex_error_code = CodexErrorInfo::ResponseStreamDisconnected { + let codex_error_info = CodexErrorInfo::ResponseStreamDisconnected { http_status_code: codex_error.http_status_code_value(), }; let event = EventMsg::StreamError(StreamErrorEvent { message: message.into(), - codex_error_code: Some(codex_error_code), + codex_error_info: Some(codex_error_info), }); self.send_event(turn_context, event).await; } @@ -1689,7 +1689,7 @@ mod handlers { id: sub_id.clone(), msg: EventMsg::Error(ErrorEvent { message: "Failed to shutdown rollout recorder".to_string(), - codex_error_code: Some(CodexErrorInfo::Other), + codex_error_info: Some(CodexErrorInfo::Other), }), }; sess.send_event_raw(event).await; diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index ee72930499..9130b40e1e 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -469,7 +469,7 @@ impl CodexErr { }; ErrorEvent { message, - codex_error_code: Some(self.to_codex_protocol_error()), + codex_error_info: Some(self.to_codex_protocol_error()), } } @@ -650,7 +650,7 @@ mod tests { "prefix: Error while reading the server response: HTTP status client error (429 Too Many Requests) for url (http://example.com/), request id: req-123" ); assert_eq!( - event.codex_error_code, + event.codex_error_info, Some(CodexErrorInfo::ResponseStreamConnectionFailed { http_status_code: Some(429) }) 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 f716f0f0f4..06f99b96b1 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -540,7 +540,7 @@ fn error_event_produces_error() { "e1", EventMsg::Error(codex_core::protocol::ErrorEvent { message: "boom".to_string(), - codex_error_code: Some(CodexErrorInfo::Other), + codex_error_info: Some(CodexErrorInfo::Other), }), )); assert_eq!( @@ -580,7 +580,7 @@ fn stream_error_event_produces_error() { "e1", EventMsg::StreamError(codex_core::protocol::StreamErrorEvent { message: "retrying".to_string(), - codex_error_code: Some(CodexErrorInfo::Other), + codex_error_info: Some(CodexErrorInfo::Other), }), )); assert_eq!( @@ -599,7 +599,7 @@ fn error_followed_by_task_complete_produces_turn_failed() { "e1", EventMsg::Error(ErrorEvent { message: "boom".to_string(), - codex_error_code: Some(CodexErrorInfo::Other), + codex_error_info: Some(CodexErrorInfo::Other), }), ); assert_eq!( diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 5806e1a6f0..e0c2e4df3f 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -716,7 +716,7 @@ pub struct ExitedReviewModeEvent { pub struct ErrorEvent { pub message: String, #[serde(default)] - pub codex_error_code: Option, + pub codex_error_info: Option, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] @@ -1395,7 +1395,7 @@ pub struct UndoCompletedEvent { pub struct StreamErrorEvent { pub message: String, #[serde(default)] - pub codex_error_code: Option, + pub codex_error_info: Option, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 45a9a8df21..61e23fb3fc 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2648,7 +2648,7 @@ fn stream_error_updates_status_indicator() { id: "sub-1".into(), msg: EventMsg::StreamError(StreamErrorEvent { message: msg.to_string(), - codex_error_code: Some(CodexErrorInfo::Other), + codex_error_info: Some(CodexErrorInfo::Other), }), }); From 1822ffe870e65da2b3fc596bc75296dc5acf8422 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Thu, 20 Nov 2025 17:06:04 -0800 Subject: [PATCH 02/13] feat(tui): default reasoning selection to medium (#7040) ## Summary - allow selection popups to request an initial highlighted row - begin the /models reasoning selector focused on the default effort ## Testing - just fmt - just fix -p codex-tui - cargo test -p codex-tui https://github.com/user-attachments/assets/b322aeb1-e8f3-4578-92f7-5c2fa5ee4c98 ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_691f75e8fc188322a910fbe2138666ef) --- codex-rs/tui/src/bottom_pane/list_selection_view.rs | 7 ++++++- codex-rs/tui/src/chatwidget.rs | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 88fbaf5b53..d294a47265 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -52,6 +52,7 @@ pub(crate) struct SelectionViewParams { pub is_searchable: bool, pub search_placeholder: Option, pub header: Box, + pub initial_selected_idx: Option, } impl Default for SelectionViewParams { @@ -64,6 +65,7 @@ impl Default for SelectionViewParams { is_searchable: false, search_placeholder: None, header: Box::new(()), + initial_selected_idx: None, } } } @@ -80,6 +82,7 @@ pub(crate) struct ListSelectionView { filtered_indices: Vec, last_selected_actual_idx: Option, header: Box, + initial_selected_idx: Option, } impl ListSelectionView { @@ -110,6 +113,7 @@ impl ListSelectionView { filtered_indices: Vec::new(), last_selected_actual_idx: None, header, + initial_selected_idx: params.initial_selected_idx, }; s.apply_filter(); s @@ -132,7 +136,8 @@ impl ListSelectionView { (!self.is_searchable) .then(|| self.items.iter().position(|item| item.is_current)) .flatten() - }); + }) + .or_else(|| self.initial_selected_idx.take()); if self.is_searchable && !self.search_query.is_empty() { let query_lower = self.search_query.to_lowercase(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index eb9389b46c..a371fa8cb8 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2120,6 +2120,14 @@ impl ChatWidget { } else { default_choice }; + let selection_choice = highlight_choice.or(default_choice); + let initial_selected_idx = choices + .iter() + .position(|choice| choice.stored == selection_choice) + .or_else(|| { + selection_choice + .and_then(|effort| choices.iter().position(|choice| choice.display == effort)) + }); let mut items: Vec = Vec::new(); for choice in choices.iter() { let effort = choice.display; @@ -2196,6 +2204,7 @@ impl ChatWidget { header: Box::new(header), footer_hint: Some(standard_popup_hint_line()), items, + initial_selected_idx, ..Default::default() }); } From 3f73e2c892cdf16f1f3f63a9f050db36170f0b60 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Thu, 20 Nov 2025 19:18:39 -0800 Subject: [PATCH 03/13] fix(app-server) remove www warning (#7046) ### Summary After #7022, we no longer need this warning. We should also clean up the schema for the notification, but this is a quick fix to just stop the behavior in the VSCE ## Testing - [x] Ran locally --- codex-rs/Cargo.lock | 1 - codex-rs/Cargo.toml | 1 - codex-rs/app-server/Cargo.toml | 1 - .../app-server/src/codex_message_processor.rs | 56 ------------------- 4 files changed, 59 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 9e365adac4..fd83a54d87 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -860,7 +860,6 @@ dependencies = [ "codex-login", "codex-protocol", "codex-utils-json-to-toml", - "codex-windows-sandbox", "core_test_support", "mcp-types", "opentelemetry-appender-tracing", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index b34cd46b4b..226a91beff 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -169,7 +169,6 @@ reqwest = "0.12" rmcp = { version = "0.8.5", default-features = false } schemars = "0.8.22" seccompiler = "0.5.0" -sentry = "0.34.0" serde = "1" serde_json = "1" serde_with = "3.14" diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 84eb68c66d..96f64afdf5 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -40,7 +40,6 @@ tracing = { workspace = true, features = ["log"] } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } opentelemetry-appender-tracing = { workspace = true } uuid = { workspace = true, features = ["serde", "v7"] } -codex-windows-sandbox.workspace = true [dev-dependencies] app_test_support = { workspace = true } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 47468b357a..ae1bed31ce 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -92,7 +92,6 @@ use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInfoResponse; use codex_app_server_protocol::UserInput as V2UserInput; use codex_app_server_protocol::UserSavedConfig; -use codex_app_server_protocol::WindowsWorldWritableWarningNotification; use codex_app_server_protocol::build_turns_from_event_msgs; use codex_backend_client::Client as BackendClient; use codex_core::AuthManager; @@ -1277,10 +1276,6 @@ impl CodexMessageProcessor { return; } }; - if cfg!(windows) && config.features.enabled(Feature::WindowsSandbox) { - self.handle_windows_world_writable_warning(config.cwd.clone()) - .await; - } match self.conversation_manager.new_conversation(config).await { Ok(conversation_id) => { @@ -2000,10 +1995,6 @@ impl CodexMessageProcessor { return; } }; - if cfg!(windows) && config.features.enabled(Feature::WindowsSandbox) { - self.handle_windows_world_writable_warning(config.cwd.clone()) - .await; - } let conversation_history = if let Some(path) = path { match RolloutRecorder::get_rollout_history(&path).await { @@ -2862,53 +2853,6 @@ impl CodexMessageProcessor { Err(_) => None, } } - - /// On Windows, when using the experimental sandbox, we need to warn the user about world-writable directories. - async fn handle_windows_world_writable_warning(&self, cwd: PathBuf) { - if !cfg!(windows) { - return; - } - - if !self.config.features.enabled(Feature::WindowsSandbox) { - return; - } - - if !matches!( - self.config.sandbox_policy, - codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { .. } - | codex_protocol::protocol::SandboxPolicy::ReadOnly - ) { - return; - } - - if self - .config - .notices - .hide_world_writable_warning - .unwrap_or(false) - { - return; - } - - // This function is stubbed out to return None on non-Windows platforms - if let Some((sample_paths, extra_count, failed_scan)) = - codex_windows_sandbox::world_writable_warning_details( - self.config.codex_home.as_path(), - cwd, - ) - { - tracing::warn!("world writable warning: {sample_paths:?} {extra_count} {failed_scan}"); - self.outgoing - .send_server_notification(ServerNotification::WindowsWorldWritableWarning( - WindowsWorldWritableWarningNotification { - sample_paths, - extra_count, - failed_scan, - }, - )) - .await; - } - } } async fn derive_config_from_params( From 830ab4ce20aa0d2602ee07b729ee0d28c7b352d7 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 20 Nov 2025 19:29:15 -0800 Subject: [PATCH 04/13] Support full powershell paths in is_safe_command (#7055) New shell implementation always uses full paths. --- .../src/command_safety/is_safe_command.rs | 14 +++++++++ .../command_safety/windows_safe_commands.rs | 30 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/command_safety/is_safe_command.rs b/codex-rs/core/src/command_safety/is_safe_command.rs index ab084c191b..5e7a7034d8 100644 --- a/codex-rs/core/src/command_safety/is_safe_command.rs +++ b/codex-rs/core/src/command_safety/is_safe_command.rs @@ -267,6 +267,20 @@ mod tests { } } + #[test] + fn windows_powershell_full_path_is_safe() { + if !cfg!(windows) { + // Windows only because on Linux path splitting doesn't handle `/` separators properly + return; + } + + assert!(is_known_safe_command(&vec_str(&[ + r"C:\Program Files\PowerShell\7\pwsh.exe", + "-Command", + "Get-Location", + ]))); + } + #[test] fn bash_lc_safe_examples() { assert!(is_known_safe_command(&vec_str(&["bash", "-lc", "ls"]))); diff --git a/codex-rs/core/src/command_safety/windows_safe_commands.rs b/codex-rs/core/src/command_safety/windows_safe_commands.rs index ff0a3d2e7b..a1d3b297f9 100644 --- a/codex-rs/core/src/command_safety/windows_safe_commands.rs +++ b/codex-rs/core/src/command_safety/windows_safe_commands.rs @@ -1,4 +1,5 @@ use shlex::split as shlex_split; +use std::path::Path; /// On Windows, we conservatively allow only clearly read-only PowerShell invocations /// that match a small safelist. Anything else (including direct CMD commands) is unsafe. @@ -131,8 +132,14 @@ fn split_into_commands(tokens: Vec) -> Option>> { /// Returns true when the executable name is one of the supported PowerShell binaries. fn is_powershell_executable(exe: &str) -> bool { + let executable_name = Path::new(exe) + .file_name() + .and_then(|osstr| osstr.to_str()) + .unwrap_or(exe) + .to_ascii_lowercase(); + matches!( - exe.to_ascii_lowercase().as_str(), + executable_name.as_str(), "powershell" | "powershell.exe" | "pwsh" | "pwsh.exe" ) } @@ -313,6 +320,27 @@ mod tests { ]))); } + #[test] + fn accepts_full_path_powershell_invocations() { + if !cfg!(windows) { + // Windows only because on Linux path splitting doesn't handle `/` separators properly + return; + } + + assert!(is_safe_command_windows(&vec_str(&[ + r"C:\Program Files\PowerShell\7\pwsh.exe", + "-NoProfile", + "-Command", + "Get-ChildItem -Path .", + ]))); + + assert!(is_safe_command_windows(&vec_str(&[ + r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe", + "-Command", + "Get-Content Cargo.toml", + ]))); + } + #[test] fn allows_read_only_pipelines_and_git_usage() { assert!(is_safe_command_windows(&vec_str(&[ From 767b66f4070277ab9192d817ba4b886a58d9079a Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 20 Nov 2025 19:44:00 -0800 Subject: [PATCH 05/13] Migrate coverage to shell_command (#7042) --- codex-rs/Cargo.lock | 6 + codex-rs/app-server/Cargo.toml | 1 + codex-rs/app-server/tests/common/Cargo.toml | 2 + codex-rs/app-server/tests/common/lib.rs | 4 +- codex-rs/app-server/tests/common/responses.rs | 19 +-- .../suite/codex_message_processor_flow.rs | 39 ++---- codex-rs/app-server/tests/suite/interrupt.rs | 4 +- .../tests/suite/v2/turn_interrupt.rs | 4 +- .../app-server/tests/suite/v2/turn_start.rs | 34 ++--- codex-rs/apply-patch/src/lib.rs | 22 ++- codex-rs/core/tests/common/Cargo.toml | 1 + codex-rs/core/tests/common/lib.rs | 9 ++ codex-rs/core/tests/common/responses.rs | 5 +- codex-rs/core/tests/suite/abort_tasks.rs | 31 +++-- codex-rs/core/tests/suite/apply_patch_cli.rs | 24 +--- codex-rs/core/tests/suite/approvals.rs | 104 +++++++------- codex-rs/core/tests/suite/codex_delegate.rs | 11 +- codex-rs/core/tests/suite/exec_policy.rs | 13 +- codex-rs/core/tests/suite/tool_parallelism.rs | 23 ++-- codex-rs/core/tests/suite/truncation.rs | 129 ++++++++---------- codex-rs/core/tests/suite/user_shell_cmd.rs | 21 ++- codex-rs/mcp-server/tests/common/Cargo.toml | 2 + codex-rs/mcp-server/tests/common/lib.rs | 3 +- codex-rs/mcp-server/tests/common/responses.rs | 19 +-- codex-rs/mcp-server/tests/suite/codex_tool.rs | 16 ++- 25 files changed, 284 insertions(+), 262 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fd83a54d87..c891e76fa7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -187,8 +187,10 @@ dependencies = [ "codex-app-server-protocol", "codex-core", "codex-protocol", + "core_test_support", "serde", "serde_json", + "shlex", "tokio", "uuid", "wiremock", @@ -868,6 +870,7 @@ dependencies = [ "serde", "serde_json", "serial_test", + "shlex", "tempfile", "tokio", "toml", @@ -1776,6 +1779,7 @@ dependencies = [ "notify", "regex-lite", "serde_json", + "shlex", "tempfile", "tokio", "walkdir", @@ -3737,11 +3741,13 @@ dependencies = [ "assert_cmd", "codex-core", "codex-mcp-server", + "core_test_support", "mcp-types", "os_info", "pretty_assertions", "serde", "serde_json", + "shlex", "tokio", "wiremock", ] diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 96f64afdf5..0e7849b168 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -53,3 +53,4 @@ serial_test = { workspace = true } tempfile = { workspace = true } toml = { workspace = true } wiremock = { workspace = true } +shlex = { workspace = true } diff --git a/codex-rs/app-server/tests/common/Cargo.toml b/codex-rs/app-server/tests/common/Cargo.toml index 6240f755ee..f8e6529d46 100644 --- a/codex-rs/app-server/tests/common/Cargo.toml +++ b/codex-rs/app-server/tests/common/Cargo.toml @@ -24,3 +24,5 @@ tokio = { workspace = true, features = [ ] } uuid = { workspace = true } wiremock = { workspace = true } +core_test_support = { path = "../../../core/tests/common" } +shlex = { workspace = true } diff --git a/codex-rs/app-server/tests/common/lib.rs b/codex-rs/app-server/tests/common/lib.rs index dc3d24ccae..7d3eb53886 100644 --- a/codex-rs/app-server/tests/common/lib.rs +++ b/codex-rs/app-server/tests/common/lib.rs @@ -9,12 +9,14 @@ pub use auth_fixtures::ChatGptIdTokenClaims; pub use auth_fixtures::encode_id_token; pub use auth_fixtures::write_chatgpt_auth; use codex_app_server_protocol::JSONRPCResponse; +pub use core_test_support::format_with_current_shell; +pub use core_test_support::format_with_current_shell_display; 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_final_assistant_message_sse_response; -pub use responses::create_shell_sse_response; +pub use responses::create_shell_command_sse_response; pub use rollout::create_fake_rollout; use serde::de::DeserializeOwned; diff --git a/codex-rs/app-server/tests/common/responses.rs b/codex-rs/app-server/tests/common/responses.rs index 9a827fb986..0a9183c043 100644 --- a/codex-rs/app-server/tests/common/responses.rs +++ b/codex-rs/app-server/tests/common/responses.rs @@ -1,17 +1,18 @@ use serde_json::json; use std::path::Path; -pub fn create_shell_sse_response( +pub fn create_shell_command_sse_response( command: Vec, workdir: Option<&Path>, timeout_ms: Option, call_id: &str, ) -> anyhow::Result { - // The `arguments`` for the `shell` tool is a serialized JSON object. + // The `arguments` for the `shell_command` tool is a serialized JSON object. + let command_str = shlex::try_join(command.iter().map(String::as_str))?; let tool_call_arguments = serde_json::to_string(&json!({ - "command": command, + "command": command_str, "workdir": workdir.map(|w| w.to_string_lossy()), - "timeout": timeout_ms + "timeout_ms": timeout_ms }))?; let tool_call = json!({ "choices": [ @@ -21,7 +22,7 @@ pub fn create_shell_sse_response( { "id": call_id, "function": { - "name": "shell", + "name": "shell_command", "arguments": tool_call_arguments } } @@ -62,10 +63,10 @@ pub fn create_apply_patch_sse_response( patch_content: &str, call_id: &str, ) -> anyhow::Result { - // Use shell command to call apply_patch with heredoc format - let shell_command = format!("apply_patch <<'EOF'\n{patch_content}\nEOF"); + // Use shell_command to call apply_patch with heredoc format + let command = format!("apply_patch <<'EOF'\n{patch_content}\nEOF"); let tool_call_arguments = serde_json::to_string(&json!({ - "command": ["bash", "-lc", shell_command] + "command": command }))?; let tool_call = json!({ @@ -76,7 +77,7 @@ pub fn create_apply_patch_sse_response( { "id": call_id, "function": { - "name": "shell", + "name": "shell_command", "arguments": tool_call_arguments } } diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 1feda42841..a64aca8051 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -2,7 +2,8 @@ use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_chat_completions_server; -use app_test_support::create_shell_sse_response; +use app_test_support::create_shell_command_sse_response; +use app_test_support::format_with_current_shell; use app_test_support::to_response; use codex_app_server_protocol::AddConversationListenerParams; use codex_app_server_protocol::AddConversationSubscriptionResponse; @@ -56,7 +57,7 @@ async fn test_codex_jsonrpc_conversation_flow() -> Result<()> { // Create a mock model server that immediately ends each turn. // Two turns are expected: initial session configure + one user message. let responses = vec![ - create_shell_sse_response( + create_shell_command_sse_response( vec!["ls".to_string()], Some(&working_directory), Some(5000), @@ -175,7 +176,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { // Mock server will request a python shell call for the first and second turn, then finish. let responses = vec![ - create_shell_sse_response( + create_shell_command_sse_response( vec![ "python3".to_string(), "-c".to_string(), @@ -186,7 +187,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { "call1", )?, create_final_assistant_message_sse_response("done 1")?, - create_shell_sse_response( + create_shell_command_sse_response( vec![ "python3".to_string(), "-c".to_string(), @@ -267,11 +268,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { ExecCommandApprovalParams { conversation_id, call_id: "call1".to_string(), - command: vec![ - "python3".to_string(), - "-c".to_string(), - "print(42)".to_string(), - ], + command: format_with_current_shell("python3 -c 'print(42)'"), cwd: working_directory.clone(), reason: None, risk: None, @@ -353,23 +350,15 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( std::fs::create_dir(&second_cwd)?; let responses = vec![ - create_shell_sse_response( - vec![ - "bash".to_string(), - "-lc".to_string(), - "echo first turn".to_string(), - ], + create_shell_command_sse_response( + vec!["echo".to_string(), "first".to_string(), "turn".to_string()], None, Some(5000), "call-first", )?, create_final_assistant_message_sse_response("done first")?, - create_shell_sse_response( - vec![ - "bash".to_string(), - "-lc".to_string(), - "echo second turn".to_string(), - ], + create_shell_command_sse_response( + vec!["echo".to_string(), "second".to_string(), "turn".to_string()], None, Some(5000), "call-second", @@ -481,13 +470,9 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( exec_begin.cwd, second_cwd, "exec turn should run from updated cwd" ); + let expected_command = format_with_current_shell("echo second turn"); assert_eq!( - exec_begin.command, - vec![ - "bash".to_string(), - "-lc".to_string(), - "echo second turn".to_string() - ], + exec_begin.command, expected_command, "exec turn should run expected command" ); diff --git a/codex-rs/app-server/tests/suite/interrupt.rs b/codex-rs/app-server/tests/suite/interrupt.rs index 86b0a3f3f8..d8e6182be8 100644 --- a/codex-rs/app-server/tests/suite/interrupt.rs +++ b/codex-rs/app-server/tests/suite/interrupt.rs @@ -19,7 +19,7 @@ use tokio::time::timeout; use app_test_support::McpProcess; use app_test_support::create_mock_chat_completions_server; -use app_test_support::create_shell_sse_response; +use app_test_support::create_shell_command_sse_response; use app_test_support::to_response; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); @@ -56,7 +56,7 @@ async fn shell_command_interruption() -> anyhow::Result<()> { std::fs::create_dir(&working_directory)?; // Create mock server with a single SSE response: the long sleep command - let server = create_mock_chat_completions_server(vec![create_shell_sse_response( + let server = create_mock_chat_completions_server(vec![create_shell_command_sse_response( shell_command.clone(), Some(&working_directory), Some(10_000), // 10 seconds timeout in ms diff --git a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs index beafec9cfc..83389ed1d1 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs @@ -3,7 +3,7 @@ use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_mock_chat_completions_server; -use app_test_support::create_shell_sse_response; +use app_test_support::create_shell_command_sse_response; use app_test_support::to_response; use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCResponse; @@ -41,7 +41,7 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> { std::fs::create_dir(&working_directory)?; // Mock server: long-running shell command then (after abort) nothing else needed. - let server = create_mock_chat_completions_server(vec![create_shell_sse_response( + let server = create_mock_chat_completions_server(vec![create_shell_command_sse_response( shell_command.clone(), Some(&working_directory), Some(10_000), 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 b644315d97..a25a9acea0 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -4,7 +4,8 @@ use app_test_support::create_apply_patch_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; -use app_test_support::create_shell_sse_response; +use app_test_support::create_shell_command_sse_response; +use app_test_support::format_with_current_shell_display; use app_test_support::to_response; use codex_app_server_protocol::ApprovalDecision; use codex_app_server_protocol::CommandExecutionStatus; @@ -203,7 +204,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { // Mock server: first turn requests a shell call (elicitation), then completes. // Second turn same, but we'll set approval_policy=never to avoid elicitation. let responses = vec![ - create_shell_sse_response( + create_shell_command_sse_response( vec![ "python3".to_string(), "-c".to_string(), @@ -214,7 +215,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { "call1", )?, create_final_assistant_message_sse_response("done 1")?, - create_shell_sse_response( + create_shell_command_sse_response( vec![ "python3".to_string(), "-c".to_string(), @@ -343,23 +344,15 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { std::fs::create_dir(&second_cwd)?; let responses = vec![ - create_shell_sse_response( - vec![ - "bash".to_string(), - "-lc".to_string(), - "echo first turn".to_string(), - ], + create_shell_command_sse_response( + vec!["echo".to_string(), "first".to_string(), "turn".to_string()], None, Some(5000), "call-first", )?, create_final_assistant_message_sse_response("done first")?, - create_shell_sse_response( - vec![ - "bash".to_string(), - "-lc".to_string(), - "echo second turn".to_string(), - ], + create_shell_command_sse_response( + vec!["echo".to_string(), "second".to_string(), "turn".to_string()], None, Some(5000), "call-second", @@ -465,7 +458,8 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { unreachable!("loop ensures we break on command execution items"); }; assert_eq!(cwd, second_cwd); - assert_eq!(command, "bash -lc 'echo second turn'"); + let expected_command = format_with_current_shell_display("echo second turn"); + assert_eq!(command, expected_command); assert_eq!(status, CommandExecutionStatus::InProgress); timeout( @@ -480,6 +474,10 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { #[tokio::test] async fn turn_start_file_change_approval_v2() -> Result<()> { skip_if_no_network!(Ok(())); + if cfg!(windows) { + // TODO apply_patch approvals are not parsed from powershell commands yet + return Ok(()); + } let tmp = TempDir::new()?; let codex_home = tmp.path().join("codex_home"); @@ -626,6 +624,10 @@ async fn turn_start_file_change_approval_v2() -> Result<()> { #[tokio::test] async fn turn_start_file_change_approval_decline_v2() -> Result<()> { skip_if_no_network!(Ok(())); + if cfg!(windows) { + // TODO apply_patch approvals are not parsed from powershell commands yet + return Ok(()); + } let tmp = TempDir::new()?; let codex_home = tmp.path().join("codex_home"); diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index ac2f409791..a2bea0514a 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -30,6 +30,7 @@ pub use standalone_executable::main; pub const APPLY_PATCH_TOOL_INSTRUCTIONS: &str = include_str!("../apply_patch_tool_instructions.md"); const APPLY_PATCH_COMMANDS: [&str; 2] = ["apply_patch", "applypatch"]; +const APPLY_PATCH_SHELLS: [&str; 3] = ["bash", "zsh", "sh"]; #[derive(Debug, Error, PartialEq)] pub enum ApplyPatchError { @@ -96,6 +97,13 @@ pub struct ApplyPatchArgs { pub workdir: Option, } +fn shell_supports_apply_patch(shell: &str) -> bool { + std::path::Path::new(shell) + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| APPLY_PATCH_SHELLS.contains(&name)) +} + pub fn maybe_parse_apply_patch(argv: &[String]) -> MaybeApplyPatch { match argv { // Direct invocation: apply_patch @@ -104,7 +112,7 @@ pub fn maybe_parse_apply_patch(argv: &[String]) -> MaybeApplyPatch { Err(e) => MaybeApplyPatch::PatchParseError(e), }, // Bash heredoc form: (optional `cd &&`) apply_patch <<'EOF' ... - [bash, flag, script] if bash == "bash" && flag == "-lc" => { + [shell, flag, script] if shell_supports_apply_patch(shell) && flag == "-lc" => { match extract_apply_patch_from_bash(script) { Ok((body, workdir)) => match parse_patch(&body) { Ok(mut source) => { @@ -224,12 +232,12 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp ); } } - [bash, flag, script] if bash == "bash" && flag == "-lc" => { - if parse_patch(script).is_ok() { - return MaybeApplyPatchVerified::CorrectnessError( - ApplyPatchError::ImplicitInvocation, - ); - } + [shell, flag, script] + if shell_supports_apply_patch(shell) + && flag == "-lc" + && parse_patch(script).is_ok() => + { + return MaybeApplyPatchVerified::CorrectnessError(ApplyPatchError::ImplicitInvocation); } _ => {} } diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 65abe23c63..094f33a26b 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -18,3 +18,4 @@ tempfile = { workspace = true } tokio = { workspace = true, features = ["time"] } walkdir = { workspace = true } wiremock = { workspace = true } +shlex = { workspace = true } diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 3f75ed1819..e7b1e71efa 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -172,6 +172,15 @@ pub fn sandbox_network_env_var() -> &'static str { codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR } +pub fn format_with_current_shell(command: &str) -> Vec { + codex_core::shell::default_user_shell().derive_exec_args(command, true) +} + +pub fn format_with_current_shell_display(command: &str) -> String { + let args = format_with_current_shell(command); + shlex::try_join(args.iter().map(String::as_str)).expect("serialize current shell command") +} + pub mod fs_wait { use anyhow::Result; use anyhow::anyhow; diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 7c5a103f42..b84e966399 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -462,8 +462,11 @@ pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value { pub fn ev_shell_command_call(call_id: &str, command: &str) -> Value { let args = serde_json::json!({ "command": command }); - let arguments = serde_json::to_string(&args).expect("serialize shell arguments"); + ev_shell_command_call_with_args(call_id, &args) +} +pub fn ev_shell_command_call_with_args(call_id: &str, args: &serde_json::Value) -> Value { + let arguments = serde_json::to_string(args).expect("serialize shell command arguments"); ev_function_call(call_id, "shell_command", &arguments) } diff --git a/codex-rs/core/tests/suite/abort_tasks.rs b/codex-rs/core/tests/suite/abort_tasks.rs index 1774619843..0d4a807a3c 100644 --- a/codex-rs/core/tests/suite/abort_tasks.rs +++ b/codex-rs/core/tests/suite/abort_tasks.rs @@ -17,15 +17,11 @@ use core_test_support::wait_for_event; use regex_lite::Regex; use serde_json::json; -/// Integration test: spawn a long‑running shell tool via a mocked Responses SSE +/// Integration test: spawn a long‑running shell_command tool via a mocked Responses SSE /// function call, then interrupt the session and expect TurnAborted. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn interrupt_long_running_tool_emits_turn_aborted() { - let command = vec![ - "bash".to_string(), - "-lc".to_string(), - "sleep 60".to_string(), - ]; + let command = "sleep 60"; let args = json!({ "command": command, @@ -33,14 +29,19 @@ async fn interrupt_long_running_tool_emits_turn_aborted() { }) .to_string(); let body = sse(vec![ - ev_function_call("call_sleep", "shell", &args), + ev_function_call("call_sleep", "shell_command", &args), ev_completed("done"), ]); let server = start_mock_server().await; mount_sse_once(&server, body).await; - let codex = test_codex().build(&server).await.unwrap().codex; + let codex = test_codex() + .with_model("gpt-5.1") + .build(&server) + .await + .unwrap() + .codex; // Kick off a turn that triggers the function call. codex @@ -67,11 +68,7 @@ async fn interrupt_long_running_tool_emits_turn_aborted() { /// responses server, and ensures the model receives the synthesized abort. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn interrupt_tool_records_history_entries() { - let command = vec![ - "bash".to_string(), - "-lc".to_string(), - "sleep 60".to_string(), - ]; + let command = "sleep 60"; let call_id = "call-history"; let args = json!({ @@ -81,7 +78,7 @@ async fn interrupt_tool_records_history_entries() { .to_string(); let first_body = sse(vec![ ev_response_created("resp-history"), - ev_function_call(call_id, "shell", &args), + ev_function_call(call_id, "shell_command", &args), ev_completed("resp-history"), ]); let follow_up_body = sse(vec![ @@ -92,7 +89,11 @@ async fn interrupt_tool_records_history_entries() { let server = start_mock_server().await; let response_mock = mount_sse_sequence(&server, vec![first_body, follow_up_body]).await; - let fixture = test_codex().build(&server).await.unwrap(); + let fixture = test_codex() + .with_model("gpt-5.1") + .build(&server) + .await + .unwrap(); let codex = Arc::clone(&fixture.codex); codex diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 59ecf421e6..596db602c5 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -667,7 +667,7 @@ async fn apply_patch_cli_verification_failure_has_no_side_effects( } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_shell_heredoc_with_cd_updates_relative_workdir() -> Result<()> { +async fn apply_patch_shell_command_heredoc_with_cd_updates_relative_workdir() -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness_with(|config| { @@ -684,14 +684,11 @@ async fn apply_patch_shell_heredoc_with_cd_updates_relative_workdir() -> Result< let script = "cd sub && apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: in_sub.txt\n@@\n-before\n+after\n*** End Patch\nEOF\n"; let call_id = "shell-heredoc-cd"; - let args = json!({ - "command": ["bash", "-lc", script], - "timeout_ms": 5_000, - }); + let args = json!({ "command": script, "timeout_ms": 5_000 }); let bodies = vec![ sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), sse(vec![ @@ -706,14 +703,14 @@ async fn apply_patch_shell_heredoc_with_cd_updates_relative_workdir() -> Result< let out = harness.function_call_stdout(call_id).await; assert!( out.contains("Success."), - "expected successful apply_patch invocation via shell: {out}" + "expected successful apply_patch invocation via shell_command: {out}" ); assert_eq!(fs::read_to_string(&target)?, "after\n"); Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_shell_failure_propagates_error_and_skips_diff() -> Result<()> { +async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness_with(|config| { @@ -730,14 +727,11 @@ async fn apply_patch_shell_failure_propagates_error_and_skips_diff() -> Result<( let script = "apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: invalid.txt\n@@\n-nope\n+changed\n*** End Patch\nEOF\n"; let call_id = "shell-apply-failure"; - let args = json!({ - "command": ["bash", "-lc", script], - "timeout_ms": 5_000, - }); + let args = json!({ "command": script, "timeout_ms": 5_000 }); let bodies = vec![ sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), sse(vec![ @@ -780,10 +774,6 @@ async fn apply_patch_shell_failure_propagates_error_and_skips_diff() -> Result<( ); let out = harness.function_call_stdout(call_id).await; - assert!( - out.contains("apply_patch verification failed"), - "expected verification failure message" - ); assert!( out.contains("Failed to find expected lines in"), "expected failure diagnostics: {out}" diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index a106d2eae0..3eacdc3350 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -71,7 +71,7 @@ enum ActionKind { response_body: &'static str, }, RunCommand { - command: &'static [&'static str], + command: &'static str, }, RunUnifiedExecCommand { command: &'static str, @@ -97,20 +97,12 @@ impl ActionKind { server: &MockServer, call_id: &str, with_escalated_permissions: bool, - ) -> Result<(Value, Option>)> { + ) -> Result<(Value, Option)> { match self { ActionKind::WriteFile { target, content } => { let (path, _) = target.resolve_for_patch(test); let _ = fs::remove_file(&path); - let command = vec![ - "/bin/sh".to_string(), - "-c".to_string(), - format!( - "printf {content:?} > {path:?} && cat {path:?}", - content = content, - path = path - ), - ]; + let command = format!("printf {content:?} > {path:?} && cat {path:?}"); let event = shell_event(call_id, &command, 1_000, with_escalated_permissions)?; Ok((event, Some(command))) } @@ -127,21 +119,18 @@ impl ActionKind { .await; let url = format!("{}{}", server.uri(), endpoint); + let escaped_url = url.replace('\'', "\\'"); let script = format!( - "import sys\nimport urllib.request\nurl = {url:?}\ntry:\n data = urllib.request.urlopen(url, timeout=2).read().decode()\n print('OK:' + data.strip())\nexcept Exception as exc:\n print('ERR:' + exc.__class__.__name__)\n sys.exit(1)", + "import sys\nimport urllib.request\nurl = '{escaped_url}'\ntry:\n data = urllib.request.urlopen(url, timeout=2).read().decode()\n print('OK:' + data.strip())\nexcept Exception as exc:\n print('ERR:' + exc.__class__.__name__)\n sys.exit(1)", ); - let command = vec!["python3".to_string(), "-c".to_string(), script]; + let command = format!("python3 -c \"{script}\""); let event = shell_event(call_id, &command, 1_000, with_escalated_permissions)?; Ok((event, Some(command))) } ActionKind::RunCommand { command } => { - let command: Vec = command - .iter() - .map(std::string::ToString::to_string) - .collect(); - let event = shell_event(call_id, &command, 1_000, with_escalated_permissions)?; - Ok((event, Some(command))) + let event = shell_event(call_id, command, 1_000, with_escalated_permissions)?; + Ok((event, Some(command.to_string()))) } ActionKind::RunUnifiedExecCommand { command, @@ -154,14 +143,7 @@ impl ActionKind { with_escalated_permissions, *justification, )?; - Ok(( - event, - Some(vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - command.to_string(), - ]), - )) + Ok((event, Some(command.to_string()))) } ActionKind::ApplyPatchFunction { target, content } => { let (path, patch_path) = target.resolve_for_patch(test); @@ -185,19 +167,19 @@ fn build_add_file_patch(patch_path: &str, content: &str) -> String { format!("*** Begin Patch\n*** Add File: {patch_path}\n+{content}\n*** End Patch\n") } -fn shell_apply_patch_command(patch: &str) -> Vec { +fn shell_apply_patch_command(patch: &str) -> String { let mut script = String::from("apply_patch <<'PATCH'\n"); script.push_str(patch); if !patch.ends_with('\n') { script.push('\n'); } script.push_str("PATCH\n"); - vec!["bash".to_string(), "-lc".to_string(), script] + script } fn shell_event( call_id: &str, - command: &[String], + command: &str, timeout_ms: u64, with_escalated_permissions: bool, ) -> Result { @@ -209,7 +191,7 @@ fn shell_event( args["with_escalated_permissions"] = json!(true); } let args_str = serde_json::to_string(&args)?; - Ok(ev_function_call(call_id, "shell", &args_str)) + Ok(ev_function_call(call_id, "shell_command", &args_str)) } fn exec_command_event( @@ -296,7 +278,10 @@ impl Expectation { } Expectation::FileCreatedNoExitCode { target, content } => { let (path, _) = target.resolve_for_patch(test); - assert_eq!(result.exit_code, None, "expected no exit code for {path:?}"); + assert!( + result.exit_code.is_none() || result.exit_code == Some(0), + "expected no exit code for {path:?}", + ); assert!( result.stdout.contains(content), "stdout missing {content:?}: {}", @@ -385,8 +370,8 @@ impl Expectation { ); } Expectation::NetworkSuccessNoExitCode { body_contains } => { - assert_eq!( - result.exit_code, None, + assert!( + result.exit_code.is_none() || result.exit_code == Some(0), "expected no exit code for successful network call: {}", result.stdout ); @@ -433,8 +418,8 @@ impl Expectation { ); } Expectation::CommandSuccessNoExitCode { stdout_contains } => { - assert_eq!( - result.exit_code, None, + assert!( + result.exit_code.is_none() || result.exit_code == Some(0), "expected no exit code for trusted command: {}", result.stdout ); @@ -531,10 +516,18 @@ fn parse_result(item: &Value) -> CommandResult { CommandResult { exit_code, stdout } } Err(_) => { + let structured = Regex::new(r"(?s)^Exit code:\s*(-?\d+).*?Output:\n(.*)$").unwrap(); let regex = Regex::new(r"(?s)^.*?Process exited with code (\d+)\n.*?Output:\n(.*)$").unwrap(); // parse freeform output - if let Some(captures) = regex.captures(output_str) { + if let Some(captures) = structured.captures(output_str) { + let exit_code = captures.get(1).unwrap().as_str().parse::().unwrap(); + let output = captures.get(2).unwrap().as_str(); + CommandResult { + exit_code: Some(exit_code), + stdout: output.to_string(), + } + } else if let Some(captures) = regex.captures(output_str) { let exit_code = captures.get(1).unwrap().as_str().parse::().unwrap(); let output = captures.get(2).unwrap().as_str(); CommandResult { @@ -553,7 +546,7 @@ fn parse_result(item: &Value) -> CommandResult { async fn expect_exec_approval( test: &TestCodex, - expected_command: &[String], + expected_command: &str, ) -> ExecApprovalRequestEvent { let event = wait_for_event(&test.codex, |event| { matches!( @@ -565,7 +558,12 @@ async fn expect_exec_approval( match event { EventMsg::ExecApprovalRequest(approval) => { - assert_eq!(approval.command, expected_command); + let last_arg = approval + .command + .last() + .map(std::string::String::as_str) + .unwrap_or_default(); + assert_eq!(last_arg, expected_command); approval } EventMsg::TaskComplete(_) => panic!("expected approval request before completion"), @@ -660,7 +658,7 @@ fn scenarios() -> Vec { features: vec![], model_override: Some("gpt-5.1"), outcome: Outcome::Auto, - expectation: Expectation::FileCreatedNoExitCode { + expectation: Expectation::FileCreated { target: TargetPath::OutsideWorkspace("dfa_on_request_5_1.txt"), content: "danger-on-request", }, @@ -702,7 +700,7 @@ fn scenarios() -> Vec { approval_policy: UnlessTrusted, sandbox_policy: SandboxPolicy::DangerFullAccess, action: ActionKind::RunCommand { - command: &["echo", "trusted-unless"], + command: "echo trusted-unless", }, with_escalated_permissions: false, features: vec![], @@ -717,7 +715,7 @@ fn scenarios() -> Vec { approval_policy: UnlessTrusted, sandbox_policy: SandboxPolicy::DangerFullAccess, action: ActionKind::RunCommand { - command: &["echo", "trusted-unless"], + command: "echo trusted-unless", }, with_escalated_permissions: false, features: vec![], @@ -880,7 +878,7 @@ fn scenarios() -> Vec { approval_policy: OnRequest, sandbox_policy: SandboxPolicy::ReadOnly, action: ActionKind::RunCommand { - command: &["echo", "trusted-read-only"], + command: "echo trusted-read-only", }, with_escalated_permissions: false, features: vec![], @@ -895,7 +893,7 @@ fn scenarios() -> Vec { approval_policy: OnRequest, sandbox_policy: SandboxPolicy::ReadOnly, action: ActionKind::RunCommand { - command: &["echo", "trusted-read-only"], + command: "echo trusted-read-only", }, with_escalated_permissions: false, features: vec![], @@ -1020,7 +1018,7 @@ fn scenarios() -> Vec { }, }, ScenarioSpec { - name: "apply_patch_shell_requires_patch_approval", + name: "apply_patch_shell_command_requires_patch_approval", approval_policy: UnlessTrusted, sandbox_policy: workspace_write(false), action: ActionKind::ApplyPatchShell { @@ -1114,7 +1112,7 @@ fn scenarios() -> Vec { }, }, ScenarioSpec { - name: "apply_patch_shell_outside_requires_patch_approval", + name: "apply_patch_shell_command_outside_requires_patch_approval", approval_policy: OnRequest, sandbox_policy: workspace_write(false), action: ActionKind::ApplyPatchShell { @@ -1229,7 +1227,10 @@ fn scenarios() -> Vec { message_contains: if cfg!(target_os = "linux") { &["Permission denied"] } else { - &["Permission denied|Operation not permitted|Read-only file system"] + &[ + "Permission denied|Operation not permitted|operation not permitted|\ + Read-only file system", + ] }, }, }, @@ -1238,7 +1239,7 @@ fn scenarios() -> Vec { approval_policy: Never, sandbox_policy: SandboxPolicy::ReadOnly, action: ActionKind::RunCommand { - command: &["echo", "trusted-never"], + command: "echo trusted-never", }, with_escalated_permissions: false, features: vec![], @@ -1373,7 +1374,10 @@ fn scenarios() -> Vec { message_contains: if cfg!(target_os = "linux") { &["Permission denied"] } else { - &["Permission denied|Operation not permitted|Read-only file system"] + &[ + "Permission denied|Operation not permitted|operation not permitted|\ + Read-only file system", + ] }, }, }, @@ -1509,7 +1513,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { expected_reason, } => { let command = expected_command - .as_ref() + .as_deref() .expect("exec approval requires shell command"); let approval = expect_exec_approval(&test, command).await; if let Some(expected_reason) = expected_reason { diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index 6339bfa71a..36cf0b865f 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -1,3 +1,4 @@ +use codex_core::model_family::find_family_for_model; use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; @@ -25,17 +26,17 @@ use pretty_assertions::assert_eq; async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { skip_if_no_network!(); - // Sub-agent turn 1: emit a shell function_call requiring approval, then complete. + // Sub-agent turn 1: emit a shell_command function_call requiring approval, then complete. let call_id = "call-exec-1"; let args = serde_json::json!({ - "command": ["bash", "-lc", "rm -rf delegated"], + "command": "rm -rf delegated", "timeout_ms": 1000, "with_escalated_permissions": true, }) .to_string(); let sse1 = sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &args), + ev_function_call(call_id, "shell_command", &args), ev_completed("resp-1"), ]); @@ -61,6 +62,8 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { let mut builder = test_codex().with_config(|config| { config.approval_policy = AskForApproval::OnRequest; config.sandbox_policy = SandboxPolicy::ReadOnly; + config.model = "gpt-5.1".to_string(); + config.model_family = find_family_for_model("gpt-5.1").expect("gpt-5.1 is a valid model"); }); let test = builder.build(&server).await.expect("build test codex"); @@ -138,6 +141,8 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() { // Use a restricted sandbox so patch approval is required config.sandbox_policy = SandboxPolicy::ReadOnly; config.include_apply_patch_tool = true; + config.model = "gpt-5.1".to_string(); + config.model_family = find_family_for_model("gpt-5.1").expect("gpt-5.1 is a valid model"); }); let test = builder.build(&server).await.expect("build test codex"); diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index bca917a252..75a80e8d0e 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -1,6 +1,7 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use anyhow::Result; +use codex_core::model_family::find_family_for_model; use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; @@ -21,6 +22,11 @@ use std::fs; #[tokio::test] async fn execpolicy_blocks_shell_invocation() -> Result<()> { + // TODO execpolicy doesn't parse powershell commands yet + if cfg!(windows) { + return Ok(()); + } + let mut builder = test_codex().with_config(|config| { let policy_path = config.codex_home.join("policy").join("policy.codexpolicy"); fs::create_dir_all( @@ -34,13 +40,16 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> { r#"prefix_rule(pattern=["echo"], decision="forbidden")"#, ) .expect("write policy file"); + config.model = "gpt-5.1".to_string(); + config.model_family = + find_family_for_model("gpt-5.1").expect("gpt-5.1 should have a model family"); }); let server = start_mock_server().await; let test = builder.build(&server).await?; let call_id = "shell-forbidden"; let args = json!({ - "command": ["echo", "blocked"], + "command": "echo blocked", "timeout_ms": 1_000, }); @@ -48,7 +57,7 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> { &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) diff --git a/codex-rs/core/tests/suite/tool_parallelism.rs b/codex-rs/core/tests/suite/tool_parallelism.rs index 807bb9d82c..96998f5d15 100644 --- a/codex-rs/core/tests/suite/tool_parallelism.rs +++ b/codex-rs/core/tests/suite/tool_parallelism.rs @@ -146,10 +146,11 @@ async fn non_parallel_tools_run_serially() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let test = test_codex().build(&server).await?; + let mut builder = test_codex().with_model("gpt-5.1"); + let test = builder.build(&server).await?; let shell_args = json!({ - "command": ["/bin/sh", "-c", "sleep 0.3"], + "command": "sleep 0.3", "timeout_ms": 1_000, }); let args_one = serde_json::to_string(&shell_args)?; @@ -157,8 +158,8 @@ async fn non_parallel_tools_run_serially() -> anyhow::Result<()> { let first_response = sse(vec![ json!({"type": "response.created", "response": {"id": "resp-1"}}), - ev_function_call("call-1", "shell", &args_one), - ev_function_call("call-2", "shell", &args_two), + ev_function_call("call-1", "shell_command", &args_one), + ev_function_call("call-2", "shell_command", &args_two), ev_completed("resp-1"), ]); let second_response = sse(vec![ @@ -167,7 +168,7 @@ async fn non_parallel_tools_run_serially() -> anyhow::Result<()> { ]); mount_sse_sequence(&server, vec![first_response, second_response]).await; - let duration = run_turn_and_measure(&test, "run shell twice").await?; + let duration = run_turn_and_measure(&test, "run shell_command twice").await?; assert_serial_duration(duration); Ok(()) @@ -185,14 +186,14 @@ async fn mixed_tools_fall_back_to_serial() -> anyhow::Result<()> { }) .to_string(); let shell_args = serde_json::to_string(&json!({ - "command": ["/bin/sh", "-c", "sleep 0.3"], + "command": "sleep 0.3", "timeout_ms": 1_000, }))?; let first_response = sse(vec![ json!({"type": "response.created", "response": {"id": "resp-1"}}), ev_function_call("call-1", "test_sync_tool", &sync_args), - ev_function_call("call-2", "shell", &shell_args), + ev_function_call("call-2", "shell_command", &shell_args), ev_completed("resp-1"), ]); let second_response = sse(vec![ @@ -215,7 +216,7 @@ async fn tool_results_grouped() -> anyhow::Result<()> { let test = build_codex_with_test_tool(&server).await?; let shell_args = serde_json::to_string(&json!({ - "command": ["/bin/sh", "-c", "echo 'shell output'"], + "command": "echo 'shell output'", "timeout_ms": 1_000, }))?; @@ -223,9 +224,9 @@ async fn tool_results_grouped() -> anyhow::Result<()> { &server, sse(vec![ json!({"type": "response.created", "response": {"id": "resp-1"}}), - ev_function_call("call-1", "shell", &shell_args), - ev_function_call("call-2", "shell", &shell_args), - ev_function_call("call-3", "shell", &shell_args), + ev_function_call("call-1", "shell_command", &shell_args), + ev_function_call("call-2", "shell_command", &shell_args), + ev_function_call("call-3", "shell_command", &shell_args), ev_completed("resp-1"), ]), ) diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index 7d611f55fc..f06694f628 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -98,7 +98,7 @@ async fn truncate_function_error_trims_respond_to_model() -> Result<()> { Ok(()) } -// Verifies that a standard tool call (shell) exceeding the model formatting +// Verifies that a standard tool call (shell_command) exceeding the model formatting // limits is truncated before being sent back to the model. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn tool_call_output_configured_limit_chars_type() -> Result<()> { @@ -106,7 +106,7 @@ async fn tool_call_output_configured_limit_chars_type() -> Result<()> { let server = start_mock_server().await; - // Use a model that exposes the generic shell tool. + // Use a model that exposes the shell_command tool. let mut builder = test_codex().with_model("gpt-5.1").with_config(|config| { config.tool_output_token_limit = Some(100_000); }); @@ -114,28 +114,22 @@ async fn tool_call_output_configured_limit_chars_type() -> Result<()> { let fixture = builder.build(&server).await?; let call_id = "shell-too-large"; - let args = if cfg!(windows) { - serde_json::json!({ - "command": [ - "powershell", - "-Command", - "for ($i=1; $i -le 100000; $i++) { Write-Output $i }" - ], - "timeout_ms": 5_000, - }) + let command = if cfg!(windows) { + "for ($i=1; $i -le 100000; $i++) { Write-Output $i }" } else { - serde_json::json!({ - "command": ["/bin/sh", "-c", "seq 1 100000"], - "timeout_ms": 5_000, - }) + "seq 1 100000" }; + let args = serde_json::json!({ + "command": command, + "timeout_ms": 5_000, + }); // First response: model tells us to run the tool; second: complete the turn. mount_sse_once( &server, sse(vec![ responses::ev_response_created("resp-1"), - responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + responses::ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), responses::ev_completed("resp-1"), ]), ) @@ -167,7 +161,10 @@ async fn tool_call_output_configured_limit_chars_type() -> Result<()> { "expected truncated shell output to be plain text" ); - assert_eq!(output.len(), 400097, "we should be almost 100k tokens"); + assert!( + (400000..=401000).contains(&output.len()), + "we should be almost 100k tokens" + ); assert!( !output.contains("tokens truncated"), @@ -177,7 +174,7 @@ async fn tool_call_output_configured_limit_chars_type() -> Result<()> { Ok(()) } -// Verifies that a standard tool call (shell) exceeding the model formatting +// Verifies that a standard tool call (shell_command) exceeding the model formatting // limits is truncated before being sent back to the model. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn tool_call_output_exceeds_limit_truncated_chars_limit() -> Result<()> { @@ -185,34 +182,28 @@ async fn tool_call_output_exceeds_limit_truncated_chars_limit() -> Result<()> { let server = start_mock_server().await; - // Use a model that exposes the generic shell tool. + // Use a model that exposes the shell_command tool. let mut builder = test_codex().with_model("gpt-5.1"); let fixture = builder.build(&server).await?; let call_id = "shell-too-large"; - let args = if cfg!(windows) { - serde_json::json!({ - "command": [ - "powershell", - "-Command", - "for ($i=1; $i -le 100000; $i++) { Write-Output $i }" - ], - "timeout_ms": 5_000, - }) + let command = if cfg!(windows) { + "for ($i=1; $i -le 100000; $i++) { Write-Output $i }" } else { - serde_json::json!({ - "command": ["/bin/sh", "-c", "seq 1 100000"], - "timeout_ms": 5_000, - }) + "seq 1 100000" }; + let args = serde_json::json!({ + "command": command, + "timeout_ms": 5_000, + }); // First response: model tells us to run the tool; second: complete the turn. mount_sse_once( &server, sse(vec![ responses::ev_response_created("resp-1"), - responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + responses::ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), responses::ev_completed("resp-1"), ]), ) @@ -250,14 +241,14 @@ async fn tool_call_output_exceeds_limit_truncated_chars_limit() -> Result<()> { let len = output.len(); assert!( - (9_900..=10_000).contains(&len), + (9_900..=10_100).contains(&len), "expected ~10k chars after truncation, got {len}" ); Ok(()) } -// Verifies that a standard tool call (shell) exceeding the model formatting +// Verifies that a standard tool call (shell_command) exceeding the model formatting // limits is truncated before being sent back to the model. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> { @@ -265,7 +256,7 @@ async fn tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> { let server = start_mock_server().await; - // Use a model that exposes the generic shell tool. + // Use a model that exposes the shell_command tool. let mut builder = test_codex().with_config(|config| { config.model = "gpt-5.1-codex".to_string(); config.model_family = @@ -274,28 +265,22 @@ async fn tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> { let fixture = builder.build(&server).await?; let call_id = "shell-too-large"; - let args = if cfg!(windows) { - serde_json::json!({ - "command": [ - "powershell", - "-Command", - "for ($i=1; $i -le 100000; $i++) { Write-Output $i }" - ], - "timeout_ms": 5_000, - }) + let command = if cfg!(windows) { + "for ($i=1; $i -le 100000; $i++) { Write-Output $i }" } else { - serde_json::json!({ - "command": ["/bin/sh", "-c", "seq 1 100000"], - "timeout_ms": 5_000, - }) + "seq 1 100000" }; + let args = serde_json::json!({ + "command": command, + "timeout_ms": 5_000, + }); // First response: model tells us to run the tool; second: complete the turn. mount_sse_once( &server, sse(vec![ responses::ev_response_created("resp-1"), - responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + responses::ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), responses::ev_completed("resp-1"), ]), ) @@ -345,7 +330,7 @@ $"#; Ok(()) } -// Ensures shell tool outputs that exceed the line limit are truncated only once. +// Ensures shell_command outputs that exceed the line limit are truncated only once. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn tool_call_output_truncated_only_once() -> Result<()> { skip_if_no_network!(Ok(())); @@ -359,27 +344,21 @@ async fn tool_call_output_truncated_only_once() -> Result<()> { }); let fixture = builder.build(&server).await?; let call_id = "shell-single-truncation"; - let args = if cfg!(windows) { - serde_json::json!({ - "command": [ - "powershell", - "-Command", - "for ($i=1; $i -le 10000; $i++) { Write-Output $i }" - ], - "timeout_ms": 5_000, - }) + let command = if cfg!(windows) { + "for ($i=1; $i -le 10000; $i++) { Write-Output $i }" } else { - serde_json::json!({ - "command": ["/bin/sh", "-c", "seq 1 10000"], - "timeout_ms": 5_000, - }) + "seq 1 10000" }; + let args = serde_json::json!({ + "command": command, + "timeout_ms": 5_000, + }); mount_sse_once( &server, sse(vec![ responses::ev_response_created("resp-1"), - responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + responses::ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), responses::ev_completed("resp-1"), ]), ) @@ -619,7 +598,7 @@ async fn token_policy_marker_reports_tokens() -> Result<()> { let call_id = "shell-token-marker"; let args = json!({ - "command": ["/bin/sh", "-c", "seq 1 150"], + "command": "seq 1 150", "timeout_ms": 5_000, }); @@ -627,7 +606,7 @@ async fn token_policy_marker_reports_tokens() -> Result<()> { &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) @@ -650,7 +629,7 @@ async fn token_policy_marker_reports_tokens() -> Result<()> { .function_call_output_text(call_id) .context("shell output present")?; - let pattern = r#"(?s)^\{"output":"Total output lines: 150\\n\\n1\\n2\\n3\\n4\\n5\\n.*?…\d+ tokens truncated…7\\n138\\n139\\n140\\n141\\n142\\n143\\n144\\n145\\n146\\n147\\n148\\n149\\n150\\n","metadata":\{"exit_code":0,"duration_seconds":0\.0\}\}$"#; + let pattern = r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nTotal output lines: 150\nOutput:\n1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19.*tokens truncated.*129\n130\n131\n132\n133\n134\n135\n136\n137\n138\n139\n140\n141\n142\n143\n144\n145\n146\n147\n148\n149\n150\n$"; assert_regex_match(pattern, &output); @@ -672,7 +651,7 @@ async fn byte_policy_marker_reports_bytes() -> Result<()> { let call_id = "shell-byte-marker"; let args = json!({ - "command": ["/bin/sh", "-c", "seq 1 150"], + "command": "seq 1 150", "timeout_ms": 5_000, }); @@ -680,7 +659,7 @@ async fn byte_policy_marker_reports_bytes() -> Result<()> { &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) @@ -703,16 +682,16 @@ async fn byte_policy_marker_reports_bytes() -> Result<()> { .function_call_output_text(call_id) .context("shell output present")?; - let pattern = r#"(?s)^\{"output":"Total output lines: 150\\n\\n1\\n2\\n3\\n4\\n5.*?…\d+ chars truncated…7\\n138\\n139\\n140\\n141\\n142\\n143\\n144\\n145\\n146\\n147\\n148\\n149\\n150\\n","metadata":\{"exit_code":0,"duration_seconds":0\.0\}\}$"#; + let pattern = r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nTotal output lines: 150\nOutput:\n1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19.*chars truncated.*129\n130\n131\n132\n133\n134\n135\n136\n137\n138\n139\n140\n141\n142\n143\n144\n145\n146\n147\n148\n149\n150\n$"; assert_regex_match(pattern, &output); Ok(()) } -// Shell tool output should remain intact when the config opts into a large token budget. +// shell_command output should remain intact when the config opts into a large token budget. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_tool_output_not_truncated_with_custom_limit() -> Result<()> { +async fn shell_command_output_not_truncated_with_custom_limit() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -726,7 +705,7 @@ async fn shell_tool_output_not_truncated_with_custom_limit() -> Result<()> { let call_id = "shell-no-trunc"; let args = json!({ - "command": ["/bin/sh", "-c", "seq 1 1000"], + "command": "seq 1 1000", "timeout_ms": 5_000, }); let expected_body: String = (1..=1000).map(|i| format!("{i}\n")).collect(); @@ -735,7 +714,7 @@ async fn shell_tool_output_not_truncated_with_custom_limit() -> Result<()> { &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) diff --git a/codex-rs/core/tests/suite/user_shell_cmd.rs b/codex-rs/core/tests/suite/user_shell_cmd.rs index e6fbdee9e5..f6e53b95e3 100644 --- a/codex-rs/core/tests/suite/user_shell_cmd.rs +++ b/codex-rs/core/tests/suite/user_shell_cmd.rs @@ -279,23 +279,19 @@ async fn user_shell_command_is_truncated_only_once() -> anyhow::Result<()> { config.tool_output_token_limit = Some(100); config.model = "gpt-5.1-codex".to_string(); config.model_family = - find_family_for_model("gpt-5-codex").expect("gpt-5-codex is a model family"); + find_family_for_model("gpt-5.1-codex").expect("gpt-5.1-codex is a model family"); }); let fixture = builder.build(&server).await?; let call_id = "user-shell-double-truncation"; let args = if cfg!(windows) { serde_json::json!({ - "command": [ - "powershell", - "-Command", - "for ($i=1; $i -le 2000; $i++) { Write-Output $i }" - ], + "command": "for ($i=1; $i -le 2000; $i++) { Write-Output $i }", "timeout_ms": 5_000, }) } else { serde_json::json!({ - "command": ["/bin/sh", "-c", "seq 1 2000"], + "command": "seq 1 2000", "timeout_ms": 5_000, }) }; @@ -304,7 +300,7 @@ async fn user_shell_command_is_truncated_only_once() -> anyhow::Result<()> { &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) @@ -319,19 +315,22 @@ async fn user_shell_command_is_truncated_only_once() -> anyhow::Result<()> { .await; fixture - .submit_turn_with_policy("trigger big shell output", SandboxPolicy::DangerFullAccess) + .submit_turn_with_policy( + "trigger big shell_command output", + SandboxPolicy::DangerFullAccess, + ) .await?; let output = mock2 .single_request() .function_call_output_text(call_id) - .context("function_call_output present for shell call")?; + .context("function_call_output present for shell_command call")?; let truncation_headers = output.matches("Total output lines:").count(); assert_eq!( truncation_headers, 1, - "shell output should carry only one truncation header: {output}" + "shell_command output should carry only one truncation header: {output}" ); Ok(()) diff --git a/codex-rs/mcp-server/tests/common/Cargo.toml b/codex-rs/mcp-server/tests/common/Cargo.toml index 7c2bc2266a..286cd63c7f 100644 --- a/codex-rs/mcp-server/tests/common/Cargo.toml +++ b/codex-rs/mcp-server/tests/common/Cargo.toml @@ -23,3 +23,5 @@ tokio = { workspace = true, features = [ "rt-multi-thread", ] } wiremock = { workspace = true } +core_test_support = { path = "../../../core/tests/common" } +shlex = { workspace = true } diff --git a/codex-rs/mcp-server/tests/common/lib.rs b/codex-rs/mcp-server/tests/common/lib.rs index d088b184ea..d8d6b67384 100644 --- a/codex-rs/mcp-server/tests/common/lib.rs +++ b/codex-rs/mcp-server/tests/common/lib.rs @@ -2,12 +2,13 @@ mod mcp_process; mod mock_model_server; mod responses; +pub use core_test_support::format_with_current_shell; pub use mcp_process::McpProcess; use mcp_types::JSONRPCResponse; pub use mock_model_server::create_mock_chat_completions_server; pub use responses::create_apply_patch_sse_response; pub use responses::create_final_assistant_message_sse_response; -pub use responses::create_shell_sse_response; +pub use responses::create_shell_command_sse_response; use serde::de::DeserializeOwned; pub fn to_response(response: JSONRPCResponse) -> anyhow::Result { diff --git a/codex-rs/mcp-server/tests/common/responses.rs b/codex-rs/mcp-server/tests/common/responses.rs index 9a827fb986..0a9183c043 100644 --- a/codex-rs/mcp-server/tests/common/responses.rs +++ b/codex-rs/mcp-server/tests/common/responses.rs @@ -1,17 +1,18 @@ use serde_json::json; use std::path::Path; -pub fn create_shell_sse_response( +pub fn create_shell_command_sse_response( command: Vec, workdir: Option<&Path>, timeout_ms: Option, call_id: &str, ) -> anyhow::Result { - // The `arguments`` for the `shell` tool is a serialized JSON object. + // The `arguments` for the `shell_command` tool is a serialized JSON object. + let command_str = shlex::try_join(command.iter().map(String::as_str))?; let tool_call_arguments = serde_json::to_string(&json!({ - "command": command, + "command": command_str, "workdir": workdir.map(|w| w.to_string_lossy()), - "timeout": timeout_ms + "timeout_ms": timeout_ms }))?; let tool_call = json!({ "choices": [ @@ -21,7 +22,7 @@ pub fn create_shell_sse_response( { "id": call_id, "function": { - "name": "shell", + "name": "shell_command", "arguments": tool_call_arguments } } @@ -62,10 +63,10 @@ pub fn create_apply_patch_sse_response( patch_content: &str, call_id: &str, ) -> anyhow::Result { - // Use shell command to call apply_patch with heredoc format - let shell_command = format!("apply_patch <<'EOF'\n{patch_content}\nEOF"); + // Use shell_command to call apply_patch with heredoc format + let command = format!("apply_patch <<'EOF'\n{patch_content}\nEOF"); let tool_call_arguments = serde_json::to_string(&json!({ - "command": ["bash", "-lc", shell_command] + "command": command }))?; let tool_call = json!({ @@ -76,7 +77,7 @@ pub fn create_apply_patch_sse_response( { "id": call_id, "function": { - "name": "shell", + "name": "shell_command", "arguments": tool_call_arguments } } diff --git a/codex-rs/mcp-server/tests/suite/codex_tool.rs b/codex-rs/mcp-server/tests/suite/codex_tool.rs index ae0c23f1d8..f65495c473 100644 --- a/codex-rs/mcp-server/tests/suite/codex_tool.rs +++ b/codex-rs/mcp-server/tests/suite/codex_tool.rs @@ -30,7 +30,8 @@ use mcp_test_support::McpProcess; use mcp_test_support::create_apply_patch_sse_response; use mcp_test_support::create_final_assistant_message_sse_response; use mcp_test_support::create_mock_chat_completions_server; -use mcp_test_support::create_shell_sse_response; +use mcp_test_support::create_shell_command_sse_response; +use mcp_test_support::format_with_current_shell; // Allow ample time on slower CI or under load to avoid flakes. const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); @@ -71,13 +72,16 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { "-c".to_string(), format!("import pathlib; pathlib.Path('{created_filename}').touch()"), ]; + let expected_shell_command = format_with_current_shell(&format!( + "python3 -c \"import pathlib; pathlib.Path('{created_filename}').touch()\"" + )); let McpHandle { process: mut mcp_process, server: _server, dir: _dir, } = create_mcp_process(vec![ - create_shell_sse_response( + create_shell_command_sse_response( shell_command.clone(), Some(workdir_for_shell_function_call.path()), Some(5_000), @@ -111,7 +115,7 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { )?; let expected_elicitation_request = create_expected_elicitation_request( elicitation_request_id.clone(), - shell_command.clone(), + expected_shell_command, workdir_for_shell_function_call.path(), codex_request_id.to_string(), params.codex_event_id.clone(), @@ -218,6 +222,12 @@ async fn test_patch_approval_triggers_elicitation() { } async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> { + if cfg!(windows) { + // powershell apply_patch shell calls are not parsed into apply patch approvals + + return Ok(()); + } + let cwd = TempDir::new()?; let test_file = cwd.path().join("destination_file.txt"); std::fs::write(&test_file, "original content\n")?; From ab5972d447da78d3e4dd8461cf7d43a22e5d2acb Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 20 Nov 2025 20:45:28 -0800 Subject: [PATCH 06/13] Support all types of search actions (#7061) Fixes the ``` { "error": { "message": "Invalid value: 'other'. Supported values are: 'search', 'open_page', and 'find_in_page'.", "type": "invalid_request_error", "param": "input[150].action.type", "code": "invalid_value" } ``` error. The actual-actual fix here is supporting absent `query` parameter. --- codex-rs/core/src/event_mapping.rs | 4 +- codex-rs/core/tests/suite/client.rs | 2 +- codex-rs/protocol/src/models.rs | 84 ++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index d0aa1d8188..6b4bed4db3 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -117,7 +117,7 @@ pub fn parse_turn_item(item: &ResponseItem) -> Option { .. } => Some(TurnItem::WebSearch(WebSearchItem { id: id.clone().unwrap_or_default(), - query: query.clone(), + query: query.clone().unwrap_or_default(), })), _ => None, } @@ -306,7 +306,7 @@ mod tests { id: Some("ws_1".to_string()), status: Some("completed".to_string()), action: WebSearchAction::Search { - query: "weather".to_string(), + query: Some("weather".to_string()), }, }; diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 59b7e34171..71bcd5192d 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -992,7 +992,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { id: Some("web-search-id".into()), status: Some("completed".into()), action: WebSearchAction::Search { - query: "weather".into(), + query: Some("weather".into()), }, }); prompt.input.push(ResponseItem::FunctionCall { diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 2e623eefef..b6bd92c6ea 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -230,8 +230,24 @@ pub struct LocalShellExecAction { #[serde(tag = "type", rename_all = "snake_case")] pub enum WebSearchAction { Search { - query: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + query: Option, }, + OpenPage { + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + url: Option, + }, + FindInPage { + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + url: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pattern: Option, + }, + #[serde(other)] Other, } @@ -634,6 +650,72 @@ mod tests { Ok(()) } + #[test] + fn roundtrips_web_search_call_actions() -> Result<()> { + let cases = vec![ + ( + r#"{ + "type": "web_search_call", + "status": "completed", + "action": { + "type": "search", + "query": "weather seattle" + } + }"#, + WebSearchAction::Search { + query: Some("weather seattle".into()), + }, + Some("completed".into()), + ), + ( + r#"{ + "type": "web_search_call", + "status": "open", + "action": { + "type": "open_page", + "url": "https://example.com" + } + }"#, + WebSearchAction::OpenPage { + url: Some("https://example.com".into()), + }, + Some("open".into()), + ), + ( + r#"{ + "type": "web_search_call", + "status": "in_progress", + "action": { + "type": "find_in_page", + "url": "https://example.com/docs", + "pattern": "installation" + } + }"#, + WebSearchAction::FindInPage { + url: Some("https://example.com/docs".into()), + pattern: Some("installation".into()), + }, + Some("in_progress".into()), + ), + ]; + + for (json_literal, expected_action, expected_status) in cases { + let parsed: ResponseItem = serde_json::from_str(json_literal)?; + let expected = ResponseItem::WebSearchCall { + id: None, + status: expected_status.clone(), + action: expected_action.clone(), + }; + assert_eq!(parsed, expected); + + let serialized = serde_json::to_value(&parsed)?; + let original_value: serde_json::Value = serde_json::from_str(json_literal)?; + assert_eq!(serialized, original_value); + } + + Ok(()) + } + #[test] fn deserialize_shell_tool_call_params() -> Result<()> { let json = r#"{ From bacdc004be5b6d065eec3a9f807593c3b6ff38da Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 21 Nov 2025 00:45:40 -0600 Subject: [PATCH 07/13] Fixed two tests that can fail in some environments that have global git rewrite rules (#7068) This fixes https://github.com/openai/codex/issues/7044 --- codex-rs/core/src/git_info.rs | 18 ++++++++++++++---- codex-rs/core/tests/suite/cli_stream.rs | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/codex-rs/core/src/git_info.rs b/codex-rs/core/src/git_info.rs index 387e9a682e..34e0afc729 100644 --- a/codex-rs/core/src/git_info.rs +++ b/codex-rs/core/src/git_info.rs @@ -825,11 +825,21 @@ mod tests { .await .expect("Should collect git info from repo"); + let remote_url_output = Command::new("git") + .args(["remote", "get-url", "origin"]) + .current_dir(&repo_path) + .output() + .await + .expect("Failed to read remote url"); + // Some dev environments rewrite remotes (e.g., force SSH), so compare against + // whatever URL Git reports instead of a fixed placeholder. + let expected_remote = String::from_utf8(remote_url_output.stdout) + .unwrap() + .trim() + .to_string(); + // Should have repository URL - assert_eq!( - git_info.repository_url, - Some("https://github.com/example/repo.git".to_string()) - ); + assert_eq!(git_info.repository_url, Some(expected_remote)); } #[tokio::test] diff --git a/codex-rs/core/tests/suite/cli_stream.rs b/codex-rs/core/tests/suite/cli_stream.rs index b484eca22a..d7f0fb9830 100644 --- a/codex-rs/core/tests/suite/cli_stream.rs +++ b/codex-rs/core/tests/suite/cli_stream.rs @@ -499,9 +499,20 @@ async fn integration_git_info_unit_test() { "Git info should contain repository_url" ); let repo_url = git_info.repository_url.as_ref().unwrap(); + // Some hosts rewrite remotes (e.g., github.com → git@github.com), so assert against + // the actual remote reported by git instead of a static URL. + let expected_remote_url = std::process::Command::new("git") + .args(["remote", "get-url", "origin"]) + .current_dir(&git_repo) + .output() + .unwrap(); + let expected_remote_url = String::from_utf8(expected_remote_url.stdout) + .unwrap() + .trim() + .to_string(); assert_eq!( - repo_url, "https://github.com/example/integration-test.git", - "Repository URL should match what we configured" + repo_url, &expected_remote_url, + "Repository URL should match git remote get-url output" ); println!("✅ Git info collection test passed!"); From c9e149fd5cd071386210198600a522cad82803f4 Mon Sep 17 00:00:00 2001 From: Yorling Date: Fri, 21 Nov 2025 14:46:34 +0800 Subject: [PATCH 08/13] fix: read `max_output_tokens` param from config (#4139) Request param `max_output_tokens` is documented in `https://github.com/openai/codex/blob/main/docs/config.md`, but nowhere uses the item in config, this commit read it from config for GPT responses API. see https://github.com/openai/codex/issues/4138 for issue report. Signed-off-by: Yorling --- codex-rs/core/src/client.rs | 1 + codex-rs/core/src/client_common.rs | 5 +++++ codex-rs/core/tests/suite/compact_resume_fork.rs | 15 ++++++++++----- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 68bd30f4e7..f4d830900c 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -273,6 +273,7 @@ impl ModelClient { include, prompt_cache_key: Some(self.conversation_id.to_string()), text, + max_output_tokens: self.config.model_max_output_tokens, }; let mut payload_json = serde_json::to_value(&payload)?; diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 21f6fc6576..e16db3d619 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -280,6 +280,8 @@ pub(crate) struct ResponsesApiRequest<'a> { pub(crate) prompt_cache_key: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) text: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) max_output_tokens: Option, } pub(crate) mod tools { @@ -463,6 +465,7 @@ mod tests { verbosity: Some(OpenAiVerbosity::Low), format: None, }), + max_output_tokens: Some(10_000), }; let v = serde_json::to_value(&req).expect("json"); @@ -501,6 +504,7 @@ mod tests { include: vec![], prompt_cache_key: None, text: Some(text_controls), + max_output_tokens: Some(10_000), }; let v = serde_json::to_value(&req).expect("json"); @@ -537,6 +541,7 @@ mod tests { include: vec![], prompt_cache_key: None, text: None, + max_output_tokens: Some(10_000), }; let v = serde_json::to_value(&req).expect("json"); diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index f81294baf3..cb82581617 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -282,7 +282,8 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key + "prompt_cache_key": prompt_cache_key, + "max_output_tokens": 128000, }); let compact_1 = json!( { @@ -351,7 +352,8 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key + "prompt_cache_key": prompt_cache_key, + "max_output_tokens": 128000, }); let user_turn_2_after_compact = json!( { @@ -411,7 +413,8 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key + "prompt_cache_key": prompt_cache_key, + "max_output_tokens": 128000, }); let usert_turn_3_after_resume = json!( { @@ -491,7 +494,8 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key + "prompt_cache_key": prompt_cache_key, + "max_output_tokens": 128000, }); let user_turn_3_after_fork = json!( { @@ -571,7 +575,8 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": fork_prompt_cache_key + "prompt_cache_key": fork_prompt_cache_key, + "max_output_tokens": 128000, }); let mut expected = json!([ user_turn_1, From b315b22f7baae557de96ba1b23688cd254516c7a Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 21 Nov 2025 00:46:55 -0600 Subject: [PATCH 09/13] Fixed the deduplicator github action (#7070) It stopped working (found zero duplicates) starting three days ago when the model was switched from `gpt-5` to `gpt-5.1`. I'm not sure why it stopped working. This is an attempt to get it working again by using the default model for the codex action (which is presumably `gpt-5.1-codex-max`). --- .github/workflows/issue-deduplicator.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/issue-deduplicator.yml b/.github/workflows/issue-deduplicator.yml index 579b6a3685..5c5463e2c7 100644 --- a/.github/workflows/issue-deduplicator.yml +++ b/.github/workflows/issue-deduplicator.yml @@ -46,7 +46,6 @@ jobs: with: openai-api-key: ${{ secrets.CODEX_OPENAI_API_KEY }} allow-users: "*" - model: gpt-5.1 prompt: | You are an assistant that triages new GitHub issues by identifying potential duplicates. From f4af6e389ec9868ddea77c2b1da04bc03e453035 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 20 Nov 2025 22:59:55 -0800 Subject: [PATCH 10/13] Windows Sandbox: support network_access and exclude_tmpdir_env_var (#7030) --- codex-rs/windows-sandbox-rs/src/allow.rs | 37 +++++++++++++++++++++- codex-rs/windows-sandbox-rs/src/lib.rs | 39 +++++++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/allow.rs b/codex-rs/windows-sandbox-rs/src/allow.rs index de62159994..7a879df4ac 100644 --- a/codex-rs/windows-sandbox-rs/src/allow.rs +++ b/codex-rs/windows-sandbox-rs/src/allow.rs @@ -18,6 +18,13 @@ pub fn compute_allow_paths( allow.push(p); } }; + let include_tmp_env_vars = matches!( + policy, + SandboxPolicy::WorkspaceWrite { + exclude_tmpdir_env_var: false, + .. + } + ); if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) { add_path(command_cwd.to_path_buf()); @@ -33,7 +40,7 @@ pub fn compute_allow_paths( } } } - if !matches!(policy, SandboxPolicy::ReadOnly) { + if include_tmp_env_vars { for key in ["TEMP", "TMP"] { if let Some(v) = env_map.get(key) { let abs = PathBuf::from(v); @@ -80,4 +87,32 @@ mod tests { "additional writable root should be allowed" ); } + + #[test] + fn excludes_tmp_env_vars_when_requested() { + let command_cwd = PathBuf::from(r"C:\Workspace"); + let temp_dir = PathBuf::from(r"C:\TempDir"); + let _ = fs::create_dir_all(&command_cwd); + let _ = fs::create_dir_all(&temp_dir); + + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: false, + }; + let mut env_map = HashMap::new(); + env_map.insert("TEMP".into(), temp_dir.to_string_lossy().to_string()); + + let allow = compute_allow_paths(&policy, &command_cwd, &command_cwd, &env_map); + + assert!( + allow.iter().any(|p| p == &command_cwd), + "command cwd should be allowed" + ); + assert!( + !allow.iter().any(|p| p == &temp_dir), + "TEMP should be excluded when exclude_tmpdir_env_var is true" + ); + } } diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 1720294f2e..e2277daade 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -71,6 +71,10 @@ mod windows_impl { type PipeHandles = ((HANDLE, HANDLE), (HANDLE, HANDLE), (HANDLE, HANDLE)); + fn should_apply_network_block(policy: &SandboxPolicy) -> bool { + !policy.has_full_network_access() + } + fn ensure_dir(p: &Path) -> Result<()> { if let Some(d) = p.parent() { std::fs::create_dir_all(d)?; @@ -214,9 +218,12 @@ mod windows_impl { timeout_ms: Option, ) -> Result { let policy = parse_policy(policy_json_or_preset)?; + let apply_network_block = should_apply_network_block(&policy); normalize_null_device_env(&mut env_map); ensure_non_interactive_pager(&mut env_map); - apply_no_network_to_env(&mut env_map)?; + if apply_network_block { + apply_no_network_to_env(&mut env_map)?; + } ensure_codex_home_exists(codex_home)?; let current_dir = cwd.to_path_buf(); @@ -447,6 +454,36 @@ mod windows_impl { timed_out, }) } + + #[cfg(test)] + mod tests { + use super::should_apply_network_block; + use crate::policy::SandboxPolicy; + + fn workspace_policy(network_access: bool) -> SandboxPolicy { + SandboxPolicy::WorkspaceWrite { + writable_roots: Vec::new(), + network_access, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + } + } + + #[test] + fn applies_network_block_when_access_is_disabled() { + assert!(should_apply_network_block(&workspace_policy(false))); + } + + #[test] + fn skips_network_block_when_access_is_allowed() { + assert!(!should_apply_network_block(&workspace_policy(true))); + } + + #[test] + fn applies_network_block_for_read_only() { + assert!(should_apply_network_block(&SandboxPolicy::ReadOnly)); + } + } } #[cfg(not(target_os = "windows"))] From bce030ddb5d0c428aeb909d961a2d5641695c06c Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 21 Nov 2025 11:40:02 +0100 Subject: [PATCH 11/13] Revert "fix: read `max_output_tokens` param from config" (#7088) Reverts openai/codex#4139 --- codex-rs/core/src/client.rs | 1 - codex-rs/core/src/client_common.rs | 5 ----- codex-rs/core/tests/suite/compact_resume_fork.rs | 15 +++++---------- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index f4d830900c..68bd30f4e7 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -273,7 +273,6 @@ impl ModelClient { include, prompt_cache_key: Some(self.conversation_id.to_string()), text, - max_output_tokens: self.config.model_max_output_tokens, }; let mut payload_json = serde_json::to_value(&payload)?; diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index e16db3d619..21f6fc6576 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -280,8 +280,6 @@ pub(crate) struct ResponsesApiRequest<'a> { pub(crate) prompt_cache_key: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) text: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub(crate) max_output_tokens: Option, } pub(crate) mod tools { @@ -465,7 +463,6 @@ mod tests { verbosity: Some(OpenAiVerbosity::Low), format: None, }), - max_output_tokens: Some(10_000), }; let v = serde_json::to_value(&req).expect("json"); @@ -504,7 +501,6 @@ mod tests { include: vec![], prompt_cache_key: None, text: Some(text_controls), - max_output_tokens: Some(10_000), }; let v = serde_json::to_value(&req).expect("json"); @@ -541,7 +537,6 @@ mod tests { include: vec![], prompt_cache_key: None, text: None, - max_output_tokens: Some(10_000), }; let v = serde_json::to_value(&req).expect("json"); diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index cb82581617..f81294baf3 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -282,8 +282,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key, - "max_output_tokens": 128000, + "prompt_cache_key": prompt_cache_key }); let compact_1 = json!( { @@ -352,8 +351,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key, - "max_output_tokens": 128000, + "prompt_cache_key": prompt_cache_key }); let user_turn_2_after_compact = json!( { @@ -413,8 +411,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key, - "max_output_tokens": 128000, + "prompt_cache_key": prompt_cache_key }); let usert_turn_3_after_resume = json!( { @@ -494,8 +491,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": prompt_cache_key, - "max_output_tokens": 128000, + "prompt_cache_key": prompt_cache_key }); let user_turn_3_after_fork = json!( { @@ -575,8 +571,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "include": [ "reasoning.encrypted_content" ], - "prompt_cache_key": fork_prompt_cache_key, - "max_output_tokens": 128000, + "prompt_cache_key": fork_prompt_cache_key }); let mut expected = json!([ user_turn_1, From d363a0968e17a0f794b5cd9dd021c46738201f21 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 21 Nov 2025 08:16:36 -0800 Subject: [PATCH 12/13] feat: codex-shell-tool-mcp (#7005) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a GitHub workflow for building a new npm module we are experimenting with that contains an MCP server for running Bash commands. The new workflow, `shell-tool-mcp`, is a dependency of the general `release` workflow so that we continue to use one version number for all artifacts across the project in one GitHub release. `.github/workflows/shell-tool-mcp.yml` is the primary workflow introduced by this PR, which does the following: - builds the `codex-exec-mcp-server` and `codex-execve-wrapper` executables for both arm64 and x64 versions of Mac and Linux (preferring the MUSL version for Linux) - builds Bash (dynamically linked) for a [comically] large number of platforms (both x64 and arm64 for most) with a small patch specified by `shell-tool-mcp/patches/bash-exec-wrapper.patch`: - `debian-11` - `debian-12` - `ubuntu-20.04` - `ubuntu-22.04` - `ubuntu-24.04` - `centos-9` - `macos-13` (x64 only) - `macos-14` (arm64 only) - `macos-15` (arm64 only) - builds the TypeScript for the [new] Node module declared in the `shell-tool-mcp/` folder, which creates `bin/mcp-server.js` - adds all of the native binaries to `shell-tool-mcp/vendor/` folder; `bin/mcp-server.js` does a runtime check to determine which ones to execute - uses `npm pack` to create the `.tgz` for the module - if `publish: true` is set, invokes the `npm publish` call with the `.tgz` The justification for building Bash for so many different operating systems is because, since it is dynamically linked, we want to increase our confidence that the version we build is compatible with the glibc whatever OS we end up running on. (Note this is less of a concern with `codex-exec-mcp-server` and `codex-execve-wrapper` on Linux, as they are statically linked.) This PR also introduces the code for the npm module in `shell-tool-mcp/` (the proposed module name is `@openai/codex-shell-tool-mcp`). Initially, I intended the module to be a single file of vanilla JavaScript (like [`codex-cli/bin/codex.js`](https://github.com/openai/codex/blob/ab5972d447da78d3e4dd8461cf7d43a22e5d2acb/codex-cli/bin/codex.js)), but some of the logic seemed a bit tricky, so I decided to port it to TypeScript and add unit tests. `shell-tool-mcp/src/index.ts` defines the `main()` function for the module, which performs runtime checks to determine the clang triple to find the path to the Rust executables within the `vendor/` folder (`resolveTargetTriple()`). It uses a combination of `readOsRelease()` and `resolveBashPath()` to determine the correct Bash executable to run in the environment. Ultimately, it spawns a command like the following: ``` codex-exec-mcp-server \ --execve codex-execve-wrapper \ --bash custom-bash "$@" ``` Note `.github/workflows/shell-tool-mcp-ci.yml` defines a fairly standard CI job for the module (`format`/`build`/`test`). To test this PR, I pushed this branch to my personal fork of Codex and ran the CI job there: https://github.com/bolinfest/codex/actions/runs/19564311320 Admittedly, the graph looks a bit wild now: Screenshot 2025-11-20 at 11 44
58 PM But when it finished, I was able to download `codex-shell-tool-mcp-npm` from the **Artifacts** for the workflow in an empty temp directory, unzip the `.zip` and then the `.tgz` inside it, followed by `xattr -rc .` to remove the quarantine bits. Then I ran: ```shell npx @modelcontextprotocol/inspector node /private/tmp/foobar4/package/bin/mcp-server.js ``` which launched the MCP Inspector and I was able to use it as expected! This bodes well that this should work once the package is published to npm: ```shell npx @modelcontextprotocol/inspector npx @openai/codex-shell-tool-mcp ``` Also, to verify the package contains what I expect: ```shell /tmp/foobar4/package$ tree . ├── bin │   └── mcp-server.js ├── package.json ├── README.md └── vendor ├── aarch64-apple-darwin │   ├── bash │   │   ├── macos-14 │   │   │   └── bash │   │   └── macos-15 │   │   └── bash │   ├── codex-exec-mcp-server │   └── codex-execve-wrapper ├── aarch64-unknown-linux-musl │   ├── bash │   │   ├── centos-9 │   │   │   └── bash │   │   ├── debian-11 │   │   │   └── bash │   │   ├── debian-12 │   │   │   └── bash │   │   ├── ubuntu-20.04 │   │   │   └── bash │   │   ├── ubuntu-22.04 │   │   │   └── bash │   │   └── ubuntu-24.04 │   │   └── bash │   ├── codex-exec-mcp-server │   └── codex-execve-wrapper ├── x86_64-apple-darwin │   ├── bash │   │   └── macos-13 │   │   └── bash │   ├── codex-exec-mcp-server │   └── codex-execve-wrapper └── x86_64-unknown-linux-musl ├── bash │   ├── centos-9 │   │   └── bash │   ├── debian-11 │   │   └── bash │   ├── debian-12 │   │   └── bash │   ├── ubuntu-20.04 │   │   └── bash │   ├── ubuntu-22.04 │   │   └── bash │   └── ubuntu-24.04 │   └── bash ├── codex-exec-mcp-server └── codex-execve-wrapper 26 directories, 26 files ``` --- .github/workflows/rust-release.yml | 14 +- .github/workflows/shell-tool-mcp-ci.yml | 48 ++ .github/workflows/shell-tool-mcp.yml | 412 ++++++++++++++++++ pnpm-lock.yaml | 24 + pnpm-workspace.yaml | 1 + shell-tool-mcp/.gitignore | 2 + shell-tool-mcp/README.md | 31 ++ shell-tool-mcp/jest.config.cjs | 6 + shell-tool-mcp/package.json | 40 ++ .../patches/bash-exec-wrapper.patch | 24 + shell-tool-mcp/src/bashSelection.ts | 115 +++++ shell-tool-mcp/src/constants.ts | 20 + shell-tool-mcp/src/index.ts | 101 +++++ shell-tool-mcp/src/osRelease.ts | 34 ++ shell-tool-mcp/src/platform.ts | 21 + shell-tool-mcp/src/types.ts | 21 + shell-tool-mcp/tests/bashSelection.test.ts | 41 ++ shell-tool-mcp/tests/osRelease.test.ts | 30 ++ shell-tool-mcp/tsconfig.json | 13 + shell-tool-mcp/tsup.config.ts | 15 + 20 files changed, 1012 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/shell-tool-mcp-ci.yml create mode 100644 .github/workflows/shell-tool-mcp.yml create mode 100644 shell-tool-mcp/.gitignore create mode 100644 shell-tool-mcp/README.md create mode 100644 shell-tool-mcp/jest.config.cjs create mode 100644 shell-tool-mcp/package.json create mode 100644 shell-tool-mcp/patches/bash-exec-wrapper.patch create mode 100644 shell-tool-mcp/src/bashSelection.ts create mode 100644 shell-tool-mcp/src/constants.ts create mode 100644 shell-tool-mcp/src/index.ts create mode 100644 shell-tool-mcp/src/osRelease.ts create mode 100644 shell-tool-mcp/src/platform.ts create mode 100644 shell-tool-mcp/src/types.ts create mode 100644 shell-tool-mcp/tests/bashSelection.test.ts create mode 100644 shell-tool-mcp/tests/osRelease.test.ts create mode 100644 shell-tool-mcp/tsconfig.json create mode 100644 shell-tool-mcp/tsup.config.ts diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 6f27fbf543..5819c0a226 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -371,8 +371,20 @@ jobs: path: | codex-rs/dist/${{ matrix.target }}/* + shell-tool-mcp: + name: shell-tool-mcp + needs: tag-check + uses: ./.github/workflows/shell-tool-mcp.yml + with: + release-tag: ${{ github.ref_name }} + # We are not ready to publish yet. + publish: false + secrets: inherit + release: - needs: build + needs: + - build + - shell-tool-mcp name: release runs-on: ubuntu-latest permissions: diff --git a/.github/workflows/shell-tool-mcp-ci.yml b/.github/workflows/shell-tool-mcp-ci.yml new file mode 100644 index 0000000000..8354b127f9 --- /dev/null +++ b/.github/workflows/shell-tool-mcp-ci.yml @@ -0,0 +1,48 @@ +name: shell-tool-mcp CI + +on: + push: + paths: + - "shell-tool-mcp/**" + - ".github/workflows/shell-tool-mcp-ci.yml" + - "pnpm-lock.yaml" + - "pnpm-workspace.yaml" + pull_request: + paths: + - "shell-tool-mcp/**" + - ".github/workflows/shell-tool-mcp-ci.yml" + - "pnpm-lock.yaml" + - "pnpm-workspace.yaml" + +env: + NODE_VERSION: 22 + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v5 + + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + run_install: false + + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: ${{ env.NODE_VERSION }} + cache: "pnpm" + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Format check + run: pnpm --filter @openai/codex-shell-tool-mcp run format + + - name: Run tests + run: pnpm --filter @openai/codex-shell-tool-mcp test + + - name: Build + run: pnpm --filter @openai/codex-shell-tool-mcp run build diff --git a/.github/workflows/shell-tool-mcp.yml b/.github/workflows/shell-tool-mcp.yml new file mode 100644 index 0000000000..d12f57476d --- /dev/null +++ b/.github/workflows/shell-tool-mcp.yml @@ -0,0 +1,412 @@ +name: shell-tool-mcp + +on: + workflow_call: + inputs: + release-version: + description: Version to publish (x.y.z or x.y.z-alpha.N). Defaults to GITHUB_REF_NAME when it starts with rust-v. + required: false + type: string + release-tag: + description: Tag name to use when downloading release artifacts (defaults to rust-v). + required: false + type: string + publish: + description: Whether to publish to npm when the version is releasable. + required: false + default: true + type: boolean + +env: + NODE_VERSION: 22 + +jobs: + metadata: + runs-on: ubuntu-latest + outputs: + version: ${{ steps.compute.outputs.version }} + release_tag: ${{ steps.compute.outputs.release_tag }} + should_publish: ${{ steps.compute.outputs.should_publish }} + npm_tag: ${{ steps.compute.outputs.npm_tag }} + steps: + - name: Compute version and tags + id: compute + run: | + set -euo pipefail + + version="${{ inputs.release-version }}" + release_tag="${{ inputs.release-tag }}" + + if [[ -z "$version" ]]; then + if [[ -n "$release_tag" && "$release_tag" =~ ^rust-v.+ ]]; then + version="${release_tag#rust-v}" + elif [[ "${GITHUB_REF_NAME:-}" =~ ^rust-v.+ ]]; then + version="${GITHUB_REF_NAME#rust-v}" + release_tag="${GITHUB_REF_NAME}" + else + echo "release-version is required when GITHUB_REF_NAME is not a rust-v tag." + exit 1 + fi + fi + + if [[ -z "$release_tag" ]]; then + release_tag="rust-v${version}" + fi + + npm_tag="" + should_publish="false" + if [[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + should_publish="true" + elif [[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+-alpha\.[0-9]+$ ]]; then + should_publish="true" + npm_tag="alpha" + fi + + echo "version=${version}" >> "$GITHUB_OUTPUT" + echo "release_tag=${release_tag}" >> "$GITHUB_OUTPUT" + echo "npm_tag=${npm_tag}" >> "$GITHUB_OUTPUT" + echo "should_publish=${should_publish}" >> "$GITHUB_OUTPUT" + + rust-binaries: + name: Build Rust - ${{ matrix.target }} + needs: metadata + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + defaults: + run: + working-directory: codex-rs + strategy: + fail-fast: false + matrix: + include: + - runner: macos-15-xlarge + target: aarch64-apple-darwin + - runner: macos-15-xlarge + target: x86_64-apple-darwin + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + install_musl: true + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + install_musl: true + steps: + - name: Checkout repository + uses: actions/checkout@v5 + + - uses: dtolnay/rust-toolchain@1.90 + with: + targets: ${{ matrix.target }} + + - if: ${{ matrix.install_musl }} + name: Install musl build dependencies + run: | + sudo apt-get update + sudo apt-get install -y musl-tools pkg-config + + - name: Build exec server binaries + run: cargo build --release --target ${{ matrix.target }} --bin codex-exec-mcp-server --bin codex-execve-wrapper + + - name: Stage exec server binaries + run: | + dest="${GITHUB_WORKSPACE}/artifacts/vendor/${{ matrix.target }}" + mkdir -p "$dest" + cp "target/${{ matrix.target }}/release/codex-exec-mcp-server" "$dest/" + cp "target/${{ matrix.target }}/release/codex-execve-wrapper" "$dest/" + + - uses: actions/upload-artifact@v4 + with: + name: shell-tool-mcp-rust-${{ matrix.target }} + path: artifacts/** + if-no-files-found: error + + bash-linux: + name: Build Bash (Linux) - ${{ matrix.variant }} - ${{ matrix.target }} + needs: metadata + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + container: + image: ${{ matrix.image }} + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: ubuntu-24.04 + image: ubuntu:24.04 + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: ubuntu-22.04 + image: ubuntu:22.04 + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: ubuntu-20.04 + image: ubuntu:20.04 + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: debian-12 + image: debian:12 + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: debian-11 + image: debian:11 + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-musl + variant: centos-9 + image: quay.io/centos/centos:stream9 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: ubuntu-24.04 + image: arm64v8/ubuntu:24.04 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: ubuntu-22.04 + image: arm64v8/ubuntu:22.04 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: ubuntu-20.04 + image: arm64v8/ubuntu:20.04 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: debian-12 + image: arm64v8/debian:12 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: debian-11 + image: arm64v8/debian:11 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + variant: centos-9 + image: quay.io/centos/centos:stream9 + steps: + - name: Install build prerequisites + shell: bash + run: | + set -euo pipefail + if command -v apt-get >/dev/null 2>&1; then + apt-get update + DEBIAN_FRONTEND=noninteractive apt-get install -y git build-essential bison autoconf gettext + elif command -v dnf >/dev/null 2>&1; then + dnf install -y git gcc gcc-c++ make bison autoconf gettext + elif command -v yum >/dev/null 2>&1; then + yum install -y git gcc gcc-c++ make bison autoconf gettext + else + echo "Unsupported package manager in container" + exit 1 + fi + + - name: Checkout repository + uses: actions/checkout@v5 + + - name: Build patched Bash + shell: bash + run: | + set -euo pipefail + git clone --depth 1 https://github.com/bminor/bash /tmp/bash + cd /tmp/bash + git fetch --depth 1 origin a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b + git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b + git apply "${GITHUB_WORKSPACE}/shell-tool-mcp/patches/bash-exec-wrapper.patch" + ./configure --without-bash-malloc + cores="$(command -v nproc >/dev/null 2>&1 && nproc || getconf _NPROCESSORS_ONLN)" + make -j"${cores}" + + dest="${GITHUB_WORKSPACE}/artifacts/vendor/${{ matrix.target }}/bash/${{ matrix.variant }}" + mkdir -p "$dest" + cp bash "$dest/bash" + + - uses: actions/upload-artifact@v4 + with: + name: shell-tool-mcp-bash-${{ matrix.target }}-${{ matrix.variant }} + path: artifacts/** + if-no-files-found: error + + bash-darwin: + name: Build Bash (macOS) - ${{ matrix.variant }} - ${{ matrix.target }} + needs: metadata + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + - runner: macos-15-xlarge + target: aarch64-apple-darwin + variant: macos-15 + - runner: macos-14 + target: aarch64-apple-darwin + variant: macos-14 + - runner: macos-13 + target: x86_64-apple-darwin + variant: macos-13 + steps: + - name: Checkout repository + uses: actions/checkout@v5 + + - name: Build patched Bash + shell: bash + run: | + set -euo pipefail + git clone --depth 1 https://github.com/bminor/bash /tmp/bash + cd /tmp/bash + git fetch --depth 1 origin a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b + git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b + git apply "${GITHUB_WORKSPACE}/shell-tool-mcp/patches/bash-exec-wrapper.patch" + ./configure --without-bash-malloc + cores="$(getconf _NPROCESSORS_ONLN)" + make -j"${cores}" + + dest="${GITHUB_WORKSPACE}/artifacts/vendor/${{ matrix.target }}/bash/${{ matrix.variant }}" + mkdir -p "$dest" + cp bash "$dest/bash" + + - uses: actions/upload-artifact@v4 + with: + name: shell-tool-mcp-bash-${{ matrix.target }}-${{ matrix.variant }} + path: artifacts/** + if-no-files-found: error + + package: + name: Package npm module + needs: + - metadata + - rust-binaries + - bash-linux + - bash-darwin + runs-on: ubuntu-latest + env: + PACKAGE_VERSION: ${{ needs.metadata.outputs.version }} + steps: + - name: Checkout repository + uses: actions/checkout@v5 + + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 10.8.1 + run_install: false + + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: ${{ env.NODE_VERSION }} + + - name: Install JavaScript dependencies + run: pnpm install --frozen-lockfile + + - name: Build (shell-tool-mcp) + run: pnpm --filter @openai/codex-shell-tool-mcp run build + + - name: Download build artifacts + uses: actions/download-artifact@v4 + with: + path: artifacts + + - name: Assemble staging directory + id: staging + shell: bash + run: | + set -euo pipefail + staging="${STAGING_DIR}" + mkdir -p "$staging" "$staging/vendor" + cp shell-tool-mcp/README.md "$staging/" + cp shell-tool-mcp/package.json "$staging/" + cp -R shell-tool-mcp/bin "$staging/" + + found_vendor="false" + shopt -s nullglob + for vendor_dir in artifacts/*/vendor; do + rsync -av "$vendor_dir/" "$staging/vendor/" + found_vendor="true" + done + if [[ "$found_vendor" == "false" ]]; then + echo "No vendor payloads were downloaded." + exit 1 + fi + + node - <<'NODE' + import fs from "node:fs"; + import path from "node:path"; + + const stagingDir = process.env.STAGING_DIR; + const version = process.env.PACKAGE_VERSION; + const pkgPath = path.join(stagingDir, "package.json"); + const pkg = JSON.parse(fs.readFileSync(pkgPath, "utf8")); + pkg.version = version; + fs.writeFileSync(pkgPath, JSON.stringify(pkg, null, 2) + "\n"); + NODE + + echo "dir=$staging" >> "$GITHUB_OUTPUT" + env: + STAGING_DIR: ${{ runner.temp }}/shell-tool-mcp + + - name: Ensure binaries are executable + run: | + set -euo pipefail + staging="${{ steps.staging.outputs.dir }}" + chmod +x \ + "$staging"/vendor/*/codex-exec-mcp-server \ + "$staging"/vendor/*/codex-execve-wrapper \ + "$staging"/vendor/*/bash/*/bash + + - name: Create npm tarball + shell: bash + run: | + set -euo pipefail + mkdir -p dist/npm + staging="${{ steps.staging.outputs.dir }}" + pack_info=$(cd "$staging" && npm pack --ignore-scripts --json --pack-destination "${GITHUB_WORKSPACE}/dist/npm") + filename=$(PACK_INFO="$pack_info" node -e 'const data = JSON.parse(process.env.PACK_INFO); console.log(data[0].filename);') + mv "dist/npm/${filename}" "dist/npm/codex-shell-tool-mcp-npm-${PACKAGE_VERSION}.tgz" + + - uses: actions/upload-artifact@v4 + with: + name: codex-shell-tool-mcp-npm + path: dist/npm/codex-shell-tool-mcp-npm-${{ env.PACKAGE_VERSION }}.tgz + if-no-files-found: error + + publish: + name: Publish npm package + needs: + - metadata + - package + if: ${{ inputs.publish && needs.metadata.outputs.should_publish == 'true' }} + runs-on: ubuntu-latest + permissions: + id-token: write + contents: read + steps: + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 10.8.1 + run_install: false + + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: ${{ env.NODE_VERSION }} + registry-url: https://registry.npmjs.org + scope: "@openai" + + - name: Update npm + run: npm install -g npm@latest + + - name: Download npm tarball + uses: actions/download-artifact@v4 + with: + name: codex-shell-tool-mcp-npm + path: dist/npm + + - name: Publish to npm + env: + NPM_TAG: ${{ needs.metadata.outputs.npm_tag }} + VERSION: ${{ needs.metadata.outputs.version }} + shell: bash + run: | + set -euo pipefail + tag_args=() + if [[ -n "${NPM_TAG}" ]]; then + tag_args+=(--tag "${NPM_TAG}") + fi + npm publish "dist/npm/codex-shell-tool-mcp-npm-${VERSION}.tgz" "${tag_args[@]}" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8d944a66fc..03d88bce45 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -71,6 +71,30 @@ importers: specifier: ^3.24.6 version: 3.24.6(zod@3.25.76) + shell-tool-mcp: + devDependencies: + '@types/jest': + specifier: ^29.5.14 + version: 29.5.14 + '@types/node': + specifier: ^20.19.18 + version: 20.19.18 + jest: + specifier: ^29.7.0 + version: 29.7.0(@types/node@20.19.18)(ts-node@10.9.2(@types/node@20.19.18)(typescript@5.9.2)) + prettier: + specifier: ^3.6.2 + version: 3.6.2 + ts-jest: + specifier: ^29.3.4 + version: 29.4.4(@babel/core@7.28.4)(@jest/transform@29.7.0)(@jest/types@29.6.3)(babel-jest@29.7.0(@babel/core@7.28.4))(esbuild@0.25.10)(jest-util@29.7.0)(jest@29.7.0(@types/node@20.19.18)(ts-node@10.9.2(@types/node@20.19.18)(typescript@5.9.2)))(typescript@5.9.2) + tsup: + specifier: ^8.5.0 + version: 8.5.0(postcss@8.5.6)(typescript@5.9.2)(yaml@2.8.1) + typescript: + specifier: ^5.9.2 + version: 5.9.2 + packages: '@babel/code-frame@7.27.1': diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index fef2aa1911..ddd5674bf1 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,6 +1,7 @@ packages: - docs - sdk/typescript + - shell-tool-mcp ignoredBuiltDependencies: - esbuild diff --git a/shell-tool-mcp/.gitignore b/shell-tool-mcp/.gitignore new file mode 100644 index 0000000000..c6958891dd --- /dev/null +++ b/shell-tool-mcp/.gitignore @@ -0,0 +1,2 @@ +/bin/ +/node_modules/ diff --git a/shell-tool-mcp/README.md b/shell-tool-mcp/README.md new file mode 100644 index 0000000000..ba9d65a690 --- /dev/null +++ b/shell-tool-mcp/README.md @@ -0,0 +1,31 @@ +# @openai/codex-shell-tool-mcp + +This package wraps the `codex-exec-mcp-server` binary and its helpers so that the shell MCP can be invoked via `npx @openai/codex-shell-tool-mcp`. It bundles: + +- `codex-exec-mcp-server` and `codex-execve-wrapper` built for macOS (arm64, x64) and Linux (musl arm64, musl x64). +- A patched Bash that honors `BASH_EXEC_WRAPPER`, built for multiple glibc baselines (Ubuntu 24.04/22.04/20.04, Debian 12/11, CentOS-like 9) and macOS (15/14/13). +- A launcher (`bin/mcp-server.js`) that picks the correct binaries for the current `process.platform` / `process.arch`, specifying `--execve` and `--bash` for the MCP, as appropriate. + +## Usage + +```bash +npx @openai/codex-shell-tool-mcp --help +``` + +The launcher selects a Rust target triple based on the host and chooses the closest Bash variant by inspecting `/etc/os-release` on Linux or the Darwin major version on macOS. + +## Patched Bash + +We carry a small patch to `execute_cmd.c` (see `patches/bash-exec-wrapper.patch`) that adds support for `BASH_EXEC_WRAPPER`. The original commit message is “add support for BASH_EXEC_WRAPPER” and the patch applies cleanly to `a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b` from https://github.com/bminor/bash. To rebuild manually: + +```bash +git clone https://github.com/bminor/bash +git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b +git apply /path/to/patches/bash-exec-wrapper.patch +./configure --without-bash-malloc +make -j"$(nproc)" +``` + +## Release workflow + +`.github/workflows/shell-tool-mcp.yml` builds the Rust binaries, compiles the patched Bash variants, assembles the `vendor/` tree, and creates `codex-shell-tool-mcp-npm-.tgz` for inclusion in the Rust GitHub Release. When the version is a stable or alpha tag, the workflow also publishes the tarball to npm using OIDC. The workflow is invoked from `rust-release.yml` so the package ships alongside other Codex artifacts. diff --git a/shell-tool-mcp/jest.config.cjs b/shell-tool-mcp/jest.config.cjs new file mode 100644 index 0000000000..8ade9496e7 --- /dev/null +++ b/shell-tool-mcp/jest.config.cjs @@ -0,0 +1,6 @@ +/** @type {import('jest').Config} */ +module.exports = { + preset: "ts-jest", + testEnvironment: "node", + roots: ["/tests"], +}; diff --git a/shell-tool-mcp/package.json b/shell-tool-mcp/package.json new file mode 100644 index 0000000000..d27c0a0c59 --- /dev/null +++ b/shell-tool-mcp/package.json @@ -0,0 +1,40 @@ +{ + "name": "@openai/codex-shell-tool-mcp", + "version": "0.0.0-dev", + "description": "Codex MCP server for the shell tool with patched Bash and exec wrappers.", + "license": "Apache-2.0", + "bin": { + "codex-shell-tool-mcp": "bin/mcp-server.js" + }, + "engines": { + "node": ">=18" + }, + "files": [ + "bin", + "vendor", + "README.md" + ], + "repository": { + "type": "git", + "url": "git+https://github.com/openai/codex.git", + "directory": "shell-tool-mcp" + }, + "scripts": { + "clean": "rm -rf bin", + "build": "tsup", + "build:watch": "tsup --watch", + "test": "jest", + "test:watch": "jest --watch", + "format": "prettier --check .", + "format:fix": "prettier --write ." + }, + "devDependencies": { + "@types/jest": "^29.5.14", + "@types/node": "^20.19.18", + "jest": "^29.7.0", + "prettier": "^3.6.2", + "ts-jest": "^29.3.4", + "tsup": "^8.5.0", + "typescript": "^5.9.2" + } +} diff --git a/shell-tool-mcp/patches/bash-exec-wrapper.patch b/shell-tool-mcp/patches/bash-exec-wrapper.patch new file mode 100644 index 0000000000..6a7fedbb8f --- /dev/null +++ b/shell-tool-mcp/patches/bash-exec-wrapper.patch @@ -0,0 +1,24 @@ +diff --git a/execute_cmd.c b/execute_cmd.c +index 070f5119..d20ad2b9 100644 +--- a/execute_cmd.c ++++ b/execute_cmd.c +@@ -6129,6 +6129,19 @@ shell_execve (char *command, char **args, char **env) + char sample[HASH_BANG_BUFSIZ]; + size_t larray; + ++ char* exec_wrapper = getenv("BASH_EXEC_WRAPPER"); ++ if (exec_wrapper && *exec_wrapper && !whitespace (*exec_wrapper)) ++ { ++ char *orig_command = command; ++ ++ larray = strvec_len (args); ++ ++ memmove (args + 2, args, (++larray) * sizeof (char *)); ++ args[0] = exec_wrapper; ++ args[1] = orig_command; ++ command = exec_wrapper; ++ } ++ + SETOSTYPE (0); /* Some systems use for USG/POSIX semantics */ + execve (command, args, env); + i = errno; /* error from execve() */ diff --git a/shell-tool-mcp/src/bashSelection.ts b/shell-tool-mcp/src/bashSelection.ts new file mode 100644 index 0000000000..7da137f6a7 --- /dev/null +++ b/shell-tool-mcp/src/bashSelection.ts @@ -0,0 +1,115 @@ +import path from "node:path"; +import os from "node:os"; +import { DARWIN_BASH_VARIANTS, LINUX_BASH_VARIANTS } from "./constants"; +import { BashSelection, OsReleaseInfo } from "./types"; + +function supportedDetail(variants: ReadonlyArray<{ name: string }>): string { + return `Supported variants: ${variants.map((variant) => variant.name).join(", ")}`; +} + +export function selectLinuxBash( + bashRoot: string, + info: OsReleaseInfo, +): BashSelection { + const versionId = info.versionId; + const candidates: Array<{ + variant: (typeof LINUX_BASH_VARIANTS)[number]; + matchesVersion: boolean; + }> = []; + for (const variant of LINUX_BASH_VARIANTS) { + const matchesId = + variant.ids.includes(info.id) || + variant.ids.some((id) => info.idLike.includes(id)); + if (!matchesId) { + continue; + } + const matchesVersion = Boolean( + versionId && + variant.versions.some((prefix) => versionId.startsWith(prefix)), + ); + candidates.push({ variant, matchesVersion }); + } + + const pickVariant = (list: typeof candidates) => + list.find((item) => item.variant)?.variant; + + const preferred = pickVariant( + candidates.filter((item) => item.matchesVersion), + ); + if (preferred) { + return { + path: path.join(bashRoot, preferred.name, "bash"), + variant: preferred.name, + }; + } + + const fallbackMatch = pickVariant(candidates); + if (fallbackMatch) { + return { + path: path.join(bashRoot, fallbackMatch.name, "bash"), + variant: fallbackMatch.name, + }; + } + + const fallback = LINUX_BASH_VARIANTS[0]; + if (fallback) { + return { + path: path.join(bashRoot, fallback.name, "bash"), + variant: fallback.name, + }; + } + + const detail = supportedDetail(LINUX_BASH_VARIANTS); + throw new Error( + `Unable to select a Bash variant for ${info.id || "unknown"} ${versionId || ""}. ${detail}`, + ); +} + +export function selectDarwinBash( + bashRoot: string, + darwinRelease: string, +): BashSelection { + const darwinMajor = Number.parseInt(darwinRelease.split(".")[0] || "0", 10); + const preferred = DARWIN_BASH_VARIANTS.find( + (variant) => darwinMajor >= variant.minDarwin, + ); + if (preferred) { + return { + path: path.join(bashRoot, preferred.name, "bash"), + variant: preferred.name, + }; + } + + const fallback = DARWIN_BASH_VARIANTS[0]; + if (fallback) { + return { + path: path.join(bashRoot, fallback.name, "bash"), + variant: fallback.name, + }; + } + + const detail = supportedDetail(DARWIN_BASH_VARIANTS); + throw new Error( + `Unable to select a macOS Bash build (darwin ${darwinMajor}). ${detail}`, + ); +} + +export function resolveBashPath( + targetRoot: string, + platform: NodeJS.Platform, + darwinRelease = os.release(), + osInfo: OsReleaseInfo | null = null, +): BashSelection { + const bashRoot = path.join(targetRoot, "bash"); + + if (platform === "linux") { + if (!osInfo) { + throw new Error("Linux OS info is required to select a Bash variant."); + } + return selectLinuxBash(bashRoot, osInfo); + } + if (platform === "darwin") { + return selectDarwinBash(bashRoot, darwinRelease); + } + throw new Error(`Unsupported platform for Bash selection: ${platform}`); +} diff --git a/shell-tool-mcp/src/constants.ts b/shell-tool-mcp/src/constants.ts new file mode 100644 index 0000000000..e2bbcbf5b2 --- /dev/null +++ b/shell-tool-mcp/src/constants.ts @@ -0,0 +1,20 @@ +import { DarwinBashVariant, LinuxBashVariant } from "./types"; + +export const LINUX_BASH_VARIANTS: ReadonlyArray = [ + { name: "ubuntu-24.04", ids: ["ubuntu"], versions: ["24.04"] }, + { name: "ubuntu-22.04", ids: ["ubuntu"], versions: ["22.04"] }, + { name: "ubuntu-20.04", ids: ["ubuntu"], versions: ["20.04"] }, + { name: "debian-12", ids: ["debian"], versions: ["12"] }, + { name: "debian-11", ids: ["debian"], versions: ["11"] }, + { + name: "centos-9", + ids: ["centos", "rhel", "rocky", "almalinux"], + versions: ["9"], + }, +]; + +export const DARWIN_BASH_VARIANTS: ReadonlyArray = [ + { name: "macos-15", minDarwin: 24 }, + { name: "macos-14", minDarwin: 23 }, + { name: "macos-13", minDarwin: 22 }, +]; diff --git a/shell-tool-mcp/src/index.ts b/shell-tool-mcp/src/index.ts new file mode 100644 index 0000000000..2ce58462f4 --- /dev/null +++ b/shell-tool-mcp/src/index.ts @@ -0,0 +1,101 @@ +// Launches the codex-exec-mcp-server binary bundled in this package. + +import { spawn } from "node:child_process"; +import { accessSync, constants } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { resolveBashPath } from "./bashSelection"; +import { readOsRelease } from "./osRelease"; +import { resolveTargetTriple } from "./platform"; + +const scriptPath = process.argv[1] + ? path.resolve(process.argv[1]) + : process.cwd(); +const __dirname = path.dirname(scriptPath); + +async function main(): Promise { + const targetTriple = resolveTargetTriple(process.platform, process.arch); + const vendorRoot = path.join(__dirname, "..", "vendor"); + const targetRoot = path.join(vendorRoot, targetTriple); + const execveWrapperPath = path.join(targetRoot, "codex-execve-wrapper"); + const serverPath = path.join(targetRoot, "codex-exec-mcp-server"); + + const osInfo = process.platform === "linux" ? readOsRelease() : null; + const { path: bashPath } = resolveBashPath( + targetRoot, + process.platform, + os.release(), + osInfo, + ); + + [execveWrapperPath, serverPath, bashPath].forEach((checkPath) => { + try { + accessSync(checkPath, constants.F_OK); + } catch { + throw new Error(`Required binary missing: ${checkPath}`); + } + }); + + const args = [ + "--execve", + execveWrapperPath, + "--bash", + bashPath, + ...process.argv.slice(2), + ]; + const child = spawn(serverPath, args, { + stdio: "inherit", + }); + + const forwardSignal = (signal: NodeJS.Signals) => { + if (child.killed) { + return; + } + try { + child.kill(signal); + } catch { + /* ignore */ + } + }; + + (["SIGINT", "SIGTERM", "SIGHUP"] as const).forEach((sig) => { + process.on(sig, () => forwardSignal(sig)); + }); + + child.on("error", (err) => { + // eslint-disable-next-line no-console + console.error(err); + process.exit(1); + }); + + const childResult = await new Promise< + | { type: "signal"; signal: NodeJS.Signals } + | { type: "code"; exitCode: number } + >((resolve) => { + child.on("exit", (code, signal) => { + if (signal) { + resolve({ type: "signal", signal }); + } else { + resolve({ type: "code", exitCode: code ?? 1 }); + } + }); + }); + + if (childResult.type === "signal") { + // This environment running under `node --test` may not allow rethrowing a signal. + // Wrap in a try to avoid masking the original termination reason. + try { + process.kill(process.pid, childResult.signal); + } catch { + process.exit(1); + } + } else { + process.exit(childResult.exitCode); + } +} + +void main().catch((err) => { + // eslint-disable-next-line no-console + console.error(err); + process.exit(1); +}); diff --git a/shell-tool-mcp/src/osRelease.ts b/shell-tool-mcp/src/osRelease.ts new file mode 100644 index 0000000000..3e55bfb1da --- /dev/null +++ b/shell-tool-mcp/src/osRelease.ts @@ -0,0 +1,34 @@ +import { readFileSync } from "node:fs"; +import { OsReleaseInfo } from "./types"; + +export function parseOsRelease(contents: string): OsReleaseInfo { + const lines = contents.split("\n").filter(Boolean); + const info: Record = {}; + for (const line of lines) { + const [rawKey, rawValue] = line.split("=", 2); + if (!rawKey || rawValue === undefined) { + continue; + } + const key = rawKey.toLowerCase(); + const value = rawValue.replace(/^"/, "").replace(/"$/, ""); + info[key] = value; + } + const idLike = (info.id_like || "") + .split(/\s+/) + .map((item) => item.trim().toLowerCase()) + .filter(Boolean); + return { + id: (info.id || "").toLowerCase(), + idLike, + versionId: info.version_id || "", + }; +} + +export function readOsRelease(pathname = "/etc/os-release"): OsReleaseInfo { + try { + const contents = readFileSync(pathname, "utf8"); + return parseOsRelease(contents); + } catch { + return { id: "", idLike: [], versionId: "" }; + } +} diff --git a/shell-tool-mcp/src/platform.ts b/shell-tool-mcp/src/platform.ts new file mode 100644 index 0000000000..177ba52b22 --- /dev/null +++ b/shell-tool-mcp/src/platform.ts @@ -0,0 +1,21 @@ +export function resolveTargetTriple( + platform: NodeJS.Platform, + arch: NodeJS.Architecture, +): string { + if (platform === "linux") { + if (arch === "x64") { + return "x86_64-unknown-linux-musl"; + } + if (arch === "arm64") { + return "aarch64-unknown-linux-musl"; + } + } else if (platform === "darwin") { + if (arch === "x64") { + return "x86_64-apple-darwin"; + } + if (arch === "arm64") { + return "aarch64-apple-darwin"; + } + } + throw new Error(`Unsupported platform: ${platform} (${arch})`); +} diff --git a/shell-tool-mcp/src/types.ts b/shell-tool-mcp/src/types.ts new file mode 100644 index 0000000000..28748101c2 --- /dev/null +++ b/shell-tool-mcp/src/types.ts @@ -0,0 +1,21 @@ +export type LinuxBashVariant = { + name: string; + ids: Array; + versions: Array; +}; + +export type DarwinBashVariant = { + name: string; + minDarwin: number; +}; + +export type OsReleaseInfo = { + id: string; + idLike: Array; + versionId: string; +}; + +export type BashSelection = { + path: string; + variant: string; +}; diff --git a/shell-tool-mcp/tests/bashSelection.test.ts b/shell-tool-mcp/tests/bashSelection.test.ts new file mode 100644 index 0000000000..90753e675b --- /dev/null +++ b/shell-tool-mcp/tests/bashSelection.test.ts @@ -0,0 +1,41 @@ +import { selectDarwinBash, selectLinuxBash } from "../src/bashSelection"; +import { DARWIN_BASH_VARIANTS, LINUX_BASH_VARIANTS } from "../src/constants"; +import { OsReleaseInfo } from "../src/types"; +import path from "node:path"; + +describe("selectLinuxBash", () => { + const bashRoot = "/vendor/bash"; + + it("prefers exact version match when id is present", () => { + const info: OsReleaseInfo = { + id: "ubuntu", + idLike: ["debian"], + versionId: "24.04.1", + }; + const selection = selectLinuxBash(bashRoot, info); + expect(selection.variant).toBe("ubuntu-24.04"); + expect(selection.path).toBe(path.join(bashRoot, "ubuntu-24.04", "bash")); + }); + + it("falls back to first supported variant when no matches", () => { + const info: OsReleaseInfo = { id: "unknown", idLike: [], versionId: "1.0" }; + const selection = selectLinuxBash(bashRoot, info); + expect(selection.variant).toBe(LINUX_BASH_VARIANTS[0].name); + }); +}); + +describe("selectDarwinBash", () => { + const bashRoot = "/vendor/bash"; + + it("selects compatible darwin version", () => { + const darwinRelease = "24.0.0"; + const selection = selectDarwinBash(bashRoot, darwinRelease); + expect(selection.variant).toBe("macos-15"); + }); + + it("falls back to first darwin variant when release too old", () => { + const darwinRelease = "20.0.0"; + const selection = selectDarwinBash(bashRoot, darwinRelease); + expect(selection.variant).toBe(DARWIN_BASH_VARIANTS[0].name); + }); +}); diff --git a/shell-tool-mcp/tests/osRelease.test.ts b/shell-tool-mcp/tests/osRelease.test.ts new file mode 100644 index 0000000000..2291129868 --- /dev/null +++ b/shell-tool-mcp/tests/osRelease.test.ts @@ -0,0 +1,30 @@ +import { parseOsRelease } from "../src/osRelease"; + +describe("parseOsRelease", () => { + it("parses basic fields", () => { + const contents = `ID="ubuntu" +ID_LIKE="debian" +VERSION_ID=24.04 +OTHER=ignored`; + + const info = parseOsRelease(contents); + expect(info).toEqual({ + id: "ubuntu", + idLike: ["debian"], + versionId: "24.04", + }); + }); + + it("handles missing fields", () => { + const contents = "SOMETHING=else"; + const info = parseOsRelease(contents); + expect(info).toEqual({ id: "", idLike: [], versionId: "" }); + }); + + it("normalizes id_like entries", () => { + const contents = `ID="rhel" +ID_LIKE="CentOS Rocky"`; + const info = parseOsRelease(contents); + expect(info.idLike).toEqual(["centos", "rocky"]); + }); +}); diff --git a/shell-tool-mcp/tsconfig.json b/shell-tool-mcp/tsconfig.json new file mode 100644 index 0000000000..90bb3e0d78 --- /dev/null +++ b/shell-tool-mcp/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "CommonJS", + "moduleResolution": "Node", + "noEmit": true, + "strict": true, + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "skipLibCheck": true + }, + "include": ["src", "tests"] +} diff --git a/shell-tool-mcp/tsup.config.ts b/shell-tool-mcp/tsup.config.ts new file mode 100644 index 0000000000..8e4e2d4e38 --- /dev/null +++ b/shell-tool-mcp/tsup.config.ts @@ -0,0 +1,15 @@ +import { defineConfig } from "tsup"; + +export default defineConfig({ + entry: { + "mcp-server": "src/index.ts", + }, + outDir: "bin", + format: ["cjs"], + target: "node18", + clean: true, + sourcemap: false, + banner: { + js: "#!/usr/bin/env node", + }, +}); From 7a13c6a1b6b792123b8752dd6cc994e6251fcac7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 21 Nov 2025 09:50:02 -0800 Subject: [PATCH 13/13] fix: clear out duplicate entries for `bash` in the GitHub release --- .github/workflows/rust-release.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 5819c0a226..5c814ac8f7 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -407,6 +407,14 @@ jobs: - name: List run: ls -R dist/ + # This is a temporary fix: we should modify shell-tool-mcp.yml so these + # files do not end up in dist/ in the first place. + - name: Delete entries from dist/ that should not go in the release + run: | + rm -rf dist/shell-tool-mcp* + + ls -R dist/ + - name: Define release name id: release_name run: |