diff --git a/codex-rs/core/src/guardian/review_session_tests.rs b/codex-rs/core/src/guardian/review_session_tests.rs index 255adf4276..6220fd5a58 100644 --- a/codex-rs/core/src/guardian/review_session_tests.rs +++ b/codex-rs/core/src/guardian/review_session_tests.rs @@ -695,17 +695,6 @@ async fn guardian_review_session_config_resolves_policy_and_template( ); } -#[test] -fn had_prior_review_context_tracks_prompt_mode() { - assert!(!had_prior_review_context(&GuardianPromptMode::Full)); - assert!(had_prior_review_context(&GuardianPromptMode::Delta { - cursor: GuardianTranscriptCursor { - parent_history_version: 7, - transcript_entry_count: 42, - } - })); -} - #[test] fn token_usage_delta_never_reports_negative_usage() { let start = TokenUsage { @@ -847,41 +836,6 @@ async fn run_review_removes_trunk_when_event_stream_is_broken() { assert!(manager.trunk().await.is_none()); } -#[tokio::test] -async fn wait_for_guardian_review_ignores_prior_turn_completion() { - let (review_session, tx_event, _rx_sub) = test_review_session().await; - tx_event - .send(turn_complete_event("prior-turn", Some("stale"), Some(9))) - .await - .expect("queue prior turn completion"); - tx_event - .send(turn_complete_event("current-turn", Some("fresh"), Some(42))) - .await - .expect("queue current turn completion"); - - let mut analytics_result = GuardianReviewAnalyticsResult::without_session(); - let codex_guardian_reviewer::ReviewTurnResult { - outcome, - disposition, - turn_completed, - } = wait_for_guardian_review( - &review_session, - "current-turn", - tokio::time::Instant::now() + Duration::from_secs(1), - /*external_cancel*/ None, - &mut analytics_result, - ) - .await; - - let GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)) = outcome else { - panic!("expected current turn completion"); - }; - assert_eq!(last_agent_message.as_deref(), Some("fresh")); - assert_eq!(analytics_result.time_to_first_token_ms, Some(42)); - assert_eq!(disposition, SessionDisposition::Reusable); - assert!(turn_completed); -} - #[tokio::test] async fn wait_for_guardian_review_ignores_prior_turn_errors() { let (review_session, tx_event, _rx_sub) = test_review_session().await; diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 647b9d6173..3d430ba72b 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -398,40 +398,6 @@ fn last_user_message_text_from_body(body: &serde_json::Value) -> String { .collect::() } -#[tokio::test(flavor = "current_thread")] -async fn build_guardian_prompt_full_mode_preserves_initial_review_format() -> 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(), - Some("Sandbox denied outbound git push to github.com.".to_string()), - 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: Some("Need to push the reviewed docs fix.".to_string()), - tty: false, - }, - GuardianPromptMode::Full, - ) - .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 END\n")); - assert!(text.contains("The Codex agent has requested the following action:\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_prefers_retry_reason_over_approval_reason() -> anyhow::Result<()> { let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; @@ -896,43 +862,6 @@ fn collect_guardian_transcript_entries_skips_contextual_user_messages() { ); } -#[test] -fn collect_guardian_transcript_entries_keeps_manual_approval_developer_message() { - let approval_text = - format!("{AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX}\n\nApproved action:\n{{}}"); - let items = vec![ - ResponseItem::Message { - id: None, - role: "developer".to_string(), - content: vec![ContentItem::InputText { - text: "ordinary developer context".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }, - ResponseItem::Message { - id: None, - role: "developer".to_string(), - content: vec![ContentItem::InputText { - text: approval_text.clone(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }, - ]; - - let entries = collect_guardian_transcript_entries(&items, GUARDIAN_MAX_TOOL_ENTRY_TOKENS); - - assert_eq!( - entries, - vec![ConversationTranscriptEntry { - kind: ConversationTranscriptEntryKind::Developer, - original_bytes: approval_text.len(), - text: approval_text, - }] - ); -} - #[test] fn collect_guardian_transcript_entries_includes_recent_tool_calls_and_output() { let mut items = vec![ @@ -1572,20 +1501,6 @@ async fn routes_approval_to_guardian_requires_guardian_reviewer() { assert!(routes_approval_to_guardian(&turn)); } -#[tokio::test] -async fn routes_approval_to_guardian_can_use_app_reviewer_override() { - let (_session, turn) = crate::session::tests::make_session_and_context().await; - - assert!(!routes_approval_to_guardian_with_reviewer( - &turn, - ApprovalsReviewer::User - )); - assert!(routes_approval_to_guardian_with_reviewer( - &turn, - ApprovalsReviewer::AutoReview - )); -} - #[tokio::test] async fn routes_approval_to_guardian_allows_granular_review_policy() { let (_session, mut turn) = crate::session::tests::make_session_and_context().await; @@ -2223,9 +2138,12 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot() guardian_nested_tool_names, vec!["exec_command", "view_image", "write_stdin"] ); - let guardian_user_text = request.message_input_texts("user").join("\n"); + // Check exact separators before the snapshot normalizes trailing whitespace. + let guardian_user_text = request.message_input_texts("user").concat(); + assert!(guardian_user_text.contains(">>> TRANSCRIPT START\n")); + assert!(guardian_user_text.contains("The Codex agent has requested the following action:\n")); assert!(guardian_user_text.contains(&format!( - "Reviewed Codex session id: {}", + ">>> TRANSCRIPT END\nReviewed Codex session id: {}\n", fixed_guardian_parent_session_id() ))); assert!( @@ -2304,48 +2222,6 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot() Ok(()) } -#[tokio::test] -async fn build_guardian_prompt_items_includes_parent_session_id() -> anyhow::Result<()> { - let (session, _) = crate::session::tests::make_session_and_context().await; - let prompt = build_guardian_prompt_items( - &session, - /*retry_reason*/ None, - GuardianApprovalRequest::ExecCommand { - id: "shell-1".to_string(), - environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - command: vec!["git".to_string(), "status".to_string()], - cwd: test_path_buf("/repo").abs().into(), - guardian_cwd: native_guardian_cwd("/repo"), - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - justification: None, - tty: false, - }, - GuardianPromptMode::Full, - ) - .await?; - let prompt_text = prompt - .context - .into_user_inputs()? - .into_iter() - .map(|item| match item { - codex_protocol::user_input::UserInput::Text { text, .. } => text, - codex_protocol::user_input::UserInput::Image { .. } => String::new(), - _ => String::new(), - }) - .collect::(); - - assert!( - prompt_text.contains(&format!( - ">>> TRANSCRIPT END\nReviewed Codex session id: {}\n", - session.thread_id - )), - "guardian prompt should expose the parent session id immediately after the transcript end" - ); - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); @@ -3986,25 +3862,6 @@ async fn guardian_review_session_config_allows_pinned_disabled_feature() { assert!(!guardian_config.include_apps_instructions); } -#[tokio::test] -async fn guardian_review_session_config_uses_parent_active_model_instead_of_hardcoded_slug() { - let mut parent_config = test_config().await; - parent_config.model = Some("configured-model".to_string()); - - 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"); - - assert_eq!(guardian_config.model, Some("active-model".to_string())); -} - #[tokio::test] async fn guardian_review_session_config_keeps_bedrock_provider_for_bedrock_gpt_5_4() { let mut parent_config = test_config().await; @@ -4096,53 +3953,6 @@ async fn guardian_review_session_config_uses_requirements_guardian_policy_config ); } -#[tokio::test] -async fn guardian_review_session_config_uses_default_guardian_policy_without_requirements_override() -{ - let defaults = ResolvedModelMessages::bundled().auto_review(); - let codex_home = tempfile::tempdir().expect("create temp dir"); - let workspace = tempfile::tempdir().expect("create temp dir"); - let config_layer_stack = - ConfigLayerStack::new(Vec::new(), Default::default(), Default::default()) - .expect("config layer stack"); - let parent_config = Config::load_config_with_layer_stack( - LOCAL_FS.as_ref(), - ConfigToml::default(), - ConfigOverrides { - cwd: Some(workspace.path().to_path_buf()), - ..Default::default() - }, - codex_home.abs(), - config_layer_stack, - ) - .await - .expect("load config"); - - 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"); - - assert_eq!(guardian_config.developer_instructions, None); - assert_eq!( - guardian_config.base_instructions, - Some( - GuardianPolicyInstructions::new( - defaults.policy, - defaults.policy_template, - guardian_output_contract_prompt(), - ) - .render() - ) - ); -} - // Keep the existing reviewer tests on the production decision path. async fn review_approval_request( session: &Arc, diff --git a/codex-rs/ext/guardian-reviewer/src/assessment_tests.rs b/codex-rs/ext/guardian-reviewer/src/assessment_tests.rs index f30113a7aa..5cdd8e311e 100644 --- a/codex-rs/ext/guardian-reviewer/src/assessment_tests.rs +++ b/codex-rs/ext/guardian-reviewer/src/assessment_tests.rs @@ -50,34 +50,3 @@ fn parse_guardian_assessment_treats_bare_deny_as_high_risk() { } ); } - -#[test] -fn guardian_output_schema_requires_only_outcome_and_allows_optional_details() { - let schema = guardian_output_schema(); - - assert_eq!( - schema, - serde_json::json!({ - "type": "object", - "additionalProperties": false, - "properties": { - "risk_level": { - "type": "string", - "enum": ["low", "medium", "high", "critical"] - }, - "user_authorization": { - "type": "string", - "enum": ["unknown", "low", "medium", "high"] - }, - "outcome": { - "type": "string", - "enum": ["allow", "deny"] - }, - "rationale": { - "type": "string" - } - }, - "required": ["outcome"] - }) - ); -} 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 b7f0bc7263..f8ebb0aca6 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 @@ -2,7 +2,6 @@ use super::ContextInput; use codex_extension_api::ConversationHistorySnapshot; use codex_extension_api::ResponseItem; use codex_guardian_context::ContextTarget; -use codex_guardian_context::truncate_text as truncate_entry; use codex_protocol::AgentPath; use codex_protocol::models::AgentMessageInputContent; use codex_protocol::models::ContentItem; @@ -1029,84 +1028,6 @@ fn transcript_truncates_tool_results_using_standard_budget() { ); } -#[test] -fn transcript_preserves_outputs_with_call_ids_or_explicit_names() { - 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, - }]; - 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: FunctionCallOutputPayload::from_text(text.to_string()), - internal_chat_message_metadata_passthrough: None, - }), - ); - - assert_eq!( - TranscriptConfig::default() - .build_context(ContextInput { - target: ContextTarget::Async, - history: &TestConversationHistory(&items), - root_conversation: &[], - trusted_user_answers: &[], - planned_action: None, - previous_reviews: None, - trusted_tool: None, - trusted_skill_paths: &[], - node_repl_images: None, - }) - .expect("collect transcript") - .transcript_entries(), - vec![ - "[1] tool slack.notifications result: new message\n", - "[2] tool result: orphaned function output\n", - ] - ); - - if let ResponseItem::FunctionCallOutput { output, .. } = &mut items[0] { - *output = FunctionCallOutputPayload::from_content_items(vec![ - FunctionCallOutputContentItem::InputImage { - image: ImageReference::Inline { - image_url: "data:image/png;base64,image".to_owned(), - }, - detail: None, - }, - ]); - } - assert_eq!( - TranscriptConfig::default() - .build_context(ContextInput { - target: ContextTarget::Async, - history: &TestConversationHistory(&items), - root_conversation: &[], - trusted_user_answers: &[], - planned_action: None, - previous_reviews: None, - trusted_tool: None, - trusted_skill_paths: &[], - node_repl_images: None, - }) - .expect("collect transcript") - .transcript_entries(), - vec![ - "[1] tool slack.notifications result: [non-text output]\n", - "[2] tool result: orphaned function output\n", - ] - ); -} - #[test] fn configured_reasoning_counts_against_message_budget() { for (repeats, include_reasoning) in [(200, true), (1_000, false)] { @@ -1157,17 +1078,6 @@ fn configured_reasoning_counts_against_message_budget() { } } -#[test] -fn truncate_entry_preserves_prefix_suffix_and_utf8_boundaries() { - let text = format!("prefix é{}é suffix", "🦀".repeat(2_000)); - let truncated = truncate_entry(&text, /*max_tokens*/ 200); - - assert!(truncated.starts_with("prefix é")); - assert!(truncated.contains(") -> Vec { .expect("trusted skill evidence should remain valid JSON") } -#[test] -fn renders_verified_skill_paths() { - assert_eq!( - rendered_paths(vec!["/home/user/.codex/skills/demo/SKILL.md".to_owned()]), - vec!["/home/user/.codex/skills/demo/SKILL.md"], - ); -} - #[test] fn bounds_escaped_skill_paths_without_corrupting_json_or_utf8() { let paths = (0..16)