diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 1767ac344c..1d27abbfa4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3656,10 +3656,12 @@ dependencies = [ "codex-guardian-context", "codex-history", "codex-mcp", + "codex-models-manager", "codex-otel", "codex-prompts", "codex-protocol", "codex-protocol-test-support", + "codex-rmcp-client", "codex-shell-command", "codex-utils-output-truncation", "codex-utils-path-uri", @@ -3668,6 +3670,7 @@ dependencies = [ "opentelemetry_sdk", "pretty_assertions", "rand 0.9.3", + "rmcp", "serde", "serde_json", "test-case", diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index b1d8e2a9b4..3d85a45e01 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -11,13 +11,16 @@ use codex_exec_server::ExecutorCapabilityDiscoveryV2Snapshot; use codex_exec_server::MAX_SELECTED_CAPABILITY_ROOTS; use codex_exec_server::ResolvedSelectedCapabilityRoot; use codex_exec_server::supports_capability_discovery_v2; +use codex_guardian_reviewer::GuardianElicitationReview; +use codex_guardian_reviewer::guardian_elicitation_review_request; +use codex_guardian_reviewer::mcp_elicitation_decline_without_message; +use codex_guardian_reviewer::mcp_elicitation_response_from_guardian_decision; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ElicitationReviewRequest; use codex_mcp::ElicitationReviewer; use codex_mcp::ElicitationReviewerHandle; use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY; use codex_otel::auth_storage::AuthStorageOriginator; -use codex_prompts::ResolvedModelMessages; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::ApprovalsReviewer; @@ -26,42 +29,21 @@ use codex_protocol::mcp::is_node_repl_backed_server; use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY as MCP_ELICITATION_APPROVAL_KIND_KEY; use codex_protocol::mcp_approval_meta::APPROVAL_KIND_MCP_TOOL_CALL as MCP_ELICITATION_APPROVAL_KIND_MCP_TOOL_CALL; use codex_protocol::mcp_approval_meta::APPROVAL_KIND_TOOL_SUGGESTION as MCP_ELICITATION_APPROVAL_KIND_TOOL_SUGGESTION; -use codex_protocol::mcp_approval_meta::APPROVALS_REVIEWER_KEY as MCP_ELICITATION_APPROVALS_REVIEWER_KEY; -use codex_protocol::mcp_approval_meta::CONNECTOR_DESCRIPTION_KEY as MCP_ELICITATION_CONNECTOR_DESCRIPTION_KEY; use codex_protocol::mcp_approval_meta::CONNECTOR_ID_KEY as MCP_ELICITATION_CONNECTOR_ID_KEY; -use codex_protocol::mcp_approval_meta::CONNECTOR_NAME_KEY as MCP_ELICITATION_CONNECTOR_NAME_KEY; use codex_protocol::mcp_approval_meta::PERSIST_KEY as MCP_ELICITATION_PERSIST_KEY; -use codex_protocol::mcp_approval_meta::REQUEST_TYPE_APPROVAL_REQUEST as MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST; -use codex_protocol::mcp_approval_meta::REQUEST_TYPE_KEY as MCP_ELICITATION_REQUEST_TYPE_KEY; use codex_protocol::mcp_approval_meta::SENSITIVE_ACTION_KEY as MCP_ELICITATION_SENSITIVE_ACTION_KEY; use codex_protocol::mcp_approval_meta::STRICT_AUTO_REVIEW_KEY as MCP_ELICITATION_STRICT_AUTO_REVIEW_KEY; -use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_ELICITATION_TOOL_DESCRIPTION_KEY; use codex_protocol::mcp_approval_meta::TOOL_NAME_KEY as MCP_ELICITATION_TOOL_NAME_KEY; -use codex_protocol::mcp_approval_meta::TOOL_PARAMS_KEY as MCP_ELICITATION_TOOL_PARAMS_KEY; -use codex_protocol::mcp_approval_meta::TOOL_TITLE_KEY as MCP_ELICITATION_TOOL_TITLE_KEY; use codex_protocol::openai_models::GuardianScope; -use codex_protocol::openai_models::ModelInfo; use codex_rmcp_client::Elicitation; use rmcp::model::ElicitationAction; -use rmcp::model::RequestMetaObject; use serde_json::Map; -const MCP_ELICITATION_DECLINE_MESSAGE_KEY: &str = "message"; const TOOL_SUGGESTION_ACTION_INSTALL: &str = "install"; const TOOL_SUGGESTION_ACTION_KEY: &str = "suggest_type"; const TOOL_SUGGESTION_TOOL_ID_KEY: &str = "tool_id"; const TOOL_SUGGESTION_TOOL_TYPE_KEY: &str = "tool_type"; -#[derive(Debug, PartialEq)] -enum GuardianElicitationReview { - NotRequested, - Decline(&'static str), - ApprovalRequest { - request: Box, - tool_call_id: Option, - }, -} - struct GuardianMcpElicitationReviewer { session: std::sync::Weak, } @@ -1223,7 +1205,7 @@ async fn review_guardian_mcp_elicitation( Ok(decision.map(|decision| { if let Some(reason) = declined_reason { warn!(server_name = %request.server_name, - request_id = %mcp_elicitation_request_id(&request.request_id), reason, + request_id = %request.request_id, reason, "declining Guardian MCP elicitation before review"); return mcp_elicitation_decline_without_message(); } @@ -1231,115 +1213,12 @@ async fn review_guardian_mcp_elicitation( })) } -fn guardian_elicitation_review_request( - request: &ElicitationReviewRequest, - originating_call_id: Option<&str>, -) -> GuardianElicitationReview { - let (meta, requested_schema) = match &request.elicitation { - Elicitation::Mcp(rmcp::model::ElicitRequestParams::FormElicitationParams { - meta, - requested_schema, - .. - }) => (meta, Some(requested_schema)), - Elicitation::Mcp(rmcp::model::ElicitRequestParams::UrlElicitationParams { - meta, .. - }) => { - return if meta_requests_approval_request(meta) { - GuardianElicitationReview::Decline( - "guardian MCP elicitation review only supports form elicitations", - ) - } else { - GuardianElicitationReview::NotRequested - }; - } - Elicitation::Mcp(_) => { - return GuardianElicitationReview::Decline( - "guardian MCP elicitation review does not support this elicitation mode", - ); - } - Elicitation::OpenAiForm { .. } - | Elicitation::OpenAiElicitationForm { .. } - | Elicitation::UserVerification { .. } => { - return GuardianElicitationReview::NotRequested; - } - }; - - let Some(meta) = meta.as_ref().map(|meta| &meta.0.0) else { - return GuardianElicitationReview::NotRequested; - }; - if metadata_str(meta, MCP_ELICITATION_REQUEST_TYPE_KEY) - != Some(MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST) - { - return GuardianElicitationReview::NotRequested; - } - if metadata_str(meta, MCP_ELICITATION_APPROVAL_KIND_KEY) - != Some(MCP_ELICITATION_APPROVAL_KIND_MCP_TOOL_CALL) - { - return GuardianElicitationReview::Decline( - "guardian MCP elicitation metadata must declare mcp_tool_call approval kind", - ); - } - if requested_schema.is_some_and(|schema| !schema.properties.is_empty()) { - return GuardianElicitationReview::Decline( - "guardian MCP elicitation review only supports empty form schemas", - ); - } - - let Some(tool_name) = metadata_owned_string(meta, MCP_ELICITATION_TOOL_NAME_KEY) else { - return GuardianElicitationReview::Decline( - "guardian MCP elicitation metadata must include a non-empty tool_name", - ); - }; - let arguments = match meta.get(MCP_ELICITATION_TOOL_PARAMS_KEY) { - Some(value @ Value::Object(_)) => Some(value.clone()), - Some(_) => { - return GuardianElicitationReview::Decline( - "guardian MCP elicitation tool_params must be an object", - ); - } - None => Some(Value::Object(Map::new())), - }; - - GuardianElicitationReview::ApprovalRequest { - // Synthetic display IDs must not participate in tool observation ordering. - tool_call_id: originating_call_id.map(str::to_owned), - request: Box::new(crate::guardian::GuardianApprovalRequest::McpToolCall { - id: originating_call_id.map(str::to_owned).unwrap_or_else(|| { - format!( - "mcp_elicitation:{}:{}", - request.server_name, - mcp_elicitation_request_id(&request.request_id) - ) - }), - server: request.server_name.clone(), - tool_name, - arguments, - connector_id: metadata_owned_string(meta, MCP_ELICITATION_CONNECTOR_ID_KEY), - connector_name: metadata_owned_string(meta, MCP_ELICITATION_CONNECTOR_NAME_KEY), - connector_description: metadata_owned_string( - meta, - MCP_ELICITATION_CONNECTOR_DESCRIPTION_KEY, - ), - connected_account_email: None, - tool_title: metadata_owned_string(meta, MCP_ELICITATION_TOOL_TITLE_KEY), - tool_description: metadata_owned_string(meta, MCP_ELICITATION_TOOL_DESCRIPTION_KEY), - annotations: None, - }), - } -} - fn elicitation_connector_id(elicitation: &Elicitation) -> Option<&str> { elicitation .meta() .and_then(|meta| metadata_str(meta, MCP_ELICITATION_CONNECTOR_ID_KEY)) } -fn meta_requests_approval_request(meta: &Option) -> bool { - meta.as_ref() - .and_then(|meta| metadata_str(&meta.0, MCP_ELICITATION_REQUEST_TYPE_KEY)) - == Some(MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST) -} - fn metadata_str<'a>(meta: &'a Map, key: &str) -> Option<&'a str> { meta.get(key).and_then(Value::as_str) } @@ -1378,67 +1257,6 @@ fn plugin_install_elicitation_telemetry_metadata( }) } -fn mcp_elicitation_request_id(id: &RequestId) -> String { - match id { - rmcp::model::NumberOrString::String(value) => value.to_string(), - rmcp::model::NumberOrString::Number(value) => value.to_string(), - } -} - -fn mcp_elicitation_response_from_guardian_decision( - decision: ReviewDecision, - model_info: &ModelInfo, -) -> ElicitationResponse { - match decision { - ReviewDecision::Approved - | ReviewDecision::ApprovedForSession - | ReviewDecision::ApprovedMcpPolicyAmendment - | ReviewDecision::ApprovedExecpolicyAmendment { .. } - | ReviewDecision::NetworkPolicyAmendment { .. } => ElicitationResponse { - action: ElicitationAction::Accept, - content: Some(serde_json::json!({})), - meta: Some(mcp_elicitation_auto_meta()), - }, - ReviewDecision::Denied { rejection } => mcp_elicitation_decline_with_message(rejection), - ReviewDecision::TimedOut => mcp_elicitation_decline_with_message( - ResolvedModelMessages::from_model(model_info) - .auto_review() - .timeout_instructions - .to_string(), - ), - ReviewDecision::Abort => ElicitationResponse { - action: ElicitationAction::Cancel, - content: None, - meta: Some(mcp_elicitation_auto_meta()), - }, - } -} - -fn mcp_elicitation_decline_with_message(message: String) -> ElicitationResponse { - ElicitationResponse { - action: ElicitationAction::Decline, - content: None, - meta: Some(serde_json::json!({ - MCP_ELICITATION_DECLINE_MESSAGE_KEY: message, - MCP_ELICITATION_APPROVALS_REVIEWER_KEY: ApprovalsReviewer::AutoReview, - })), - } -} - -fn mcp_elicitation_decline_without_message() -> ElicitationResponse { - ElicitationResponse { - action: ElicitationAction::Decline, - content: None, - meta: Some(mcp_elicitation_auto_meta()), - } -} - -fn mcp_elicitation_auto_meta() -> serde_json::Value { - serde_json::json!({ - MCP_ELICITATION_APPROVALS_REVIEWER_KEY: ApprovalsReviewer::AutoReview, - }) -} - #[cfg(test)] #[path = "mcp_tests.rs"] mod tests; diff --git a/codex-rs/core/src/session/mcp_tests.rs b/codex-rs/core/src/session/mcp_tests.rs index 47791f4233..a6dd60fc13 100644 --- a/codex-rs/core/src/session/mcp_tests.rs +++ b/codex-rs/core/src/session/mcp_tests.rs @@ -1,96 +1,7 @@ use super::*; use pretty_assertions::assert_eq; -use rmcp::model::BooleanSchema; -use rmcp::model::ElicitationSchema; -use rmcp::model::PrimitiveSchemaDefinition; use serde_json::json; -fn meta(value: Value) -> Option { - let Value::Object(map) = value else { - panic!("metadata must be an object"); - }; - Some(RequestMetaObject::from(map)) -} - -fn guardian_meta(tool_params: Option) -> Option { - let mut value = json!({ - "codex_approval_kind": "mcp_tool_call", - "codex_request_type": "approval_request", - "connector_id": "browser-use", - "connector_name": "Browser Use", - "tool_name": "access_browser_origin", - "tool_title": "Access browser origin", - }); - if let Some(tool_params) = tool_params { - value["tool_params"] = tool_params; - } - meta(value) -} - -fn form_request(meta: Option) -> ElicitationReviewRequest { - ElicitationReviewRequest { - server_name: "browser-use".to_string(), - request_id: rmcp::model::NumberOrString::Number(7), - elicitation: Elicitation::Mcp(rmcp::model::ElicitRequestParams::FormElicitationParams { - meta, - message: "Allow origin?".to_string(), - requested_schema: ElicitationSchema::builder() - .build() - .expect("schema should build"), - }), - } -} - -#[test_case::test_case(None, "mcp_elicitation:browser-use:7"; "unlinked elicitation")] -#[test_case::test_case(Some("browser-call"), "browser-call"; "linked elicitation")] -fn guardian_elicitation_review_request_builds_mcp_tool_call( - originating_call_id: Option<&str>, - expected_request_id: &str, -) { - let request = form_request(guardian_meta(Some(json!({ - "origin": "https://example.com", - })))); - - assert_eq!( - guardian_elicitation_review_request(&request, originating_call_id), - GuardianElicitationReview::ApprovalRequest { - tool_call_id: originating_call_id.map(str::to_owned), - request: Box::new(crate::guardian::GuardianApprovalRequest::McpToolCall { - id: expected_request_id.to_owned(), - server: "browser-use".to_owned(), - tool_name: "access_browser_origin".to_owned(), - arguments: Some(json!({ "origin": "https://example.com" })), - connector_id: Some("browser-use".to_owned()), - connector_name: Some("Browser Use".to_owned()), - connector_description: None, - connected_account_email: None, - tool_title: Some("Access browser origin".to_owned()), - tool_description: None, - annotations: None, - }), - } - ); -} - -#[test] -fn guardian_elicitation_review_request_defaults_missing_tool_params() { - let request = form_request(guardian_meta(/*tool_params*/ None)); - - let GuardianElicitationReview::ApprovalRequest { - request: guardian_request, - .. - } = guardian_elicitation_review_request(&request, /*originating_call_id*/ None) - else { - panic!("expected Guardian MCP tool call request"); - }; - let crate::guardian::GuardianApprovalRequest::McpToolCall { arguments, .. } = *guardian_request - else { - panic!("expected Guardian MCP tool call request"); - }; - - assert_eq!(arguments, Some(json!({}))); -} - #[test] fn plugin_install_elicitation_telemetry_metadata_requires_install_tool_suggestion() { let event = EventMsg::ElicitationRequest(ElicitationRequestEvent { @@ -147,145 +58,3 @@ fn plugin_install_elicitation_telemetry_metadata_requires_install_tool_suggestio None ); } - -#[test] -fn guardian_elicitation_review_request_requires_opt_in() { - let request = form_request(meta(json!({ - "codex_approval_kind": "mcp_tool_call", - "tool_name": "access_browser_origin", - }))); - - assert_eq!( - guardian_elicitation_review_request(&request, /*originating_call_id*/ None), - GuardianElicitationReview::NotRequested - ); -} - -#[test] -fn guardian_elicitation_review_request_declines_unsupported_opt_in_shapes() { - let url_request = ElicitationReviewRequest { - server_name: "browser-use".to_string(), - request_id: rmcp::model::NumberOrString::Number(8), - elicitation: Elicitation::Mcp(rmcp::model::ElicitRequestParams::UrlElicitationParams { - meta: guardian_meta(Some(json!({}))), - message: "Open URL".to_string(), - url: "https://example.com".to_string(), - elicitation_id: "elicit-1".to_string(), - }), - }; - assert!(matches!( - guardian_elicitation_review_request(&url_request, /*originating_call_id*/ None), - GuardianElicitationReview::Decline(_) - )); - - let non_empty_schema_request = ElicitationReviewRequest { - server_name: "browser-use".to_string(), - request_id: rmcp::model::NumberOrString::Number(9), - elicitation: Elicitation::Mcp(rmcp::model::ElicitRequestParams::FormElicitationParams { - meta: guardian_meta(Some(json!({}))), - message: "Allow origin?".to_string(), - requested_schema: ElicitationSchema::builder() - .required_property( - "confirmed", - PrimitiveSchemaDefinition::Boolean(BooleanSchema::new()), - ) - .build() - .expect("schema should build"), - }), - }; - assert!(matches!( - guardian_elicitation_review_request( - &non_empty_schema_request, - /*originating_call_id*/ None - ), - GuardianElicitationReview::Decline(_) - )); - - let missing_tool_name_request = form_request(meta(json!({ - "codex_approval_kind": "mcp_tool_call", - "codex_request_type": "approval_request", - }))); - assert!(matches!( - guardian_elicitation_review_request( - &missing_tool_name_request, - /*originating_call_id*/ None - ), - GuardianElicitationReview::Decline(_) - )); -} - -#[test] -fn guardian_decisions_map_to_elicitation_responses_without_session_state() { - let model = codex_models_manager::model_info::model_info_from_slug("acting-model"); - assert_eq!( - mcp_elicitation_response_from_guardian_decision(ReviewDecision::Approved, &model), - ElicitationResponse { - action: ElicitationAction::Accept, - content: Some(json!({})), - meta: Some(json!({ - "approvals_reviewer": ApprovalsReviewer::AutoReview, - })), - } - ); - assert_eq!( - mcp_elicitation_response_from_guardian_decision( - ReviewDecision::denied("Denied by Guardian"), - &model, - ), - ElicitationResponse { - action: ElicitationAction::Decline, - content: None, - meta: Some(json!({ - "approvals_reviewer": ApprovalsReviewer::AutoReview, - "message": "Denied by Guardian", - })), - } - ); - assert_eq!( - mcp_elicitation_response_from_guardian_decision(ReviewDecision::TimedOut, &model), - ElicitationResponse { - action: ElicitationAction::Decline, - content: None, - meta: Some(json!({ - "approvals_reviewer": ApprovalsReviewer::AutoReview, - "message": codex_prompts::ResolvedModelMessages::from_model(&model).auto_review().timeout_instructions, - })), - } - ); - assert_eq!( - mcp_elicitation_response_from_guardian_decision(ReviewDecision::Abort, &model), - ElicitationResponse { - action: ElicitationAction::Cancel, - content: None, - meta: Some(json!({ - "approvals_reviewer": ApprovalsReviewer::AutoReview, - })), - } - ); -} - -#[test] -fn guardian_elicitation_timeout_uses_acting_model_instructions() { - let mut model = codex_models_manager::model_info::model_info_from_slug("acting-model"); - for timeout_instructions in ["Catalog timeout instructions.", ""] { - model.model_messages = Some( - serde_json::from_value(json!({ - "auto_review": { - "timeout_instructions": timeout_instructions, - }, - })) - .expect("model messages should deserialize"), - ); - assert_eq!( - mcp_elicitation_response_from_guardian_decision(ReviewDecision::TimedOut, &model), - ElicitationResponse { - action: ElicitationAction::Decline, - content: None, - meta: Some(json!({ - "approvals_reviewer": ApprovalsReviewer::AutoReview, - "message": timeout_instructions, - })), - } - ); - } -} diff --git a/codex-rs/ext/guardian-reviewer/Cargo.toml b/codex-rs/ext/guardian-reviewer/Cargo.toml index b7d6ff6479..26e73f0f6f 100644 --- a/codex-rs/ext/guardian-reviewer/Cargo.toml +++ b/codex-rs/ext/guardian-reviewer/Cargo.toml @@ -25,11 +25,13 @@ codex-mcp = { workspace = true } codex-otel = { workspace = true } codex-prompts = { workspace = true } codex-protocol = { workspace = true } +codex-rmcp-client = { workspace = true } codex-shell-command = { workspace = true } codex-utils-path-uri = { workspace = true } codex-utils-output-truncation = { workspace = true } codex-utils-string = { workspace = true } rand = { workspace = true } +rmcp = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true } @@ -37,6 +39,7 @@ tokio-util = { workspace = true, features = ["rt"] } tracing = { workspace = true } [dev-dependencies] +codex-models-manager = { workspace = true } codex-context-fragments = { workspace = true } codex-protocol-test-support = { workspace = true } opentelemetry = { workspace = true } diff --git a/codex-rs/ext/guardian-reviewer/src/elicitation.rs b/codex-rs/ext/guardian-reviewer/src/elicitation.rs new file mode 100644 index 0000000000..26399d564a --- /dev/null +++ b/codex-rs/ext/guardian-reviewer/src/elicitation.rs @@ -0,0 +1,210 @@ +//! Converts MCP approval elicitations to Guardian requests and review decisions to responses. +//! The host retains session lookup, provenance checks, and approval enforcement. + +use crate::GuardianApprovalRequest; +use codex_mcp::ElicitationReviewRequest; +use codex_prompts::ResolvedModelMessages; +use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY as MCP_ELICITATION_APPROVAL_KIND_KEY; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_MCP_TOOL_CALL as MCP_ELICITATION_APPROVAL_KIND_MCP_TOOL_CALL; +use codex_protocol::mcp_approval_meta::APPROVALS_REVIEWER_KEY as MCP_ELICITATION_APPROVALS_REVIEWER_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_DESCRIPTION_KEY as MCP_ELICITATION_CONNECTOR_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_ID_KEY as MCP_ELICITATION_CONNECTOR_ID_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_NAME_KEY as MCP_ELICITATION_CONNECTOR_NAME_KEY; +use codex_protocol::mcp_approval_meta::REQUEST_TYPE_APPROVAL_REQUEST as MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST; +use codex_protocol::mcp_approval_meta::REQUEST_TYPE_KEY as MCP_ELICITATION_REQUEST_TYPE_KEY; +use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_ELICITATION_TOOL_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::TOOL_NAME_KEY as MCP_ELICITATION_TOOL_NAME_KEY; +use codex_protocol::mcp_approval_meta::TOOL_PARAMS_KEY as MCP_ELICITATION_TOOL_PARAMS_KEY; +use codex_protocol::mcp_approval_meta::TOOL_TITLE_KEY as MCP_ELICITATION_TOOL_TITLE_KEY; +use codex_protocol::openai_models::ModelInfo; +use codex_protocol::protocol::ReviewDecision; +use codex_rmcp_client::Elicitation; +use codex_rmcp_client::ElicitationResponse; +use rmcp::model::ElicitationAction; +use rmcp::model::RequestMetaObject; +use serde_json::Map; +use serde_json::Value; + +const MCP_ELICITATION_DECLINE_MESSAGE_KEY: &str = "message"; + +#[derive(Debug, PartialEq)] +pub enum GuardianElicitationReview { + NotRequested, + Decline(&'static str), + ApprovalRequest { + request: Box, + tool_call_id: Option, + }, +} + +pub fn guardian_elicitation_review_request( + request: &ElicitationReviewRequest, + originating_call_id: Option<&str>, +) -> GuardianElicitationReview { + let (meta, requested_schema) = match &request.elicitation { + Elicitation::Mcp(rmcp::model::ElicitRequestParams::FormElicitationParams { + meta, + requested_schema, + .. + }) => (meta, Some(requested_schema)), + Elicitation::Mcp(rmcp::model::ElicitRequestParams::UrlElicitationParams { + meta, .. + }) => { + return if meta_requests_approval_request(meta) { + GuardianElicitationReview::Decline( + "guardian MCP elicitation review only supports form elicitations", + ) + } else { + GuardianElicitationReview::NotRequested + }; + } + Elicitation::Mcp(_) => { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation review does not support this elicitation mode", + ); + } + Elicitation::OpenAiForm { .. } + | Elicitation::OpenAiElicitationForm { .. } + | Elicitation::UserVerification { .. } => { + return GuardianElicitationReview::NotRequested; + } + }; + + let Some(meta) = meta.as_ref().map(|meta| &meta.0.0) else { + return GuardianElicitationReview::NotRequested; + }; + if metadata_str(meta, MCP_ELICITATION_REQUEST_TYPE_KEY) + != Some(MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST) + { + return GuardianElicitationReview::NotRequested; + } + if metadata_str(meta, MCP_ELICITATION_APPROVAL_KIND_KEY) + != Some(MCP_ELICITATION_APPROVAL_KIND_MCP_TOOL_CALL) + { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation metadata must declare mcp_tool_call approval kind", + ); + } + if requested_schema.is_some_and(|schema| !schema.properties.is_empty()) { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation review only supports empty form schemas", + ); + } + + let Some(tool_name) = metadata_owned_string(meta, MCP_ELICITATION_TOOL_NAME_KEY) else { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation metadata must include a non-empty tool_name", + ); + }; + let arguments = match meta.get(MCP_ELICITATION_TOOL_PARAMS_KEY) { + Some(value @ Value::Object(_)) => Some(value.clone()), + Some(_) => { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation tool_params must be an object", + ); + } + None => Some(Value::Object(Map::new())), + }; + + GuardianElicitationReview::ApprovalRequest { + // Synthetic display IDs must not participate in tool observation ordering. + tool_call_id: originating_call_id.map(str::to_owned), + request: Box::new(GuardianApprovalRequest::McpToolCall { + id: originating_call_id.map(str::to_owned).unwrap_or_else(|| { + format!( + "mcp_elicitation:{}:{}", + request.server_name, request.request_id + ) + }), + server: request.server_name.clone(), + tool_name, + arguments, + connector_id: metadata_owned_string(meta, MCP_ELICITATION_CONNECTOR_ID_KEY), + connector_name: metadata_owned_string(meta, MCP_ELICITATION_CONNECTOR_NAME_KEY), + connector_description: metadata_owned_string( + meta, + MCP_ELICITATION_CONNECTOR_DESCRIPTION_KEY, + ), + connected_account_email: None, + tool_title: metadata_owned_string(meta, MCP_ELICITATION_TOOL_TITLE_KEY), + tool_description: metadata_owned_string(meta, MCP_ELICITATION_TOOL_DESCRIPTION_KEY), + annotations: None, + }), + } +} + +fn meta_requests_approval_request(meta: &Option) -> bool { + meta.as_ref() + .and_then(|meta| metadata_str(&meta.0, MCP_ELICITATION_REQUEST_TYPE_KEY)) + == Some(MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST) +} + +fn metadata_str<'a>(meta: &'a Map, key: &str) -> Option<&'a str> { + meta.get(key).and_then(Value::as_str) +} + +fn metadata_owned_string(meta: &Map, key: &str) -> Option { + metadata_str(meta, key) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(ToOwned::to_owned) +} + +pub fn mcp_elicitation_response_from_guardian_decision( + decision: ReviewDecision, + model_info: &ModelInfo, +) -> ElicitationResponse { + match decision { + ReviewDecision::Approved + | ReviewDecision::ApprovedForSession + | ReviewDecision::ApprovedMcpPolicyAmendment + | ReviewDecision::ApprovedExecpolicyAmendment { .. } + | ReviewDecision::NetworkPolicyAmendment { .. } => ElicitationResponse { + action: ElicitationAction::Accept, + content: Some(serde_json::json!({})), + meta: Some(mcp_elicitation_auto_meta()), + }, + ReviewDecision::Denied { rejection } => mcp_elicitation_decline_with_message(rejection), + ReviewDecision::TimedOut => mcp_elicitation_decline_with_message( + ResolvedModelMessages::from_model(model_info) + .auto_review() + .timeout_instructions + .to_string(), + ), + ReviewDecision::Abort => ElicitationResponse { + action: ElicitationAction::Cancel, + content: None, + meta: Some(mcp_elicitation_auto_meta()), + }, + } +} + +fn mcp_elicitation_decline_with_message(message: String) -> ElicitationResponse { + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(serde_json::json!({ + MCP_ELICITATION_DECLINE_MESSAGE_KEY: message, + MCP_ELICITATION_APPROVALS_REVIEWER_KEY: ApprovalsReviewer::AutoReview, + })), + } +} + +pub fn mcp_elicitation_decline_without_message() -> ElicitationResponse { + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(mcp_elicitation_auto_meta()), + } +} + +fn mcp_elicitation_auto_meta() -> serde_json::Value { + serde_json::json!({ + MCP_ELICITATION_APPROVALS_REVIEWER_KEY: ApprovalsReviewer::AutoReview, + }) +} + +#[cfg(test)] +#[path = "elicitation_tests.rs"] +mod tests; diff --git a/codex-rs/ext/guardian-reviewer/src/elicitation_tests.rs b/codex-rs/ext/guardian-reviewer/src/elicitation_tests.rs new file mode 100644 index 0000000000..a1f72f53be --- /dev/null +++ b/codex-rs/ext/guardian-reviewer/src/elicitation_tests.rs @@ -0,0 +1,235 @@ +#![cfg(test)] + +use super::*; +use pretty_assertions::assert_eq; +use rmcp::model::BooleanSchema; +use rmcp::model::ElicitationSchema; +use rmcp::model::PrimitiveSchemaDefinition; +use serde_json::json; + +fn meta(value: Value) -> Option { + let Value::Object(map) = value else { + panic!("metadata must be an object"); + }; + Some(RequestMetaObject::from(map)) +} + +fn guardian_meta(tool_params: Option) -> Option { + let mut value = json!({ + "codex_approval_kind": "mcp_tool_call", + "codex_request_type": "approval_request", + "connector_id": "browser-use", + "connector_name": "Browser Use", + "tool_name": "access_browser_origin", + "tool_title": "Access browser origin", + }); + if let Some(tool_params) = tool_params { + value["tool_params"] = tool_params; + } + meta(value) +} + +fn form_request(meta: Option) -> ElicitationReviewRequest { + ElicitationReviewRequest { + server_name: "browser-use".to_string(), + request_id: rmcp::model::NumberOrString::Number(7), + elicitation: Elicitation::Mcp(rmcp::model::ElicitRequestParams::FormElicitationParams { + meta, + message: "Allow origin?".to_string(), + requested_schema: ElicitationSchema::builder() + .build() + .expect("schema should build"), + }), + } +} + +#[test_case::test_case(None, "mcp_elicitation:browser-use:7"; "unlinked elicitation")] +#[test_case::test_case(Some("browser-call"), "browser-call"; "linked elicitation")] +fn guardian_elicitation_review_request_builds_mcp_tool_call( + originating_call_id: Option<&str>, + expected_request_id: &str, +) { + let request = form_request(guardian_meta(Some(json!({ + "origin": "https://example.com", + })))); + + assert_eq!( + guardian_elicitation_review_request(&request, originating_call_id), + GuardianElicitationReview::ApprovalRequest { + tool_call_id: originating_call_id.map(str::to_owned), + request: Box::new(GuardianApprovalRequest::McpToolCall { + id: expected_request_id.to_owned(), + server: "browser-use".to_owned(), + tool_name: "access_browser_origin".to_owned(), + arguments: Some(json!({ "origin": "https://example.com" })), + connector_id: Some("browser-use".to_owned()), + connector_name: Some("Browser Use".to_owned()), + connector_description: None, + connected_account_email: None, + tool_title: Some("Access browser origin".to_owned()), + tool_description: None, + annotations: None, + }), + } + ); +} + +#[test] +fn guardian_elicitation_review_request_defaults_missing_tool_params() { + let request = form_request(guardian_meta(/*tool_params*/ None)); + + let GuardianElicitationReview::ApprovalRequest { + request: guardian_request, + .. + } = guardian_elicitation_review_request(&request, /*originating_call_id*/ None) + else { + panic!("expected Guardian MCP tool call request"); + }; + let GuardianApprovalRequest::McpToolCall { arguments, .. } = *guardian_request else { + panic!("expected Guardian MCP tool call request"); + }; + + assert_eq!(arguments, Some(json!({}))); +} + +#[test] +fn guardian_elicitation_review_request_requires_opt_in() { + let request = form_request(meta(json!({ + "codex_approval_kind": "mcp_tool_call", + "tool_name": "access_browser_origin", + }))); + + assert_eq!( + guardian_elicitation_review_request(&request, /*originating_call_id*/ None), + GuardianElicitationReview::NotRequested + ); +} + +#[test] +fn guardian_elicitation_review_request_declines_unsupported_opt_in_shapes() { + let url_request = ElicitationReviewRequest { + server_name: "browser-use".to_string(), + request_id: rmcp::model::NumberOrString::Number(8), + elicitation: Elicitation::Mcp(rmcp::model::ElicitRequestParams::UrlElicitationParams { + meta: guardian_meta(Some(json!({}))), + message: "Open URL".to_string(), + url: "https://example.com".to_string(), + elicitation_id: "elicit-1".to_string(), + }), + }; + assert!(matches!( + guardian_elicitation_review_request(&url_request, /*originating_call_id*/ None), + GuardianElicitationReview::Decline(_) + )); + + let non_empty_schema_request = ElicitationReviewRequest { + server_name: "browser-use".to_string(), + request_id: rmcp::model::NumberOrString::Number(9), + elicitation: Elicitation::Mcp(rmcp::model::ElicitRequestParams::FormElicitationParams { + meta: guardian_meta(Some(json!({}))), + message: "Allow origin?".to_string(), + requested_schema: ElicitationSchema::builder() + .required_property( + "confirmed", + PrimitiveSchemaDefinition::Boolean(BooleanSchema::new()), + ) + .build() + .expect("schema should build"), + }), + }; + assert!(matches!( + guardian_elicitation_review_request( + &non_empty_schema_request, + /*originating_call_id*/ None + ), + GuardianElicitationReview::Decline(_) + )); + + let missing_tool_name_request = form_request(meta(json!({ + "codex_approval_kind": "mcp_tool_call", + "codex_request_type": "approval_request", + }))); + assert!(matches!( + guardian_elicitation_review_request( + &missing_tool_name_request, + /*originating_call_id*/ None + ), + GuardianElicitationReview::Decline(_) + )); +} + +#[test] +fn guardian_decisions_map_to_elicitation_responses_without_session_state() { + let model = codex_models_manager::model_info::model_info_from_slug("acting-model"); + assert_eq!( + mcp_elicitation_response_from_guardian_decision(ReviewDecision::Approved, &model), + ElicitationResponse { + action: ElicitationAction::Accept, + content: Some(json!({})), + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + })), + } + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision( + ReviewDecision::denied("Denied by Guardian"), + &model, + ), + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + "message": "Denied by Guardian", + })), + } + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision(ReviewDecision::TimedOut, &model), + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + "message": codex_prompts::ResolvedModelMessages::from_model(&model).auto_review().timeout_instructions, + })), + } + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision(ReviewDecision::Abort, &model), + ElicitationResponse { + action: ElicitationAction::Cancel, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + })), + } + ); +} + +#[test] +fn guardian_elicitation_timeout_uses_acting_model_instructions() { + let mut model = codex_models_manager::model_info::model_info_from_slug("acting-model"); + for timeout_instructions in ["Catalog timeout instructions.", ""] { + model.model_messages = Some( + serde_json::from_value(json!({ + "auto_review": { + "timeout_instructions": timeout_instructions, + }, + })) + .expect("model messages should deserialize"), + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision(ReviewDecision::TimedOut, &model), + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + "message": timeout_instructions, + })), + } + ); + } +} diff --git a/codex-rs/ext/guardian-reviewer/src/lib.rs b/codex-rs/ext/guardian-reviewer/src/lib.rs index 2e272c0dc5..d14917127a 100644 --- a/codex-rs/ext/guardian-reviewer/src/lib.rs +++ b/codex-rs/ext/guardian-reviewer/src/lib.rs @@ -22,7 +22,12 @@ pub use instructions::GUARDIAN_CONVERSATION_HISTORY_PROMPT; pub use instructions::GUARDIAN_FOLLOWUP_REVIEW_REMINDER; pub use instructions::reviewer_instructions; mod deadline; +mod elicitation; mod execution; +pub use elicitation::GuardianElicitationReview; +pub use elicitation::guardian_elicitation_review_request; +pub use elicitation::mcp_elicitation_decline_without_message; +pub use elicitation::mcp_elicitation_response_from_guardian_decision; mod feedback; mod metrics; mod model;