From 62af470452fde5141f010f713f9737ccd93b0c23 Mon Sep 17 00:00:00 2001 From: Michael Chen Date: Mon, 11 May 2026 19:49:58 -0700 Subject: [PATCH 1/2] [Core] Track user input requests in MCP metadata --- codex-rs/core/src/mcp_tool_call.rs | 11 +- codex-rs/core/src/session/mcp.rs | 3 + codex-rs/core/src/session/mod.rs | 3 + codex-rs/core/src/turn_metadata.rs | 21 + codex-rs/core/src/turn_metadata_tests.rs | 50 +++ .../core/tests/suite/mcp_turn_metadata.rs | 379 ++++++++++++++++++ codex-rs/core/tests/suite/mod.rs | 1 + 7 files changed, 459 insertions(+), 9 deletions(-) create mode 100644 codex-rs/core/tests/suite/mcp_turn_metadata.rs diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index fc8ce4d8ca32..8f6e3afaf5af 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -192,12 +192,6 @@ pub(crate) async fn handle_mcp_tool_call( .unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())), }; } - let request_meta = build_mcp_tool_call_request_meta( - turn_context.as_ref(), - &server, - &call_id, - metadata.as_ref(), - ); let connector_id = metadata .as_ref() .and_then(|metadata| metadata.connector_id.clone()); @@ -235,7 +229,6 @@ pub(crate) async fn handle_mcp_tool_call( &call_id, invocation, metadata.as_ref(), - request_meta, mcp_app_resource_uri, ) .await; @@ -303,7 +296,6 @@ pub(crate) async fn handle_mcp_tool_call( &call_id, invocation, metadata.as_ref(), - request_meta, mcp_app_resource_uri, ) .await @@ -320,7 +312,6 @@ async fn handle_approved_mcp_tool_call( call_id: &str, invocation: McpInvocation, metadata: Option<&McpToolApprovalMetadata>, - request_meta: Option, mcp_app_resource_uri: Option, ) -> HandledMcpToolCall { let server = invocation.server.clone(); @@ -353,6 +344,8 @@ async fn handle_approved_mcp_tool_call( }; let result = async { let rewritten_arguments = rewrite?; + let request_meta = + build_mcp_tool_call_request_meta(turn_context, &server, call_id, metadata); let result = execute_mcp_tool_call( sess, turn_context, diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index fcaaa17c57bc..9f27751f7e04 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -154,6 +154,9 @@ impl Session { id, request, }); + turn_context + .turn_metadata_state + .mark_user_input_requested_during_turn(); self.send_event(turn_context, event).await; rx_response.await.ok() } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 0a29e9788364..d0755302ec72 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2275,6 +2275,9 @@ impl Session { turn_id: turn_context.sub_id.clone(), questions: args.questions, }); + turn_context + .turn_metadata_state + .mark_user_input_requested_during_turn(); self.send_event(turn_context, event).await; rx_response.await.ok() } diff --git a/codex-rs/core/src/turn_metadata.rs b/codex-rs/core/src/turn_metadata.rs index 02760582f286..1b2c6b4b1295 100644 --- a/codex-rs/core/src/turn_metadata.rs +++ b/codex-rs/core/src/turn_metadata.rs @@ -3,6 +3,8 @@ use std::collections::HashMap; use std::sync::Arc; use std::sync::Mutex; use std::sync::RwLock; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use codex_utils_string::to_ascii_json_string; use serde::Serialize; @@ -23,6 +25,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; const MODEL_KEY: &str = "model"; const REASONING_EFFORT_KEY: &str = "reasoning_effort"; const TURN_STARTED_AT_UNIX_MS_KEY: &str = "turn_started_at_unix_ms"; +const USER_INPUT_REQUESTED_DURING_TURN_KEY: &str = "user_input_requested_during_turn"; pub(crate) struct McpTurnMetadataContext<'a> { pub(crate) model: &'a str, @@ -186,6 +189,7 @@ pub(crate) struct TurnMetadataState { enriched_header: Arc>>, turn_started_at_unix_ms: Arc>>, responsesapi_client_metadata: Arc>>>, + user_input_requested_during_turn: Arc, enrichment_task: Arc>>>, } @@ -231,6 +235,7 @@ impl TurnMetadataState { enriched_header: Arc::new(RwLock::new(None)), turn_started_at_unix_ms: Arc::new(RwLock::new(None)), responsesapi_client_metadata: Arc::new(RwLock::new(None)), + user_input_requested_during_turn: Arc::new(AtomicBool::new(false)), enrichment_task: Arc::new(Mutex::new(None)), } } @@ -285,9 +290,25 @@ impl TurnMetadataState { metadata.remove(REASONING_EFFORT_KEY); } } + if self + .user_input_requested_during_turn + .load(Ordering::Relaxed) + { + metadata.insert( + USER_INPUT_REQUESTED_DURING_TURN_KEY.to_string(), + Value::Bool(true), + ); + } else { + metadata.remove(USER_INPUT_REQUESTED_DURING_TURN_KEY); + } Some(Value::Object(metadata)) } + pub(crate) fn mark_user_input_requested_during_turn(&self) { + self.user_input_requested_during_turn + .store(true, Ordering::Relaxed); + } + pub(crate) fn set_responsesapi_client_metadata( &self, responsesapi_client_metadata: HashMap, diff --git a/codex-rs/core/src/turn_metadata_tests.rs b/codex-rs/core/src/turn_metadata_tests.rs index 2a38447f868b..a9c7a7b87d0b 100644 --- a/codex-rs/core/src/turn_metadata_tests.rs +++ b/codex-rs/core/src/turn_metadata_tests.rs @@ -213,6 +213,56 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta( ); } +#[test] +fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_request_meta() { + let temp_dir = TempDir::new().expect("temp dir"); + let cwd = temp_dir.path().abs(); + let permission_profile = PermissionProfile::read_only(); + + let state = TurnMetadataState::new( + "session-a".to_string(), + "thread-a".to_string(), + /*thread_source*/ None, + "turn-a".to_string(), + cwd, + &permission_profile, + WindowsSandboxLevel::Disabled, + /*enforce_managed_network*/ false, + ); + + let header = state.current_header_value().expect("header"); + let header_json: Value = serde_json::from_str(&header).expect("json"); + assert!( + header_json + .get(USER_INPUT_REQUESTED_DURING_TURN_KEY) + .is_none() + ); + + let meta = state + .current_meta_value_for_mcp_request(test_mcp_turn_metadata_context()) + .expect("turn metadata should be present"); + assert!(meta.get(USER_INPUT_REQUESTED_DURING_TURN_KEY).is_none()); + + state.mark_user_input_requested_during_turn(); + + let header = state.current_header_value().expect("header"); + let header_json: Value = serde_json::from_str(&header).expect("json"); + assert!( + header_json + .get(USER_INPUT_REQUESTED_DURING_TURN_KEY) + .is_none() + ); + + let meta = state + .current_meta_value_for_mcp_request(test_mcp_turn_metadata_context()) + .expect("turn metadata should be present"); + assert_eq!( + meta.get(USER_INPUT_REQUESTED_DURING_TURN_KEY) + .and_then(Value::as_bool), + Some(true) + ); +} + #[test] fn turn_metadata_state_ignores_client_turn_started_at_unix_ms_before_start() { let temp_dir = TempDir::new().expect("temp dir"); diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs new file mode 100644 index 000000000000..6166e4664e33 --- /dev/null +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -0,0 +1,379 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use anyhow::Result; +use codex_config::types::AppToolApproval; +use codex_core::config::Config; +use codex_features::Feature; +use codex_login::CodexAuth; +use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY; +use codex_models_manager::bundled_models_response; +use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::ModeKind; +use codex_protocol::config_types::Settings; +use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::ElicitationAction; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::request_user_input::RequestUserInputAnswer; +use codex_protocol::request_user_input::RequestUserInputResponse; +use codex_protocol::user_input::UserInput; +use core_test_support::PathExt; +use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_function_call_with_namespace; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::TestCodexBuilder; +use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; +use core_test_support::wait_for_event; +use core_test_support::wait_for_event_match; +use pretty_assertions::assert_eq; +use serde_json::Value; +use serde_json::json; +use std::collections::HashMap; +use wiremock::MockServer; + +const SEARCH_CALENDAR_NAMESPACE: &str = "mcp__codex_apps__calendar"; +const SEARCH_CALENDAR_CREATE_TOOL: &str = "_create_event"; + +fn configure_search_capable_model(config: &mut Config) { + let mut model_catalog = bundled_models_response() + .unwrap_or_else(|err| panic!("bundled models.json should parse: {err}")); + let model = model_catalog + .models + .iter_mut() + .find(|model| model.slug == "gpt-5.4") + .expect("gpt-5.4 exists in bundled models.json"); + config.model = Some("gpt-5.4".to_string()); + model.supports_search_tool = true; + config.model_catalog = Some(model_catalog); +} + +fn configure_apps(config: &mut Config, apps_base_url: &str) { + config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + config.chatgpt_base_url = apps_base_url.to_string(); + configure_search_capable_model(config); +} + +fn configured_builder(apps_base_url: String) -> TestCodexBuilder { + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| configure_apps(config, apps_base_url.as_str())) +} + +fn set_calendar_approval_mode(config: &mut Config, approval_mode: AppToolApproval) { + let approval_mode = match approval_mode { + AppToolApproval::Auto => "auto", + AppToolApproval::Prompt => "prompt", + AppToolApproval::Approve => "approve", + }; + let user_config_path = config.codex_home.join("config.toml").abs(); + let user_config = toml::from_str(&format!( + r#" +[apps.calendar] +default_tools_approval_mode = "{approval_mode}" +"# + )) + .expect("apps config should parse"); + config.config_layer_stack = config + .config_layer_stack + .with_user_config(&user_config_path, user_config); +} + +async fn submit_user_turn( + test: &TestCodex, + text: &str, + approval_policy: AskForApproval, + collaboration_mode: Option, +) -> Result<()> { + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, test.cwd.path()); + test.codex + .submit(Op::UserTurn { + environments: None, + items: vec![UserInput::Text { + text: text.to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd.path().to_path_buf(), + approval_policy, + approvals_reviewer: None, + sandbox_policy, + permission_profile, + model: test.session_configured.model.clone(), + effort: None, + summary: None, + service_tier: None, + collaboration_mode, + personality: None, + }) + .await?; + Ok(()) +} + +fn app_tool_call_id(body: &Value) -> Option<&str> { + body.get("params")? + .get("_meta")? + .get(MCP_TOOL_CODEX_APPS_META_KEY)? + .get("call_id")? + .as_str() +} + +async fn recorded_apps_tool_call(server: &MockServer, call_id: &str) -> Value { + let matches = server + .received_requests() + .await + .expect("mock server should capture requests") + .into_iter() + .filter_map(|request| { + let body: Value = serde_json::from_slice(&request.body).ok()?; + (request.url.path() == "/api/codex/apps" + && body.get("method").and_then(Value::as_str) == Some("tools/call") + && app_tool_call_id(&body) == Some(call_id)) + .then_some(body) + }) + .collect::>(); + assert_eq!( + matches.len(), + 1, + "expected exactly one apps tools/call request for call_id {call_id}" + ); + matches + .into_iter() + .next() + .expect("matching apps tools/call request should be recorded") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + let call_id = "calendar-call-approval"; + let calendar_args = serde_json::to_string(&json!({ + "title": "Lunch", + "starts_at": "2026-03-10T12:00:00Z" + }))?; + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call_with_namespace( + call_id, + SEARCH_CALENDAR_NAMESPACE, + SEARCH_CALENDAR_CREATE_TOOL, + &calendar_args, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut builder = + configured_builder(apps_server.chatgpt_base_url.clone()).with_config(|config| { + config + .features + .enable(Feature::ToolCallMcpElicitation) + .expect("test config should allow feature update"); + set_calendar_approval_mode(config, AppToolApproval::Prompt); + }); + let test = builder.build(&server).await?; + + submit_user_turn( + &test, + "Use [$calendar](app://calendar) to create a calendar event.", + AskForApproval::OnRequest, + /*collaboration_mode*/ None, + ) + .await?; + + let EventMsg::McpToolCallBegin(begin) = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::McpToolCallBegin(_)) + }) + .await + else { + unreachable!("event guard guarantees McpToolCallBegin"); + }; + assert_eq!(begin.call_id, call_id); + + let EventMsg::ElicitationRequest(request) = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::ElicitationRequest(_)) + }) + .await + else { + unreachable!("event guard guarantees ElicitationRequest"); + }; + + test.codex + .submit(Op::ResolveElicitation { + server_name: request.server_name, + request_id: request.id, + decision: ElicitationAction::Accept, + content: None, + meta: None, + }) + .await?; + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert_eq!(mock.requests().len(), 2); + let apps_tool_call = recorded_apps_tool_call(&server, call_id).await; + + assert_eq!( + apps_tool_call + .pointer("/params/_meta/x-codex-turn-metadata/user_input_requested_during_turn"), + Some(&json!(true)) + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + let request_user_input_call_id = "user-input-call"; + let calendar_call_id = "calendar-call-after-user-input"; + let request_user_input_args = json!({ + "questions": [{ + "id": "confirm_path", + "header": "Confirm", + "question": "Proceed with the plan?", + "options": [{ + "label": "Yes (Recommended)", + "description": "Continue the current plan." + }, { + "label": "No", + "description": "Stop and revisit the approach." + }] + }] + }) + .to_string(); + let calendar_args = serde_json::to_string(&json!({ + "title": "Lunch", + "starts_at": "2026-03-10T12:00:00Z" + }))?; + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + request_user_input_call_id, + "request_user_input", + &request_user_input_args, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_function_call_with_namespace( + calendar_call_id, + SEARCH_CALENDAR_NAMESPACE, + SEARCH_CALENDAR_CREATE_TOOL, + &calendar_args, + ), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_response_created("resp-3"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-3"), + ]), + ], + ) + .await; + + let mut builder = + configured_builder(apps_server.chatgpt_base_url.clone()).with_config(|config| { + set_calendar_approval_mode(config, AppToolApproval::Approve); + }); + let test = builder.build(&server).await?; + + submit_user_turn( + &test, + "Ask for confirmation, then create a calendar event.", + AskForApproval::Never, + Some(CollaborationMode { + mode: ModeKind::Plan, + settings: Settings { + model: test.session_configured.model.clone(), + reasoning_effort: None, + developer_instructions: None, + }, + }), + ) + .await?; + + let request = wait_for_event_match(&test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + assert_eq!(request.call_id, request_user_input_call_id); + + test.codex + .submit(Op::UserInputAnswer { + id: request.turn_id, + response: RequestUserInputResponse { + answers: HashMap::from([( + "confirm_path".to_string(), + RequestUserInputAnswer { + answers: vec!["Yes (Recommended)".to_string()], + }, + )]), + }, + }) + .await?; + + let EventMsg::McpToolCallBegin(begin) = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::McpToolCallBegin(_)) + }) + .await + else { + unreachable!("event guard guarantees McpToolCallBegin"); + }; + assert_eq!(begin.call_id, calendar_call_id); + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert_eq!(mock.requests().len(), 3); + let apps_tool_call = recorded_apps_tool_call(&server, calendar_call_id).await; + + assert_eq!( + apps_tool_call + .pointer("/params/_meta/x-codex-turn-metadata/user_input_requested_during_turn"), + Some(&json!(true)) + ); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 2b5caf5a52e8..0d89516d3d75 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -57,6 +57,7 @@ mod image_rollout; mod items; mod json_result; mod live_cli; +mod mcp_turn_metadata; mod model_overrides; mod model_switching; mod model_visible_layout; From 88ea41c4f36ac7b18c8ceff59eb143f8e19eded9 Mon Sep 17 00:00:00 2001 From: Michael Chen Date: Thu, 14 May 2026 17:43:56 -0700 Subject: [PATCH 2/2] [Core] Share Apps test helpers --- .../core/tests/common/apps_test_server.rs | 120 +++++++++++++++++- .../core/tests/suite/mcp_turn_metadata.rs | 87 ++----------- codex-rs/core/tests/suite/openai_file_mcp.rs | 41 ++---- codex-rs/core/tests/suite/search_tool.rs | 65 ++-------- 4 files changed, 149 insertions(+), 164 deletions(-) diff --git a/codex-rs/core/tests/common/apps_test_server.rs b/codex-rs/core/tests/common/apps_test_server.rs index 757fc146f2ba..702f96ef2d5e 100644 --- a/codex-rs/core/tests/common/apps_test_server.rs +++ b/codex-rs/core/tests/common/apps_test_server.rs @@ -1,4 +1,10 @@ +use crate::test_codex::TestCodexBuilder; +use crate::test_codex::test_codex; use anyhow::Result; +use codex_core::config::Config; +use codex_features::Feature; +use codex_login::CodexAuth; +use codex_models_manager::bundled_models_response; use serde_json::Value; use serde_json::json; use wiremock::Mock; @@ -15,10 +21,21 @@ const CONNECTOR_NAME: &str = "Calendar"; const DISCOVERABLE_CALENDAR_ID: &str = "connector_2128aebfecb84f64a069897515042a44"; const DISCOVERABLE_GMAIL_ID: &str = "connector_68df038e0ba48191908c8434991bbac2"; const CONNECTOR_DESCRIPTION: &str = "Plan events and manage your calendar."; +const CODEX_APPS_META_KEY: &str = "_codex_apps"; const PROTOCOL_VERSION: &str = "2025-11-25"; const SERVER_NAME: &str = "codex-apps-test"; const SERVER_VERSION: &str = "1.0.0"; const SEARCHABLE_TOOL_COUNT: usize = 100; +const CALENDAR_CREATE_EVENT_TOOL_NAME: &str = "calendar_create_event"; +pub const CALENDAR_EXTRACT_TEXT_TOOL_NAME: &str = "calendar_extract_text"; +const CALENDAR_LIST_EVENTS_TOOL_NAME: &str = "calendar_list_events"; +pub const DIRECT_CALENDAR_CREATE_EVENT_TOOL: &str = "mcp__codex_apps__calendar_create_event"; +pub const DIRECT_CALENDAR_LIST_EVENTS_TOOL: &str = "mcp__codex_apps__calendar_list_events"; +pub const DIRECT_CALENDAR_EXTRACT_TEXT_TOOL: &str = "mcp__codex_apps__calendar_extract_text"; +pub const SEARCH_CALENDAR_NAMESPACE: &str = "mcp__codex_apps__calendar"; +pub const SEARCH_CALENDAR_CREATE_TOOL: &str = "_create_event"; +pub const SEARCH_CALENDAR_EXTRACT_TEXT_TOOL: &str = "_extract_text"; +pub const SEARCH_CALENDAR_LIST_TOOL: &str = "_list_events"; pub const CALENDAR_CREATE_EVENT_RESOURCE_URI: &str = "connector://calendar/tools/calendar_create_event"; pub const CALENDAR_CREATE_EVENT_MCP_APP_RESOURCE_URI: &str = @@ -71,6 +88,103 @@ impl AppsTestServer { } } +pub fn configure_search_capable_model(config: &mut Config) { + let mut model_catalog = bundled_models_response() + .unwrap_or_else(|err| panic!("bundled models.json should parse: {err}")); + let model = model_catalog + .models + .iter_mut() + .find(|model| model.slug == "gpt-5.4") + .expect("gpt-5.4 exists in bundled models.json"); + config.model = Some("gpt-5.4".to_string()); + model.supports_search_tool = true; + config.model_catalog = Some(model_catalog); +} + +fn configure_apps(config: &mut Config, apps_base_url: &str) { + config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + config.chatgpt_base_url = apps_base_url.to_string(); +} + +pub fn configure_search_capable_apps(config: &mut Config, apps_base_url: &str) { + configure_apps(config, apps_base_url); + configure_search_capable_model(config); +} + +pub fn apps_enabled_builder(apps_base_url: impl Into) -> TestCodexBuilder { + let apps_base_url = apps_base_url.into(); + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| configure_apps(config, apps_base_url.as_str())) +} + +pub fn search_capable_apps_builder(apps_base_url: impl Into) -> TestCodexBuilder { + let apps_base_url = apps_base_url.into(); + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| configure_search_capable_apps(config, apps_base_url.as_str())) +} + +fn apps_tool_call_id(body: &Value) -> Option<&str> { + body.get("params")? + .get("_meta")? + .get(CODEX_APPS_META_KEY)? + .get("call_id")? + .as_str() +} + +async fn recorded_apps_tool_calls(server: &MockServer) -> Vec { + server + .received_requests() + .await + .expect("mock server should capture requests") + .into_iter() + .filter_map(|request| { + let body: Value = serde_json::from_slice(&request.body).ok()?; + (request.url.path() == "/api/codex/apps" + && body.get("method").and_then(Value::as_str) == Some("tools/call")) + .then_some(body) + }) + .collect() +} + +pub async fn recorded_apps_tool_call_by_call_id(server: &MockServer, call_id: &str) -> Value { + let matches = recorded_apps_tool_calls(server) + .await + .into_iter() + .filter(|body| apps_tool_call_id(body) == Some(call_id)) + .collect::>(); + assert_eq!( + matches.len(), + 1, + "expected exactly one apps tools/call request for call_id {call_id}" + ); + matches + .into_iter() + .next() + .expect("matching apps tools/call request should be recorded") +} + +pub async fn recorded_apps_tool_call_by_name(server: &MockServer, tool_name: &str) -> Value { + let matches = recorded_apps_tool_calls(server) + .await + .into_iter() + .filter(|body| body.pointer("/params/name").and_then(Value::as_str) == Some(tool_name)) + .collect::>(); + assert_eq!( + matches.len(), + 1, + "expected exactly one apps tools/call request for tool {tool_name}" + ); + matches + .into_iter() + .next() + .expect("matching apps tools/call request should be recorded") +} + async fn mount_oauth_metadata(server: &MockServer) { Mock::given(method("GET")) .and(path("/.well-known/oauth-authorization-server/mcp")) @@ -187,7 +301,7 @@ impl Respond for CodexAppsJsonRpcResponder { "result": { "tools": [ { - "name": "calendar_create_event", + "name": CALENDAR_CREATE_EVENT_TOOL_NAME, "description": "Create a calendar event.", "annotations": { "readOnlyHint": false, @@ -217,7 +331,7 @@ impl Respond for CodexAppsJsonRpcResponder { } }, { - "name": "calendar_list_events", + "name": CALENDAR_LIST_EVENTS_TOOL_NAME, "description": "List calendar events.", "annotations": { "readOnlyHint": true @@ -242,7 +356,7 @@ impl Respond for CodexAppsJsonRpcResponder { } }, { - "name": "calendar_extract_text", + "name": CALENDAR_EXTRACT_TEXT_TOOL_NAME, "description": "Extract text from an uploaded document.", "annotations": { "readOnlyHint": false diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs index 6166e4664e33..17fe33dc8ba1 100644 --- a/codex-rs/core/tests/suite/mcp_turn_metadata.rs +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -5,9 +5,6 @@ use anyhow::Result; use codex_config::types::AppToolApproval; use codex_core::config::Config; use codex_features::Feature; -use codex_login::CodexAuth; -use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY; -use codex_models_manager::bundled_models_response; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -21,6 +18,10 @@ use codex_protocol::request_user_input::RequestUserInputResponse; use codex_protocol::user_input::UserInput; use core_test_support::PathExt; use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; +use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE; +use core_test_support::apps_test_server::recorded_apps_tool_call_by_call_id; +use core_test_support::apps_test_server::search_capable_apps_builder; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -31,47 +32,12 @@ use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; -use core_test_support::test_codex::TestCodexBuilder; -use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; use pretty_assertions::assert_eq; -use serde_json::Value; use serde_json::json; use std::collections::HashMap; -use wiremock::MockServer; - -const SEARCH_CALENDAR_NAMESPACE: &str = "mcp__codex_apps__calendar"; -const SEARCH_CALENDAR_CREATE_TOOL: &str = "_create_event"; - -fn configure_search_capable_model(config: &mut Config) { - let mut model_catalog = bundled_models_response() - .unwrap_or_else(|err| panic!("bundled models.json should parse: {err}")); - let model = model_catalog - .models - .iter_mut() - .find(|model| model.slug == "gpt-5.4") - .expect("gpt-5.4 exists in bundled models.json"); - config.model = Some("gpt-5.4".to_string()); - model.supports_search_tool = true; - config.model_catalog = Some(model_catalog); -} - -fn configure_apps(config: &mut Config, apps_base_url: &str) { - config - .features - .enable(Feature::Apps) - .expect("test config should allow feature update"); - config.chatgpt_base_url = apps_base_url.to_string(); - configure_search_capable_model(config); -} - -fn configured_builder(apps_base_url: String) -> TestCodexBuilder { - test_codex() - .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_config(move |config| configure_apps(config, apps_base_url.as_str())) -} fn set_calendar_approval_mode(config: &mut Config, approval_mode: AppToolApproval) { let approval_mode = match approval_mode { @@ -124,39 +90,6 @@ async fn submit_user_turn( Ok(()) } -fn app_tool_call_id(body: &Value) -> Option<&str> { - body.get("params")? - .get("_meta")? - .get(MCP_TOOL_CODEX_APPS_META_KEY)? - .get("call_id")? - .as_str() -} - -async fn recorded_apps_tool_call(server: &MockServer, call_id: &str) -> Value { - let matches = server - .received_requests() - .await - .expect("mock server should capture requests") - .into_iter() - .filter_map(|request| { - let body: Value = serde_json::from_slice(&request.body).ok()?; - (request.url.path() == "/api/codex/apps" - && body.get("method").and_then(Value::as_str) == Some("tools/call") - && app_tool_call_id(&body) == Some(call_id)) - .then_some(body) - }) - .collect::>(); - assert_eq!( - matches.len(), - 1, - "expected exactly one apps tools/call request for call_id {call_id}" - ); - matches - .into_iter() - .next() - .expect("matching apps tools/call request should be recorded") -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> Result<()> { skip_if_no_network!(Ok(())); @@ -190,8 +123,8 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R ) .await; - let mut builder = - configured_builder(apps_server.chatgpt_base_url.clone()).with_config(|config| { + let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone()) + .with_config(|config| { config .features .enable(Feature::ToolCallMcpElicitation) @@ -241,7 +174,7 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R .await; assert_eq!(mock.requests().len(), 2); - let apps_tool_call = recorded_apps_tool_call(&server, call_id).await; + let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, call_id).await; assert_eq!( apps_tool_call @@ -310,8 +243,8 @@ async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Resul ) .await; - let mut builder = - configured_builder(apps_server.chatgpt_base_url.clone()).with_config(|config| { + let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone()) + .with_config(|config| { set_calendar_approval_mode(config, AppToolApproval::Approve); }); let test = builder.build(&server).await?; @@ -367,7 +300,7 @@ async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Resul .await; assert_eq!(mock.requests().len(), 3); - let apps_tool_call = recorded_apps_tool_call(&server, calendar_call_id).await; + let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, calendar_call_id).await; assert_eq!( apps_tool_call diff --git a/codex-rs/core/tests/suite/openai_file_mcp.rs b/codex-rs/core/tests/suite/openai_file_mcp.rs index 0f0dcf46f132..a25cc36a8677 100644 --- a/codex-rs/core/tests/suite/openai_file_mcp.rs +++ b/codex-rs/core/tests/suite/openai_file_mcp.rs @@ -5,13 +5,16 @@ use std::path::Path; use anyhow::Context; use anyhow::Result; -use codex_core::config::Config; -use codex_features::Feature; -use codex_login::CodexAuth; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::apps_test_server::CALENDAR_EXTRACT_TEXT_TOOL_NAME; +use core_test_support::apps_test_server::DIRECT_CALENDAR_EXTRACT_TEXT_TOOL as DOCUMENT_EXTRACT_HOOK_MATCHER; use core_test_support::apps_test_server::DOCUMENT_EXTRACT_TEXT_RESOURCE_URI; +use core_test_support::apps_test_server::SEARCH_CALENDAR_EXTRACT_TEXT_TOOL as DOCUMENT_EXTRACT_TOOL; +use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE as DOCUMENT_EXTRACT_NAMESPACE; +use core_test_support::apps_test_server::apps_enabled_builder; +use core_test_support::apps_test_server::recorded_apps_tool_call_by_name; use core_test_support::hooks::trust_discovered_hooks; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -20,7 +23,6 @@ use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; -use core_test_support::test_codex::test_codex; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -31,17 +33,6 @@ use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; -const DOCUMENT_EXTRACT_NAMESPACE: &str = "mcp__codex_apps__calendar"; -const DOCUMENT_EXTRACT_TOOL: &str = "_extract_text"; -const DOCUMENT_EXTRACT_HOOK_MATCHER: &str = "mcp__codex_apps__calendar_extract_text"; - -fn configure_apps(config: &mut Config, chatgpt_base_url: &str) { - if let Err(err) = config.features.enable(Feature::Apps) { - panic!("test config should allow feature update: {err}"); - } - config.chatgpt_base_url = chatgpt_base_url.to_string(); -} - fn write_post_tool_use_hook(home: &Path) -> Result<()> { let script_path = home.join("post_tool_use_hook.py"); let log_path = home.join("post_tool_use_hook_log.jsonl"); @@ -154,15 +145,13 @@ async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Res ) .await; - let mut builder = test_codex() - .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url.clone()) .with_pre_build_hook(move |home| { if let Err(error) = write_post_tool_use_hook(home) { panic!("failed to write apps file post tool use hook fixture: {error}"); } }) .with_config(move |config| { - configure_apps(config, apps_server.chatgpt_base_url.as_str()); trust_discovered_hooks(config); }); let test = builder.build(&server).await?; @@ -192,20 +181,8 @@ async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Res })) ); - let apps_tool_call = server - .received_requests() - .await - .unwrap_or_default() - .into_iter() - .find_map(|request| { - let body: Value = serde_json::from_slice(&request.body).ok()?; - (request.url.path() == "/api/codex/apps" - && body.get("method").and_then(Value::as_str) == Some("tools/call") - && body.pointer("/params/name").and_then(Value::as_str) - == Some("calendar_extract_text")) - .then_some(body) - }) - .expect("apps calendar_extract_text tools/call request should be recorded"); + let apps_tool_call = + recorded_apps_tool_call_by_name(&server, CALENDAR_EXTRACT_TEXT_TOOL_NAME).await; assert_eq!( apps_tool_call.pointer("/params/arguments/file"), diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index bb4ca84b2964..69dc74dce347 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -7,7 +7,6 @@ use codex_config::types::McpServerTransportConfig; use codex_core::config::Config; use codex_features::Feature; use codex_login::CodexAuth; -use codex_models_manager::bundled_models_response; use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem; use codex_protocol::dynamic_tools::DynamicToolResponse; use codex_protocol::dynamic_tools::DynamicToolSpec; @@ -21,6 +20,15 @@ use codex_protocol::user_input::UserInput; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::apps_test_server::CALENDAR_CREATE_EVENT_MCP_APP_RESOURCE_URI; use core_test_support::apps_test_server::CALENDAR_CREATE_EVENT_RESOURCE_URI; +use core_test_support::apps_test_server::DIRECT_CALENDAR_CREATE_EVENT_TOOL as CALENDAR_CREATE_TOOL; +use core_test_support::apps_test_server::DIRECT_CALENDAR_LIST_EVENTS_TOOL as CALENDAR_LIST_TOOL; +use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; +use core_test_support::apps_test_server::SEARCH_CALENDAR_LIST_TOOL; +use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE; +use core_test_support::apps_test_server::configure_search_capable_apps; +use core_test_support::apps_test_server::configure_search_capable_model; +use core_test_support::apps_test_server::recorded_apps_tool_call_by_call_id; +use core_test_support::apps_test_server::search_capable_apps_builder as configured_builder; use core_test_support::responses::ResponsesRequest; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -34,7 +42,6 @@ use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::stdio_server_bin; -use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; @@ -48,11 +55,6 @@ const SEARCH_TOOL_DESCRIPTION_SNIPPETS: [&str; 2] = [ "- Calendar: Plan events and manage your calendar.", ]; const TOOL_SEARCH_TOOL_NAME: &str = "tool_search"; -const CALENDAR_CREATE_TOOL: &str = "mcp__codex_apps__calendar_create_event"; -const CALENDAR_LIST_TOOL: &str = "mcp__codex_apps__calendar_list_events"; -const SEARCH_CALENDAR_NAMESPACE: &str = "mcp__codex_apps__calendar"; -const SEARCH_CALENDAR_CREATE_TOOL: &str = "_create_event"; -const SEARCH_CALENDAR_LIST_TOOL: &str = "_list_events"; fn tool_names(body: &Value) -> Vec { body.get("tools") @@ -111,28 +113,6 @@ fn tool_search_output_has_namespace_child( namespace_child_tool(&output, namespace, tool_name).is_some() } -fn configure_search_capable_model(config: &mut Config) { - let mut model_catalog = bundled_models_response() - .unwrap_or_else(|err| panic!("bundled models.json should parse: {err}")); - let model = model_catalog - .models - .iter_mut() - .find(|model| model.slug == "gpt-5.4") - .expect("gpt-5.4 exists in bundled models.json"); - config.model = Some("gpt-5.4".to_string()); - model.supports_search_tool = true; - config.model_catalog = Some(model_catalog); -} - -fn configure_search_capable_apps(config: &mut Config, apps_base_url: &str) { - config - .features - .enable(Feature::Apps) - .expect("test config should allow feature update"); - config.chatgpt_base_url = apps_base_url.to_string(); - configure_search_capable_model(config); -} - fn configure_apps_without_tool_search(config: &mut Config, apps_base_url: &str) { configure_search_capable_apps(config, apps_base_url); config @@ -141,16 +121,6 @@ fn configure_apps_without_tool_search(config: &mut Config, apps_base_url: &str) .expect("test config should allow feature update"); } -fn configure_apps(config: &mut Config, apps_base_url: &str) { - configure_search_capable_apps(config, apps_base_url); -} - -fn configured_builder(apps_base_url: String) -> TestCodexBuilder { - test_codex() - .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_config(move |config| configure_apps(config, apps_base_url.as_str())) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn search_tool_enabled_by_default_adds_tool_search() -> Result<()> { skip_if_no_network!(Ok(())); @@ -321,7 +291,9 @@ async fn search_tool_is_hidden_for_api_key_auth() -> Result<()> { let mut builder = test_codex() .with_auth(CodexAuth::from_api_key("Test API Key")) - .with_config(move |config| configure_apps(config, apps_server.chatgpt_base_url.as_str())); + .with_config(move |config| { + configure_search_capable_apps(config, apps_server.chatgpt_base_url.as_str()) + }); let test = builder.build(&server).await?; test.submit_turn_with_approval_and_permission_profile( @@ -585,18 +557,7 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() - assert_eq!(requests.len(), 3); let first_request_body = requests[0].body_json(); - let apps_tool_call = server - .received_requests() - .await - .unwrap_or_default() - .into_iter() - .find_map(|request| { - let body: Value = serde_json::from_slice(&request.body).ok()?; - (request.url.path() == "/api/codex/apps" - && body.get("method").and_then(Value::as_str) == Some("tools/call")) - .then_some(body) - }) - .expect("apps tools/call request should be recorded"); + let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, "calendar-call-1").await; assert_eq!( apps_tool_call.pointer("/params/_meta/_codex_apps"),