fix(mcp): guard callTool/readResource/getPrompt against missing connections#1653
Draft
mattzcarey wants to merge 1 commit into
Draft
fix(mcp): guard callTool/readResource/getPrompt against missing connections#1653mattzcarey wants to merge 1 commit into
mattzcarey wants to merge 1 commit into
Conversation
…ctions MCPClientManager.callTool, readResource and getPrompt dereferenced this.mcpConnections[serverId].client directly. When no in-memory connection exists for the id, this surfaced as the opaque "Cannot read properties of undefined (reading 'client')". The in-memory mcpConnections map and the storage-backed listServers() can legitimately diverge: a caller may hold a serverId from an earlier snapshot whose connection has since been removed, the agent may have woken from hibernation with restore still in flight, or the id may be stale after a reconnect. Route all three call sites through a new #requireConnection helper that throws a named, actionable error so callers can retry, re-sync, or fail gracefully instead of crashing. Adds tests covering the missing-connection path for all three methods plus the happy path for callTool.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MCPClientManager.callTool,readResource, andgetPromptdereferencethis.mcpConnections[serverId].clientdirectly. When no in-memory connectionexists for the id, this surfaces as the opaque:
The in-memory
mcpConnectionsmap and the storage-backedlistServers()canlegitimately diverge:
serverIdfrom an earlier snapshot whose connection hassince been removed;
In all three cases callers get a
TypeErrorwith no way to react.Fix
Route all three call sites through a private
#requireConnection(serverId)helper that throws a named, actionable error when the connection is missing,
so callers can retry, re-sync, or fail gracefully instead of crashing.
Tests
Adds coverage for the missing-connection path of all three methods plus the
happy path for
callTool(client-manager.test.ts). Full workers projectsuite passes locally (145 tests).
Context
Surfaced in a downstream consumer (per-user
MCPClientManagerin a DurableObject, tools proxied across a second DO) where a snapshotted
serverIdcouldoutlive its connection within a single turn. That consumer also adds its own
wait+recheck guard, but the SDK dereferencing
undefinedis the root defect.