mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Propagate MCP elicitation event delivery failures (#38035)
## Why When the event receiver is closed, an MCP elicitation request cannot reach its consumer. Ignoring that delivery failure leaves the request and its lifecycle registration pending while waiting for a response that cannot arrive. ## What changed Return a contextual error when an MCP elicitation event cannot be delivered. This drops the pending-request guard immediately, removing the responder from the router and releasing its lifecycle registration. ## Testing Add a regression test that closes the event channel and verifies the request fails immediately, the router is empty, and the lifecycle registration is released. GitOrigin-RevId: 1067f35a5775d61c5bcd07b4b190d3907d8f219a
This commit is contained in:
committed by
copyberry
parent
ed390a5dc4
commit
e20616d265
@@ -418,7 +418,7 @@ impl ElicitationRequestManager {
|
||||
router: router.clone(),
|
||||
key: request_key,
|
||||
};
|
||||
let _ = tx_event
|
||||
tx_event
|
||||
.send(Event {
|
||||
id: "mcp_elicitation_request".to_string(),
|
||||
msg: EventMsg::ElicitationRequest(ElicitationRequestEvent {
|
||||
@@ -428,7 +428,8 @@ impl ElicitationRequestManager {
|
||||
request,
|
||||
}),
|
||||
})
|
||||
.await;
|
||||
.await
|
||||
.context("failed to deliver MCP elicitation request")?;
|
||||
rx.await
|
||||
.context("elicitation request channel closed unexpectedly")
|
||||
}
|
||||
|
||||
@@ -118,6 +118,52 @@ async fn assert_declined(marker: Value, response: Option<ReviewerResponse>) {
|
||||
assert!(events.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn closed_event_channel_immediately_cleans_up_pending_elicitation() {
|
||||
let active_elicitations = Arc::new(AtomicUsize::new(0));
|
||||
let registrations = active_elicitations.clone();
|
||||
let lifecycle = ElicitationLifecycle::new(move || {
|
||||
registrations.fetch_add(/*val*/ 1, Relaxed);
|
||||
LifecycleRegistration(registrations.clone())
|
||||
});
|
||||
let (manager, events, sender) = elicitation_fixture(
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled,
|
||||
/*reviewer*/ None,
|
||||
);
|
||||
assert!(manager.update(
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled,
|
||||
/*reviewer*/ None,
|
||||
Some(lifecycle),
|
||||
));
|
||||
drop(events);
|
||||
|
||||
let elicitation = Elicitation::Mcp(ElicitRequestParams::FormElicitationParams {
|
||||
meta: None,
|
||||
message: "Review this request".to_string(),
|
||||
requested_schema: ElicitationSchema::builder().build().unwrap(),
|
||||
});
|
||||
let error = sender(RequestId::Number(7), elicitation)
|
||||
.now_or_never()
|
||||
.expect("closed event channel must not leave an elicitation pending")
|
||||
.expect_err("closed event channel must fail the elicitation");
|
||||
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"failed to deliver MCP elicitation request"
|
||||
);
|
||||
assert!(
|
||||
manager
|
||||
.router
|
||||
.requests
|
||||
.lock()
|
||||
.expect("pending request router should be available")
|
||||
.is_empty()
|
||||
);
|
||||
assert_eq!(active_elicitations.load(Relaxed), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn strict_auto_review_respects_explicit_elicitation_denials() {
|
||||
for policy in [
|
||||
|
||||
Reference in New Issue
Block a user