Route MCP elicitation IDs across managers#29944
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a70665711b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let response_id = RequestId::String(Arc::from(format!("{route_id}:{id}"))); | ||
| { | ||
| let mut lock = elicitation_requests.lock().await; | ||
| lock.insert((server_name.clone(), id.clone()), tx); | ||
| lock.insert((server_name.clone(), response_id.clone()), tx); |
There was a problem hiding this comment.
When a refreshed/candidate manager emits an elicitation before refresh_mcp_servers swaps it in, or a superseded manager still has an unanswered prompt, the client returns this routed ID but Session::resolve_elicitation still calls mcp_connection_manager.load_full().resolve_elicitation(...) on only the current manager. Because the routed ID is stored only in the originating manager's local requests map here, the prefix does not actually route across managers and the response is rejected as elicitation request not found; please keep a session-level registry or dispatch by the route prefix to the owning manager.
Useful? React with 👍 / 👎.
| }, | ||
| }; | ||
| let (tx, rx) = oneshot::channel(); | ||
| let response_id = RequestId::String(Arc::from(format!("{route_id}:{id}"))); |
There was a problem hiding this comment.
If an MCP server has overlapping elicitations whose JSON-RPC IDs are 1 and "1", this stringified routed ID makes both entries use the same key (<route>:1), so the second insert overwrites the first responder and one request can never be resolved. The previous typed RequestId key distinguished numeric and string IDs, so the route wrapper should encode the variant as well as the value.
Useful? React with 👍 / 👎.
1 participant
Summary
Why
Runtime snapshot preparation can briefly keep an active MCP manager and a candidate manager alive together. MCP request IDs are connection-local, so responses need a manager discriminator to avoid being delivered to the wrong generation.