openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-20 12:47:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Trim Guardian tests and tighten request layout assertions (#46063)

Browse Source 
## What changed

Remove standalone tests across Guardian review sessions, configuration, assessment schemas, transcripts, and trusted skill paths.

Strengthen the existing request layout snapshot test to check exact transcript separators, the action header, and parent session ID placement before snapshot whitespace normalization. Concatenate user input text without inserting newlines so the assertions validate the actual request formatting.

GitOrigin-RevId: 06d262a1a85b1a8401796958f300c894146d2e1b
This commit is contained in:
felixxia-oai
2026-09-17 00:25:25 +00:00
committed by copyberry
parent 8ace915ace
commit e412b93d08
5 changed files with 5 additions and 370 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
codex-rs/core/src/guardian/review_session_tests.rs
View File

@@ -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;

200
codex-rs/core/src/guardian/tests.rs
View File

@@ -398,40 +398,6 @@ fn last_user_message_text_from_body(body: &serde_json::Value) -> String {
.collect::<String>()
}
#[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::<String>();
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<Session>,

31
codex-rs/ext/guardian-reviewer/src/assessment_tests.rs
View File

@@ -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"]
})
);
}

90
codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs
View File

@@ -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("<truncated omitted_approx_tokens=\""));
assert!(truncated.ends_with("é suffix"));
assert!(truncated.len() <= TruncationPolicy::Tokens(200).byte_budget());
}
#[test]
fn transcript_keeps_only_manual_approval_developer_messages() {
let approval_text = format!("{MANUAL_APPROVAL_DEVELOPER_PREFIX}\n\nApproved action:\n{{}}");

8
codex-rs/guardian-context/src/trusted_skills_tests.rs
View File

@@ -16,14 +16,6 @@ fn rendered_paths(paths: Vec<String>) -> Vec<String> {
.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)