diff --git a/.github/workflows/shell-tool-mcp.yml b/.github/workflows/shell-tool-mcp.yml index 04e127a785..609c933bd9 100644 --- a/.github/workflows/shell-tool-mcp.yml +++ b/.github/workflows/shell-tool-mcp.yml @@ -408,9 +408,8 @@ jobs: shell: bash run: | set -euo pipefail - git clone --depth 1 https://git.code.sf.net/p/zsh/code /tmp/zsh + git clone https://git.code.sf.net/p/zsh/code /tmp/zsh cd /tmp/zsh - git fetch --depth 1 origin 77045ef899e53b9598bebc5a41db93a548a40ca6 git checkout 77045ef899e53b9598bebc5a41db93a548a40ca6 git apply "${GITHUB_WORKSPACE}/shell-tool-mcp/patches/zsh-exec-wrapper.patch" ./Util/preconfig @@ -487,9 +486,8 @@ jobs: shell: bash run: | set -euo pipefail - git clone --depth 1 https://git.code.sf.net/p/zsh/code /tmp/zsh + git clone https://git.code.sf.net/p/zsh/code /tmp/zsh cd /tmp/zsh - git fetch --depth 1 origin 77045ef899e53b9598bebc5a41db93a548a40ca6 git checkout 77045ef899e53b9598bebc5a41db93a548a40ca6 git apply "${GITHUB_WORKSPACE}/shell-tool-mcp/patches/zsh-exec-wrapper.patch" ./Util/preconfig diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index aece5fa3cb..27bb78e49e 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -557,7 +557,7 @@ Today both notifications carry an empty `items` array even when item events were `ThreadItem` is the tagged union carried in turn responses and `item/*` notifications. Currently we support events for the following items: -- `userMessage` — `{id, content}` where `content` is a list of user inputs (`text`, `image`, or `localImage`). +- `userMessage` — `{id, content}` where `content` is a list of user inputs (`text`, `image`, or `localImage`). Cyber model-routing warnings are surfaced as synthetic `userMessage` items with `text` prefixed by `Warning:`. - `agentMessage` — `{id, text}` containing the accumulated agent reply. - `plan` — `{id, text}` emitted for plan-mode turns; plan text can stream via `item/plan/delta` (experimental). - `reasoning` — `{id, summary, content}` where `summary` holds streamed reasoning summaries (applicable for most OpenAI models) and `content` holds raw reasoning blocks (applicable for e.g. open source models). diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index da7abe0593..87ddf7e18a 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -68,6 +68,7 @@ use codex_app_server_protocol::TurnInterruptResponse; use codex_app_server_protocol::TurnPlanStep; use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStatus; +use codex_app_server_protocol::UserInput as V2UserInput; use codex_app_server_protocol::build_turns_from_rollout_items; use codex_core::CodexThread; use codex_core::parse_command::shlex_join; @@ -95,6 +96,8 @@ use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUse use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; use std::collections::HashMap; use std::convert::TryFrom; +use std::hash::Hash; +use std::hash::Hasher; use std::path::PathBuf; use std::sync::Arc; use tokio::sync::Mutex; @@ -122,6 +125,35 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::TurnComplete(_ev) => { handle_turn_complete(conversation_id, event_turn_id, &outgoing, &thread_state).await; } + EventMsg::Warning(warning_event) => { + if matches!(api_version, ApiVersion::V2) + && is_safety_check_downgrade_warning(&warning_event.message) + { + let item = ThreadItem::UserMessage { + id: warning_item_id(&event_turn_id, &warning_event.message), + content: vec![V2UserInput::Text { + text: format!("Warning: {}", warning_event.message), + text_elements: Vec::new(), + }], + }; + let started = ItemStartedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item: item.clone(), + }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(started)) + .await; + let completed = ItemCompletedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemCompleted(completed)) + .await; + } + } EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { call_id, turn_id, @@ -1286,6 +1318,18 @@ async fn complete_command_execution_item( .await; } +fn is_safety_check_downgrade_warning(message: &str) -> bool { + message.contains("Your account was flagged for potentially high-risk cyber activity") + && message.contains("apply for trusted access: https://chatgpt.com/cyber") +} + +fn warning_item_id(turn_id: &str, message: &str) -> String { + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + message.hash(&mut hasher); + let digest = hasher.finish(); + format!("{turn_id}-warning-{digest:x}") +} + async fn maybe_emit_raw_response_item_completed( api_version: ApiVersion, conversation_id: ThreadId, @@ -2016,6 +2060,18 @@ mod tests { assert_eq!(item, expected); } + #[test] + fn safety_check_downgrade_warning_detection_matches_expected_message() { + let warning = "Your account was flagged for potentially high-risk cyber activity and this request was routed to gpt-5.2 as a fallback. To regain access to gpt-5.3-codex, apply for trusted access: https://chatgpt.com/cyber\nLearn more: https://developers.openai.com/codex/concepts/cyber-safety"; + assert!(is_safety_check_downgrade_warning(warning)); + } + + #[test] + fn safety_check_downgrade_warning_detection_ignores_other_warnings() { + let warning = "Model metadata for `mock-model` not found. Defaulting to fallback metadata; this can degrade performance and cause issues."; + assert!(!is_safety_check_downgrade_warning(warning)); + } + #[tokio::test] async fn test_handle_error_records_message() -> Result<()> { let conversation_id = ThreadId::new(); diff --git a/codex-rs/app-server/tests/suite/send_message.rs b/codex-rs/app-server/tests/suite/send_message.rs index d5e0154121..25dad0e0ae 100644 --- a/codex-rs/app-server/tests/suite/send_message.rs +++ b/codex-rs/app-server/tests/suite/send_message.rs @@ -477,7 +477,6 @@ fn assert_permissions_message(item: &ResponseItem) { &SandboxPolicy::DangerFullAccess, AskForApproval::Never, &Policy::empty(), - false, &PathBuf::from("/tmp"), ) .into_text(); diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 48622acddb..1eacc2a84e 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -15,6 +15,7 @@ mod plan_item; mod rate_limits; mod request_user_input; mod review; +mod safety_check_downgrade; mod skills_list; mod thread_archive; mod thread_fork; diff --git a/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs b/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs new file mode 100644 index 0000000000..20a7c60236 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs @@ -0,0 +1,266 @@ +use anyhow::Result; +use app_test_support::McpProcess; +use app_test_support::to_response; +use codex_app_server_protocol::ItemCompletedNotification; +use codex_app_server_protocol::ItemStartedNotification; +use codex_app_server_protocol::JSONRPCNotification; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::UserInput; +use core_test_support::responses; +use core_test_support::skip_if_no_network; +use pretty_assertions::assert_eq; +use tempfile::TempDir; +use tokio::time::timeout; + +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +const REQUESTED_MODEL: &str = "gpt-5.1-codex-max"; +const SERVER_MODEL: &str = "gpt-5.2-codex"; + +#[tokio::test] +async fn openai_model_header_mismatch_emits_warning_item_v2() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("msg-1", "Done"), + responses::ev_completed("resp-1"), + ]); + let response = responses::sse_response(body).insert_header("OpenAI-Model", SERVER_MODEL); + let _response_mock = responses::mount_response_once(&server, response).await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams { + model: Some(REQUESTED_MODEL.to_string()), + ..Default::default() + }) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "trigger safeguard".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let _turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + let _turn_start: TurnStartResponse = to_response(_turn_resp)?; + + let warning_started = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notification: JSONRPCNotification = mcp + .read_stream_until_notification_message("item/started") + .await?; + let params = notification.params.expect("item/started params"); + let started: ItemStartedNotification = + serde_json::from_value(params).expect("deserialize item/started"); + if warning_text_from_item(&started.item).is_some_and(is_cyber_model_warning_text) { + return Ok::(started); + } + } + }) + .await??; + + let warning_text = + warning_text_from_item(&warning_started.item).expect("expected warning user message item"); + assert!(warning_text.contains("Warning:")); + assert!(warning_text.contains("gpt-5.2 as a fallback")); + assert!(warning_text.contains("regain access to gpt-5.3-codex")); + + let warning_completed = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notification: JSONRPCNotification = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let params = notification.params.expect("item/completed params"); + let completed: ItemCompletedNotification = + serde_json::from_value(params).expect("deserialize item/completed"); + if warning_text_from_item(&completed.item).is_some_and(is_cyber_model_warning_text) { + return Ok::(completed); + } + } + }) + .await??; + assert_eq!( + warning_text_from_item(&warning_completed.item), + warning_text_from_item(&warning_started.item) + ); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + +#[tokio::test] +async fn response_model_field_mismatch_emits_warning_item_v2_when_header_matches_requested() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + serde_json::json!({ + "type": "response.created", + "response": { + "id": "resp-1", + "model": SERVER_MODEL, + } + }), + responses::ev_assistant_message("msg-1", "Done"), + responses::ev_completed("resp-1"), + ]); + let response = responses::sse_response(body).insert_header("OpenAI-Model", REQUESTED_MODEL); + let _response_mock = responses::mount_response_once(&server, response).await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams { + model: Some(REQUESTED_MODEL.to_string()), + ..Default::default() + }) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "trigger response model check".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + let _turn_start: TurnStartResponse = to_response(turn_resp)?; + + let warning_started = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notification: JSONRPCNotification = mcp + .read_stream_until_notification_message("item/started") + .await?; + let params = notification.params.expect("item/started params"); + let started: ItemStartedNotification = + serde_json::from_value(params).expect("deserialize item/started"); + if warning_text_from_item(&started.item).is_some_and(is_cyber_model_warning_text) { + return Ok::(started); + } + } + }) + .await??; + let warning_text = + warning_text_from_item(&warning_started.item).expect("expected warning user message item"); + assert!(warning_text.contains("gpt-5.2 as a fallback")); + + let warning_completed = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notification: JSONRPCNotification = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let params = notification.params.expect("item/completed params"); + let completed: ItemCompletedNotification = + serde_json::from_value(params).expect("deserialize item/completed"); + if warning_text_from_item(&completed.item).is_some_and(is_cyber_model_warning_text) { + return Ok::(completed); + } + } + }) + .await??; + assert_eq!( + warning_text_from_item(&warning_completed.item), + warning_text_from_item(&warning_started.item) + ); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + +fn warning_text_from_item(item: &ThreadItem) -> Option<&str> { + let ThreadItem::UserMessage { content, .. } = item else { + return None; + }; + + content.iter().find_map(|input| match input { + UserInput::Text { text, .. } if text.starts_with("Warning: ") => Some(text.as_str()), + _ => None, + }) +} + +fn is_cyber_model_warning_text(text: &str) -> bool { + text.contains("flagged for potentially high-risk cyber activity") + && text.contains("apply for trusted access: https://chatgpt.com/cyber") +} + +fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> { + let config_toml = codex_home.join("config.toml"); + std::fs::write( + config_toml, + format!( + r#" +model = "{REQUESTED_MODEL}" +approval_policy = "never" +sandbox_mode = "read-only" + +model_provider = "mock_provider" + +[features] +remote_models = false +personality = true + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{server_uri}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +"# + ), + ) +} diff --git a/codex-rs/codex-api/src/common.rs b/codex-rs/codex-api/src/common.rs index af08b6c977..3ef96c185b 100644 --- a/codex-rs/codex-api/src/common.rs +++ b/codex-rs/codex-api/src/common.rs @@ -56,6 +56,9 @@ pub enum ResponseEvent { Created, OutputItemDone(ResponseItem), OutputItemAdded(ResponseItem), + /// Emitted when the server includes `OpenAI-Model` on the stream response. + /// This can differ from the requested model when backend safety routing applies. + ServerModel(String), /// Emitted when `X-Reasoning-Included: true` is present on the response, /// meaning the server already accounted for past reasoning tokens and the /// client should not re-estimate them. diff --git a/codex-rs/codex-api/src/endpoint/aggregate.rs b/codex-rs/codex-api/src/endpoint/aggregate.rs index a91eec90a3..6d8e785c82 100644 --- a/codex-rs/codex-api/src/endpoint/aggregate.rs +++ b/codex-rs/codex-api/src/endpoint/aggregate.rs @@ -63,6 +63,9 @@ impl Stream for AggregatedStream { Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))) => { return Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))); } + Poll::Ready(Some(Ok(ResponseEvent::ServerModel(model)))) => { + return Poll::Ready(Some(Ok(ResponseEvent::ServerModel(model)))); + } Poll::Ready(Some(Ok(ResponseEvent::Completed { response_id, token_usage, diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index 6ebf5ab65e..aa559e9836 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -163,6 +163,7 @@ impl Drop for WsStream { const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state"; const X_MODELS_ETAG_HEADER: &str = "x-models-etag"; const X_REASONING_INCLUDED_HEADER: &str = "x-reasoning-included"; +const OPENAI_MODEL_HEADER: &str = "openai-model"; pub struct ResponsesWebsocketConnection { stream: Arc>>, @@ -170,6 +171,7 @@ pub struct ResponsesWebsocketConnection { idle_timeout: Duration, server_reasoning_included: bool, models_etag: Option, + server_model: Option, telemetry: Option>, } @@ -179,6 +181,7 @@ impl ResponsesWebsocketConnection { idle_timeout: Duration, server_reasoning_included: bool, models_etag: Option, + server_model: Option, telemetry: Option>, ) -> Self { Self { @@ -186,6 +189,7 @@ impl ResponsesWebsocketConnection { idle_timeout, server_reasoning_included, models_etag, + server_model, telemetry, } } @@ -204,12 +208,16 @@ impl ResponsesWebsocketConnection { let idle_timeout = self.idle_timeout; let server_reasoning_included = self.server_reasoning_included; let models_etag = self.models_etag.clone(); + let server_model = self.server_model.clone(); let telemetry = self.telemetry.clone(); let request_body = serde_json::to_value(&request).map_err(|err| { ApiError::Stream(format!("failed to encode websocket request: {err}")) })?; tokio::spawn(async move { + if let Some(model) = server_model { + let _ = tx_event.send(Ok(ResponseEvent::ServerModel(model))).await; + } if let Some(etag) = models_etag { let _ = tx_event.send(Ok(ResponseEvent::ModelsEtag(etag))).await; } @@ -273,13 +281,14 @@ impl ResponsesWebsocketClient { merge_request_headers(&self.provider.headers, extra_headers, default_headers); add_auth_headers_to_header_map(&self.auth, &mut headers); - let (stream, server_reasoning_included, models_etag) = + let (stream, server_reasoning_included, models_etag, server_model) = connect_websocket(ws_url, headers, turn_state.clone()).await?; Ok(ResponsesWebsocketConnection::new( stream, self.provider.stream_idle_timeout, server_reasoning_included, models_etag, + server_model, telemetry, )) } @@ -304,7 +313,7 @@ async fn connect_websocket( url: Url, headers: HeaderMap, turn_state: Option>>, -) -> Result<(WsStream, bool, Option), ApiError> { +) -> Result<(WsStream, bool, Option, Option), ApiError> { ensure_rustls_crypto_provider(); info!("connecting to websocket: {url}"); @@ -341,6 +350,11 @@ async fn connect_websocket( .get(X_MODELS_ETAG_HEADER) .and_then(|value| value.to_str().ok()) .map(ToString::to_string); + let server_model = response + .headers() + .get(OPENAI_MODEL_HEADER) + .and_then(|value| value.to_str().ok()) + .map(ToString::to_string); if let Some(turn_state) = turn_state && let Some(header_value) = response .headers() @@ -349,7 +363,12 @@ async fn connect_websocket( { let _ = turn_state.set(header_value.to_string()); } - Ok((WsStream::new(stream), reasoning_included, models_etag)) + Ok(( + WsStream::new(stream), + reasoning_included, + models_etag, + server_model, + )) } fn websocket_config() -> WebSocketConfig { @@ -469,6 +488,7 @@ async fn run_websocket_response_stream( idle_timeout: Duration, telemetry: Option>, ) -> Result<(), ApiError> { + let mut last_server_model: Option = None; let request_text = match serde_json::to_string(&request_body) { Ok(text) => text, Err(err) => { @@ -536,6 +556,14 @@ async fn run_websocket_response_stream( } continue; } + if let Some(model) = event.response_model() + && last_server_model.as_deref() != Some(model.as_str()) + { + let _ = tx_event + .send(Ok(ResponseEvent::ServerModel(model.clone()))) + .await; + last_server_model = Some(model); + } match process_responses_event(event) { Ok(Some(event)) => { let is_completed = matches!(event, ResponseEvent::Completed { .. }); diff --git a/codex-rs/codex-api/src/sse/responses.rs b/codex-rs/codex-api/src/sse/responses.rs index 11e4f9de34..75c79ba5c1 100644 --- a/codex-rs/codex-api/src/sse/responses.rs +++ b/codex-rs/codex-api/src/sse/responses.rs @@ -26,6 +26,7 @@ use tracing::debug; use tracing::trace; const X_REASONING_INCLUDED_HEADER: &str = "x-reasoning-included"; +const OPENAI_MODEL_HEADER: &str = "openai-model"; /// Streams SSE events from an on-disk fixture for tests. pub fn stream_from_fixture( @@ -60,6 +61,11 @@ pub fn spawn_response_stream( .get("X-Models-Etag") .and_then(|v| v.to_str().ok()) .map(ToString::to_string); + let server_model = stream_response + .headers + .get(OPENAI_MODEL_HEADER) + .and_then(|v| v.to_str().ok()) + .map(ToString::to_string); let reasoning_included = stream_response .headers .get(X_REASONING_INCLUDED_HEADER) @@ -74,6 +80,9 @@ pub fn spawn_response_stream( } let (tx_event, rx_event) = mpsc::channel::>(1600); tokio::spawn(async move { + if let Some(model) = server_model { + let _ = tx_event.send(Ok(ResponseEvent::ServerModel(model))).await; + } for snapshot in rate_limit_snapshots { let _ = tx_event.send(Ok(ResponseEvent::RateLimits(snapshot))).await; } @@ -169,6 +178,41 @@ impl ResponsesStreamEvent { pub fn kind(&self) -> &str { &self.kind } + + pub fn response_model(&self) -> Option { + self.response.as_ref().and_then(extract_server_model) + } +} + +fn extract_server_model(value: &Value) -> Option { + value + .get("model") + .and_then(json_value_as_string) + .or_else(|| { + value + .get("headers") + .and_then(header_openai_model_value_from_json) + }) +} + +fn header_openai_model_value_from_json(value: &Value) -> Option { + let headers = value.as_object()?; + headers.iter().find_map(|(name, value)| { + if name.eq_ignore_ascii_case("openai-model") || name.eq_ignore_ascii_case("x-openai-model") + { + json_value_as_string(value) + } else { + None + } + }) +} + +fn json_value_as_string(value: &Value) -> Option { + match value { + Value::String(value) => Some(value.clone()), + Value::Array(items) => items.first().and_then(json_value_as_string), + _ => None, + } } #[derive(Debug)] @@ -339,6 +383,7 @@ pub async fn process_sse( ) { let mut stream = stream.eventsource(); let mut response_error: Option = None; + let mut last_server_model: Option = None; loop { let start = Instant::now(); @@ -378,6 +423,19 @@ pub async fn process_sse( } }; + if let Some(model) = event.response_model() + && last_server_model.as_deref() != Some(model.as_str()) + { + if tx_event + .send(Ok(ResponseEvent::ServerModel(model.clone()))) + .await + .is_err() + { + return; + } + last_server_model = Some(model); + } + match process_responses_event(event) { Ok(Some(event)) => { let is_completed = matches!(event, ResponseEvent::Completed { .. }); @@ -456,9 +514,13 @@ mod tests { use super::*; use assert_matches::assert_matches; use bytes::Bytes; + use codex_client::StreamResponse; use codex_protocol::models::MessagePhase; use codex_protocol::models::ResponseItem; use futures::stream; + use http::HeaderMap; + use http::HeaderValue; + use http::StatusCode; use pretty_assertions::assert_eq; use serde_json::json; use tokio::sync::mpsc; @@ -870,6 +932,149 @@ mod tests { } } + #[tokio::test] + async fn spawn_response_stream_emits_server_model_header() { + let mut headers = HeaderMap::new(); + headers.insert( + OPENAI_MODEL_HEADER, + HeaderValue::from_static(CYBER_RESTRICTED_MODEL_FOR_TESTS), + ); + let bytes = stream::iter(Vec::>::new()); + let stream_response = StreamResponse { + status: StatusCode::OK, + headers, + bytes: Box::pin(bytes), + }; + + let mut stream = spawn_response_stream(stream_response, idle_timeout(), None, None); + let event = stream + .rx_event + .recv() + .await + .expect("expected server model event") + .expect("expected ok event"); + + match event { + ResponseEvent::ServerModel(model) => { + assert_eq!(model, CYBER_RESTRICTED_MODEL_FOR_TESTS); + } + other => panic!("expected server model event, got {other:?}"), + } + } + + #[tokio::test] + async fn process_sse_emits_server_model_from_response_payload() { + let events = run_sse(vec![ + json!({ + "type": "response.created", + "response": { + "id": "resp-1", + "model": CYBER_RESTRICTED_MODEL_FOR_TESTS + } + }), + json!({ + "type": "response.completed", + "response": { + "id": "resp-1", + "model": CYBER_RESTRICTED_MODEL_FOR_TESTS + } + }), + ]) + .await; + + assert_eq!(events.len(), 3); + assert_matches!( + &events[0], + ResponseEvent::ServerModel(model) if model == CYBER_RESTRICTED_MODEL_FOR_TESTS + ); + assert_matches!(&events[1], ResponseEvent::Created); + assert_matches!( + &events[2], + ResponseEvent::Completed { + response_id, + token_usage: None, + can_append: false + } if response_id == "resp-1" + ); + } + + #[tokio::test] + async fn process_sse_emits_server_model_from_response_headers_payload() { + let events = run_sse(vec![ + json!({ + "type": "response.created", + "response": { + "id": "resp-1", + "headers": { + "OpenAI-Model": CYBER_RESTRICTED_MODEL_FOR_TESTS + } + } + }), + json!({ + "type": "response.completed", + "response": { + "id": "resp-1" + } + }), + ]) + .await; + + assert_eq!(events.len(), 3); + assert_matches!( + &events[0], + ResponseEvent::ServerModel(model) if model == CYBER_RESTRICTED_MODEL_FOR_TESTS + ); + assert_matches!(&events[1], ResponseEvent::Created); + assert_matches!( + &events[2], + ResponseEvent::Completed { + response_id, + token_usage: None, + can_append: false + } if response_id == "resp-1" + ); + } + + #[tokio::test] + async fn process_sse_emits_server_model_again_when_response_model_changes() { + let events = run_sse(vec![ + json!({ + "type": "response.created", + "response": { + "id": "resp-1", + "model": "gpt-5.2-codex" + } + }), + json!({ + "type": "response.completed", + "response": { + "id": "resp-1", + "model": "gpt-5.3-codex" + } + }), + ]) + .await; + + assert_eq!(events.len(), 4); + assert_matches!( + &events[0], + ResponseEvent::ServerModel(model) if model == "gpt-5.2-codex" + ); + assert_matches!(&events[1], ResponseEvent::Created); + assert_matches!( + &events[2], + ResponseEvent::ServerModel(model) if model == "gpt-5.3-codex" + ); + assert_matches!( + &events[3], + ResponseEvent::Completed { + response_id, + token_usage: None, + can_append: false + } if response_id == "resp-1" + ); + } + #[test] fn test_try_parse_retry_after() { let err = Error { @@ -909,4 +1114,6 @@ mod tests { let delay = try_parse_retry_after(&err); assert_eq!(delay, Some(Duration::from_secs(35))); } + + const CYBER_RESTRICTED_MODEL_FOR_TESTS: &str = "gpt-5.3-codex"; } diff --git a/codex-rs/core/src/apps/render.rs b/codex-rs/core/src/apps/render.rs index f450eb3a3a..7f706f737e 100644 --- a/codex-rs/core/src/apps/render.rs +++ b/codex-rs/core/src/apps/render.rs @@ -2,6 +2,6 @@ use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; pub(crate) fn render_apps_section() -> String { format!( - "## Apps\nApps are mentioned in the prompt in the format `[$app-name](apps://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either already provided in `{CODEX_APPS_MCP_SERVER_NAME}`, or do not exist because the user did not install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned." + "## Apps\nApps are mentioned in the prompt in the format `[$app-name](app://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either already provided in `{CODEX_APPS_MCP_SERVER_NAME}`, or do not exist because the user did not install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned." ) } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6d4976ece5..a13072930f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -279,6 +279,8 @@ pub struct CodexSpawnOk { pub(crate) const INITIAL_SUBMIT_ID: &str = ""; pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 64; +const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber"; +const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety"; impl Codex { /// Spawn a new [`Codex`] and initialize the session. @@ -2004,7 +2006,6 @@ impl Session { &next.sandbox_policy, next.approval_policy, self.services.exec_policy.current().as_ref(), - self.features.enabled(Feature::RequestRule), &next.cwd, ) .into(), @@ -2561,6 +2562,35 @@ impl Session { self.record_conversation_items(ctx, &[item]).await; } + async fn maybe_warn_on_server_model_mismatch( + self: &Arc, + turn_context: &Arc, + server_model: String, + ) -> bool { + let requested_model = turn_context.model_info.slug.as_str(); + if server_model == requested_model { + info!("server reported model {server_model} (matches requested model)"); + return false; + } + + warn!("server reported model {server_model} while requested model was {requested_model}"); + + let warning_message = format!( + "Your account was flagged for potentially high-risk cyber activity and this request was routed to gpt-5.2 as a fallback. To regain access to gpt-5.3-codex, apply for trusted access: {CYBER_VERIFY_URL} or learn more: {CYBER_SAFETY_URL}" + ); + + self.send_event( + turn_context, + EventMsg::Warning(WarningEvent { + message: warning_message.clone(), + }), + ) + .await; + self.record_model_warning(warning_message, turn_context) + .await; + true + } + pub(crate) async fn replace_history(&self, items: Vec) { let mut state = self.state.lock().await; state.replace_history(items); @@ -2624,7 +2654,6 @@ impl Session { &turn_context.sandbox_policy, turn_context.approval_policy, self.services.exec_policy.current().as_ref(), - self.features.enabled(Feature::RequestRule), &turn_context.cwd, ) .into(), @@ -4437,6 +4466,7 @@ pub(crate) async fn run_turn( // Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains // many turns, from the perspective of the user, it is a single turn. let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); + let mut server_model_warning_emitted_for_turn = false; // `ModelClientSession` is turn-scoped and caches WebSocket + sticky routing state, so we reuse // one instance across retries within this turn. @@ -4499,6 +4529,7 @@ pub(crate) async fn run_turn( sampling_request_input, &explicitly_enabled_connectors, skills_outcome.as_ref(), + &mut server_model_warning_emitted_for_turn, cancellation_token.child_token(), ) .await @@ -4872,6 +4903,7 @@ async fn run_sampling_request( input: Vec, explicitly_enabled_connectors: &HashSet, skills_outcome: Option<&SkillLoadOutcome>, + server_model_warning_emitted_for_turn: &mut bool, cancellation_token: CancellationToken, ) -> CodexResult { let router = built_tools( @@ -4908,6 +4940,7 @@ async fn run_sampling_request( client_session, turn_metadata_header, Arc::clone(&turn_diff_tracker), + server_model_warning_emitted_for_turn, &prompt, cancellation_token.child_token(), ) @@ -4997,14 +5030,13 @@ async fn built_tools( skills_outcome: Option<&SkillLoadOutcome>, cancellation_token: &CancellationToken, ) -> CodexResult> { - let mut mcp_tools = sess - .services - .mcp_connection_manager - .read() - .await + let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; + let has_mcp_servers = mcp_connection_manager.has_servers(); + let mut mcp_tools = mcp_connection_manager .list_all_tools() .or_cancel(cancellation_token) .await?; + drop(mcp_connection_manager); let mut effective_explicitly_enabled_connectors = explicitly_enabled_connectors.clone(); effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await); @@ -5050,12 +5082,12 @@ async fn built_tools( Ok(Arc::new(ToolRouter::from_config( &turn_context.tools_config, - Some( + has_mcp_servers.then(|| { mcp_tools .into_iter() .map(|(name, tool)| (name, tool.tool)) - .collect(), - ), + .collect() + }), app_tools, turn_context.dynamic_tools.as_slice(), ))) @@ -5477,6 +5509,7 @@ async fn try_run_sampling_request( client_session: &mut ModelClientSession, turn_metadata_header: Option<&str>, turn_diff_tracker: SharedTurnDiffTracker, + server_model_warning_emitted_for_turn: &mut bool, prompt: &Prompt, cancellation_token: CancellationToken, ) -> CodexResult { @@ -5619,6 +5652,15 @@ async fn try_run_sampling_request( active_item = Some(turn_item); } } + ResponseEvent::ServerModel(server_model) => { + if !*server_model_warning_emitted_for_turn + && sess + .maybe_warn_on_server_model_mismatch(&turn_context, server_model) + .await + { + *server_model_warning_emitted_for_turn = true; + } + } ResponseEvent::ServerReasoningIncluded(included) => { sess.set_server_reasoning_included(included).await; } diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 298bc2679e..62ebdde43d 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -170,23 +170,26 @@ impl ExecPolicyManager { prefix_rule, } = req; let exec_policy = self.current(); - let (commands, used_heredoc_fallback) = commands_for_exec_policy(command); + let (commands, used_complex_parsing) = commands_for_exec_policy(command); // Keep heredoc prefix parsing for rule evaluation so existing // allow/prompt/forbidden rules still apply, but avoid auto-derived // amendments when only the heredoc fallback parser matched. - let auto_amendment_allowed = !used_heredoc_fallback; + let auto_amendment_allowed = !used_complex_parsing; let exec_policy_fallback = |cmd: &[String]| { render_decision_for_unmatched_command( approval_policy, sandbox_policy, cmd, sandbox_permissions, + used_complex_parsing, ) }; let evaluation = exec_policy.check_multiple(commands.iter(), &exec_policy_fallback); - let requested_amendment = - derive_requested_execpolicy_amendment(prefix_rule.as_ref(), &evaluation.matched_rules); + let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule( + prefix_rule.as_ref(), + &evaluation.matched_rules, + ); match evaluation.decision { Decision::Forbidden => ExecApprovalRequirement::Forbidden { @@ -406,8 +409,9 @@ pub fn render_decision_for_unmatched_command( sandbox_policy: &SandboxPolicy, command: &[String], sandbox_permissions: SandboxPermissions, + used_complex_parsing: bool, ) -> Decision { - if is_known_safe_command(command) { + if is_known_safe_command(command) && !used_complex_parsing { return Decision::Allow; } @@ -534,7 +538,7 @@ fn try_derive_execpolicy_amendment_for_allow_rules( }) } -fn derive_requested_execpolicy_amendment( +fn derive_requested_execpolicy_amendment_from_prefix_rule( prefix_rule: Option<&Vec>, matched_rules: &[RuleMatch], ) -> Option { @@ -552,10 +556,8 @@ fn derive_requested_execpolicy_amendment( return None; } - if matched_rules - .iter() - .any(|rule_match| is_policy_match(rule_match) && rule_match.decision() == Decision::Prompt) - { + // if any policy rule already matches, don't suggest an additional rule that might conflict or not apply + if matched_rules.iter().any(is_policy_match) { return None; } @@ -686,8 +688,6 @@ mod tests { use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; - use crate::features::Feature; - use crate::features::Features; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; @@ -1281,8 +1281,6 @@ prefix_rule( "cargo-insta".to_string(), ]; let manager = ExecPolicyManager::default(); - let mut features = Features::with_defaults(); - features.enable(Feature::RequestRule); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { @@ -1564,14 +1562,17 @@ prefix_rule( #[test] fn derive_requested_execpolicy_amendment_returns_none_for_missing_prefix_rule() { - assert_eq!(None, derive_requested_execpolicy_amendment(None, &[])); + assert_eq!( + None, + derive_requested_execpolicy_amendment_from_prefix_rule(None, &[]) + ); } #[test] fn derive_requested_execpolicy_amendment_returns_none_for_empty_prefix_rule() { assert_eq!( None, - derive_requested_execpolicy_amendment(Some(&Vec::new()), &[]) + derive_requested_execpolicy_amendment_from_prefix_rule(Some(&Vec::new()), &[]) ); } @@ -1579,7 +1580,7 @@ prefix_rule( fn derive_requested_execpolicy_amendment_returns_none_for_exact_banned_prefix_rule() { assert_eq!( None, - derive_requested_execpolicy_amendment( + derive_requested_execpolicy_amendment_from_prefix_rule( Some(&vec!["python".to_string(), "-c".to_string()]), &[], ) @@ -1598,7 +1599,7 @@ prefix_rule( ] { assert_eq!( None, - derive_requested_execpolicy_amendment(Some(&prefix_rule), &[]) + derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[]) ); } } @@ -1624,7 +1625,7 @@ prefix_rule( ] { assert_eq!( None, - derive_requested_execpolicy_amendment(Some(&prefix_rule), &[]) + derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[]) ); } } @@ -1639,22 +1640,52 @@ prefix_rule( assert_eq!( Some(ExecPolicyAmendment::new(prefix_rule.clone())), - derive_requested_execpolicy_amendment(Some(&prefix_rule), &[]) + derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[]) ); } #[test] - fn derive_requested_execpolicy_amendment_returns_none_when_policy_prompt_matches() { + fn derive_requested_execpolicy_amendment_returns_none_when_policy_matches() { let prefix_rule = vec!["cargo".to_string(), "build".to_string()]; - let matched_rules = vec![RuleMatch::PrefixRuleMatch { + + let matched_rules_prompt = vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["cargo".to_string()], decision: Decision::Prompt, justification: None, }]; - assert_eq!( None, - derive_requested_execpolicy_amendment(Some(&prefix_rule), &matched_rules) + derive_requested_execpolicy_amendment_from_prefix_rule( + Some(&prefix_rule), + &matched_rules_prompt + ), + "should return none when prompt policy matches" + ); + let matched_rules_allow = vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["cargo".to_string()], + decision: Decision::Allow, + justification: None, + }]; + assert_eq!( + None, + derive_requested_execpolicy_amendment_from_prefix_rule( + Some(&prefix_rule), + &matched_rules_allow + ), + "should return none when prompt policy matches" + ); + let matched_rules_forbidden = vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["cargo".to_string()], + decision: Decision::Forbidden, + justification: None, + }]; + assert_eq!( + None, + derive_requested_execpolicy_amendment_from_prefix_rule( + Some(&prefix_rule), + &matched_rules_forbidden + ), + "should return none when prompt policy matches" ); } diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 00e38223bf..3cab14d1fa 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -511,8 +511,8 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::RequestRule, key: "request_rule", - stage: Stage::Stable, - default_enabled: true, + stage: Stage::Removed, + default_enabled: false, }, FeatureSpec { id: Feature::WindowsSandbox, @@ -554,9 +554,9 @@ pub const FEATURES: &[FeatureSpec] = &[ id: Feature::Collab, key: "multi_agent", stage: Stage::Experimental { - name: "Sub-agents", + name: "Multi-agents", menu_description: "Ask Codex to spawn multiple agents to parallelize the work and win in efficiency.", - announcement: "NEW: Sub-agents can now be spawned by Codex. Enable in /experimental and restart Codex!", + announcement: "NEW: Multi-agents can now be spawned by Codex. Enable in /experimental and restart Codex!", }, default_enabled: false, }, diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 5a3c4d6e5c..589be7aac2 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -353,6 +353,10 @@ pub(crate) struct McpConnectionManager { } impl McpConnectionManager { + pub(crate) fn has_servers(&self) -> bool { + !self.clients.is_empty() + } + pub async fn initialize( &mut self, mcp_servers: &HashMap, diff --git a/codex-rs/core/src/models_manager/model_info.rs b/codex-rs/core/src/models_manager/model_info.rs index c5ad8899a1..19a945f951 100644 --- a/codex-rs/core/src/models_manager/model_info.rs +++ b/codex-rs/core/src/models_manager/model_info.rs @@ -20,8 +20,10 @@ const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective so const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}"; pub(crate) fn with_config_overrides(mut model: ModelInfo, config: &Config) -> ModelInfo { - if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries { - model.supports_reasoning_summaries = supports_reasoning_summaries; + if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries + && supports_reasoning_summaries + { + model.supports_reasoning_summaries = true; } if let Some(context_window) = config.model_context_window { model.context_window = Some(context_window); @@ -100,3 +102,46 @@ fn local_personality_messages_for_slug(slug: &str) -> Option { _ => None, } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::test_config; + use pretty_assertions::assert_eq; + + #[test] + fn reasoning_summaries_override_true_enables_support() { + let model = model_info_from_slug("unknown-model"); + let mut config = test_config(); + config.model_supports_reasoning_summaries = Some(true); + + let updated = with_config_overrides(model.clone(), &config); + let mut expected = model; + expected.supports_reasoning_summaries = true; + + assert_eq!(updated, expected); + } + + #[test] + fn reasoning_summaries_override_false_does_not_disable_support() { + let mut model = model_info_from_slug("unknown-model"); + model.supports_reasoning_summaries = true; + let mut config = test_config(); + config.model_supports_reasoning_summaries = Some(false); + + let updated = with_config_overrides(model.clone(), &config); + + assert_eq!(updated, model); + } + + #[test] + fn reasoning_summaries_override_false_is_noop_when_model_is_false() { + let model = model_info_from_slug("unknown-model"); + let mut config = test_config(); + config.model_supports_reasoning_summaries = Some(false); + + let updated = with_config_overrides(model.clone(), &config); + + assert_eq!(updated, model); + } +} diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 9f4a1a16ce..b0e1b3c0aa 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -1,11 +1,11 @@ pub mod apply_patch; -pub(crate) mod collab; mod dynamic; mod grep_files; mod js_repl; mod list_dir; mod mcp; mod mcp_resource; +pub(crate) mod multi_agents; mod plan; mod read_file; mod request_user_input; @@ -20,7 +20,6 @@ use serde::Deserialize; use crate::function_tool::FunctionCallError; pub use apply_patch::ApplyPatchHandler; -pub use collab::CollabHandler; pub use dynamic::DynamicToolHandler; pub use grep_files::GrepFilesHandler; pub use js_repl::JsReplHandler; @@ -28,6 +27,7 @@ pub use js_repl::JsReplResetHandler; pub use list_dir::ListDirHandler; pub use mcp::McpHandler; pub use mcp_resource::McpResourceHandler; +pub use multi_agents::MultiAgentHandler; pub use plan::PlanHandler; pub use read_file::ReadFileHandler; pub use request_user_input::RequestUserInputHandler; diff --git a/codex-rs/core/src/tools/handlers/collab.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs similarity index 97% rename from codex-rs/core/src/tools/handlers/collab.rs rename to codex-rs/core/src/tools/handlers/multi_agents.rs index 8235b291e8..c5a0f59ae3 100644 --- a/codex-rs/core/src/tools/handlers/collab.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -34,7 +34,7 @@ use codex_protocol::user_input::UserInput; use serde::Deserialize; use serde::Serialize; -pub struct CollabHandler; +pub struct MultiAgentHandler; /// Minimum wait timeout to prevent tight polling loops from burning CPU. pub(crate) const MIN_WAIT_TIMEOUT_MS: i64 = 10_000; @@ -47,7 +47,7 @@ struct CloseAgentArgs { } #[async_trait] -impl ToolHandler for CollabHandler { +impl ToolHandler for MultiAgentHandler { fn kind(&self) -> ToolKind { ToolKind::Function } @@ -917,7 +917,7 @@ mod tests { input: "hello".to_string(), }, ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("payload should be rejected"); }; assert_eq!( @@ -937,7 +937,7 @@ mod tests { "unknown_tool", function_payload(json!({})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("tool should be rejected"); }; assert_eq!( @@ -955,7 +955,7 @@ mod tests { "spawn_agent", function_payload(json!({"message": " "})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("empty message should be rejected"); }; assert_eq!( @@ -978,7 +978,7 @@ mod tests { "items": [{"type": "mention", "name": "drive", "path": "app://drive"}] })), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("message+items should be rejected"); }; assert_eq!( @@ -1016,7 +1016,7 @@ mod tests { "agent_type": "explorer" })), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(invocation) .await .expect("spawn_agent should succeed"); @@ -1048,7 +1048,7 @@ mod tests { "spawn_agent", function_payload(json!({"message": "hello"})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("spawn should fail without a manager"); }; assert_eq!( @@ -1074,7 +1074,7 @@ mod tests { "spawn_agent", function_payload(json!({"message": "hello"})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("spawn should fail when depth limit exceeded"); }; assert_eq!( @@ -1094,7 +1094,7 @@ mod tests { "send_input", function_payload(json!({"id": ThreadId::new().to_string(), "message": ""})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("empty message should be rejected"); }; assert_eq!( @@ -1118,7 +1118,7 @@ mod tests { "items": [{"type": "mention", "name": "drive", "path": "app://drive"}] })), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("message+items should be rejected"); }; assert_eq!( @@ -1138,7 +1138,7 @@ mod tests { "send_input", function_payload(json!({"id": "not-a-uuid", "message": "hi"})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("invalid id should be rejected"); }; let FunctionCallError::RespondToModel(msg) = err else { @@ -1159,7 +1159,7 @@ mod tests { "send_input", function_payload(json!({"id": agent_id.to_string(), "message": "hi"})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("missing agent should be reported"); }; assert_eq!( @@ -1186,7 +1186,7 @@ mod tests { "interrupt": true })), ); - CollabHandler + MultiAgentHandler .handle(invocation) .await .expect("send_input should succeed"); @@ -1227,7 +1227,7 @@ mod tests { ] })), ); - CollabHandler + MultiAgentHandler .handle(invocation) .await .expect("send_input should succeed"); @@ -1267,7 +1267,7 @@ mod tests { "resume_agent", function_payload(json!({"id": "not-a-uuid"})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("invalid id should be rejected"); }; let FunctionCallError::RespondToModel(msg) = err else { @@ -1288,7 +1288,7 @@ mod tests { "resume_agent", function_payload(json!({"id": agent_id.to_string()})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("missing agent should be reported"); }; assert_eq!( @@ -1313,7 +1313,7 @@ mod tests { function_payload(json!({"id": agent_id.to_string()})), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(invocation) .await .expect("resume_agent should succeed"); @@ -1382,7 +1382,7 @@ mod tests { "resume_agent", function_payload(json!({"id": agent_id.to_string()})), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(resume_invocation) .await .expect("resume_agent should succeed"); @@ -1405,7 +1405,7 @@ mod tests { "send_input", function_payload(json!({"id": agent_id.to_string(), "message": "hello"})), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(send_invocation) .await .expect("send_input should succeed after resume"); @@ -1450,7 +1450,7 @@ mod tests { "resume_agent", function_payload(json!({"id": ThreadId::new().to_string()})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("resume should fail when depth limit exceeded"); }; assert_eq!( @@ -1479,7 +1479,7 @@ mod tests { "timeout_ms": 0 })), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("non-positive timeout should be rejected"); }; assert_eq!( @@ -1497,7 +1497,7 @@ mod tests { "wait", function_payload(json!({"ids": ["invalid"]})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("invalid id should be rejected"); }; let FunctionCallError::RespondToModel(msg) = err else { @@ -1515,7 +1515,7 @@ mod tests { "wait", function_payload(json!({"ids": []})), ); - let Err(err) = CollabHandler.handle(invocation).await else { + let Err(err) = MultiAgentHandler.handle(invocation).await else { panic!("empty ids should be rejected"); }; assert_eq!( @@ -1540,7 +1540,7 @@ mod tests { "timeout_ms": 1000 })), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(invocation) .await .expect("wait should succeed"); @@ -1584,7 +1584,7 @@ mod tests { "timeout_ms": MIN_WAIT_TIMEOUT_MS })), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(invocation) .await .expect("wait should succeed"); @@ -1632,7 +1632,11 @@ mod tests { })), ); - let early = timeout(Duration::from_millis(50), CollabHandler.handle(invocation)).await; + let early = timeout( + Duration::from_millis(50), + MultiAgentHandler.handle(invocation), + ) + .await; assert!( early.is_err(), "wait should not return before the minimum timeout clamp" @@ -1677,7 +1681,7 @@ mod tests { "timeout_ms": 1000 })), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(invocation) .await .expect("wait should succeed"); @@ -1717,7 +1721,7 @@ mod tests { "close_agent", function_payload(json!({"id": agent_id.to_string()})), ); - let output = CollabHandler + let output = MultiAgentHandler .handle(invocation) .await .expect("close_agent should succeed"); diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 9093dfba4c..31bf3f1294 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -243,14 +243,6 @@ impl ShellHandler { freeform, } = args; - let features = session.features(); - let request_rule_enabled = features.enabled(crate::features::Feature::RequestRule); - let prefix_rule = if request_rule_enabled { - prefix_rule - } else { - None - }; - let mut exec_params = exec_params; let dependency_env = session.dependency_env().await; if !dependency_env.is_empty() { diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index c06889b37b..e689e5bba9 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -142,14 +142,6 @@ impl ToolHandler for UnifiedExecHandler { .. } = args; - let features = session.features(); - let request_rule_enabled = features.enabled(crate::features::Feature::RequestRule); - let prefix_rule = if request_rule_enabled { - prefix_rule - } else { - None - }; - if sandbox_permissions.requires_escalated_permissions() && !matches!( context.turn.approval_policy, diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 23efb0c67e..ef0cdad004 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -114,6 +114,7 @@ struct ExecContext { struct ExecToolCalls { in_flight: usize, notify: Arc, + cancel: CancellationToken, } enum KernelStreamEnd { @@ -300,6 +301,7 @@ impl JsReplManager { async fn clear_exec_tool_calls(&self, exec_id: &str) { if let Some(state) = self.exec_tool_calls.lock().await.remove(exec_id) { + state.cancel.cancel(); state.notify.notify_waiters(); } } @@ -320,32 +322,14 @@ impl JsReplManager { } } - async fn wait_for_all_exec_tool_calls(&self) { - loop { - let notified = { - let calls = self.exec_tool_calls.lock().await; - calls - .values() - .find(|state| state.in_flight > 0) - .map(|state| Arc::clone(&state.notify).notified_owned()) - }; - match notified { - Some(notified) => notified.await, - None => return, - } - } - } - async fn begin_exec_tool_call( exec_tool_calls: &Arc>>, exec_id: &str, - ) -> bool { + ) -> Option { let mut calls = exec_tool_calls.lock().await; - let Some(state) = calls.get_mut(exec_id) else { - return false; - }; + let state = calls.get_mut(exec_id)?; state.in_flight += 1; - true + Some(state.cancel.clone()) } async fn finish_exec_tool_call( @@ -396,14 +380,30 @@ impl JsReplManager { exec_id: &str, ) { if let Some(state) = exec_tool_calls.lock().await.remove(exec_id) { + state.cancel.cancel(); + state.notify.notify_waiters(); + } + } + + async fn clear_all_exec_tool_calls_map( + exec_tool_calls: &Arc>>, + ) { + let states = { + let mut calls = exec_tool_calls.lock().await; + calls.drain().map(|(_, state)| state).collect::>() + }; + for state in states { + state.cancel.cancel(); state.notify.notify_waiters(); } } pub async fn reset(&self) -> Result<(), FunctionCallError> { + let _permit = self.exec_lock.clone().acquire_owned().await.map_err(|_| { + FunctionCallError::RespondToModel("js_repl execution unavailable".to_string()) + })?; self.reset_kernel().await; - self.wait_for_all_exec_tool_calls().await; - self.exec_tool_calls.lock().await.clear(); + Self::clear_all_exec_tool_calls_map(&self.exec_tool_calls).await; Ok(()) } @@ -530,7 +530,9 @@ impl JsReplManager { return Err(FunctionCallError::RespondToModel(message)); } Err(_) => { - self.reset().await?; + self.reset_kernel().await; + self.wait_for_exec_tool_calls(&req_id).await; + self.exec_tool_calls.lock().await.clear(); return Err(FunctionCallError::RespondToModel( "js_repl execution timed out; kernel reset, rerun your request".to_string(), )); @@ -854,7 +856,9 @@ impl JsReplManager { JsReplManager::clear_exec_tool_calls_map(&exec_tool_calls, &id).await; } KernelToHost::RunTool(req) => { - if !JsReplManager::begin_exec_tool_call(&exec_tool_calls, &req.exec_id).await { + let Some(reset_cancel) = + JsReplManager::begin_exec_tool_call(&exec_tool_calls, &req.exec_id).await + else { let exec_id = req.exec_id.clone(); let tool_call_id = req.id.clone(); let payload = HostToKernel::RunToolResult(RunToolResult { @@ -877,10 +881,10 @@ impl JsReplManager { ); } continue; - } + }; let stdin_clone = Arc::clone(&stdin); let exec_contexts = Arc::clone(&exec_contexts); - let exec_tool_calls = Arc::clone(&exec_tool_calls); + let exec_tool_calls_for_task = Arc::clone(&exec_tool_calls); let recent_stderr = Arc::clone(&recent_stderr); tokio::spawn(async move { let exec_id = req.exec_id.clone(); @@ -888,15 +892,26 @@ impl JsReplManager { let tool_name = req.tool_name.clone(); let context = { exec_contexts.lock().await.get(&exec_id).cloned() }; let result = match context { - Some(ctx) => JsReplManager::run_tool_request(ctx, req).await, + Some(ctx) => { + tokio::select! { + _ = reset_cancel.cancelled() => RunToolResult { + id: tool_call_id.clone(), + ok: false, + response: None, + error: Some("js_repl execution reset".to_string()), + }, + result = JsReplManager::run_tool_request(ctx, req) => result, + } + } None => RunToolResult { - id: req.id.clone(), + id: tool_call_id.clone(), ok: false, response: None, error: Some("js_repl exec context not found".to_string()), }, }; - JsReplManager::finish_exec_tool_call(&exec_tool_calls, &exec_id).await; + JsReplManager::finish_exec_tool_call(&exec_tool_calls_for_task, &exec_id) + .await; let payload = HostToKernel::RunToolResult(result); if let Err(err) = JsReplManager::write_message(&stdin_clone, &payload).await { @@ -1417,7 +1432,11 @@ mod tests { .lock() .await .insert(exec_id.clone(), ExecToolCalls::default()); - assert!(JsReplManager::begin_exec_tool_call(&exec_tool_calls, &exec_id).await); + assert!( + JsReplManager::begin_exec_tool_call(&exec_tool_calls, &exec_id) + .await + .is_some() + ); let wait_map = Arc::clone(&exec_tool_calls); let wait_exec_id = exec_id.clone(); @@ -1441,6 +1460,110 @@ mod tests { JsReplManager::clear_exec_tool_calls_map(&exec_tool_calls, &exec_id).await; } } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn reset_waits_for_exec_lock_before_clearing_exec_tool_calls() { + let manager = JsReplManager::new(None, PathBuf::from(".")) + .await + .expect("manager should initialize"); + let permit = manager + .exec_lock + .clone() + .acquire_owned() + .await + .expect("lock should be acquirable"); + let exec_id = Uuid::new_v4().to_string(); + manager.register_exec_tool_calls(&exec_id).await; + + let reset_manager = Arc::clone(&manager); + let mut reset_task = tokio::spawn(async move { reset_manager.reset().await }); + tokio::time::sleep(Duration::from_millis(50)).await; + + assert!( + !reset_task.is_finished(), + "reset should wait until execute lock is released" + ); + assert!( + manager.exec_tool_calls.lock().await.contains_key(&exec_id), + "reset must not clear tool-call contexts while execute lock is held" + ); + + drop(permit); + + tokio::time::timeout(Duration::from_secs(1), &mut reset_task) + .await + .expect("reset should complete after execute lock release") + .expect("reset task should not panic") + .expect("reset should succeed"); + assert!( + !manager.exec_tool_calls.lock().await.contains_key(&exec_id), + "reset should clear tool-call contexts after lock acquisition" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn reset_clears_inflight_exec_tool_calls_without_waiting() { + let manager = JsReplManager::new(None, std::env::temp_dir()) + .await + .expect("manager should initialize"); + let exec_id = Uuid::new_v4().to_string(); + manager.register_exec_tool_calls(&exec_id).await; + assert!( + JsReplManager::begin_exec_tool_call(&manager.exec_tool_calls, &exec_id) + .await + .is_some() + ); + + let wait_manager = Arc::clone(&manager); + let wait_exec_id = exec_id.clone(); + let waiter = tokio::spawn(async move { + wait_manager.wait_for_exec_tool_calls(&wait_exec_id).await; + }); + tokio::task::yield_now().await; + + tokio::time::timeout(Duration::from_secs(1), manager.reset()) + .await + .expect("reset should not hang") + .expect("reset should succeed"); + + tokio::time::timeout(Duration::from_secs(1), waiter) + .await + .expect("waiter should be released") + .expect("wait task should not panic"); + + assert!(manager.exec_tool_calls.lock().await.is_empty()); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn reset_aborts_inflight_exec_tool_tasks() { + let manager = JsReplManager::new(None, std::env::temp_dir()) + .await + .expect("manager should initialize"); + let exec_id = Uuid::new_v4().to_string(); + manager.register_exec_tool_calls(&exec_id).await; + let reset_cancel = JsReplManager::begin_exec_tool_call(&manager.exec_tool_calls, &exec_id) + .await + .expect("exec should be registered"); + + let task = tokio::spawn(async move { + tokio::select! { + _ = reset_cancel.cancelled() => "cancelled", + _ = tokio::time::sleep(Duration::from_secs(60)) => "timed_out", + } + }); + + tokio::time::timeout(Duration::from_secs(1), manager.reset()) + .await + .expect("reset should not hang") + .expect("reset should succeed"); + + let outcome = tokio::time::timeout(Duration::from_secs(1), task) + .await + .expect("cancelled task should resolve promptly") + .expect("task should not panic"); + assert_eq!(outcome, "cancelled"); + } + async fn can_run_js_repl_runtime_tests() -> bool { if std::env::var_os("CODEX_SANDBOX").is_some() { return false; diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 817b7d7877..a897e5a8f6 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -10,9 +10,9 @@ use crate::tools::handlers::SEARCH_TOOL_BM25_DEFAULT_LIMIT; use crate::tools::handlers::SEARCH_TOOL_BM25_TOOL_NAME; use crate::tools::handlers::apply_patch::create_apply_patch_freeform_tool; use crate::tools::handlers::apply_patch::create_apply_patch_json_tool; -use crate::tools::handlers::collab::DEFAULT_WAIT_TIMEOUT_MS; -use crate::tools::handlers::collab::MAX_WAIT_TIMEOUT_MS; -use crate::tools::handlers::collab::MIN_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents::DEFAULT_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents::MAX_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents::MIN_WAIT_TIMEOUT_MS; use crate::tools::handlers::request_user_input_tool_description; use crate::tools::registry::ToolRegistryBuilder; use codex_protocol::config_types::WebSearchMode; @@ -41,7 +41,6 @@ pub(crate) struct ToolsConfig { pub js_repl_tools_only: bool, pub collab_tools: bool, pub collaboration_modes_tools: bool, - pub request_rule_enabled: bool, pub experimental_supported_tools: Vec, } @@ -64,7 +63,6 @@ impl ToolsConfig { include_js_repl && features.enabled(Feature::JsReplToolsOnly); let include_collab_tools = features.enabled(Feature::Collab); let include_collaboration_modes_tools = features.enabled(Feature::CollaborationModes); - let request_rule_enabled = features.enabled(Feature::RequestRule); let include_search_tool = features.enabled(Feature::Apps); let shell_type = if !features.enabled(Feature::ShellTool) { @@ -101,7 +99,6 @@ impl ToolsConfig { js_repl_tools_only: include_js_repl_tools_only, collab_tools: include_collab_tools, collaboration_modes_tools: include_collaboration_modes_tools, - request_rule_enabled, experimental_supported_tools: model_info.experimental_supported_tools.clone(), } } @@ -174,7 +171,7 @@ impl From for AdditionalProperties { } } -fn create_approval_parameters(include_prefix_rule: bool) -> BTreeMap { +fn create_approval_parameters() -> BTreeMap { let mut properties = BTreeMap::from([ ( "sandbox_permissions".to_string(), @@ -200,23 +197,22 @@ fn create_approval_parameters(include_prefix_rule: bool) -> BTreeMap ToolSpec { +fn create_exec_command_tool() -> ToolSpec { let mut properties = BTreeMap::from([ ( "cmd".to_string(), @@ -274,7 +270,7 @@ fn create_exec_command_tool(include_prefix_rule: bool) -> ToolSpec { }, ), ]); - properties.extend(create_approval_parameters(include_prefix_rule)); + properties.extend(create_approval_parameters()); ToolSpec::Function(ResponsesApiTool { name: "exec_command".to_string(), @@ -337,7 +333,7 @@ fn create_write_stdin_tool() -> ToolSpec { }) } -fn create_shell_tool(include_prefix_rule: bool) -> ToolSpec { +fn create_shell_tool() -> ToolSpec { let mut properties = BTreeMap::from([ ( "command".to_string(), @@ -359,7 +355,7 @@ fn create_shell_tool(include_prefix_rule: bool) -> ToolSpec { }, ), ]); - properties.extend(create_approval_parameters(include_prefix_rule)); + properties.extend(create_approval_parameters()); let description = if cfg!(windows) { r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"]. @@ -390,7 +386,7 @@ Examples of valid command strings: }) } -fn create_shell_command_tool(include_prefix_rule: bool) -> ToolSpec { +fn create_shell_command_tool() -> ToolSpec { let mut properties = BTreeMap::from([ ( "command".to_string(), @@ -422,7 +418,7 @@ fn create_shell_command_tool(include_prefix_rule: bool) -> ToolSpec { }, ), ]); - properties.extend(create_approval_parameters(include_prefix_rule)); + properties.extend(create_approval_parameters()); let description = if cfg!(windows) { r#"Runs a Powershell command (Windows) and returns its output. @@ -1405,7 +1401,6 @@ pub(crate) fn build_specs( dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { use crate::tools::handlers::ApplyPatchHandler; - use crate::tools::handlers::CollabHandler; use crate::tools::handlers::DynamicToolHandler; use crate::tools::handlers::GrepFilesHandler; use crate::tools::handlers::JsReplHandler; @@ -1413,6 +1408,7 @@ pub(crate) fn build_specs( use crate::tools::handlers::ListDirHandler; use crate::tools::handlers::McpHandler; use crate::tools::handlers::McpResourceHandler; + use crate::tools::handlers::MultiAgentHandler; use crate::tools::handlers::PlanHandler; use crate::tools::handlers::ReadFileHandler; use crate::tools::handlers::RequestUserInputHandler; @@ -1442,19 +1438,13 @@ pub(crate) fn build_specs( match &config.shell_type { ConfigShellToolType::Default => { - builder.push_spec_with_parallel_support( - create_shell_tool(config.request_rule_enabled), - true, - ); + builder.push_spec_with_parallel_support(create_shell_tool(), true); } ConfigShellToolType::Local => { builder.push_spec_with_parallel_support(ToolSpec::LocalShell {}, true); } ConfigShellToolType::UnifiedExec => { - builder.push_spec_with_parallel_support( - create_exec_command_tool(config.request_rule_enabled), - true, - ); + builder.push_spec_with_parallel_support(create_exec_command_tool(), true); builder.push_spec(create_write_stdin_tool()); builder.register_handler("exec_command", unified_exec_handler.clone()); builder.register_handler("write_stdin", unified_exec_handler); @@ -1463,10 +1453,7 @@ pub(crate) fn build_specs( // Do nothing. } ConfigShellToolType::ShellCommand => { - builder.push_spec_with_parallel_support( - create_shell_command_tool(config.request_rule_enabled), - true, - ); + builder.push_spec_with_parallel_support(create_shell_command_tool(), true); } } @@ -1478,12 +1465,14 @@ pub(crate) fn build_specs( builder.register_handler("shell_command", shell_command_handler); } - builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true); - builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true); - builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true); - builder.register_handler("list_mcp_resources", mcp_resource_handler.clone()); - builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone()); - builder.register_handler("read_mcp_resource", mcp_resource_handler); + if mcp_tools.is_some() { + builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true); + builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true); + builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true); + builder.register_handler("list_mcp_resources", mcp_resource_handler.clone()); + builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone()); + builder.register_handler("read_mcp_resource", mcp_resource_handler); + } builder.push_spec(PLAN_TOOL.clone()); builder.register_handler("update_plan", plan_handler); @@ -1574,17 +1563,17 @@ pub(crate) fn build_specs( builder.register_handler("view_image", view_image_handler); if config.collab_tools { - let collab_handler = Arc::new(CollabHandler); + let multi_agent_handler = Arc::new(MultiAgentHandler); builder.push_spec(create_spawn_agent_tool()); builder.push_spec(create_send_input_tool()); builder.push_spec(create_resume_agent_tool()); builder.push_spec(create_wait_tool()); builder.push_spec(create_close_agent_tool()); - builder.register_handler("spawn_agent", collab_handler.clone()); - builder.register_handler("send_input", collab_handler.clone()); - builder.register_handler("resume_agent", collab_handler.clone()); - builder.register_handler("wait", collab_handler.clone()); - builder.register_handler("close_agent", collab_handler); + builder.register_handler("spawn_agent", multi_agent_handler.clone()); + builder.register_handler("send_input", multi_agent_handler.clone()); + builder.register_handler("resume_agent", multi_agent_handler.clone()); + builder.register_handler("wait", multi_agent_handler.clone()); + builder.register_handler("close_agent", multi_agent_handler); } if let Some(mcp_tools) = mcp_tools { @@ -1830,11 +1819,8 @@ mod tests { // Build expected from the same helpers used by the builder. let mut expected: BTreeMap = BTreeMap::from([]); for spec in [ - create_exec_command_tool(true), + create_exec_command_tool(), create_write_stdin_tool(), - create_list_mcp_resources_tool(), - create_list_mcp_resource_templates_tool(), - create_read_mcp_resource_tool(), PLAN_TOOL.clone(), create_request_user_input_tool(), create_apply_patch_freeform_tool(), @@ -2039,7 +2025,7 @@ mod tests { features, web_search_mode, }); - let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build(); + let (tools, _) = build_specs(&tools_config, None, None, &[]).build(); let tool_names = tools.iter().map(|t| t.spec.name()).collect::>(); assert_eq!(&tool_names, &expected_tools,); } @@ -2106,6 +2092,53 @@ mod tests { ); } + #[test] + fn mcp_resource_tools_are_hidden_without_mcp_servers() { + let config = test_config(); + let model_info = + ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::CollaborationModes); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + }); + let (tools, _) = build_specs(&tools_config, None, None, &[]).build(); + + assert!( + !tools.iter().any(|tool| matches!( + tool.spec.name(), + "list_mcp_resources" | "list_mcp_resource_templates" | "read_mcp_resource" + )), + "MCP resource tools should be omitted when no MCP servers are configured" + ); + } + + #[test] + fn mcp_resource_tools_are_included_when_mcp_servers_are_present() { + let config = test_config(); + let model_info = + ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::CollaborationModes); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + }); + let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build(); + + assert_contains_tool_names( + &tools, + &[ + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + ], + ); + } + #[test] fn test_build_specs_gpt5_codex_default() { let mut features = Features::with_defaults(); @@ -2116,9 +2149,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2138,9 +2168,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2162,9 +2189,6 @@ mod tests { &[ "exec_command", "write_stdin", - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2186,9 +2210,6 @@ mod tests { &[ "exec_command", "write_stdin", - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2208,9 +2229,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2230,9 +2248,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2252,9 +2267,6 @@ mod tests { Some(WebSearchMode::Cached), "shell", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "web_search", @@ -2273,9 +2285,6 @@ mod tests { Some(WebSearchMode::Cached), "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2297,9 +2306,6 @@ mod tests { &[ "exec_command", "write_stdin", - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -2796,7 +2802,7 @@ mod tests { #[test] fn test_shell_tool() { - let tool = super::create_shell_tool(true); + let tool = super::create_shell_tool(); let ToolSpec::Function(ResponsesApiTool { description, name, .. }) = &tool @@ -2826,7 +2832,7 @@ Examples of valid command strings: #[test] fn test_shell_command_tool() { - let tool = super::create_shell_command_tool(true); + let tool = super::create_shell_command_tool(); let ToolSpec::Function(ResponsesApiTool { description, name, .. }) = &tool diff --git a/codex-rs/core/templates/search_tool/tool_description.md b/codex-rs/core/templates/search_tool/tool_description.md index 0c84a6f52e..05667b7356 100644 --- a/codex-rs/core/templates/search_tool/tool_description.md +++ b/codex-rs/core/templates/search_tool/tool_description.md @@ -24,5 +24,5 @@ Notes: - `description` - `connector_name` - input schema property keys (`input_keys`) -- If the needed app is already explicit in the prompt (for example an `apps://...` mention) or already present in the current `tools` list, you can call that tool directly. +- If the needed app is already explicit in the prompt (for example `[$app-name](app://{connector_id})`) or already present in the current `tools` list, you can call that tool directly. - Do not use `search_tool_bm25` for non-apps/local tasks (filesystem, repo search, or shell-only workflows) or anything not related to {{app_names}}. diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 72de76bab8..09d6755d3d 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -1479,6 +1479,44 @@ fn scenarios() -> Vec { output_contains: "rejected by user", }, }, + ScenarioSpec { + name: "safe command with heredoc and redirect still requires approval", + approval_policy: AskForApproval::OnRequest, + sandbox_policy: workspace_write(false), + action: ActionKind::RunUnifiedExecCommand { + command: "cat <<'EOF' > /tmp/out.txt \nhello\nEOF", + justification: None, + }, + sandbox_permissions: SandboxPermissions::RequireEscalated, + features: vec![Feature::UnifiedExec], + model_override: None, + outcome: Outcome::ExecApproval { + decision: ReviewDecision::Denied, + expected_reason: None, + }, + expectation: Expectation::CommandFailure { + output_contains: "rejected by user", + }, + }, + ScenarioSpec { + name: "compound command with one safe command still requires approval", + approval_policy: AskForApproval::OnRequest, + sandbox_policy: workspace_write(false), + action: ActionKind::RunUnifiedExecCommand { + command: "cat ./one.txt && touch ./two.txt", + justification: None, + }, + sandbox_permissions: SandboxPermissions::RequireEscalated, + features: vec![Feature::UnifiedExec], + model_override: None, + outcome: Outcome::ExecApproval { + decision: ReviewDecision::Denied, + expected_reason: None, + }, + expectation: Expectation::CommandFailure { + output_contains: "rejected by user", + }, + }, ] } @@ -1890,14 +1928,15 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts Ok(()) } +// todo(dylan) add ScenarioSpec support for rules #[tokio::test(flavor = "current_thread")] #[cfg(unix)] -async fn heredoc_with_chained_allowed_prefix_still_requires_approval() -> Result<()> { +async fn compound_command_with_one_safe_command_still_requires_approval() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; let approval_policy = AskForApproval::UnlessTrusted; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let sandbox_policy = SandboxPolicy::new_workspace_write_policy(); let sandbox_policy_for_config = sandbox_policy.clone(); let mut builder = test_codex().with_config(move |config| { config.permissions.approval_policy = Constrained::allow_any(approval_policy); @@ -1913,12 +1952,12 @@ async fn heredoc_with_chained_allowed_prefix_still_requires_approval() -> Result )?; let call_id = "heredoc-with-chained-prefix"; - let command = "cat <<'EOF' > /tmp/test.txt && touch allow-prefix.txt\nhello\nEOF"; + let command = "touch ./test.txt && rm ./test.txt"; let (event, expected_command) = ActionKind::RunCommand { command } .prepare(&test, &server, call_id, SandboxPermissions::UseDefault) .await?; let expected_command = - expected_command.expect("heredoc chained command scenario should produce a shell command"); + expected_command.expect("compound command should produce a shell command"); let _ = mount_sse_once( &server, @@ -1940,7 +1979,7 @@ async fn heredoc_with_chained_allowed_prefix_still_requires_approval() -> Result submit_turn( &test, - "heredoc chained prefix", + "compound command", approval_policy, sandbox_policy.clone(), ) diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 113cf9f674..03eec5bde2 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -103,6 +103,7 @@ mod resume_warning; mod review; mod rmcp_client; mod rollout_list_find; +mod safety_check_downgrade; mod search_tool; mod seatbelt; mod shell_command; diff --git a/codex-rs/core/tests/suite/model_tools.rs b/codex-rs/core/tests/suite/model_tools.rs index 676b34eb70..836807881b 100644 --- a/codex-rs/core/tests/suite/model_tools.rs +++ b/codex-rs/core/tests/suite/model_tools.rs @@ -75,9 +75,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -94,9 +91,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -113,9 +107,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", @@ -132,9 +123,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "web_search", @@ -150,9 +138,6 @@ async fn model_selects_expected_tools() { expected_default_tools( "shell_command", &[ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index d272036067..5db3bf9f0a 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -488,7 +488,6 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { &sandbox_policy, AskForApproval::OnRequest, &Policy::empty(), - true, test.config.cwd.as_path(), ) .into_text(); diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 97fe9b4161..71a565540b 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -144,9 +144,6 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> { vec!["exec_command", "write_stdin"] }; expected_tools_names.extend([ - "list_mcp_resources", - "list_mcp_resource_templates", - "read_mcp_resource", "update_plan", "request_user_input", "apply_patch", diff --git a/codex-rs/core/tests/suite/safety_check_downgrade.rs b/codex-rs/core/tests/suite/safety_check_downgrade.rs new file mode 100644 index 0000000000..a22fead90f --- /dev/null +++ b/codex-rs/core/tests/suite/safety_check_downgrade.rs @@ -0,0 +1,228 @@ +use anyhow::Result; +use codex_core::protocol::AskForApproval; +use codex_core::protocol::EventMsg; +use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::user_input::UserInput; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_response_once; +use core_test_support::responses::mount_response_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::sse_completed; +use core_test_support::responses::sse_response; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; + +const SERVER_MODEL: &str = "gpt-5.2"; +const REQUESTED_MODEL: &str = "gpt-5.3-codex"; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn openai_model_header_mismatch_emits_warning_event_and_warning_item() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response = + sse_response(sse_completed("resp-1")).insert_header("OpenAI-Model", SERVER_MODEL); + let _mock = mount_response_once(&server, response).await; + + let mut builder = test_codex().with_model(REQUESTED_MODEL); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "trigger safety check".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: REQUESTED_MODEL.to_string(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + let warning = wait_for_event(&test.codex, |event| matches!(event, EventMsg::Warning(_))).await; + let EventMsg::Warning(warning) = warning else { + panic!("expected warning event"); + }; + assert!(warning.message.contains(REQUESTED_MODEL)); + assert!(warning.message.contains(SERVER_MODEL)); + + let warning_item = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::RawResponseItem(raw) + if matches!( + &raw.item, + ResponseItem::Message { content, .. } + if content.iter().any(|item| matches!( + item, + ContentItem::InputText { text } if text.starts_with("Warning: ") + )) + ) + ) + }) + .await; + let EventMsg::RawResponseItem(raw) = warning_item else { + panic!("expected raw response item event"); + }; + let ResponseItem::Message { role, content, .. } = raw.item else { + panic!("expected warning to be recorded as a message item"); + }; + assert_eq!(role, "user"); + let warning_text = content.iter().find_map(|item| match item { + ContentItem::InputText { text } => Some(text.as_str()), + _ => None, + }); + let warning_text = warning_text.expect("warning message should include input_text content"); + assert!(warning_text.contains(REQUESTED_MODEL)); + assert!(warning_text.contains(SERVER_MODEL)); + + let _ = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn response_model_field_mismatch_emits_warning_when_header_matches_requested() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let response = sse_response(sse(vec![ + serde_json::json!({ + "type": "response.created", + "response": { + "id": "resp-1", + "model": SERVER_MODEL, + } + }), + core_test_support::responses::ev_completed("resp-1"), + ])) + .insert_header("OpenAI-Model", REQUESTED_MODEL); + let _mock = mount_response_once(&server, response).await; + + let mut builder = test_codex().with_model(REQUESTED_MODEL); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "trigger response model check".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: REQUESTED_MODEL.to_string(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + let warning = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::Warning(warning) + if warning + .message + .contains("flagged for potentially high-risk cyber activity") + ) + }) + .await; + let EventMsg::Warning(warning) = warning else { + panic!("expected warning event"); + }; + assert!(warning.message.contains("gpt-5.2 as a fallback")); + + let _ = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn openai_model_header_mismatch_only_emits_one_warning_per_turn() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let tool_args = serde_json::json!({ + "command": "echo hello", + "timeout_ms": 1_000 + }); + + let first_response = sse_response(sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + "call-1", + "shell_command", + &serde_json::to_string(&tool_args)?, + ), + core_test_support::responses::ev_completed("resp-1"), + ])) + .insert_header("OpenAI-Model", SERVER_MODEL); + let second_response = sse_response(sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + core_test_support::responses::ev_completed("resp-2"), + ])) + .insert_header("OpenAI-Model", SERVER_MODEL); + let _mock = mount_response_sequence(&server, vec![first_response, second_response]).await; + + let mut builder = test_codex().with_model(REQUESTED_MODEL); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "trigger follow-up turn".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: REQUESTED_MODEL.to_string(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + let mut warning_count = 0; + loop { + let event = wait_for_event(&test.codex, |_| true).await; + match event { + EventMsg::Warning(warning) if warning.message.contains(REQUESTED_MODEL) => { + warning_count += 1; + } + EventMsg::TurnComplete(_) => break, + _ => {} + } + } + + assert_eq!(warning_count, 1); + + Ok(()) +} diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index 4a227fe093..adda7173a1 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -279,7 +279,7 @@ impl CodexLogSnapshot { } let level = match classification { - "bug" | "bad_result" => Level::Error, + "bug" | "bad_result" | "safety_check" => Level::Error, _ => Level::Info, }; @@ -342,6 +342,7 @@ fn display_classification(classification: &str) -> String { "bug" => "Bug".to_string(), "bad_result" => "Bad result".to_string(), "good_result" => "Good result".to_string(), + "safety_check" => "Safety check".to_string(), _ => "Other".to_string(), } } diff --git a/codex-rs/otel/src/traces/otel_manager.rs b/codex-rs/otel/src/traces/otel_manager.rs index f50eb5dfac..e3ba85eab1 100644 --- a/codex-rs/otel/src/traces/otel_manager.rs +++ b/codex-rs/otel/src/traces/otel_manager.rs @@ -748,6 +748,7 @@ impl OtelManager { ResponseEvent::ReasoningSummaryPartAdded { .. } => { "reasoning_summary_part_added".into() } + ResponseEvent::ServerModel(_) => "server_model".into(), ResponseEvent::ServerReasoningIncluded(_) => "server_reasoning_included".into(), ResponseEvent::RateLimits(_) => "rate_limits".into(), ResponseEvent::ModelsEtag(_) => "models_etag".into(), diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index eba748f2e4..3a98aa5398 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -225,8 +225,6 @@ const APPROVAL_POLICY_UNLESS_TRUSTED: &str = include_str!("prompts/permissions/approval_policy/unless_trusted.md"); const APPROVAL_POLICY_ON_FAILURE: &str = include_str!("prompts/permissions/approval_policy/on_failure.md"); -const APPROVAL_POLICY_ON_REQUEST: &str = - include_str!("prompts/permissions/approval_policy/on_request.md"); const APPROVAL_POLICY_ON_REQUEST_RULE: &str = include_str!("prompts/permissions/approval_policy/on_request_rule.md"); @@ -241,29 +239,20 @@ impl DeveloperInstructions { Self { text: text.into() } } - pub fn from( - approval_policy: AskForApproval, - exec_policy: &Policy, - request_rule_enabled: bool, - ) -> DeveloperInstructions { + pub fn from(approval_policy: AskForApproval, exec_policy: &Policy) -> DeveloperInstructions { let text = match approval_policy { AskForApproval::Never => APPROVAL_POLICY_NEVER.to_string(), AskForApproval::UnlessTrusted => APPROVAL_POLICY_UNLESS_TRUSTED.to_string(), AskForApproval::OnFailure => APPROVAL_POLICY_ON_FAILURE.to_string(), AskForApproval::OnRequest => { - if !request_rule_enabled { - APPROVAL_POLICY_ON_REQUEST.to_string() - } else { - let command_prefixes = - format_allow_prefixes(exec_policy.get_allowed_prefixes()); - match command_prefixes { - Some(prefixes) => { - format!( - "{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}" - ) - } - None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(), + let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes()); + match command_prefixes { + Some(prefixes) => { + format!( + "{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}" + ) } + None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(), } } }; @@ -301,7 +290,6 @@ impl DeveloperInstructions { sandbox_policy: &SandboxPolicy, approval_policy: AskForApproval, exec_policy: &Policy, - request_rule_enabled: bool, cwd: &Path, ) -> Self { let network_access = if sandbox_policy.has_full_network_access() { @@ -325,7 +313,6 @@ impl DeveloperInstructions { network_access, approval_policy, exec_policy, - request_rule_enabled, writable_roots, ) } @@ -349,7 +336,6 @@ impl DeveloperInstructions { network_access: NetworkAccess, approval_policy: AskForApproval, exec_policy: &Policy, - request_rule_enabled: bool, writable_roots: Option>, ) -> Self { let start_tag = DeveloperInstructions::new(""); @@ -359,11 +345,7 @@ impl DeveloperInstructions { sandbox_mode, network_access, )) - .concat(DeveloperInstructions::from( - approval_policy, - exec_policy, - request_rule_enabled, - )) + .concat(DeveloperInstructions::from(approval_policy, exec_policy)) .concat(DeveloperInstructions::from_writable_roots(writable_roots)) .concat(end_tag) } @@ -1207,7 +1189,6 @@ mod tests { NetworkAccess::Enabled, AskForApproval::OnRequest, &Policy::empty(), - false, None, ); @@ -1217,7 +1198,7 @@ mod tests { "expected network access to be enabled in message" ); assert!( - text.contains("`approval_policy` is `on-request`"), + text.contains("How to request escalation"), "expected approval guidance to be included" ); } @@ -1236,7 +1217,6 @@ mod tests { &policy, AskForApproval::UnlessTrusted, &Policy::empty(), - false, &PathBuf::from("/tmp"), ); let text = instructions.into_text(); @@ -1245,7 +1225,7 @@ mod tests { } #[test] - fn includes_request_rule_instructions_when_enabled() { + fn includes_request_rule_instructions_for_on_request() { let mut exec_policy = Policy::empty(); exec_policy .add_prefix_rule( @@ -1258,7 +1238,6 @@ mod tests { NetworkAccess::Enabled, AskForApproval::OnRequest, &exec_policy, - true, None, ); diff --git a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request.md b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request.md deleted file mode 100644 index 16c90e8012..0000000000 --- a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request.md +++ /dev/null @@ -1,12 +0,0 @@ - Approvals are your mechanism to get user consent to run shell commands without the sandbox. `approval_policy` is `on-request`: Commands will be run in the sandbox by default, and you can specify in your tool call if you want to escalate a command to run without sandboxing. If the completing the task requires escalated permissions, Do not let these settings or the sandbox deter you from attempting to accomplish the user's task. - -Here are scenarios where you'll need to request approval: -- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /var) -- You need to run a GUI app (e.g., open/xdg-open/osascript) to open browsers or files. -- You are running sandboxed and need to run a command that requires network access (e.g. installing packages) -- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval. ALWAYS proceed to use the `sandbox_permissions` and `justification` parameters - do not message the user before requesting approval for the command. -- You are about to take a potentially destructive action such as an `rm` or `git reset` that the user did not explicitly ask for. - -When requesting approval to execute a command that will require escalated privileges: - - Provide the `sandbox_permissions` parameter with the value `"require_escalated"` - - Include a short, 1 sentence explanation for why you need escalated permissions in the justification parameter \ No newline at end of file diff --git a/codex-rs/shell-command/src/bash.rs b/codex-rs/shell-command/src/bash.rs index 47573621a8..770ff4795b 100644 --- a/codex-rs/shell-command/src/bash.rs +++ b/codex-rs/shell-command/src/bash.rs @@ -128,9 +128,6 @@ pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option) -> bool { node.named_children(&mut cursor).next().is_none() } -fn has_named_descendant_kind(node: Node<'_>, kind: &str) -> bool { - let mut stack = vec![node]; - while let Some(current) = stack.pop() { - if current.kind() == kind { - return true; - } - let mut cursor = current.walk(); - for child in current.named_children(&mut cursor) { - stack.push(child); - } - } - false -} - fn is_allowed_heredoc_attachment_kind(kind: &str) -> bool { matches!( kind, @@ -532,7 +515,10 @@ mod tests { "-lc".to_string(), "echo hello > /tmp/out.txt".to_string(), ]; - assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + assert_eq!( + parse_shell_lc_single_command_prefix(&command), + Some(vec!["echo".to_string(), "hello".to_string()]) + ); } #[test] @@ -548,6 +534,16 @@ mod tests { ); } + #[test] + fn parse_shell_lc_single_command_prefix_rejects_herestring_with_chaining() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + r#"echo hello > /tmp/out.txt && cat /tmp/out.txt"#.to_string(), + ]; + assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + } + #[test] fn parse_shell_lc_single_command_prefix_rejects_herestring_with_substitution() { let command = vec![ diff --git a/codex-rs/state/src/bin/logs_client.rs b/codex-rs/state/src/bin/logs_client.rs index 796a968cb8..6b0039ee97 100644 --- a/codex-rs/state/src/bin/logs_client.rs +++ b/codex-rs/state/src/bin/logs_client.rs @@ -46,6 +46,10 @@ struct Args { #[arg(long = "thread-id")] thread_id: Vec, + /// Substring match against the log message. + #[arg(long)] + search: Option, + /// Include logs that do not have a thread id. #[arg(long)] threadless: bool, @@ -57,6 +61,10 @@ struct Args { /// Poll interval in milliseconds. #[arg(long, default_value_t = 500)] poll_ms: u64, + + /// Show compact output with only time, level, and message. + #[arg(long)] + compact: bool, } #[derive(Debug, Clone)] @@ -67,6 +75,7 @@ struct LogFilter { module_like: Vec, file_like: Vec, thread_ids: Vec, + search: Option, include_threadless: bool, } @@ -81,7 +90,8 @@ async fn main() -> anyhow::Result<()> { .unwrap_or_else(|| PathBuf::from(".")); let runtime = StateRuntime::init(codex_home, "logs-client".to_string(), None).await?; - let mut last_id = print_backfill(runtime.as_ref(), &filter, args.backfill).await?; + let mut last_id = + print_backfill(runtime.as_ref(), &filter, args.backfill, args.compact).await?; if last_id == 0 { last_id = fetch_max_id(runtime.as_ref(), &filter).await?; } @@ -91,7 +101,7 @@ async fn main() -> anyhow::Result<()> { let rows = fetch_new_rows(runtime.as_ref(), &filter, last_id).await?; for row in rows { last_id = last_id.max(row.id); - println!("{}", format_row(&row)); + println!("{}", format_row(&row, args.compact)); } tokio::time::sleep(poll_interval).await; } @@ -154,6 +164,7 @@ fn build_filter(args: &Args) -> anyhow::Result { module_like, file_like, thread_ids, + search: args.search.clone(), include_threadless: args.threadless, }) } @@ -172,6 +183,7 @@ async fn print_backfill( runtime: &StateRuntime, filter: &LogFilter, backfill: usize, + compact: bool, ) -> anyhow::Result { if backfill == 0 { return Ok(0); @@ -183,7 +195,7 @@ async fn print_backfill( let mut last_id = 0; for row in rows { last_id = last_id.max(row.id); - println!("{}", format_row(&row)); + println!("{}", format_row(&row, compact)); } Ok(last_id) } @@ -233,6 +245,7 @@ fn to_log_query( module_like: filter.module_like.clone(), file_like: filter.file_like.clone(), thread_ids: filter.thread_ids.clone(), + search: filter.search.clone(), include_threadless: filter.include_threadless, after_id, limit, @@ -240,8 +253,8 @@ fn to_log_query( } } -fn format_row(row: &LogRow) -> String { - let timestamp = formatter::ts(row.ts, row.ts_nanos); +fn format_row(row: &LogRow, compact: bool) -> String { + let timestamp = formatter::ts(row.ts, row.ts_nanos, compact); let level = row.level.as_str(); let target = row.target.as_str(); let message = row.message.as_deref().unwrap_or(""); @@ -251,9 +264,13 @@ fn format_row(row: &LogRow) -> String { let thread_id_colored = thread_id.blue().dimmed().to_string(); let target_colored = target.dimmed().to_string(); let message_colored = heuristic_formatting(message); - format!( - "{timestamp_colored} {level_colored} [{thread_id_colored}] {target_colored} - {message_colored}" - ) + if compact { + format!("{timestamp_colored} {level_colored} {message_colored}") + } else { + format!( + "{timestamp_colored} {level_colored} [{thread_id_colored}] {target_colored} - {message_colored}" + ) + } } fn heuristic_formatting(message: &str) -> String { @@ -292,9 +309,10 @@ mod formatter { .join("\n") } - pub(super) fn ts(ts: i64, ts_nanos: i64) -> String { + pub(super) fn ts(ts: i64, ts_nanos: i64, compact: bool) -> String { let nanos = u32::try_from(ts_nanos).unwrap_or(0); match DateTime::::from_timestamp(ts, nanos) { + Some(dt) if compact => dt.format("%H:%M:%S").to_string(), Some(dt) => dt.to_rfc3339_opts(SecondsFormat::Millis, true), None => format!("{ts}.{ts_nanos:09}Z"), } diff --git a/codex-rs/state/src/model/log.rs b/codex-rs/state/src/model/log.rs index 2aeaf2d5c8..cf973bceeb 100644 --- a/codex-rs/state/src/model/log.rs +++ b/codex-rs/state/src/model/log.rs @@ -37,6 +37,7 @@ pub struct LogQuery { pub module_like: Vec, pub file_like: Vec, pub thread_ids: Vec, + pub search: Option, pub include_threadless: bool, pub after_id: Option, pub limit: Option, diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 3a743a9c70..f9beacdae8 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -711,6 +711,11 @@ fn push_log_filters<'a>(builder: &mut QueryBuilder<'a, Sqlite>, query: &'a LogQu if let Some(after_id) = query.after_id { builder.push(" AND id > ").push_bind(after_id); } + if let Some(search) = query.search.as_ref() { + builder.push(" AND INSTR(message, "); + builder.push_bind(search.as_str()); + builder.push(") > 0"); + } } fn push_like_filters<'a>( @@ -906,6 +911,8 @@ mod tests { use super::StateRuntime; use super::ThreadMetadata; use super::state_db_filename; + use crate::LogEntry; + use crate::LogQuery; use crate::STATE_DB_FILENAME; use crate::STATE_DB_VERSION; use crate::model::Phase2JobClaimOutcome; @@ -2495,6 +2502,57 @@ VALUES (?, ?, ?, ?, ?) let _ = tokio::fs::remove_dir_all(codex_home).await; } + #[tokio::test] + async fn query_logs_with_search_matches_substring() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + runtime + .insert_logs(&[ + LogEntry { + ts: 1_700_000_001, + ts_nanos: 0, + level: "INFO".to_string(), + target: "cli".to_string(), + message: Some("alpha".to_string()), + thread_id: Some("thread-1".to_string()), + process_uuid: None, + file: Some("main.rs".to_string()), + line: Some(42), + module_path: None, + }, + LogEntry { + ts: 1_700_000_002, + ts_nanos: 0, + level: "INFO".to_string(), + target: "cli".to_string(), + message: Some("alphabet".to_string()), + thread_id: Some("thread-1".to_string()), + process_uuid: None, + file: Some("main.rs".to_string()), + line: Some(43), + module_path: None, + }, + ]) + .await + .expect("insert test logs"); + + let rows = runtime + .query_logs(&LogQuery { + search: Some("alphab".to_string()), + ..Default::default() + }) + .await + .expect("query matching logs"); + + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].message.as_deref(), Some("alphabet")); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + fn test_thread_metadata( codex_home: &Path, thread_id: ThreadId, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7bd105695d..b23b5f1a13 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -978,6 +978,27 @@ impl App { self.refresh_status_line(); } + fn should_wait_for_initial_session(session_selection: &SessionSelection) -> bool { + matches!( + session_selection, + SessionSelection::StartFresh | SessionSelection::Exit + ) + } + + fn should_handle_active_thread_events( + waiting_for_initial_session_configured: bool, + has_active_thread_receiver: bool, + ) -> bool { + has_active_thread_receiver && !waiting_for_initial_session_configured + } + + fn should_stop_waiting_for_initial_session( + waiting_for_initial_session_configured: bool, + primary_thread_id: Option, + ) -> bool { + waiting_for_initial_session_configured && primary_thread_id.is_some() + } + #[allow(clippy::too_many_arguments)] pub async fn run( tui: &mut tui::Tui, @@ -1068,6 +1089,8 @@ impl App { let status_line_invalid_items_warned = Arc::new(AtomicBool::new(false)); let enhanced_keys_supported = tui.enhanced_keys_supported(); + let wait_for_initial_session_configured = + Self::should_wait_for_initial_session(&session_selection); let mut chat_widget = match session_selection { SessionSelection::StartFresh | SessionSelection::Exit => { let init = crate::chatwidget::ChatWidgetInit { @@ -1252,6 +1275,7 @@ impl App { let mut thread_created_rx = thread_manager.subscribe_thread_created(); let mut listen_for_threads = true; + let mut waiting_for_initial_session_configured = wait_for_initial_session_configured; let exit_reason = loop { let control = select! { @@ -1264,7 +1288,10 @@ impl App { } else { None } - }, if app.active_thread_rx.is_some() => { + }, if App::should_handle_active_thread_events( + waiting_for_initial_session_configured, + app.active_thread_rx.is_some() + ) => { if let Some(event) = active { app.handle_active_thread_event(tui, event).await?; } else { @@ -1291,6 +1318,12 @@ impl App { AppRunControl::Continue } }; + if App::should_stop_waiting_for_initial_session( + waiting_for_initial_session_configured, + app.primary_thread_id, + ) { + waiting_for_initial_session_configured = false; + } match control { AppRunControl::Continue => {} AppRunControl::Exit(reason) => break reason, @@ -2915,6 +2948,76 @@ mod tests { Ok(()) } + #[test] + fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::StartFresh), + true + ); + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::Exit), + true + ); + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::Resume(PathBuf::from( + "/tmp/restore" + ))), + false + ); + assert_eq!( + App::should_wait_for_initial_session(&SessionSelection::Fork(PathBuf::from( + "/tmp/fork" + ))), + false + ); + } + + #[test] + fn startup_waiting_gate_holds_active_thread_events_until_primary_thread_configured() { + let mut wait_for_initial_session = + App::should_wait_for_initial_session(&SessionSelection::StartFresh); + assert_eq!(wait_for_initial_session, true); + assert_eq!( + App::should_handle_active_thread_events(wait_for_initial_session, true), + false + ); + + assert_eq!( + App::should_stop_waiting_for_initial_session(wait_for_initial_session, None), + false + ); + if App::should_stop_waiting_for_initial_session( + wait_for_initial_session, + Some(ThreadId::new()), + ) { + wait_for_initial_session = false; + } + assert_eq!(wait_for_initial_session, false); + + assert_eq!( + App::should_handle_active_thread_events(wait_for_initial_session, true), + true + ); + } + + #[test] + fn startup_waiting_gate_not_applied_for_resume_or_fork_session_selection() { + let wait_for_resume = App::should_wait_for_initial_session(&SessionSelection::Resume( + PathBuf::from("/tmp/restore"), + )); + assert_eq!( + App::should_handle_active_thread_events(wait_for_resume, true), + true + ); + let wait_for_fork = App::should_wait_for_initial_session(&SessionSelection::Fork( + PathBuf::from("/tmp/fork"), + )); + assert_eq!( + App::should_handle_active_thread_events(wait_for_fork, true), + true + ); + } + #[tokio::test] async fn enqueue_thread_event_does_not_block_when_channel_full() -> Result<()> { let mut app = make_test_app().await; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 8bf3ab3703..1c9e8ce88e 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -371,5 +371,6 @@ pub(crate) enum FeedbackCategory { BadResult, GoodResult, Bug, + SafetyCheck, Other, } diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 48cf255ee0..1c2291465e 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -45,12 +45,15 @@ pub(crate) struct CommandPopupFlags { impl CommandPopup { pub(crate) fn new(mut prompts: Vec, flags: CommandPopupFlags) -> Self { // Keep built-in availability in sync with the composer. - let builtins = slash_commands::builtins_for_input( + let builtins: Vec<(&'static str, SlashCommand)> = slash_commands::builtins_for_input( flags.collaboration_modes_enabled, flags.connectors_enabled, flags.personality_command_enabled, flags.windows_degraded_sandbox_active, - ); + ) + .into_iter() + .filter(|(name, _)| !name.starts_with("debug")) + .collect(); // Exclude prompts that collide with builtin command names and sort by name. let exclude: HashSet = builtins.iter().map(|(n, _)| (*n).to_string()).collect(); prompts.retain(|p| !exclude.contains(&p.name)); @@ -566,4 +569,22 @@ mod tests { other => panic!("expected personality to be selected for exact match, got {other:?}"), } } + + #[test] + fn debug_commands_are_hidden_from_popup() { + let popup = CommandPopup::new(Vec::new(), CommandPopupFlags::default()); + let cmds: Vec<&str> = popup + .filtered_items() + .into_iter() + .filter_map(|item| match item { + CommandItem::Builtin(cmd) => Some(cmd.command()), + CommandItem::UserPrompt(_) => None, + }) + .collect(); + + assert!( + !cmds.iter().any(|name| name.starts_with("debug")), + "expected no /debug* command in popup menu, got {cmds:?}" + ); + } } diff --git a/codex-rs/tui/src/bottom_pane/feedback_view.rs b/codex-rs/tui/src/bottom_pane/feedback_view.rs index 6bc0ab4474..6cbfbdb9e5 100644 --- a/codex-rs/tui/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui/src/bottom_pane/feedback_view.rs @@ -353,6 +353,10 @@ fn feedback_title_and_placeholder(category: FeedbackCategory) -> (String, String "Tell us more (bug)".to_string(), "(optional) Write a short description to help us further".to_string(), ), + FeedbackCategory::SafetyCheck => ( + "Tell us more (safety check)".to_string(), + "(optional) Share what was refused and why it should have been allowed".to_string(), + ), FeedbackCategory::Other => ( "Tell us more (other)".to_string(), "(optional) Write a short description to help us further".to_string(), @@ -365,6 +369,7 @@ fn feedback_classification(category: FeedbackCategory) -> &'static str { FeedbackCategory::BadResult => "bad_result", FeedbackCategory::GoodResult => "good_result", FeedbackCategory::Bug => "bug", + FeedbackCategory::SafetyCheck => "safety_check", FeedbackCategory::Other => "other", } } @@ -378,14 +383,15 @@ fn issue_url_for_category( // the external GitHub behavior identical while routing internal users to // the internal go link. match category { - FeedbackCategory::Bug | FeedbackCategory::BadResult | FeedbackCategory::Other => { - Some(match feedback_audience { - FeedbackAudience::OpenAiEmployee => slack_feedback_url(thread_id), - FeedbackAudience::External => { - format!("{BASE_BUG_ISSUE_URL}&steps=Uploaded%20thread:%20{thread_id}") - } - }) - } + FeedbackCategory::Bug + | FeedbackCategory::BadResult + | FeedbackCategory::SafetyCheck + | FeedbackCategory::Other => Some(match feedback_audience { + FeedbackAudience::OpenAiEmployee => slack_feedback_url(thread_id), + FeedbackAudience::External => { + format!("{BASE_BUG_ISSUE_URL}&steps=Uploaded%20thread:%20{thread_id}") + } + }), FeedbackCategory::GoodResult => None, } } @@ -423,6 +429,12 @@ pub(crate) fn feedback_selection_params( "Helpful, correct, high‑quality, or delightful result worth celebrating.", FeedbackCategory::GoodResult, ), + make_feedback_item( + app_event_tx.clone(), + "safety check", + "Benign usage blocked due to safety checks or refusals.", + FeedbackCategory::SafetyCheck, + ), make_feedback_item( app_event_tx, "other", @@ -616,7 +628,14 @@ mod tests { } #[test] - fn issue_url_available_for_bug_bad_result_and_other() { + fn feedback_view_safety_check() { + let view = make_view(FeedbackCategory::SafetyCheck); + let rendered = render(&view, 60); + insta::assert_snapshot!("feedback_view_safety_check", rendered); + } + + #[test] + fn issue_url_available_for_bug_bad_result_safety_check_and_other() { let bug_url = issue_url_for_category( FeedbackCategory::Bug, "thread-1", @@ -639,6 +658,13 @@ mod tests { ); assert!(other_url.is_some()); + let safety_check_url = issue_url_for_category( + FeedbackCategory::SafetyCheck, + "thread-4", + FeedbackAudience::OpenAiEmployee, + ); + assert!(safety_check_url.is_some()); + assert!( issue_url_for_category( FeedbackCategory::GoodResult, diff --git a/codex-rs/tui/src/bottom_pane/slash_commands.rs b/codex-rs/tui/src/bottom_pane/slash_commands.rs index cdd0ba8f7d..4613161f2f 100644 --- a/codex-rs/tui/src/bottom_pane/slash_commands.rs +++ b/codex-rs/tui/src/bottom_pane/slash_commands.rs @@ -63,3 +63,15 @@ pub(crate) fn has_builtin_prefix( .into_iter() .any(|(command_name, _)| fuzzy_match(command_name, name).is_some()) } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn debug_command_still_resolves_for_dispatch() { + let cmd = find_builtin_command("debug-config", true, true, true, false); + assert_eq!(cmd, Some(SlashCommand::DebugConfig)); + } +} diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_safety_check.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_safety_check.snap new file mode 100644 index 0000000000..52148d0e86 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_safety_check.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/bottom_pane/feedback_view.rs +expression: rendered +--- +▌ Tell us more (safety check) +▌ +▌ (optional) Share what was refused and why it should have b + +Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index f72255b22b..dcbee999bc 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -184,7 +184,6 @@ use crate::bottom_pane::SelectionViewParams; use crate::bottom_pane::custom_prompt_view::CustomPromptView; use crate::bottom_pane::popup_consts::standard_popup_hint_line; use crate::clipboard_paste::paste_image_to_temp_png; -use crate::collab; use crate::collaboration_modes; use crate::diff_render::display_path_for; use crate::exec_cell::CommandOutput; @@ -201,6 +200,7 @@ use crate::history_cell::WebSearchCell; use crate::key_hint; use crate::key_hint::KeyBinding; use crate::markdown::append_markdown; +use crate::multi_agents; use crate::render::Insets; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::FlexRenderable; @@ -4113,17 +4113,19 @@ impl ChatWidget { EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review), EventMsg::ContextCompacted(_) => self.on_agent_message("Context compacted".to_owned()), EventMsg::CollabAgentSpawnBegin(_) => {} - EventMsg::CollabAgentSpawnEnd(ev) => self.on_collab_event(collab::spawn_end(ev)), + EventMsg::CollabAgentSpawnEnd(ev) => self.on_collab_event(multi_agents::spawn_end(ev)), EventMsg::CollabAgentInteractionBegin(_) => {} EventMsg::CollabAgentInteractionEnd(ev) => { - self.on_collab_event(collab::interaction_end(ev)) + self.on_collab_event(multi_agents::interaction_end(ev)) } - EventMsg::CollabWaitingBegin(ev) => self.on_collab_event(collab::waiting_begin(ev)), - EventMsg::CollabWaitingEnd(ev) => self.on_collab_event(collab::waiting_end(ev)), + EventMsg::CollabWaitingBegin(ev) => { + self.on_collab_event(multi_agents::waiting_begin(ev)) + } + EventMsg::CollabWaitingEnd(ev) => self.on_collab_event(multi_agents::waiting_end(ev)), EventMsg::CollabCloseBegin(_) => {} - EventMsg::CollabCloseEnd(ev) => self.on_collab_event(collab::close_end(ev)), - EventMsg::CollabResumeBegin(ev) => self.on_collab_event(collab::resume_begin(ev)), - EventMsg::CollabResumeEnd(ev) => self.on_collab_event(collab::resume_end(ev)), + EventMsg::CollabCloseEnd(ev) => self.on_collab_event(multi_agents::close_end(ev)), + EventMsg::CollabResumeBegin(ev) => self.on_collab_event(multi_agents::resume_begin(ev)), + EventMsg::CollabResumeEnd(ev) => self.on_collab_event(multi_agents::resume_end(ev)), EventMsg::ThreadRolledBack(rollback) => { if from_replay { self.app_event_tx.send(AppEvent::ApplyThreadRollback { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap index 4a98242027..1b7627a97e 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap @@ -4,8 +4,10 @@ expression: popup --- How was this? -› 1. bug Crash, error message, hang, or broken UI/behavior. - 2. bad result Output was off-target, incorrect, incomplete, or unhelpful. - 3. good result Helpful, correct, high‑quality, or delightful result worth - celebrating. - 4. other Slowness, feature suggestion, UX feedback, or anything else. +› 1. bug Crash, error message, hang, or broken UI/behavior. + 2. bad result Output was off-target, incorrect, incomplete, or unhelpful. + 3. good result Helpful, correct, high‑quality, or delightful result worth + celebrating. + 4. safety check Benign usage blocked due to safety checks or refusals. + 5. other Slowness, feature suggestion, UX feedback, or anything + else. diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 639916a79f..cb4e311576 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -64,7 +64,6 @@ mod bottom_pane; mod chatwidget; mod cli; mod clipboard_paste; -mod collab; mod collaboration_modes; mod color; pub mod custom_terminal; @@ -86,6 +85,7 @@ mod markdown_render; mod markdown_stream; mod mention_codec; mod model_migration; +mod multi_agents; mod notifications; pub mod onboarding; mod oss_selection; diff --git a/codex-rs/tui/src/collab.rs b/codex-rs/tui/src/multi_agents.rs similarity index 100% rename from codex-rs/tui/src/collab.rs rename to codex-rs/tui/src/multi_agents.rs diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 94eb038cef..b41a2fa3ca 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -218,9 +218,11 @@ impl Renderable for StatusIndicatorWidget { return; } - // Schedule next animation frame. - self.frame_requester - .schedule_frame_in(Duration::from_millis(32)); + if self.animations_enabled { + // Schedule next animation frame. + self.frame_requester + .schedule_frame_in(Duration::from_millis(32)); + } let now = Instant::now(); let elapsed_duration = self.elapsed_duration_at(now); let pretty_elapsed = fmt_elapsed_compact(elapsed_duration.as_secs());