diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 00fc096d22..8d2db30d7b 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -865,6 +865,26 @@ impl ModelClient { Ok(request) } + fn filter_tool_result_metadata(input: &mut [ResponseItem], api_provider: &ApiProvider) { + // Check the resolved destination only when sending, not for local budget estimates. + // HTTP and WS (including v2 compaction) share this raw-metadata-only filter. + let result_metadata_allowed = + url::Url::parse(&api_provider.base_url) + .ok() + .is_some_and(|url| { + url.scheme() == "https" + && url.host_str().is_some_and(|host| { + host == "api.openai.com" + || codex_http_client::is_allowed_chatgpt_host(host) + }) + }); + if !result_metadata_allowed { + for item in input { + item.clear_tool_result_metadata(); + } + } + } + fn prepare_response_items_for_request(&self, input: &mut [ResponseItem]) { for item in input { if item.id().is_some_and(|id| !id.is_prefixed()) { @@ -1479,6 +1499,10 @@ impl ModelClientSession { service_tier.clone(), responses_metadata, )?; + ModelClient::filter_tool_result_metadata( + &mut request.input, + &client_setup.api_provider, + ); self.client.set_guardian_metadata( &mut request.client_metadata, responses_metadata.parent_response_id.as_deref(), @@ -1631,6 +1655,10 @@ impl ModelClientSession { service_tier.clone(), responses_metadata, )?; + ModelClient::filter_tool_result_metadata( + &mut request.input, + &client_setup.api_provider, + ); if endpoint == ResponsesEndpoint::Guardian { request.service_tier = None; } diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index 5ffa239a1b..7f1fa2ef4e 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -44,7 +44,13 @@ use codex_protocol::error::CodexErr; use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::BaseInstructions; use codex_protocol::models::ContentItem; +use codex_protocol::models::ExecutedToolCall; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; +use codex_protocol::models::ToolResultMetadata; +use codex_protocol::models::ToolResultSource; +use codex_protocol::models::ToolResultSources; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; @@ -83,6 +89,11 @@ use tracing_subscriber::layer::Context as LayerContext; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::registry::LookupSpan; use tracing_subscriber::util::SubscriberInitExt; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; const TEST_INSTALLATION_ID: &str = "11111111-1111-4111-8111-111111111111"; @@ -164,6 +175,159 @@ fn test_model_info() -> ModelInfo { .expect("deserialize test model info") } +fn output_with_tool_result_metadata(metadata: ToolResultMetadata) -> ResponseItem { + let mut call = ExecutedToolCall::new("test_tool".to_string(), json!({ "query": "keep" })); + call.set_tool_result_sources(ToolResultSources::new(vec![ToolResultSource { + r#type: "test_resource".to_string(), + id: "R1".to_string(), + }])); + call.set_tool_result_metadata(metadata); + let mut output = ResponseItem::from(ResponseInputItem::FunctionCallOutput { + call_id: "tool-call".to_string(), + output: FunctionCallOutputPayload::from_text("unchanged tool result".to_string()), + }); + output.append_executed_tool_calls(vec![call]); + output.mark_tool_calls_complete(); + output +} + +#[test] +fn responses_request_limits_raw_tool_metadata_to_resolved_first_party_https_endpoint() +-> anyhow::Result<()> { + let provider = + ModelProviderInfo::create_openai_provider(Some("https://api.openai.com/v1".to_string())); + let mut api_provider = provider.to_api_provider(/*auth_mode*/ None)?; + let mut client = test_model_client(SessionSource::Cli); + Arc::get_mut(&mut client.state) + .expect("test client should have unique session state") + .provider = create_model_provider(provider, /*auth_manager*/ None); + let output = output_with_tool_result_metadata(ToolResultMetadata::new(&json!({ + "private": { "resource": "raw-result-metadata" }, + }))); + let without_raw_metadata = output_with_tool_result_metadata(ToolResultMetadata::default()); + let prompt = Prompt { + input: vec![output.clone()], + ..Default::default() + }; + let responses_metadata = test_responses_metadata_for_client( + &client, + /*turn_id*/ None, + format!("{}:0", client.state.thread_id), + /*parent_thread_id*/ None, + TestCodexResponsesRequestKind::Turn, + ); + for (base_url, allowed) in [ + ("https://api.openai.com/v1", true), + ("https://chatgpt.com/backend-api/codex", true), + ("https://api.chatgpt-staging.com/v1", true), + ("https://proxy.example.com/v1", false), + ("http://api.openai.com/v1", false), + ("https://api.openai.com.evil.example/v1", false), + ("https://chatgpt.com.evil.example/v1", false), + ("https://api.openai.com@proxy.example.com/v1", false), + ("not a URL", false), + ] { + api_provider.base_url = base_url.to_string(); + for responses_lite in [false, true] { + let mut model = test_model_info(); + model.use_responses_lite = responses_lite; + let mut request = client.build_responses_request( + &prompt, + &model, + /*effort*/ None, + codex_protocol::config_types::ReasoningSummary::None, + /*service_tier*/ None, + &responses_metadata, + )?; + ModelClient::filter_tool_result_metadata(&mut request.input, &api_provider); + assert_eq!( + request.input.last(), + Some(if allowed { + &output + } else { + &without_raw_metadata + }), + "resolved endpoint: {base_url}, responses_lite: {responses_lite}", + ); + assert_eq!(prompt.input, vec![output.clone()]); + } + } + Ok(()) +} + +#[tokio::test] +async fn responses_http_omits_raw_tool_metadata_for_openai_named_custom_endpoint() +-> anyhow::Result<()> { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with( + ResponseTemplate::new(/*status*/ 200) + .insert_header("content-type", "text/event-stream") + .set_body_string(concat!( + "data: {\"type\":\"response.created\",\"response\":{\"id\":\"resp-1\"}}\n\n", + "data: {\"type\":\"response.completed\",\"response\":{\"id\":\"resp-1\"}}\n\n", + )), + ) + .expect(/*requests*/ 1) + .mount(&server) + .await; + let mut provider = + ModelProviderInfo::create_openai_provider(Some(format!("{}/v1", server.uri()))); + provider.requires_openai_auth = false; + provider.supports_websockets = false; + let mut client = test_model_client(SessionSource::Cli); + Arc::get_mut(&mut client.state) + .expect("test client should have unique session state") + .provider = create_model_provider(provider, /*auth_manager*/ None); + let output = output_with_tool_result_metadata(ToolResultMetadata::new(&json!({ + "private": "raw-result-metadata", + }))); + let prompt = Prompt { + input: vec![output.clone()], + ..Default::default() + }; + let responses_metadata = test_responses_metadata_for_client( + &client, + /*turn_id*/ None, + format!("{}:0", client.state.thread_id), + /*parent_thread_id*/ None, + TestCodexResponsesRequestKind::Turn, + ); + let mut session = client.new_session(); + let mut stream = session + .stream( + &prompt, + &test_model_info(), + &test_session_telemetry(), + /*effort*/ None, + codex_protocol::config_types::ReasoningSummary::None, + /*service_tier*/ None, + &responses_metadata, + &InferenceTraceContext::disabled(), + ) + .await?; + let mut completed = false; + while let Some(event) = stream.next().await { + if let ResponseEvent::Completed { response_id, .. } = event? { + assert_eq!(response_id, "resp-1"); + completed = true; + } + } + assert!(completed); + let requests = server.received_requests().await.expect("received requests"); + assert_eq!(requests.len(), 1); + let body: serde_json::Value = serde_json::from_slice(&requests[0].body)?; + assert_eq!( + body["input"], + serde_json::to_value(vec![output_with_tool_result_metadata( + ToolResultMetadata::default(), + )])?, + ); + assert_eq!(prompt.input, vec![output]); + Ok(()) +} + #[test] fn responses_lite_prefix_ids_track_thread_and_payload() -> anyhow::Result<()> { let thread_id = ThreadId::new(); diff --git a/codex-rs/core/src/test_support.rs b/codex-rs/core/src/test_support.rs index 47427773eb..8ce1f815d7 100644 --- a/codex-rs/core/src/test_support.rs +++ b/codex-rs/core/src/test_support.rs @@ -49,6 +49,21 @@ static TEST_MODEL_PRESETS: Lazy> = Lazy::new(|| { presets }); +/// Reattaches request-only observations to a completed turn's history for capture assertions. +/// Tests inspect this separately from the destination-filtered HTTP/WS request. +pub async fn history_with_tool_call_metadata( + thread: &crate::CodexThread, +) -> Vec { + let history = thread.conversation_history_snapshot().await; + let mut items = history.items().cloned().collect::>(); + thread + .session + .services + .executed_tool_calls + .attach_to_prompt(&mut items, &mut Default::default()); + items +} + /// Test-only provider that supplies no user instructions. #[derive(Debug, Default)] pub struct EmptyUserInstructionsProvider; diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 5e023fd4c0..937277b87e 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -102,6 +102,8 @@ impl ToolInvocation { pub struct McpToolOutput { pub result: CallToolResult, pub tool_input: JsonValue, + // Keep the original metadata for hooks; this flag only controls analytics capture. + pub(crate) result_metadata_capture_allowed: bool, pub wall_time: Duration, pub original_image_detail_supported: bool, pub truncation_policy: TruncationPolicy, @@ -139,6 +141,13 @@ impl ToolOutput for McpToolOutput { self.result.code_mode_result(payload) } + fn tool_result_metadata(&self) -> Option<&JsonValue> { + if !self.result_metadata_capture_allowed { + return None; + } + self.result.meta.as_ref() + } + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { Some(self.tool_input.clone()) } diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index 391b8b0c03..7a393d35d7 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -96,6 +96,7 @@ fn mcp_tool_output_response_item_includes_wall_time() { meta: None, }, tool_input: json!({}), + result_metadata_capture_allowed: false, wall_time: std::time::Duration::from_millis(1250), original_image_detail_supported: false, truncation_policy: TruncationPolicy::Bytes(1024), @@ -142,6 +143,7 @@ fn mcp_tool_output_response_item_truncates_large_structured_content() { meta: None, }, tool_input: json!({}), + result_metadata_capture_allowed: false, wall_time: std::time::Duration::from_millis(1250), original_image_detail_supported: false, truncation_policy: TruncationPolicy::Bytes(128), @@ -192,6 +194,7 @@ fn mcp_tool_output_response_item_preserves_content_items() { meta: None, }, tool_input: json!({}), + result_metadata_capture_allowed: false, wall_time: std::time::Duration::from_millis(500), original_image_detail_supported: false, truncation_policy: TruncationPolicy::Bytes(1024), @@ -251,6 +254,7 @@ fn mcp_tool_output_code_mode_result_preserves_content_without_private_metadata( })), }, tool_input: json!({}), + result_metadata_capture_allowed: false, wall_time: std::time::Duration::from_millis(1250), original_image_detail_supported: false, truncation_policy, diff --git a/codex-rs/core/src/tools/executed_tool_calls.rs b/codex-rs/core/src/tools/executed_tool_calls.rs index c30184d3ff..959699e093 100644 --- a/codex-rs/core/src/tools/executed_tool_calls.rs +++ b/codex-rs/core/src/tools/executed_tool_calls.rs @@ -60,7 +60,7 @@ struct RetainedToolCalls { runtime_cell_id: Option, // Invocation IDs stay local and are retained only with their original output's calls. call_index_by_id: HashMap, - sources_updated: bool, + result_metadata_updated: bool, } #[derive(Default, PartialEq, Eq)] @@ -147,9 +147,9 @@ impl ExecutedToolCalls { result: &dyn ToolOutput, ) { if self.state.is_some() - && let Some(sources) = result.tool_result_sources() + && let Some(metadata) = result.tool_result_metadata() { - self.record_tool_result_sources(source, call_id, sources); + self.record_tool_result_metadata(source, call_id, metadata); } } @@ -282,15 +282,17 @@ impl ExecutedToolCalls { state.pending_nested_calls += 1; } - fn record_tool_result_sources( + fn record_tool_result_metadata( &self, source: &ToolCallSource, call_id: &str, - result_sources: codex_protocol::models::ToolResultSources, + metadata: &JsonValue, ) -> bool { let Some(state) = &self.state else { return false; }; + let metadata = codex_protocol::models::ToolResultMetadata::new(metadata); + let has_metadata = metadata.is_some(); let mut state = state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -304,7 +306,8 @@ impl ExecutedToolCalls { .and_then(|cell| cell.pending_calls.get_mut(call_id)), }; if let Some(call) = call { - return call.set_tool_result_sources(result_sources); + call.set_tool_result_metadata(metadata); + return has_metadata; } let ToolCallSource::CodeMode { cell_id, .. } = source else { return false; @@ -319,8 +322,9 @@ impl ExecutedToolCalls { return false; }; // Older retry copies must not overwrite this output's accepted result metadata. - retained.sources_updated = true; - retained.calls[index].set_tool_result_sources(result_sources) + retained.result_metadata_updated = true; + retained.calls[index].set_tool_result_metadata(metadata); + has_metadata } pub(crate) fn register_cell(&self, cell_id: &CellId, output_call_id: &str) { diff --git a/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs b/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs index 0e75d15e0c..5db02d71e7 100644 --- a/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs +++ b/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs @@ -39,7 +39,7 @@ impl ExecutedToolCalls { !state .retained_calls .get(key) - .is_some_and(|retained| retained.sources_updated) + .is_some_and(|retained| retained.result_metadata_updated) }); let mut pending_retry_outputs = retry_cache.keys().cloned().collect::>(); let mut pending_retained_outputs = @@ -128,7 +128,7 @@ impl ExecutedToolCalls { cell_id: cell_id.clone(), runtime_cell_id, call_index_by_id, - sources_updated: false, + result_metadata_updated: false, }, ); calls @@ -189,11 +189,10 @@ impl ExecutedToolCalls { { let runtime_cell_id = previous.and_then(|retained| retained.runtime_cell_id.clone()); - // Budget rewriting or duplicate outputs invalidate the original index slots. + // Metadata-only shedding preserves slots; changed calls or duplicate outputs do not. let call_index_by_id = previous .filter(|retained| { - unique_output - && metadata.executed_tool_calls.as_ref() == Some(&retained.calls) + unique_output && metadata.has_same_tool_calls(&retained.calls) }) .map(|retained| retained.call_index_by_id.clone()) .unwrap_or_default(); @@ -205,8 +204,8 @@ impl ExecutedToolCalls { } retained.complete |= metadata.tool_calls_complete == Some(true); retained.call_index_by_id = call_index_by_id; - retained.sources_updated = - previous.is_some_and(|retained| retained.sources_updated); + retained.result_metadata_updated = + previous.is_some_and(|retained| retained.result_metadata_updated); } } } diff --git a/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs b/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs index e06e675fa2..e13a0679d6 100644 --- a/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs +++ b/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs @@ -1,6 +1,6 @@ use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; -use codex_protocol::models::ToolResultSources; +use codex_protocol::models::ToolResultMetadata; use pretty_assertions::assert_eq; use serde_json::json; @@ -20,10 +20,10 @@ fn output(call_id: &str) -> ResponseItem { } #[tokio::test] -async fn recorder_preserves_session_and_turn_gates_and_lazy_result_sources() { - struct SourceLookup(std::cell::Cell); +async fn recorder_preserves_session_and_turn_gates_and_lazy_result_metadata() { + struct MetadataLookup(std::cell::Cell, JsonValue); - impl ToolOutput for SourceLookup { + impl ToolOutput for MetadataLookup { fn log_output(&self) -> String { panic!("recording must not read the diagnostic output") } @@ -36,9 +36,9 @@ async fn recorder_preserves_session_and_turn_gates_and_lazy_result_sources() { panic!("recording must not rebuild the tool result") } - fn tool_result_sources(&self) -> Option { + fn tool_result_metadata(&self) -> Option<&JsonValue> { self.0.set(self.0.get() + 1); - Some(ToolResultSources::new(Vec::new())) + Some(&self.1) } } @@ -74,9 +74,9 @@ async fn recorder_preserves_session_and_turn_gates_and_lazy_result_sources() { encrypted_function_args: None, }; calls.record_tool_call(&call, &ToolCallSource::Direct, &step); - let result = SourceLookup(std::cell::Cell::new(0)); + let result = MetadataLookup(std::cell::Cell::new(0), json!({})); calls.record_accepted_result(&ToolCallSource::Direct, &call.call_id, &result); - // Accepted sources follow session availability even when this turn stops recording calls. + // Accepted metadata follows session availability even when this turn stops recording calls. assert_eq!(result.0.get(), usize::from(session_enabled)); let input = serde_json::from_value(json!({ @@ -97,7 +97,7 @@ async fn recorder_preserves_session_and_turn_gates_and_lazy_result_sources() { } let mut expected_call = ExecutedToolCall::new("test_tool".to_string(), json!({"value": 7})); - assert!(expected_call.set_tool_result_sources(ToolResultSources::new(Vec::new()))); + expected_call.set_tool_result_metadata(ToolResultMetadata::new(&json!({}))); let expected_calls = vec![expected_call]; assert_eq!( prompt[1] @@ -128,11 +128,8 @@ fn executed_tool_call_recorder_bounds_pending_calls_and_preserves_overflow() { ToolMode::Direct, ); } - assert!(recorder.record_tool_result_sources( - &ToolCallSource::Direct, - "direct-0", - ToolResultSources::new(Vec::new()) - )); + let metadata = json!({ "arbitrary-key": ["R1", null] }); + assert!(recorder.record_tool_result_metadata(&ToolCallSource::Direct, "direct-0", &metadata)); let cell_id = CellId::new("bounded-cell".to_string()); recorder.start_cell(&cell_id, "bounded-output"); @@ -163,6 +160,10 @@ fn executed_tool_call_recorder_bounds_pending_calls_and_preserves_overflow() { state.direct_calls.len(), MAX_PENDING_EXECUTED_TOOL_CALLS + 1 ); + assert_eq!( + serde_json::to_value(state.direct_calls.get("direct-0").unwrap()).unwrap()["tool_result_metadata"], + metadata, + ); assert_eq!( serde_json::to_value( state @@ -384,16 +385,8 @@ fn tool_call_completeness_requires_finished_lossless_recording() { #[test] fn tool_call_completeness_survives_waits_without_changing_deltas() { - let sources = ToolResultSources::new(vec![codex_protocol::models::ToolResultSource { - r#type: "test_channel".to_string(), - id: "CLATE".to_string(), - }]); - for (truncated, sources) in [ - (false, sources.clone()), - (true, sources), - (false, ToolResultSources::parse_failed()), - (true, ToolResultSources::parse_failed()), - ] { + let metadata = json!({ "provider": { "ids": ["CLATE"] } }); + for truncated in [false, true] { let recorder = new_recorder(); let cell_id = CellId::new("multi-wait".to_string()); recorder.start_cell(&cell_id, "exec"); @@ -431,30 +424,22 @@ fn tool_call_completeness_survives_waits_without_changing_deltas() { cell_id: cell_id.to_string(), runtime_tool_call_id: "runtime-call".to_string(), }; - assert!(!recorder.record_tool_result_sources( + assert!(!recorder.record_tool_result_metadata( &source("other-cell"), "nested-0", - ToolResultSources::new(Vec::new()), + &json!({}), )); let source = source(cell_id.as_str()); - assert!(recorder.record_tool_result_sources( - &source, - "nested-0", - sources.clone(), - )); + assert!(recorder.record_tool_result_metadata(&source, "nested-0", &metadata)); let mut delayed_call = ExecutedToolCall::new("nested_tool".to_string(), json!({})); - assert!(delayed_call.set_tool_result_sources(sources.clone())); + delayed_call.set_tool_result_metadata(ToolResultMetadata::new(&metadata)); let mut delayed_output = output("exec"); delayed_output.append_executed_tool_calls(vec![delayed_call]); delayed_output.set_tool_call_cell_id("exec"); expected[0] = delayed_output; - assert!(recorder.record_tool_result_sources( - &source, - "nested-1", - ToolResultSources::new(Vec::new()) - )); - assert!(call.set_tool_result_sources(ToolResultSources::new(Vec::new()))); + assert!(recorder.record_tool_result_metadata(&source, "nested-1", &json!({}))); + call.set_tool_result_metadata(ToolResultMetadata::new(&json!({}))); } expected_output.append_executed_tool_calls(vec![call]); } else if index == 3 { @@ -548,10 +533,10 @@ fn request_truncation_prevents_completion_after_compaction() { runtime_tool_call_id: "runtime-call".to_string(), }; for index in 0..4 { - assert!(!recorder.record_tool_result_sources( + assert!(!recorder.record_tool_result_metadata( &source, &format!("nested-{index}"), - ToolResultSources::new(Vec::new()), + &json!({}), )); } @@ -569,7 +554,58 @@ fn request_truncation_prevents_completion_after_compaction() { } #[test] -fn source_shedding_preserves_completion_after_compaction() { +fn result_metadata_updates_the_exact_retained_call_and_marks_oversized_retries() { + let recorder = new_recorder(); + let cell_id = CellId::new("metadata-cell".to_string()); + recorder.start_cell(&cell_id, "exec"); + let call = ExecutedToolCall::new("nested_tool".to_string(), json!({})); + for call_id in ["first", "second"] { + recorder.record_nested_tool_call( + cell_id.clone(), + call_id.to_string(), + call.clone(), + /*original_bytes*/ 2, + ); + } + recorder.finish_cell_recording(&cell_id); + let mut retry_cache = HashMap::new(); + let mut initial = [output("exec")]; + assert!(recorder.attach_pending_to_prompt(&mut initial, &mut retry_cache)); + + let metadata = json!({ "all-keys": { "ids": ["R1"] }, "other": true }); + let source = |cell_id: &str| ToolCallSource::CodeMode { + cell_id: cell_id.to_string(), + runtime_tool_call_id: "runtime-call".to_string(), + }; + assert!(!recorder.record_tool_result_metadata(&source("other-cell"), "first", &metadata)); + assert!(!recorder.record_tool_result_metadata(&source(cell_id.as_str()), "unknown", &metadata)); + assert!(recorder.record_tool_result_metadata(&source(cell_id.as_str()), "first", &metadata)); + + let mut expected_call = call.clone(); + expected_call.set_tool_result_metadata(ToolResultMetadata::new(&metadata)); + let mut expected = output("exec"); + expected.append_executed_tool_calls(vec![expected_call, call]); + expected.set_tool_call_cell_id("exec"); + expected.mark_tool_calls_complete(); + let mut retry = [output("exec")]; + assert!(recorder.attach_pending_to_prompt(&mut retry, &mut retry_cache)); + assert_eq!(retry, [expected]); + + assert!(recorder.record_tool_result_metadata( + &source(cell_id.as_str()), + "first", + &json!({ "oversized": "x".repeat(32 * 1024) }), + )); + let mut expected = serde_json::to_value(&initial).unwrap(); + expected[0]["internal_chat_message_metadata_passthrough"]["executed_tool_calls"][0]["tool_result_metadata"] = + json!("omitted_due_to_size_limit"); + let mut retry = [output("exec")]; + assert!(recorder.attach_pending_to_prompt(&mut retry, &mut retry_cache)); + assert_eq!(serde_json::to_value(&retry).unwrap(), expected); +} + +#[test] +fn result_metadata_shedding_preserves_completion_after_compaction() { let recorder = new_recorder(); let cell_id = CellId::new("compacted-cell".to_string()); recorder.start_cell(&cell_id, "exec"); @@ -592,52 +628,48 @@ fn source_shedding_preserves_completion_after_compaction() { }; let mut retry_cache = HashMap::new(); recorder.attach_pending_to_prompt(&mut [output("exec")], &mut retry_cache); - assert!( - recorder.record_tool_result_sources( - &source, - "nested-0", - ToolResultSources::new( - (0..32) - .map(|index| codex_protocol::models::ToolResultSource { - r#type: "test_channel".to_string(), - id: format!("C{index:063}"), - }) - .collect() - ), - ) - ); + // This snapshot fits alone, but pushes the four calls over the aggregate budget. + assert!(recorder.record_tool_result_metadata( + &source, + "nested-0", + &json!({ "provider_data": "x".repeat(4 * 1024) }), + )); - assert!(recorder.record_tool_result_sources( + assert!(recorder.record_tool_result_metadata( &source, "nested-1", - ToolResultSources::parse_failed() + &json!({ "status": "unavailable" }) )); let mut initial = [output("exec")]; assert!(recorder.attach_pending_to_prompt(&mut initial, &mut retry_cache)); let calls = initial[0] .executed_tool_call_metadata() .and_then(|metadata| metadata.executed_tool_calls.as_ref()) - .expect("source shedding must preserve recorded calls"); - assert_eq!( - calls, - &vec![ExecutedToolCall::new("nested_tool".to_string(), arguments); 4] - ); - assert!(!recorder.record_tool_result_sources( - &source, - "nested-0", - ToolResultSources::new(Vec::new()) - )); + .expect("metadata shedding must preserve recorded calls"); + let mut expected_calls = vec![ExecutedToolCall::new("nested_tool".to_string(), arguments); 4]; + for call in &mut expected_calls[..2] { + call.set_tool_result_metadata(ToolResultMetadata::new(&json!("omitted_due_to_size_limit"))); + } + assert_eq!(calls, &expected_calls); let mut retry = [output("exec")]; assert!(recorder.attach_pending_to_prompt(&mut retry, &mut retry_cache)); assert_eq!(retry, initial); + // A different call can still attach a small result after the oversized metadata is shed. + let metadata = json!({ "id": "R2" }); + assert!(recorder.record_tool_result_metadata(&source, "nested-2", &metadata)); + let mut expected_calls = calls.clone(); + expected_calls[2].set_tool_result_metadata(ToolResultMetadata::new(&metadata)); + let mut expected = output("exec"); + expected.append_executed_tool_calls(expected_calls); + expected.set_tool_call_cell_id("exec"); + let mut retry = [output("exec")]; + assert!(recorder.attach_pending_to_prompt(&mut retry, &mut retry_cache)); + assert_eq!(retry, [expected]); + recorder.attach_pending_to_prompt(&mut [], &mut retry_cache); assert!(retry_cache.is_empty()); - assert!(!recorder.record_tool_result_sources( - &source, - "nested-1", - ToolResultSources::new(Vec::new()) - )); + assert!(!recorder.record_tool_result_metadata(&source, "nested-2", &metadata)); recorder.register_cell(&cell_id, "wait"); recorder.finish_cell_recording(&cell_id); let mut final_output = [output("wait")]; diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 5a7642085b..f7ebcdd98f 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -184,6 +184,9 @@ impl McpHandler { self.tool_info.tool.name.as_ref(), ) .await; + // Use the executed call's binding; a later catalog refresh must not change eligibility. + // Only the new metadata is internal; tool execution and call accounting are not. + let result_metadata_capture_allowed = false; let mcp_tool = prepared_mcp_call.as_ref().map(|call| { McpToolContext::from_prepared_call( call, @@ -241,6 +244,7 @@ impl McpHandler { Ok(boxed_tool_output(McpToolOutput { result: result.result, tool_input: result.tool_input, + result_metadata_capture_allowed, wall_time: started.elapsed(), original_image_detail_supported: can_request_original_image_detail( &step_context.settings.model_info, @@ -566,6 +570,7 @@ mod tests { use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::turn_diff_tracker::TurnDiffTracker; + use codex_features::Feature; use pretty_assertions::assert_eq; use serde_json::json; use std::time::Duration; @@ -692,11 +697,21 @@ mod tests { "file_id": "file_123" } }), + result_metadata_capture_allowed: false, wall_time: Duration::from_millis(42), original_image_detail_supported: true, truncation_policy: codex_utils_output_truncation::TruncationPolicy::Bytes(1024), }; let (session, turn) = make_session_and_context().await; + let mut session = session; + let mut turn = turn; + Arc::make_mut(&mut turn.config) + .features + .enable(Feature::ExecutedToolCallMetadata) + .expect("test feature must be configurable"); + let recorder = + crate::tools::executed_tool_calls::ExecutedToolCalls::new(&turn.config.features); + session.services.executed_tool_calls = recorder.clone(); let turn = Arc::new(turn); let handler = McpHandler::new(tool_info("filesystem", "filesystem", "read_file")) .expect("MCP tool spec should build"); @@ -730,6 +745,34 @@ mod tests { }), }) ); + + // Reuse the direct invocation to catch recording placed after the Code Mode-only return. + let mut output = output; + output.result.meta = Some(json!({ "provider/custom": { "items": [1, null] } })); + output.result_metadata_capture_allowed = true; + recorder.record_tool_call( + &crate::tools::router::ToolCall { + tool_name: invocation.tool_name.clone(), + call_id: invocation.call_id.clone(), + payload: invocation.payload.clone(), + encrypted_function_args: None, + }, + &invocation.source, + &invocation.step_context, + ); + handler.on_tool_result_accepted(&invocation, &output); + let mut items = [codex_protocol::models::ResponseItem::from( + output.to_response_item(&invocation.call_id, &invocation.payload), + )]; + recorder.attach_to_prompt(&mut items, &mut Default::default()); + assert_eq!( + serde_json::to_value(items[0].executed_tool_call_metadata()).unwrap()["executed_tool_calls"], + json!([{ + "name": codex_tools::code_mode_name_for_tool_name(&invocation.tool_name), + "arguments": { "path": "/tmp/notes.txt" }, + "tool_result_metadata": { "provider/custom": { "items": [1, null] } }, + }]), + ); } #[test] diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 4e7d5a1a89..7cc8c128c3 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -243,8 +243,8 @@ impl ToolOutput for PostToolUseFeedbackOutput { self.original.code_mode_result(payload) } - fn tool_result_sources(&self) -> Option { - self.original.tool_result_sources() + fn tool_result_metadata(&self) -> Option<&Value> { + self.original.tool_result_metadata() } } diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index edc68df33b..b09d12ba15 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -609,6 +609,7 @@ fn post_tool_use_feedback_output_preserves_fallback_token_limit_override( meta: None, }, tool_input: serde_json::json!({}), + result_metadata_capture_allowed: false, wall_time: Duration::ZERO, original_image_detail_supported: false, truncation_policy, @@ -691,6 +692,43 @@ fn post_tool_use_feedback_output_keeps_code_mode_result_typed() { ); } +#[test_case::test_case(false; "success")] +#[test_case::test_case(true; "tool error")] +fn post_tool_use_feedback_output_preserves_mcp_result_metadata(tool_error: bool) { + let metadata = serde_json::json!({ + "openai/resource_access": { "resources": [] }, + "provider/custom": { "items": [1, null, { "value": true }] }, + }); + let result = PostToolUseFeedbackOutput { + original: Box::new(crate::tools::context::McpToolOutput { + result: codex_protocol::mcp::CallToolResult { + content: vec![serde_json::json!({ + "type": "text", + "text": "original result", + })], + structured_content: None, + is_error: Some(tool_error), + meta: Some(metadata.clone()), + }, + tool_input: serde_json::json!({ "query": "rewritten query" }), + result_metadata_capture_allowed: true, + wall_time: std::time::Duration::ZERO, + original_image_detail_supported: false, + truncation_policy: codex_utils_output_truncation::TruncationPolicy::Bytes(64), + }), + model_visible: FunctionToolOutput::from_text( + "unrelated hook feedback".to_string(), + /*success*/ None, + ), + }; + let payload = ToolPayload::Function { + arguments: "{}".to_string(), + }; + let original_result = result.original.code_mode_result(&payload); + assert_eq!(result.tool_result_metadata(), Some(&metadata)); + assert_eq!(result.code_mode_result(&payload), original_result); +} + #[tokio::test] async fn dispatch_uses_canonical_tool_names_for_lifecycle_contributors() -> anyhow::Result<()> { let (mut session, turn) = crate::session::tests::make_session_and_context().await; diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 3263b9e4c9..afe211268f 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -29,7 +29,13 @@ use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ServiceTier; use codex_protocol::models::BaseInstructions; use codex_protocol::models::ContentItem; +use codex_protocol::models::ExecutedToolCall; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; +use codex_protocol::models::ToolResultMetadata; +use codex_protocol::models::ToolResultSource; +use codex_protocol::models::ToolResultSources; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelServiceTier; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; @@ -201,6 +207,56 @@ async fn responses_websocket_preserves_credit_usage_metadata() { server.shutdown().await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn responses_websocket_omits_raw_tool_metadata_for_openai_named_custom_endpoint() { + skip_if_no_network!(); + + let server = start_websocket_server(vec![vec![vec![ + ev_response_created("resp-1"), + ev_completed("resp-1"), + ]]]) + .await; + let harness = websocket_harness_for_codex_backend(&server).await; + let mut call = ExecutedToolCall::new("test_tool".to_string(), json!({ "query": "keep" })); + call.set_tool_result_sources(ToolResultSources::new(vec![ToolResultSource { + r#type: "test_resource".to_string(), + id: "R1".to_string(), + }])); + call.set_tool_result_metadata(ToolResultMetadata::new(&json!({ + "private": "raw-result-metadata", + }))); + let mut output = ResponseItem::from(ResponseInputItem::FunctionCallOutput { + call_id: "tool-call".to_string(), + output: FunctionCallOutputPayload::from_text("unchanged tool result".to_string()), + }); + output.append_executed_tool_calls(vec![call]); + output.mark_tool_calls_complete(); + let prompt = prompt_with_input(vec![output.clone()]); + let mut expected = serde_json::to_value(&output).unwrap(); + expected["internal_chat_message_metadata_passthrough"]["executed_tool_calls"][0] + .as_object_mut() + .unwrap() + .remove("tool_result_metadata"); + + let mut client_session = harness.client.new_session(); + stream_until_complete_with_model_info( + &mut client_session, + &harness, + &prompt, + &harness.model_info, + "resp-1", + ) + .await; + + let connection = server.single_connection(); + assert_eq!(connection.len(), 1); + let body = connection.first().expect("missing request").body_json(); + assert_eq!(body["type"], "response.create"); + assert_eq!(body["input"], json!([expected])); + assert_eq!(prompt.input, vec![output]); + server.shutdown().await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn responses_websocket_streams_request() { skip_if_no_network!(); diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 387af5e852..76bf9cd661 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -12,7 +12,11 @@ use codex_core::config::Constrained; use codex_core::config::CurrentTimeReminderConfig; use codex_core::context::NodeReplReviewEvidence; use codex_extension_api::ExtensionData; +use codex_extension_api::ExtensionFuture; use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::McpServerContribution; +use codex_extension_api::McpServerContributionContext; +use codex_extension_api::McpServerContributor; use codex_extension_api::ToolCallOutcome; use codex_extension_api::ToolContributor; use codex_extension_api::ToolFinishInput; @@ -22,6 +26,8 @@ use codex_extension_api::ToolStartInput; use codex_features::CurrentTimeSource; use codex_features::Feature; use codex_login::CodexAuth; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_mcp::codex_apps_mcp_server_config; use codex_models_manager::bundled_models_response; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; @@ -111,7 +117,9 @@ use test_case::test_case; use tokio::sync::oneshot; use wiremock::Mock; use wiremock::MockServer; +use wiremock::Request; use wiremock::ResponseTemplate; +use wiremock::matchers::body_partial_json; use wiremock::matchers::method; use wiremock::matchers::path; @@ -1335,6 +1343,422 @@ await new Promise(() => {}); Ok(()) } +const RESULT_METADATA_TOOL: &str = "message_search"; +const RESULT_METADATA_PRIVATE_RESULT: &str = "connector result text is not result metadata"; + +struct ResultMetadataTestControl { + server: Mutex, + gate: Mutex, oneshot::Receiver<()>)>>, +} + +impl McpServerContributor for ResultMetadataTestControl { + fn id(&self) -> &'static str { + "result_metadata_apps" + } + + fn contribute<'a>( + &'a self, + _context: McpServerContributionContext<'a, Config>, + ) -> ExtensionFuture<'a, Vec> { + let server = self.server.lock().unwrap().clone(); + Box::pin(async move { vec![server] }) + } +} + +impl ToolLifecycleContributor for ResultMetadataTestControl { + fn on_tool_finish<'a>(&'a self, input: ToolFinishInput<'a>) -> ToolLifecycleFuture<'a> { + Box::pin(async move { + if !input.tool_name.name.ends_with(RESULT_METADATA_TOOL) { + return; + } + let gate = self.gate.lock().unwrap().take(); + if let Some((reached, release)) = gate { + reached.send(()).unwrap(); + // Finish notification precedes accepted-result metadata capture. + release + .await + .expect("test should release the accepted result"); + } + }) + } +} + +fn result_metadata_fixture_calls(input: &[Value]) -> impl Iterator { + input.iter().flat_map(|output| { + output + .pointer("/internal_chat_message_metadata_passthrough/executed_tool_calls") + .and_then(Value::as_array) + .into_iter() + .flatten() + .map(move |_| output) + }) +} + +async fn mount_result_metadata_app( + server: &MockServer, + result_metadata: Option, + is_error: bool, +) -> Result { + let apps_server = AppsTestServer::mount(server).await?; + let mut tool_result = serde_json::json!({ + "content": [{ "type": "text", "text": RESULT_METADATA_PRIVATE_RESULT }], + "isError": is_error, + }); + if let Some(metadata) = result_metadata { + tool_result["_meta"] = metadata; + } + for (method_name, result) in [ + ( + "tools/list", + serde_json::json!({ + "tools": [{ + "name": RESULT_METADATA_TOOL, + "annotations": { "readOnlyHint": true }, + "inputSchema": { "type": "object" }, + "_meta": { + "connector_id": "test_connector", + "connector_name": "MessageSearch", + }, + }], + "nextCursor": null, + }), + ), + ("tools/call", tool_result), + ] { + Mock::given(method("POST")) + .and(path("/api/codex/ps/mcp")) + .and(body_partial_json( + serde_json::json!({ "method": method_name }), + )) + .respond_with(move |request: &Request| { + let request: Value = serde_json::from_slice(&request.body).unwrap(); + ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "jsonrpc": "2.0", + "id": request["id"], + "result": result, + })) + }) + .with_priority(/*p*/ 1) + .mount(server) + .await; + } + Ok(apps_server) +} + +fn result_metadata_apps_builder(base_url: String, account_email: &str) -> TestCodexBuilder { + let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode( + serde_json::json!({ + "email": account_email, + "https://api.openai.com/auth": { "chatgpt_account_id": "account_id" }, + }) + .to_string(), + ); + let auth = CodexAuth::from_external_chatgpt_tokens( + &format!("e30.{payload}.signature"), + "account_id", + /*chatgpt_plan_type*/ None, + ) + .unwrap(); + search_capable_apps_builder(base_url) + .with_auth(auth) + .with_config(|config| { + for feature in [ + Feature::CodeMode, + Feature::CodeModeOnly, + Feature::ExecutedToolCallMetadata, + ] { + config.features.enable(feature).unwrap(); + } + }) +} + +fn assert_result_metadata_call( + output: &Value, + arguments: &Value, + expected_metadata: Option, +) { + let metadata = &output["internal_chat_message_metadata_passthrough"]; + let call_name = metadata["executed_tool_calls"][0]["name"].as_str().unwrap(); + assert!(call_name.ends_with(RESULT_METADATA_TOOL)); + let mut expected_call = serde_json::json!({ "name": call_name, "arguments": arguments }); + if let Some(result_metadata) = expected_metadata { + expected_call["tool_result_metadata"] = result_metadata; + } + assert_eq!( + metadata["executed_tool_calls"], + serde_json::json!([expected_call]) + ); + assert!(metadata.get("tool_result_metadata").is_none()); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[test_case(true, true, true, false, "employee@openai.com"; "copies_full_metadata_without_rules")] +#[test_case(true, true, true, true, "employee@openai.com"; "accepted_error_keeps_metadata")] +#[test_case(true, true, false, false, "employee@openai.com"; "missing_metadata_stays_absent")] +#[test_case(false, true, true, false, "employee@openai.com"; "feature_off_does_not_record")] +#[test_case(true, false, true, false, "employee@openai.com"; "extension_owned_apps_do_not_record")] +#[test_case(true, true, true, false, "employee@example.com"; "external_user_keeps_calls_without_metadata")] +#[test_case(true, true, true, false, "employee@openai.com.attacker.invalid"; "lookalike_domain_does_not_record")] +async fn code_mode_result_metadata_follows_call_binding( + metadata_enabled: bool, + host_owned: bool, + has_metadata: bool, + is_error: bool, + account_email: &str, +) -> Result<()> { + skip_if_no_network!(Ok(())); + let server = responses::start_mock_server().await; + let result_metadata = has_metadata.then(|| { + serde_json::json!({ + "openai/resource_access": { + "resource_coverage": "incomplete", + "coverage_reasons": ["tool_error"], + }, + "provider_state": { + "items": [{ "id": "room-42", "labels": ["one", "two"] }], + "ready": true, + "count": 2, + "missing": null, + }, + }) + }); + let apps_server = mount_result_metadata_app(&server, result_metadata.clone(), is_error).await?; + let mut builder = + result_metadata_apps_builder(apps_server.chatgpt_base_url.clone(), account_email) + .with_config(move |config| { + if !metadata_enabled { + config + .features + .disable(Feature::ExecutedToolCallMetadata) + .unwrap(); + } + }); + if !host_owned { + let mut extensions = ExtensionRegistryBuilder::::new(); + extensions.mcp_server_contributor(Arc::new(ResultMetadataTestControl { + server: Mutex::new(McpServerContribution::Set { + name: CODEX_APPS_MCP_SERVER_NAME.to_string(), + config: Box::new(codex_apps_mcp_server_config( + &apps_server.chatgpt_base_url, + /*apps_mcp_product_sku*/ None, + /*originator*/ None, + )), + }), + gate: Mutex::new(None), + })); + builder = builder.with_extensions(Arc::new(extensions.build())); + } + let arguments = serde_json::json!({ "search": "launch plan" }); + let code = format!( + "const tool = ALL_TOOLS.find(({{ name }}) => name.endsWith(\"{RESULT_METADATA_TOOL}\")); \ + const result = await tools[tool.name]({arguments}); \ + text(JSON.stringify({{ isError: Boolean(result.isError), hasMeta: Object.hasOwn(result, \"_meta\") }}));" + ); + let (test, follow_up) = + run_code_mode_turn_with_builder(&server, "Search a connected app", &code, builder).await?; + let request = follow_up.single_request(); + assert_eq!(recorded_apps_tool_calls(&server).await.len(), 1); + assert!( + !request + .body_json() + .to_string() + .contains(RESULT_METADATA_PRIVATE_RESULT) + ); + let (body, success) = custom_tool_output_body_and_success(&request, "call-1"); + assert_ne!(success, Some(false), "Code Mode failed: {body}"); + assert_eq!( + serde_json::from_str::(&body)?, + serde_json::json!({ "isError": is_error, "hasMeta": false }), + ); + assert_eq!( + result_metadata_fixture_calls(&request.input()).count(), + usize::from(metadata_enabled), + ); + let output = request.custom_tool_call_output("call-1"); + if metadata_enabled { + // The custom inference endpoint gets no raw metadata; inspect capture independently. + assert_result_metadata_call(&output, &arguments, /*expected_metadata*/ None); + let captured = codex_core::test_support::history_with_tool_call_metadata(&test.codex).await; + let captured = serde_json::to_value(captured)?; + let captured_output = captured + .as_array() + .unwrap() + .iter() + .find(|item| item["type"] == "custom_tool_call_output" && item["call_id"] == "call-1") + .expect("captured exec output"); + // The public build never captures result metadata, even with employee test credentials. + let expected_metadata = None; + assert_result_metadata_call(captured_output, &arguments, expected_metadata); + assert_eq!( + output["internal_chat_message_metadata_passthrough"]["tool_calls_complete"], + true + ); + } else { + assert!( + output["internal_chat_message_metadata_passthrough"] + .get("tool_calls_complete") + .is_none() + ); + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_result_metadata_keeps_prepared_call_binding_across_runtime_refresh() -> Result<()> +{ + skip_if_no_network!(Ok(())); + let server = responses::start_mock_server().await; + let original_metadata = + serde_json::json!({ "provider": { "origin": "original", "items": [] } }); + let apps_server = mount_result_metadata_app( + &server, + Some(original_metadata.clone()), + /*is_error*/ false, + ) + .await?; + let refreshed_server = responses::start_mock_server().await; + let refreshed_apps = mount_result_metadata_app( + &refreshed_server, + Some(serde_json::json!({ "provider": { "origin": "refreshed" } })), + /*is_error*/ false, + ) + .await?; + let (reached_tx, reached_rx) = oneshot::channel(); + let (release_tx, release_rx) = oneshot::channel(); + let control = Arc::new(ResultMetadataTestControl { + server: Mutex::new(McpServerContribution::HostedApps { + config: Box::new(codex_apps_mcp_server_config( + &apps_server.chatgpt_base_url, + /*apps_mcp_product_sku*/ None, + /*originator*/ None, + )), + }), + gate: Mutex::new(Some((reached_tx, release_rx))), + }); + let mut extensions = ExtensionRegistryBuilder::::new(); + extensions.mcp_server_contributor(control.clone()); + extensions.tool_lifecycle_contributor(control.clone()); + let builder = result_metadata_apps_builder(apps_server.chatgpt_base_url, "employee@openai.com") + .with_extensions(Arc::new(extensions.build())); + let arguments = serde_json::json!({ + "query": "launch plan", + "response_format": "detailed", + "content_types": "messages", + }); + let code = format!( + "const tool = ALL_TOOLS.find(({{ name }}) => name.endsWith(\"{RESULT_METADATA_TOOL}\")); \ + const pending = tools[tool.name]({arguments}); yield_control(); await pending; \ + await tools[tool.name]({arguments}); text(\"done\");" + ); + let (test, follow_up) = + run_code_mode_turn_with_builder(&server, "Search a connected app", &code, builder).await?; + let first_items = custom_tool_output_items(&follow_up.single_request(), "call-1"); + assert!( + text_item(&first_items, /*index*/ 0).starts_with("Script running with cell ID "), + "expected the held call to yield: {first_items:?}", + ); + let cell_id = extract_running_cell_id(text_item(&first_items, /*index*/ 0)); + tokio::time::timeout(Duration::from_secs(10), reached_rx).await??; + + // Yield may precede call admission. Drain inventory after the lifecycle gate is reached; + // the short wait is only a poll budget while the gate keeps acceptance blocked. + let held_wait = responses::mount_function_call_agent_response( + &server, + "call-2", + &serde_json::to_string(&serde_json::json!({ + "cell_id": cell_id, + "yield_time_ms": 1, + }))?, + "wait", + ) + .await; + test.submit_turn("Read the pending call inventory").await?; + let held_request = held_wait.completion.single_request(); + let held_items = function_tool_output_items(&held_request, "call-2"); + assert_eq!( + extract_running_cell_id(text_item(&held_items, /*index*/ 0)), + cell_id + ); + let held_input = held_request.input(); + let emitted_calls = result_metadata_fixture_calls(&held_input).collect::>(); + assert_eq!( + emitted_calls.len(), + 1, + "held wait must not duplicate inventory" + ); + let original_output = emitted_calls[0]; + assert_result_metadata_call(original_output, &arguments, /*expected_metadata*/ None); + assert_ne!( + original_output["internal_chat_message_metadata_passthrough"]["tool_calls_complete"], + true + ); + // The next call uses an extension-owned binding, but the held call keeps its host proof. + *control.server.lock().unwrap() = McpServerContribution::Set { + name: CODEX_APPS_MCP_SERVER_NAME.to_string(), + config: Box::new(codex_apps_mcp_server_config( + &refreshed_apps.chatgpt_base_url, + /*apps_mcp_product_sku*/ None, + /*originator*/ None, + )), + }; + test.codex.refresh_runtime_config(test.config.clone()).await; + release_tx.send(()).unwrap(); + let wait = responses::mount_function_call_agent_response( + &server, + "call-3", + &serde_json::to_string(&serde_json::json!({ + "cell_id": cell_id, + "yield_time_ms": 10_000, + }))?, + "wait", + ) + .await; + test.submit_turn("Wait for the accepted app result").await?; + let request = wait.completion.single_request(); + assert_eq!(recorded_apps_tool_calls(&server).await.len(), 1); + assert_eq!(recorded_apps_tool_calls(&refreshed_server).await.len(), 1); + assert!( + !request + .body_json() + .to_string() + .contains(RESULT_METADATA_PRIVATE_RESULT) + ); + let input = request.input(); + assert_eq!( + result_metadata_fixture_calls(&input).count(), + 2, + "late results must not duplicate the call" + ); + let captured = codex_core::test_support::history_with_tool_call_metadata(&test.codex).await; + let captured = serde_json::to_value(captured)?; + // A's accepted result must update the output that first reported it, not the final wait. + let expected_metadata = None; + for (call_id, call_type, expected_metadata) in [ + ( + original_output["call_id"].as_str().unwrap(), + original_output["type"].as_str().unwrap(), + expected_metadata, + ), + ("call-3", "function_call_output", None), + ] { + let output = request.call_output(call_id, call_type); + assert_result_metadata_call(&output, &arguments, /*expected_metadata*/ None); + let captured_output = captured + .as_array() + .unwrap() + .iter() + .find(|item| item["type"] == call_type && item["call_id"] == call_id) + .expect("captured tool output"); + assert_result_metadata_call(captured_output, &arguments, expected_metadata); + } + let terminal_output = request.function_call_output("call-3"); + assert_eq!( + terminal_output["internal_chat_message_metadata_passthrough"]["tool_calls_complete"], + true + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_only_guides_all_tools_search_and_calls_deferred_app_tools() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/login/src/internal_identity.rs b/codex-rs/login/src/internal_identity.rs new file mode 100644 index 0000000000..95ac0cc959 --- /dev/null +++ b/codex-rs/login/src/internal_identity.rs @@ -0,0 +1 @@ +//! Authentication identity helpers for optional diagnostics. diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index f2fe1af541..faff4b9824 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -44,6 +44,7 @@ pub use executed_tool_calls::ExecutedToolCall; pub use executed_tool_calls::ExecutedToolCallArguments; pub use executed_tool_calls::ExecutedToolCallTruncation; pub use executed_tool_calls::MAX_TOOL_RESULT_SOURCE_FIELD_BYTES; +pub use executed_tool_calls::ToolResultMetadata; pub use executed_tool_calls::ToolResultSource; pub use executed_tool_calls::ToolResultSources; pub use executed_tool_calls::bound_executed_tool_calls_for_prompt; diff --git a/codex-rs/protocol/src/models/executed_tool_calls.rs b/codex-rs/protocol/src/models/executed_tool_calls.rs index b07ffba3b5..fa4d1aaf4e 100644 --- a/codex-rs/protocol/src/models/executed_tool_calls.rs +++ b/codex-rs/protocol/src/models/executed_tool_calls.rs @@ -61,7 +61,7 @@ pub fn executed_tool_call_metadata_bytes(item: &ResponseItem) -> usize { } impl InternalChatMessageMetadataPassthrough { - /// Compares call order, names and arguments, ignoring optional source metadata. + /// Compares call order, names and arguments, ignoring optional result metadata. pub fn has_same_tool_calls(&self, calls: &[ExecutedToolCall]) -> bool { self.executed_tool_calls.as_ref().is_some_and(|recorded| { recorded.len() == calls.len() @@ -128,6 +128,35 @@ fn bound_executed_tool_calls_for_prompt_with_priority( original_metadata_bytes.saturating_add(executed_tool_call_metadata_bytes(item)); } + // Raw result metadata must not displace existing source evidence, calls or completion proof. + if original_metadata_bytes > MAX_EXECUTED_TOOL_CALL_METADATA_BYTES { + original_metadata_bytes = 0; + for item in items.iter_mut() { + if let Some(metadata) = item + .internal_chat_message_metadata_passthrough_mut() + .and_then(Option::as_mut) + { + for call in metadata.executed_tool_calls.iter_mut().flatten() { + if call.tool_result_metadata.is_some() { + call.tool_result_metadata = ToolResultMetadata::omitted_due_to_size_limit(); + } + } + } + original_metadata_bytes = + original_metadata_bytes.saturating_add(executed_tool_call_metadata_bytes(item)); + } + } + + // Omission markers are optional too; keep the original call budget if they cannot fit. + if original_metadata_bytes > MAX_EXECUTED_TOOL_CALL_METADATA_BYTES { + original_metadata_bytes = 0; + for item in items.iter_mut() { + item.clear_tool_result_metadata(); + original_metadata_bytes = + original_metadata_bytes.saturating_add(executed_tool_call_metadata_bytes(item)); + } + } + // Source evidence is optional; dropping it must not discard calls or their completion proof. if original_metadata_bytes > MAX_EXECUTED_TOOL_CALL_METADATA_BYTES { original_metadata_bytes = 0; @@ -336,24 +365,81 @@ pub struct ExecutedToolCall { #[serde(default, skip_deserializing, skip_serializing_if = "Option::is_none")] #[schemars(skip)] #[ts(skip)] - tool_result_sources: Option, + tool_result_sources: Option>, + /// Raw MCP result metadata is host-recorded only, never trusted from input JSON or exposed + /// in public schemas. Its Debug implementation also prevents raw values reaching logs. + #[serde( + default, + skip_deserializing, + skip_serializing_if = "ToolResultMetadata::is_none" + )] + #[schemars(skip)] + #[ts(skip)] + tool_result_metadata: ToolResultMetadata, +} + +/// An entire MCP result's `_meta`, or a string marker when omitted due to the size limit. +#[derive(Clone, Default, Serialize, PartialEq, Eq)] +#[serde(transparent)] +pub struct ToolResultMetadata(Option); + +impl std::fmt::Debug for ToolResultMetadata { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str("ToolResultMetadata([redacted])") + } +} + +impl ToolResultMetadata { + /// Bounds serialization before cloning arbitrary MCP metadata; no keys are filtered. + pub fn new(metadata: &serde_json::Value) -> Self { + let mut limit = MetadataSizeLimit(MAX_EXECUTED_TOOL_CALL_METADATA_BYTES); + if serde_json::to_writer(&mut limit, metadata).is_ok() { + Self(Some(metadata.clone())) + } else { + Self::omitted_due_to_size_limit() + } + } + + fn omitted_due_to_size_limit() -> Self { + // MCP `_meta` is an object; this string is a harness omission status, not provider data. + Self(Some(serde_json::Value::String( + "omitted_due_to_size_limit".to_string(), + ))) + } + + fn is_none(&self) -> bool { + self.0.is_none() + } + + /// Whether the bounded snapshot contains metadata or an omission marker. + pub fn is_some(&self) -> bool { + self.0.is_some() + } +} + +struct MetadataSizeLimit(usize); + +impl std::io::Write for MetadataSizeLimit { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0 = self + .0 + .checked_sub(bytes.len()) + .ok_or_else(|| std::io::Error::other("tool result metadata exceeds the byte limit"))?; + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } } /// A bounded capture update. Omitted updates still clear any previously recorded evidence. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct ToolResultSources(Option); - -#[derive(Debug, Clone, Serialize, PartialEq, Eq)] -enum ToolResultSourcesValue { - #[serde(rename = "parse_failed")] - ParseFailed, - // Preserve the existing array shape for successful captures, including an empty array. - #[serde(untagged)] - Sources(Vec), -} +pub struct ToolResultSources(Option>); impl ToolResultSources { - /// Deduplicates a complete source list, discarding all sources if it exceeds a limit. + /// Deduplicates a captured snapshot, discarding all sources if it exceeds a limit. + /// Capture rules determine coverage; this is not a resource or permission inventory. pub fn new(sources: Vec) -> Self { let mut unique_sources = Vec::new(); for source in sources { @@ -368,16 +454,20 @@ impl ToolResultSources { } unique_sources.push(source); } - Self(Some(ToolResultSourcesValue::Sources(unique_sources))) + Self(Some(unique_sources)) } - /// Records that source parsing was attempted but failed, not that a budget was exceeded. + /// Records a failed parse using the receiver's existing array-of-sources shape. + /// `parse_failed` is a status marker, not a resource type; the required ID is empty. pub fn parse_failed() -> Self { - Self(Some(ToolResultSourcesValue::ParseFailed)) + Self(Some(vec![ToolResultSource { + r#type: "parse_failed".to_string(), + id: String::new(), + }])) } } -/// A trusted source identity observed in an accepted tool result by the host. +/// A captured source ID, or a `parse_failed` status marker with an empty ID. #[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct ToolResultSource { #[serde(rename = "type")] @@ -411,6 +501,7 @@ impl ExecutedToolCall { name, arguments: ExecutedToolCallArguments::Raw(arguments), tool_result_sources: None, + tool_result_metadata: ToolResultMetadata::default(), } } @@ -432,6 +523,11 @@ impl ExecutedToolCall { self.tool_result_sources.is_some() } + /// Replaces the entire `_meta` snapshot, including with an omission marker if oversized. + pub fn set_tool_result_metadata(&mut self, metadata: ToolResultMetadata) { + self.tool_result_metadata = metadata; + } + fn truncation(&self) -> Option<&ExecutedToolCallTruncation> { match &self.arguments { ExecutedToolCallArguments::Raw(_) => None, @@ -510,6 +606,18 @@ impl ResponseItem { self.internal_chat_message_metadata_passthrough() } + /// Omits raw tool results without changing existing call, source or completion metadata. + pub fn clear_tool_result_metadata(&mut self) { + if let Some(metadata) = self + .internal_chat_message_metadata_passthrough_mut() + .and_then(Option::as_mut) + { + for call in metadata.executed_tool_calls.iter_mut().flatten() { + call.tool_result_metadata = ToolResultMetadata::default(); + } + } + } + /// Bounds one request item's attempted calls to its share of the prompt budget. fn bound_executed_tool_calls_with_budget(&mut self, max_metadata_bytes: usize) { let Some(metadata) = self.internal_chat_message_metadata_passthrough_mut() else { diff --git a/codex-rs/protocol/src/models/executed_tool_calls_tests.rs b/codex-rs/protocol/src/models/executed_tool_calls_tests.rs index 34f673a8ea..f0edb398ac 100644 --- a/codex-rs/protocol/src/models/executed_tool_calls_tests.rs +++ b/codex-rs/protocol/src/models/executed_tool_calls_tests.rs @@ -267,7 +267,7 @@ fn tool_call_completeness_is_host_only_and_fail_closed() -> Result<()> { assert_eq!(untrusted, passthrough_metadata("turn-1")); for sources in [ serde_json::json!([{ "type": "test_resource", "id": "ATTACKER" }]), - serde_json::json!("parse_failed"), + serde_json::json!([{ "type": "parse_failed", "id": "" }]), ] { let untrusted_call = serde_json::from_value::(serde_json::json!({ "name": "test_tool", @@ -334,10 +334,7 @@ fn tool_result_source_snapshots_replace_atomically() -> Result<()> { sources.push(sources[0].clone()); let capture = ToolResultSources::new(sources.clone()); sources.truncate(MAX_TOOL_RESULT_SOURCES); - assert_eq!( - capture, - ToolResultSources(Some(ToolResultSourcesValue::Sources(sources.clone()))) - ); + assert_eq!(capture, ToolResultSources(Some(sources.clone()))); assert!(call.set_tool_result_sources(capture)); sources.push(source("test_resource", "OVERFLOW")); let capture = ToolResultSources::new(sources); @@ -361,10 +358,7 @@ fn tool_result_source_snapshots_replace_atomically() -> Result<()> { source("test_resource", &oversized), ] { assert!(call.set_tool_result_sources(ToolResultSources::new(vec![bounded.clone()]))); - assert_eq!( - call.tool_result_sources, - Some(ToolResultSourcesValue::Sources(vec![bounded.clone()])) - ); + assert_eq!(call.tool_result_sources, Some(vec![bounded.clone()])); let capture = ToolResultSources::new(vec![source("test_resource", "R1"), invalid]); assert_eq!(capture, ToolResultSources(None)); assert!(!call.set_tool_result_sources(capture)); @@ -378,7 +372,7 @@ fn tool_result_source_snapshots_replace_atomically() -> Result<()> { serde_json::json!({ "name": "test_tool", "arguments": {}, - "tool_result_sources": "parse_failed", + "tool_result_sources": [{ "type": "parse_failed", "id": "" }], }) ); assert!(call.set_tool_result_sources(ToolResultSources::new(Vec::new()))); @@ -388,3 +382,161 @@ fn tool_result_source_snapshots_replace_atomically() -> Result<()> { ); Ok(()) } + +#[test] +fn tool_result_metadata_is_host_only_bounded_and_redacted() -> Result<()> { + let mut call = ExecutedToolCall::new("test_tool".to_string(), serde_json::json!({})); + let without_metadata = call.clone(); + for metadata in [ + serde_json::json!({ + "arbitrary-key": { "secret": "not-for-debug", "ids": [1, "é", null] }, + "other": false, + }), + serde_json::json!({}), + serde_json::json!(null), + ] { + let capture = ToolResultMetadata::new(&metadata); + assert_eq!(format!("{capture:?}"), "ToolResultMetadata([redacted])"); + assert!(capture.is_some()); + call.set_tool_result_metadata(capture); + let wire = serde_json::to_value(&call)?; + assert_eq!( + wire, + serde_json::json!({ + "name": "test_tool", + "arguments": {}, + "tool_result_metadata": metadata, + }) + ); + assert!(!format!("{call:?}").contains("not-for-debug")); + assert_eq!( + serde_json::from_value::(wire)?, + without_metadata + ); + } + + let exact_limit = serde_json::json!({ + "value": "x".repeat(MAX_EXECUTED_TOOL_CALL_METADATA_BYTES - r#"{"value":""}"#.len()), + }); + let capture = ToolResultMetadata::new(&exact_limit); + assert!(capture.is_some()); + call.set_tool_result_metadata(capture); + assert_eq!( + serde_json::to_value(&call)?["tool_result_metadata"], + exact_limit + ); + // JSON escaping counts toward the bound, not just the in-memory string length. + let oversized = + serde_json::json!({ "value": "\0".repeat(MAX_EXECUTED_TOOL_CALL_METADATA_BYTES / 2) }); + let capture = ToolResultMetadata::new(&oversized); + assert!(capture.is_some()); + call.set_tool_result_metadata(capture); + let wire = serde_json::to_value(&call)?; + assert_eq!( + wire, + serde_json::json!({ + "name": "test_tool", + "arguments": {}, + "tool_result_metadata": "omitted_due_to_size_limit", + }) + ); + assert_eq!( + serde_json::from_value::(wire)?, + without_metadata + ); + Ok(()) +} + +#[test] +fn raw_result_metadata_is_shed_before_sources_calls_or_completion() -> Result<()> { + let mut call = ExecutedToolCall::new("test_tool".to_string(), serde_json::json!({})); + call.set_tool_result_sources(ToolResultSources::new(vec![ToolResultSource { + r#type: "test_resource".to_string(), + id: "R1".to_string(), + }])); + let mut items = ["first", "second", "without-metadata"].map(|call_id| { + let mut item = ResponseItem::from(ResponseInputItem::FunctionCallOutput { + call_id: call_id.to_string(), + output: FunctionCallOutputPayload::from_text("unchanged output".to_string()), + }); + item.append_executed_tool_calls(vec![call.clone()]); + item.set_tool_call_cell_id(call_id); + item.mark_tool_calls_complete(); + item + }); + let mut expected = items.clone(); + let metadata = ToolResultMetadata::new(&serde_json::json!({ "large": "x".repeat(20_000) })); + for item in &mut items[..2] { + first_executed_tool_call(item) + .unwrap() + .set_tool_result_metadata(metadata.clone()); + } + for item in &mut expected[..2] { + first_executed_tool_call(item) + .unwrap() + .set_tool_result_metadata(ToolResultMetadata::new(&serde_json::json!( + "omitted_due_to_size_limit" + ))); + } + assert!( + items + .iter() + .map(executed_tool_call_metadata_bytes) + .sum::() + > MAX_EXECUTED_TOOL_CALL_METADATA_BYTES + ); + bound_executed_tool_calls_for_prompt(&mut items); + assert_eq!(items, expected); + assert!( + items + .iter() + .map(executed_tool_call_metadata_bytes) + .sum::() + <= MAX_EXECUTED_TOOL_CALL_METADATA_BYTES + ); + bound_executed_tool_calls_for_prompt(&mut items); + assert_eq!(items, expected); + Ok(()) +} + +#[test] +fn result_metadata_markers_do_not_displace_existing_evidence() { + let mut item = ResponseItem::from(ResponseInputItem::FunctionCallOutput { + call_id: "exec".to_string(), + output: FunctionCallOutputPayload::from_text("unchanged output".to_string()), + }); + let mut call = ExecutedToolCall::new("test_tool".to_string(), serde_json::json!({})); + call.set_tool_result_sources(ToolResultSources::new(vec![ToolResultSource { + r#type: "test_resource".to_string(), + id: "R1".to_string(), + }])); + item.append_executed_tool_calls(vec![call; 4]); + item.set_tool_call_cell_id("exec"); + item.mark_tool_calls_complete(); + let remaining_bytes = + MAX_EXECUTED_TOOL_CALL_METADATA_BYTES - executed_tool_call_metadata_bytes(&item); + let calls = item + .ensure_tool_call_metadata() + .unwrap() + .executed_tool_calls + .as_mut() + .unwrap(); + for (index, call) in calls.iter_mut().enumerate() { + // Fill the budget exactly, with every argument still below its individual limit. + call.arguments = ExecutedToolCallArguments::Raw(serde_json::json!( + "x".repeat(remaining_bytes / 4 + usize::from(index < remaining_bytes % 4)) + )); + } + assert_eq!( + executed_tool_call_metadata_bytes(&item), + MAX_EXECUTED_TOOL_CALL_METADATA_BYTES + ); + let expected = item.clone(); + first_executed_tool_call(&mut item) + .unwrap() + .set_tool_result_metadata(ToolResultMetadata::new( + &serde_json::json!({ "too-large": "x".repeat(32 * 1024) }), + )); + bound_executed_tool_calls_for_prompt(std::slice::from_mut(&mut item)); + assert_eq!(item, expected); +} diff --git a/codex-rs/tools/src/tool_output.rs b/codex-rs/tools/src/tool_output.rs index a33232e20d..e95e662941 100644 --- a/codex-rs/tools/src/tool_output.rs +++ b/codex-rs/tools/src/tool_output.rs @@ -56,8 +56,8 @@ pub trait ToolOutput: Send { response_input_to_code_mode_result(self.to_response_item("", payload)) } - /// Reports configured source capture only after acceptance; `None` means no capture attempt. - fn tool_result_sources(&self) -> Option { + /// Borrows original host-only metadata for recording, not for model output or logging. + fn tool_result_metadata(&self) -> Option<&JsonValue> { None } } @@ -102,8 +102,8 @@ where (**self).code_mode_result(payload) } - fn tool_result_sources(&self) -> Option { - (**self).tool_result_sources() + fn tool_result_metadata(&self) -> Option<&JsonValue> { + (**self).tool_result_metadata() } }