mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Keep MCP user interaction on the root thread (#46066)
## Why MCP requests that need human input, including browser sign-in, must be handled by the root thread. Subagents need guidance to hand these blockers to their parent without prompting the user or automatically accepting requests that require input. ## What changed - Reject interactive MCP elicitations and tool approval prompts in subagents with guidance to ask the parent and wait before retrying. - Recognize browser authentication and `codex_requires_user_input` metadata even when the form schema is empty, while preserving automatic permission approvals and review decisions. - Carry user-interaction eligibility through MCP runtime creation and connection reuse, and guard prompt registration. - Preserve connector authentication diagnostics alongside handoff guidance, subject to normal tool-output limits. ## Testing Add unit and integration coverage for blocked subagent prompts, root browser authentication, automatic approval and review, connection reuse, and authentication diagnostic preservation and truncation. Add a request-history snapshot for browser-auth handoff guidance. GitOrigin-RevId: 83e146b6e5c1a2a22a34681ad45615f504b3c96c
This commit is contained in:
@@ -113,6 +113,7 @@ impl AppsRequestProcessor {
|
||||
client_mcp_extensions: ClientMcpExtensions::default(),
|
||||
auth: auth.clone(),
|
||||
auth_manager: codex_apps_auth_manager,
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
})
|
||||
|
||||
@@ -222,6 +222,7 @@ impl McpConnectionSet {
|
||||
client_mcp_extensions,
|
||||
auth,
|
||||
auth_manager,
|
||||
allow_user_interaction,
|
||||
elicitation_reviewer,
|
||||
elicitation_lifecycle,
|
||||
} = input;
|
||||
@@ -258,6 +259,7 @@ impl McpConnectionSet {
|
||||
!previous.servers.is_empty()
|
||||
&& previous.elicitation_requests.update(
|
||||
Arc::clone(&config),
|
||||
allow_user_interaction,
|
||||
elicitation_reviewer.clone(),
|
||||
elicitation_lifecycle.clone(),
|
||||
)
|
||||
@@ -267,6 +269,7 @@ impl McpConnectionSet {
|
||||
} else {
|
||||
ElicitationRequestManager::new(
|
||||
Arc::clone(&config),
|
||||
allow_user_interaction,
|
||||
elicitation_reviewer,
|
||||
elicitation_lifecycle,
|
||||
elicitation_router,
|
||||
|
||||
@@ -117,6 +117,7 @@ impl McpConnectionSet {
|
||||
approval_policy.value(),
|
||||
permission_profile.get().clone(),
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -912,6 +913,7 @@ fn elicitation_granular_policy_respects_never_and_config() {
|
||||
async fn disabled_permissions_auto_accept_elicitation_with_empty_form_schema() {
|
||||
let manager = ElicitationRequestManager::new(
|
||||
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -950,6 +952,7 @@ async fn disabled_permissions_auto_accept_elicitation_with_empty_form_schema() {
|
||||
async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fields() {
|
||||
let manager = ElicitationRequestManager::new(
|
||||
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -1045,6 +1048,7 @@ async fn assert_elicitation_declined_with_reviewer_calls(
|
||||
let reviewer = Arc::new(DecliningElicitationReviewer::default());
|
||||
let manager = ElicitationRequestManager::new(
|
||||
test_elicitation_config(server_name, approval_policy, PermissionProfile::Disabled),
|
||||
/*allow_user_interaction*/ true,
|
||||
Some(reviewer.clone()),
|
||||
/*lifecycle*/ None,
|
||||
full_access_form_input_enabled_router(),
|
||||
@@ -1089,6 +1093,7 @@ async fn assert_requested_user_input_is_declined(
|
||||
) {
|
||||
let manager = ElicitationRequestManager::new(
|
||||
test_elicitation_config("server", approval_policy, permission_profile),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
router,
|
||||
@@ -1180,6 +1185,7 @@ async fn assert_disabled_permissions_surface_requested_user_input(
|
||||
let reviewer = Arc::new(DecliningElicitationReviewer::default());
|
||||
let manager = ElicitationRequestManager::new(
|
||||
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
|
||||
/*allow_user_interaction*/ true,
|
||||
Some(reviewer.clone()),
|
||||
/*lifecycle*/ None,
|
||||
router.clone(),
|
||||
@@ -1330,6 +1336,7 @@ async fn on_request_approval_forms_remain_with_the_reviewer() {
|
||||
async fn disabled_permissions_decline_user_input_without_an_event_channel() {
|
||||
let manager = ElicitationRequestManager::new(
|
||||
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
full_access_form_input_enabled_router(),
|
||||
@@ -1369,6 +1376,7 @@ async fn concurrent_authority_updates_never_auto_approve_mixed_policy() {
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::default(),
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -1382,6 +1390,7 @@ async fn concurrent_authority_updates_never_auto_approve_mixed_policy() {
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
));
|
||||
@@ -1391,6 +1400,7 @@ async fn concurrent_authority_updates_never_auto_approve_mixed_policy() {
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::default()
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
));
|
||||
@@ -1452,6 +1462,7 @@ async fn shared_elicitation_router_targets_the_exact_pending_request() {
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::default(),
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
Some(lifecycle.clone()),
|
||||
router.clone(),
|
||||
@@ -1462,6 +1473,7 @@ async fn shared_elicitation_router_targets_the_exact_pending_request() {
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::default(),
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
Some(lifecycle),
|
||||
router.clone(),
|
||||
@@ -1566,6 +1578,7 @@ async fn cancelled_elicitation_is_removed_without_affecting_other_pending_reques
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::default(),
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
router.clone(),
|
||||
@@ -2045,6 +2058,7 @@ async fn read_only_apps_discovery_never_uses_a_shared_writable_catalog() -> anyh
|
||||
client_mcp_extensions: ClientMcpExtensions::default(),
|
||||
auth: None,
|
||||
auth_manager: None,
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
},
|
||||
@@ -2100,6 +2114,7 @@ async fn hosted_apps_protocol_mode_is_independent_of_generic_mode() -> anyhow::R
|
||||
/*previous*/ None,
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction: true,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(config),
|
||||
plugins_available: false,
|
||||
@@ -2206,6 +2221,7 @@ async fn codex_apps_extension_does_not_share_host_owned_tools_cache() -> anyhow:
|
||||
/*previous*/ None,
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction: true,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(config),
|
||||
plugins_available: false,
|
||||
@@ -4603,6 +4619,7 @@ async fn executor_owned_chatgpt_mcp_accepts_only_safe_explicit_authorization() -
|
||||
/*previous*/ None,
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction: true,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(runtime_config.clone()),
|
||||
plugins_available: false,
|
||||
@@ -4717,6 +4734,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() {
|
||||
/*previous*/ None,
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction: true,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(crate::mcp::tests::test_mcp_config(
|
||||
codex_home.path().to_path_buf(),
|
||||
@@ -5140,6 +5158,7 @@ async fn reconcile_reusable_server(
|
||||
config,
|
||||
runtime_context,
|
||||
crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf()),
|
||||
/*allow_user_interaction*/ true,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -5150,12 +5169,14 @@ async fn reconcile_reusable_server_with_mcp_config(
|
||||
config: McpServerConfig,
|
||||
runtime_context: McpRuntimeContext,
|
||||
mcp_config: crate::McpConfig,
|
||||
allow_user_interaction: bool,
|
||||
) -> McpConnectionSet {
|
||||
let (tx_event, _rx_event) = async_channel::unbounded();
|
||||
McpConnectionSet::new(
|
||||
Some(previous),
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(mcp_config),
|
||||
plugins_available: false,
|
||||
@@ -5204,6 +5225,7 @@ async fn read_only_policy_does_not_reuse_a_writable_connection() {
|
||||
config.clone(),
|
||||
runtime_context.clone(),
|
||||
mcp_config,
|
||||
/*allow_user_interaction*/ true,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
@@ -5433,6 +5455,7 @@ async fn refreshed_catalog_follows_reused_client_without_mutating_old_bindings()
|
||||
config,
|
||||
runtime_context,
|
||||
crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf()),
|
||||
/*allow_user_interaction*/ true,
|
||||
)
|
||||
.await,
|
||||
);
|
||||
@@ -5907,6 +5930,7 @@ async fn reconciliation_replaces_connection_when_protocol_mode_changes() {
|
||||
Some(&previous),
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction: true,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(mcp_config),
|
||||
plugins_available: false,
|
||||
@@ -5965,6 +5989,7 @@ async fn reconciliation_reuses_legacy_stdio_server_when_modern_protocol_is_enabl
|
||||
Some(&previous),
|
||||
McpPublicationGate::already_published(),
|
||||
McpRuntimeInput {
|
||||
allow_user_interaction: true,
|
||||
startup_policy: McpStartupPolicy::Eager,
|
||||
config: Arc::new(mcp_config),
|
||||
plugins_available: false,
|
||||
@@ -5999,42 +6024,96 @@ async fn reconciliation_reuses_legacy_stdio_server_when_modern_protocol_is_enabl
|
||||
async fn reconciliation_updates_elicitation_policy_without_restarting_ready_server() {
|
||||
let runtime_context = reusable_server_runtime_context();
|
||||
let config = reusable_server_config("http://127.0.0.1:1");
|
||||
let previous = manager_with_reusable_ready_server(
|
||||
let mut previous = manager_with_reusable_ready_server(
|
||||
&config,
|
||||
&runtime_context,
|
||||
vec![create_test_tool("docs", "search")],
|
||||
)
|
||||
.await;
|
||||
{
|
||||
let mut authority = previous
|
||||
.elicitation_requests
|
||||
.authority
|
||||
.lock()
|
||||
.expect("elicitation authority lock");
|
||||
let config = Arc::make_mut(
|
||||
&mut authority
|
||||
.as_mut()
|
||||
.expect("test manager should have permission authority")
|
||||
.config,
|
||||
let router = ElicitationRequestRouter::default();
|
||||
previous.elicitation_requests = ElicitationRequestManager::new(
|
||||
test_elicitation_config("docs", AskForApproval::Never, PermissionProfile::Disabled),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
router.clone(),
|
||||
);
|
||||
let (tx_event, events) = async_channel::unbounded();
|
||||
let sender = previous.elicitation_requests.make_sender(
|
||||
"docs".to_string(),
|
||||
Some(tx_event),
|
||||
&ClientMcpExtensions::default(),
|
||||
);
|
||||
let elicitation =
|
||||
codex_rmcp_client::Elicitation::Mcp(ElicitRequestParams::FormElicitationParams {
|
||||
meta: None,
|
||||
message: "What should I say?".to_string(),
|
||||
requested_schema: requested_user_input_schema(),
|
||||
});
|
||||
let response = ElicitationResponse {
|
||||
action: ElicitationAction::Accept,
|
||||
content: Some(serde_json::json!({"message": "continue"})),
|
||||
meta: None,
|
||||
};
|
||||
|
||||
for allow_user_interaction in [true, false, true] {
|
||||
let mcp_config = test_elicitation_config(
|
||||
"docs",
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::default(),
|
||||
);
|
||||
config.approval_policy = Constrained::allow_any(AskForApproval::Never);
|
||||
config.permission_profile = PermissionProfile::Disabled;
|
||||
let reconciled = reconcile_reusable_server_with_mcp_config(
|
||||
&previous,
|
||||
"docs",
|
||||
config.clone(),
|
||||
runtime_context.clone(),
|
||||
mcp_config.as_ref().clone(),
|
||||
allow_user_interaction,
|
||||
)
|
||||
.await;
|
||||
assert!(previous.shares_test_connection_with(&reconciled, "docs"));
|
||||
{
|
||||
let authority = reconciled
|
||||
.elicitation_requests
|
||||
.authority
|
||||
.lock()
|
||||
.expect("elicitation authority lock");
|
||||
let config = &authority.as_ref().expect("elicitation authority").config;
|
||||
assert_eq!(config.approval_policy.value(), AskForApproval::OnRequest);
|
||||
assert_eq!(config.permission_profile, PermissionProfile::default());
|
||||
}
|
||||
|
||||
// A sender captured before reconciliation must observe each ownership update.
|
||||
let mut pending = sender(NumberOrString::Number(7), elicitation.clone());
|
||||
if allow_user_interaction {
|
||||
assert!(futures::poll!(pending.as_mut()).is_pending());
|
||||
let EventMsg::ElicitationRequest(request) =
|
||||
events.try_recv().expect("user-input event").msg
|
||||
else {
|
||||
panic!("expected MCP elicitation");
|
||||
};
|
||||
let codex_protocol::mcp::RequestId::String(request_id) = request.id else {
|
||||
panic!("expected Codex-owned string request ID");
|
||||
};
|
||||
router
|
||||
.resolve(
|
||||
"docs".to_string(),
|
||||
NumberOrString::String(request_id.into()),
|
||||
response.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("user response should resolve the retained sender");
|
||||
assert_eq!(pending.await.expect("elicitation should resolve"), response);
|
||||
} else {
|
||||
let error = pending
|
||||
.now_or_never()
|
||||
.expect("blocked elicitation must not wait for user input")
|
||||
.expect_err("subagent user input must be rejected");
|
||||
assert_eq!(error.to_string(), crate::MCP_ELICITATION_HANDOFF_MESSAGE);
|
||||
}
|
||||
assert!(events.is_empty());
|
||||
previous = reconciled;
|
||||
}
|
||||
|
||||
let reconciled = reconcile_reusable_server(&previous, config, runtime_context).await;
|
||||
|
||||
assert!(previous.shares_test_connection_with(&reconciled, "docs"));
|
||||
let authority = reconciled
|
||||
.elicitation_requests
|
||||
.authority
|
||||
.lock()
|
||||
.expect("elicitation authority lock");
|
||||
let config = &authority
|
||||
.as_ref()
|
||||
.expect("reconciled manager should have permission authority")
|
||||
.config;
|
||||
assert_eq!(config.approval_policy.value(), AskForApproval::OnRequest);
|
||||
assert_eq!(config.permission_profile, PermissionProfile::default());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -6220,6 +6299,7 @@ async fn reconciliation_reconnects_when_host_plugin_root_changes() {
|
||||
server_config.clone(),
|
||||
runtime_context.clone(),
|
||||
config_for_root(original_root),
|
||||
/*allow_user_interaction*/ true,
|
||||
)
|
||||
.await;
|
||||
assert!(previous.shares_test_connection_with(&unchanged, "docs"));
|
||||
@@ -6231,6 +6311,7 @@ async fn reconciliation_reconnects_when_host_plugin_root_changes() {
|
||||
server_config,
|
||||
runtime_context,
|
||||
replacement_config,
|
||||
/*allow_user_interaction*/ true,
|
||||
)
|
||||
.await;
|
||||
assert!(!unchanged.shares_test_connection_with(&replacement, "docs"));
|
||||
|
||||
@@ -4,6 +4,8 @@
|
||||
//! from the user. It decides whether the request can be automatically accepted,
|
||||
//! must be declined by policy, or should be surfaced as a Codex protocol event
|
||||
//! and later resolved through the stored responder.
|
||||
//! Non-root agents may use automatic decisions, but explicit user-input requests
|
||||
//! are rejected before review and no pending user prompt is registered.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
@@ -24,9 +26,11 @@ use codex_protocol::approvals::ElicitationRequestEvent;
|
||||
use codex_protocol::mcp::ClientMcpExtensions;
|
||||
use codex_protocol::mcp::OPENAI_ELICITATION_EXTENSION_ID;
|
||||
use codex_protocol::mcp::RequestId as ProtocolRequestId;
|
||||
use codex_protocol::mcp_approval_meta::APPROVAL_KIND_BROWSER_AUTH;
|
||||
use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY;
|
||||
use codex_protocol::mcp_approval_meta::APPROVAL_KIND_TOOL_SUGGESTION;
|
||||
use codex_protocol::mcp_approval_meta::APPROVALS_REVIEWER_KEY;
|
||||
use codex_protocol::mcp_approval_meta::REQUIRES_USER_INPUT_KEY;
|
||||
use codex_protocol::mcp_approval_meta::STRICT_AUTO_REVIEW_KEY;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::Event;
|
||||
@@ -44,6 +48,12 @@ use tokio::sync::oneshot;
|
||||
static NEXT_ELICITATION_REQUEST_ID: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
const STRICT_AUTO_REVIEW_DECLINE_MESSAGE: &str = "Automated review of this operation failed. Do not proceed without asking the user for explicit approval.";
|
||||
/// Recovery guidance for a request that a non-root agent cannot present to the user.
|
||||
pub const MCP_ELICITATION_HANDOFF_MESSAGE: &str = concat!(
|
||||
"MCP server elicitations can only be requested by the root thread. ",
|
||||
"Ask the parent agent to handle this request. ",
|
||||
"Do not retry the blocked action until the parent confirms the blocker is resolved."
|
||||
);
|
||||
|
||||
#[path = "user_verification_elicitation.rs"]
|
||||
mod user_verification_elicitation;
|
||||
@@ -154,11 +164,78 @@ impl ElicitationRequestRouter {
|
||||
.send(response)
|
||||
.map_err(|_| anyhow!("elicitation response receiver closed"))
|
||||
}
|
||||
|
||||
async fn request_user_interaction(
|
||||
&self,
|
||||
events: Option<Sender<Event>>,
|
||||
authority: &ElicitationAuthority,
|
||||
server_name: String,
|
||||
request: ElicitationRequest,
|
||||
) -> Result<ElicitationResponse> {
|
||||
anyhow::ensure!(
|
||||
authority.allow_user_interaction,
|
||||
MCP_ELICITATION_HANDOFF_MESSAGE
|
||||
);
|
||||
let Some(events) = events else {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
});
|
||||
};
|
||||
let (delivery_context, response_context) =
|
||||
if matches!(&request, ElicitationRequest::UserVerification { .. }) {
|
||||
(
|
||||
"failed to deliver user-verification request",
|
||||
"user-verification response channel closed",
|
||||
)
|
||||
} else {
|
||||
(
|
||||
"failed to deliver MCP elicitation request",
|
||||
"elicitation request channel closed unexpectedly",
|
||||
)
|
||||
};
|
||||
let public_request_id = format!(
|
||||
"codex-mcp-elicitation-{}",
|
||||
NEXT_ELICITATION_REQUEST_ID.fetch_add(1, Ordering::Relaxed)
|
||||
);
|
||||
let request_key = (
|
||||
server_name.clone(),
|
||||
RequestId::String(public_request_id.clone().into()),
|
||||
);
|
||||
let (tx, rx) = oneshot::channel();
|
||||
let _active_elicitation = authority
|
||||
.lifecycle
|
||||
.as_ref()
|
||||
.map(ElicitationLifecycle::start);
|
||||
self.requests
|
||||
.lock()
|
||||
.map_err(|_| anyhow!("elicitation request router unavailable"))?
|
||||
.insert(request_key.clone(), tx);
|
||||
let _pending_request = PendingElicitationRequest {
|
||||
router: self.clone(),
|
||||
key: request_key,
|
||||
};
|
||||
events
|
||||
.send(Event {
|
||||
id: "mcp_elicitation_request".to_string(),
|
||||
msg: EventMsg::ElicitationRequest(ElicitationRequestEvent {
|
||||
turn_id: None,
|
||||
server_name,
|
||||
id: ProtocolRequestId::String(public_request_id),
|
||||
request,
|
||||
}),
|
||||
})
|
||||
.await
|
||||
.context(delivery_context)?;
|
||||
rx.await.context(response_context)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ElicitationAuthority {
|
||||
pub(crate) config: Arc<McpConfig>,
|
||||
pub(crate) allow_user_interaction: bool,
|
||||
reviewer: Option<ElicitationReviewerHandle>,
|
||||
lifecycle: Option<ElicitationLifecycle>,
|
||||
}
|
||||
@@ -172,6 +249,7 @@ pub(crate) struct ElicitationRequestManager {
|
||||
impl ElicitationRequestManager {
|
||||
pub(crate) fn new(
|
||||
config: Arc<McpConfig>,
|
||||
allow_user_interaction: bool,
|
||||
reviewer: Option<ElicitationReviewerHandle>,
|
||||
lifecycle: Option<ElicitationLifecycle>,
|
||||
router: ElicitationRequestRouter,
|
||||
@@ -180,6 +258,7 @@ impl ElicitationRequestManager {
|
||||
router,
|
||||
authority: Arc::new(StdMutex::new(Some(ElicitationAuthority {
|
||||
config,
|
||||
allow_user_interaction,
|
||||
reviewer,
|
||||
lifecycle,
|
||||
}))),
|
||||
@@ -189,6 +268,7 @@ impl ElicitationRequestManager {
|
||||
pub(crate) fn update(
|
||||
&self,
|
||||
config: Arc<McpConfig>,
|
||||
allow_user_interaction: bool,
|
||||
reviewer: Option<ElicitationReviewerHandle>,
|
||||
lifecycle: Option<ElicitationLifecycle>,
|
||||
) -> bool {
|
||||
@@ -197,6 +277,7 @@ impl ElicitationRequestManager {
|
||||
};
|
||||
*authority = Some(ElicitationAuthority {
|
||||
config,
|
||||
allow_user_interaction,
|
||||
reviewer,
|
||||
lifecycle,
|
||||
});
|
||||
@@ -223,6 +304,40 @@ impl ElicitationRequestManager {
|
||||
let server_name = server_name.clone();
|
||||
let authority = authority.clone();
|
||||
async move {
|
||||
let authority = authority
|
||||
.lock()
|
||||
.ok()
|
||||
.and_then(|authority| authority.clone());
|
||||
let user_interaction_disabled = authority
|
||||
.as_ref()
|
||||
.is_some_and(|authority| !authority.allow_user_interaction);
|
||||
if user_interaction_disabled {
|
||||
// Browser sign-in can use an empty schema. Explicit user-input
|
||||
// markers must take precedence over automatic approval or review.
|
||||
let requires_user_input = elicitation.meta().is_some_and(|meta| {
|
||||
meta.get(REQUIRES_USER_INPUT_KEY) == Some(&Value::Bool(true))
|
||||
|| meta.get(APPROVAL_KIND_KEY).and_then(Value::as_str)
|
||||
== Some(APPROVAL_KIND_BROWSER_AUTH)
|
||||
}) || match &elicitation {
|
||||
Elicitation::Mcp(
|
||||
rmcp::model::ElicitRequestParams::FormElicitationParams {
|
||||
requested_schema,
|
||||
..
|
||||
},
|
||||
) => !requested_schema.properties.is_empty(),
|
||||
Elicitation::OpenAiForm {
|
||||
requested_schema, ..
|
||||
}
|
||||
| Elicitation::OpenAiElicitationForm {
|
||||
requested_schema, ..
|
||||
} => requested_schema
|
||||
.get("properties")
|
||||
.and_then(Value::as_object)
|
||||
.is_some_and(|properties| !properties.is_empty()),
|
||||
Elicitation::Mcp(_) | Elicitation::UserVerification { .. } => true,
|
||||
};
|
||||
anyhow::ensure!(!requires_user_input, MCP_ELICITATION_HANDOFF_MESSAGE);
|
||||
}
|
||||
if let Elicitation::UserVerification {
|
||||
title,
|
||||
description,
|
||||
@@ -270,8 +385,7 @@ impl ElicitationRequestManager {
|
||||
});
|
||||
}
|
||||
|
||||
let Ok(Some(authority)) = authority.lock().map(|authority| authority.clone())
|
||||
else {
|
||||
let Some(authority) = authority else {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
@@ -282,7 +396,8 @@ impl ElicitationRequestManager {
|
||||
config,
|
||||
reviewer,
|
||||
lifecycle,
|
||||
} = authority;
|
||||
..
|
||||
} = &authority;
|
||||
let approval_policy = config.approval_policy.value();
|
||||
let Some(permission_profile) = config.permission_profile_for_server(&server_name)
|
||||
else {
|
||||
@@ -400,19 +515,6 @@ impl ElicitationRequestManager {
|
||||
}
|
||||
}
|
||||
|
||||
let Some(tx_event) = tx_event else {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
});
|
||||
};
|
||||
|
||||
let public_request_id = format!(
|
||||
"codex-mcp-elicitation-{}",
|
||||
NEXT_ELICITATION_REQUEST_ID.fetch_add(1, Ordering::Relaxed)
|
||||
);
|
||||
let routed_request_id = RequestId::String(public_request_id.clone().into());
|
||||
let request = match elicitation {
|
||||
Elicitation::UserVerification { .. } => {
|
||||
return Ok(ElicitationResponse {
|
||||
@@ -474,32 +576,9 @@ impl ElicitationRequestManager {
|
||||
requested_schema,
|
||||
},
|
||||
};
|
||||
let (tx, rx) = oneshot::channel();
|
||||
let _active_elicitation = lifecycle.as_ref().map(ElicitationLifecycle::start);
|
||||
let request_key = (server_name.clone(), routed_request_id);
|
||||
router
|
||||
.requests
|
||||
.lock()
|
||||
.map_err(|_| anyhow!("elicitation request router unavailable"))?
|
||||
.insert(request_key.clone(), tx);
|
||||
let _pending_request = PendingElicitationRequest {
|
||||
router: router.clone(),
|
||||
key: request_key,
|
||||
};
|
||||
tx_event
|
||||
.send(Event {
|
||||
id: "mcp_elicitation_request".to_string(),
|
||||
msg: EventMsg::ElicitationRequest(ElicitationRequestEvent {
|
||||
turn_id: None,
|
||||
server_name,
|
||||
id: ProtocolRequestId::String(public_request_id),
|
||||
request,
|
||||
}),
|
||||
})
|
||||
.request_user_interaction(tx_event, &authority, server_name, request)
|
||||
.await
|
||||
.context("failed to deliver MCP elicitation request")?;
|
||||
rx.await
|
||||
.context("elicitation request channel closed unexpectedly")
|
||||
}
|
||||
.boxed()
|
||||
})
|
||||
|
||||
@@ -38,11 +38,17 @@ impl ElicitationReviewer for RecordingReviewer {
|
||||
assert_eq!(request.server_name, "independent-mcp");
|
||||
self.calls.fetch_add(/*val*/ 1, Relaxed);
|
||||
let active_elicitations = self.active_elicitations.clone();
|
||||
let active_during_review = usize::from(
|
||||
request
|
||||
.elicitation
|
||||
.meta()
|
||||
.is_some_and(|meta| meta.get(STRICT_AUTO_REVIEW_KEY) == Some(&Value::Bool(true))),
|
||||
);
|
||||
let response = self.response.clone();
|
||||
async move {
|
||||
assert_eq!(active_elicitations.load(Relaxed), 1);
|
||||
assert_eq!(active_elicitations.load(Relaxed), active_during_review);
|
||||
tokio::task::yield_now().await;
|
||||
assert_eq!(active_elicitations.load(Relaxed), 1);
|
||||
assert_eq!(active_elicitations.load(Relaxed), active_during_review);
|
||||
response.map_err(anyhow::Error::msg)
|
||||
}
|
||||
.boxed()
|
||||
@@ -87,6 +93,7 @@ fn elicitation_fixture(
|
||||
.insert("another-independent-mcp".to_string(), permission_profile);
|
||||
let manager = ElicitationRequestManager::new(
|
||||
config,
|
||||
/*allow_user_interaction*/ true,
|
||||
reviewer.map(|reviewer| reviewer as Arc<dyn ElicitationReviewer>),
|
||||
lifecycle,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -113,6 +120,243 @@ async fn send_elicitation(sender: &SendElicitation, marker: Option<Value>) -> El
|
||||
.expect("elicitation must receive a terminal response")
|
||||
}
|
||||
|
||||
fn disable_user_interaction(manager: &ElicitationRequestManager) {
|
||||
let authority = manager.authority.lock().unwrap().clone().unwrap();
|
||||
assert!(manager.update(
|
||||
authority.config,
|
||||
/*allow_user_interaction*/ false,
|
||||
authority.reviewer,
|
||||
authority.lifecycle,
|
||||
));
|
||||
}
|
||||
|
||||
fn form_elicitations(meta: Option<Value>, properties: Value) -> [Elicitation; 3] {
|
||||
let schema = json!({"type": "object", "properties": properties});
|
||||
[
|
||||
Elicitation::Mcp(ElicitRequestParams::FormElicitationParams {
|
||||
meta: meta
|
||||
.as_ref()
|
||||
.map(|meta| RequestMetaObject::from(meta.as_object().unwrap().clone())),
|
||||
message: "Review this request".into(),
|
||||
requested_schema: serde_json::from_value(schema.clone()).unwrap(),
|
||||
}),
|
||||
Elicitation::OpenAiForm {
|
||||
meta: meta.clone(),
|
||||
message: "Review this request".into(),
|
||||
requested_schema: schema.clone(),
|
||||
},
|
||||
Elicitation::OpenAiElicitationForm {
|
||||
meta,
|
||||
message: "Review this request".into(),
|
||||
requested_schema: schema,
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
async fn assert_subagent_elicitation_blocked(sender: &SendElicitation, elicitation: Elicitation) {
|
||||
let error = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(1),
|
||||
sender(RequestId::Number(7), elicitation),
|
||||
)
|
||||
.await
|
||||
.expect("blocked elicitation must not wait for user input")
|
||||
.unwrap_err();
|
||||
assert_eq!(error.to_string(), MCP_ELICITATION_HANDOFF_MESSAGE);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_human_input_is_rejected_before_automatic_approval() {
|
||||
let mut requests = Vec::new();
|
||||
for meta in [
|
||||
json!({"codex_approval_kind": "browser_auth"}),
|
||||
json!({"codex_approval_kind": "browser_auth", "codex_requires_user_input": true}),
|
||||
json!({"codex_requires_user_input": true}),
|
||||
json!({
|
||||
"codex_approval_kind": "mcp_tool_call",
|
||||
"codex_request_type": "approval_request",
|
||||
"codex_strict_auto_review": true,
|
||||
"codex_requires_user_input": true,
|
||||
}),
|
||||
] {
|
||||
requests.extend(form_elicitations(Some(meta), json!({})));
|
||||
}
|
||||
for meta in [None, Some(json!({"codex_strict_auto_review": true}))] {
|
||||
requests.extend(form_elicitations(
|
||||
meta.clone(),
|
||||
json!({"answer": {"type": "string"}}),
|
||||
));
|
||||
requests.push(Elicitation::Mcp(
|
||||
ElicitRequestParams::UrlElicitationParams {
|
||||
meta: meta.map(|meta| RequestMetaObject::from(meta.as_object().unwrap().clone())),
|
||||
message: "Sign in to continue".into(),
|
||||
url: "https://example.com/auth".into(),
|
||||
elicitation_id: "test-auth".into(),
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
for approval_policy in [AskForApproval::OnRequest, AskForApproval::Never] {
|
||||
let reviewer = RecordingReviewer::new(Ok(Some(approved_response())));
|
||||
let (manager, events, sender) = elicitation_fixture(
|
||||
approval_policy,
|
||||
PermissionProfile::Disabled,
|
||||
Some(reviewer.clone()),
|
||||
);
|
||||
// Update after creating the sender so a retained connection observes the restriction.
|
||||
disable_user_interaction(&manager);
|
||||
for elicitation in requests.iter().cloned() {
|
||||
assert_subagent_elicitation_blocked(&sender, elicitation).await;
|
||||
}
|
||||
assert!(events.is_empty());
|
||||
assert!(manager.router.requests.lock().unwrap().is_empty());
|
||||
assert_eq!(reviewer.calls.load(Relaxed), 0);
|
||||
assert_eq!(reviewer.active_elicitations.load(Relaxed), 0);
|
||||
|
||||
let (manager, events, sender) =
|
||||
verification_fixture(approval_policy, Some(reviewer.clone()));
|
||||
disable_user_interaction(&manager);
|
||||
assert_subagent_elicitation_blocked(
|
||||
&sender,
|
||||
Elicitation::UserVerification {
|
||||
title: "Approve purchase".into(),
|
||||
description: "Pay $200".into(),
|
||||
challenge: "AQID".into(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
assert!(events.is_empty());
|
||||
assert!(manager.router.requests.lock().unwrap().is_empty());
|
||||
assert_eq!(reviewer.calls.load(Relaxed), 0);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_permission_preserves_automatic_review_decisions() {
|
||||
for (strict_auto_review, approval_policy, expected_reviews) in [
|
||||
(false, AskForApproval::OnRequest, 1),
|
||||
(true, AskForApproval::OnRequest, 1),
|
||||
(true, AskForApproval::Never, 1),
|
||||
(false, AskForApproval::Never, 0),
|
||||
] {
|
||||
for response in [
|
||||
approved_response(),
|
||||
ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: Some(json!({"approvals_reviewer": "auto_review", "message": "Denied"})),
|
||||
},
|
||||
] {
|
||||
let reviewer = RecordingReviewer::new(Ok(Some(response.clone())));
|
||||
let (manager, events, sender) = elicitation_fixture(
|
||||
approval_policy,
|
||||
PermissionProfile::Disabled,
|
||||
Some(reviewer.clone()),
|
||||
);
|
||||
disable_user_interaction(&manager);
|
||||
let [request, _, _] = form_elicitations(
|
||||
Some(json!({
|
||||
"codex_request_type": "approval_request",
|
||||
"codex_approval_kind": "mcp_tool_call",
|
||||
"codex_strict_auto_review": strict_auto_review,
|
||||
"tool_name": "access_browser_origin",
|
||||
"tool_params": {"origin": "https://example.com"},
|
||||
})),
|
||||
json!({}),
|
||||
);
|
||||
let expected = if expected_reviews == 0 {
|
||||
ElicitationResponse {
|
||||
meta: None,
|
||||
..approved_response()
|
||||
}
|
||||
} else {
|
||||
response
|
||||
};
|
||||
assert_eq!(
|
||||
sender(RequestId::Number(7), request).await.unwrap(),
|
||||
expected
|
||||
);
|
||||
assert_eq!(reviewer.calls.load(Relaxed), expected_reviews);
|
||||
assert_eq!(reviewer.active_elicitations.load(Relaxed), 0);
|
||||
assert!(events.is_empty());
|
||||
assert!(manager.router.requests.lock().unwrap().is_empty());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_without_an_automatic_decision_does_not_prompt() {
|
||||
for reviewer in [None, Some(RecordingReviewer::new(Ok(None)))] {
|
||||
let (manager, events, sender) = elicitation_fixture(
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::read_only(),
|
||||
reviewer.clone(),
|
||||
);
|
||||
disable_user_interaction(&manager);
|
||||
for elicitation in form_elicitations(/*meta*/ None, json!({})) {
|
||||
assert_subagent_elicitation_blocked(&sender, elicitation).await;
|
||||
}
|
||||
assert_eq!(
|
||||
send_elicitation(&sender, Some(json!(true))).await,
|
||||
strict_auto_review_decline()
|
||||
);
|
||||
if let Some(reviewer) = reviewer {
|
||||
assert_eq!(reviewer.calls.load(Relaxed), 4);
|
||||
assert_eq!(reviewer.active_elicitations.load(Relaxed), 0);
|
||||
}
|
||||
assert!(events.is_empty());
|
||||
assert!(manager.router.requests.lock().unwrap().is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn final_prompt_guard_rejects_subagents_before_registration() {
|
||||
let (manager, _, _) = verification_fixture(AskForApproval::OnRequest, /*reviewer*/ None);
|
||||
let mut authority = manager.authority.lock().unwrap().clone().unwrap();
|
||||
authority.allow_user_interaction = false;
|
||||
let registrations = Arc::new(AtomicUsize::new(0));
|
||||
let starts = registrations.clone();
|
||||
authority.lifecycle = Some(ElicitationLifecycle::new(move || {
|
||||
starts.fetch_add(/*val*/ 1, Relaxed);
|
||||
}));
|
||||
let (tx, events) = async_channel::bounded(1);
|
||||
for sender in [Some(tx.clone()), None] {
|
||||
let error = manager
|
||||
.router
|
||||
.request_user_interaction(
|
||||
sender,
|
||||
&authority,
|
||||
crate::CODEX_APPS_MCP_SERVER_NAME.into(),
|
||||
ElicitationRequest::Form {
|
||||
meta: None,
|
||||
message: "Confirm the action".into(),
|
||||
requested_schema: json!({"type": "object", "properties": {}}),
|
||||
},
|
||||
)
|
||||
.now_or_never()
|
||||
.expect("a subagent must not wait for user input")
|
||||
.unwrap_err();
|
||||
assert_eq!(error.to_string(), MCP_ELICITATION_HANDOFF_MESSAGE);
|
||||
}
|
||||
let error = user_verification_elicitation::route(
|
||||
manager.router.clone(),
|
||||
Some(tx),
|
||||
Some(authority),
|
||||
crate::CODEX_APPS_MCP_SERVER_NAME.into(),
|
||||
ElicitationRequest::UserVerification {
|
||||
title: "Approve purchase".into(),
|
||||
description: "Pay $200".into(),
|
||||
challenge: "AQID".into(),
|
||||
},
|
||||
)
|
||||
.now_or_never()
|
||||
.expect("direct verification routing must not wait for a subagent")
|
||||
.unwrap_err();
|
||||
assert_eq!(error.to_string(), MCP_ELICITATION_HANDOFF_MESSAGE);
|
||||
assert_eq!(registrations.load(Relaxed), 0);
|
||||
assert!(events.is_empty());
|
||||
assert!(manager.router.requests.lock().unwrap().is_empty());
|
||||
}
|
||||
|
||||
async fn assert_declined(marker: Value, response: Option<ReviewerResponse>) {
|
||||
let expected_calls = usize::from(marker == Value::Bool(true));
|
||||
let reviewer = response.map(RecordingReviewer::new);
|
||||
@@ -150,6 +394,7 @@ fn closed_event_channel_immediately_cleans_up_pending_elicitation() {
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled
|
||||
),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
Some(lifecycle),
|
||||
));
|
||||
@@ -338,6 +583,7 @@ async fn reused_elicitation_senders_follow_each_servers_latest_permission_author
|
||||
|
||||
let manager = ElicitationRequestManager::new(
|
||||
Arc::new(config.clone()),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -368,6 +614,7 @@ async fn reused_elicitation_senders_follow_each_servers_latest_permission_author
|
||||
);
|
||||
assert!(manager.update(
|
||||
Arc::new(config.clone()),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
));
|
||||
@@ -391,6 +638,7 @@ async fn reused_elicitation_senders_follow_each_servers_latest_permission_author
|
||||
);
|
||||
assert!(manager.update(
|
||||
Arc::new(config.clone()),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
));
|
||||
@@ -403,6 +651,7 @@ async fn reused_elicitation_senders_follow_each_servers_latest_permission_author
|
||||
config.set_server_permission_profiles(&servers, std::iter::empty());
|
||||
assert!(manager.update(
|
||||
Arc::new(config.clone()),
|
||||
/*allow_user_interaction*/ true,
|
||||
/*reviewer*/ None,
|
||||
/*lifecycle*/ None,
|
||||
));
|
||||
@@ -434,6 +683,7 @@ fn verification_fixture(
|
||||
Arc::make_mut(&mut config).mcp_server_catalog = catalog.build();
|
||||
let manager = ElicitationRequestManager::new(
|
||||
config,
|
||||
/*allow_user_interaction*/ true,
|
||||
reviewer.map(|reviewer| reviewer as Arc<dyn ElicitationReviewer>),
|
||||
/*lifecycle*/ None,
|
||||
ElicitationRequestRouter::default(),
|
||||
@@ -574,6 +824,18 @@ async fn user_verification_cancels_for_an_event_receiver_without_host_activation
|
||||
async fn user_verification_drops_pending_response_when_the_request_is_cancelled() {
|
||||
let (manager, events, sender) =
|
||||
verification_fixture(AskForApproval::OnRequest, /*reviewer*/ None);
|
||||
let active_elicitations = Arc::new(AtomicUsize::new(0));
|
||||
let registrations = active_elicitations.clone();
|
||||
manager
|
||||
.authority
|
||||
.lock()
|
||||
.unwrap()
|
||||
.as_mut()
|
||||
.unwrap()
|
||||
.lifecycle = Some(ElicitationLifecycle::new(move || {
|
||||
registrations.fetch_add(/*val*/ 1, Relaxed);
|
||||
LifecycleRegistration(registrations.clone())
|
||||
}));
|
||||
let pending = tokio::spawn(sender(
|
||||
RequestId::Number(7),
|
||||
Elicitation::UserVerification {
|
||||
@@ -583,9 +845,11 @@ async fn user_verification_drops_pending_response_when_the_request_is_cancelled(
|
||||
},
|
||||
));
|
||||
events.recv().await.unwrap();
|
||||
assert_eq!(active_elicitations.load(Relaxed), 1);
|
||||
pending.abort();
|
||||
assert!(pending.await.unwrap_err().is_cancelled());
|
||||
assert!(manager.router.requests.lock().unwrap().is_empty());
|
||||
assert_eq!(active_elicitations.load(Relaxed), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -8,6 +8,7 @@ pub use elicitation::ElicitationLifecycle;
|
||||
pub use elicitation::ElicitationReviewRequest;
|
||||
pub use elicitation::ElicitationReviewer;
|
||||
pub use elicitation::ElicitationReviewerHandle;
|
||||
pub use elicitation::MCP_ELICITATION_HANDOFF_MESSAGE;
|
||||
pub use event_stream::McpEventStreamOpener;
|
||||
pub use resource_client::CodexAppsResourceListParams;
|
||||
pub use resource_client::McpEventCatalogSnapshot;
|
||||
|
||||
@@ -458,6 +458,7 @@ pub async fn read_mcp_resource(
|
||||
client_mcp_extensions: ClientMcpExtensions::default(),
|
||||
auth: auth.cloned(),
|
||||
auth_manager: None,
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
},
|
||||
@@ -537,6 +538,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
|
||||
client_mcp_extensions: ClientMcpExtensions::default(),
|
||||
auth: auth.cloned(),
|
||||
auth_manager: None,
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
},
|
||||
|
||||
@@ -71,7 +71,7 @@ pub enum McpStartupPolicy {
|
||||
LazyWhenCached,
|
||||
}
|
||||
|
||||
/// Everything needed to materialize one exact MCP configuration.
|
||||
/// Configuration and owning-thread state needed to materialize an MCP runtime.
|
||||
pub struct McpRuntimeInput {
|
||||
pub startup_policy: McpStartupPolicy,
|
||||
pub config: Arc<McpConfig>,
|
||||
@@ -88,6 +88,8 @@ pub struct McpRuntimeInput {
|
||||
pub client_mcp_extensions: ClientMcpExtensions,
|
||||
pub auth: Option<CodexAuth>,
|
||||
pub auth_manager: Option<Arc<AuthManager>>,
|
||||
/// Whether the owning thread may prompt the user; automatic decisions remain available.
|
||||
pub allow_user_interaction: bool,
|
||||
pub elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
pub elicitation_lifecycle: Option<ElicitationLifecycle>,
|
||||
}
|
||||
|
||||
@@ -5,14 +5,10 @@ use super::*;
|
||||
pub(super) async fn route(
|
||||
router: ElicitationRequestRouter,
|
||||
events: Option<Sender<Event>>,
|
||||
authority: Arc<StdMutex<Option<ElicitationAuthority>>>,
|
||||
authority: Option<ElicitationAuthority>,
|
||||
server_name: String,
|
||||
request: ElicitationRequest,
|
||||
) -> Result<ElicitationResponse> {
|
||||
let authority = authority
|
||||
.lock()
|
||||
.ok()
|
||||
.and_then(|authority| authority.clone());
|
||||
let plugin_service = authority.as_ref().is_some_and(|authority| {
|
||||
authority
|
||||
.config
|
||||
@@ -24,42 +20,17 @@ pub(super) async fn route(
|
||||
.is_host_owned_apps(&server_name, server.config())
|
||||
})
|
||||
});
|
||||
let Some(events) = events.filter(|_| plugin_service && !router.auto_deny()) else {
|
||||
let Some((events, authority)) = events
|
||||
.zip(authority)
|
||||
.filter(|_| plugin_service && !router.auto_deny())
|
||||
else {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
});
|
||||
};
|
||||
let id = format!(
|
||||
"codex-mcp-elicitation-{}",
|
||||
NEXT_ELICITATION_REQUEST_ID.fetch_add(1, Ordering::Relaxed)
|
||||
);
|
||||
let key = (server_name.clone(), RequestId::String(id.clone().into()));
|
||||
let (response, receiver) = oneshot::channel();
|
||||
router
|
||||
.requests
|
||||
.lock()
|
||||
.map_err(|_| anyhow!("elicitation request router unavailable"))?
|
||||
.insert(key.clone(), response);
|
||||
let _pending = PendingElicitationRequest { router, key };
|
||||
let _active = authority
|
||||
.as_ref()
|
||||
.and_then(|authority| authority.lifecycle.as_ref())
|
||||
.map(ElicitationLifecycle::start);
|
||||
events
|
||||
.send(Event {
|
||||
id: "mcp_elicitation_request".to_string(),
|
||||
msg: EventMsg::ElicitationRequest(ElicitationRequestEvent {
|
||||
turn_id: None,
|
||||
server_name,
|
||||
id: ProtocolRequestId::String(id),
|
||||
request,
|
||||
}),
|
||||
})
|
||||
.request_user_interaction(Some(events), &authority, server_name, request)
|
||||
.await
|
||||
.context("failed to deliver user-verification request")?;
|
||||
receiver
|
||||
.await
|
||||
.context("user-verification response channel closed")
|
||||
}
|
||||
|
||||
@@ -269,6 +269,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
client_mcp_extensions: ClientMcpExtensions::default(),
|
||||
auth: auth.clone(),
|
||||
auth_manager: codex_apps_auth_manager,
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
})
|
||||
|
||||
@@ -781,7 +781,7 @@ async fn maybe_request_codex_apps_auth_elicitation(
|
||||
url: plan.elicitation.url,
|
||||
elicitation_id: plan.elicitation.elicitation_id,
|
||||
};
|
||||
let response = sess
|
||||
let outcome = match sess
|
||||
.request_mcp_server_elicitation(
|
||||
turn_context,
|
||||
CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
@@ -789,8 +789,33 @@ async fn maybe_request_codex_apps_auth_elicitation(
|
||||
request,
|
||||
)
|
||||
.await
|
||||
.response;
|
||||
if !response
|
||||
{
|
||||
Ok(outcome) => outcome,
|
||||
Err(error) => {
|
||||
let mut result = result;
|
||||
// Direct output prefers structured content. Preserve its diagnostic as
|
||||
// text so both output paths show it alongside the recovery guidance.
|
||||
if let Some(structured_content) = result.structured_content.take() {
|
||||
result.content.push(serde_json::json!({
|
||||
"type": "text",
|
||||
"text": structured_content.to_string(),
|
||||
}));
|
||||
}
|
||||
result.content.insert(
|
||||
/*index*/ 0,
|
||||
serde_json::json!({
|
||||
"type": "text",
|
||||
"text": format!(
|
||||
"Authentication for {} could not be completed. {error}",
|
||||
plan.auth_failure.connector_name
|
||||
),
|
||||
}),
|
||||
);
|
||||
return result;
|
||||
}
|
||||
};
|
||||
if !outcome
|
||||
.response
|
||||
.as_ref()
|
||||
.is_some_and(|response| response.action == ElicitationAction::Accept)
|
||||
{
|
||||
@@ -1597,6 +1622,10 @@ pub(crate) async fn request_mcp_tool_user_approval(
|
||||
);
|
||||
}
|
||||
|
||||
if turn_context.session_source.is_non_root_agent() {
|
||||
return ReviewDecision::denied(codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE);
|
||||
}
|
||||
|
||||
let tool_call_mcp_elicitation_enabled = turn_context
|
||||
.config
|
||||
.features
|
||||
@@ -1662,9 +1691,13 @@ pub(crate) async fn request_mcp_tool_user_approval(
|
||||
.map(|rendered_template| rendered_template.elicitation_message.as_str()),
|
||||
prompt_options,
|
||||
});
|
||||
let outcome = sess
|
||||
let outcome = match sess
|
||||
.request_mcp_server_elicitation(turn_context, server.clone(), request_id, request)
|
||||
.await;
|
||||
.await
|
||||
{
|
||||
Ok(outcome) => outcome,
|
||||
Err(error) => return ReviewDecision::denied(error.to_string()),
|
||||
};
|
||||
(
|
||||
outcome.sent,
|
||||
parse_mcp_tool_approval_elicitation_response(outcome.response, &question_id),
|
||||
|
||||
@@ -13,6 +13,9 @@ use crate::session::tests::update_turn_settings_for_test;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::state::ActiveTurn;
|
||||
use crate::test_support::models_manager_with_provider;
|
||||
use crate::tools::context::McpToolOutput;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::turn_metadata::ExecutionMetadata;
|
||||
use codex_app_server_protocol as app_server_protocol;
|
||||
@@ -30,6 +33,7 @@ use codex_hooks::HooksConfig;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::ResponseItemId;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EnvironmentConfig;
|
||||
@@ -1724,6 +1728,88 @@ async fn codex_apps_auth_elicitation_granular_mcp_disabled_returns_original_resu
|
||||
assert!(rx_event.try_recv().is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_apps_auth_elicitation_returns_subagent_handoff_and_diagnostics() {
|
||||
let (session, mut turn_context, rx_event) = make_session_and_context_with_rx().await;
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("single turn context ref")
|
||||
.session_source = SessionSource::SubAgent(SubAgentSource::Review);
|
||||
let structured_content = serde_json::json!({
|
||||
"error": "reauthentication_required", "status": 401
|
||||
});
|
||||
let mut result = codex_apps_auth_failure_result();
|
||||
result.structured_content = Some(structured_content.clone());
|
||||
let diagnostic = format!(
|
||||
"Connector reauthentication required: {}",
|
||||
"diagnostic detail ".repeat(/*n*/ 500)
|
||||
);
|
||||
result.content[0]["text"] = serde_json::json!(diagnostic);
|
||||
let metadata = codex_apps_auth_failure_metadata();
|
||||
let returned = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(1),
|
||||
maybe_request_codex_apps_auth_elicitation(
|
||||
&session,
|
||||
&turn_context,
|
||||
turn_context.approval_policy(),
|
||||
"call_123",
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
Some(&metadata),
|
||||
result.clone(),
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("subagent auth must not wait for user input");
|
||||
|
||||
assert_eq!(returned.meta, result.meta);
|
||||
assert_eq!(returned.is_error, Some(true));
|
||||
assert!(rx_event.try_recv().is_err());
|
||||
|
||||
let output = McpToolOutput {
|
||||
result: returned,
|
||||
tool_input: serde_json::json!({}),
|
||||
result_metadata_capture_allowed: true,
|
||||
wall_time: std::time::Duration::ZERO,
|
||||
original_image_detail_supported: false,
|
||||
truncation_policy: TruncationPolicy::Tokens(256),
|
||||
};
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: "{}".to_string(),
|
||||
};
|
||||
let code_mode = output.code_mode_result(&payload);
|
||||
assert_eq!(code_mode["isError"], serde_json::json!(true));
|
||||
assert!(code_mode.get("_meta").is_none());
|
||||
let code_text = code_mode["content"]
|
||||
.as_array()
|
||||
.expect("MCP content")
|
||||
.iter()
|
||||
.map(|item| item["text"].as_str().expect("auth diagnostic text"))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(code_text.contains("Authentication for Google Calendar could not be completed."));
|
||||
assert!(code_text.contains(codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE));
|
||||
assert!(code_text.contains(&diagnostic));
|
||||
assert!(code_text.contains(&structured_content.to_string()));
|
||||
assert_eq!(output.tool_result_metadata(), result.meta.as_ref());
|
||||
let mut expected_hook = code_mode;
|
||||
expected_hook["_meta"] = result.meta.expect("original auth metadata");
|
||||
assert_eq!(
|
||||
output.post_tool_use_response("call_123", &payload),
|
||||
Some(expected_hook)
|
||||
);
|
||||
|
||||
let ResponseInputItem::FunctionCallOutput {
|
||||
output: truncated, ..
|
||||
} = output.to_response_item("call_123", &payload)
|
||||
else {
|
||||
panic!("expected FunctionCallOutput");
|
||||
};
|
||||
let truncated_text = truncated.body.to_text().expect("truncated auth diagnostic");
|
||||
assert!(truncated_text.contains(codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE));
|
||||
assert!(truncated_text.contains("truncated"));
|
||||
assert!(!truncated_text.contains(&diagnostic));
|
||||
assert_eq!(truncated.success, Some(false));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_apps_auth_elicitation_enabled_by_default_requests_elicitation() {
|
||||
let (session, turn_context, rx_event) = make_session_and_context_with_rx().await;
|
||||
|
||||
@@ -556,16 +556,20 @@ impl Session {
|
||||
server_name: String,
|
||||
request_id: RequestId,
|
||||
request: ElicitationRequest,
|
||||
) -> McpServerElicitationOutcome {
|
||||
) -> anyhow::Result<McpServerElicitationOutcome> {
|
||||
anyhow::ensure!(
|
||||
!turn_context.session_source.is_non_root_agent(),
|
||||
codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE
|
||||
);
|
||||
if self.services.mcp_runtime.elicitations_auto_deny() {
|
||||
return McpServerElicitationOutcome {
|
||||
return Ok(McpServerElicitationOutcome {
|
||||
response: Some(ElicitationResponse {
|
||||
action: codex_rmcp_client::ElicitationAction::Accept,
|
||||
content: Some(serde_json::json!({})),
|
||||
meta: None,
|
||||
}),
|
||||
sent: false,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
let _elicitation = self.services.elicitations.register();
|
||||
@@ -617,10 +621,10 @@ impl Session {
|
||||
plugin_install_telemetry.tool_name.as_str(),
|
||||
);
|
||||
}
|
||||
McpServerElicitationOutcome {
|
||||
Ok(McpServerElicitationOutcome {
|
||||
response: rx_response.await.ok(),
|
||||
sent: true,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
#[expect(
|
||||
|
||||
@@ -404,6 +404,7 @@ impl Session {
|
||||
client_mcp_extensions: self.services.client_mcp_extensions.for_mcp_servers(),
|
||||
auth,
|
||||
auth_manager: Some(Arc::clone(&self.services.auth_manager)),
|
||||
allow_user_interaction: !desired.session_source.is_non_root_agent(),
|
||||
elicitation_reviewer,
|
||||
elicitation_lifecycle: Some(self.mcp_elicitation_lifecycle()),
|
||||
}
|
||||
|
||||
@@ -87,6 +87,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
use codex_protocol::protocol::InternalSessionSource;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope;
|
||||
@@ -652,7 +653,8 @@ async fn request_mcp_server_elicitation_auto_accepts_when_auto_deny_is_enabled()
|
||||
}),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
.await
|
||||
.expect("root thread elicitation should be accepted");
|
||||
|
||||
assert_eq!(
|
||||
response.response,
|
||||
@@ -666,6 +668,58 @@ async fn request_mcp_server_elicitation_auto_accepts_when_auto_deny_is_enabled()
|
||||
assert!(rx.try_recv().is_err());
|
||||
}
|
||||
|
||||
#[test_case(false; "interactive")]
|
||||
#[test_case(true; "auto_accept")]
|
||||
#[tokio::test]
|
||||
async fn request_mcp_server_elicitation_rejects_non_root_threads(auto_deny: bool) {
|
||||
for source in [
|
||||
SessionSource::SubAgent(SubAgentSource::Review),
|
||||
SessionSource::Internal(InternalSessionSource::Guardian),
|
||||
] {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("turn context should not be shared")
|
||||
.session_source = source;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
session
|
||||
.services
|
||||
.mcp_runtime
|
||||
.set_elicitations_auto_deny(auto_deny);
|
||||
let paused = session.subscribe_elicitation_pause_state();
|
||||
|
||||
let Err(error) = tokio::time::timeout(
|
||||
Duration::from_secs(1),
|
||||
session.request_mcp_server_elicitation(
|
||||
turn_context.as_ref(),
|
||||
"codex_apps".to_string(),
|
||||
RequestId::String("request-1".into()),
|
||||
ElicitationRequest::Url {
|
||||
meta: None,
|
||||
message: "Connect this app to continue.".to_string(),
|
||||
url: "https://example.com/connect".to_string(),
|
||||
elicitation_id: "connect-1".to_string(),
|
||||
},
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("non-root elicitation must not wait for user input") else {
|
||||
panic!("non-root elicitation must be rejected");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE
|
||||
);
|
||||
assert!(rx.try_recv().is_err());
|
||||
assert!(!*paused.borrow());
|
||||
assert!(
|
||||
!paused
|
||||
.has_changed()
|
||||
.expect("elicitation service should remain available")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn interrupting_regular_turn_waiting_on_startup_prewarm_emits_turn_aborted() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
|
||||
@@ -254,7 +254,8 @@ impl RequestPluginInstallHandler {
|
||||
request_id,
|
||||
request,
|
||||
)
|
||||
.await;
|
||||
.await
|
||||
.map_err(|error| FunctionCallError::RespondToModel(error.to_string()))?;
|
||||
let response = elicitation.response;
|
||||
if let Some(response) = response.as_ref() {
|
||||
maybe_persist_disabled_install_request(&session, &turn, &tool, response).await;
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
//! Verify MCP auth prompts and elicitation analytics through actual turns.
|
||||
//! Verify MCP prompts, non-root refusal, and elicitation analytics through actual turns.
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_analytics::AppServerRpcTransport;
|
||||
use codex_app_server_protocol as app;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::TurnInputSubmission;
|
||||
use codex_core::config::Constrained;
|
||||
@@ -14,13 +15,19 @@ use codex_extension_api::ToolLifecycleFuture;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE;
|
||||
use codex_protocol::approvals::ElicitationRequest;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::items::McpToolCallStatus;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ElicitationAction;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::apps_test_server::AppsTestServer;
|
||||
@@ -31,6 +38,8 @@ use core_test_support::apps_test_server::recorded_apps_tool_calls;
|
||||
use core_test_support::apps_test_server::search_capable_apps_builder;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_mcp_server;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -50,44 +59,63 @@ const CALL_ID: &str = "calendar-elicitation-call";
|
||||
const PRIVATE_SENTINEL: &str = "synthetic-private-value";
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct AuthFailureResponder(Scenario);
|
||||
enum AuthFailureFormat {
|
||||
Text,
|
||||
Structured,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct AuthFailureResponder {
|
||||
scenario: Scenario,
|
||||
format: AuthFailureFormat,
|
||||
}
|
||||
|
||||
impl AuthFailureResponder {
|
||||
fn result(self) -> Value {
|
||||
let mut response = json!({
|
||||
"content": [{
|
||||
"type": "text",
|
||||
"text": PRIVATE_SENTINEL,
|
||||
}],
|
||||
"isError": true,
|
||||
"_meta": {
|
||||
"_codex_apps": {
|
||||
"connector_auth_failure": {
|
||||
"is_auth_failure": true,
|
||||
"auth_reason": "reauthentication_required",
|
||||
"connector_id": "calendar",
|
||||
"link_id": "link_123",
|
||||
"error_code": "UNAUTHORIZED",
|
||||
"error_http_status_code": 401,
|
||||
"error_action": "TRIGGER_REAUTHENTICATION",
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
if self.scenario == Scenario::AuthMetadataRemoved {
|
||||
response["_meta"] = json!({"_codex_apps": {"connector_auth_failure": {
|
||||
"is_auth_failure": true, "connector_id": "calendar",
|
||||
}}});
|
||||
}
|
||||
if matches!(self.format, AuthFailureFormat::Structured) {
|
||||
response["structuredContent"] = json!({
|
||||
"error": "synthetic-structured-auth-failure",
|
||||
"details": {"reason": "reauthentication_required", "status": 401},
|
||||
});
|
||||
}
|
||||
response
|
||||
}
|
||||
}
|
||||
|
||||
impl Respond for AuthFailureResponder {
|
||||
fn respond(&self, request: &Request) -> ResponseTemplate {
|
||||
let body: Value =
|
||||
serde_json::from_slice(&request.body).expect("tools/call request should be valid JSON");
|
||||
let id = body.get("id").cloned().unwrap_or(Value::Null);
|
||||
|
||||
let mut response = json!({
|
||||
ResponseTemplate::new(/*status*/ 200).set_body_json(json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": id,
|
||||
"result": {
|
||||
"content": [{
|
||||
"type": "text",
|
||||
"text": PRIVATE_SENTINEL,
|
||||
}],
|
||||
"isError": true,
|
||||
"_meta": {
|
||||
"_codex_apps": {
|
||||
"connector_auth_failure": {
|
||||
"is_auth_failure": true,
|
||||
"auth_reason": "reauthentication_required",
|
||||
"connector_id": "calendar",
|
||||
"link_id": "link_123",
|
||||
"error_code": "UNAUTHORIZED",
|
||||
"error_http_status_code": 401,
|
||||
"error_action": "TRIGGER_REAUTHENTICATION",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
if self.0 == Scenario::AuthMetadataRemoved {
|
||||
response["result"]["_meta"] = json!({"_codex_apps": {"connector_auth_failure": {
|
||||
"is_auth_failure": true, "connector_id": "calendar",
|
||||
}}});
|
||||
}
|
||||
ResponseTemplate::new(/*status*/ 200).set_body_json(response)
|
||||
"id": body.get("id").cloned().unwrap_or(Value::Null),
|
||||
"result": self.result(),
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -141,7 +169,10 @@ async fn actual_turn_elicitation_analytics(scenario: Scenario) -> Result<()> {
|
||||
"method": "tools/call",
|
||||
"params": {"name": "calendar_create_event"},
|
||||
})))
|
||||
.respond_with(AuthFailureResponder(scenario))
|
||||
.respond_with(AuthFailureResponder {
|
||||
scenario,
|
||||
format: AuthFailureFormat::Text,
|
||||
})
|
||||
.with_priority(/*p*/ 1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
@@ -485,3 +516,207 @@ approvals_reviewer = "user"
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum SubagentRequestKind {
|
||||
ToolApproval,
|
||||
LegacyToolApproval,
|
||||
ConnectorAuth(AuthFailureFormat),
|
||||
}
|
||||
|
||||
#[test_case(SubagentRequestKind::ToolApproval, None; "child_tool_approval_is_rejected_before_execution")]
|
||||
#[test_case(SubagentRequestKind::LegacyToolApproval, None; "child_legacy_tool_approval_is_rejected_before_execution")]
|
||||
#[test_case(SubagentRequestKind::ConnectorAuth(AuthFailureFormat::Text), None; "child_connector_auth_preserves_text_diagnostics_and_requests_handoff")]
|
||||
#[test_case(SubagentRequestKind::ConnectorAuth(AuthFailureFormat::Structured), None; "child_connector_auth_preserves_structured_diagnostics_and_requests_handoff")]
|
||||
#[test_case(SubagentRequestKind::ConnectorAuth(AuthFailureFormat::Structured), Some(1); "child_connector_auth_structured_diagnostics_respect_output_limit")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn core_generated_mcp_elicitations_are_root_only(
|
||||
kind: SubagentRequestKind,
|
||||
tool_output_token_limit: Option<usize>,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = responses::start_mock_server().await;
|
||||
let apps = AppsTestServer::mount_searchable(&server).await?;
|
||||
if let SubagentRequestKind::ConnectorAuth(format) = kind {
|
||||
Mock::given(method("POST"))
|
||||
.and(path_regex("^/api/codex/ps/mcp/?$"))
|
||||
.and(body_partial_json(json!({
|
||||
"method": "tools/call",
|
||||
"params": {"name": "calendar_create_event"}
|
||||
})))
|
||||
.respond_with(AuthFailureResponder {
|
||||
scenario: Scenario::DefaultAuth,
|
||||
format,
|
||||
})
|
||||
.with_priority(/*p*/ 1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
}
|
||||
let test = search_capable_apps_builder(apps.chatgpt_base_url)
|
||||
.with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
config.tool_output_token_limit = tool_output_token_limit;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::AuthElicitation)
|
||||
.expect("enable auth elicitation");
|
||||
config
|
||||
.features
|
||||
.set_enabled(
|
||||
Feature::ToolCallMcpElicitation,
|
||||
!matches!(kind, SubagentRequestKind::LegacyToolApproval),
|
||||
)
|
||||
.expect("configure MCP approval transport");
|
||||
let approval_mode = match kind {
|
||||
SubagentRequestKind::ToolApproval | SubagentRequestKind::LegacyToolApproval => {
|
||||
"prompt"
|
||||
}
|
||||
SubagentRequestKind::ConnectorAuth(_) => "approve",
|
||||
};
|
||||
let user_config = toml::from_str(&format!(
|
||||
r#"[apps.calendar]
|
||||
default_tools_approval_mode = "{approval_mode}"
|
||||
approvals_reviewer = "user"
|
||||
"#
|
||||
))
|
||||
.expect("apps config");
|
||||
config.config_layer_stack = config
|
||||
.config_layer_stack
|
||||
.with_user_config(&config.codex_home.join("config.toml").abs(), user_config)
|
||||
.expect("apply apps config");
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
let child = test
|
||||
.thread_manager
|
||||
.start_thread(StartThreadOptions {
|
||||
session_source: Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: test.session_configured.thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
})),
|
||||
environments: Some(vec![test.executor_environment().selection().clone()]),
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?
|
||||
.thread;
|
||||
wait_for_mcp_server(&child, CODEX_APPS_MCP_SERVER_NAME).await?;
|
||||
responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_function_call_with_namespace(
|
||||
"calendar-call",
|
||||
SEARCH_CALENDAR_NAMESPACE,
|
||||
SEARCH_CALENDAR_CREATE_TOOL,
|
||||
&json!({"title": "Lunch", "starts_at": "2026-06-18T12:00:00Z"}).to_string(),
|
||||
),
|
||||
responses::ev_completed("tool-response"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let follow_up = responses::mount_sse_once(&server, responses::sse_completed("done")).await;
|
||||
child
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec to create a calendar event.".into(),
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
permission_profile: Some(PermissionProfile::Disabled),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let mut completed_status = None;
|
||||
let mut completed_result = None;
|
||||
wait_for_event(&child, |event| {
|
||||
assert!(
|
||||
!matches!(
|
||||
event,
|
||||
EventMsg::ElicitationRequest(_) | EventMsg::RequestUserInput(_)
|
||||
),
|
||||
"child MCP tool calls must not prompt the user"
|
||||
);
|
||||
if let EventMsg::ItemCompleted(event) = event
|
||||
&& let TurnItem::McpToolCall(item) = &event.item
|
||||
&& item.id == "calendar-call"
|
||||
{
|
||||
completed_status = Some(item.status);
|
||||
completed_result = item.result.clone();
|
||||
}
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
assert_eq!(completed_status, Some(McpToolCallStatus::Failed));
|
||||
assert_eq!(
|
||||
recorded_apps_tool_calls(&server).await.len(),
|
||||
usize::from(matches!(kind, SubagentRequestKind::ConnectorAuth(_)))
|
||||
);
|
||||
let output = follow_up
|
||||
.single_request()
|
||||
.function_call_output("calendar-call");
|
||||
let output_text = match &output["output"] {
|
||||
Value::String(text) => text.clone(),
|
||||
Value::Array(items) => items
|
||||
.iter()
|
||||
.filter_map(|item| item["text"].as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n"),
|
||||
other => panic!("unexpected MCP output: {other}"),
|
||||
};
|
||||
assert_eq!(
|
||||
output_text.contains(MCP_ELICITATION_HANDOFF_MESSAGE),
|
||||
tool_output_token_limit.is_none(),
|
||||
"handoff guidance must use the ordinary tool-output limit"
|
||||
);
|
||||
if tool_output_token_limit.is_some() {
|
||||
assert!(output_text.contains("truncated") || output_text.contains("omitted"));
|
||||
}
|
||||
if let SubagentRequestKind::ConnectorAuth(format) = kind {
|
||||
let original = AuthFailureResponder {
|
||||
scenario: Scenario::DefaultAuth,
|
||||
format,
|
||||
}
|
||||
.result();
|
||||
let completed_result = completed_result.expect("completed auth failure result");
|
||||
assert_eq!(completed_result.is_error, Some(true));
|
||||
assert_eq!(completed_result.meta, Some(original["_meta"].clone()));
|
||||
assert_eq!(completed_result.structured_content, None);
|
||||
|
||||
let mut expected_content = vec![json!({
|
||||
"type": "text",
|
||||
"text": format!(
|
||||
"Authentication for Calendar could not be completed. {MCP_ELICITATION_HANDOFF_MESSAGE}"
|
||||
),
|
||||
})];
|
||||
expected_content.extend(
|
||||
original["content"]
|
||||
.as_array()
|
||||
.expect("original content")
|
||||
.clone(),
|
||||
);
|
||||
if let Some(structured_content) = original.get("structuredContent") {
|
||||
expected_content.push(json!({"type": "text", "text": structured_content.to_string()}));
|
||||
}
|
||||
assert_eq!(completed_result.content, expected_content);
|
||||
for content in &expected_content {
|
||||
let text = content["text"].as_str().expect("auth diagnostic text");
|
||||
assert_eq!(
|
||||
output_text.contains(text),
|
||||
tool_output_token_limit.is_none(),
|
||||
"auth diagnostics and guidance must share the ordinary tool-output limit"
|
||||
);
|
||||
}
|
||||
assert!(
|
||||
!output_text.contains("link_123"),
|
||||
"private auth metadata must not reach the model"
|
||||
);
|
||||
}
|
||||
child.shutdown_and_wait().await?;
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -109,6 +109,7 @@ async fn hosted_plugin_runtime_ps_mcp_tool_calls_use_current_auth_manager_token(
|
||||
client_mcp_extensions: ClientMcpExtensions::default(),
|
||||
auth: Some(expected_auth.clone()),
|
||||
auth_manager: Some(Arc::clone(&auth_manager)),
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
})
|
||||
|
||||
@@ -0,0 +1,288 @@
|
||||
//! MCP requests keep human input on the root and allow automatic approval in subagents.
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_protocol::approvals::ElicitationAction;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::ResponsesRequest;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_wine_exec;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_mcp_server;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use test_case::test_case;
|
||||
use wiremock::matchers::body_partial_json;
|
||||
|
||||
const ROOT_ONLY_MESSAGE: &str = codex_mcp::MCP_ELICITATION_HANDOFF_MESSAGE;
|
||||
|
||||
const SERVER: &str = r#"
|
||||
import json
|
||||
import sys
|
||||
|
||||
def send(message):
|
||||
print(json.dumps({"jsonrpc": "2.0", **message}), flush=True)
|
||||
|
||||
elicitation = json.loads(sys.argv[1])
|
||||
pending = None
|
||||
for line in sys.stdin:
|
||||
request = json.loads(line)
|
||||
method = request.get("method")
|
||||
if method == "initialize":
|
||||
result = {"protocolVersion": request["params"]["protocolVersion"],
|
||||
"capabilities": {"tools": {}},
|
||||
"serverInfo": {"name": "elicitation-test", "version": "1"}}
|
||||
elif method == "tools/list":
|
||||
result = {"tools": [{"name": "request_input", "inputSchema": {"type": "object", "properties": {}},
|
||||
"annotations": {"readOnlyHint": True}}]}
|
||||
elif method == "tools/call":
|
||||
pending = request["id"]
|
||||
send({"id": "input-request", "method": "elicitation/create", "params": elicitation})
|
||||
continue
|
||||
elif method is None and request.get("id") == "input-request":
|
||||
send({"id": pending, "result": {"content": [{"type": "text", "text": json.dumps(
|
||||
request.get("result", request.get("error")))}]}})
|
||||
continue
|
||||
elif method == "resources/list":
|
||||
result = {"resources": []}
|
||||
elif method == "resources/templates/list":
|
||||
result = {"resourceTemplates": []}
|
||||
elif "id" not in request:
|
||||
continue
|
||||
else:
|
||||
result = {}
|
||||
send({"id": request["id"], "result": result})
|
||||
"#;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub(super) enum Caller {
|
||||
Root,
|
||||
Subagent,
|
||||
FullAccessSubagent,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub(super) enum RequestKind {
|
||||
BrowserAuth,
|
||||
Permission,
|
||||
StrictReview,
|
||||
}
|
||||
|
||||
#[test_case(Caller::FullAccessSubagent, RequestKind::BrowserAuth; "full_access_cannot_auto_accept_subagent_browser_auth")]
|
||||
#[test_case(Caller::Root, RequestKind::BrowserAuth; "root_browser_auth_remains_interactive")]
|
||||
#[test_case(Caller::FullAccessSubagent, RequestKind::Permission; "full_access_subagent_automatically_approves_permission")]
|
||||
#[test_case(Caller::Subagent, RequestKind::StrictReview; "subagent_keeps_strict_automatic_safety_review")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn mcp_server_elicitations_keep_user_interaction_on_root(
|
||||
caller: Caller,
|
||||
request_kind: RequestKind,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_wine_exec!(Ok(()), "the MCP fixture requires a host Python interpreter");
|
||||
mcp_server_elicitation_scenario(caller, request_kind).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) async fn mcp_server_elicitation_scenario(
|
||||
caller: Caller,
|
||||
request_kind: RequestKind,
|
||||
) -> Result<Vec<ResponsesRequest>> {
|
||||
let server = responses::start_mock_server().await;
|
||||
let test = test_codex().build_with_auto_env(&server).await?;
|
||||
let mut elicitation = json!({
|
||||
"mode": "form",
|
||||
"message": "Provide the requested input.",
|
||||
"requestedSchema": {"type": "object", "properties": {}}
|
||||
});
|
||||
match request_kind {
|
||||
RequestKind::BrowserAuth => {
|
||||
// Browser credentials travel through the browser broker, so the MCP schema is empty.
|
||||
elicitation["_meta"] = json!({
|
||||
"codex_approval_kind": "browser_auth",
|
||||
"browser_auth_challenge_id": "test_browser_auth_challenge_0123456789",
|
||||
"origin": "https://example.com",
|
||||
"reason": "Sign in to continue browsing.",
|
||||
"fields": [{"id": "password", "label": "Password", "type": "password", "required": true}]
|
||||
});
|
||||
}
|
||||
RequestKind::Permission | RequestKind::StrictReview => {
|
||||
elicitation["_meta"] = json!({
|
||||
"codex_request_type": "approval_request",
|
||||
"codex_approval_kind": "mcp_tool_call",
|
||||
"codex_strict_auto_review": matches!(request_kind, RequestKind::StrictReview),
|
||||
"codex_sensitive_action": true,
|
||||
"tool_name": "write_record",
|
||||
"tool_params": {"value": 42}
|
||||
});
|
||||
}
|
||||
}
|
||||
let mut config = test.config.clone();
|
||||
config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
config.mcp_servers.set(serde_json::from_value(json!({
|
||||
"elicitation": {
|
||||
"command": if cfg!(windows) { "python" } else { "python3" },
|
||||
"args": ["-u", "-c", SERVER, elicitation.to_string()],
|
||||
"default_tools_approval_mode": "approve"
|
||||
}
|
||||
}))?)?;
|
||||
let expects_prompt = matches!(caller, Caller::Root);
|
||||
let session_source = if expects_prompt {
|
||||
SessionSource::Exec
|
||||
} else {
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: test.session_configured.thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
})
|
||||
};
|
||||
let thread = test
|
||||
.thread_manager
|
||||
.start_thread(StartThreadOptions {
|
||||
session_source: Some(session_source),
|
||||
environments: Some(vec![test.executor_environment().selection().clone()]),
|
||||
..StartThreadOptions::new(config)
|
||||
})
|
||||
.await?
|
||||
.thread;
|
||||
wait_for_mcp_server(&thread, "elicitation").await?;
|
||||
let tool_call = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_function_call_with_namespace(
|
||||
"input-call",
|
||||
"mcp__elicitation",
|
||||
"request_input",
|
||||
"{}",
|
||||
),
|
||||
responses::ev_completed("tool-response"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let guardian = if matches!(request_kind, RequestKind::StrictReview) {
|
||||
Some(
|
||||
responses::mount_sse_once_match(
|
||||
&server,
|
||||
body_partial_json(json!({"client_metadata": {"x-openai-subagent": "guardian"}})),
|
||||
responses::sse(vec![
|
||||
responses::ev_assistant_message(
|
||||
"review-result",
|
||||
&json!({
|
||||
"risk_level": "low", "user_authorization": "high", "outcome": "allow",
|
||||
"rationale": "The user requested this action."
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
responses::ev_completed("guardian-review"),
|
||||
]),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let follow_up = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_assistant_message("done-message", "done"),
|
||||
responses::ev_completed("done-response"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let full_access = matches!(caller, Caller::FullAccessSubagent);
|
||||
thread
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "Run the input tool.".into(),
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
approval_policy: Some(if full_access {
|
||||
AskForApproval::Never
|
||||
} else {
|
||||
AskForApproval::OnRequest
|
||||
}),
|
||||
permission_profile: Some(if full_access {
|
||||
PermissionProfile::Disabled
|
||||
} else {
|
||||
PermissionProfile::read_only()
|
||||
}),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let mut prompts = 0;
|
||||
loop {
|
||||
let event = wait_for_event(&thread, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ElicitationRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
let EventMsg::ElicitationRequest(request) = event else {
|
||||
break;
|
||||
};
|
||||
assert!(
|
||||
expects_prompt,
|
||||
"subagent MCP elicitations must not reach the user"
|
||||
);
|
||||
assert_eq!(prompts, 0, "one tool call must not prompt twice");
|
||||
prompts += 1;
|
||||
assert!(
|
||||
follow_up.requests().is_empty(),
|
||||
"the tool must wait for input"
|
||||
);
|
||||
thread
|
||||
.submit(Op::ResolveElicitation {
|
||||
server_name: request.server_name,
|
||||
request_id: request.id,
|
||||
decision: ElicitationAction::Accept,
|
||||
content: Some(json!({})),
|
||||
meta: None,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
assert_eq!(prompts, usize::from(expects_prompt));
|
||||
let output = follow_up
|
||||
.single_request()
|
||||
.function_call_output("input-call");
|
||||
let response: Value = serde_json::from_str(
|
||||
output["output"][1]["text"]
|
||||
.as_str()
|
||||
.expect("MCP response must reach the model"),
|
||||
)?;
|
||||
let expected = if matches!(request_kind, RequestKind::StrictReview) {
|
||||
json!({
|
||||
"action": "accept", "content": {},
|
||||
"_meta": {"approvals_reviewer": "auto_review"}
|
||||
})
|
||||
} else if expects_prompt || full_access && matches!(request_kind, RequestKind::Permission) {
|
||||
json!({"action": "accept", "content": {}})
|
||||
} else {
|
||||
json!({"code": -32603, "message": ROOT_ONLY_MESSAGE})
|
||||
};
|
||||
assert_eq!(response, expected);
|
||||
let mut requests = tool_call.requests();
|
||||
if let Some(guardian) = guardian {
|
||||
assert!(guardian.single_request().body_contains_text("write_record"));
|
||||
requests.extend(guardian.requests());
|
||||
}
|
||||
requests.extend(follow_up.requests());
|
||||
thread.shutdown_and_wait().await?;
|
||||
test.codex.shutdown_and_wait().await?;
|
||||
Ok(requests)
|
||||
}
|
||||
@@ -113,6 +113,7 @@ mod mcp_optional_startup_grace;
|
||||
#[cfg(unix)]
|
||||
mod mcp_refresh_cleanup;
|
||||
mod mcp_startup_refresh_http_proxy;
|
||||
mod mcp_subagent_elicitation;
|
||||
mod mcp_tool_cache;
|
||||
mod mcp_tool_exposure;
|
||||
mod mcp_turn_metadata;
|
||||
|
||||
@@ -756,6 +756,30 @@ async fn astra_refreshes_plugin_tools_and_skills_in_an_existing_thread() -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn subagent_browser_auth_returns_handoff_without_prompting() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_wine_exec!(Ok(()), "the MCP fixture requires a host Python interpreter");
|
||||
use super::mcp_subagent_elicitation::Caller;
|
||||
use super::mcp_subagent_elicitation::RequestKind;
|
||||
use super::mcp_subagent_elicitation::mcp_server_elicitation_scenario;
|
||||
|
||||
let requests =
|
||||
mcp_server_elicitation_scenario(Caller::Subagent, RequestKind::BrowserAuth).await?;
|
||||
let snapshot = context_snapshot::format_request_history_snapshot(
|
||||
"An MCP browser sign-in request fails in a subagent without prompting the user; the next model request contains guidance to ask the parent.",
|
||||
&requests,
|
||||
&ContextSnapshotOptions::default()
|
||||
.rewrite_known_segments()
|
||||
.include_request_settings(),
|
||||
);
|
||||
let snapshot = regex_lite::Regex::new(r"Wall time: [0-9]+(?:\.[0-9]+)? seconds")?
|
||||
.replace_all(&snapshot, "Wall time: <DURATION> seconds")
|
||||
.into_owned();
|
||||
insta::assert_snapshot!("subagent_browser_auth_handoff", snapshot);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn guardian_checkpoint_migration_request_history() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
+40
@@ -0,0 +1,40 @@
|
||||
---
|
||||
source: core/tests/suite/scenarios.rs
|
||||
expression: snapshot
|
||||
---
|
||||
Scenario: An MCP browser sign-in request fails in a subagent without prompting the user; the next model request contains guidance to ask the parent.
|
||||
|
||||
## Window 1
|
||||
Settings:
|
||||
include: ["reasoning.encrypted_content"]
|
||||
instructions: <MODEL_INSTRUCTIONS>
|
||||
model: "gpt-5.5"
|
||||
parallel_tool_calls: true
|
||||
prompt_cache_key: "<PROMPT_CACHE_KEY 1>"
|
||||
reasoning: {"effort":"medium"}
|
||||
store: false
|
||||
stream: true
|
||||
text: {"verbosity":"low"}
|
||||
tool_choice: "auto"
|
||||
tools (10; hash=1d541727c70b02b8):
|
||||
- function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5
|
||||
- function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2
|
||||
- function/list_mcp_resources: Lists resources provided by MCP servers. Resources allow servers to share data that provides cont...; args=[cursor, server]; hash=6792edaff13561bd
|
||||
- function/list_mcp_resource_templates: Lists resource templates provided by MCP servers. Parameterized resource templates allow servers ...; args=[cursor, server]; hash=60f17c90ef7a4ebb
|
||||
- function/read_mcp_resource: Read a specific resource from an MCP server given the server name and resource URI.; args=[server, uri]; hash=24246896b39008ae
|
||||
- function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6
|
||||
- custom/apply_patch: The `apply_patch` tool can be used to edit files. This is a FREEFORM tool, so do not wrap the pat...; hash=dea9f12b0111a4ba
|
||||
- function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[detail, path]; hash=3b766d5bdec5d50e
|
||||
- tool_search: # Tool discovery Searches over deferred tool metadata with BM25 and exposes matching tools for t...; args=[limit, query]; hash=cea8b3c4515459b0
|
||||
- web_search; hash=f103e327c6870a46
|
||||
-- request 1 (turn) --
|
||||
00:message/developer:
|
||||
<PERMISSIONS_INSTRUCTIONS>
|
||||
01:message/user:
|
||||
<ENVIRONMENT_CONTEXT>
|
||||
02:message/user:
|
||||
Run the input tool.
|
||||
-- request 2 (turn) --
|
||||
03:function_call/mcp__elicitation.request_input:{}
|
||||
04:function_call_output:Wall time: <DURATION> seconds
|
||||
Output: | {"code": -32603, "message": "MCP server elicitations can only be requested by the root thread. Ask the p... until the parent confirms the blocker is resolved."} [hash=096a35c5742e4b6f]
|
||||
@@ -176,6 +176,7 @@ impl Fixture {
|
||||
client_mcp_extensions: Default::default(),
|
||||
auth: Some(auth_value.clone()),
|
||||
auth_manager: Some(Arc::clone(&auth)),
|
||||
allow_user_interaction: true,
|
||||
elicitation_reviewer: None,
|
||||
elicitation_lifecycle: None,
|
||||
};
|
||||
|
||||
@@ -3,7 +3,10 @@
|
||||
/// form-forwarding safeguards.
|
||||
pub const APPROVAL_KIND_KEY: &str = "codex_approval_kind";
|
||||
pub const APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call";
|
||||
pub const APPROVAL_KIND_BROWSER_AUTH: &str = "browser_auth";
|
||||
pub const APPROVAL_KIND_TOOL_SUGGESTION: &str = "tool_suggestion";
|
||||
/// Marks requests that need user input even when their form schema is empty.
|
||||
pub const REQUIRES_USER_INPUT_KEY: &str = "codex_requires_user_input";
|
||||
pub const REQUEST_TYPE_KEY: &str = "codex_request_type";
|
||||
pub const REQUEST_TYPE_APPROVAL_REQUEST: &str = "approval_request";
|
||||
pub const STRICT_AUTO_REVIEW_KEY: &str = "codex_strict_auto_review";
|
||||
|
||||
Reference in New Issue
Block a user