History

Answer dead-session standalone GETs with 405 to stop reconnect loops (#1622)

Rhys Sullivan committed 2 months ago

Commit f3ec48d1 parent a8d3d3c6 files · +156 −15Browse files

Unified · Split

6 files
  1. apps/cloud/src/mcp/agent-handler.ts +23 −4
  2. apps/host-cloudflare/src/mcp/agent-handler.ts +22 −3
  3. packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts +28 −0
  4. packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +20 −2
  5. packages/hosts/mcp/src/envelope.test.ts +37 −0
  6. packages/hosts/mcp/src/envelope.ts +26 −6
apps/cloud/src/mcp/agent-handler.ts+23−4
@@ -45,6 +45,25 @@ const jsonRpcResponse = ( · 44 unmodified lines
45 ? jsonRpcErrorBody(status, code, message)45 ? jsonRpcErrorBody(status, code, message)
46 : jsonRpcErrorBody(status, code, message, { challenge });46 : jsonRpcErrorBody(status, code, message, { challenge });
47 47
48/**
49 * A dead session id answers by request method. POST/DELETE keep the 404 that
50 * tells a compliant client to re-initialize. A standalone GET gets 405: the
51 * v1 SDK treats that as "no SSE stream offered" and stops retrying quietly,
52 * which breaks the reconnect loops of pre-cutover always-on deployments —
53 * their GET-404 path never re-initialized, it just retried forever.
54 */
55const deadSessionResponse = (method: string, message: string): Response =>
56 method === "GET"
57 ? new Response(JSON.stringify({ jsonrpc: "2.0", error: { code: -32001, message }, id: null }), {
58 status: 405,
59 headers: {
60 "content-type": "application/json",
61 allow: "POST, DELETE",
62 "access-control-allow-origin": "*",
63 },
64 })
65 : jsonRpcResponse(404, -32001, message);
66
48const renderAuthError = (67const renderAuthError = (
49 auth: McpAuthProvider["Service"],68 auth: McpAuthProvider["Service"],
50 request: Request,69 request: Request,
@@ -217,7 +236,7 @@ export const makeCloudMcpAgentHandler = () => { · 166 unmodified lines
217 236
218 const existingSession = sessionId ? mcpSessionStub(env.MCP_SESSION, sessionId) : null;237 const existingSession = sessionId ? mcpSessionStub(env.MCP_SESSION, sessionId) : null;
219 if (sessionId && !existingSession) {238 if (sessionId && !existingSession) {
220 return jsonRpcResponse(404, -32001, "Session not found");239 return deadSessionResponse(request.method, "Session not found");
221 }240 }
222 if (existingSession) {241 if (existingSession) {
223 const owner = await existingSession.validateMcpSessionOwner({242 const owner = await existingSession.validateMcpSessionOwner({
@@ -225,13 +244,13 @@ export const makeCloudMcpAgentHandler = () => { · 1 unmodified line
225 organizationId: outcome.principal.organizationId,244 organizationId: outcome.principal.organizationId,
226 });245 });
227 if (owner === "not_found") {246 if (owner === "not_found") {
228 return jsonRpcResponse(404, -32001, "Session not found");247 return deadSessionResponse(request.method, "Session not found");
229 }248 }
230 if (owner === "terminated") {249 if (owner === "terminated") {
231 // DELETE-condemned but the deferred destroy alarm hasn't wiped storage250 // DELETE-condemned but the deferred destroy alarm hasn't wiped storage
232 // yet. Same envelope as the post-destroy race below: the client must251 // yet. Same envelope as the post-destroy race below: the client must
233 // treat the id as dead and reconnect.252 // treat the id as dead and reconnect.
234 return jsonRpcResponse(404, -32001, "Session timed out, please reconnect");253 return deadSessionResponse(request.method, "Session timed out, please reconnect");
235 }254 }
236 if (owner === "forbidden") {255 if (owner === "forbidden") {
237 return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer");256 return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer");
@@ -267,7 +286,7 @@ export const makeCloudMcpAgentHandler = () => { · 29 unmodified lines
267 // client to be told to reconnect, matching a timed-out session).286 // client to be told to reconnect, matching a timed-out session).
268 // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: the abort reason is a plain runtime Error whose message IS the signal287 // oxlint-disable-next-line executor/no-unknown-error-message -- adapter boundary: the abort reason is a plain runtime Error whose message IS the signal
269 if (Predicate.isError(error) && error.message === "destroyed") {288 if (Predicate.isError(error) && error.message === "destroyed") {
270 return jsonRpcResponse(404, -32001, "Session timed out, please reconnect");289 return deadSessionResponse(request.method, "Session timed out, please reconnect");
271 }290 }
272 // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: rethrow anything that isn't the condemned-DO abort to the Workers runtime unchanged291 // oxlint-disable-next-line executor/no-try-catch-or-throw -- adapter boundary: rethrow anything that isn't the condemned-DO abort to the Workers runtime unchanged
273 throw error;292 throw error;
apps/host-cloudflare/src/mcp/agent-handler.ts+22−3
@@ -41,6 +41,25 @@ const jsonRpcResponse = ( · 40 unmodified lines
41 ? jsonRpcErrorBody(status, code, message)41 ? jsonRpcErrorBody(status, code, message)
42 : jsonRpcErrorBody(status, code, message, { challenge });42 : jsonRpcErrorBody(status, code, message, { challenge });
43 43
44/**
45 * A dead session id answers by request method. POST/DELETE keep the 404 that
46 * tells a compliant client to re-initialize. A standalone GET gets 405: the
47 * v1 SDK treats that as "no SSE stream offered" and stops retrying quietly,
48 * which breaks the reconnect loops of pre-cutover always-on deployments —
49 * their GET-404 path never re-initialized, it just retried forever.
50 */
51const deadSessionResponse = (method: string, message: string): Response =>
52 method === "GET"
53 ? new Response(JSON.stringify({ jsonrpc: "2.0", error: { code: -32001, message }, id: null }), {
54 status: 405,
55 headers: {
56 "content-type": "application/json",
57 allow: "POST, DELETE",
58 "access-control-allow-origin": "*",
59 },
60 })
61 : jsonRpcResponse(404, -32001, message);
62
44const renderAuthError = (63const renderAuthError = (
45 auth: McpAuthProvider["Service"],64 auth: McpAuthProvider["Service"],
46 request: Request,65 request: Request,
@@ -145,7 +164,7 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { · 98 unmodified lines
145 164
146 const existingSession = sessionId ? mcpSessionStub(env.MCP_SESSION, sessionId) : null;165 const existingSession = sessionId ? mcpSessionStub(env.MCP_SESSION, sessionId) : null;
147 if (sessionId && !existingSession) {166 if (sessionId && !existingSession) {
148 return jsonRpcResponse(404, -32001, "Session not found");167 return deadSessionResponse(request.method, "Session not found");
149 }168 }
150 if (existingSession) {169 if (existingSession) {
151 const owner = await existingSession.validateMcpSessionOwner({170 const owner = await existingSession.validateMcpSessionOwner({
@@ -153,12 +172,12 @@ export const makeCloudflareMcpAgentHandler = (config: CloudflareConfig) => { · 1 unmodified line
153 organizationId: outcome.principal.organizationId,172 organizationId: outcome.principal.organizationId,
154 });173 });
155 if (owner === "not_found") {174 if (owner === "not_found") {
156 return jsonRpcResponse(404, -32001, "Session not found");175 return deadSessionResponse(request.method, "Session not found");
157 }176 }
158 if (owner === "terminated") {177 if (owner === "terminated") {
159 // DELETE-condemned but the deferred destroy alarm hasn't wiped storage178 // DELETE-condemned but the deferred destroy alarm hasn't wiped storage
160 // yet; the terminated id must read as dead immediately.179 // yet; the terminated id must read as dead immediately.
161 return jsonRpcResponse(404, -32001, "Session timed out, please reconnect");180 return deadSessionResponse(request.method, "Session timed out, please reconnect");
162 }181 }
163 if (owner === "forbidden") {182 if (owner === "forbidden") {
164 return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer");183 return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer");
packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts+28−0
@@ -576,4 +576,32 @@ describe("McpAgentSessionDOBase session serving", () => { · 575 unmodified lines
576 error: { code: -32001, message: "Session not found" },576 error: { code: -32001, message: "Session not found" },
577 });577 });
578 });578 });
579
580 it("answers a dead-session standalone GET with 405 so old clients stop retrying", async () => {
581 const state = new MemoryDurableObjectState();
582 await state.storage.put("session-meta", {
583 organizationId: ORGANIZATION_ID,
584 organizationName: "Old Agent Org",
585 userId: ACCOUNT_ID,
586 resource: defaultMcpResource,
587 } satisfies SessionMeta);
588 const session = new HarnessSession(state, {} as Cloudflare.Env);
589 const request = verifiedRequest(
590 new Request("https://executor.test/mcp", {
591 method: "GET",
592 headers: {
593 accept: "text/event-stream",
594 "mcp-session-id": SESSION_ID,
595 },
596 }),
597 );
598
599 const response = await session.fetch(request);
600
601 expect(response.status).toBe(405);
602 expect(response.headers.get("allow")).toBe("POST, DELETE");
603 await expect(response.json()).resolves.toMatchObject({
604 error: { code: -32001, message: "Session not found" },
605 });
606 });
579});607});
packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts+20−2
@@ -298,6 +298,24 @@ type QueuedTransportMessage = { · 297 unmodified lines
298 readonly extra?: MessageExtraInfo;298 readonly extra?: MessageExtraInfo;
299};299};
300 300
301/**
302 * Dead-session answer by method: POST/DELETE keep the 404 that drives client
303 * re-initialization; a standalone GET gets 405, which the v1 SDK reads as
304 * "no SSE stream offered" and stops retrying — breaking the reconnect loops
305 * of pre-cutover deployments whose GET-404 path never re-initialized.
306 */
307const deadSessionDoResponse = (method: string): Response =>
308 method === "GET"
309 ? new Response(
310 JSON.stringify({
311 jsonrpc: "2.0",
312 error: { code: -32001, message: "Session not found" },
313 id: null,
314 }),
315 { status: 405, headers: { "content-type": "application/json", allow: "POST, DELETE" } },
316 )
317 : jsonRpcErrorBody(404, -32001, "Session not found", { cors: false });
318
301export abstract class McpAgentSessionDOBase<319export abstract class McpAgentSessionDOBase<
302 Env extends Cloudflare.Env = Cloudflare.Env,320 Env extends Cloudflare.Env = Cloudflare.Env,
303 TDbHandle extends SessionDbHandle = SessionDbHandle,321 TDbHandle extends SessionDbHandle = SessionDbHandle,
@@ -1182,7 +1200,7 @@ export abstract class McpAgentSessionDOBase< · 878 unmodified lines
1182 return this.serializedTransportRequest(async () => {1200 return this.serializedTransportRequest(async () => {
1183 const transport = this.transport;1201 const transport = this.transport;
1184 if (!transport) {1202 if (!transport) {
1185 return jsonRpcErrorBody(404, -32001, "Session not found", { cors: false });1203 return deadSessionDoResponse(request.method);
1186 }1204 }
1187 if (request.method === "GET") {1205 if (request.method === "GET") {
1188 const lastEventId = request.headers.get("last-event-id");1206 const lastEventId = request.headers.get("last-event-id");
@@ -1282,7 +1300,7 @@ export abstract class McpAgentSessionDOBase< · 93 unmodified lines
1282 if (!stored) {1300 if (!stored) {
1283 if (!isInitializeBody(parsedBody)) {1301 if (!isInitializeBody(parsedBody)) {
1284 return request.headers.has("mcp-session-id")1302 return request.headers.has("mcp-session-id")
1285 ? jsonRpcErrorBody(404, -32001, "Session not found", { cors: false })1303 ? deadSessionDoResponse(request.method)
1286 : jsonRpcErrorBody(400, -32000, "Bad Request: Server not initialized", {1304 : jsonRpcErrorBody(400, -32000, "Bad Request: Server not initialized", {
1287 cors: false,1305 cors: false,
1288 });1306 });
packages/hosts/mcp/src/envelope.test.ts+37−0
@@ -252,6 +252,43 @@ describe("McpServingRoutes envelope", () => { · 251 unmodified lines
252 });252 });
253 });253 });
254 254
255 it("answers a dead-session standalone GET with 405 so old clients stop retrying", async () => {
256 const NotFoundStoreLive = Layer.succeed(McpSessionStore)({
257 dispatch: (): Effect.Effect<McpDispatchResult> => Effect.succeed("not-found"),
258 dispose: () => Effect.void,
259 });
260 const handler = buildHandler(NotFoundStoreLive, McpErrorReporterNoop);
261
262 const get = await handler(
263 new Request("https://host.test/mcp", {
264 method: "GET",
265 headers: {
266 authorization: "Bearer x",
267 accept: "text/event-stream",
268 "mcp-session-id": "dead-session",
269 "mcp-protocol-version": "2025-06-18",
270 },
271 }),
272 );
273 expect(get.status).toBe(405);
274 expect(get.headers.get("allow")).toBe("POST, DELETE");
275 expect(await get.json()).toMatchObject({ error: { code: -32001 } });
276
277 const post = await handler(
278 new Request("https://host.test/mcp", {
279 method: "POST",
280 headers: {
281 authorization: "Bearer x",
282 "content-type": "application/json",
283 "mcp-session-id": "dead-session",
284 "mcp-protocol-version": "2025-06-18",
285 },
286 body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }),
287 }),
288 );
289 expect(post.status).toBe(404);
290 });
291
255 it("404s a modern request whose toolkit route is not served", async () => {292 it("404s a modern request whose toolkit route is not served", async () => {
256 const handler = buildHandler(OkStoreLive, McpErrorReporterNoop);293 const handler = buildHandler(OkStoreLive, McpErrorReporterNoop);
257 const response = await handler(modernRequest("https://host.test/mcp/toolkits/unknown/extra"));294 const response = await handler(modernRequest("https://host.test/mcp/toolkits/unknown/extra"));
packages/hosts/mcp/src/envelope.ts+26−6
@@ -238,11 +238,29 @@ const renderAuthError = ( · 237 unmodified lines
238 Match.exhaustive,238 Match.exhaustive,
239 );239 );
240 240
241/** Render a non-`Response` {@link McpDispatchResult} discriminant. */241/**
242const renderDispatchError = (lookup: "not-found" | "forbidden"): Response =>242 * Render a non-`Response` {@link McpDispatchResult} discriminant. A dead
243 lookup === "not-found"243 * session answers by method: POST/DELETE keep the 404 that drives client
244 ? jsonRpcResponse(404, -32001, "Session not found")244 * re-initialization; a standalone GET gets 405, which the v1 SDK reads as
245 : jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer");245 * "no SSE stream offered" and stops retrying — breaking pre-cutover
246 * reconnect loops whose GET-404 path never re-initialized.
247 */
248const renderDispatchError = (lookup: "not-found" | "forbidden", method: string): Response => {
249 if (lookup === "forbidden") {
250 return jsonRpcResponse(403, -32003, "MCP session does not belong to the current bearer");
251 }
252 if (method === "GET") {
253 return new Response(
254 JSON.stringify({
255 jsonrpc: "2.0",
256 error: { code: -32001, message: "Session not found" },
257 id: null,
258 }),
259 { status: 405, headers: { "content-type": "application/json", allow: "POST, DELETE" } },
260 );
261 }
262 return jsonRpcResponse(404, -32001, "Session not found");
263};
246 264
247const withModernMcpCors = (response: Response): Response => {265const withModernMcpCors = (response: Response): Response => {
248 const headers = new Headers(response.headers);266 const headers = new Headers(response.headers);
@@ -398,7 +416,9 @@ const mcpDispatch = (resource: McpResource, modern: ModernMcpRouter) => · 149 unmodified lines
398 sessionId,416 sessionId,
399 method: request.method,417 method: request.method,
400 });418 });
401 return fromWebResponse(result instanceof Response ? result : renderDispatchError(result));419 return fromWebResponse(
420 result instanceof Response ? result : renderDispatchError(result, request.method),
421 );
402 });422 });
403 423
404/**424/**