From 032d15cba77e28d4eb697b1f11bc395c2522d12b Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Mon, 31 Aug 2026 15:46:42 +0000 Subject: [PATCH] Use shared transcript collection for Guardian reviews (#41870) ## What changed - Build Guardian review transcripts with `codex-guardian-context` while preserving Guardian-specific filtering and transcript budgets. - Apply per-entry truncation during collection, including the larger limit for Node REPL output. - Retain standalone function and custom tool outputs even when their matching calls are unavailable. ## Testing - Update Guardian transcript tests to cover shared entry types, Node REPL truncation limits, standalone tool outputs, and recent-tool retention. GitOrigin-RevId: ec73e80c09fe63c3cf100684cb15574d5d6c2c95 --- codex-rs/core/src/guardian/mod.rs | 7 +- codex-rs/core/src/guardian/prompt.rs | 344 +++++++-------------------- codex-rs/core/src/guardian/tests.rs | 230 +++++++++++++----- 3 files changed, 262 insertions(+), 319 deletions(-) diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 9f09b4c0e2..2e8b136f5d 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -65,7 +65,7 @@ pub(crate) const MAX_RECENT_CYBER_AUTO_REVIEW_DENIALS_PER_TURN: u32 = 1; pub(crate) const MAX_RECENT_AUTO_REVIEW_DENIALS_PER_TURN: u32 = 10; pub(crate) const AUTO_REVIEW_DENIAL_WINDOW_SIZE: usize = 50; pub(crate) const AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX: &str = - "The user has manually approved a specific action that was previously `Rejected`."; + codex_guardian_context::MANUAL_APPROVAL_DEVELOPER_PREFIX; const GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS: usize = 10_000; const GUARDIAN_MAX_TOOL_TRANSCRIPT_TOKENS: usize = 10_000; const GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS: usize = 2_000; @@ -73,7 +73,6 @@ const GUARDIAN_MAX_TOOL_ENTRY_TOKENS: usize = 1_000; pub(crate) const GUARDIAN_MAX_NODE_REPL_TOOL_RESULT_TOKENS: usize = 6_000; const GUARDIAN_MAX_ACTION_STRING_TOKENS: usize = 16_000; const GUARDIAN_RECENT_ENTRY_LIMIT: usize = 40; -const TRUNCATION_TAG: &str = "truncated"; /// Captures review inputs from the issuing step without retaining its MCP bindings or tool router. /// Background network approvals and Unix interception use the active task's resolved settings. @@ -254,10 +253,6 @@ use prompt::GuardianPromptMode; #[cfg(test)] use prompt::GuardianTranscriptCursor; #[cfg(test)] -use prompt::GuardianTranscriptEntry; -#[cfg(test)] -use prompt::GuardianTranscriptEntryKind; -#[cfg(test)] use prompt::build_guardian_prompt_items; #[cfg(test)] use prompt::build_guardian_prompt_items_with_parent_turn; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index b3e9278bdf..8bbf5e6b37 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -1,15 +1,19 @@ -use std::collections::HashMap; - -use codex_protocol::mcp::is_node_repl_backed_tool; +use codex_extension_api::ConversationHistorySnapshot; +use codex_guardian_context::ConversationTranscriptConfig; +use codex_guardian_context::ConversationTranscriptEntry; +use codex_guardian_context::ConversationTranscriptEntryKind; +use codex_guardian_context::ConversationTranscriptOptions; +use codex_guardian_context::SectionHistory; +use codex_guardian_context::TranscriptEntryLimits; +use codex_guardian_context::TranscriptRetentionConfig; +use codex_guardian_context::collect_transcript; use codex_protocol::models::ResponseItem; -use codex_protocol::models::plaintext_agent_message_content; use codex_protocol::protocol::GuardianRiskLevel; use codex_protocol::protocol::GuardianUserAuthorization; use codex_protocol::user_input::UserInput; use serde::Deserialize; use serde_json::Value; -use crate::compact::content_items_to_text; use crate::context::GuardianReviewEvidence; use crate::context::NodeReplReviewEvidence; use crate::context::NodeReplReviewEvidenceMode; @@ -19,10 +23,8 @@ use crate::session::session::Session; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::approx_bytes_for_tokens; use codex_utils_output_truncation::approx_token_count; -use codex_utils_output_truncation::approx_tokens_from_byte_count; use codex_utils_output_truncation::truncate_text; -use super::AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX; use super::ApprovalRequestReasons; use super::GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS; use super::GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS; @@ -33,47 +35,16 @@ use super::GUARDIAN_RECENT_ENTRY_LIMIT; use super::GuardianApprovalRequest; use super::GuardianAssessment; use super::GuardianReviewContext; -use super::TRUNCATION_TAG; use super::approval_request::format_guardian_action_pretty; const GUARDIAN_MAX_APPROVAL_REASON_TOKENS: usize = 512; +const GUARDIAN_TRANSCRIPT_RETENTION: TranscriptRetentionConfig = TranscriptRetentionConfig { + max_message_transcript_tokens: GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS, + max_tool_transcript_tokens: GUARDIAN_MAX_TOOL_TRANSCRIPT_TOKENS, + max_recent_non_user_entries: GUARDIAN_RECENT_ENTRY_LIMIT, +}; pub(super) const GUARDIAN_TRANSCRIPT_START: &str = ">>> TRANSCRIPT START\n"; -/// Transcript entry retained for guardian review after filtering. -#[derive(Debug, PartialEq, Eq)] -pub(crate) struct GuardianTranscriptEntry { - pub(crate) kind: GuardianTranscriptEntryKind, - pub(crate) text: String, -} - -#[derive(Debug, PartialEq, Eq)] -pub(crate) enum GuardianTranscriptEntryKind { - Developer, - User, - Assistant, - Tool(String), - NodeReplToolResult(String), -} - -impl GuardianTranscriptEntryKind { - fn role(&self) -> &str { - match self { - Self::Developer => "developer", - Self::User => "user", - Self::Assistant => "assistant", - Self::Tool(role) | Self::NodeReplToolResult(role) => role.as_str(), - } - } - - fn is_user(&self) -> bool { - matches!(self, Self::User) - } - - fn is_tool(&self) -> bool { - matches!(self, Self::Tool(_) | Self::NodeReplToolResult(_)) - } -} - pub(crate) struct GuardianPromptItems { pub(crate) items: Vec, pub(crate) transcript_cursor: GuardianTranscriptCursor, @@ -153,7 +124,10 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( .get::() .map(|evidence| evidence.user_input_fragments(history.as_ref())) .unwrap_or_default(); - let transcript_entries = collect_guardian_transcript_entries(history.review_items()); + let transcript_entries = collect_guardian_transcript_entries( + &GuardianReviewHistory(history.as_ref()), + node_repl_result_token_limit, + ); let transcript_cursor = GuardianTranscriptCursor { parent_history_version: history.review_history_version(), transcript_entry_count: transcript_entries.len(), @@ -181,7 +155,6 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( transcript_entries.as_slice(), /*entry_number_offset*/ 0, "", - node_repl_result_token_limit, ); ( transcript_entries, @@ -202,7 +175,6 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( &transcript_entries[already_seen_entry_count..], already_seen_entry_count, "", - node_repl_result_token_limit, ); ( transcript_entries, @@ -375,11 +347,10 @@ struct GuardianPromptHeadings { action_intro: &'static str, } -/// Renders a compact guardian transcript from the retained history entries, -/// which are only user, assistant, and tool call entries. +/// Renders a compact guardian transcript from shared, per-entry-bounded evidence. /// /// Selection is intentionally simple and predictable: -/// - each entry is truncated to its per-entry cap +/// - collection has already applied each entry's per-entry cap /// - user and assistant entries share the message budget /// - tool calls/results use a separate tool budget so tool evidence cannot /// crowd out the human conversation @@ -393,21 +364,19 @@ struct GuardianPromptHeadings { /// skipped. #[cfg(test)] pub(crate) fn render_guardian_transcript_entries( - entries: &[GuardianTranscriptEntry], + entries: &[ConversationTranscriptEntry], ) -> (Vec, Option) { render_guardian_transcript_entries_with_offset( entries, /*entry_number_offset*/ 0, "", - GUARDIAN_MAX_TOOL_ENTRY_TOKENS, ) } fn render_guardian_transcript_entries_with_offset( - entries: &[GuardianTranscriptEntry], + entries: &[ConversationTranscriptEntry], entry_number_offset: usize, empty_placeholder: &str, - node_repl_result_token_limit: usize, ) -> (Vec, Option) { if entries.is_empty() { return (vec![empty_placeholder.to_string()], None); @@ -417,22 +386,11 @@ fn render_guardian_transcript_entries_with_offset( .iter() .enumerate() .map(|(index, entry)| { - let token_cap = if matches!( - entry.kind, - GuardianTranscriptEntryKind::NodeReplToolResult(_) - ) { - node_repl_result_token_limit - } else if entry.kind.is_tool() { - GUARDIAN_MAX_TOOL_ENTRY_TOKENS - } else { - GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS - }; - let (text, _) = guardian_truncate_text(&entry.text, token_cap); let rendered = format!( "[{}] {}: {}", index + entry_number_offset + 1, entry.kind.role(), - text + entry.text ); let token_count = approx_token_count(&rendered); (rendered, token_count) @@ -445,7 +403,9 @@ fn render_guardian_transcript_entries_with_offset( let user_indices = entries .iter() .enumerate() - .filter_map(|(index, entry)| entry.kind.is_user().then_some(index)) + .filter_map(|(index, entry)| { + matches!(entry.kind, ConversationTranscriptEntryKind::User).then_some(index) + }) .collect::>(); if let Some(&first_user_index) = user_indices.first() { @@ -456,7 +416,7 @@ fn render_guardian_transcript_entries_with_offset( if let Some(&last_user_index) = user_indices.last() && !included[last_user_index] && message_tokens + rendered_entries[last_user_index].1 - <= GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS + <= GUARDIAN_TRANSCRIPT_RETENTION.max_message_transcript_tokens { included[last_user_index] = true; message_tokens += rendered_entries[last_user_index].1; @@ -468,7 +428,9 @@ fn render_guardian_transcript_entries_with_offset( } let token_count = rendered_entries[index].1; - if message_tokens + token_count > GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS { + if message_tokens + token_count + > GUARDIAN_TRANSCRIPT_RETENTION.max_message_transcript_tokens + { continue; } @@ -479,15 +441,25 @@ fn render_guardian_transcript_entries_with_offset( let mut retained_non_user_entries = 0usize; for index in (0..entries.len()).rev() { let entry = &entries[index]; - if entry.kind.is_user() || retained_non_user_entries >= GUARDIAN_RECENT_ENTRY_LIMIT { + if matches!(entry.kind, ConversationTranscriptEntryKind::User) + || retained_non_user_entries + >= GUARDIAN_TRANSCRIPT_RETENTION.max_recent_non_user_entries + { continue; } let token_count = rendered_entries[index].1; - let within_budget = if entry.kind.is_tool() { - tool_tokens + token_count <= GUARDIAN_MAX_TOOL_TRANSCRIPT_TOKENS + let is_tool = matches!( + entry.kind, + ConversationTranscriptEntryKind::ToolCall(_) + | ConversationTranscriptEntryKind::ToolOutput(_) + | ConversationTranscriptEntryKind::NodeReplToolOutput(_) + ); + let within_budget = if is_tool { + tool_tokens + token_count <= GUARDIAN_TRANSCRIPT_RETENTION.max_tool_transcript_tokens } else { - message_tokens + token_count <= GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS + message_tokens + token_count + <= GUARDIAN_TRANSCRIPT_RETENTION.max_message_transcript_tokens }; if !within_budget { continue; @@ -495,7 +467,7 @@ fn render_guardian_transcript_entries_with_offset( included[index] = true; retained_non_user_entries += 1; - if entry.kind.is_tool() { + if is_tool { tool_tokens += token_count; } else { message_tokens += token_count; @@ -521,199 +493,55 @@ fn render_guardian_transcript_entries_with_offset( /// Keep both tool calls and tool results here. The reviewer often needs the /// agent's exact queried path / arguments as well as the returned evidence to /// decide whether the pending approval is justified. -pub(crate) fn collect_guardian_transcript_entries<'a>( - items: impl IntoIterator, -) -> Vec { - let mut entries = Vec::new(); - let mut tool_names_by_call_id = HashMap::new(); - let non_empty_entry = |kind, text: String| { - (!text.trim().is_empty()).then_some(GuardianTranscriptEntry { kind, text }) +/// Per-entry truncation happens during collection, using the current review's +/// Node REPL cap; the cursor still counts every non-empty evidence entry. +pub(crate) fn collect_guardian_transcript_entries( + history: &dyn SectionHistory, + node_repl_result_token_limit: usize, +) -> Vec { + let transcript = ConversationTranscriptConfig { + options: ConversationTranscriptOptions::default(), + entry_limits: TranscriptEntryLimits { + message_tokens: GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS, + tool_tokens: GUARDIAN_MAX_TOOL_ENTRY_TOKENS, + node_repl_output_tokens: node_repl_result_token_limit, + }, }; - let content_entry = - |kind, content| content_items_to_text(content).and_then(|text| non_empty_entry(kind, text)); - let serialized_entry = - |kind, serialized: Option| serialized.and_then(|text| non_empty_entry(kind, text)); + let history = FilteredGuardianHistory(history); - for item in items { - let entry = match item { - ResponseItem::Message { role, content, .. } if role == "user" => { - if is_contextual_user_message_content(content) { - None - } else { - content_entry(GuardianTranscriptEntryKind::User, content) - } - } - ResponseItem::Message { role, content, .. } if role == "developer" => { - content_items_to_text(content).and_then(|text| { - // Preserve only the explicit auto-review approval marker for - // Guardian context; other developer messages are intentionally - // excluded from the review transcript. - text.starts_with(AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX) - .then_some(GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Developer, - text, - }) - }) - } - ResponseItem::Message { role, content, .. } if role == "assistant" => { - content_entry(GuardianTranscriptEntryKind::Assistant, content) - } - ResponseItem::AgentMessage { - author, content, .. - } => plaintext_agent_message_content(content).map(|text| GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Assistant, - text: format!("Agent message from {author}:\n{text}"), - }), - ResponseItem::LocalShellCall { action, .. } => serialized_entry( - GuardianTranscriptEntryKind::Tool("tool shell call".to_string()), - serde_json::to_string(action).ok(), - ), - ResponseItem::FunctionCall { - call_id, - name, - namespace, - arguments, - .. - } => { - tool_names_by_call_id - .insert(call_id.as_str(), (name.as_str(), namespace.as_deref())); - (!arguments.trim().is_empty()).then(|| GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool(format!("tool {name} call")), - text: arguments.clone(), - }) - } - ResponseItem::CustomToolCall { - call_id, - name, - namespace, - input, - .. - } => { - tool_names_by_call_id - .insert(call_id.as_str(), (name.as_str(), namespace.as_deref())); - (!input.trim().is_empty()).then(|| GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool(format!("tool {name} call")), - text: input.clone(), - }) - } - ResponseItem::WebSearchCall { action, .. } => action.as_ref().and_then(|action| { - serialized_entry( - GuardianTranscriptEntryKind::Tool("tool web_search call".to_string()), - serde_json::to_string(action).ok(), - ) - }), - ResponseItem::FunctionCallOutput { - call_id: Some(call_id), - output, - .. - } - | ResponseItem::CustomToolCallOutput { - call_id, output, .. - } => output.body.to_text().and_then(|text| { - let kind = match tool_names_by_call_id.get(call_id.as_str()) { - Some((name, namespace)) if is_node_repl_backed_tool(name, *namespace) => { - GuardianTranscriptEntryKind::NodeReplToolResult(format!( - "tool {name} result" - )) - } - Some((name, _)) => { - GuardianTranscriptEntryKind::Tool(format!("tool {name} result")) - } - None => GuardianTranscriptEntryKind::Tool("tool result".to_string()), - }; - 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, - }; + collect_transcript(&history, &transcript) + .into_iter() + .filter(|entry| entry.kind != ConversationTranscriptEntryKind::Reasoning) + .collect() +} - if let Some(entry) = entry { - entries.push(entry); - } +struct GuardianReviewHistory<'a>(&'a dyn ConversationHistorySnapshot); + +impl SectionHistory for GuardianReviewHistory<'_> { + fn items(&self) -> Box + Send + '_> { + self.0.review_items() } +} - entries +struct FilteredGuardianHistory<'a>(&'a dyn SectionHistory); + +impl SectionHistory for FilteredGuardianHistory<'_> { + fn items(&self) -> Box + Send + '_> { + Box::new(self.0.items().filter(|item| { + !matches!( + item, + ResponseItem::Message { role, content, .. } + if role == "user" && is_contextual_user_message_content(content) + ) + })) + } } pub(crate) fn guardian_truncate_text(content: &str, token_cap: usize) -> (String, bool) { - if content.is_empty() { - return (String::new(), false); - } - - let max_bytes = approx_bytes_for_tokens(token_cap); - if content.len() <= max_bytes { - return (content.to_string(), false); - } - - let omitted_tokens = approx_tokens_from_byte_count(content.len().saturating_sub(max_bytes)); - let marker = format!("<{TRUNCATION_TAG} omitted_approx_tokens=\"{omitted_tokens}\" />"); - if max_bytes <= marker.len() { - return (marker, true); - } - - let available_bytes = max_bytes.saturating_sub(marker.len()); - let prefix_budget = available_bytes / 2; - let suffix_budget = available_bytes.saturating_sub(prefix_budget); - let (prefix, suffix) = split_guardian_truncation_bounds(content, prefix_budget, suffix_budget); - - (format!("{prefix}{marker}{suffix}"), true) -} - -fn split_guardian_truncation_bounds( - content: &str, - prefix_bytes: usize, - suffix_bytes: usize, -) -> (&str, &str) { - if content.is_empty() { - return ("", ""); - } - - let len = content.len(); - let suffix_start_target = len.saturating_sub(suffix_bytes); - let mut prefix_end = 0usize; - let mut suffix_start = len; - let mut suffix_started = false; - - for (index, ch) in content.char_indices() { - let char_end = index + ch.len_utf8(); - if char_end <= prefix_bytes { - prefix_end = char_end; - continue; - } - - if index >= suffix_start_target { - if !suffix_started { - suffix_start = index; - suffix_started = true; - } - continue; - } - } - - if suffix_start < prefix_end { - suffix_start = prefix_end; - } - - (&content[..prefix_end], &content[suffix_start..]) + ( + codex_guardian_context::truncate_text(content, token_cap), + content.len() > approx_bytes_for_tokens(token_cap), + ) } /// The model is asked for strict JSON, but we still accept a surrounding prose diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index aa108f5fd0..6ee7fd4bad 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -29,6 +29,8 @@ use codex_config::config_toml::ConfigToml; use codex_config::types::McpServerConfig; use codex_exec_server::LOCAL_FS; use codex_features::Feature; +use codex_guardian_context::ConversationTranscriptEntry; +use codex_guardian_context::ConversationTranscriptEntryKind; use codex_history::RolloutItem; use codex_model_provider::create_model_provider; use codex_model_provider_info::AMAZON_BEDROCK_GPT_5_4_MODEL_ID; @@ -467,17 +469,20 @@ fn last_user_message_text_from_body(body: &serde_json::Value) -> String { #[test] fn build_guardian_transcript_keeps_original_numbering() { let entries = [ - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::User, + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::User, text: "first".to_string(), + original_bytes: "first".len(), }, - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Assistant, + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::Assistant, text: "second".to_string(), + original_bytes: "second".len(), }, - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Assistant, + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ProtectedAssistant, text: "third".to_string(), + original_bytes: "third".len(), }, ]; @@ -939,14 +944,15 @@ fn collect_guardian_transcript_entries_skips_contextual_user_messages() { }, ]; - let entries = collect_guardian_transcript_entries(&items); + let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); assert_eq!(entries.len(), 1); assert_eq!( entries[0], - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Assistant, + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ProtectedAssistant, text: "hello".to_string(), + original_bytes: "hello".len(), } ); } @@ -976,12 +982,13 @@ fn collect_guardian_transcript_entries_keeps_manual_approval_developer_message() }, ]; - let entries = collect_guardian_transcript_entries(&items); + let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); assert_eq!( entries, - vec![GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Developer, + vec![ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::Developer, + original_bytes: approval_text.len(), text: approval_text, }] ); @@ -1029,30 +1036,67 @@ fn collect_guardian_transcript_entries_includes_recent_tool_calls_and_output() { }, ]; - let entries = collect_guardian_transcript_entries(&items); + let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); assert_eq!(entries.len(), 4); assert_eq!( entries[1], - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool("tool read_file call".to_string()), + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolCall("tool read_file call".to_string()), text: "{\"path\":\"README.md\"}".to_string(), + original_bytes: "{\"path\":\"README.md\"}".len(), } ); assert_eq!( entries[2], - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool("tool read_file result".to_string()), + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolOutput("tool read_file result".to_string()), text: "repo is public".to_string(), + original_bytes: "repo is public".len(), } ); if let ResponseItem::FunctionCall { namespace, .. } = &mut items[1] { *namespace = Some("mcp__node_repl__".to_string()); } assert!(matches!( - collect_guardian_transcript_entries(&items)[2].kind, - GuardianTranscriptEntryKind::NodeReplToolResult(_) + collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS)[2].kind, + ConversationTranscriptEntryKind::NodeReplToolOutput(_) )); + + let oversized_result = "é🙂".repeat(/*n*/ 10_000); + if let ResponseItem::FunctionCallOutput { output, .. } = &mut items[2] { + *output = + codex_protocol::models::FunctionCallOutputPayload::from_text(oversized_result.clone()); + } + for token_cap in [ + GUARDIAN_MAX_TOOL_ENTRY_TOKENS, + GUARDIAN_MAX_NODE_REPL_TOOL_RESULT_TOKENS, + ] { + let entries = collect_guardian_transcript_entries(&items, token_cap); + assert_eq!( + entries[2], + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::NodeReplToolOutput( + "tool read_file result".to_string() + ), + text: guardian_truncate_text(&oversized_result, token_cap).0, + original_bytes: oversized_result.len(), + } + ); + assert_eq!(entries.len(), 4); + assert_eq!( + render_guardian_transcript_entries(&entries), + ( + vec![ + "[1] user: check the repo".to_string(), + "[2] tool read_file call: {\"path\":\"README.md\"}".to_string(), + format!("[3] tool read_file result: {}", entries[2].text), + "[4] assistant: I need to push a fix".to_string(), + ], + None, + ) + ); + } } #[test] @@ -1067,13 +1111,49 @@ fn collect_guardian_transcript_entries_preserves_named_unpaired_tool_sources() { ), internal_chat_message_metadata_passthrough: None, }]; + items.extend( + [ + (None, "anonymous output"), + (Some("missing-call"), "orphaned function output"), + ] + .map(|(call_id, text)| ResponseItem::FunctionCallOutput { + id: None, + call_id: call_id.map(str::to_string), + name: None, + namespace: None, + output: codex_protocol::models::FunctionCallOutputPayload::from_text(text.to_string()), + internal_chat_message_metadata_passthrough: None, + }), + ); + items.push(ResponseItem::CustomToolCallOutput { + id: None, + call_id: "missing-custom-call".to_string(), + name: None, + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "orphaned custom output".to_string(), + ), + internal_chat_message_metadata_passthrough: None, + }); + let mut expected = vec![ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolOutput( + "tool slack.notifications result".to_string(), + ), + text: "new message".to_string(), + original_bytes: "new message".len(), + }]; + expected.extend( + ["orphaned function output", "orphaned custom output"].map(|text| { + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolOutput("tool result".to_string()), + text: text.to_string(), + original_bytes: text.len(), + } + }), + ); assert_eq!( - collect_guardian_transcript_entries(&items), - vec![GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool("tool slack.notifications result".to_string()), - text: "new message".to_string(), - }] + collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS), + expected, ); if let ResponseItem::FunctionCallOutput { output, .. } = &mut items[0] { @@ -1084,12 +1164,16 @@ fn collect_guardian_transcript_entries_preserves_named_unpaired_tool_sources() { }, ]); } + expected[0] = ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolOutput( + "tool slack.notifications result".to_string(), + ), + text: "[non-text output]".to_string(), + original_bytes: "[non-text output]".len(), + }; assert_eq!( - collect_guardian_transcript_entries(&items), - vec![GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool("tool slack.notifications result".to_string()), - text: "[non-text output]".to_string(), - }] + collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS), + expected, ); } @@ -1630,21 +1714,35 @@ async fn routes_approval_to_guardian_allows_granular_review_policy() { #[test] fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() { let repeated = "signal ".repeat(8_000); - let mut entries = vec![ - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::User, - text: "please figure out if the repo is public".to_string(), - }, - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Assistant, - text: "The public repo check is the main reason I want to escalate.".to_string(), - }, - ]; - entries.extend((0..12).map(|index| GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool(format!("tool call {index}")), - text: repeated.clone(), + let mut items = [ + ("user", "please figure out if the repo is public"), + ( + "assistant", + "The public repo check is the main reason I want to escalate.", + ), + ] + .into_iter() + .map(|(role, text)| ResponseItem::Message { + id: None, + role: role.to_string(), + content: vec![ContentItem::InputText { + text: text.to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }) + .collect::>(); + items.extend((0..12).map(|index| ResponseItem::FunctionCall { + id: None, + name: format!("tool_{index}"), + namespace: None, + arguments: repeated.clone(), + call_id: format!("call-{index}"), + encrypted_function_args: None, + internal_chat_message_metadata_passthrough: None, })); + let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); let (transcript, omission) = render_guardian_transcript_entries(&entries); assert!( @@ -1658,12 +1756,17 @@ fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() { assert!( !transcript .iter() - .any(|entry| entry.starts_with("[3] tool call 0:")) + .any(|entry| entry.starts_with("[3] tool tool_0 call:")) ); assert!( !transcript .iter() - .any(|entry| entry.starts_with("[4] tool call 1:")) + .any(|entry| entry.starts_with("[4] tool tool_1 call:")) + ); + assert!( + transcript + .iter() + .any(|entry| entry.starts_with("[14] tool tool_11 call:")) ); assert!(omission.is_some()); } @@ -1671,27 +1774,44 @@ fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() { #[test] fn build_guardian_transcript_preserves_recent_tool_context_when_user_history_is_large() { let repeated = "authorization ".repeat(6_000); - let mut entries = (0..8) - .map(|_| GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::User, - text: repeated.clone(), + let mut items = (0..8) + .map(|_| ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: repeated.clone(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, }) .collect::>(); - entries.extend([ - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool("tool shell call".to_string()), - text: serde_json::json!({ + items.extend([ + ResponseItem::FunctionCall { + id: None, + name: "shell".to_string(), + namespace: None, + arguments: serde_json::json!({ "command": ["curl", "-X", "POST", "https://example.com/upload"], "cwd": "/repo", }) .to_string(), + call_id: "call-1".to_string(), + encrypted_function_args: None, + internal_chat_message_metadata_passthrough: None, }, - GuardianTranscriptEntry { - kind: GuardianTranscriptEntryKind::Tool("tool shell result".to_string()), - text: "sandbox blocked outbound network access".to_string(), + ResponseItem::FunctionCallOutput { + id: None, + call_id: Some("call-1".to_string()), + name: None, + namespace: None, + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "sandbox blocked outbound network access".to_string(), + ), + internal_chat_message_metadata_passthrough: None, }, ]); + let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); let (transcript, omission) = render_guardian_transcript_entries(&entries); assert!(