From 14ebfbced9dc502713cf68d457ea78618563b7dc Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Mon, 20 Apr 2026 13:43:50 -0700 Subject: [PATCH 1/7] chore(guardian) disable mcps and plugins (#18722) ## Summary Disables apps, plugins, mcps for the guardian subagent thread ## Testing - [x] Added unit tests --- codex-rs/core/src/guardian/review_session.rs | 9 +++++ codex-rs/core/src/guardian/tests.rs | 36 ++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 7aea5978f1..5bb3e9d1b8 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -732,6 +732,13 @@ pub(crate) fn build_guardian_review_session_config( guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); guardian_config.permissions.sandbox_policy = Constrained::allow_only(SandboxPolicy::new_read_only_policy()); + guardian_config.include_apps_instructions = false; + guardian_config + .mcp_servers + .set(HashMap::new()) + .map_err(|err| { + anyhow::anyhow!("guardian review session could not clear MCP servers: {err}") + })?; if let Some(live_network_config) = live_network_config && guardian_config.permissions.network.is_some() { @@ -751,6 +758,8 @@ pub(crate) fn build_guardian_review_session_config( Feature::SpawnCsv, Feature::Collab, Feature::CodexHooks, + Feature::Apps, + Feature::Plugins, Feature::WebSearchRequest, Feature::WebSearchCached, ] { diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 19007e9d32..098e0f047a 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -16,7 +16,9 @@ use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::test_support; use codex_config::config_toml::ConfigToml; +use codex_config::types::McpServerConfig; use codex_exec_server::LOCAL_FS; +use codex_features::Feature; use codex_model_provider::create_model_provider; use codex_network_proxy::NetworkProxyConfig; use codex_protocol::ThreadId; @@ -52,6 +54,7 @@ use insta::Settings; use insta::assert_snapshot; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; use tempfile::TempDir; @@ -1684,6 +1687,39 @@ async fn guardian_review_session_config_uses_live_network_proxy_state() { ); } +#[tokio::test] +async fn guardian_review_session_config_disables_mcp_apps_and_plugins() { + let mut parent_config = test_config().await; + let server: McpServerConfig = + toml::from_str("command = \"docs-server\"").expect("deserialize MCP server"); + parent_config + .mcp_servers + .set(HashMap::from([("docs".to_string(), server)])) + .expect("parent MCP servers are configurable"); + parent_config + .features + .enable(Feature::Apps) + .expect("apps feature is configurable"); + parent_config + .features + .enable(Feature::Plugins) + .expect("plugins feature is configurable"); + parent_config.include_apps_instructions = true; + + let guardian_config = build_guardian_review_session_config_for_test( + &parent_config, + /*live_network_config*/ None, + "active-model", + /*reasoning_effort*/ None, + ) + .expect("guardian config"); + + assert!(guardian_config.mcp_servers.get().is_empty()); + assert!(!guardian_config.features.enabled(Feature::Apps)); + assert!(!guardian_config.features.enabled(Feature::Plugins)); + assert!(!guardian_config.include_apps_instructions); +} + #[tokio::test] async fn guardian_review_session_config_rejects_pinned_collab_feature() { let mut parent_config = test_config().await; From 126bd6e7a8839a861ee9bb40ec72c72ea1bf7b4d Mon Sep 17 00:00:00 2001 From: guinness-oai Date: Mon, 20 Apr 2026 14:04:09 -0700 Subject: [PATCH 2/7] Update realtime handoff transcript handling (#18597) ## Summary This PR aims to improve integration between the realtime model and the codex agent by sharing more context with each other. In particular, we now share full realtime conversation transcript deltas in addition to the delegation message. realtime_conversation.rs now turns a handoff into: ``` ... ... ``` ## Implementation notes The transcript is accumulated in the realtime websocket layer as parsed realtime events arrive. When a background-agent handoff is requested, the current transcript snapshot is copied onto the handoff event and then serialized by `realtime_conversation.rs` into the hidden realtime delegation envelope that Codex receives as user-turn context. For Realtime V2, the session now explicitly enables input audio transcription, and the parser handles the relevant input/output transcript completion events so the snapshot includes both user speech and realtime model responses. The delegation `` remains the actual handoff request, while `` carries the surrounding conversation history for context. Reviewers should note that the transcript payload is intended for Codex context sharing, not UI rendering. The realtime delegation envelope should stay hidden from the user-facing transcript surface, while still being included in the background-agent turn so Codex can answer with the same conversational context the realtime model had. --- .../tests/suite/v2/realtime_conversation.rs | 101 ++++++- .../endpoint/realtime_websocket/methods.rs | 276 ++++++++++++++++-- .../endpoint/realtime_websocket/methods_v1.rs | 1 + .../endpoint/realtime_websocket/methods_v2.rs | 8 + .../endpoint/realtime_websocket/protocol.rs | 7 + .../realtime_websocket/protocol_v1.rs | 32 +- .../realtime_websocket/protocol_v2.rs | 41 +-- .../codex-api/tests/realtime_websocket_e2e.rs | 85 +++++- codex-rs/core/src/realtime_conversation.rs | 41 ++- .../core/src/realtime_conversation_tests.rs | 60 +++- .../core/tests/suite/realtime_conversation.rs | 18 +- 11 files changed, 572 insertions(+), 98 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs b/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs index c61b628a6d..e5676b55ff 100644 --- a/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs +++ b/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs @@ -458,7 +458,10 @@ fn v2_background_agent_tool_call(call_id: &str, prompt: &str) -> Value { async fn realtime_conversation_streams_v2_notifications() -> Result<()> { skip_if_no_network!(Ok(())); - let responses_server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; + let responses_server = create_mock_responses_server_sequence_unchecked(vec![ + create_final_assistant_message_sse_response("delegated")?, + ]) + .await; let realtime_server = start_websocket_server(vec![vec![ vec![json!({ "type": "session.updated", @@ -489,6 +492,10 @@ async fn realtime_conversation_streams_v2_notifications() -> Result<()> { "type": "response.output_text.delta", "delta": "working" }), + json!({ + "type": "response.output_text.done", + "text": "working on it" + }), json!({ "type": "conversation.item.done", "item": { @@ -677,7 +684,13 @@ async fn realtime_conversation_streams_v2_notifications() -> Result<()> { handoff_item_added.item["input_transcript"], json!("delegate now") ); - assert_eq!(handoff_item_added.item["active_transcript"], json!([])); + assert_eq!( + handoff_item_added.item["active_transcript"], + json!([ + {"role": "user", "text": "delegate now"}, + {"role": "assistant", "text": "working on it"} + ]) + ); let realtime_error = read_notification::(&mut mcp, "thread/realtime/error") @@ -1142,7 +1155,7 @@ async fn realtime_webrtc_start_emits_sdp_notification() -> Result<()> { Some("multipart/form-data; boundary=codex-realtime-call-boundary") ); let body = String::from_utf8(request.body).context("multipart body should be utf-8")?; - let session = r#"{"tool_choice":"auto","type":"realtime","model":"gpt-realtime-1.5","instructions":"backend prompt\n\nstartup context","output_modalities":["audio"],"audio":{"input":{"format":{"type":"audio/pcm","rate":24000},"noise_reduction":{"type":"near_field"},"turn_detection":{"type":"server_vad","interrupt_response":true,"create_response":true,"silence_duration_ms":500}},"output":{"format":{"type":"audio/pcm","rate":24000},"voice":"marin"}},"tools":[{"type":"function","name":"background_agent","description":"Send a user request to the background agent. Use this as the default action. Do not rephrase the user's ask or rewrite it in your own words; pass along the user's own words. If the background agent is idle, this starts a new task and returns the final result to the user. If the background agent is already working on a task, this sends the request as guidance to steer that previous task. If the user asks to do something next, later, after this, or once current work finishes, call this tool so the work is actually queued instead of merely promising to do it later.","parameters":{"type":"object","properties":{"prompt":{"type":"string","description":"The user request to delegate to the background agent."}},"required":["prompt"],"additionalProperties":false}}]}"#; + let session = r#"{"tool_choice":"auto","type":"realtime","model":"gpt-realtime-1.5","instructions":"backend prompt\n\nstartup context","output_modalities":["audio"],"audio":{"input":{"format":{"type":"audio/pcm","rate":24000},"noise_reduction":{"type":"near_field"},"transcription":{"model":"gpt-4o-mini-transcribe"},"turn_detection":{"type":"server_vad","interrupt_response":true,"create_response":true,"silence_duration_ms":500}},"output":{"format":{"type":"audio/pcm","rate":24000},"voice":"marin"}},"tools":[{"type":"function","name":"background_agent","description":"Send a user request to the background agent. Use this as the default action. Do not rephrase the user's ask or rewrite it in your own words; pass along the user's own words. If the background agent is idle, this starts a new task and returns the final result to the user. If the background agent is already working on a task, this sends the request as guidance to steer that previous task. If the user asks to do something next, later, after this, or once current work finishes, call this tool so the work is actually queued instead of merely promising to do it later.","parameters":{"type":"object","properties":{"prompt":{"type":"string","description":"The user request to delegate to the background agent."}},"required":["prompt"],"additionalProperties":false}}]}"#; let session = normalized_json_string(session)?; assert_eq!( body, @@ -1241,8 +1254,16 @@ async fn webrtc_v1_handoff_request_delegates_and_appends_result() -> Result<()> vec![ session_updated("sess_v1_handoff"), json!({ - "type": "conversation.input_transcript.delta", - "delta": "delegate from v1" + "type": "conversation.item.input_audio_transcription.completed", + "transcript": "delegate from v1" + }), + json!({ + "type": "response.output_audio_transcript.delta", + "delta": "the secret word is " + }), + json!({ + "type": "response.output_audio_transcript.delta", + "delta": "kumquat" }), json!({ "type": "conversation.handoff.requested", @@ -1258,6 +1279,12 @@ async fn webrtc_v1_handoff_request_delegates_and_appends_result() -> Result<()> let started = harness.start_webrtc_realtime("v=offer\r\n").await?; assert_eq!(started.started.version, RealtimeConversationVersion::V1); + assert_call_create_multipart( + harness.call_capture.single_request(), + "v=offer\r\n", + v1_session_create_json(), + )?; + assert_v1_session_update(&harness.sideband_outbound_request(/*request_index*/ 0).await)?; // Phase 2: wait for the delegated background agent turn that is launched by the handoff request. let turn_started = harness @@ -1274,11 +1301,13 @@ async fn webrtc_v1_handoff_request_delegates_and_appends_result() -> Result<()> let requests = harness.main_loop_responses_requests().await?; assert_eq!(requests.len(), 1); assert!( - response_request_contains_text(&requests[0], "user: delegate from v1"), - "delegated Responses request should contain realtime text: {}", + response_request_contains_text( + &requests[0], + "\n delegate from v1\n user: delegate from v1\nassistant: the secret word is kumquat\n", + ), + "delegated Responses request should contain realtime delegation envelope: {}", requests[0] ); - let handoff_append = harness.sideband_outbound_request(/*request_index*/ 1).await; assert_eq!( handoff_append, @@ -1526,7 +1555,34 @@ async fn webrtc_v2_background_agent_tool_call_delegates_and_returns_function_out realtime_sideband(vec![realtime_sideband_connection(vec![ vec![ session_updated("sess_v2_tool"), - v2_background_agent_tool_call("call_v2", "delegate from v2"), + json!({ + "type": "conversation.item.input_audio_transcription.completed", + "transcript": "Hi how are you" + }), + json!({ + "type": "response.output_audio_transcript.done", + "transcript": "Doing well, what can I help you with?" + }), + json!({ + "type": "conversation.item.input_audio_transcription.completed", + "transcript": "The secret word is strawberry" + }), + json!({ + "type": "conversation.item.created", + "item": { + "type": "message", + "role": "user", + "content": [{ + "type": "input_text", + "text": "silent_delegate" + }] + } + }), + json!({ + "type": "response.output_audio_transcript.delta", + "delta": "Got it-strawberry. What's next on the menu?" + }), + v2_background_agent_tool_call("call_v2", "run ls"), ], vec![], vec![], @@ -1553,8 +1609,16 @@ async fn webrtc_v2_background_agent_tool_call_delegates_and_returns_function_out let requests = harness.main_loop_responses_requests().await?; assert_eq!(requests.len(), 1); assert!( - response_request_contains_text(&requests[0], "delegate from v2"), - "delegated Responses request should contain tool prompt: {}", + response_request_contains_text( + &requests[0], + "\n run ls\n user: Hi how are you\nassistant: Doing well, what can I help you with?\nuser: The secret word is strawberry\nassistant: Got it-strawberry. What's next on the menu?\nuser: run ls\n", + ), + "delegated Responses request should contain realtime delegation envelope: {}", + requests[0] + ); + assert!( + !response_request_contains_text(&requests[0], ""), + "delegated Responses request should not include realtime control injects: {}", requests[0] ); @@ -2068,7 +2132,16 @@ async fn responses_requests(server: &MockServer) -> Result> { } fn response_request_contains_text(request: &Value, text: &str) -> bool { - request.to_string().contains(text) + match request { + Value::String(value) => value.contains(text), + Value::Array(values) => values + .iter() + .any(|value| response_request_contains_text(value, text)), + Value::Object(map) => map + .values() + .any(|value| response_request_contains_text(value, text)), + Value::Null | Value::Bool(_) | Value::Number(_) => false, + } } fn realtime_tool_ok_command() -> Vec { @@ -2187,6 +2260,10 @@ fn assert_v2_session_update(request: &Value) -> Result<()> { request["session"]["tools"][0]["name"].as_str(), Some("background_agent") ); + assert_eq!( + request["session"]["audio"]["input"]["transcription"]["model"].as_str(), + Some("gpt-4o-mini-transcribe") + ); Ok(()) } diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs index 656b006374..a4ddb5dee4 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs @@ -217,6 +217,8 @@ pub struct RealtimeWebsocketEvents { #[derive(Default)] struct ActiveTranscriptState { entries: Vec, + new_input_entry: bool, + new_output_entry: bool, } impl RealtimeWebsocketConnection { @@ -417,38 +419,76 @@ impl RealtimeWebsocketEvents { async fn update_active_transcript(&self, event: &mut RealtimeEvent) { let mut active_transcript = self.active_transcript.lock().await; match event { - RealtimeEvent::InputAudioSpeechStarted(_) => {} + RealtimeEvent::InputAudioSpeechStarted(_) => { + active_transcript.new_input_entry = true; + } RealtimeEvent::InputTranscriptDelta(RealtimeTranscriptDelta { delta, .. }) => { - append_transcript_delta(&mut active_transcript.entries, "user", delta); + let force_new = active_transcript.new_input_entry; + append_transcript_delta(&mut active_transcript.entries, "user", delta, force_new); + active_transcript.new_input_entry = false; } RealtimeEvent::OutputTranscriptDelta(RealtimeTranscriptDelta { delta, .. }) => { - append_transcript_delta(&mut active_transcript.entries, "assistant", delta); + let force_new = active_transcript.new_output_entry; + append_transcript_delta( + &mut active_transcript.entries, + "assistant", + delta, + force_new, + ); + active_transcript.new_output_entry = false; + } + RealtimeEvent::InputTranscriptDone(done) => { + let force_new = active_transcript.new_input_entry; + apply_transcript_done( + &mut active_transcript.entries, + "user", + &done.text, + force_new, + ); + active_transcript.new_input_entry = false; + } + RealtimeEvent::OutputTranscriptDone(done) => { + let force_new = active_transcript.new_output_entry; + apply_transcript_done( + &mut active_transcript.entries, + "assistant", + &done.text, + force_new, + ); + active_transcript.new_output_entry = false; } RealtimeEvent::HandoffRequested(handoff) => { - if self.event_parser == RealtimeEventParser::V1 { - handoff.active_transcript = std::mem::take(&mut active_transcript.entries); - } + append_handoff_input(&mut active_transcript.entries, &handoff.input_transcript); + handoff.active_transcript = active_transcript.entries.clone(); + active_transcript.new_input_entry = true; + active_transcript.new_output_entry = true; + } + RealtimeEvent::ResponseCreated(_) => { + active_transcript.new_output_entry = true; } RealtimeEvent::SessionUpdated { .. } - | RealtimeEvent::InputTranscriptDone(_) - | RealtimeEvent::OutputTranscriptDone(_) | RealtimeEvent::AudioOut(_) - | RealtimeEvent::ResponseCreated(_) | RealtimeEvent::ResponseCancelled(_) | RealtimeEvent::ResponseDone(_) - | RealtimeEvent::ConversationItemAdded(_) | RealtimeEvent::ConversationItemDone { .. } + | RealtimeEvent::ConversationItemAdded(_) | RealtimeEvent::Error(_) => {} } } } -fn append_transcript_delta(entries: &mut Vec, role: &str, delta: &str) { +fn append_transcript_delta( + entries: &mut Vec, + role: &str, + delta: &str, + force_new: bool, +) { if delta.is_empty() { return; } - if let Some(last_entry) = entries.last_mut() + if !force_new + && let Some(last_entry) = entries.last_mut() && last_entry.role == role { last_entry.text.push_str(delta); @@ -461,6 +501,48 @@ fn append_transcript_delta(entries: &mut Vec, role: &st }); } +fn apply_transcript_done( + entries: &mut Vec, + role: &str, + text: &str, + force_new: bool, +) { + if text.is_empty() { + return; + } + + if !force_new + && let Some(last_entry) = entries.last_mut() + && last_entry.role == role + { + last_entry.text = text.to_string(); + return; + } + + entries.push(RealtimeTranscriptEntry { + role: role.to_string(), + text: text.to_string(), + }); +} + +fn append_handoff_input(entries: &mut Vec, input: &str) { + let input = input.trim(); + if input.is_empty() || contains_transcript_entry(entries, "user", input) { + return; + } + + entries.push(RealtimeTranscriptEntry { + role: "user".to_string(), + text: input.to_string(), + }); +} + +fn contains_transcript_entry(entries: &[RealtimeTranscriptEntry], role: &str, text: &str) -> bool { + entries + .iter() + .any(|entry| entry.role == role && entry.text.trim() == text.trim()) +} + pub struct RealtimeWebsocketClient { provider: Provider, } @@ -867,6 +949,44 @@ mod tests { ); } + #[test] + fn parse_v1_input_audio_transcription_delta_event() { + let payload = json!({ + "type": "conversation.item.input_audio_transcription.delta", + "item_id": "item_input_1", + "content_index": 0, + "delta": "hello" + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::V1), + Some(RealtimeEvent::InputTranscriptDelta( + RealtimeTranscriptDelta { + delta: "hello".to_string(), + } + )) + ); + } + + #[test] + fn parse_v1_input_audio_transcription_completed_event() { + let payload = json!({ + "type": "conversation.item.input_audio_transcription.completed", + "item_id": "item_input_1", + "content_index": 0, + "transcript": "hello world" + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::V1), + Some(RealtimeEvent::InputTranscriptDone(RealtimeTranscriptDone { + text: "hello world".to_string(), + })) + ); + } + #[test] fn parse_output_transcript_delta_event() { let payload = json!({ @@ -885,6 +1005,66 @@ mod tests { ); } + #[test] + fn parse_v1_output_audio_transcript_delta_event() { + let payload = json!({ + "type": "response.output_audio_transcript.delta", + "delta": "hi" + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::V1), + Some(RealtimeEvent::OutputTranscriptDelta( + RealtimeTranscriptDelta { + delta: "hi".to_string(), + } + )) + ); + } + + #[test] + fn parse_v1_output_audio_transcript_done_event() { + let payload = json!({ + "type": "response.output_audio_transcript.done", + "transcript": "hi there" + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::V1), + Some(RealtimeEvent::OutputTranscriptDone( + RealtimeTranscriptDone { + text: "hi there".to_string(), + } + )) + ); + } + + #[test] + fn parse_v1_item_done_output_text_event() { + let payload = json!({ + "type": "conversation.item.done", + "item": { + "id": "item_output_1", + "type": "message", + "role": "assistant", + "content": [ + {"type": "output_text", "text": "hello"}, + {"type": "output_text", "text": " world"} + ] + } + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::V1), + Some(RealtimeEvent::ConversationItemDone { + item_id: "item_output_1".to_string(), + }) + ); + } + #[test] fn parse_realtime_v2_handoff_tool_call_event() { let payload = json!({ @@ -930,6 +1110,58 @@ mod tests { ); } + #[test] + fn parse_realtime_v2_output_audio_transcript_done_event() { + let payload = json!({ + "type": "response.output_audio_transcript.done", + "transcript": "hello there" + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::RealtimeV2), + Some(RealtimeEvent::OutputTranscriptDone( + RealtimeTranscriptDone { + text: "hello there".to_string(), + } + )) + ); + } + + #[test] + fn parse_realtime_v2_output_text_done_event() { + let payload = json!({ + "type": "response.output_text.done", + "text": "hello there" + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::RealtimeV2), + Some(RealtimeEvent::OutputTranscriptDone( + RealtimeTranscriptDone { + text: "hello there".to_string(), + } + )) + ); + } + + #[test] + fn parse_realtime_v2_conversation_item_created_event() { + let payload = json!({ + "type": "conversation.item.created", + "item": {"type": "message", "role": "user"} + }) + .to_string(); + + assert_eq!( + parse_realtime_event(payload.as_str(), RealtimeEventParser::RealtimeV2), + Some(RealtimeEvent::ConversationItemAdded( + json!({"type": "message", "role": "user"}) + )) + ); + } + #[test] fn parse_realtime_v2_item_done_output_text_event() { let payload = json!({ @@ -948,11 +1180,9 @@ mod tests { assert_eq!( parse_realtime_event(payload.as_str(), RealtimeEventParser::RealtimeV2), - Some(RealtimeEvent::OutputTranscriptDone( - RealtimeTranscriptDone { - text: "hello world".to_string(), - } - )) + Some(RealtimeEvent::ConversationItemDone { + item_id: "item_output_1".to_string(), + }) ); } @@ -1582,6 +1812,12 @@ mod tests { "type": "near_field", }) ); + assert_eq!( + first_json["session"]["audio"]["input"]["transcription"], + json!({ + "model": "gpt-4o-mini-transcribe", + }) + ); assert_eq!( first_json["session"]["audio"]["input"]["turn_detection"], json!({ @@ -1756,6 +1992,12 @@ mod tests { Value::String("transcription".to_string()) ); assert!(first_json["session"].get("instructions").is_none()); + assert_eq!( + first_json["session"]["audio"]["input"]["transcription"], + json!({ + "model": "gpt-4o-mini-transcribe", + }) + ); assert!(first_json["session"]["audio"].get("output").is_none()); assert!(first_json["session"].get("tools").is_none()); diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v1.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v1.rs index 22e728dcff..19e4fa203a 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v1.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v1.rs @@ -55,6 +55,7 @@ pub(super) fn session_update_session( rate: REALTIME_AUDIO_SAMPLE_RATE, }, noise_reduction: None, + transcription: None, turn_detection: None, }, output: Some(SessionAudioOutput { diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v2.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v2.rs index 32f23a970b..b1f50b02be 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v2.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods_v2.rs @@ -18,6 +18,7 @@ use crate::endpoint::realtime_websocket::protocol::SessionAudioInput; use crate::endpoint::realtime_websocket::protocol::SessionAudioOutput; use crate::endpoint::realtime_websocket::protocol::SessionAudioOutputFormat; use crate::endpoint::realtime_websocket::protocol::SessionFunctionTool; +use crate::endpoint::realtime_websocket::protocol::SessionInputAudioTranscription; use crate::endpoint::realtime_websocket::protocol::SessionNoiseReduction; use crate::endpoint::realtime_websocket::protocol::SessionToolType; use crate::endpoint::realtime_websocket::protocol::SessionTurnDetection; @@ -31,6 +32,7 @@ const REALTIME_V2_OUTPUT_MODALITY_TEXT: &str = "text"; const REALTIME_V2_TOOL_CHOICE: &str = "auto"; const REALTIME_V2_BACKGROUND_AGENT_TOOL_NAME: &str = "background_agent"; const REALTIME_V2_BACKGROUND_AGENT_TOOL_DESCRIPTION: &str = "Send a user request to the background agent. Use this as the default action. Do not rephrase the user's ask or rewrite it in your own words; pass along the user's own words. If the background agent is idle, this starts a new task and returns the final result to the user. If the background agent is already working on a task, this sends the request as guidance to steer that previous task. If the user asks to do something next, later, after this, or once current work finishes, call this tool so the work is actually queued instead of merely promising to do it later."; +const REALTIME_V2_INPUT_TRANSCRIPTION_MODEL: &str = "gpt-4o-mini-transcribe"; pub(super) fn conversation_item_create_message(text: String) -> RealtimeOutboundMessage { RealtimeOutboundMessage::ConversationItemCreate { @@ -80,6 +82,9 @@ pub(super) fn session_update_session( noise_reduction: Some(SessionNoiseReduction { r#type: NoiseReductionType::NearField, }), + transcription: Some(SessionInputAudioTranscription { + model: REALTIME_V2_INPUT_TRANSCRIPTION_MODEL.to_string(), + }), turn_detection: Some(SessionTurnDetection { r#type: TurnDetectionType::ServerVad, interrupt_response: true, @@ -126,6 +131,9 @@ pub(super) fn session_update_session( rate: REALTIME_AUDIO_SAMPLE_RATE, }, noise_reduction: None, + transcription: Some(SessionInputAudioTranscription { + model: REALTIME_V2_INPUT_TRANSCRIPTION_MODEL.to_string(), + }), turn_detection: None, }, output: None, diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol.rs index 8a88cc34d1..d689f6ea96 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol.rs @@ -89,9 +89,16 @@ pub(super) struct SessionAudioInput { #[serde(skip_serializing_if = "Option::is_none")] pub(super) noise_reduction: Option, #[serde(skip_serializing_if = "Option::is_none")] + pub(super) transcription: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub(super) turn_detection: Option, } +#[derive(Debug, Clone, Serialize)] +pub(super) struct SessionInputAudioTranscription { + pub(super) model: String, +} + #[derive(Debug, Clone, Serialize)] pub(super) struct SessionAudioFormat { #[serde(rename = "type")] diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v1.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v1.rs index b66cf2b24b..3c1d25aed7 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v1.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v1.rs @@ -2,6 +2,7 @@ use crate::endpoint::realtime_websocket::protocol_common::parse_error_event; use crate::endpoint::realtime_websocket::protocol_common::parse_realtime_payload; use crate::endpoint::realtime_websocket::protocol_common::parse_session_updated_event; use crate::endpoint::realtime_websocket::protocol_common::parse_transcript_delta_event; +use crate::endpoint::realtime_websocket::protocol_common::parse_transcript_done_event; use codex_protocol::protocol::RealtimeAudioFrame; use codex_protocol::protocol::RealtimeEvent; use codex_protocol::protocol::RealtimeHandoffRequested; @@ -38,23 +39,28 @@ pub(super) fn parse_realtime_event_v1(payload: &str) -> Option { item_id: None, })) } - "conversation.input_transcript.delta" => { + "conversation.input_transcript.delta" + | "conversation.item.input_audio_transcription.delta" => { parse_transcript_delta_event(&parsed, "delta").map(RealtimeEvent::InputTranscriptDelta) } - "conversation.output_transcript.delta" => { + "conversation.item.input_audio_transcription.completed" => { + parse_transcript_done_event(&parsed, "transcript") + .map(RealtimeEvent::InputTranscriptDone) + } + "conversation.output_transcript.delta" + | "response.output_text.delta" + | "response.output_audio_transcript.delta" => { parse_transcript_delta_event(&parsed, "delta").map(RealtimeEvent::OutputTranscriptDelta) } + "response.output_audio_transcript.done" => { + parse_transcript_done_event(&parsed, "transcript") + .map(RealtimeEvent::OutputTranscriptDone) + } "conversation.item.added" => parsed .get("item") .cloned() .map(RealtimeEvent::ConversationItemAdded), - "conversation.item.done" => parsed - .get("item") - .and_then(Value::as_object) - .and_then(|item| item.get("id")) - .and_then(Value::as_str) - .map(str::to_string) - .map(|item_id| RealtimeEvent::ConversationItemDone { item_id }), + "conversation.item.done" => parse_conversation_item_done_event(&parsed), "conversation.handoff.requested" => { let handoff_id = parsed .get("handoff_id") @@ -82,3 +88,11 @@ pub(super) fn parse_realtime_event_v1(payload: &str) -> Option { } } } + +fn parse_conversation_item_done_event(parsed: &Value) -> Option { + let item = parsed.get("item")?.as_object()?; + item.get("id") + .and_then(Value::as_str) + .map(str::to_string) + .map(|item_id| RealtimeEvent::ConversationItemDone { item_id }) +} diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v2.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v2.rs index 559e83426b..9aa535757c 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v2.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/protocol_v2.rs @@ -10,7 +10,6 @@ use codex_protocol::protocol::RealtimeInputAudioSpeechStarted; use codex_protocol::protocol::RealtimeResponseCancelled; use codex_protocol::protocol::RealtimeResponseCreated; use codex_protocol::protocol::RealtimeResponseDone; -use codex_protocol::protocol::RealtimeTranscriptDone; use serde_json::Map as JsonMap; use serde_json::Value; use tracing::debug; @@ -38,6 +37,13 @@ pub(super) fn parse_realtime_event_v2(payload: &str) -> Option { "response.output_text.delta" | "response.output_audio_transcript.delta" => { parse_transcript_delta_event(&parsed, "delta").map(RealtimeEvent::OutputTranscriptDelta) } + "response.output_text.done" => { + parse_transcript_done_event(&parsed, "text").map(RealtimeEvent::OutputTranscriptDone) + } + "response.output_audio_transcript.done" => { + parse_transcript_done_event(&parsed, "transcript") + .map(RealtimeEvent::OutputTranscriptDone) + } "input_audio_buffer.speech_started" => Some(RealtimeEvent::InputAudioSpeechStarted( RealtimeInputAudioSpeechStarted { item_id: parsed @@ -46,7 +52,7 @@ pub(super) fn parse_realtime_event_v2(payload: &str) -> Option { .map(str::to_string), }, )), - "conversation.item.added" => parsed + "conversation.item.added" | "conversation.item.created" => parsed .get("item") .cloned() .map(RealtimeEvent::ConversationItemAdded), @@ -122,43 +128,12 @@ fn parse_conversation_item_done_event(parsed: &Value) -> Option { return Some(handoff); } - if let Some(transcript_done) = parse_item_done_transcript(item) { - return Some(transcript_done); - } - item.get("id") .and_then(Value::as_str) .map(str::to_string) .map(|item_id| RealtimeEvent::ConversationItemDone { item_id }) } -fn parse_item_done_transcript(item: &JsonMap) -> Option { - let role = item.get("role").and_then(Value::as_str)?; - let text = item - .get("content") - .and_then(Value::as_array)? - .iter() - .filter_map(item_content_text) - .collect::(); - if text.is_empty() { - return None; - } - - let done = RealtimeTranscriptDone { text }; - match role { - "user" => Some(RealtimeEvent::InputTranscriptDone(done)), - "assistant" => Some(RealtimeEvent::OutputTranscriptDone(done)), - _ => None, - } -} - -fn item_content_text(content: &Value) -> Option<&str> { - content - .get("text") - .or_else(|| content.get("transcript")) - .and_then(Value::as_str) -} - fn parse_handoff_requested_event(item: &JsonMap) -> Option { let item_type = item.get("type").and_then(Value::as_str); let item_name = item.get("name").and_then(Value::as_str); diff --git a/codex-rs/codex-api/tests/realtime_websocket_e2e.rs b/codex-rs/codex-api/tests/realtime_websocket_e2e.rs index abafaef2ae..2f38d0abcc 100644 --- a/codex-rs/codex-api/tests/realtime_websocket_e2e.rs +++ b/codex-rs/codex-api/tests/realtime_websocket_e2e.rs @@ -12,6 +12,9 @@ use codex_api::RealtimeSessionMode; use codex_api::RealtimeWebsocketClient; use codex_api::RetryConfig; use codex_protocol::protocol::RealtimeHandoffRequested; +use codex_protocol::protocol::RealtimeTranscriptDelta; +use codex_protocol::protocol::RealtimeTranscriptDone; +use codex_protocol::protocol::RealtimeTranscriptEntry; use codex_protocol::protocol::RealtimeVoice; use futures::SinkExt; use futures::StreamExt; @@ -493,6 +496,46 @@ async fn realtime_ws_e2e_realtime_v2_parser_emits_handoff_requested() { let first_json: Value = serde_json::from_str(&first).expect("json"); assert_eq!(first_json["type"], "session.update"); + ws.send(Message::Text( + json!({ + "type": "conversation.item.input_audio_transcription.completed", + "transcript": "delegate now" + }) + .to_string() + .into(), + )) + .await + .expect("send input transcript"); + + ws.send(Message::Text( + json!({ + "type": "response.output_audio_transcript.delta", + "delta": "secret context" + }) + .to_string() + .into(), + )) + .await + .expect("send output transcript"); + + ws.send(Message::Text( + json!({ + "type": "conversation.item.created", + "item": { + "type": "message", + "role": "user", + "content": [{ + "type": "input_text", + "text": "silent_delegate" + }] + } + }) + .to_string() + .into(), + )) + .await + .expect("send control item echo"); + ws.send(Message::Text( json!({ "type": "conversation.item.done", @@ -530,6 +573,37 @@ async fn realtime_ws_e2e_realtime_v2_parser_emits_handoff_requested() { .await .expect("connect"); + let event = connection + .next_event() + .await + .expect("next event") + .expect("event"); + assert_eq!( + event, + RealtimeEvent::InputTranscriptDone(RealtimeTranscriptDone { + text: "delegate now".to_string() + }) + ); + + let event = connection + .next_event() + .await + .expect("next event") + .expect("event"); + assert_eq!( + event, + RealtimeEvent::OutputTranscriptDelta(RealtimeTranscriptDelta { + delta: "secret context".to_string() + }) + ); + + let event = connection + .next_event() + .await + .expect("next event") + .expect("event"); + assert!(matches!(event, RealtimeEvent::ConversationItemAdded(_))); + let event = connection .next_event() .await @@ -541,7 +615,16 @@ async fn realtime_ws_e2e_realtime_v2_parser_emits_handoff_requested() { handoff_id: "call_123".to_string(), item_id: "item_123".to_string(), input_transcript: "delegate now".to_string(), - active_transcript: Vec::new(), + active_transcript: vec![ + RealtimeTranscriptEntry { + role: "user".to_string(), + text: "delegate now".to_string(), + }, + RealtimeTranscriptEntry { + role: "assistant".to_string(), + text: "secret context".to_string(), + }, + ], }) ); diff --git a/codex-rs/core/src/realtime_conversation.rs b/codex-rs/core/src/realtime_conversation.rs index 63b0e1191f..662f7a590f 100644 --- a/codex-rs/core/src/realtime_conversation.rs +++ b/codex-rs/core/src/realtime_conversation.rs @@ -831,16 +831,14 @@ async fn handle_start_inner( } let maybe_routed_text = match &event { RealtimeEvent::HandoffRequested(handoff) => { - realtime_text_from_handoff_request(handoff) + realtime_delegation_from_handoff(handoff) } _ => None, }; if let Some(text) = maybe_routed_text { debug!(text = %text, "[realtime-text] realtime conversation text output"); let sess_for_routed_text = Arc::clone(&sess_clone); - sess_for_routed_text - .route_realtime_text_input(wrap_realtime_delegation_input(&text)) - .await; + sess_for_routed_text.route_realtime_text_input(text).await; } if !fanout_realtime_active.load(Ordering::Relaxed) { break; @@ -890,23 +888,40 @@ pub(crate) async fn handle_audio( } } -fn realtime_text_from_handoff_request(handoff: &RealtimeHandoffRequested) -> Option { +fn realtime_transcript_delta_from_handoff(handoff: &RealtimeHandoffRequested) -> Option { let active_transcript = handoff .active_transcript .iter() .map(|entry| format!("{role}: {text}", role = entry.role, text = entry.text)) .collect::>() .join("\n"); - (!active_transcript.is_empty()) - .then_some(active_transcript) - .or((!handoff.input_transcript.is_empty()).then_some(handoff.input_transcript.clone())) + (!active_transcript.is_empty()).then_some(active_transcript) } -fn wrap_realtime_delegation_input(input: &str) -> String { - format!( - "\n {}\n", - escape_xml_text(input) - ) +fn realtime_text_from_handoff_request(handoff: &RealtimeHandoffRequested) -> Option { + (!handoff.input_transcript.is_empty()) + .then_some(handoff.input_transcript.clone()) + .or_else(|| realtime_transcript_delta_from_handoff(handoff)) +} + +fn realtime_delegation_from_handoff(handoff: &RealtimeHandoffRequested) -> Option { + let input = realtime_text_from_handoff_request(handoff)?; + Some(wrap_realtime_delegation_input( + &input, + realtime_transcript_delta_from_handoff(handoff).as_deref(), + )) +} + +fn wrap_realtime_delegation_input(input: &str, transcript_delta: Option<&str>) -> String { + let input = escape_xml_text(input); + if let Some(transcript_delta) = transcript_delta.filter(|text| !text.is_empty()) { + let transcript_delta = escape_xml_text(transcript_delta); + return format!( + "\n {input}\n {transcript_delta}\n" + ); + } + + format!("\n {input}\n") } fn escape_xml_text(input: &str) -> String { diff --git a/codex-rs/core/src/realtime_conversation_tests.rs b/codex-rs/core/src/realtime_conversation_tests.rs index cda130e182..a146c43869 100644 --- a/codex-rs/core/src/realtime_conversation_tests.rs +++ b/codex-rs/core/src/realtime_conversation_tests.rs @@ -1,5 +1,6 @@ use super::RealtimeHandoffState; use super::RealtimeSessionKind; +use super::realtime_delegation_from_handoff; use super::realtime_text_from_handoff_request; use super::wrap_realtime_delegation_input; use async_channel::bounded; @@ -8,7 +9,7 @@ use codex_protocol::protocol::RealtimeTranscriptEntry; use pretty_assertions::assert_eq; #[test] -fn extracts_text_from_handoff_request_active_transcript() { +fn prefers_handoff_input_transcript_over_active_transcript() { let handoff = RealtimeHandoffRequested { handoff_id: "handoff_1".to_string(), item_id: "item_1".to_string(), @@ -26,7 +27,50 @@ fn extracts_text_from_handoff_request_active_transcript() { }; assert_eq!( realtime_text_from_handoff_request(&handoff), - Some("user: hello\nassistant: hi there".to_string()) + Some("ignored".to_string()) + ); +} + +#[test] +fn extracts_text_from_handoff_request_active_transcript_if_input_missing() { + let handoff = RealtimeHandoffRequested { + handoff_id: "handoff_1".to_string(), + item_id: "item_1".to_string(), + input_transcript: String::new(), + active_transcript: vec![RealtimeTranscriptEntry { + role: "user".to_string(), + text: "hello".to_string(), + }], + }; + assert_eq!( + realtime_text_from_handoff_request(&handoff), + Some("user: hello".to_string()) + ); +} + +#[test] +fn wraps_handoff_with_transcript_delta() { + let handoff = RealtimeHandoffRequested { + handoff_id: "handoff_1".to_string(), + item_id: "item_1".to_string(), + input_transcript: "delegate this".to_string(), + active_transcript: vec![ + RealtimeTranscriptEntry { + role: "user".to_string(), + text: "hello".to_string(), + }, + RealtimeTranscriptEntry { + role: "assistant".to_string(), + text: "hi there".to_string(), + }, + ], + }; + assert_eq!( + realtime_delegation_from_handoff(&handoff), + Some( + "\n delegate this\n user: hello\nassistant: hi there\n" + .to_string() + ) ); } @@ -58,7 +102,7 @@ fn ignores_empty_handoff_request_input_transcript() { #[test] fn wraps_realtime_delegation_input() { assert_eq!( - wrap_realtime_delegation_input("hello"), + wrap_realtime_delegation_input("hello", /*transcript_delta*/ None), "\n hello\n" ); } @@ -66,7 +110,15 @@ fn wraps_realtime_delegation_input() { #[test] fn wraps_realtime_delegation_input_with_xml_escaping() { assert_eq!( - wrap_realtime_delegation_input("use a < b && c > d"), + wrap_realtime_delegation_input("use a < b && c > d", Some("saw ")), + "\n use a < b && c > d\n saw <that>\n" + ); +} + +#[test] +fn wraps_realtime_delegation_input_with_xml_escaping_without_transcript() { + assert_eq!( + wrap_realtime_delegation_input("use a < b && c > d", /*transcript_delta*/ None), "\n use a < b && c > d\n" ); } diff --git a/codex-rs/core/tests/suite/realtime_conversation.rs b/codex-rs/core/tests/suite/realtime_conversation.rs index d92d72b821..0d6e130564 100644 --- a/codex-rs/core/tests/suite/realtime_conversation.rs +++ b/codex-rs/core/tests/suite/realtime_conversation.rs @@ -2480,7 +2480,7 @@ async fn inbound_handoff_request_starts_turn() -> Result<()> { let request = response_mock.single_request(); let user_texts = request.message_input_texts("user"); assert!(user_texts.iter().any(|text| text - == "\n user: text from realtime\n")); + == "\n text from realtime\n user: text from realtime\n")); realtime_server.shutdown().await; Ok(()) @@ -2562,14 +2562,14 @@ async fn inbound_handoff_request_uses_active_transcript() -> Result<()> { let request = response_mock.single_request(); let user_texts = request.message_input_texts("user"); assert!(user_texts.iter().any(|text| text - == "\n assistant: assistant context\nuser: delegated query\nassistant: assist confirm\n")); + == "\n ignored\n assistant: assistant context\nuser: delegated query\nassistant: assist confirm\nuser: ignored\n")); realtime_server.shutdown().await; Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn inbound_handoff_request_clears_active_transcript_after_each_handoff() -> Result<()> { +async fn inbound_handoff_request_keeps_transcript_history_after_each_handoff() -> Result<()> { skip_if_no_network!(Ok(())); let api_server = start_mock_server().await; @@ -2677,13 +2677,13 @@ async fn inbound_handoff_request_clears_active_transcript_after_each_handoff() - let first_user_texts = requests[0].message_input_texts("user"); assert!(first_user_texts.iter().any(|text| text - == "\n user: first question\n")); + == "\n first question\n user: first question\n")); let second_user_texts = requests[1].message_input_texts("user"); assert!(second_user_texts.iter().any(|text| text - == "\n user: second question\n")); + == "\n second question\n user: first question\nuser: second question\n")); assert!(!second_user_texts.iter().any(|text| text - == "\n user: first question\nuser: second question\n")); + == "\n second question\n user: first questionsecond question\n")); realtime_server.shutdown().await; Ok(()) @@ -3207,11 +3207,11 @@ async fn inbound_handoff_request_steers_active_turn() -> Result<()> { !first_texts .iter() .any(|text| text - == "\n user: steer via realtime\n") + == "\n steer via realtime\n user: steer via realtime\n") ); assert!(second_texts.iter().any(|text| text == "first prompt")); assert!(second_texts.iter().any(|text| text - == "\n user: steer via realtime\n")); + == "\n steer via realtime\n user: steer via realtime\n")); realtime_server.shutdown().await; api_server.shutdown().await; @@ -3327,7 +3327,7 @@ async fn inbound_handoff_request_starts_turn_and_does_not_block_realtime_audio() let first_body: Value = serde_json::from_slice(&requests[0]).expect("parse first request"); let first_texts = message_input_texts(&first_body, "user"); let expected_text = format!( - "\n user: {delegated_text}\n" + "\n {delegated_text}\n user: {delegated_text}\n" ); assert!(first_texts.iter().any(|text| text == &expected_text)); From baa5dd7b29ac03224a04dfb8b71481ae12b8e4c5 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 20 Apr 2026 14:43:04 -0700 Subject: [PATCH 3/7] Surface TUI skills refresh failures (#18627) ## Why `skills/list` refreshes are best-effort metadata updates. If one fails during startup or thread switching, the TUI should keep running and show enough detail to diagnose the app-server failure instead of leaving the user with only a log entry. This addresses the recoverability and observability issue reported in #16914. ## What Changed - Preserve the full startup `skills/list` error chain before sending it back through the app event queue. - Surface failed skills refreshes as recoverable TUI error messages while still logging the warning. This is related to the recent bug fix from [PR #18370](https://github.com/openai/codex/pull/18370). --- codex-rs/tui/src/app.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index fe14c36630..eeba62de00 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -2212,7 +2212,7 @@ impl App { tokio::spawn(async move { let result = fetch_skills_list(request_handle, cwd) .await - .map_err(|err| err.to_string()); + .map_err(|err| format!("{err:#}")); app_event_tx.send(AppEvent::SkillsListLoaded { result }); }); } @@ -2795,6 +2795,8 @@ impl App { Ok(response) => self.handle_skills_list_response(response), Err(err) => { tracing::warn!("{failure_message}: {err:#}"); + self.chat_widget + .add_error_message(format!("{failure_message}: {err:#}")); } } } From cc96a03f105ae6af03f1593a167dd7dd8c9d7b47 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 20 Apr 2026 14:52:30 -0700 Subject: [PATCH 4/7] Fix stale model test fixtures (#18719) Fixes stale test fixtures left after the active bundled model catalog updates in #18586 and #18388. Those changes made `gpt-5.4` the current default and removed several older hardcoded slugs, which left Windows Bazel shards failing TUI and config tests. What changed: - Refresh TUI model migration, availability NUX, plan-mode, status, and snapshot fixtures to use active bundled model slugs. - Update the config edit test expectation for the TOML-quoted `"gpt-5.2"` migration key. - Move the model catalog tests into `codex-rs/tui/src/app/tests/model_catalog.rs` so touching them does not trip the blob-size policy for `app.rs`. Verification: - CI Bazel/lint checks are expected to cover the affected test shards. --- codex-rs/core/src/config/edit_tests.rs | 2 +- codex-rs/tui/src/app.rs | 310 +----------------- codex-rs/tui/src/app/tests/model_catalog.rs | 293 +++++++++++++++++ ...gration_prompt_shows_for_hidden_model.snap | 12 + ..._review_denied_renders_denied_request.snap | 2 +- ...w_timed_out_renders_timed_out_request.snap | 2 +- ...artup_failure_renders_warning_history.snap | 2 +- ...i__chatwidget__tests__chatwidget_tall.snap | 2 +- ...compact_queues_user_messages_snapshot.snap | 2 +- ...pproved_exec_renders_approved_request.snap | 2 +- ...ec_renders_warning_and_denied_request.snap | 2 +- ...allel_reviews_render_aggregate_status.snap | 2 +- ...renders_warning_and_timed_out_request.snap | 2 +- ...et__tests__mcp_startup_header_booting.snap | 2 +- ...ests__model_reasoning_selection_popup.snap | 2 +- ...ng_selection_popup_extra_high_warning.snap | 16 +- ...twidget__tests__model_selection_popup.snap | 16 +- ..._tests__preamble_keeps_working_status.snap | 2 +- ...tests__rate_limit_switch_prompt_popup.snap | 6 +- ..._review_queues_user_messages_snapshot.snap | 2 +- ...e_context_label_preserves_status_line.snap | 2 +- ...ide_context_label_shows_parent_status.snap | 2 +- ...rked_side_question_while_task_running.snap | 2 +- ...atwidget__tests__status_widget_active.snap | 2 +- ...ed_exec_begin_restores_working_status.snap | 2 +- ...renders_command_in_single_details_row.snap | 2 +- .../tui/src/chatwidget/tests/plan_mode.rs | 87 +++-- .../chatwidget/tests/popups_and_settings.rs | 34 +- .../src/chatwidget/tests/slash_commands.rs | 4 +- .../chatwidget/tests/status_command_tests.rs | 4 +- ...gration_prompt_shows_for_hidden_model.snap | 11 - 31 files changed, 405 insertions(+), 428 deletions(-) create mode 100644 codex-rs/tui/src/app/tests/model_catalog.rs create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_catalog__model_migration_prompt_shows_for_hidden_model.snap delete mode 100644 codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index d2efb21886..ec81c7c06d 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -547,7 +547,7 @@ existing = "value" existing = "value" [notice.model_migrations] -gpt-5 = "gpt-5.4" +"gpt-5.2" = "gpt-5.4" "#; assert_eq!(contents, expected); } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index eeba62de00..dc8ca2784c 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -7114,7 +7114,6 @@ mod tests { use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as AppServerUserInput; use codex_app_server_protocol::WarningNotification; - use codex_config::types::ModelAvailabilityNuxConfig; use codex_otel::SessionTelemetry; use codex_protocol::ThreadId; use codex_protocol::config_types::CollaborationMode; @@ -7125,7 +7124,6 @@ mod tests { use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; - use codex_protocol::openai_models::ModelAvailabilityNux; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; @@ -7152,6 +7150,8 @@ mod tests { use tempfile::tempdir; use tokio::time; + mod model_catalog; + fn test_absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::try_from(PathBuf::from(path)).expect("absolute test path") } @@ -11474,135 +11474,6 @@ guardian_approval = true .and_then(TomlValue::as_bool) } - fn all_model_presets() -> Vec { - crate::legacy_core::test_support::all_model_presets().clone() - } - - fn model_availability_nux_config(shown_count: &[(&str, u32)]) -> ModelAvailabilityNuxConfig { - ModelAvailabilityNuxConfig { - shown_count: shown_count - .iter() - .map(|(model, count)| ((*model).to_string(), *count)) - .collect(), - } - } - - fn model_migration_copy_to_plain_text( - copy: &crate::model_migration::ModelMigrationCopy, - ) -> String { - if let Some(markdown) = copy.markdown.as_ref() { - return markdown.clone(); - } - let mut s = String::new(); - for span in ©.heading { - s.push_str(&span.content); - } - s.push('\n'); - s.push('\n'); - for line in ©.content { - for span in &line.spans { - s.push_str(&span.content); - } - s.push('\n'); - } - s - } - - #[tokio::test] - async fn model_migration_prompt_only_shows_for_deprecated_models() { - let seen = BTreeMap::new(); - assert!(should_show_model_migration_prompt( - "gpt-5", - "gpt-5.2-codex", - &seen, - &all_model_presets() - )); - assert!(should_show_model_migration_prompt( - "gpt-5-codex", - "gpt-5.2-codex", - &seen, - &all_model_presets() - )); - assert!(should_show_model_migration_prompt( - "gpt-5-codex-mini", - "gpt-5.2-codex", - &seen, - &all_model_presets() - )); - assert!(should_show_model_migration_prompt( - "gpt-5.1-codex", - "gpt-5.2-codex", - &seen, - &all_model_presets() - )); - assert!(!should_show_model_migration_prompt( - "gpt-5.1-codex", - "gpt-5.1-codex", - &seen, - &all_model_presets() - )); - } - - #[test] - fn select_model_availability_nux_picks_only_eligible_model() { - let mut presets = all_model_presets(); - presets.iter_mut().for_each(|preset| { - preset.availability_nux = None; - }); - let target = presets - .iter_mut() - .find(|preset| preset.model == "gpt-5") - .expect("target preset present"); - target.availability_nux = Some(ModelAvailabilityNux { - message: "gpt-5 is available".to_string(), - }); - - let selected = select_model_availability_nux(&presets, &model_availability_nux_config(&[])); - - assert_eq!( - selected, - Some(StartupTooltipOverride { - model_slug: "gpt-5".to_string(), - message: "gpt-5 is available".to_string(), - }) - ); - } - - #[test] - fn select_model_availability_nux_skips_missing_and_exhausted_models() { - let mut presets = all_model_presets(); - presets.iter_mut().for_each(|preset| { - preset.availability_nux = None; - }); - let gpt_5 = presets - .iter_mut() - .find(|preset| preset.model == "gpt-5") - .expect("gpt-5 preset present"); - gpt_5.availability_nux = Some(ModelAvailabilityNux { - message: "gpt-5 is available".to_string(), - }); - let gpt_5_2 = presets - .iter_mut() - .find(|preset| preset.model == "gpt-5.2") - .expect("gpt-5.2 preset present"); - gpt_5_2.availability_nux = Some(ModelAvailabilityNux { - message: "gpt-5.2 is available".to_string(), - }); - - let selected = select_model_availability_nux( - &presets, - &model_availability_nux_config(&[("gpt-5", MODEL_AVAILABILITY_NUX_MAX_SHOW_COUNT)]), - ); - - assert_eq!( - selected, - Some(StartupTooltipOverride { - model_slug: "gpt-5.2".to_string(), - message: "gpt-5.2 is available".to_string(), - }) - ); - } - #[test] fn active_turn_not_steerable_turn_error_extracts_structured_server_error() { let turn_error = AppServerTurnError { @@ -11665,183 +11536,6 @@ guardian_approval = true ); } - #[test] - fn select_model_availability_nux_uses_existing_model_order_as_priority() { - let mut presets = all_model_presets(); - presets.iter_mut().for_each(|preset| { - preset.availability_nux = None; - }); - let first = presets - .iter_mut() - .find(|preset| preset.model == "gpt-5") - .expect("gpt-5 preset present"); - first.availability_nux = Some(ModelAvailabilityNux { - message: "first".to_string(), - }); - let second = presets - .iter_mut() - .find(|preset| preset.model == "gpt-5.2") - .expect("gpt-5.2 preset present"); - second.availability_nux = Some(ModelAvailabilityNux { - message: "second".to_string(), - }); - - let selected = select_model_availability_nux(&presets, &model_availability_nux_config(&[])); - - assert_eq!( - selected, - Some(StartupTooltipOverride { - model_slug: "gpt-5.2".to_string(), - message: "second".to_string(), - }) - ); - } - - #[test] - fn select_model_availability_nux_returns_none_when_all_models_are_exhausted() { - let mut presets = all_model_presets(); - presets.iter_mut().for_each(|preset| { - preset.availability_nux = None; - }); - let target = presets - .iter_mut() - .find(|preset| preset.model == "gpt-5") - .expect("target preset present"); - target.availability_nux = Some(ModelAvailabilityNux { - message: "gpt-5 is available".to_string(), - }); - - let selected = select_model_availability_nux( - &presets, - &model_availability_nux_config(&[("gpt-5", MODEL_AVAILABILITY_NUX_MAX_SHOW_COUNT)]), - ); - - assert_eq!(selected, None); - } - - #[tokio::test] - async fn model_migration_prompt_respects_hide_flag_and_self_target() { - let mut seen = BTreeMap::new(); - seen.insert("gpt-5".to_string(), "gpt-5.1".to_string()); - assert!(!should_show_model_migration_prompt( - "gpt-5", - "gpt-5.1", - &seen, - &all_model_presets() - )); - assert!(!should_show_model_migration_prompt( - "gpt-5.1", - "gpt-5.1", - &seen, - &all_model_presets() - )); - } - - #[tokio::test] - async fn model_migration_prompt_skips_when_target_missing_or_hidden() { - let mut available = all_model_presets(); - let mut current = available - .iter() - .find(|preset| preset.model == "gpt-5-codex") - .cloned() - .expect("preset present"); - current.upgrade = Some(ModelUpgrade { - id: "missing-target".to_string(), - reasoning_effort_mapping: None, - migration_config_key: HIDE_GPT5_1_MIGRATION_PROMPT_CONFIG.to_string(), - model_link: None, - upgrade_copy: None, - migration_markdown: None, - }); - available.retain(|preset| preset.model != "gpt-5-codex"); - available.push(current.clone()); - - assert!(!should_show_model_migration_prompt( - ¤t.model, - "missing-target", - &BTreeMap::new(), - &available, - )); - - assert!(target_preset_for_upgrade(&available, "missing-target").is_none()); - - let mut with_hidden_target = all_model_presets(); - let target = with_hidden_target - .iter_mut() - .find(|preset| preset.model == "gpt-5.2-codex") - .expect("target preset present"); - target.show_in_picker = false; - - assert!(!should_show_model_migration_prompt( - "gpt-5-codex", - "gpt-5.2-codex", - &BTreeMap::new(), - &with_hidden_target, - )); - assert!(target_preset_for_upgrade(&with_hidden_target, "gpt-5.2-codex").is_none()); - } - - #[tokio::test] - async fn model_migration_prompt_shows_for_hidden_model() { - let codex_home = tempdir().expect("temp codex home"); - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .build() - .await - .expect("config"); - - let mut available_models = all_model_presets(); - let current = available_models - .iter() - .find(|preset| preset.model == "gpt-5.1-codex") - .cloned() - .expect("gpt-5.1-codex preset present"); - assert!( - !current.show_in_picker, - "expected gpt-5.1-codex to be hidden from picker for this test" - ); - - let upgrade = current.upgrade.as_ref().expect("upgrade configured"); - // Test "hidden current model still prompts" even if bundled - // catalog data changes the target model's picker visibility. - available_models - .iter_mut() - .find(|preset| preset.model == upgrade.id) - .expect("upgrade target present") - .show_in_picker = true; - assert!( - should_show_model_migration_prompt( - ¤t.model, - &upgrade.id, - &config.notices.model_migrations, - &available_models, - ), - "expected migration prompt to be eligible for hidden model" - ); - - let target = target_preset_for_upgrade(&available_models, &upgrade.id) - .expect("upgrade target present"); - let target_description = - (!target.description.is_empty()).then(|| target.description.clone()); - let can_opt_out = true; - let copy = migration_copy_for_models( - ¤t.model, - &upgrade.id, - upgrade.model_link.clone(), - upgrade.upgrade_copy.clone(), - upgrade.migration_markdown.clone(), - target.display_name.clone(), - target_description, - can_opt_out, - ); - - // Snapshot the copy we would show; rendering is covered by model_migration snapshots. - assert_snapshot!( - "model_migration_prompt_shows_for_hidden_model", - model_migration_copy_to_plain_text(©) - ); - } - #[tokio::test] async fn update_reasoning_effort_updates_collaboration_mode() { let mut app = make_test_app().await; diff --git a/codex-rs/tui/src/app/tests/model_catalog.rs b/codex-rs/tui/src/app/tests/model_catalog.rs new file mode 100644 index 0000000000..e74d01e6db --- /dev/null +++ b/codex-rs/tui/src/app/tests/model_catalog.rs @@ -0,0 +1,293 @@ +use super::*; +use codex_config::types::ModelAvailabilityNuxConfig; +use codex_protocol::openai_models::ModelAvailabilityNux; +use pretty_assertions::assert_eq; + +fn all_model_presets() -> Vec { + crate::legacy_core::test_support::all_model_presets().clone() +} + +fn model_availability_nux_config(shown_count: &[(&str, u32)]) -> ModelAvailabilityNuxConfig { + ModelAvailabilityNuxConfig { + shown_count: shown_count + .iter() + .map(|(model, count)| ((*model).to_string(), *count)) + .collect(), + } +} + +fn model_migration_copy_to_plain_text(copy: &crate::model_migration::ModelMigrationCopy) -> String { + if let Some(markdown) = copy.markdown.as_ref() { + return markdown.clone(); + } + let mut s = String::new(); + for span in ©.heading { + s.push_str(&span.content); + } + s.push('\n'); + s.push('\n'); + for line in ©.content { + for span in &line.spans { + s.push_str(&span.content); + } + s.push('\n'); + } + s +} + +#[tokio::test] +async fn model_migration_prompt_only_shows_for_deprecated_models() { + let seen = BTreeMap::new(); + assert!(should_show_model_migration_prompt( + "gpt-5.2", + "gpt-5.4", + &seen, + &all_model_presets() + )); + assert!(should_show_model_migration_prompt( + "gpt-5.3-codex", + "gpt-5.4", + &seen, + &all_model_presets() + )); + assert!(!should_show_model_migration_prompt( + "gpt-5.3-codex", + "gpt-5.3-codex", + &seen, + &all_model_presets() + )); +} + +#[test] +fn select_model_availability_nux_picks_only_eligible_model() { + let mut presets = all_model_presets(); + presets.iter_mut().for_each(|preset| { + preset.availability_nux = None; + }); + let target = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4") + .expect("target preset present"); + target.availability_nux = Some(ModelAvailabilityNux { + message: "gpt-5.4 is available".to_string(), + }); + + let selected = select_model_availability_nux(&presets, &model_availability_nux_config(&[])); + + assert_eq!( + selected, + Some(StartupTooltipOverride { + model_slug: "gpt-5.4".to_string(), + message: "gpt-5.4 is available".to_string(), + }) + ); +} + +#[test] +fn select_model_availability_nux_skips_missing_and_exhausted_models() { + let mut presets = all_model_presets(); + presets.iter_mut().for_each(|preset| { + preset.availability_nux = None; + }); + let gpt_5 = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4") + .expect("gpt-5.4 preset present"); + gpt_5.availability_nux = Some(ModelAvailabilityNux { + message: "gpt-5.4 is available".to_string(), + }); + let gpt_5_2 = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4-mini") + .expect("gpt-5.4-mini preset present"); + gpt_5_2.availability_nux = Some(ModelAvailabilityNux { + message: "gpt-5.4-mini is available".to_string(), + }); + + let selected = select_model_availability_nux( + &presets, + &model_availability_nux_config(&[("gpt-5.4", MODEL_AVAILABILITY_NUX_MAX_SHOW_COUNT)]), + ); + + assert_eq!( + selected, + Some(StartupTooltipOverride { + model_slug: "gpt-5.4-mini".to_string(), + message: "gpt-5.4-mini is available".to_string(), + }) + ); +} + +#[test] +fn select_model_availability_nux_uses_existing_model_order_as_priority() { + let mut presets = all_model_presets(); + presets.iter_mut().for_each(|preset| { + preset.availability_nux = None; + }); + let first = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4-mini") + .expect("gpt-5.4-mini preset present"); + first.availability_nux = Some(ModelAvailabilityNux { + message: "first".to_string(), + }); + let second = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4") + .expect("gpt-5.4 preset present"); + second.availability_nux = Some(ModelAvailabilityNux { + message: "second".to_string(), + }); + + let selected = select_model_availability_nux(&presets, &model_availability_nux_config(&[])); + + assert_eq!( + selected, + Some(StartupTooltipOverride { + model_slug: "gpt-5.4".to_string(), + message: "second".to_string(), + }) + ); +} + +#[test] +fn select_model_availability_nux_returns_none_when_all_models_are_exhausted() { + let mut presets = all_model_presets(); + presets.iter_mut().for_each(|preset| { + preset.availability_nux = None; + }); + let target = presets + .iter_mut() + .find(|preset| preset.model == "gpt-5.4") + .expect("target preset present"); + target.availability_nux = Some(ModelAvailabilityNux { + message: "gpt-5.4 is available".to_string(), + }); + + let selected = select_model_availability_nux( + &presets, + &model_availability_nux_config(&[("gpt-5.4", MODEL_AVAILABILITY_NUX_MAX_SHOW_COUNT)]), + ); + + assert_eq!(selected, None); +} + +#[tokio::test] +async fn model_migration_prompt_respects_hide_flag_and_self_target() { + let mut seen = BTreeMap::new(); + seen.insert("gpt-5.2".to_string(), "gpt-5.4".to_string()); + assert!(!should_show_model_migration_prompt( + "gpt-5.2", + "gpt-5.4", + &seen, + &all_model_presets() + )); + assert!(!should_show_model_migration_prompt( + "gpt-5.4", + "gpt-5.4", + &seen, + &all_model_presets() + )); +} + +#[tokio::test] +async fn model_migration_prompt_skips_when_target_missing_or_hidden() { + let mut available = all_model_presets(); + let mut current = available + .iter() + .find(|preset| preset.model == "gpt-5.2") + .cloned() + .expect("preset present"); + current.upgrade = Some(ModelUpgrade { + id: "missing-target".to_string(), + reasoning_effort_mapping: None, + migration_config_key: HIDE_GPT5_1_MIGRATION_PROMPT_CONFIG.to_string(), + model_link: None, + upgrade_copy: None, + migration_markdown: None, + }); + available.retain(|preset| preset.model != "gpt-5.2"); + available.push(current.clone()); + + assert!(!should_show_model_migration_prompt( + ¤t.model, + "missing-target", + &BTreeMap::new(), + &available, + )); + + assert!(target_preset_for_upgrade(&available, "missing-target").is_none()); + + let mut with_hidden_target = all_model_presets(); + let target = with_hidden_target + .iter_mut() + .find(|preset| preset.model == "gpt-5.4") + .expect("target preset present"); + target.show_in_picker = false; + + assert!(!should_show_model_migration_prompt( + "gpt-5.2", + "gpt-5.4", + &BTreeMap::new(), + &with_hidden_target, + )); + assert!(target_preset_for_upgrade(&with_hidden_target, "gpt-5.4").is_none()); +} + +#[tokio::test] +async fn model_migration_prompt_shows_for_hidden_model() { + let codex_home = tempdir().expect("temp codex home"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config"); + + let mut available_models = all_model_presets(); + let current = available_models + .iter_mut() + .find(|preset| preset.model == "gpt-5.3-codex") + .expect("gpt-5.3-codex preset present"); + current.show_in_picker = false; + let current = current.clone(); + assert!( + !current.show_in_picker, + "expected gpt-5.3-codex to be hidden from picker for this test" + ); + + let upgrade = current.upgrade.as_ref().expect("upgrade configured"); + available_models + .iter_mut() + .find(|preset| preset.model == upgrade.id) + .expect("upgrade target present") + .show_in_picker = true; + assert!( + should_show_model_migration_prompt( + ¤t.model, + &upgrade.id, + &config.notices.model_migrations, + &available_models, + ), + "expected migration prompt to be eligible for hidden model" + ); + + let target = + target_preset_for_upgrade(&available_models, &upgrade.id).expect("upgrade target present"); + let target_description = (!target.description.is_empty()).then(|| target.description.clone()); + let can_opt_out = true; + let copy = migration_copy_for_models( + ¤t.model, + &upgrade.id, + upgrade.model_link.clone(), + upgrade.upgrade_copy.clone(), + upgrade.migration_markdown.clone(), + target.display_name.clone(), + target_description, + can_opt_out, + ); + + assert_snapshot!( + "model_migration_prompt_shows_for_hidden_model", + model_migration_copy_to_plain_text(©) + ); +} diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_catalog__model_migration_prompt_shows_for_hidden_model.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_catalog__model_migration_prompt_shows_for_hidden_model.snap new file mode 100644 index 0000000000..c5be01e00e --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_catalog__model_migration_prompt_shows_for_hidden_model.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/app/tests/model_catalog.rs +assertion_line: 289 +expression: model_migration_copy_to_plain_text(©) +--- +Introducing GPT-5.4 + +Codex just got an upgrade with GPT-5.4, our most capable model for professional work. It outperforms prior models while being more token efficient, with notable improvements on long-running tasks, tool calling, computer use, and frontend development. + +Learn more: https://openai.com/index/introducing-gpt-5-4 + +You can always keep using GPT-5.3-Codex if you prefer. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_denied_renders_denied_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_denied_renders_denied_request.snap index f783bd4d6d..59d02ea1d2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_denied_renders_denied_request.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_denied_renders_denied_request.snap @@ -10,4 +10,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_timed_out_renders_timed_out_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_timed_out_renders_timed_out_request.snap index 2bd33f6359..becb20490b 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_timed_out_renders_timed_out_request.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_guardian_review_timed_out_renders_timed_out_request.snap @@ -17,4 +17,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap index fedd9d6bc2..eb21334162 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__app_server_mcp_startup_failure_renders_warning_history.snap @@ -8,4 +8,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap index a287d9d7a2..1ef31b7504 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap @@ -26,4 +26,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__compact_queues_user_messages_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__compact_queues_user_messages_snapshot.snap index d39932f3a6..5827486bf9 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__compact_queues_user_messages_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__compact_queues_user_messages_snapshot.snap @@ -9,4 +9,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap index 80592e0dd8..ed1d644cb3 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_approved_exec_renders_approved_request.snap @@ -8,4 +8,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_denied_exec_renders_warning_and_denied_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_denied_exec_renders_warning_and_denied_request.snap index d910d6d6f3..b8cd76c135 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_denied_exec_renders_warning_and_denied_request.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_denied_exec_renders_warning_and_denied_request.snap @@ -14,4 +14,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_parallel_reviews_render_aggregate_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_parallel_reviews_render_aggregate_status.snap index 821c8d1a6b..393e1a304e 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_parallel_reviews_render_aggregate_status.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_parallel_reviews_render_aggregate_status.snap @@ -9,4 +9,4 @@ expression: normalize_snapshot_paths(rendered) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_timed_out_exec_renders_warning_and_timed_out_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_timed_out_exec_renders_warning_and_timed_out_request.snap index dc3b546742..9d1eb96bf1 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_timed_out_exec_renders_warning_and_timed_out_request.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_timed_out_exec_renders_warning_and_timed_out_request.snap @@ -21,4 +21,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap index f3936540a1..8a5dd2046f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap @@ -8,4 +8,4 @@ expression: normalized_backend_snapshot(terminal.backend()) " " "› Ask Codex to do anything " " " -" gpt-5.3-codex default · /tmp/project " +" gpt-5.4 default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup.snap index a4a86a41ba..27c7d061ee 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup.snap @@ -2,7 +2,7 @@ source: tui/src/chatwidget/tests.rs expression: popup --- - Select Reasoning Level for gpt-5.1-codex-max + Select Reasoning Level for gpt-5.4 1. Low Fast responses with lighter reasoning 2. Medium (default) Balances speed and reasoning depth for everyday tasks diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup_extra_high_warning.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup_extra_high_warning.snap index 2404dced5d..7cde99f3a9 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup_extra_high_warning.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup_extra_high_warning.snap @@ -1,14 +1,16 @@ --- -source: tui/src/chatwidget/tests.rs +source: tui/src/chatwidget/tests/popups_and_settings.rs expression: popup --- - Select Reasoning Level for gpt-5.1-codex-max + Select Reasoning Level for gpt-5.2 - 1. Low Fast responses with lighter reasoning - 2. Medium (default) Balances speed and reasoning depth for everyday - tasks - 3. High Greater reasoning depth for complex problems -› 4. Extra high (current) Extra high reasoning depth for complex problems + 1. Low Balances speed with some reasoning; useful for + straightforward queries and short explanations + 2. Medium (default) Provides a solid balance of reasoning depth and + latency for general-purpose tasks + 3. High Maximizes reasoning depth for complex or ambiguous + problems +› 4. Extra high (current) Extra high reasoning for complex problems ⚠ Extra high reasoning effort can quickly consume Plus plan rate limits. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_selection_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_selection_popup.snap index d322bf35ed..41773729e0 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_selection_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_selection_popup.snap @@ -1,18 +1,14 @@ --- -source: tui/src/chatwidget/tests.rs +source: tui/src/chatwidget/tests/popups_and_settings.rs expression: popup --- Select Model and Effort Access legacy models by running codex -m or in your config.toml -› 1. gpt-5.3-codex (default) Latest frontier agentic coding model. - 2. gpt-5.4 Latest frontier agentic coding model. - 3. gpt-5.2-codex Frontier agentic coding model. - 4. gpt-5.1-codex-max Codex-optimized flagship for deep and fast - reasoning. - 5. gpt-5.2 Latest frontier model with improvements across - knowledge, reasoning and coding - 6. gpt-5.1-codex-mini Optimized for codex. Cheaper, faster, but less - capable. + 1. gpt-5.4 (default) Latest frontier agentic coding model. + 2. gpt-5.4-mini Smaller frontier agentic coding model. + 3. gpt-5.3-codex Frontier Codex-optimized agentic coding model. +› 4. gpt-5.2 (current) Optimized for professional work and long-running + agents Press enter to select reasoning effort, or esc to dismiss. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__preamble_keeps_working_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__preamble_keeps_working_status.snap index ad7fa52531..3dbbd8f309 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__preamble_keeps_working_status.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__preamble_keeps_working_status.snap @@ -8,4 +8,4 @@ expression: normalized_backend_snapshot(terminal.backend()) " " "› Ask Codex to do anything " " " -" gpt-5.3-codex default · /tmp/project " +" gpt-5.4 default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__rate_limit_switch_prompt_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__rate_limit_switch_prompt_popup.snap index e210d1f0a3..97ed0c7196 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__rate_limit_switch_prompt_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__rate_limit_switch_prompt_popup.snap @@ -3,10 +3,10 @@ source: tui/src/chatwidget/tests.rs expression: popup --- Approaching rate limits - Switch to gpt-5.1-codex-mini for lower credit usage? + Switch to gpt-5.4-mini for lower credit usage? -› 1. Switch to gpt-5.1-codex-mini Optimized for codex. Cheaper, - faster, but less capable. +› 1. Switch to gpt-5.4-mini Smaller frontier agentic coding + model. 2. Keep current model 3. Keep current model (never show again) Hide future rate limit reminders about switching models. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap index bb4a8cefe7..5148441eb6 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_queues_user_messages_snapshot.snap @@ -9,4 +9,4 @@ expression: normalize_snapshot_paths(term.backend().vt100().screen().contents()) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap index db30e31a6e..e4d5788602 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap @@ -6,4 +6,4 @@ expression: terminal.backend() " " "› Check recently modified functions for compatibility " " " -" gpt-5.3-codex Side from main thread · Esc to return " +" gpt-5.4 Side from main thread · Esc to return " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap index 0dd45297b6..75e4f53d3a 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap @@ -6,4 +6,4 @@ expression: terminal.backend() " " "› Check recently modified functions for compatibility " " " -" gpt-5.3-codex defa… Side from main thread · main needs input · Esc to return " +" gpt-5.4 default · … Side from main thread · main needs input · Esc to return " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_side_requests_forked_side_question_while_task_running.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_side_requests_forked_side_question_while_task_running.snap index 014fb41f1d..4ab1cfb6ea 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_side_requests_forked_side_question_while_task_running.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_side_requests_forked_side_question_while_task_running.snap @@ -8,4 +8,4 @@ expression: normalized_backend_snapshot(terminal.backend()) " " "› Ask Codex to do anything " " " -" gpt-5.3-codex default Side starting... " +" gpt-5.4 default Side starting... " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap index fea8a4db0f..91d80a1a2f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap @@ -8,4 +8,4 @@ expression: normalized_backend_snapshot(terminal.backend()) " " "› Ask Codex to do anything " " " -" gpt-5.3-codex default · /tmp/project " +" gpt-5.4 default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap index 7a2d0b4b9d..43a3188fbf 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap @@ -8,4 +8,4 @@ expression: normalized_backend_snapshot(terminal.backend()) " " "› Ask Codex to do anything " " " -" gpt-5.3-codex default · /tmp/project " +" gpt-5.4 default · /tmp/project " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap index a5b839e00c..0a59dd9909 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap @@ -8,4 +8,4 @@ expression: normalize_snapshot_paths(rendered) › Ask Codex to do anything - gpt-5.3-codex default · /tmp/project + gpt-5.4 default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs index 9f2c81010a..8f3de7897f 100644 --- a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs @@ -169,7 +169,7 @@ async fn submit_user_message_with_mode_sets_coding_collaboration_mode() { #[tokio::test] async fn reasoning_selection_in_plan_mode_opens_scope_prompt_event() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.thread_id = Some(ThreadId::new()); chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) @@ -179,7 +179,7 @@ async fn reasoning_selection_in_plan_mode_opens_scope_prompt_event() { set_chatgpt_auth(&mut chat); chat.set_reasoning_effort(Some(ReasoningEffortConfig::High)); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -190,13 +190,13 @@ async fn reasoning_selection_in_plan_mode_opens_scope_prompt_event() { AppEvent::OpenPlanReasoningScopePrompt { model, effort: Some(_) - } if model == "gpt-5.1-codex-max" + } if model == "gpt-5.4" ); } #[tokio::test] async fn reasoning_selection_in_plan_mode_without_effort_change_does_not_open_scope_prompt_event() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.thread_id = Some(ThreadId::new()); chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) @@ -205,10 +205,10 @@ async fn reasoning_selection_in_plan_mode_without_effort_change_does_not_open_sc let _ = drain_insert_history(&mut rx); set_chatgpt_auth(&mut chat); - let current_preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let current_preset = get_available_model(&chat, "gpt-5.4"); chat.set_reasoning_effort(Some(current_preset.default_reasoning_effort)); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -216,7 +216,7 @@ async fn reasoning_selection_in_plan_mode_without_effort_change_does_not_open_sc assert!( events.iter().any(|event| matches!( event, - AppEvent::UpdateModel(model) if model == "gpt-5.1-codex-max" + AppEvent::UpdateModel(model) if model == "gpt-5.4" )), "expected model update event; events: {events:?}" ); @@ -231,7 +231,7 @@ async fn reasoning_selection_in_plan_mode_without_effort_change_does_not_open_sc #[tokio::test] async fn reasoning_selection_in_plan_mode_matching_plan_effort_but_different_global_opens_scope_prompt() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.thread_id = Some(ThreadId::new()); chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) @@ -245,7 +245,7 @@ async fn reasoning_selection_in_plan_mode_matching_plan_effort_but_different_glo // should open the scope prompt rather than silently rewriting the global default. chat.set_reasoning_effort(Some(ReasoningEffortConfig::High)); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -255,13 +255,13 @@ async fn reasoning_selection_in_plan_mode_matching_plan_effort_but_different_glo AppEvent::OpenPlanReasoningScopePrompt { model, effort: Some(ReasoningEffortConfig::Medium) - } if model == "gpt-5.1-codex-max" + } if model == "gpt-5.4" ); } #[tokio::test] async fn plan_mode_reasoning_override_is_marked_current_in_reasoning_popup() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); set_chatgpt_auth(&mut chat); chat.set_reasoning_effort(Some(ReasoningEffortConfig::High)); @@ -271,7 +271,7 @@ async fn plan_mode_reasoning_override_is_marked_current_in_reasoning_popup() { .expect("expected plan collaboration mode"); chat.set_collaboration_mask(plan_mask); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); let popup = render_bottom_popup(&chat, /*width*/ 100); @@ -284,7 +284,7 @@ async fn plan_mode_reasoning_override_is_marked_current_in_reasoning_popup() { #[tokio::test] async fn reasoning_selection_in_plan_mode_model_switch_does_not_open_scope_prompt_event() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.thread_id = Some(ThreadId::new()); chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); let plan_mask = collaboration_modes::plan_mask(chat.model_catalog.as_ref()) @@ -293,7 +293,7 @@ async fn reasoning_selection_in_plan_mode_model_switch_does_not_open_scope_promp let _ = drain_insert_history(&mut rx); set_chatgpt_auth(&mut chat); - let preset = get_available_model(&chat, "gpt-5"); + let preset = get_available_model(&chat, "gpt-5.2"); chat.open_reasoning_popup(preset); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -301,7 +301,7 @@ async fn reasoning_selection_in_plan_mode_model_switch_does_not_open_scope_promp assert!( events.iter().any(|event| matches!( event, - AppEvent::UpdateModel(model) if model == "gpt-5" + AppEvent::UpdateModel(model) if model == "gpt-5.2" )), "expected model update event; events: {events:?}" ); @@ -315,11 +315,8 @@ async fn reasoning_selection_in_plan_mode_model_switch_does_not_open_scope_promp #[tokio::test] async fn plan_reasoning_scope_popup_all_modes_persists_global_and_plan_override() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; - chat.open_plan_reasoning_scope_prompt( - "gpt-5.1-codex-max".to_string(), - Some(ReasoningEffortConfig::High), - ); + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; + chat.open_plan_reasoning_scope_prompt("gpt-5.4".to_string(), Some(ReasoningEffortConfig::High)); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -343,7 +340,7 @@ async fn plan_reasoning_scope_popup_all_modes_persists_global_and_plan_override( events.iter().any(|event| matches!( event, AppEvent::PersistModelSelection { model, effort: Some(ReasoningEffortConfig::High) } - if model == "gpt-5.1-codex-max" + if model == "gpt-5.4" )), "expected global model reasoning selection persistence; events: {events:?}" ); @@ -369,7 +366,7 @@ fn plan_mode_prompt_notification_uses_dedicated_type_name() { #[tokio::test] async fn open_plan_implementation_prompt_sets_pending_notification() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.config.tui_notifications.notifications = Notifications::Custom(vec!["plan-mode-prompt".to_string()]); @@ -383,14 +380,11 @@ async fn open_plan_implementation_prompt_sets_pending_notification() { #[tokio::test] async fn open_plan_reasoning_scope_prompt_sets_pending_notification() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.config.tui_notifications.notifications = Notifications::Custom(vec!["plan-mode-prompt".to_string()]); - chat.open_plan_reasoning_scope_prompt( - "gpt-5.1-codex-max".to_string(), - Some(ReasoningEffortConfig::High), - ); + chat.open_plan_reasoning_scope_prompt("gpt-5.4".to_string(), Some(ReasoningEffortConfig::High)); assert_matches!( chat.pending_notification, @@ -400,7 +394,7 @@ async fn open_plan_reasoning_scope_prompt_sets_pending_notification() { #[tokio::test] async fn agent_turn_complete_does_not_override_pending_plan_mode_prompt_notification() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.open_plan_implementation_prompt(); chat.notify(Notification::AgentTurnComplete { @@ -415,7 +409,7 @@ async fn agent_turn_complete_does_not_override_pending_plan_mode_prompt_notifica #[tokio::test] async fn request_user_input_notification_overrides_pending_agent_turn_complete_notification() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.notify(Notification::AgentTurnComplete { response: "done".to_string(), @@ -444,7 +438,7 @@ async fn request_user_input_notification_overrides_pending_agent_turn_complete_n #[tokio::test] async fn handle_request_user_input_sets_pending_notification() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.config.tui_notifications.notifications = Notifications::Custom(vec!["plan-mode-prompt".to_string()]); @@ -472,10 +466,10 @@ async fn handle_request_user_input_sets_pending_notification() { #[tokio::test] async fn plan_reasoning_scope_popup_mentions_selected_reasoning() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.set_plan_mode_reasoning_effort(Some(ReasoningEffortConfig::Low)); chat.open_plan_reasoning_scope_prompt( - "gpt-5.1-codex-max".to_string(), + "gpt-5.4".to_string(), Some(ReasoningEffortConfig::Medium), ); @@ -489,9 +483,9 @@ async fn plan_reasoning_scope_popup_mentions_selected_reasoning() { #[tokio::test] async fn plan_reasoning_scope_popup_mentions_built_in_plan_default_when_no_override() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.open_plan_reasoning_scope_prompt( - "gpt-5.1-codex-max".to_string(), + "gpt-5.4".to_string(), Some(ReasoningEffortConfig::Medium), ); @@ -501,11 +495,8 @@ async fn plan_reasoning_scope_popup_mentions_built_in_plan_default_when_no_overr #[tokio::test] async fn plan_reasoning_scope_popup_plan_only_does_not_update_all_modes_reasoning() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; - chat.open_plan_reasoning_scope_prompt( - "gpt-5.1-codex-max".to_string(), - Some(ReasoningEffortConfig::High), - ); + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; + chat.open_plan_reasoning_scope_prompt("gpt-5.4".to_string(), Some(ReasoningEffortConfig::High)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -1123,7 +1114,7 @@ async fn mode_switch_surfaces_model_change_notification_when_effective_model_cha let mut plan_mask = collaboration_modes::mask_for_kind(chat.model_catalog.as_ref(), ModeKind::Plan) .expect("expected plan collaboration mode"); - plan_mask.model = Some("gpt-5.1-codex-mini".to_string()); + plan_mask.model = Some("gpt-5.4-mini".to_string()); chat.set_collaboration_mask(plan_mask); let plan_messages = drain_insert_history(&mut rx) @@ -1132,7 +1123,7 @@ async fn mode_switch_surfaces_model_change_notification_when_effective_model_cha .collect::>() .join("\n"); assert!( - plan_messages.contains("Model changed to gpt-5.1-codex-mini medium for Plan mode."), + plan_messages.contains("Model changed to gpt-5.4-mini medium for Plan mode."), "expected Plan-mode model switch notice, got: {plan_messages:?}" ); @@ -1336,21 +1327,21 @@ async fn collaboration_modes_defaults_to_code_on_startup() { #[tokio::test] async fn set_model_updates_active_collaboration_mask() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); let plan_mask = collaboration_modes::mask_for_kind(chat.model_catalog.as_ref(), ModeKind::Plan) .expect("expected plan collaboration mask"); chat.set_collaboration_mask(plan_mask); - chat.set_model("gpt-5.1-codex-mini"); + chat.set_model("gpt-5.4-mini"); - assert_eq!(chat.current_model(), "gpt-5.1-codex-mini"); + assert_eq!(chat.current_model(), "gpt-5.4-mini"); assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Plan); } #[tokio::test] async fn set_reasoning_effort_updates_active_collaboration_mask() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); let plan_mask = collaboration_modes::mask_for_kind(chat.model_catalog.as_ref(), ModeKind::Plan) .expect("expected plan collaboration mask"); @@ -1367,7 +1358,7 @@ async fn set_reasoning_effort_updates_active_collaboration_mask() { #[tokio::test] async fn set_reasoning_effort_does_not_override_active_plan_override() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; chat.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true); chat.set_plan_mode_reasoning_effort(Some(ReasoningEffortConfig::High)); let plan_mask = collaboration_modes::mask_for_kind(chat.model_catalog.as_ref(), ModeKind::Plan) @@ -1437,10 +1428,10 @@ async fn collab_mode_applies_default_preset() { #[tokio::test] async fn user_turn_includes_personality_from_config() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.set_feature_enabled(Feature::Personality, /*enabled*/ true); chat.thread_id = Some(ThreadId::new()); - chat.set_model("gpt-5.2-codex"); + chat.set_model("gpt-5.3-codex"); chat.set_personality(Personality::Friendly); chat.bottom_pane diff --git a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs index a3792c31f4..e05dcfd5cd 100644 --- a/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs +++ b/codex-rs/tui/src/chatwidget/tests/popups_and_settings.rs @@ -1901,7 +1901,7 @@ async fn memories_reset_confirmation_sends_event_on_confirm() { #[tokio::test] async fn model_selection_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5-codex")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; chat.thread_id = Some(ThreadId::new()); chat.open_model_popup(); @@ -1911,7 +1911,7 @@ async fn model_selection_popup_snapshot() { #[tokio::test] async fn personality_selection_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.thread_id = Some(ThreadId::new()); chat.open_personality_popup(); @@ -1922,7 +1922,7 @@ async fn personality_selection_popup_snapshot() { #[cfg(not(target_os = "linux"))] #[tokio::test] async fn realtime_audio_selection_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.open_realtime_audio_popup(); let popup = render_bottom_popup(&chat, /*width*/ 80); @@ -1932,7 +1932,7 @@ async fn realtime_audio_selection_popup_snapshot() { #[cfg(not(target_os = "linux"))] #[tokio::test] async fn realtime_audio_selection_popup_narrow_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.open_realtime_audio_popup(); let popup = render_bottom_popup(&chat, /*width*/ 56); @@ -1942,7 +1942,7 @@ async fn realtime_audio_selection_popup_narrow_snapshot() { #[cfg(not(target_os = "linux"))] #[tokio::test] async fn realtime_microphone_picker_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.config.realtime_audio.microphone = Some("Studio Mic".to_string()); chat.open_realtime_audio_device_selection_with_names( RealtimeAudioDeviceKind::Microphone, @@ -1956,7 +1956,7 @@ async fn realtime_microphone_picker_popup_snapshot() { #[cfg(not(target_os = "linux"))] #[tokio::test] async fn realtime_audio_picker_emits_persist_event() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.open_realtime_audio_device_selection_with_names( RealtimeAudioDeviceKind::Speaker, vec!["Desk Speakers".to_string(), "Headphones".to_string()], @@ -2017,8 +2017,8 @@ async fn model_picker_hides_show_in_picker_false_models_from_cache() { #[tokio::test] async fn server_overloaded_error_does_not_switch_models() { - let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; - chat.set_model("gpt-5.2-codex"); + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; + chat.set_model("gpt-5.3-codex"); while rx.try_recv().is_ok() {} while op_rx.try_recv().is_ok() {} @@ -2033,7 +2033,7 @@ async fn server_overloaded_error_does_not_switch_models() { while let Ok(event) = rx.try_recv() { if let AppEvent::UpdateModel(model) = event { assert_eq!( - model, "gpt-5.2-codex", + model, "gpt-5.3-codex", "did not expect model switch on server-overloaded error" ); } @@ -2051,12 +2051,12 @@ async fn server_overloaded_error_does_not_switch_models() { #[tokio::test] async fn model_reasoning_selection_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; set_chatgpt_auth(&mut chat); chat.set_reasoning_effort(Some(ReasoningEffortConfig::High)); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); let popup = render_bottom_popup(&chat, /*width*/ 80); @@ -2065,12 +2065,12 @@ async fn model_reasoning_selection_popup_snapshot() { #[tokio::test] async fn model_reasoning_selection_popup_extra_high_warning_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; set_chatgpt_auth(&mut chat); chat.set_reasoning_effort(Some(ReasoningEffortConfig::XHigh)); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.2"); chat.open_reasoning_popup(preset); let popup = render_bottom_popup(&chat, /*width*/ 80); @@ -2079,11 +2079,11 @@ async fn model_reasoning_selection_popup_extra_high_warning_snapshot() { #[tokio::test] async fn reasoning_popup_shows_extra_high_with_space() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; set_chatgpt_auth(&mut chat); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); let popup = render_bottom_popup(&chat, /*width*/ 120); @@ -2193,11 +2193,11 @@ async fn feedback_good_result_consent_popup_includes_connectivity_diagnostics_fi #[tokio::test] async fn reasoning_popup_escape_returns_to_model_popup() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.thread_id = Some(ThreadId::new()); chat.open_model_popup(); - let preset = get_available_model(&chat, "gpt-5.1-codex-max"); + let preset = get_available_model(&chat, "gpt-5.4"); chat.open_reasoning_popup(preset); let before_escape = render_bottom_popup(&chat, /*width*/ 80); diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 905ae98377..e91a11eca5 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -299,7 +299,7 @@ async fn queued_bang_shell_waits_for_user_shell_completion_before_next_input() { } async fn assert_cancelled_queued_menu_drains_next_input(command: &str, expected_popup_text: &str) { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5-codex")).await; + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; chat.thread_id = Some(ThreadId::new()); chat.handle_codex_event(Event { id: "turn-start".into(), @@ -351,7 +351,7 @@ async fn queued_slash_menu_cancel_drains_next_input() { #[tokio::test] async fn queued_slash_menu_selection_drains_next_input() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5-codex")).await; + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await; chat.thread_id = Some(ThreadId::new()); chat.handle_codex_event(Event { id: "turn-start".into(), diff --git a/codex-rs/tui/src/chatwidget/tests/status_command_tests.rs b/codex-rs/tui/src/chatwidget/tests/status_command_tests.rs index a0a32056a5..c206313549 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_command_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_command_tests.rs @@ -78,7 +78,7 @@ async fn status_command_renders_immediately_without_rate_limit_refresh() { #[tokio::test] async fn status_command_uses_catalog_default_reasoning_when_config_empty() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await; chat.config.model_reasoning_effort = None; chat.dispatch_command(SlashCommand::Status); @@ -90,7 +90,7 @@ async fn status_command_uses_catalog_default_reasoning_when_config_empty() { other => panic!("expected status output, got {other:?}"), }; assert!( - rendered.contains("gpt-5.1-codex-max (reasoning medium, summaries auto)"), + rendered.contains("gpt-5.4 (reasoning medium, summaries auto)"), "expected /status to render the catalog default reasoning effort, got: {rendered}" ); } diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap deleted file mode 100644 index 23fb02d563..0000000000 --- a/codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap +++ /dev/null @@ -1,11 +0,0 @@ ---- -source: tui/src/app.rs -expression: model_migration_copy_to_plain_text(©) ---- -**Codex just got an upgrade. Introducing gpt-5.3-codex.** - -Codex is now powered by gpt-5.3-codex, our most capable agentic coding model yet. It's built for long-running, project-scale work, with mid-turn steering + frequent progress updates so you can collaborate while it runs (and it's faster too). - -Learn more: https://openai.com/index/introducing-gpt-5-3-codex/ - -You can keep using gpt-5.1-codex if you prefer. From f04f04bb9abd0fe4570ccb7899ccac11fa885fc8 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 20 Apr 2026 14:58:01 -0700 Subject: [PATCH 5/7] refactor: drop data locks before async work --- codex-rs/login/src/auth/external_bearer.rs | 45 +++++++++++++++++----- codex-rs/network-proxy/src/runtime.rs | 27 ++++++------- 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/codex-rs/login/src/auth/external_bearer.rs b/codex-rs/login/src/auth/external_bearer.rs index c179f48619..380d393344 100644 --- a/codex-rs/login/src/auth/external_bearer.rs +++ b/codex-rs/login/src/auth/external_bearer.rs @@ -13,6 +13,7 @@ use std::sync::Arc; use std::time::Instant; use tokio::process::Command; use tokio::sync::Mutex; +use tokio::sync::Semaphore; #[derive(Clone)] pub(crate) struct BearerTokenRefresher { @@ -34,8 +35,26 @@ impl ExternalAuth for BearerTokenRefresher { } async fn resolve(&self) -> io::Result> { - let access_token = { - let mut cached = self.state.cached_token.lock().await; + if let Some(cached_token) = self.state.cached_token.lock().await.as_ref() { + let should_use_cached_token = match self.state.config.refresh_interval() { + Some(refresh_interval) => cached_token.fetched_at.elapsed() < refresh_interval, + None => true, + }; + if should_use_cached_token { + return Ok(Some(ExternalAuthTokens::access_token_only( + cached_token.access_token.clone(), + ))); + } + } + + let _refresh_guard = self + .state + .refresh_lock + .acquire() + .await + .map_err(|_| io::Error::other("external bearer token refresh semaphore closed"))?; + { + let cached = self.state.cached_token.lock().await; if let Some(cached_token) = cached.as_ref() { let should_use_cached_token = match self.state.config.refresh_interval() { Some(refresh_interval) => cached_token.fetched_at.elapsed() < refresh_interval, @@ -47,14 +66,14 @@ impl ExternalAuth for BearerTokenRefresher { ))); } } + } - let access_token = run_provider_auth_command(&self.state.config).await?; - *cached = Some(CachedExternalBearerToken { - access_token: access_token.clone(), - fetched_at: Instant::now(), - }); - access_token - }; + let access_token = run_provider_auth_command(&self.state.config).await?; + let mut cached = self.state.cached_token.lock().await; + *cached = Some(CachedExternalBearerToken { + access_token: access_token.clone(), + fetched_at: Instant::now(), + }); Ok(Some(ExternalAuthTokens::access_token_only(access_token))) } @@ -62,6 +81,12 @@ impl ExternalAuth for BearerTokenRefresher { &self, _context: ExternalAuthRefreshContext, ) -> io::Result { + let _refresh_guard = self + .state + .refresh_lock + .acquire() + .await + .map_err(|_| io::Error::other("external bearer token refresh semaphore closed"))?; let access_token = run_provider_auth_command(&self.state.config).await?; let mut cached = self.state.cached_token.lock().await; *cached = Some(CachedExternalBearerToken { @@ -82,6 +107,7 @@ impl fmt::Debug for BearerTokenRefresher { struct ExternalBearerAuthState { config: ModelProviderAuthInfo, cached_token: Mutex>, + refresh_lock: Semaphore, } impl ExternalBearerAuthState { @@ -89,6 +115,7 @@ impl ExternalBearerAuthState { Self { config, cached_token: Mutex::new(None), + refresh_lock: Semaphore::new(1), } } } diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index da090a69d4..b065fa6ee8 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -420,32 +420,27 @@ impl NetworkProxyState { let blocked_for_observer = entry.clone(); let blocked_request_observer = self.blocked_request_observer.read().await.clone(); let violation_line = blocked_request_violation_log_line(&entry); - let mut guard = self.state.write().await; let host = entry.host.clone(); let reason = entry.reason.clone(); let decision = entry.decision.clone(); let source = entry.source.clone(); let protocol = entry.protocol.clone(); let port = entry.port; - guard.blocked.push_back(entry); - guard.blocked_total = guard.blocked_total.saturating_add(1); - let total = guard.blocked_total; - while guard.blocked.len() > MAX_BLOCKED_EVENTS { - guard.blocked.pop_front(); - } + let (total, buffered) = { + let mut guard = self.state.write().await; + guard.blocked.push_back(entry); + guard.blocked_total = guard.blocked_total.saturating_add(1); + let total = guard.blocked_total; + while guard.blocked.len() > MAX_BLOCKED_EVENTS { + guard.blocked.pop_front(); + } + (total, guard.blocked.len()) + }; debug!( "recorded blocked request telemetry (total={}, host={}, reason={}, decision={:?}, source={:?}, protocol={}, port={:?}, buffered={})", - total, - host, - reason, - decision, - source, - protocol, - port, - guard.blocked.len() + total, host, reason, decision, source, protocol, port, buffered ); debug!("{violation_line}"); - drop(guard); if let Some(observer) = blocked_request_observer { observer.on_blocked_request(blocked_for_observer).await; From 832c25b0235676f78a521f19b2c5057c6543e4fd Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 20 Apr 2026 14:58:01 -0700 Subject: [PATCH 6/7] chore: document intentional await-holding cases --- .../app-server/src/codex_message_processor.rs | 8 +++ .../src/message_processor/tracing_tests.rs | 8 +++ .../src/transport/remote_control/websocket.rs | 12 ++++ codex-rs/clippy.toml | 5 ++ .../src/endpoint/responses_websocket.rs | 4 ++ codex-rs/core/src/guardian/review_session.rs | 4 ++ codex-rs/core/src/mcp_tool_call.rs | 12 ++++ codex-rs/core/src/session/handlers.rs | 4 ++ codex-rs/core/src/session/mcp.rs | 28 +++++++++ codex-rs/core/src/session/mod.rs | 60 +++++++++++++++++++ codex-rs/core/src/session/session.rs | 4 ++ codex-rs/core/src/session/tests.rs | 4 ++ .../core/src/session/tests/guardian_tests.rs | 4 ++ codex-rs/core/src/session/turn.rs | 8 +++ codex-rs/core/src/tools/code_mode/mod.rs | 4 ++ codex-rs/core/src/tools/handlers/dynamic.rs | 4 ++ .../core/src/tools/handlers/mcp_resource.rs | 8 +++ .../core/src/tools/handlers/tool_suggest.rs | 8 +++ codex-rs/core/src/tools/js_repl/mod.rs | 16 +++++ codex-rs/core/src/tools/registry.rs | 4 ++ codex-rs/core/src/tools/router_tests.rs | 12 ++++ codex-rs/rmcp-client/src/oauth.rs | 8 +++ 22 files changed, 229 insertions(+) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 51b1c48c83..a7bea99da3 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -7945,6 +7945,10 @@ impl CodexMessageProcessor { .await } + #[expect( + clippy::await_holding_invalid_type, + reason = "listener subscription must be serialized against pending thread unloads" + )] async fn ensure_conversation_listener_task( listener_task_context: ListenerTaskContext, conversation_id: ThreadId, @@ -8752,6 +8756,10 @@ async fn handle_thread_listener_command( } #[allow(clippy::too_many_arguments)] +#[expect( + clippy::await_holding_invalid_type, + reason = "running-thread resume subscription must be serialized against pending unloads" +)] async fn handle_pending_thread_resume_request( conversation_id: ThreadId, conversation: &Arc, diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index c1bb995bab..ac6c48df06 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -504,6 +504,10 @@ where } #[tokio::test(flavor = "current_thread")] +#[expect( + clippy::await_holding_invalid_type, + reason = "test serializes access to global tracing state for its full duration" +)] async fn thread_start_jsonrpc_span_exports_server_span_and_parents_children() -> Result<()> { let _guard = tracing_test_guard().lock().await; let mut harness = TracingHarness::new().await?; @@ -583,6 +587,10 @@ async fn thread_start_jsonrpc_span_exports_server_span_and_parents_children() -> } #[tokio::test(flavor = "current_thread")] +#[expect( + clippy::await_holding_invalid_type, + reason = "test serializes access to global tracing state for its full duration" +)] async fn turn_start_jsonrpc_span_parents_core_turn_spans() -> Result<()> { let _guard = tracing_test_guard().lock().await; let mut harness = TracingHarness::new().await?; diff --git a/codex-rs/app-server/src/transport/remote_control/websocket.rs b/codex-rs/app-server/src/transport/remote_control/websocket.rs index ac91882f52..dee177d30e 100644 --- a/codex-rs/app-server/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server/src/transport/remote_control/websocket.rs @@ -205,6 +205,10 @@ impl RemoteControlWebsocket { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "remote-control client shutdown must serialize tracker state" + )] pub(crate) async fn run( mut self, app_server_client_name_rx: Option>, @@ -416,6 +420,10 @@ impl RemoteControlWebsocket { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "remote-control server event receiver is shared across reconnects" + )] async fn run_server_writer_inner( state: Arc>, server_event_rx: Arc>>, @@ -557,6 +565,10 @@ impl RemoteControlWebsocket { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "remote-control client tracking must stay serialized while processing inbound events" + )] async fn run_websocket_reader_inner( client_tracker: Arc>, state: Arc>, diff --git a/codex-rs/clippy.toml b/codex-rs/clippy.toml index a2aae6591b..2feed8a487 100644 --- a/codex-rs/clippy.toml +++ b/codex-rs/clippy.toml @@ -1,5 +1,10 @@ allow-expect-in-tests = true allow-unwrap-in-tests = true +await-holding-invalid-types = [ + "tokio::sync::MutexGuard", + "tokio::sync::RwLockReadGuard", + "tokio::sync::RwLockWriteGuard", +] disallowed-methods = [ { path = "ratatui::style::Color::Rgb", reason = "Use ANSI colors, which work better in various terminal themes." }, { path = "ratatui::style::Color::Indexed", reason = "Use ANSI colors, which work better in various terminal themes." }, diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index bc72aee411..aea9b917fb 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -229,6 +229,10 @@ impl ResponsesWebsocketConnection { let current_span = Span::current(); tokio::spawn( + #[expect( + clippy::await_holding_invalid_type, + reason = "the guard serializes exclusive use of the websocket stream for the lifetime of the response stream" + )] async move { if let Some(model) = server_model { let _ = tx_event.send(Ok(ResponseEvent::ServerModel(model))).await; diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 5bb3e9d1b8..a66d79e865 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -265,6 +265,10 @@ impl GuardianReviewSessionManager { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "review session selection and trunk spawning must stay serialized" + )] pub(crate) async fn run_review( &self, params: GuardianReviewSessionParams, diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 32c7cf879e..c5d4068d27 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -494,6 +494,10 @@ async fn execute_mcp_tool_call( ) } +#[expect( + clippy::await_holding_invalid_type, + reason = "MCP sandbox metadata reads through the session-owned manager guard" +)] async fn augment_mcp_tool_request_meta_with_sandbox_state( sess: &Session, turn_context: &TurnContext, @@ -1067,6 +1071,10 @@ fn mcp_tool_approval_callsite_mode( } } +#[expect( + clippy::await_holding_invalid_type, + reason = "MCP approval metadata reads through the session-owned manager guard" +)] pub(crate) async fn lookup_mcp_tool_metadata( sess: &Session, turn_context: &TurnContext, @@ -1149,6 +1157,10 @@ fn get_mcp_app_resource_uri( }) } +#[expect( + clippy::await_holding_invalid_type, + reason = "MCP app metadata reads through the session-owned manager guard" +)] async fn lookup_mcp_app_usage_metadata( sess: &Session, server: &str, diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index b45b6fc10e..9b7eb75549 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -468,6 +468,10 @@ pub async fn reload_user_config(sess: &Arc) { sess.reload_user_config_layer().await; } +#[expect( + clippy::await_holding_invalid_type, + reason = "MCP tool listing reads through the session-owned manager guard" +)] pub async fn list_mcp_tools(sess: &Session, config: &Arc, sub_id: String) { let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; let auth = sess.services.auth_manager.auth().await; diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index d15ca2cae8..be7504d9e7 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -1,6 +1,10 @@ use super::*; impl Session { + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn request_mcp_server_elicitation( &self, turn_context: &TurnContext, @@ -80,6 +84,10 @@ impl Session { rx_response.await.ok() } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and manager fallback must stay serialized" + )] pub async fn resolve_elicitation( &self, server_name: String, @@ -111,6 +119,10 @@ impl Session { .await } + #[expect( + clippy::await_holding_invalid_type, + reason = "MCP resource calls are serialized through the session-owned manager guard" + )] pub async fn list_resources( &self, server: &str, @@ -124,6 +136,10 @@ impl Session { .await } + #[expect( + clippy::await_holding_invalid_type, + reason = "MCP resource calls are serialized through the session-owned manager guard" + )] pub async fn list_resource_templates( &self, server: &str, @@ -137,6 +153,10 @@ impl Session { .await } + #[expect( + clippy::await_holding_invalid_type, + reason = "MCP resource calls are serialized through the session-owned manager guard" + )] pub async fn read_resource( &self, server: &str, @@ -150,6 +170,10 @@ impl Session { .await } + #[expect( + clippy::await_holding_invalid_type, + reason = "MCP tool calls are serialized through the session-owned manager guard" + )] pub async fn call_tool( &self, server: &str, @@ -165,6 +189,10 @@ impl Session { .await } + #[expect( + clippy::await_holding_invalid_type, + reason = "MCP tool metadata reads through the session-owned manager guard" + )] pub(crate) async fn resolve_mcp_tool_info(&self, tool_name: &ToolName) -> Option { self.services .mcp_connection_manager diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index f159546575..aaaf8d75bd 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1786,6 +1786,10 @@ impl Session { /// be used to derive the available decisions via /// [ExecApprovalRequestEvent::default_available_decisions]. #[allow(clippy::too_many_arguments)] + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn request_command_approval( &self, turn_context: &TurnContext, @@ -1857,6 +1861,10 @@ impl Session { rx_approve.await.unwrap_or(ReviewDecision::Abort) } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn request_patch_approval( &self, turn_context: &TurnContext, @@ -1893,6 +1901,10 @@ impl Session { rx_approve } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn request_permissions( &self, turn_context: &TurnContext, @@ -1948,6 +1960,10 @@ impl Session { rx_response.await.ok() } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn request_user_input( &self, turn_context: &TurnContext, @@ -1980,6 +1996,10 @@ impl Session { rx_response.await.ok() } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn notify_user_input_response( &self, sub_id: &str, @@ -2005,6 +2025,10 @@ impl Session { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn notify_request_permissions_response( &self, call_id: &str, @@ -2046,6 +2070,10 @@ impl Session { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn reads must stay consistent with the matching turn state" + )] pub(crate) async fn granted_turn_permissions(&self) -> Option { let active = self.active_turn.lock().await; let active = active.as_ref()?; @@ -2058,6 +2086,10 @@ impl Session { state.granted_permissions() } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn notify_dynamic_tool_response(&self, call_id: &str, response: DynamicToolResponse) { let entry = { let mut active = self.active_turn.lock().await; @@ -2079,6 +2111,10 @@ impl Session { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn notify_approval(&self, approval_id: &str, decision: ReviewDecision) { let entry = { let mut active = self.active_turn.lock().await; @@ -2239,6 +2275,10 @@ impl Session { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "MCP app context rendering reads through the session-owned manager guard" + )] pub(crate) async fn build_initial_context( &self, turn_context: &TurnContext, @@ -2701,6 +2741,10 @@ impl Session { /// Inject additional user input into the currently active turn. /// /// Returns the active turn id when accepted. + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn steer_input( &self, input: Vec, @@ -2760,6 +2804,10 @@ impl Session { } /// Returns the input if there was no task running to inject into. + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn inject_response_items( &self, input: Vec, @@ -2833,6 +2881,10 @@ impl Session { self.mailbox_rx.lock().await.has_pending_trigger_turn() } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn prepend_pending_input(&self, input: Vec) -> Result<(), ()> { let mut active = self.active_turn.lock().await; match active.as_mut() { @@ -2845,6 +2897,10 @@ impl Session { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] pub async fn get_pending_input(&self) -> Vec { let (pending_input, accepts_mailbox_delivery) = { let mut active = self.active_turn.lock().await; @@ -2900,6 +2956,10 @@ impl Session { !self.idle_pending_input.lock().await.is_empty() } + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state reads must remain atomic" + )] pub async fn has_pending_input(&self) -> bool { let (has_turn_pending_input, accepts_mailbox_delivery) = { let active = self.active_turn.lock().await; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index a2080bbeab..16e86e3aea 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -209,6 +209,10 @@ pub(crate) struct AppServerClientMetadata { impl Session { #[instrument(name = "session_init", level = "info", skip_all)] #[allow(clippy::too_many_arguments)] + #[expect( + clippy::await_holding_invalid_type, + reason = "session initialization must serialize access through session-owned manager guards" + )] pub(crate) async fn new( mut session_configuration: SessionConfiguration, config: Arc, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index bd41da9f20..40daa54e25 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -6320,6 +6320,10 @@ async fn abort_review_task_emits_exited_then_aborted_and_records_history() { } #[tokio::test] +#[expect( + clippy::await_holding_invalid_type, + reason = "test builds a router from session-owned MCP manager state" +)] async fn fatal_tool_error_stops_turn_and_reports_error() { let (session, turn_context, _rx) = make_session_and_context_with_rx().await; let tools = { diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 9ccd6eb4e0..97543105b6 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -298,6 +298,10 @@ async fn process_compacted_history_preserves_separate_guardian_developer_message #[tokio::test] #[cfg(unix)] +#[expect( + clippy::await_holding_invalid_type, + reason = "test mutates active turn state directly to seed granted permissions" +)] async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_permissions_feature() { let (mut session, turn_context_raw) = make_session_and_context().await; session diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index efbcca6716..643007787a 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -127,6 +127,10 @@ use tracing::warn; /// - If the model sends only an assistant message, we record it in the /// conversation history and consider the turn complete. /// +#[expect( + clippy::await_holding_invalid_type, + reason = "turn execution must keep active-turn state transitions atomic" +)] pub(crate) async fn run_turn( sess: Arc, turn_context: Arc, @@ -1143,6 +1147,10 @@ async fn run_sampling_request( } } +#[expect( + clippy::await_holding_invalid_type, + reason = "tool router construction reads through the session-owned manager guard" +)] pub(crate) async fn built_tools( sess: &Session, turn_context: &TurnContext, diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index 5049836393..575d3f9a89 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -259,6 +259,10 @@ pub(super) async fn build_enabled_tools( collect_code_mode_tool_definitions(&specs) } +#[expect( + clippy::await_holding_invalid_type, + reason = "nested tool router construction reads through the session-owned manager guard" +)] async fn build_nested_router(exec: &ExecContext) -> ToolRouter { let nested_tools_config = exec.turn.tools_config.for_code_mode_nested_tools(); let listed_mcp_tools = exec diff --git a/codex-rs/core/src/tools/handlers/dynamic.rs b/codex-rs/core/src/tools/handlers/dynamic.rs index 140d586be9..c989d70f19 100644 --- a/codex-rs/core/src/tools/handlers/dynamic.rs +++ b/codex-rs/core/src/tools/handlers/dynamic.rs @@ -71,6 +71,10 @@ impl ToolHandler for DynamicToolHandler { } } +#[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and dynamic tool response registration must remain atomic" +)] async fn request_dynamic_tool( session: &Session, turn_context: &TurnContext, diff --git a/codex-rs/core/src/tools/handlers/mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource.rs index 67f44cb699..fa4a066741 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource.rs @@ -240,6 +240,10 @@ impl ToolHandler for McpResourceHandler { } } +#[expect( + clippy::await_holding_invalid_type, + reason = "MCP resource listing reads through the session-owned manager guard" +)] async fn handle_list_resources( session: Arc, turn: Arc, @@ -344,6 +348,10 @@ async fn handle_list_resources( } } +#[expect( + clippy::await_holding_invalid_type, + reason = "MCP resource template listing reads through the session-owned manager guard" +)] async fn handle_list_resource_templates( session: Arc, turn: Arc, diff --git a/codex-rs/core/src/tools/handlers/tool_suggest.rs b/codex-rs/core/src/tools/handlers/tool_suggest.rs index cc433601bc..5c51c90d66 100644 --- a/codex-rs/core/src/tools/handlers/tool_suggest.rs +++ b/codex-rs/core/src/tools/handlers/tool_suggest.rs @@ -34,6 +34,10 @@ impl ToolHandler for ToolSuggestHandler { ToolKind::Function } + #[expect( + clippy::await_holding_invalid_type, + reason = "tool suggestion discovery reads through the session-owned manager guard" + )] async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { payload, @@ -193,6 +197,10 @@ async fn verify_tool_suggestion_completed( } } +#[expect( + clippy::await_holding_invalid_type, + reason = "connector cache refresh reads through the session-owned manager guard" +)] async fn refresh_missing_suggested_connectors( session: &crate::session::session::Session, turn: &crate::session::turn_context::TurnContext, diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 23f4906e5f..fffa7028dc 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -836,6 +836,10 @@ impl JsReplManager { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "js_repl kernel initialization must be serialized with kernel state" + )] pub async fn execute( &self, session: Arc, @@ -1162,6 +1166,10 @@ impl JsReplManager { Ok(kernel_path) } + #[expect( + clippy::await_holding_invalid_type, + reason = "js_repl stdin writes must be serialized per kernel" + )] async fn write_message( stdin: &Arc>, msg: &HostToKernel, @@ -1209,6 +1217,10 @@ impl JsReplManager { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "js_repl child shutdown must serialize process inspection and termination" + )] async fn kill_kernel_child(child: &Arc>, reason: &'static str) { let mut guard = child.lock().await; let pid = guard.id(); @@ -1532,6 +1544,10 @@ impl JsReplManager { } } + #[expect( + clippy::await_holding_invalid_type, + reason = "nested js_repl tool routing reads through the session-owned manager guard" + )] async fn run_tool_request(exec: ExecContext, req: RunToolRequest) -> RunToolResult { if is_js_repl_internal_tool(&req.tool_name) { let error = "js_repl cannot invoke itself".to_string(); diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 4c0e3fbea8..aca1d10144 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -239,6 +239,10 @@ impl ToolRegistry { // } // } + #[expect( + clippy::await_holding_invalid_type, + reason = "tool dispatch must keep active-turn accounting atomic" + )] pub(crate) async fn dispatch_any( &self, invocation: ToolInvocation, diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 1a59f1e1ba..14536e3985 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -14,6 +14,10 @@ use super::ToolRouter; use super::ToolRouterParams; #[tokio::test] +#[expect( + clippy::await_holding_invalid_type, + reason = "test builds a router from session-owned MCP manager state" +)] async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> { let (session, mut turn) = make_session_and_context().await; turn.tools_config.js_repl_tools_only = true; @@ -68,6 +72,10 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> { } #[tokio::test] +#[expect( + clippy::await_holding_invalid_type, + reason = "test builds a router from session-owned MCP manager state" +)] async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()> { let (session, mut turn) = make_session_and_context().await; turn.tools_config.js_repl_tools_only = true; @@ -171,6 +179,10 @@ async fn js_repl_tools_only_blocks_namespaced_js_repl_tool() -> anyhow::Result<( } #[tokio::test] +#[expect( + clippy::await_holding_invalid_type, + reason = "test builds a router from session-owned MCP manager state" +)] async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow::Result<()> { let (session, turn) = make_session_and_context().await; let mcp_tools = session diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index 979a7ad217..5adfae9347 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -283,6 +283,10 @@ impl OAuthPersistor { /// Persists the latest stored credentials if they have changed. /// Deletes the credentials if they are no longer present. + #[expect( + clippy::await_holding_invalid_type, + reason = "AuthorizationManager async access must be serialized through its mutex" + )] pub(crate) async fn persist_if_needed(&self) -> Result<()> { let (client_id, maybe_credentials) = { let manager = self.inner.authorization_manager.clone(); @@ -335,6 +339,10 @@ impl OAuthPersistor { Ok(()) } + #[expect( + clippy::await_holding_invalid_type, + reason = "AuthorizationManager async access must be serialized through its mutex" + )] pub(crate) async fn refresh_if_needed(&self) -> Result<()> { let expires_at = { let guard = self.inner.last_credentials.lock().await; From 2dd173fc66754d5deb0cf173e165a98a58268c3b Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 20 Apr 2026 14:58:01 -0700 Subject: [PATCH 7/7] chore: enable await-holding clippy lints Follow-up to https://github.com/openai/codex/pull/18178, where we said the await-holding clippy rule would be enabled separately. Enable `await_holding_lock` and `await_holding_invalid_type` after the preceding commits fixed or explicitly documented the current offenders. --- .bazelrc | 2 ++ codex-rs/Cargo.toml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.bazelrc b/.bazelrc index 3a49a22c20..fb6e93e58c 100644 --- a/.bazelrc +++ b/.bazelrc @@ -82,6 +82,8 @@ build:clippy --@rules_rust//rust/settings:clippy.toml=//codex-rs:clippy.toml # in their own `Cargo.toml`, but `rules_rust` Bazel clippy does not read Cargo lint levels. # `clippy.toml` can configure lint behavior, but it cannot set allow/warn/deny/forbid levels. build:clippy --@rules_rust//rust/settings:clippy_flag=-Dwarnings +build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::await_holding_invalid_type +build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::await_holding_lock build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::expect_used build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::identity_op build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_clamp diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index b1a8ae000a..4b348371b8 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -380,6 +380,8 @@ zeroize = "1.8.2" rust = {} [workspace.lints.clippy] +await_holding_invalid_type = "deny" +await_holding_lock = "deny" expect_used = "deny" identity_op = "deny" manual_clamp = "deny"