diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 454e4aa660..fa86d97d2e 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -1,6 +1,5 @@ use super::*; use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; -use crate::mcp_tool_call::MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX; use async_channel::bounded; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::NetworkPermissions; @@ -504,7 +503,7 @@ async fn delegated_mcp_guardian_abort_returns_synthetic_decline_answer() { call_id: "call-1".to_string(), turn_id: "child-turn-1".to_string(), questions: vec![RequestUserInputQuestion { - id: format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_call-1"), + id: "mcp_tool_call_approval_call-1".to_string(), header: "Approve app tool call?".to_string(), question: "Allow this app tool?".to_string(), is_other: false, @@ -520,7 +519,7 @@ async fn delegated_mcp_guardian_abort_returns_synthetic_decline_answer() { response, Some(RequestUserInputResponse { answers: HashMap::from([( - format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_call-1"), + "mcp_tool_call_approval_call-1".to_string(), RequestUserInputAnswer { answers: vec![MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()], }, diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 60cfc03c9f..b813d9ea2a 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -875,7 +875,7 @@ fn with_mcp_tool_call_thread_id_meta( } } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy)] struct McpToolApprovalPromptOptions { allow_session_remember: bool, allow_persistent_approval: bool, @@ -889,8 +889,8 @@ struct McpToolApprovalPrompt { tool_params_display: Option>, } -pub(crate) const MCP_TOOL_APPROVAL_ACCEPT: &str = "Allow"; -pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this session"; +const MCP_TOOL_APPROVAL_ACCEPT: &str = "Allow"; +const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this session"; const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Allow and don't ask me again"; const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; // Internal-only token used when guardian auto-reviews delegated MCP approvals on the @@ -898,12 +898,12 @@ const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; // real "Decline" answer, so this lets guardian denials round-trip distinctly from user cancel. // This is not a user-facing option. pub(crate) const MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC: &str = "__codex_mcp_decline__"; -pub(crate) const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; +const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; const MCP_TOOL_APPROVAL_KIND_KEY: &str = "codex_approval_kind"; const MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call"; -pub(crate) const MCP_TOOL_APPROVAL_PERSIST_KEY: &str = "persist"; -pub(crate) const MCP_TOOL_APPROVAL_PERSIST_SESSION: &str = "session"; -pub(crate) const MCP_TOOL_APPROVAL_PERSIST_ALWAYS: &str = "always"; +const MCP_TOOL_APPROVAL_PERSIST_KEY: &str = "persist"; +const MCP_TOOL_APPROVAL_PERSIST_SESSION: &str = "session"; +const MCP_TOOL_APPROVAL_PERSIST_ALWAYS: &str = "always"; const MCP_TOOL_APPROVAL_SOURCE_KEY: &str = "source"; const MCP_TOOL_APPROVAL_SOURCE_CONNECTOR: &str = "connector"; const MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: &str = "connector_id"; @@ -934,10 +934,6 @@ struct McpToolApprovalKey { tool_name: String, } -pub(crate) fn mcp_tool_approval_question_id(call_id: &str) -> String { - format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}") -} - pub(crate) fn is_mcp_tool_approval_question_id(question_id: &str) -> bool { question_id .strip_prefix(MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX) @@ -1363,7 +1359,7 @@ async fn maybe_request_mcp_tool_approval( persistent_approval_key.as_ref(), tool_call_mcp_elicitation_enabled, ); - let question_id = mcp_tool_approval_question_id(call_id); + let question_id = format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}"); let Some(prompt) = mcp_tool_approval_prompt( &approval_request, question_id.clone(), diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index bbfe891127..446edb4a5a 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -383,8 +383,8 @@ fn mcp_tool_approval_compat_response_uses_synthetic_decline_for_abort() { } #[test] -fn mcp_tool_approval_question_id_helpers_round_trip() { - let question_id = mcp_tool_approval_question_id("call-1"); +fn mcp_tool_approval_question_id_detection_round_trips() { + let question_id = format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_call-1"); assert_eq!(question_id, "mcp_tool_call_approval_call-1"); assert!(is_mcp_tool_approval_question_id(&question_id));