Migrate the mcp/ worker to the v2 MCP SDK (stateless dual-era) - #3760
Migrate the mcp/ worker to the v2 MCP SDK (stateless dual-era)#3760chelojimenez wants to merge 5 commits into
Conversation
The `@mcpjam/mcp` Cloudflare Worker was the last v1 holdout in the monorepo: it extended `McpAgent` from `agents@0.5` and imported `@modelcontextprotocol/sdk@1.26.0`. `McpAgent` is deprecated and feature-frozen in `agents@0.20`; the endorsed replacement is a stateless per-request server built from a factory. Serving is now `createMcpHandler` from `@modelcontextprotocol/server` directly (not the `agents/mcp/server` wrapper — we already own routing, auth, landing and well-known in `index.ts`, and the raw handler skips the wrapper's localhost-only Origin allowlist that would 403 every browser client on a custom domain). One factory serves both eras: the modern 2026-07-28 revision, and 2025-era Streamable HTTP clients through the default `legacy: "stateless"` posture. Consequences: - The MCPJam inspector's v2 client now negotiates the modern era against this server instead of silently falling back to legacy, with zero inspector-side changes. - The Durable Object is gone, which dissolves two open security-audit findings for free: no sessions to hijack, and no bearer tokens persisted at rest in DO storage. - The ~228-line two-mode `sessionToolRegistrar` and the `cf-mcp-message` header-sniffing `onConnect` hack both disappear — they existed only because the DO built its tool list once, before it knew the client's capabilities. Notable decisions: - MCP Apps UI `_meta` is now always advertised. Statelessly a legacy `tools/list` arrives with no memory of the `initialize` capabilities, so per-request gating is impossible. Always-advertise is a SHOULD deviation from SEP-1865; the MUST (a meaningful `content` array) still holds and `_meta.ui` is inert for non-UI hosts. This also fixes the `resources/read` widget path, which used to break whenever UI was not negotiated because the `ui://` resource got disabled outright. - The guest-token cache moves from DO instance state to an isolate-global Map keyed by client IP, mirroring the existing `jwksCache`. Best-effort by design — the inspector's mint route is the authoritative rate limiter. - `index.ts` now owns the `/mcp` CORS contract, including the OPTIONS preflight: the v2 handler is deliberately validation-free and emits no CORS headers, where `McpAgent.serve()` used to do both. - Wrangler migrations are append-only, so `staging` keeps its applied `v1` entry and adds `v2` with `deleted_classes`. `dev`, `preview` and `production` drop migrations entirely — no DO was ever created there. `@modelcontextprotocol/sdk` stays a dependency for the browser widget bundle only (`src/ui`, via ext-apps' peer); the worker runtime no longer imports it, and the root `check:mcp-v1-runtime-imports` guard now covers `mcp/src` outside `src/ui` to keep it that way. Verified against a local `wrangler dev --env dev`: the SDK client manager negotiates 2026-07-28 and lists all 44 tools; clients pinned to 2025-06-18 / 2025-03-26 / 2025-11-25 get the same catalog statelessly with no session id and a spec-compliant 405 on GET/DELETE; the `ui://` resources read back as `text/html;profile=mcp-app`; an anonymous client mints exactly one guest token lazily on first tool execution (none at connect or `tools/list`) and reuses it; a verified bearer is passed through verbatim with no mint; and an unreachable mint route degrades to a clean tool error rather than a crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ARLzF6yHBkZzZyc1gMTSv
GitHub Actions
Inactive
dosubot
Bot
added
size:XL
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_751e7ffe-bea0-4269-886e-c4793162750d) |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
MCP worker previewPreview URL: https://mcpjam-mcp-pr-3760.marcelo-1cb.workers.dev |
Internal previewPreview URL: https://mcp-inspector-pr-3760.up.railway.app |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe MCP worker now uses the v2 server package and creates stateless MCP servers per request. Durable Object bindings, sessions, and migrations are removed from runtime configuration. The entrypoint adds MCP CORS handling and converts verified authentication data into Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcp/wrangler.jsonc (1)
51-61: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winConfirm per-PR preview workers before merging
The open preview workers share one
mcpjam-mcp-pr-*name for the life of each PR. An earlier preview push can have already applied thev1migration to that worker; thepreviewenv does not declare any migrations orMcpJamMcpServerclass, so the next preview deploy will migrate that worker to empty while leaving the migration history behind. Delete stale per-PR workers or be prepared to fix the first failed preview deploy after this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/wrangler.jsonc` around lines 51 - 61, Before merging, verify and clean up existing per-PR preview workers using the mcpjam-mcp-pr-* naming pattern. Remove stale workers that may retain prior v1 migration history, or ensure the first preview deployment after this change handles the resulting migration failure; do not treat the preview environment as declaring migrations or the McpJamMcpServer class.
🧹 Nitpick comments (1)
mcp/src/index.ts (1)
63-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCarry the token
expthrough asAuthInfo.expiresAt.MCP TypeScript SDK 2.0.0 defines
AuthInfo.expiresAtas seconds since the Unix epoch, and bearer authentication rejects tokens without an expiry. Useverified.payload.expinstead of relying on downstream reads later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp/src/index.ts` around lines 63 - 80, The toAuthInfo function currently omits the verified token expiry, causing bearer authentication to reject the resulting AuthInfo. Extract the exp claim from verified.payload and assign it to AuthInfo.expiresAt as Unix-epoch seconds, preserving the existing token, clientId, scopes, and claims mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcp/README.md`:
- Around line 19-22: Update the Status paragraph in mcp/README.md to document
both authentication paths: authenticated requests use the caller’s AuthKit JWT,
while anonymous requests use the lazily minted guest token described later near
line 224. Replace the “every call” wording so it does not imply anonymous access
is impossible.
In `@mcp/src/server.ts`:
- Around line 144-147: Add a requirePlatformApiUrl helper in server.ts that
validates Env.PLATFORM_API_URL and throws a clear configuration error when
missing, then use its returned value when constructing toolContext.runtimeEnv
instead of the unsafe as Required<Env> cast.
- Around line 156-197: The module-scoped currentEnv binding in handleMcpRequest
is unsafe because mcpHandler.fetch() invokes the factory asynchronously after
request classification. Remove this shared state and pass each request’s Env
through request-level options or create a per-request handler closure, ensuring
the factory used by buildServer receives the Env belonging to that request.
---
Outside diff comments:
In `@mcp/wrangler.jsonc`:
- Around line 51-61: Before merging, verify and clean up existing per-PR preview
workers using the mcpjam-mcp-pr-* naming pattern. Remove stale workers that may
retain prior v1 migration history, or ensure the first preview deployment after
this change handles the resulting migration failure; do not treat the preview
environment as declaring migrations or the McpJamMcpServer class.
---
Nitpick comments:
In `@mcp/src/index.ts`:
- Around line 63-80: The toAuthInfo function currently omits the verified token
expiry, causing bearer authentication to reject the resulting AuthInfo. Extract
the exp claim from verified.payload and assign it to AuthInfo.expiresAt as
Unix-epoch seconds, preserving the existing token, clientId, scopes, and claims
mapping.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b5c5ce15-555f-4ff2-900e-f0c866910fd7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.github/workflows/pr-mcp-preview.ymlmcp/README.mdmcp/package.jsonmcp/src/index.tsmcp/src/server.tsmcp/src/shared/platform-widgets.tsmcp/src/tools/platformTools.tsmcp/src/tools/sessionToolRegistrar.tsmcp/src/tools/showServers.tsmcp/tests/platformTools.test.tsmcp/worker-configuration.d.tsmcp/wrangler.jsoncpackage.json
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 14 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Three valid findings from the automated reviewers on #3760. `mcp-method` / `mcp-name` were missing from the `/mcp` CORS allow-list (cubic, P1). This is a real browser-client breakage, not a nit: the v2 client derives both headers from the message body on *every* modern-era request, so a browser preflight would have been rejected before the worker saw the request. My earlier verification ran over Node, which enforces no CORS, so it could not surface this. The allow-list is now every non-safelisted header the official client sends, `last-event-id` and `accept` included, and is verified by simulating the preflight with `Access-Control-Request-Headers` and asserting each one comes back granted. The module-scoped `currentEnv` binding was unsound (cubic P2, CodeRabbit). `createMcpHandler.fetch()` awaits request classification — body read plus era routing — before it invokes the factory, so a second request could reassign the binding in between and the first would build its server against the wrong bindings. My comment claiming the factory read it "synchronously on the same turn" was simply wrong. The handler is now built per request, closing over that request's `env`. The reason I had avoided this — per-request tool re-registration — does not apply: the factory is invoked once per request under this SDK either way, so all a per-request handler adds is the handler object and its event bus. The isolate-global guest-token cache is unaffected and still shows one mint across three calls. `env as Required<Env>` asserted away a real doubt (CodeRabbit). `PLATFORM_API_URL` is optional on `Env`, and if it were ever unset every tool call would fetch `undefined/projects` — an opaque network error rather than a legible configuration one. Replaced with a `requirePlatformApiUrl` check, mirroring the `AUTHKIT_DOMAIN` / `WORKOS_CLIENT_ID` fail-fast checks already in `index.ts`. Also carried the token's `exp` through as `AuthInfo.expiresAt` so the pass-through stays faithful, and corrected the README Status paragraph, which described the AuthKit JWT as the only bearer and so read as though anonymous access were unsupported. Re-verified against a local `wrangler dev`: modern era still negotiates 2026-07-28 with all 44 tools and 7 `ui://` resources; the three pinned 2025 eras still serve statelessly with 405 on GET/DELETE; anonymous still mints exactly one guest token lazily and reuses it; a verified bearer still passes through verbatim with no mint; and the simulated browser preflight now grants every client header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ARLzF6yHBkZzZyc1gMTSv
GitHub Actions
Inactive
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b68f06d8-ea22-472e-97e7-ea98fb62bcb2) |
|
Pushed Fixed — Fixed — the module-scoped Fixed — Skipping — stale per-PR preview workers with Re-verified against a local Generated by Claude Code |
GitHub Actions
Inactive
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_377c9b73-500f-4c35-9bcb-49588169962e) |
|
That file isn't in this diff. Timeline (UTC):
That also explains why the same check passed on my first commit: it merged against
Generated by Claude Code |
|
1 failure out of 13,059 tests, in Root cause — a time bomb that detonated today. The test builds its fixture with The giveaway is the very next test in the same file: Evidence it isn't mine:
Fix (for whoever owns this surface — mirrors the pattern already used one test below): it("shows per-host verified dates only in public caniuse mode", () => {
const subject = makeSubject(
"preset:claude", "Claude", { hostStyle: "claude" }, "claude",
Date.UTC(2026, 6, 8)
);
+ vi.useFakeTimers();
+ vi.setSystemTime(new Date("2026-07-10T00:00:00.000Z"));
+
const { rerender } = render(<HostConfigComparisonMatrix subjects={[subject]} />);I've left it alone since it's outside this PR's scope, but flagging that it blocks Generated by Claude Code |
Unrelated to this PR's MCP worker migration, but it blocks main and every open PR, so it rides along here. `shows per-host verified dates only in public caniuse mode` builds its fixture with `Date.UTC(2026, 6, 8)` and asserts the literal date string renders, without pinning the clock. The matrix swaps that date for a "Last checked over 30 days ago" warning once the fixture ages past 30 days, so the assertion passed every day since it was written and began failing on 2026-08-07 — exactly 30 days on — on every branch at once. The fix is the pattern the next test in the same file already uses: `vi.useFakeTimers()` plus `vi.setSystemTime()`, pinned to the same 2026-07-10 it picks. The file's `afterEach` already restores real timers, so no other cleanup is needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ARLzF6yHBkZzZyc1gMTSv
GitHub Actions
Inactive
|
Pushed the fix as It's the two-line change from my previous comment: Flagging the scope explicitly: Generated by Claude Code |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a2554668-d553-4f07-b595-8e44435e3ba4) |
…migration-81myc3 # Conflicts: # mcpjam-inspector/client/src/components/hosts/comparison/__tests__/host-config-comparison-matrix.test.tsx
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_913bd1be-2d98-4afe-a58a-ce62aa1735ed) |
2 participants
The
@mcpjam/mcpCloudflare Worker was the last v1 holdout in the monorepo — it extendedMcpAgentfromagents@0.5and imported@modelcontextprotocol/sdk@1.26.0.McpAgentis deprecated and feature-frozen inagents@0.20; the endorsed replacement is a stateless per-request server built from a factory.Serving is now
createMcpHandlerfrom@modelcontextprotocol/server@2.0.0directly (not theagents/mcp/serverwrapper — we already own routing, auth, landing and well-known inindex.ts, and the raw handler skips the wrapper's localhost-only Origin allowlist that would 403 every browser client on a custom domain). One factory serves both eras: the modern 2026-07-28 revision, and 2025-era Streamable HTTP clients through the defaultlegacy: 'stateless'posture.What this buys:
sessionToolRegistrarand thecf-mcp-messageheader-sniffingonConnecthack both disappear — they existed only because the DO built its tool list once, before it knew the client's capabilities.Notable decisions
MCP Apps UI
_metais always advertised now. Statelessly a legacytools/listarrives with no memory of theinitializecapabilities, so per-request gating is impossible. Always-advertise is a SHOULD deviation from SEP-1865; the MUST (a meaningfulcontentarray) still holds, and_meta.uiis inert for non-UI hosts. This also fixes theresources/readwidget path, which used to break whenever UI wasn't negotiated because theui://resource got disabled outright. Novisibility: ["app"]is emitted (we don't today, and it would let hosts hide tools from the model).Guest-token cache moved from DO instance state to an isolate-global
Mapkeyed by client IP, mirroring the existingjwksCacheinauth.ts. Best-effort by design — a cold isolate re-mints — because the inspector's mint route is the authoritative rate limiter (10/min/IP), and its own comment notes worker-side limiting is unreliable. Two anonymous users behind one NAT share a guest identity; acceptable for throwaway anon use. Added a 1000-entry bound with oldest-first eviction so the map can't grow without limit in a long-lived isolate.index.tsnow owns the/mcpCORS contract, including the OPTIONS preflight. The v2 handler is deliberately validation-free and emits no CORS headers, whereMcpAgent.serve()used to do both. Header set is pinned:allow-origin: *,allow-methods: GET, POST, DELETE, OPTIONS,allow-headers: authorization, content-type, mcp-session-id, mcp-protocol-version,expose-headers: mcp-session-id, WWW-Authenticate. The 401 challenge responses get the same headers.Handler is built once at module scope, with the factory reading a module-scoped
currentEnvset at the top offetch— safe in a single-threaded isolate, and it avoids re-registering the 44-tool catalog plus inlined widget HTML on every request.handler.close()is never called, which is safe only under the no-SSE invariant (no subscriptions, no mid-call notifications, soresponseMode: 'auto'never upgrades to a stream and no keepalive interval is ever created). That invariant is stated explicitly in a code comment.Wrangler migrations are append-only, so
stagingkeeps its appliedv1entry and addsv2withdeleted_classes— a lonev2would be rejected against the deployed worker's last-applied tag.dev,previewandproductiondrop migrations entirely:devis local-only,previewworkers are created fresh per PR so they never create the DO, andproductionhas never been deployed.@modelcontextprotocol/sdkstays a dependency, for the browser widget bundle only (src/ui, via ext-apps' peer, which imports it at runtime). The worker runtime no longer imports it, and the rootcheck:mcp-v1-runtime-importsguard now coversmcp/srcoutsidesrc/uito keep it that way. The childoverridesblock is dropped (npm ignored it anyway — only rootoverridesapply, so it never actually pinned the workspace).Changes
mcp/src/server.tsMcpAgentsubclass →createMcpHandlerfactory; module-scope guest cache +getBearerTokenfree functionmcp/src/index.ts/mcpbranch delegates to the v2 handler with pass-throughauthInfo; owns CORS + preflight; DO export removedmcp/src/tools/sessionToolRegistrar.tsregisterTool/registerResourcehelper; allsetUiEnabled/enable/disablemutation gone; 4-arg call shape keptmcp/src/tools/platformTools.ts,showServers.tsagent: McpJamMcpServer→context: PlatformToolContext({ getBearerToken, runtimeEnv })mcp/src/shared/platform-widgets.tsRESOURCE_MIME_TYPE/RESOURCE_URI_META_KEY(byte-identical to ext-apps'), so the worker drops@modelcontextprotocol/ext-apps/servermcp/wrangler.jsonc,worker-configuration.d.tsv1+v2historymcp/README.md,.github/workflows/pr-mcp-preview.ymlmcp/src/auth.tsand its 15 tests are untouched, as areplatformWidgets.test.tsandformat.test.ts.platformTools.test.tsneeded fixture retyping only — every assertion survives.Verification
npm run build -w @mcpjam/sdk,npm run typecheck -w @mcpjam/mcp,npm run test:fast -w @mcpjam/mcp(36/36),check:mcp-v1-runtime-imports(verified it actually catches a planted v1 import inmcp/src),check:bundled-runtime-paths, andnpm ci --dry-runfor lockfile consistency — all green.Driven against a local
wrangler dev --env dev, with a stub inspector standing in for the guest JWKS / mint / platform API:MCPClientManager(the inspector's own client path, era mode left at itsautodefault) negotiates 2026-07-28;tools/listreturns all 44 tools;show_serverscarries both the nested_meta.ui.resourceUriand the flatui/resourceUri; all 7ui://resources list, andresources/readreturnstext/html;profile=mcp-app.2025-06-18,2025-03-26and2025-11-25each negotiate their pin and get the same 44-tool catalog statelessly, with no session id; rawGETandDELETE /mcpreturn a clean405 Method not allowed.rather than a crash.tools/listmints zero guest tokens; the first tool execution mints exactly one; three consecutive calls reuse it (one mint, three API hits all bearing the same token). With the mint route unreachable,runPlatformOperationdegrades to a tool error (No bearer token on the request.), not a crash.OPTIONS /mcp→ 204 with the full pinned header set; an invalid bearer → 401 withWWW-Authenticateand CORS;/mcp/subpath still reaches the handler; every/.well-known/*route and the/landing page (which the CI smoke test greps) are unchanged.Preview-worker deploy is the remaining gate — the
deleted_classesmigration wants a real Cloudflare run before staging picks it up on merge. Prod staysworkflow_dispatchbehind its green-staging gate.Generated by Claude Code
Note
High Risk
This is a core auth/MCP serving rewrite plus a staging DO teardown migration; wrong deploy order or CORS/auth regressions would break all remote MCP clients until rolled back.
Overview
Replaces the
mcp/Cloudflare Worker’sMcpAgent+ Durable Object stack withcreateMcpHandlerfrom@modelcontextprotocol/server@2.0.0, serving modern 2026-07-28 and legacy Streamable HTTP clients vialegacy: "stateless"(no sessions, no tokens at rest).index.tsnow handles/mcpCORS (including OPTIONS) and passes verified JWTs asauthInfointohandleMcpRequest;server.tsbuilds a freshMcpServerper request, moves guest-token mint/cache to an isolate-global IP-keyedMap(lazy on first tool call), and drops the DO class. Tool wiring usesPlatformToolContextinstead ofMcpJamMcpServer.sessionToolRegistraris simplified: widget tools always register MCP Apps_metaandui://resources (no per-session UI gating). MCP Apps constants are inlined inplatform-widgets.tsso the worker runtime drops@modelcontextprotocol/ext-apps/server.wrangler.jsoncremoves DO bindings from all envs; staging adds migrationv2withdeleted_classes: ["McpJamMcpServer"].agentsis removed; rootcheck:mcp-v1-runtime-importsnow coversmcp/src(excluding UI). Docs/CI comments updated for a stateless preview worker.Reviewed by Cursor Bugbot for commit c225799. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Migrated the
@mcpjam/mcpWorker to the v2 MCP server with a stateless per-request handler. Drops the Durable Object, serves both modern (2026-07-28) and 2025-era clients, and stores no sessions or tokens at rest. Also stabilizes a flaky inspector test by pinning the clock.Refactors
createMcpHandlerfrom@modelcontextprotocol/server@2.0.0withlegacy: "stateless"; removedagentsand the Durable Object.index.tsowns/mcprouting and CORS (OPTIONS preflight; GET/DELETE → 405; allowmcp-method,mcp-name,accept,last-event-id)._metaand registerui://resources; tools use aPlatformToolContext; inlined MCP Apps wire constants.AuthInfo.expiresAt.requirePlatformApiUrlfail-fast; kept@modelcontextprotocol/sdkonly for browser widgets and extended the root guard to block v1 imports in worker code; removed DO bindings and added a stagingv2migration withdeleted_classes.Bug Fixes
HostConfigComparisonMatrixtest to prevent date-based flakiness and unblock CI.Written for commit 63db780. Summary will update on new commits.