From aead844f64e911f89e556485e3f47d757431c3b1 Mon Sep 17 00:00:00 2001 From: rka-oai Date: Thu, 20 Aug 2026 19:35:54 +0000 Subject: [PATCH] Handle standalone tool outputs as external context (#39791) ## What changed - Treat standalone `function_call_output` items without a `call_id` as external context, including items injected into new and forked threads, and mark the thread memory mode polluted when `memories.disable_on_external_context` is enabled. - Include these outputs in guardian transcripts with their namespaced tool name and a placeholder for non-text content. - Allow image generation to reuse images from standalone and otherwise unpaired function or custom-tool outputs. ## Testing - Cover injected thread items, external-context detection, guardian transcript rendering, and recent-image selection. GitOrigin-RevId: ca4e8c16a4b731e1256a5956908d00f785b4feb1 --- .../tests/suite/v2/thread_inject_items.rs | 17 ++++++++ codex-rs/core/src/guardian/prompt.rs | 20 +++++++++ codex-rs/core/src/guardian/tests.rs | 39 +++++++++++++++++ codex-rs/core/src/session/mod.rs | 25 ++++++++++- codex-rs/core/src/stream_events_utils.rs | 1 + .../core/src/stream_events_utils_tests.rs | 8 ++++ .../src/async_scorer/transcript.rs | 24 ++++++++++- .../src/async_scorer/transcript_tests.rs | 30 +++++++++++++ codex-rs/ext/image-generation/src/tests.rs | 12 +++--- codex-rs/ext/image-generation/src/tool.rs | 43 +------------------ 10 files changed, 170 insertions(+), 49 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs b/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs index b511dff55b..20415effba 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs @@ -13,10 +13,13 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; use codex_core::RolloutRecorder; use codex_features::Feature; +use codex_protocol::ThreadId; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_rollout::InitialHistory; use codex_rollout::RolloutItem; +use codex_state::StateRuntime; +use codex_utils_absolute_path::test_support::PathExt; use core_test_support::responses; use core_test_support::responses::strip_response_item_id; use core_test_support::responses::strip_response_item_ids_from_json; @@ -40,8 +43,15 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history() -> Resu let codex_home = TempDir::new()?; MockResponsesConfig::new(&server.uri()) + .enable_feature(Feature::Sqlite) .enable_feature(Feature::RetainClientDeveloperMessages) + .with_extra_config("[memories]\ndisable_on_external_context = true") .write(codex_home.path())?; + let state_db = StateRuntime::init( + codex_state::SqliteConfig::new_for_testing(codex_home.path().abs()), + "mock_provider".into(), + ) + .await?; let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -100,6 +110,13 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history() -> Resu .await?; let _response: ThreadInjectItemsResponse = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(inject_req)).await??; + assert_eq!( + state_db + .get_thread_memory_mode(ThreadId::from_string(&thread.id)?) + .await? + .as_deref(), + Some("polluted") + ); let rollout_path = thread.path.as_ref().context("thread path missing")?; let history = RolloutRecorder::get_rollout_history(rollout_path).await?; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index e82d6bf79f..f1b1de3abf 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -595,6 +595,26 @@ pub(crate) fn collect_guardian_transcript_entries<'a>( }; non_empty_entry(kind, text) }), + ResponseItem::FunctionCallOutput { + call_id: None, + name: Some(name), + namespace, + output, + .. + } => { + let text = output + .body + .to_text() + .unwrap_or_else(|| "[non-text output]".into()); + let name = match namespace { + Some(namespace) => format!("{namespace}.{name}"), + None => name.to_string(), + }; + non_empty_entry( + GuardianTranscriptEntryKind::Tool(format!("tool {name} result")), + text, + ) + } _ => None, }; diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 772e680ae4..a15c306a9d 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -40,6 +40,7 @@ use codex_protocol::ThreadId; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelsResponse; @@ -1046,6 +1047,44 @@ fn collect_guardian_transcript_entries_includes_recent_tool_calls_and_output() { )); } +#[test] +fn collect_guardian_transcript_entries_preserves_named_unpaired_tool_sources() { + let mut items = vec![ResponseItem::FunctionCallOutput { + id: None, + call_id: None, + name: Some("notifications".to_string()), + namespace: Some("slack".to_string()), + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "new message".to_string(), + ), + internal_chat_message_metadata_passthrough: None, + }]; + + assert_eq!( + collect_guardian_transcript_entries(&items), + vec![GuardianTranscriptEntry { + kind: GuardianTranscriptEntryKind::Tool("tool slack.notifications result".to_string()), + text: "new message".to_string(), + }] + ); + + if let ResponseItem::FunctionCallOutput { output, .. } = &mut items[0] { + *output = codex_protocol::models::FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,image".to_string(), + detail: None, + }, + ]); + } + assert_eq!( + collect_guardian_transcript_entries(&items), + vec![GuardianTranscriptEntry { + kind: GuardianTranscriptEntryKind::Tool("tool slack.notifications result".to_string()), + text: "[non-text output]".to_string(), + }] + ); +} + #[test] fn guardian_truncate_text_keeps_prefix_suffix_and_xml_marker() { let content = "prefix ".repeat(200) + &" suffix".repeat(200); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index e9b573b67c..a72d3f441b 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -42,6 +42,7 @@ use crate::session::step_context::StepContext; use crate::session::turn_context::TurnEnvironment; use crate::session_prefix::format_inter_agent_completion_message; use crate::skills_load_input_from_config; +use crate::stream_events_utils::mark_thread_memory_mode_polluted_if_external_context; use crate::turn_metadata::TurnMetadataState; use crate::turn_timing::now_unix_timestamp_ms; use async_channel::Receiver; @@ -1346,12 +1347,13 @@ impl Session { let mut state = self.state.lock().await; state.set_next_turn_is_first(!has_prior_user_turns); } - match conversation_history { + let turn_context = match conversation_history { InitialHistory::New | InitialHistory::Cleared => { // Defer initial context insertion until the first real turn starts so // turn/start overrides can be merged before we write model-visible context. self.set_previous_turn_settings(/*previous_turn_settings*/ None) .await; + None } InitialHistory::Resumed(resumed_history) => { let turn_context = self.new_default_turn().await; @@ -1401,6 +1403,7 @@ impl Session { if !is_subagent { let _ = self.flush_rollout().await; } + None } InitialHistory::Forked(mut rollout_items) => { let turn_context = self.new_default_turn().await; @@ -1449,6 +1452,19 @@ impl Session { if !is_subagent { let _ = self.flush_rollout().await; } + Some(turn_context) + } + }; + if let Some(turn_context) = turn_context + && turn_context.config.memories.disable_on_external_context + { + let history = self.clone_history().await; + if let Some(item) = history + .raw_items() + .find(|item| matches!(item, ResponseItem::FunctionCallOutput { call_id: None, .. })) + { + mark_thread_memory_mode_polluted_if_external_context(self, &turn_context, item) + .await; } } } @@ -3087,6 +3103,13 @@ impl Session { let rollout_items: Vec = items.into_iter().map(RolloutItem::ResponseItem).collect(); self.persist_rollout_items(&rollout_items).await; + if turn_context.config.memories.disable_on_external_context + && let Some(item) = response_items + .iter() + .find(|item| matches!(item, ResponseItem::FunctionCallOutput { call_id: None, .. })) + { + mark_thread_memory_mode_polluted_if_external_context(self, turn_context, item).await; + } self.send_raw_response_items(turn_context, &response_items) .await; } diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index 95cce62951..53afbf1d7e 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -135,6 +135,7 @@ fn response_item_may_include_external_context(item: &ResponseItem) -> bool { ResponseItem::ToolSearchCall { .. } | ResponseItem::ToolSearchOutput { .. } | ResponseItem::WebSearchCall { .. } + | ResponseItem::FunctionCallOutput { call_id: None, .. } ) } diff --git a/codex-rs/core/src/stream_events_utils_tests.rs b/codex-rs/core/src/stream_events_utils_tests.rs index 8181ef53bd..20893b7fdd 100644 --- a/codex-rs/core/src/stream_events_utils_tests.rs +++ b/codex-rs/core/src/stream_events_utils_tests.rs @@ -70,6 +70,14 @@ fn external_context_pollution_items_include_web_search_and_tool_search() { tools: Vec::new(), internal_chat_message_metadata_passthrough: None, }, + ResponseItem::FunctionCallOutput { + id: None, + call_id: None, + name: Some("notifications".to_string()), + namespace: Some("slack".to_string()), + output: FunctionCallOutputPayload::from_text("new message".to_string()), + internal_chat_message_metadata_passthrough: None, + }, ]; assert!( diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs b/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs index fe15cf93be..888ef3bd59 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs @@ -247,6 +247,29 @@ impl TranscriptConfig { ); (role, output) } + ResponseItem::FunctionCallOutput { + call_id: None, + name, + namespace, + output, + .. + } => { + if !self.sources.contains(&TranscriptSource::ToolOutputs) { + continue; + } + let output = output + .body + .to_text() + .unwrap_or_else(|| "[non-text output]".into()); + let role = match (name.as_deref(), namespace.as_deref()) { + (Some(name), Some(namespace)) => { + format!("tool {namespace}.{name} result") + } + (Some(name), None) => format!("tool {name} result"), + (None, _) => "tool result".to_owned(), + }; + (role, output) + } ResponseItem::Reasoning { summary, content, .. } => { @@ -289,7 +312,6 @@ impl TranscriptConfig { ("tool web_search call".to_owned(), text) } ResponseItem::AdditionalTools { .. } - | ResponseItem::FunctionCallOutput { call_id: None, .. } | ResponseItem::ImageGenerationCall { .. } | ResponseItem::ToolSearchCall { .. } | ResponseItem::ToolSearchOutput { .. } diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs index 75d5d23628..c49cd6b52b 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs @@ -485,6 +485,36 @@ fn transcript_truncates_tool_results_using_standard_budget() { ); } +#[test] +fn transcript_preserves_named_unpaired_tool_source() { + let mut items = vec![ResponseItem::FunctionCallOutput { + id: None, + call_id: None, + name: Some("notifications".to_owned()), + namespace: Some("slack".to_owned()), + output: FunctionCallOutputPayload::from_text("new message".to_owned()), + internal_chat_message_metadata_passthrough: None, + }]; + + assert_eq!( + TranscriptConfig::default().build(&items), + vec!["[1] tool slack.notifications result: new message\n"] + ); + + if let ResponseItem::FunctionCallOutput { output, .. } = &mut items[0] { + *output = FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,image".to_owned(), + detail: None, + }, + ]); + } + assert_eq!( + TranscriptConfig::default().build(&items), + vec!["[1] tool slack.notifications result: [non-text output]\n"] + ); +} + #[test] fn configured_reasoning_counts_against_message_budget() { let mut items = (0..8) diff --git a/codex-rs/ext/image-generation/src/tests.rs b/codex-rs/ext/image-generation/src/tests.rs index 966b2719da..c7bca61e8a 100644 --- a/codex-rs/ext/image-generation/src/tests.rs +++ b/codex-rs/ext/image-generation/src/tests.rs @@ -138,10 +138,10 @@ async fn recent_image_fallback_selects_newest_images_in_chronological_order() { }, ResponseItem::FunctionCallOutput { id: None, - call_id: Some("orphan-call".to_string()), - name: None, - namespace: None, - output: image_output("orphan"), + call_id: None, + name: Some("notifications".to_string()), + namespace: Some("slack".to_string()), + output: image_output("standalone"), internal_chat_message_metadata_passthrough: None, }, ]; @@ -151,7 +151,7 @@ async fn recent_image_fallback_selects_newest_images_in_chronological_order() { &ImagegenArgs { prompt: "change the lighting".to_string(), referenced_image_paths: None, - num_last_images_to_include: Some(4), + num_last_images_to_include: Some(5), }, &history, &[], @@ -160,7 +160,7 @@ async fn recent_image_fallback_selects_newest_images_in_chronological_order() { .expect("history-backed edit request should build"), ImageRequest::Edit(expected_edit_request( "change the lighting", - &["user-2", "mcp", "code-mode", "generated"], + &["user-2", "mcp", "code-mode", "generated", "standalone"], )) ); } diff --git a/codex-rs/ext/image-generation/src/tool.rs b/codex-rs/ext/image-generation/src/tool.rs index da927d7440..819dbf3000 100644 --- a/codex-rs/ext/image-generation/src/tool.rs +++ b/codex-rs/ext/image-generation/src/tool.rs @@ -1,4 +1,3 @@ -use std::collections::HashSet; use std::io; use base64::Engine; @@ -470,34 +469,6 @@ async fn request_for_call_args( } fn recent_images(history: &[ResponseItem], count: usize) -> Vec { - let mut function_call_ids = HashSet::new(); - let mut custom_tool_call_ids = HashSet::new(); - for item in history { - match item { - ResponseItem::FunctionCall { call_id, .. } => { - function_call_ids.insert(call_id.as_str()); - } - ResponseItem::CustomToolCall { call_id, .. } => { - custom_tool_call_ids.insert(call_id.as_str()); - } - ResponseItem::AdditionalTools { .. } - | ResponseItem::Message { .. } - | ResponseItem::AgentMessage { .. } - | ResponseItem::Reasoning { .. } - | ResponseItem::LocalShellCall { .. } - | ResponseItem::ToolSearchCall { .. } - | ResponseItem::FunctionCallOutput { .. } - | ResponseItem::CustomToolCallOutput { .. } - | ResponseItem::ToolSearchOutput { .. } - | ResponseItem::WebSearchCall { .. } - | ResponseItem::ImageGenerationCall { .. } - | ResponseItem::Compaction { .. } - | ResponseItem::CompactionTrigger { .. } - | ResponseItem::ContextCompaction { .. } - | ResponseItem::Other => {} - } - } - let mut images = Vec::with_capacity(count); 'history: for item in history.iter().rev() { let mut image_urls = Vec::new(); @@ -510,16 +481,8 @@ fn recent_images(history: &[ResponseItem], count: usize) -> Vec { | ContentItem::OutputText { .. } => None, })); } - ResponseItem::FunctionCallOutput { - call_id: Some(call_id), - output, - .. - } if function_call_ids.contains(call_id.as_str()) => { - image_urls.extend(output_image_urls(output)); - } - ResponseItem::CustomToolCallOutput { - call_id, output, .. - } if custom_tool_call_ids.contains(call_id.as_str()) => { + ResponseItem::FunctionCallOutput { output, .. } + | ResponseItem::CustomToolCallOutput { output, .. } => { image_urls.extend(output_image_urls(output)); } ResponseItem::ImageGenerationCall { result, .. } if !result.is_empty() => { @@ -532,8 +495,6 @@ fn recent_images(history: &[ResponseItem], count: usize) -> Vec { | ResponseItem::FunctionCall { .. } | ResponseItem::ToolSearchCall { .. } | ResponseItem::CustomToolCall { .. } - | ResponseItem::FunctionCallOutput { .. } - | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::ToolSearchOutput { .. } | ResponseItem::WebSearchCall { .. } | ResponseItem::ImageGenerationCall { .. }