Skip to content

Commit 9fd342e

Browse files
authored
fix(mcp): tolerate legacy session meta with no resource (#1210)
Sessions persisted in Durable Object storage before scoped toolkits added the `resource` field to SessionMeta deserialize with `resource: undefined`. On reconnect, owner validation calls `mcpResourceKey(sessionMeta.resource)`, which read `.kind` off undefined and threw `TypeError: Cannot read properties of undefined (reading 'kind')`, breaking every reconnect to a pre-toolkits session (~33 distinct sessions/day in prod). Every such session was minted against the default `/mcp` endpoint, so a missing resource is the default resource: - Backfill `resource` to `defaultMcpResource` when loading stored session meta, healing legacy data at the storage boundary. - Harden `mcpResourceKey` to accept null/undefined and key it to "default", so no caller can throw on a missing resource. Add unit coverage for the guard.
1 parent ffa4f70 commit 9fd342e

3 files changed

Lines changed: 50 additions & 4 deletions

File tree

packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
PAUSED_APPROVAL_TIMEOUT_MS,
1515
type PausedExecutionHooks,
1616
} from "@executor-js/host-mcp/tool-server";
17-
import type { McpResource } from "@executor-js/host-mcp";
17+
import { defaultMcpResource, type McpResource } from "@executor-js/host-mcp";
1818

1919
import type { IncomingPropagationHeaders, McpElicitationMode } from "./do-headers";
2020
import {
@@ -290,7 +290,13 @@ export abstract class McpAgentSessionDOBase<
290290
return Effect.promise(async () => {
291291
if (this.sessionMeta) return this.sessionMeta;
292292
const stored = await this.ctx.storage.get<SessionMeta>(SESSION_META_KEY);
293-
this.sessionMeta = stored ?? null;
293+
// Backfill `resource` for sessions persisted before scoped toolkits added
294+
// the field. Their stored meta has no `resource`, and every such session
295+
// was minted against the default `/mcp` endpoint, so default it here
296+
// rather than let owner validation read `.kind` off undefined.
297+
this.sessionMeta = stored
298+
? { ...stored, resource: stored.resource ?? defaultMcpResource }
299+
: null;
294300
return this.sessionMeta;
295301
}).pipe(Effect.withSpan("mcp.session.load_meta"));
296302
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// ---------------------------------------------------------------------------
2+
// `mcpResourceKey` must never throw on a missing resource.
3+
//
4+
// Sessions persisted before scoped toolkits added the `resource` field
5+
// deserialize with `resource: undefined`. Owner validation keys the stored
6+
// session's resource against the request's, so an unguarded `resource.kind`
7+
// read there threw `TypeError: Cannot read properties of undefined (reading
8+
// 'kind')` on every reconnect to a legacy session. A missing resource is a
9+
// default `/mcp` session, so it must key to "default".
10+
// ---------------------------------------------------------------------------
11+
12+
import { describe, expect, it } from "@effect/vitest";
13+
14+
import { defaultMcpResource, mcpResourceKey } from "./index";
15+
16+
describe("mcpResourceKey", () => {
17+
it('keys the default resource to "default"', () => {
18+
expect(mcpResourceKey(defaultMcpResource)).toBe("default");
19+
expect(mcpResourceKey({ kind: "default" })).toBe("default");
20+
});
21+
22+
it('keys a toolkit resource to "toolkit:<slug>"', () => {
23+
expect(mcpResourceKey({ kind: "toolkit", slug: "github" })).toBe("toolkit:github");
24+
});
25+
26+
it("treats a missing resource (legacy session meta) as the default key", () => {
27+
expect(mcpResourceKey(undefined)).toBe("default");
28+
expect(mcpResourceKey(null)).toBe("default");
29+
});
30+
31+
it("matches a legacy (missing) resource against an explicit default resource", () => {
32+
// The exact comparison owner validation performs: a reconnect carries the
33+
// default resource, the stored legacy meta carries none, and they match.
34+
expect(mcpResourceKey(undefined)).toBe(mcpResourceKey(defaultMcpResource));
35+
});
36+
});

packages/hosts/mcp/src/seams.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,12 @@ export type McpResource =
7272

7373
export const defaultMcpResource: McpResource = { kind: "default" };
7474

75-
export const mcpResourceKey = (resource: McpResource): string =>
76-
resource.kind === "default" ? "default" : `toolkit:${resource.slug}`;
75+
// Tolerates a missing resource (null/undefined): sessions persisted before
76+
// scoped toolkits existed have no `resource` field, and every such session was
77+
// minted against the default `/mcp` endpoint, so a missing resource keys to
78+
// "default". Keeping this guard here means no caller can throw on legacy data.
79+
export const mcpResourceKey = (resource: McpResource | null | undefined): string =>
80+
resource && resource.kind !== "default" ? `toolkit:${resource.slug}` : "default";
7781

7882
// ---------------------------------------------------------------------------
7983
// AuthOutcome — the result of `McpAuthProvider.authenticate`.

0 commit comments

Comments
 (0)