mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Centralize Guardian action preparation for review (#45987)
## What changed Move duplicated action preparation into `codex_guardian_context::action_for_review`, shared by approval rendering and cached-evidence size checks. Preserve the existing behavior: omit top-level `tool_description` and `connector_description` metadata from MCP tool calls while keeping tool arguments intact. ## Testing Add unit tests verifying that MCP arguments retain description fields, including nested fields, and that other action types remain unchanged. GitOrigin-RevId: 8e890fb263556596f695177f917750377c04d43c
This commit is contained in:
@@ -537,20 +537,8 @@ pub(crate) fn guardian_request_turn_id<'a>(
|
||||
pub(crate) fn format_guardian_action_pretty(
|
||||
action: &GuardianApprovalRequest,
|
||||
) -> serde_json::Result<String> {
|
||||
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<Value> {
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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
|
||||
|
||||
41
codex-rs/guardian-context/src/action_tests.rs
Normal file
41
codex-rs/guardian-context/src/action_tests.rs
Normal file
@@ -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);
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user