diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index ffed746fc8..e1c67e52b0 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -537,20 +537,8 @@ pub(crate) fn guardian_request_turn_id<'a>( pub(crate) fn format_guardian_action_pretty( action: &GuardianApprovalRequest, ) -> serde_json::Result { - let mut value = guardian_action_for_review(action)?; + let mut value = + codex_guardian_context::action_for_review(guardian_approval_request_to_json(action)?); value.sort_all_objects(); serde_json::to_string_pretty(&value) } - -fn guardian_action_for_review(action: &GuardianApprovalRequest) -> serde_json::Result { - let mut value = guardian_approval_request_to_json(action)?; - if matches!(action, GuardianApprovalRequest::McpToolCall { .. }) - && let Some(fields) = value.as_object_mut() - { - // Only host-provided metadata is optional. A nested argument named - // "description" is still part of the exact action under review. - fields.remove("tool_description"); - fields.remove("connector_description"); - } - Ok(value) -} diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs b/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs index 2e4c2d24e2..a557ce1845 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs @@ -140,14 +140,7 @@ async fn cached_evidence( return Err(GuardianReviewReason::MissingScore); }; // Elicitations and intercepted execs can expand beyond the original scored action. - let mut action = input.action.clone(); - if action.get("tool").and_then(serde_json::Value::as_str) == Some("mcp_tool_call") - && let Some(fields) = action.as_object_mut() - { - // Match the action renderer: host descriptions are optional, arguments are not. - fields.remove("tool_description"); - fields.remove("connector_description"); - } + let action = codex_guardian_context::action_for_review(input.action.clone()); let max_action_bytes = TruncationPolicy::Tokens(config.max_action_tokens).byte_budget(); let action_fits = serde_json::to_string_pretty(&action) .is_ok_and(|action| action.len().saturating_add(1) <= max_action_bytes); diff --git a/codex-rs/guardian-context/src/action.rs b/codex-rs/guardian-context/src/action.rs index 8b2022cc28..3883de031e 100644 --- a/codex-rs/guardian-context/src/action.rs +++ b/codex-rs/guardian-context/src/action.rs @@ -8,6 +8,18 @@ use crate::SectionError; use crate::SectionInput; use crate::SectionScope; +/// Projects an approval action onto the exact JSON reviewed and measured by Guardian. +/// Host descriptions are optional; nested tool arguments must remain complete. +pub fn action_for_review(mut action: serde_json::Value) -> serde_json::Value { + if action.get("tool").and_then(serde_json::Value::as_str) == Some("mcp_tool_call") + && let Some(fields) = action.as_object_mut() + { + fields.remove("tool_description"); + fields.remove("connector_description"); + } + action +} + /// Host-prepared action evidence. JSON stays complete through request admission. #[derive(Clone, PartialEq)] pub struct PlannedAction { @@ -111,6 +123,10 @@ impl PlannedAction { pub(crate) struct PlannedActionSection; +#[cfg(test)] +#[path = "action_tests.rs"] +mod tests; + impl SectionContributor for PlannedActionSection { fn scope(&self) -> SectionScope { SectionScope::Shared diff --git a/codex-rs/guardian-context/src/action_tests.rs b/codex-rs/guardian-context/src/action_tests.rs new file mode 100644 index 0000000000..c1efc4331a --- /dev/null +++ b/codex-rs/guardian-context/src/action_tests.rs @@ -0,0 +1,41 @@ +//! Approval projections omit optional host metadata without changing the actual action. + +use super::action_for_review; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[test] +fn mcp_projection_preserves_arguments_with_description_fields() { + let arguments = json!({ + "description": "required argument", + "tool_description": "also an argument", + "nested": {"connector_description": "required nested argument"}, + }); + let action = json!({ + "tool": "mcp_tool_call", + "server": "example", + "tool_name": "publish", + "tool_description": "optional host metadata", + "connector_description": "optional connector metadata", + "arguments": arguments, + }); + assert_eq!( + action_for_review(action), + json!({ + "tool": "mcp_tool_call", + "server": "example", + "tool_name": "publish", + "arguments": arguments, + }), + ); +} + +#[test] +fn other_actions_preserve_description_fields() { + let action = json!({ + "tool": "custom_action", + "tool_description": "required argument", + "connector_description": "also required", + }); + assert_eq!(action_for_review(action.clone()), action); +} diff --git a/codex-rs/guardian-context/src/lib.rs b/codex-rs/guardian-context/src/lib.rs index f12d9b1baa..36bce55ae1 100644 --- a/codex-rs/guardian-context/src/lib.rs +++ b/codex-rs/guardian-context/src/lib.rs @@ -23,6 +23,7 @@ use transcript::ConversationTranscriptSection; pub use action::ActionPresentation; pub use action::PlannedAction; pub use action::PlannedActionKind; +pub use action::action_for_review; pub use authorization::GuardianRootMessage; pub use section::ContextSection;