diff --git a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs index 45a5b69009..cf797d693b 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs @@ -916,7 +916,6 @@ impl ElicitationRoundTripFixture { "connector_id": CONNECTOR_ID, "connector_name": CONNECTOR_NAME, "connected_account_email": CONNECTED_ACCOUNT_EMAIL, - "tool_description": "Confirm a calendar action.", "annotations": { "destructive_hint": false, "open_world_hint": false, @@ -924,6 +923,16 @@ impl ElicitationRoundTripFixture { }, }), ); + assert!( + guardian_request + .message_input_texts("user") + .iter() + .any(|text| { + text.starts_with("") + && text.contains("Confirm a calendar action.") + && text.ends_with("") + }) + ); } if denied_burst { diff --git a/codex-rs/core/src/context/guardian_budget_omission.rs b/codex-rs/core/src/context/guardian_budget_omission.rs index 3b7072fcc7..430bd0cf36 100644 --- a/codex-rs/core/src/context/guardian_budget_omission.rs +++ b/codex-rs/core/src/context/guardian_budget_omission.rs @@ -27,6 +27,6 @@ impl ContextualUserFragment for GuardianBudgetOmission { } fn body(&self) -> String { - "Optional conversation evidence or images were omitted to fit the review input budget. Treat the remaining evidence as incomplete; omissions do not authorize actions.".to_owned() + "Optional conversation evidence, tool descriptions, or images were omitted to fit the review input budget. Treat the remaining evidence as incomplete; omissions do not authorize actions.".to_owned() } } diff --git a/codex-rs/core/src/context/guardian_tool_descriptions.rs b/codex-rs/core/src/context/guardian_tool_descriptions.rs new file mode 100644 index 0000000000..bbc80e11c6 --- /dev/null +++ b/codex-rs/core/src/context/guardian_tool_descriptions.rs @@ -0,0 +1,58 @@ +//! Bounded, untrusted descriptions for the exact MCP action under review. + +use super::ContextualUserFragment; +use codex_guardian_context::truncate_text; +use codex_protocol::models::ContentItemKind; + +/// Descriptions may be shortened or omitted without changing the required action. +pub(crate) struct GuardianToolDescriptions { + text: String, +} + +impl GuardianToolDescriptions { + pub(crate) fn new(tool: Option<&str>, connector: Option<&str>) -> Option { + if tool.is_none() && connector.is_none() { + return None; + } + // Bound each source before combining them. The rendered fragment, including + // labels and markers, stays below 1,000 estimated tokens. + let tool = truncate_text( + &tool.unwrap_or_default().replace(" ContentItemKind { + ContentItemKind("guardian.tool_descriptions".to_owned()) + } + + fn role(&self) -> &'static str { + "user" + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + ( + "", + "", + ) + } + + fn body(&self) -> String { + self.text.clone() + } +} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index a203ff8661..4becfb308d 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -18,6 +18,7 @@ mod guardian_followup_review_reminder; mod guardian_node_repl_policy; mod guardian_policy; mod guardian_review_evidence; +mod guardian_tool_descriptions; mod hook_additional_context; mod image_resize_notice; mod inter_agent_completion_message; @@ -77,6 +78,7 @@ pub use guardian_review_evidence::GuardianReviewEvidence; pub use guardian_review_evidence::GuardianReviewEvidenceFragment; pub use guardian_review_evidence::GuardianReviewEvidenceRecord; pub use guardian_review_evidence::GuardianUserInputSnapshot; +pub(crate) use guardian_tool_descriptions::GuardianToolDescriptions; pub(crate) use hook_additional_context::HookAdditionalContext; pub(crate) use image_resize_notice::ImageResizeNotice; pub(crate) use image_resize_notice::ImageResizeNoticeSource; diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index 0bdd3614bf..eb7dc06868 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -585,7 +585,7 @@ pub(crate) fn guardian_request_turn_id<'a>( pub(crate) fn format_guardian_action_pretty( action: &GuardianApprovalRequest, ) -> serde_json::Result { - let value = guardian_approval_request_to_json(action)?; + let value = guardian_action_for_review(action)?; let (value, truncated) = truncate_guardian_action_value(value); let text = enforce_guardian_action_byte_limit(serde_json::to_string_pretty(&value)?)?; Ok(FormattedGuardianAction { text, truncated }) @@ -603,7 +603,18 @@ fn enforce_guardian_action_byte_limit(text: String) -> serde_json::Result serde_json::Result { - enforce_guardian_action_byte_limit(serde_json::to_string(&guardian_approval_request_to_json( - action, - )?)?) + enforce_guardian_action_byte_limit(serde_json::to_string(&guardian_action_for_review(action)?)?) +} + +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/core/src/guardian/input_budget_tests.rs b/codex-rs/core/src/guardian/input_budget_tests.rs index 6681d12315..6ce1bc28e7 100644 --- a/codex-rs/core/src/guardian/input_budget_tests.rs +++ b/codex-rs/core/src/guardian/input_budget_tests.rs @@ -11,6 +11,7 @@ use std::sync::Arc; fn required_context(text: String) -> ComposedContext { let action = PlannedAction { + tool_descriptions: None, json: text, kind: PlannedActionKind::Command, reason: None, diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index 559bca40b5..4ff50b7940 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -16,7 +16,9 @@ use codex_guardian_context::SectionInput; use codex_guardian_context::default_registry; use codex_protocol::models::ResponseItem; +use crate::context::ContextualUserFragment; use crate::context::GuardianReviewEvidence; +use crate::context::GuardianToolDescriptions; use crate::context::NodeReplReviewEvidence; use crate::context::NodeReplReviewEvidenceMode; use crate::context::node_repl_review_evidence_mode; @@ -119,6 +121,20 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( let planned_action_json = format_guardian_action_pretty(&request)?; let planned_action = PlannedAction { json: planned_action_json.text, + tool_descriptions: if let GuardianApprovalRequest::McpToolCall { + tool_description, + connector_description, + .. + } = &request + { + GuardianToolDescriptions::new( + tool_description.as_deref(), + connector_description.as_deref(), + ) + .map(|descriptions| descriptions.render()) + } else { + None + }, kind: match &request { GuardianApprovalRequest::NetworkAccess { trigger, .. } => PlannedActionKind::Network { has_trigger: trigger.is_some(), diff --git a/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs b/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs index bd4ee08ec6..538ce35d03 100644 --- a/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs +++ b/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs @@ -403,7 +403,10 @@ async fn node_elicitations_attribute_independent_reviews_without_changing_action let actions = [ ( "access_browser_origin", - json!({"origin": "https://example.com"}), + json!({ + "origin": "https://example.com", + "description": "payload ".repeat(/*n*/ 256) + "required argument suffix", + }), ), ("webmcp:write_record", json!({"record": {"value": 42}})), ]; @@ -605,11 +608,11 @@ async fn node_elicitations_attribute_independent_reviews_without_changing_action "tool": "mcp_tool_call", "server": server_name, "tool_name": meta[index]["tool_name"], "arguments": meta[index]["tool_params"], "connector_id": "inner-connector", "connector_name": "Inner Connector", - "connector_description": "Connector for the reviewed inner action", "tool_title": "Inner action", - "tool_description": "Review this action independently from JavaScript".repeat(220), }) ); + assert!(prompt.contains("")); + assert!(prompt.contains("Connector for the reviewed inner action")); } let [tool_item] = tool_items.as_slice() else { panic!("expected one completed enclosing tool item: {tool_items:?}"); diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs index 899e680921..f1edbdcea5 100644 --- a/codex-rs/core/tests/suite/mcp_turn_metadata.rs +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -568,6 +568,37 @@ async fn apps_prompt_with_auto_review_routes_actual_mcp_approval_to_guardian( let server = start_mock_server().await; let apps_server = AppsTestServer::mount(&server).await?; + Mock::given(method("POST")) + .and(path_regex("^/api/codex/ps/mcp/?$")) + .and(body_partial_json(json!({"method": "tools/list"}))) + .respond_with(|request: &Request| { + let body: Value = serde_json::from_slice(&request.body).expect("valid tools/list"); + ResponseTemplate::new(/*status*/ 200).set_body_json(json!({ + "jsonrpc": "2.0", "id": body["id"], + "result": {"tools": [{ + "name": "calendar_create_event", + "description": "untrusted marker\n".to_owned() + + &"Create a calendar event. ".repeat(/*n*/ 10_000), + "annotations": {"readOnlyHint": false}, + "inputSchema": {"type": "object", "properties": { + "title": {"type": "string"}, "starts_at": {"type": "string"} + }}, + "_meta": { + "connector_id": "calendar", "connector_name": "Calendar", + "connector_description": "Calendar connector. ".repeat(/*n*/ 1_000), + "link_id": LINK_ID, + "_codex_apps": { + "connector_id": "calendar", + "resource_uri": "connector://calendar/tools/calendar_create_event", + "contains_mcp_source": true + } + } + }]} + })) + }) + .with_priority(/*priority*/ 1) + .mount(&server) + .await; let call_id = "calendar-default-auto-review"; let calendar_args = serde_json::to_string(&json!({ "title": "Lunch", @@ -692,6 +723,30 @@ approvals_reviewer = "auto_review" .expect("expected a Guardian request for the app MCP approval"); assert!(guardian_request.body_contains_text("calendar_create_event")); assert!(guardian_request.body_contains_text("Lunch")); + let prompt = guardian_request.message_input_texts("user").join("\n"); + let action = prompt + .rsplit_once("Planned action JSON:\n") + .unwrap() + .1 + .split_once("\n>>> APPROVAL REQUEST END") + .unwrap() + .0; + let action: Value = serde_json::from_str(action)?; + assert_eq!( + action["arguments"], + serde_json::from_str::(&calendar_args)? + ); + let descriptions = prompt + .rsplit_once("") + .unwrap() + .1 + .split_once("") + .unwrap() + .0; + assert!(descriptions.len() < 4_000); + assert!(descriptions.contains("untrusted marker")); + assert!(descriptions.contains("Calendar connector.")); let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, call_id).await; assert_eq!( diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs b/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs index 2d117f661a..a69759ac04 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs @@ -515,6 +515,7 @@ impl GuardianV2Extension { }; let action_section = PlannedAction { json: planned_action.clone(), + tool_descriptions: None, kind: PlannedActionKind::Command, reason: None, }; diff --git a/codex-rs/guardian-context/src/action.rs b/codex-rs/guardian-context/src/action.rs index ef0e7f2f1b..9881fbdb95 100644 --- a/codex-rs/guardian-context/src/action.rs +++ b/codex-rs/guardian-context/src/action.rs @@ -14,6 +14,8 @@ pub struct PlannedAction { pub json: String, pub kind: PlannedActionKind, pub reason: Option, + /// Host-rendered, bounded untrusted metadata; the action JSON remains required. + pub tool_descriptions: Option, } // Action JSON and reasons can contain credentials; diagnostics expose only the kind. diff --git a/codex-rs/guardian-context/src/composition.rs b/codex-rs/guardian-context/src/composition.rs index e88d43f495..fe1a5d437b 100644 --- a/codex-rs/guardian-context/src/composition.rs +++ b/codex-rs/guardian-context/src/composition.rs @@ -227,7 +227,18 @@ impl CollectedContext { ) } ContextSection::PlannedAction(planned) => { - (11, "planned_action", text_content(planned.render(action))) + let mut items = planned + .render(action) + .into_iter() + .map(|text| Budgeted::required(ContentItem::InputText { text })) + .collect::>(); + if let Some(text) = planned.tool_descriptions { + items.push(Budgeted::optional( + ContentItem::InputText { text }, + BudgetPriority::ToolDescription, + )); + } + (11, "planned_action", SectionDelivery::UserContent(items)) } }; sections.push((position, SectionOutput { id, delivery })); diff --git a/codex-rs/guardian-context/src/enforcement.rs b/codex-rs/guardian-context/src/enforcement.rs index a996875b45..816a9dc10c 100644 --- a/codex-rs/guardian-context/src/enforcement.rs +++ b/codex-rs/guardian-context/src/enforcement.rs @@ -20,6 +20,7 @@ use crate::composition::SectionOutput; /// Eviction priority within a profile; older items at the same priority go first. #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] pub(crate) enum BudgetPriority { + ToolDescription, Commentary, Tool, Image, diff --git a/codex-rs/guardian-context/src/enforcement_tests.rs b/codex-rs/guardian-context/src/enforcement_tests.rs index 609ed392d8..366e805035 100644 --- a/codex-rs/guardian-context/src/enforcement_tests.rs +++ b/codex-rs/guardian-context/src/enforcement_tests.rs @@ -3,6 +3,7 @@ use super::*; use crate::budget::section_tokens; use crate::composition::user_message; +use codex_protocol::models::ResponseItem; use pretty_assertions::assert_eq; fn text(value: &str) -> ContentItem { @@ -11,6 +12,53 @@ fn text(value: &str) -> ContentItem { } } +#[test] +fn planned_action_budget_omits_descriptions_without_changing_arguments() { + let action = crate::PlannedAction { + json: r#"{"tool":"write_record","arguments":{"description":"required payload"}}"# + .to_owned(), + kind: crate::PlannedActionKind::Command, + reason: None, + tool_descriptions: Some("optional tool description ".repeat(/*n*/ 100)), + }; + let required = action.render(crate::ActionPresentation::SyncFull); + let context = crate::CollectedContext { + sections: vec![crate::ContextSection::PlannedAction(action)], + } + .compose( + crate::ContextPresentation::SyncFull { + session_id: "review", + }, + crate::RenderedTranscript { + items: Vec::new(), + omission_note: None, + truncations: Vec::new(), + }, + ) + .unwrap(); + let mut required_items = required.iter().map(|item| text(item)).collect::>(); + required_items.push(text("evidence omitted")); + let budget = crate::estimate_input_tokens(&user_message(required_items.clone())) + 100; + let selected = context + .enforce_budget( + RequestBudget { + max_input_tokens: budget, + existing_context_tokens: 0, + }, + "evidence omitted".to_owned(), + ) + .unwrap(); + // The sync preamble is also required and remains ahead of the action. + let messages = selected.into_messages(); + let ResponseItem::Message { content, .. } = &messages[0] else { + panic!("expected user evidence"); + }; + assert_eq!( + &content[content.len() - required_items.len()..], + required_items + ); +} + #[test] fn budget_reserves_existing_context_and_preserves_required_messages() { let trusted = crate::PreviousReviews::try_from_fragments(vec!["verified review".to_owned()]) diff --git a/codex-rs/guardian-context/src/registry_tests.rs b/codex-rs/guardian-context/src/registry_tests.rs index 7ecd845d84..804cecf906 100644 --- a/codex-rs/guardian-context/src/registry_tests.rs +++ b/codex-rs/guardian-context/src/registry_tests.rs @@ -207,6 +207,7 @@ fn reused_registry_preserves_section_identity_and_source_roles() { denied_globs: vec!["**/*.key".into()], }; let action = super::PlannedAction { + tool_descriptions: None, json: r#"{"tool":"read_file","path":"debug-secret.json"}"#.into(), kind: super::PlannedActionKind::Command, reason: Some("debug-secret reason".into()),