From 61635845491fd720bb49909466aa19fe5633c773 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Tue, 3 Nov 2026 13:06:57 +0000 Subject: [PATCH] Move Guardian unit tests into their owning crates (#59159) ## What changed - Move approval-request and prompt-composition tests from `core` to `guardian-reviewer`. - Move transcript collection, budgeting, and approval-reason truncation tests to `guardian-context`, removing the test-only rendering helper from `core`. - Move the managed-feature reviewer configuration test to `guardian-v2`. GitOrigin-RevId: 1886a10cd6aba07b7ca2bcce28d45e0a698111a6 --- codex-rs/core/src/guardian/mod.rs | 4 - codex-rs/core/src/guardian/prompt.rs | 37 -- codex-rs/core/src/guardian/tests.rs | 597 ------------------ .../src/approval_request_tests.rs | 56 ++ .../ext/guardian-reviewer/src/prompt_tests.rs | 297 +++++++++ .../ext/guardian-v2/src/sync_reviewer/mod.rs | 4 + .../sync_reviewer/reviewer_config_tests.rs | 34 + .../src/preparation/source_tests.rs | 56 +- .../src/preparation/transcript_tests.rs | 135 ++++ .../guardian-context/src/transcript_tests.rs | 134 ++++ 10 files changed, 715 insertions(+), 639 deletions(-) create mode 100644 codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config_tests.rs diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 93d612267b..f725cb33cf 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -207,8 +207,6 @@ use codex_guardian_reviewer::guardian_output_schema; #[cfg(test)] use approval_request::guardian_assessment_action; #[cfg(test)] -use approval_request::guardian_request_turn_id; -#[cfg(test)] use codex_guardian_reviewer::GuardianReviewOutcome; #[cfg(test)] use prompt::GuardianPromptMode; @@ -219,8 +217,6 @@ use prompt::build_guardian_prompt_items; #[cfg(test)] use prompt::collect_guardian_prompt_items_with_parent_turn; #[cfg(test)] -use prompt::render_guardian_transcript_entries; -#[cfg(test)] use review::run_guardian_review_session_with_retry as run_guardian_review_session_for_test; #[cfg(test)] use review_session::build_guardian_review_session_config as build_guardian_review_session_config_for_test; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index b9b6879cb6..5057fb7cc7 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -1,11 +1,7 @@ use codex_extension_api::ConversationHistorySnapshot; use codex_features::Feature; -#[cfg(test)] -use codex_guardian_context::Budgeted; use codex_guardian_context::CollectedContext; use codex_guardian_context::ContextProfile; -#[cfg(test)] -use codex_guardian_context::ConversationTranscriptEntry; use codex_guardian_context::GuardianRootMessage; use codex_guardian_context::PlannedAction; use codex_guardian_context::SectionError; @@ -205,39 +201,6 @@ pub(super) async fn collect_guardian_prompt_items_with_parent_turn( }) } -/// Exercises the sync profile through the host's existing transcript tests. -#[cfg(test)] -pub(crate) fn render_guardian_transcript_entries( - entries: &[ConversationTranscriptEntry], - node_repl_result_token_limit: usize, -) -> (Vec, Option) { - let mut profile = ContextProfile::synchronous(); - profile.transcript.entry_limits.node_repl_output_tokens = node_repl_result_token_limit; - let mut transcript = profile.prepare_transcript(entries, /*entry_number_offset*/ 0); - if entries.is_empty() { - transcript.items.push(Budgeted::required( - codex_guardian_context::TranscriptContent::Text( - "".to_owned(), - ), - )); - } - ( - transcript - .items - .into_iter() - .map(|item| match item.content { - codex_guardian_context::TranscriptContent::Text(text) => text, - codex_guardian_context::TranscriptContent::Record(record) => record.to_string(), - codex_guardian_context::TranscriptContent::ScheduledMessage(_) - | codex_guardian_context::TranscriptContent::NativeMessage(_) => { - panic!("expected text transcript") - } - }) - .collect(), - transcript.omission_note, - ) -} - /// Retains the human-readable conversation plus recent tool call / result /// evidence for guardian review and skips synthetic contextual scaffolding that /// would just add noise because the guardian reviewer already gets the normal diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index e45475db76..de4b5227e6 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -3,14 +3,12 @@ use super::*; use crate::config::Config; use crate::config::ConfigOverrides; use crate::config::Constrained; -use crate::config::ManagedFeatures; use crate::config::NetworkProxySpec; use crate::config::PermissionProfileSnapshot; use crate::config::test_config; use crate::context::ContextualUserFragment; use crate::environment_selection::EnvironmentAttachment; use crate::environment_selection::TurnEnvironmentState; -use crate::guardian::approval_request::guardian_request_target_item_id; use crate::guardian::review::guardian_review_session_config; use crate::session::session::Session; use crate::session::tests::update_turn_settings_for_test; @@ -18,7 +16,6 @@ use crate::session::turn_context::TurnContext; use crate::test_support; use codex_analytics::GuardianApprovalRequestSource; use codex_config::ConfigLayerStack; -use codex_config::FeatureRequirementsToml; use codex_config::NetworkConstraints; use codex_config::NetworkDomainPermissionToml; use codex_config::NetworkDomainPermissionsToml; @@ -90,7 +87,6 @@ use core_test_support::test_path_buf; use insta::Settings; use insta::assert_snapshot; use pretty_assertions::assert_eq; -use std::collections::BTreeMap; use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; @@ -503,68 +499,6 @@ async fn build_guardian_prompt_prefers_retry_reason_over_approval_reason() -> an Ok(()) } -#[tokio::test(flavor = "current_thread")] -async fn build_guardian_prompt_truncates_oversized_approval_reason() -> anyhow::Result<()> { - let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; - seed_guardian_parent_history(&session, &turn).await; - let context = GuardianReviewContext::from(&turn); - let approval_reason = format!("policy-start {} policy-end", "x".repeat(10_000)); - let expected_reason = codex_utils_output_truncation::truncate_text( - &approval_reason, - codex_utils_output_truncation::TruncationPolicy::Tokens(/*tokens*/ 512), - ); - - let collected = collect_guardian_prompt_items_with_parent_turn( - session.as_ref(), - GuardianTranscriptHistory::Retained(session.conversation_history_snapshot().await.as_ref()), - Some(&context), - ApprovalRequestReasons { - approval: Some(approval_reason), - retry: None, - }, - GuardianApprovalRequest::ExecCommand { - id: "shell-1".to_string(), - environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - command: vec!["git".to_string(), "push".to_string()], - cwd: test_path_buf("/repo/codex-rs/core").abs().into(), - guardian_cwd: native_guardian_cwd("/repo/codex-rs/core"), - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - justification: None, - tty: false, - }, - GuardianPromptMode::Full, - /*reviewed_node_repl_evidence_sequence*/ 0, - ) - .await?; - let prompt = codex_guardian_reviewer::compose_guardian_prompt( - collected.sections, - &collected.profile, - collected.mode, - collected.history_version, - &collected.session_id, - collected.node_repl_evidence_sequence, - )?; - - let items = prompt.context.into_user_inputs()?; - let reason_item = items - .iter() - .find_map(|item| match item { - codex_protocol::user_input::UserInput::Text { text, .. } - if text.contains("tokens truncated") => - { - Some(text) - } - _ => None, - }) - .expect("oversized approval reason should include a truncation marker"); - assert!(reason_item.starts_with("policy-start")); - assert!(reason_item.ends_with("policy-end\n\n")); - assert_eq!(reason_item, &format!("{expected_reason}\n\n")); - - Ok(()) -} - #[tokio::test(flavor = "current_thread")] async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Result<()> { let (mut session, mut turn) = crate::session::tests::make_session_and_context().await; @@ -879,154 +813,6 @@ async fn guardian_mcp_uses_thread_permissions_for_an_unavailable_captured_enviro Ok(()) } -#[tokio::test(flavor = "current_thread")] -async fn build_guardian_prompt_delta_mode_preserves_original_numbering() -> anyhow::Result<()> { - let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; - seed_guardian_parent_history(&session, &turn).await; - session - .record_conversation_items( - turn.as_ref(), - turn.model_info(), - &[ - ResponseItem::Message { - status: None, - encrypted_content: None, - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "Please also push the second docs fix.".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }, - ResponseItem::Message { - status: None, - encrypted_content: None, - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - annotations: None, - logprobs: None, - text: "I need approval for the second push.".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }, - ], - ) - .await; - - let prompt = build_guardian_prompt_items( - session.as_ref(), - /*retry_reason*/ None, - GuardianApprovalRequest::ExecCommand { - id: "shell-2".to_string(), - environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - command: vec!["git".to_string(), "push".to_string()], - cwd: test_path_buf("/repo/codex-rs/core").abs().into(), - guardian_cwd: native_guardian_cwd("/repo/codex-rs/core"), - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - justification: Some("Need to push the second docs fix.".to_string()), - tty: false, - }, - GuardianPromptMode::Delta { - cursor: GuardianTranscriptCursor { - parent_history_version: 0, - transcript_entry_count: 4, - }, - }, - ) - .await?; - - let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); - assert!(text.contains("added since your last approval assessment")); - assert!(text.contains(">>> TRANSCRIPT DELTA START\n")); - assert!(text.contains("[5] user: Please also push the second docs fix.")); - assert!(text.contains("[6] assistant: I need approval for the second push.")); - assert!(text.contains(">>> TRANSCRIPT DELTA END\n")); - assert!(text.contains("The Codex agent has requested the following next action:\n")); - assert!(!text.contains("[1] user: Please check the repo visibility")); - assert_eq!(prompt.transcript_cursor.transcript_entry_count, 6); - - Ok(()) -} - -#[tokio::test(flavor = "current_thread")] -async fn build_guardian_prompt_delta_mode_handles_empty_delta() -> anyhow::Result<()> { - let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; - seed_guardian_parent_history(&session, &turn).await; - - let prompt = build_guardian_prompt_items( - session.as_ref(), - /*retry_reason*/ None, - GuardianApprovalRequest::ExecCommand { - id: "shell-2".to_string(), - environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - command: vec!["git".to_string(), "push".to_string()], - cwd: test_path_buf("/repo/codex-rs/core").abs().into(), - guardian_cwd: native_guardian_cwd("/repo/codex-rs/core"), - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - justification: Some("Need to push the second docs fix.".to_string()), - tty: false, - }, - GuardianPromptMode::Delta { - cursor: GuardianTranscriptCursor { - parent_history_version: 0, - transcript_entry_count: 4, - }, - }, - ) - .await?; - - let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); - assert!(text.contains(">>> TRANSCRIPT DELTA START\n")); - assert!(text.contains("")); - assert!(text.contains(">>> TRANSCRIPT DELTA END\n")); - assert_eq!(prompt.transcript_cursor.transcript_entry_count, 4); - - Ok(()) -} - -#[tokio::test(flavor = "current_thread")] -async fn build_guardian_prompt_stale_delta_cursor_falls_back_to_full_prompt() -> anyhow::Result<()> -{ - let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; - seed_guardian_parent_history(&session, &turn).await; - - let prompt = build_guardian_prompt_items( - session.as_ref(), - /*retry_reason*/ None, - GuardianApprovalRequest::ExecCommand { - id: "shell-3".to_string(), - environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - command: vec!["git".to_string(), "push".to_string()], - cwd: test_path_buf("/repo/codex-rs/core").abs().into(), - guardian_cwd: native_guardian_cwd("/repo/codex-rs/core"), - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - justification: Some("Need to push the docs fix.".to_string()), - tty: false, - }, - GuardianPromptMode::Delta { - cursor: GuardianTranscriptCursor { - parent_history_version: 0, - transcript_entry_count: 99, - }, - }, - ) - .await?; - - let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); - assert!(text.contains("whose request action you are assessing")); - assert!(text.contains(">>> TRANSCRIPT START\n")); - assert!(!text.contains("TRANSCRIPT DELTA")); - assert_eq!(prompt.transcript_cursor.transcript_entry_count, 4); - - Ok(()) -} - #[tokio::test(flavor = "current_thread")] async fn build_guardian_prompt_stale_delta_version_falls_back_to_full_prompt() -> anyhow::Result<()> { @@ -1194,127 +980,6 @@ fn collect_guardian_transcript_entries_skips_contextual_user_messages() { ); } -#[test] -fn collect_guardian_transcript_entries_includes_recent_tool_calls_and_output() { - let mut items = vec![ - ResponseItem::Message { - status: None, - encrypted_content: None, - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "check the repo".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }, - ResponseItem::FunctionCall { - status: None, - encrypted_content: None, - id: None, - name: "read_file".to_string(), - namespace: None, - arguments: "{\"path\":\"README.md\"}".to_string(), - call_id: "call-1".to_string(), - encrypted_function_args: None, - internal_chat_message_metadata_passthrough: None, - }, - ResponseItem::FunctionCallOutput { - id: None, - call_id: Some("call-1".to_string()), - name: None, - namespace: None, - output: codex_protocol::models::FunctionCallOutputPayload::from_text( - "repo is public".to_string(), - ), - internal_chat_message_metadata_passthrough: None, - }, - ResponseItem::Message { - status: None, - encrypted_content: None, - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - annotations: None, - logprobs: None, - text: "I need to push a fix".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }, - ]; - - let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); - - assert_eq!(entries.len(), 4); - assert_eq!( - entries[1], - ConversationTranscriptEntry { - kind: ConversationTranscriptEntryKind::ToolCall("tool read_file call".to_string()), - content: codex_guardian_context::TranscriptContent::Text( - "{\"path\":\"README.md\"}".to_string() - ), - original_bytes: "{\"path\":\"README.md\"}".len(), - retained_source: None, - } - ); - assert_eq!( - entries[2], - ConversationTranscriptEntry { - kind: ConversationTranscriptEntryKind::ToolOutput("tool read_file result".to_string()), - content: codex_guardian_context::TranscriptContent::Text("repo is public".to_string()), - original_bytes: "repo is public".len(), - retained_source: None, - } - ); - if let ResponseItem::FunctionCall { namespace, .. } = &mut items[1] { - *namespace = Some("mcp__node_repl__".to_string()); - } - assert!(matches!( - 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() - ), - content: codex_guardian_context::TranscriptContent::Text(oversized_result.clone()), - original_bytes: oversized_result.len(), - retained_source: None, - } - ); - assert_eq!(entries.len(), 4); - assert_eq!( - render_guardian_transcript_entries(&entries, token_cap), - ( - vec![ - "[1] user: check the repo".to_string(), - "[2] tool read_file call: {\"path\":\"README.md\"}".to_string(), - format!( - "[3] tool read_file result: {}", - guardian_truncate_text(&oversized_result, token_cap).0 - ), - "[4] assistant: I need to push a fix".to_string(), - ], - None, - ) - ); - } -} - #[tokio::test] async fn guardian_prompt_rejects_oversized_environment_ids() -> anyhow::Result<()> { let (session, _) = crate::session::tests::make_session_and_context().await; @@ -1408,49 +1073,6 @@ async fn build_guardian_prompt_items_keeps_required_node_repl_reviews_generic() Ok(()) } -#[tokio::test(flavor = "current_thread")] -async fn build_guardian_prompt_items_keeps_other_requests_generic() -> anyhow::Result<()> { - let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; - seed_guardian_parent_history(&session, &turn).await; - let context = GuardianReviewContext::from(&turn); - - for request in [ - guardian_mcp_request("node_repl", "js"), - guardian_mcp_request("node_repl", "inspect"), - guardian_mcp_request("another_server", "js"), - guardian_exec_command_request("shell-1"), - ] { - let collected = collect_guardian_prompt_items_with_parent_turn( - session.as_ref(), - GuardianTranscriptHistory::Retained( - session.conversation_history_snapshot().await.as_ref(), - ), - Some(&context), - ApprovalRequestReasons::default(), - request, - GuardianPromptMode::Full, - /*reviewed_node_repl_evidence_sequence*/ 0, - ) - .await?; - let prompt = codex_guardian_reviewer::compose_guardian_prompt( - collected.sections, - &collected.profile, - collected.mode, - collected.history_version, - &collected.session_id, - collected.node_repl_evidence_sequence, - )?; - - let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); - assert!(text.contains("Assess the exact planned action below.")); - assert!(text.contains("Planned action JSON:")); - assert!(!text.contains("Node REPL action JSON:")); - assert!(!text.contains("Distinguish preparation")); - } - - Ok(()) -} - #[tokio::test(flavor = "current_thread")] async fn build_guardian_prompt_items_explains_network_access_review_scope() -> anyhow::Result<()> { let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; @@ -1576,62 +1198,6 @@ async fn guardian_exec_command_uses_executor_cwd_convention( Ok(()) } -#[test] -fn guardian_request_turn_id_prefers_network_access_owner_turn() { - let network_access = GuardianApprovalRequest::NetworkAccess { - id: "network-1".to_string(), - turn_id: "owner-turn".to_string(), - environment_id: "local".to_string(), - target: "https://example.com:443".to_string(), - host: "example.com".to_string(), - protocol: NetworkApprovalProtocol::Https, - port: 443, - trigger: None, - }; - let apply_patch = GuardianApprovalRequest::ApplyPatch { - environment_id: "local".to_string(), - id: "patch-1".to_string(), - cwd: test_path_buf("/tmp").abs().into(), - files: vec![test_path_buf("/tmp/guardian.txt").abs().into()], - patch: "*** Begin Patch\n*** Update File: guardian.txt\n@@\n+hello\n*** End Patch" - .to_string(), - }; - - assert_eq!( - guardian_request_turn_id(&network_access, "fallback-turn"), - "owner-turn" - ); - assert_eq!( - guardian_request_turn_id(&apply_patch, "fallback-turn"), - "fallback-turn" - ); -} - -#[test] -fn guardian_request_target_item_id_omits_network_access_trigger_call_id() { - let network_access = GuardianApprovalRequest::NetworkAccess { - id: "network-1".to_string(), - turn_id: "owner-turn".to_string(), - environment_id: "local".to_string(), - target: "https://example.com:443".to_string(), - host: "example.com".to_string(), - protocol: NetworkApprovalProtocol::Https, - port: 443, - trigger: Some(GuardianNetworkAccessTrigger { - call_id: "call-1".to_string(), - tool_name: "shell".to_string(), - command: vec!["curl".to_string(), "https://example.com".to_string()], - cwd: PathUri::from_abs_path(&test_path_buf("/repo").abs()), - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - justification: None, - tty: None, - }), - }; - - assert_eq!(guardian_request_target_item_id(&network_access), None); -} - #[derive(Clone, Copy)] enum ApprovalCancellation { BeforeRouting, @@ -1723,139 +1289,6 @@ async fn cancelled_guardian_review_emits_terminal_abort_without_warning( assert!(warnings.is_empty()); } -#[test] -fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() { - let repeated = "signal ".repeat(8_000); - 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 { - status: None, - encrypted_content: None, - 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 { - status: None, - encrypted_content: None, - 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, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); - - assert!( - transcript - .iter() - .any(|entry| entry == "[1] user: please figure out if the repo is public") - ); - assert!(transcript.iter().any(|entry| { - entry == "[2] assistant: The public repo check is the main reason I want to escalate." - })); - assert!( - !transcript - .iter() - .any(|entry| entry.starts_with("[3] tool tool_0 call:")) - ); - assert!( - !transcript - .iter() - .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()); -} - -#[test] -fn build_guardian_transcript_preserves_recent_tool_context_when_user_history_is_large() { - let repeated = "authorization ".repeat(6_000); - let mut items = (0..8) - .map(|_| ResponseItem::Message { - status: None, - encrypted_content: None, - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: repeated.clone(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }) - .collect::>(); - items.extend([ - ResponseItem::FunctionCall { - status: None, - encrypted_content: None, - 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, - }, - 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, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); - - assert_eq!( - transcript[..8], - (1..=8) - .map(|index| format!("[{index}] user: {repeated}")) - .collect::>() - ); - assert!(transcript.iter().any(|entry| { - entry.contains("tool shell call:") - && entry.contains("curl") - && entry.contains("https://example.com/upload") - })); - assert!( - transcript - .iter() - .any(|entry| entry - .contains("tool shell result: sandbox blocked outbound network access")) - ); - assert_eq!(omission, None); -} - enum GuardianTestCatalog { Bundled, Explicit, @@ -4151,36 +3584,6 @@ async fn guardian_review_session_config_isolates_parent_customizations() { ); } -#[tokio::test] -async fn guardian_review_session_config_allows_pinned_disabled_feature() { - let mut parent_config = test_config().await; - parent_config.features = ManagedFeatures::from_configured( - parent_config.features.get().clone(), - Some(Sourced { - value: FeatureRequirementsToml { - entries: BTreeMap::from([("multi_agent".to_string(), true)]), - }, - source: RequirementSource::Unknown, - }), - ) - .expect("managed features"); - - let guardian_config = build_guardian_review_session_config_for_test( - crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"), - /*live_network_config*/ None, - "active-model", - /*reasoning_effort*/ None, - ReasoningSummary::default(), - /*personality*/ None, - ResolvedModelMessages::bundled(), - ) - .expect("guardian config should continue when a disabled feature is pinned on"); - - assert!(guardian_config.features.enabled(Feature::Collab)); - assert!(guardian_config.mcp_servers.get().is_empty()); - assert!(!guardian_config.include_apps_instructions); -} - #[tokio::test] async fn guardian_review_session_config_keeps_bedrock_provider_for_bedrock_gpt_5_5() { let mut parent_config = test_config().await; diff --git a/codex-rs/ext/guardian-reviewer/src/approval_request_tests.rs b/codex-rs/ext/guardian-reviewer/src/approval_request_tests.rs index 342e97f37e..15e86e7b9a 100644 --- a/codex-rs/ext/guardian-reviewer/src/approval_request_tests.rs +++ b/codex-rs/ext/guardian-reviewer/src/approval_request_tests.rs @@ -38,3 +38,59 @@ fn guardian_apply_patch_preserves_foreign_paths_and_redacts_patch_text() -> serd ); Ok(()) } + +#[test] +fn guardian_request_turn_id_prefers_network_access_owner_turn() { + let network_access = GuardianApprovalRequest::NetworkAccess { + id: "network-1".to_string(), + turn_id: "owner-turn".to_string(), + environment_id: "local".to_string(), + target: "https://example.com:443".to_string(), + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + port: 443, + trigger: None, + }; + let apply_patch = GuardianApprovalRequest::ApplyPatch { + environment_id: "local".to_string(), + id: "patch-1".to_string(), + cwd: PathUri::parse("file:///tmp").expect("valid cwd"), + files: vec![PathUri::parse("file:///tmp/guardian.txt").expect("valid file path")], + patch: "*** Begin Patch\n*** Update File: guardian.txt\n@@\n+hello\n*** End Patch" + .to_string(), + }; + + assert_eq!( + guardian_request_turn_id(&network_access, "fallback-turn"), + "owner-turn" + ); + assert_eq!( + guardian_request_turn_id(&apply_patch, "fallback-turn"), + "fallback-turn" + ); +} + +#[test] +fn guardian_request_target_item_id_omits_network_access_trigger_call_id() { + let network_access = GuardianApprovalRequest::NetworkAccess { + id: "network-1".to_string(), + turn_id: "owner-turn".to_string(), + environment_id: "local".to_string(), + target: "https://example.com:443".to_string(), + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + port: 443, + trigger: Some(GuardianNetworkAccessTrigger { + call_id: "call-1".to_string(), + tool_name: "shell".to_string(), + command: vec!["curl".to_string(), "https://example.com".to_string()], + cwd: PathUri::parse("file:///repo").expect("valid cwd"), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: None, + tty: None, + }), + }; + + assert_eq!(guardian_request_target_item_id(&network_access), None); +} diff --git a/codex-rs/ext/guardian-reviewer/src/prompt_tests.rs b/codex-rs/ext/guardian-reviewer/src/prompt_tests.rs index 63377f6b6e..5a5e3eba49 100644 --- a/codex-rs/ext/guardian-reviewer/src/prompt_tests.rs +++ b/codex-rs/ext/guardian-reviewer/src/prompt_tests.rs @@ -1,6 +1,23 @@ #![cfg(test)] +use super::GuardianPromptItems; +use super::GuardianPromptMode; +use super::GuardianTranscriptCursor; +use super::compose_guardian_prompt; use super::guardian_truncate_text; +use crate::GuardianApprovalRequest; +use crate::GuardianMcpAnnotations; +use crate::build_guardian_planned_action; +use codex_guardian_context::ContextProfile; +use codex_guardian_context::SectionHistory; +use codex_guardian_context::SectionInput; +use codex_guardian_context::default_registry; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::models::SandboxPermissions; +use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; #[test] fn guardian_truncate_text_keeps_prefix_suffix_and_xml_marker() { @@ -13,3 +30,283 @@ fn guardian_truncate_text_keeps_prefix_suffix_and_xml_marker() { assert!(truncated.ends_with("suffix")); assert!(was_truncated); } + +fn guardian_parent_history() -> Vec { + vec![ + ResponseItem::Message { + status: None, + encrypted_content: None, + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "Please check the repo visibility and push the docs fix if needed." + .to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::FunctionCall { + status: None, + encrypted_content: None, + id: None, + name: "gh_repo_view".to_string(), + namespace: None, + arguments: "{\"repo\":\"openai/codex\"}".to_string(), + call_id: "call-1".to_string(), + encrypted_function_args: None, + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::FunctionCallOutput { + id: None, + call_id: Some("call-1".to_string()), + name: None, + namespace: None, + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "repo visibility: public".to_string(), + ), + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::Message { + status: None, + encrypted_content: None, + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + annotations: None, + logprobs: None, + text: "The repo is public; I now need approval to push the docs fix.".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ] +} + +fn guardian_mcp_request(server: &str, tool_name: &str) -> GuardianApprovalRequest { + GuardianApprovalRequest::McpToolCall { + id: "mcp-1".to_string(), + server: server.to_string(), + tool_name: tool_name.to_string(), + arguments: Some(serde_json::json!({ + "code": "await browser.open('https://example.com')", + })), + connector_id: Some("connector-1".to_string()), + connector_name: Some("Connected tools".to_string()), + connector_description: None, + connected_account_email: None, + tool_title: Some("Execute JavaScript".to_string()), + tool_description: None, + annotations: Some(GuardianMcpAnnotations { + destructive_hint: None, + open_world_hint: Some(true), + read_only_hint: None, + }), + prompt_permissions: None, + } +} + +fn guardian_prompt_text(items: &[codex_protocol::user_input::UserInput]) -> String { + items + .iter() + .map(|item| match item { + codex_protocol::user_input::UserInput::Text { text, .. } => text.as_str(), + _ => "", + }) + .collect::() +} + +fn build_guardian_prompt_items( + history: &dyn SectionHistory, + request: GuardianApprovalRequest, + mode: GuardianPromptMode, +) -> anyhow::Result { + let profile = ContextProfile::synchronous(); + let action = build_guardian_planned_action(&request, /*reason*/ None)?; + let sections = default_registry().prepare(&SectionInput { + target: profile.target, + history, + transcript: &profile.transcript, + root_conversation: &[], + root_conversation_candidates: None, + trusted_user_answers: &[], + planned_action: Some(&action), + permissions: None, + previous_reviews: None, + trusted_tool: None, + trusted_skill_paths: &[], + images: None, + node_repl: None, + })?; + compose_guardian_prompt( + sections, + &profile, + mode, + /*history_version*/ 0, + "11111111-1111-4111-8111-111111111111", + /*node_repl_evidence_sequence*/ 0, + ) +} + +#[test] +fn build_guardian_prompt_delta_mode_preserves_original_numbering() -> anyhow::Result<()> { + let mut history = guardian_parent_history(); + history.extend([ + ResponseItem::Message { + status: None, + encrypted_content: None, + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "Please also push the second docs fix.".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::Message { + status: None, + encrypted_content: None, + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + annotations: None, + logprobs: None, + text: "I need approval for the second push.".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ]); + + let prompt = build_guardian_prompt_items( + &history, + GuardianApprovalRequest::ExecCommand { + id: "shell-2".to_string(), + environment_id: "local".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: PathUri::parse("file:///repo/codex-rs/core").expect("valid cwd"), + guardian_cwd: LegacyAppPathString::from_string("/repo/codex-rs/core"), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the second docs fix.".to_string()), + tty: false, + }, + GuardianPromptMode::Delta { + cursor: GuardianTranscriptCursor { + parent_history_version: 0, + transcript_entry_count: 4, + }, + }, + )?; + + let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); + assert!(text.contains("added since your last approval assessment")); + assert!(text.contains(">>> TRANSCRIPT DELTA START\n")); + assert!(text.contains("[5] user: Please also push the second docs fix.")); + assert!(text.contains("[6] assistant: I need approval for the second push.")); + assert!(text.contains(">>> TRANSCRIPT DELTA END\n")); + assert!(text.contains("The Codex agent has requested the following next action:\n")); + assert!(!text.contains("[1] user: Please check the repo visibility")); + assert_eq!(prompt.transcript_cursor.transcript_entry_count, 6); + + Ok(()) +} + +#[test] +fn build_guardian_prompt_delta_mode_handles_empty_delta() -> anyhow::Result<()> { + let history = guardian_parent_history(); + + let prompt = build_guardian_prompt_items( + &history, + GuardianApprovalRequest::ExecCommand { + id: "shell-2".to_string(), + environment_id: "local".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: PathUri::parse("file:///repo/codex-rs/core").expect("valid cwd"), + guardian_cwd: LegacyAppPathString::from_string("/repo/codex-rs/core"), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the second docs fix.".to_string()), + tty: false, + }, + GuardianPromptMode::Delta { + cursor: GuardianTranscriptCursor { + parent_history_version: 0, + transcript_entry_count: 4, + }, + }, + )?; + + let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); + assert!(text.contains(">>> TRANSCRIPT DELTA START\n")); + assert!(text.contains("")); + assert!(text.contains(">>> TRANSCRIPT DELTA END\n")); + assert_eq!(prompt.transcript_cursor.transcript_entry_count, 4); + + Ok(()) +} + +#[test] +fn build_guardian_prompt_stale_delta_cursor_falls_back_to_full_prompt() -> anyhow::Result<()> { + let history = guardian_parent_history(); + + let prompt = build_guardian_prompt_items( + &history, + GuardianApprovalRequest::ExecCommand { + id: "shell-3".to_string(), + environment_id: "local".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: PathUri::parse("file:///repo/codex-rs/core").expect("valid cwd"), + guardian_cwd: LegacyAppPathString::from_string("/repo/codex-rs/core"), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the docs fix.".to_string()), + tty: false, + }, + GuardianPromptMode::Delta { + cursor: GuardianTranscriptCursor { + parent_history_version: 0, + transcript_entry_count: 99, + }, + }, + )?; + + let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); + assert!(text.contains("whose request action you are assessing")); + assert!(text.contains(">>> TRANSCRIPT START\n")); + assert!(!text.contains("TRANSCRIPT DELTA")); + assert_eq!(prompt.transcript_cursor.transcript_entry_count, 4); + + Ok(()) +} + +#[test] +fn build_guardian_prompt_items_keeps_other_requests_generic() -> anyhow::Result<()> { + let history = guardian_parent_history(); + + for request in [ + guardian_mcp_request("node_repl", "js"), + guardian_mcp_request("node_repl", "inspect"), + guardian_mcp_request("another_server", "js"), + GuardianApprovalRequest::ExecCommand { + id: "shell-1".to_string(), + environment_id: "local".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: PathUri::parse("file:///repo/codex-rs/core").expect("valid cwd"), + guardian_cwd: LegacyAppPathString::from_string("/repo/codex-rs/core"), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Need to push the reviewed docs fix.".to_string()), + tty: false, + }, + ] { + let prompt = build_guardian_prompt_items(&history, request, GuardianPromptMode::Full)?; + + let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); + assert!(text.contains("Assess the exact planned action below.")); + assert!(text.contains("Planned action JSON:")); + assert!(!text.contains("Node REPL action JSON:")); + assert!(!text.contains("Distinguish preparation")); + } + + Ok(()) +} diff --git a/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs b/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs index e6fb25fdca..fd2c5fa9d5 100644 --- a/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs +++ b/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs @@ -280,3 +280,7 @@ pub fn install( #[cfg(test)] #[path = "lifecycle_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "reviewer_config_tests.rs"] +mod reviewer_config_tests; diff --git a/codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config_tests.rs b/codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config_tests.rs new file mode 100644 index 0000000000..21bcad7ded --- /dev/null +++ b/codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config_tests.rs @@ -0,0 +1,34 @@ +#![cfg(test)] + +//! Checks managed feature constraints while applying isolated reviewer settings. + +use super::reviewer_config::build_reviewer_config; +use codex_config::FeatureRequirementsToml; +use codex_config::RequirementSource; +use codex_config::Sourced; +use codex_core::config::ManagedFeatures; +use codex_features::Feature; +use std::collections::BTreeMap; + +#[tokio::test] +async fn guardian_review_session_config_allows_pinned_disabled_feature() { + let home = tempfile::tempdir().expect("create temp dir"); + let mut parent_config = core_test_support::load_default_config_for_test(&home).await; + parent_config.features = ManagedFeatures::from_configured( + parent_config.features.get().clone(), + Some(Sourced { + value: FeatureRequirementsToml { + entries: BTreeMap::from([("multi_agent".to_string(), true)]), + }, + source: RequirementSource::Unknown, + }), + ) + .expect("managed features"); + + let guardian_config = build_reviewer_config(&parent_config) + .expect("guardian config should continue when a disabled feature is pinned on"); + + assert!(guardian_config.features.enabled(Feature::Collab)); + assert!(guardian_config.mcp_servers.get().is_empty()); + assert!(!guardian_config.include_apps_instructions); +} diff --git a/codex-rs/guardian-context/src/preparation/source_tests.rs b/codex-rs/guardian-context/src/preparation/source_tests.rs index 902296e859..8834ca5994 100644 --- a/codex-rs/guardian-context/src/preparation/source_tests.rs +++ b/codex-rs/guardian-context/src/preparation/source_tests.rs @@ -1,6 +1,6 @@ #![cfg(test)] -//! Checks that elastic preparation keeps captured root and REPL evidence. +//! Checks that source preparation preserves captured evidence and bounds supporting text. use super::*; use crate::GuardianRootMessage; @@ -112,3 +112,57 @@ fn elastic_repl_rendering_keeps_stored_text_images_and_omissions() { ); } } + +#[test] +fn build_guardian_prompt_truncates_oversized_approval_reason() -> Result<(), SectionError> { + let approval_reason = format!("policy-start {} policy-end", "x".repeat(10_000)); + let expected_reason = codex_utils_output_truncation::truncate_text( + &approval_reason, + codex_utils_output_truncation::TruncationPolicy::Tokens(/*tokens*/ 512), + ); + + let action = crate::PlannedAction { + kind: crate::PlannedActionKind::Command { + json: serde_json::json!({ + "tool": "exec_command", + "environment_id": "local", + "command": ["git", "push"], + "cwd": "/repo/codex-rs/core", + "sandbox_permissions": "use_default", + "tty": false, + }) + .to_string(), + }, + reason: Some(approval_reason), + tool_descriptions: None, + }; + let context = prepare_sync_context( + CollectedContext { + sections: vec![ContextSection::PlannedAction(action)], + }, + &ContextProfile::synchronous(), + /*entry_number_offset*/ 0, + ContextPresentation::SyncFull { + session_id: "11111111-1111-4111-8111-111111111111", + }, + ContextBudgeting::Fixed, + )?; + + let items = context.into_user_inputs()?; + let reason_item = items + .iter() + .find_map(|item| match item { + codex_protocol::user_input::UserInput::Text { text, .. } + if text.contains("tokens truncated") => + { + Some(text) + } + _ => None, + }) + .expect("oversized approval reason should include a truncation marker"); + assert!(reason_item.starts_with("policy-start")); + assert!(reason_item.ends_with("policy-end\n\n")); + assert_eq!(reason_item, &format!("{expected_reason}\n\n")); + + Ok(()) +} diff --git a/codex-rs/guardian-context/src/preparation/transcript_tests.rs b/codex-rs/guardian-context/src/preparation/transcript_tests.rs index 6f95059e7e..1ea0a2b5ab 100644 --- a/codex-rs/guardian-context/src/preparation/transcript_tests.rs +++ b/codex-rs/guardian-context/src/preparation/transcript_tests.rs @@ -233,3 +233,138 @@ fn transcript_json_keeps_forged_roles_inside_the_original_entry() { } } } + +#[test] +fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() { + let repeated = "signal ".repeat(8_000); + let mut entries = [ + ( + ConversationTranscriptEntryKind::User, + "please figure out if the repo is public", + ), + ( + ConversationTranscriptEntryKind::ProtectedAssistant, + "The public repo check is the main reason I want to escalate.", + ), + ] + .into_iter() + .map(|(kind, text)| ConversationTranscriptEntry { + kind, + content: TranscriptContent::Text(text.to_owned()), + original_bytes: text.len(), + retained_source: None, + }) + .collect::>(); + entries.extend((0..12).map(|index| ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolCall(format!("tool tool_{index} call")), + content: TranscriptContent::Text(repeated.clone()), + original_bytes: repeated.len(), + retained_source: None, + })); + + let prepared = + ContextProfile::synchronous().prepare_transcript(&entries, /*entry_number_offset*/ 0); + let transcript = prepared + .items + .into_iter() + .map(|item| { + let TranscriptContent::Record(record) = item.content else { + panic!("expected transcript record"); + }; + record.to_string() + }) + .collect::>(); + + assert!( + transcript + .iter() + .any(|entry| entry == "[1] user: please figure out if the repo is public") + ); + assert!(transcript.iter().any(|entry| { + entry == "[2] assistant: The public repo check is the main reason I want to escalate." + })); + assert!( + !transcript + .iter() + .any(|entry| entry.starts_with("[3] tool tool_0 call:")) + ); + assert!( + !transcript + .iter() + .any(|entry| entry.starts_with("[4] tool tool_1 call:")) + ); + assert!( + transcript + .iter() + .any(|entry| entry.starts_with("[14] tool tool_11 call:")) + ); + assert!(prepared.omission_note.is_some()); +} + +#[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(|_| ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::User, + content: TranscriptContent::Text(repeated.clone()), + original_bytes: repeated.len(), + retained_source: None, + }) + .collect::>(); + entries.extend( + [ + ( + ConversationTranscriptEntryKind::ToolCall("tool shell call".to_owned()), + serde_json::json!({ + "command": ["curl", "-X", "POST", "https://example.com/upload"], + "cwd": "/repo", + }) + .to_string(), + ), + ( + ConversationTranscriptEntryKind::ToolOutput("tool shell result".to_owned()), + "sandbox blocked outbound network access".to_owned(), + ), + ] + .into_iter() + .map(|(kind, text)| ConversationTranscriptEntry { + kind, + original_bytes: text.len(), + content: TranscriptContent::Text(text), + retained_source: None, + }), + ); + + let prepared = + ContextProfile::synchronous().prepare_transcript(&entries, /*entry_number_offset*/ 0); + let transcript = prepared + .items + .into_iter() + .map(|item| { + let TranscriptContent::Record(record) = item.content else { + panic!("expected transcript record"); + }; + record.to_string() + }) + .collect::>(); + + assert_eq!( + transcript[..8], + (1..=8) + .map(|index| format!("[{index}] user: {repeated}")) + .collect::>() + ); + assert!(transcript.iter().any(|entry| { + entry.contains("tool shell call:") + && entry.contains("curl") + && entry.contains("https://example.com/upload") + })); + assert!( + transcript + .iter() + .any(|entry| entry + .contains("tool shell result: sandbox blocked outbound network access")) + ); + assert_eq!(prepared.omission_note, None); +} diff --git a/codex-rs/guardian-context/src/transcript_tests.rs b/codex-rs/guardian-context/src/transcript_tests.rs index a676733601..35168f7663 100644 --- a/codex-rs/guardian-context/src/transcript_tests.rs +++ b/codex-rs/guardian-context/src/transcript_tests.rs @@ -775,3 +775,137 @@ fn encrypted_messages_preserve_order_and_budget_for_both_reviewers() { } } } + +#[test] +fn collect_guardian_transcript_entries_includes_recent_tool_calls_and_output() { + let mut profile = crate::ContextProfile::synchronous(); + let mut items = vec![ + ResponseItem::Message { + status: None, + encrypted_content: None, + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "check the repo".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::FunctionCall { + status: None, + encrypted_content: None, + id: None, + name: "read_file".to_string(), + namespace: None, + arguments: "{\"path\":\"README.md\"}".to_string(), + call_id: "call-1".to_string(), + encrypted_function_args: None, + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::FunctionCallOutput { + id: None, + call_id: Some("call-1".to_string()), + name: None, + namespace: None, + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "repo is public".to_string(), + ), + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::Message { + status: None, + encrypted_content: None, + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + annotations: None, + logprobs: None, + text: "I need to push a fix".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ]; + + let entries = + super::collect_transcript_for_target(&items, &profile.transcript, ContextTarget::Sync); + + assert_eq!(entries.len(), 4); + assert_eq!( + entries[1], + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolCall("tool read_file call".to_string()), + content: crate::TranscriptContent::Text("{\"path\":\"README.md\"}".to_string()), + original_bytes: "{\"path\":\"README.md\"}".len(), + retained_source: None, + } + ); + assert_eq!( + entries[2], + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::ToolOutput("tool read_file result".to_string()), + content: crate::TranscriptContent::Text("repo is public".to_string()), + original_bytes: "repo is public".len(), + retained_source: None, + } + ); + if let ResponseItem::FunctionCall { namespace, .. } = &mut items[1] { + *namespace = Some("mcp__node_repl__".to_string()); + } + assert!(matches!( + super::collect_transcript_for_target(&items, &profile.transcript, ContextTarget::Sync)[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 [profile.transcript.entry_limits.tool_tokens, 6_000] { + profile.transcript.entry_limits.node_repl_output_tokens = token_cap; + let entries = + super::collect_transcript_for_target(&items, &profile.transcript, ContextTarget::Sync); + assert_eq!( + entries[2], + ConversationTranscriptEntry { + kind: ConversationTranscriptEntryKind::NodeReplToolOutput( + "tool read_file result".to_string() + ), + content: crate::TranscriptContent::Text(oversized_result.clone()), + original_bytes: oversized_result.len(), + retained_source: None, + } + ); + assert_eq!(entries.len(), 4); + let prepared = profile.prepare_transcript(&entries, /*entry_number_offset*/ 0); + let rendered = prepared + .items + .into_iter() + .map(|item| match item.content { + crate::TranscriptContent::Text(text) => text, + crate::TranscriptContent::Record(record) => record.to_string(), + crate::TranscriptContent::ScheduledMessage(_) + | crate::TranscriptContent::NativeMessage(_) => { + panic!("expected text transcript") + } + }) + .collect::>(); + assert_eq!( + (rendered, prepared.omission_note), + ( + vec![ + "[1] user: check the repo".to_string(), + "[2] tool read_file call: {\"path\":\"README.md\"}".to_string(), + format!( + "[3] tool read_file result: {}", + truncate_text(&oversized_result, token_cap) + ), + "[4] assistant: I need to push a fix".to_string(), + ], + None, + ) + ); + } +}