Serve /mcp on dual protocol lanes: stateless 2026-07-28 + instrumented legacy, with CIMD and tool metadata - #1237
Merged
Conversation
…etadata - Route /mcp by protocol era after auth: 2025-era requests stay on the sessionful McpAgent Durable Object lane; 2026-07-28 envelope requests are served per-request by a stateless SDK v2 server (no DO, no session) - Record one Analytics Engine data point per authenticated /mcp request (lane, method, protocol version, client info) so legacy-lane retirement is a metrics decision; deliberately not stored in the primary D1 - Share one tool registration across both server generations; advertise loose outputSchema on search/execute (both lanes) and tool icons (modern lane; SDK v1 ignores the key) - Enable OAuth Client ID Metadata Documents alongside DCR - ADR 0005 documents the dual-lane design and retirement policy Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThe worker adds a stateless MCP lane for protocol revision ChangesMCP dual-lane serving
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPAuth
participant ProtocolMetrics
participant StatelessLane
participant McpAgent
participant AnalyticsEngine
Client->>MCPAuth: Authenticated /mcp request
MCPAuth->>ProtocolMetrics: Classify request
ProtocolMetrics-->>MCPAuth: Legacy or modern lane
MCPAuth->>AnalyticsEngine: Record protocol event
alt Modern 2026-07-28 request
MCPAuth->>StatelessLane: Handle per-request MCP server
StatelessLane-->>Client: MCP response
else Legacy request
MCPAuth->>McpAgent: Forward to sessionful lane
McpAgent-->>Client: MCP response
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
kody-bot
marked this pull request as ready for review
August 5, 2026 14:51
Contributor
|
🔎 Preview deployed: https://kody-pr-1237.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/worker/src/index.ts`:
- Around line 430-432: Narrow the authentication wording so it does not promise
fallback after a failed CIMD fetch: update packages/worker/src/index.ts lines
430-432 and docs/contributing/architecture/authentication.md lines 476-478 to
state that clients may present a CIMD client_id, while clients that do not use
CIMD register through DCR; remove the claim that CIMD-capable clients skip
registration or that other clients fall back after CIMD failure.
In `@packages/worker/src/mcp-auth.ts`:
- Around line 273-293: The modern branch in the MCP dispatch must bypass
withAccountWriteLease and call handleStatelessMcpRequest directly; retain the
lease-wrapped path only for legacy requests, and enforce account-deletion
protection at the relevant mutating tool boundary. In
packages/worker/src/mcp-auth.ts:273-293, update the dispatch accordingly. In
docs/contributing/architecture/request-lifecycle.md:54-63 and
docs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.md:32-34,
retain the no-Durable-Object claims only if this bypass is implemented;
otherwise revise them to document the account Durable Object hop.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 300f6de9-a2eb-4b0f-a2f1-d4fa1072888c
📒 Files selected for processing (23)
docs/contributing/architecture/authentication.mddocs/contributing/architecture/data-storage.mddocs/contributing/architecture/request-lifecycle.mddocs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.mddocs/contributing/decisions/index.mdpackages/worker/src/env-schema.tspackages/worker/src/index.tspackages/worker/src/mcp-auth.tspackages/worker/src/mcp-auth.workers.test.tspackages/worker/src/mcp/index.tspackages/worker/src/mcp/mcp-registration-agent.tspackages/worker/src/mcp/protocol-metrics.node.test.tspackages/worker/src/mcp/protocol-metrics.tspackages/worker/src/mcp/stateless-lane.mcp-e2e.test.tspackages/worker/src/mcp/stateless-lane.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/mcp/tools/output-schemas.node.test.tspackages/worker/src/mcp/tools/search-register.tspackages/worker/src/mcp/tools/search-tool-definition.tspackages/worker/src/mcp/tools/tool-icons.tspackages/worker/worker-configuration.d.tspackages/worker/wrangler.jsonctools/mcp-test-support.ts
Comment on lines
273
to
+293
| return await withAccountWriteLease({ | ||
| db: env.APP_DB, | ||
| stableUserId: mcpUser.userId, | ||
| holder: `mcp:${request.method} ${url.pathname}`, | ||
| env, | ||
| write: async () => | ||
| await fetchMcp( | ||
| request, | ||
| env, | ||
| context as ExecutionContext<OAuthContextProps>, | ||
| ), | ||
| classification.lane === 'legacy' | ||
| ? await fetchMcp( | ||
| request, | ||
| env, | ||
| context as ExecutionContext<OAuthContextProps>, | ||
| ) | ||
| : await handleStatelessMcpRequest({ | ||
| request, | ||
| env, | ||
| ctx, | ||
| callerContext: props, | ||
| ...(classification.parsedBody === undefined | ||
| ? {} | ||
| : { parsedBody: classification.parsedBody }), | ||
| }), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Remove the account Durable Object from the modern request path.
handleStatelessMcpRequest runs inside withAccountWriteLease. The supplied lease implementation calls acquireDoAccountWriteLeaseAndWrite. Every modern request therefore depends on an account Durable Object. This defeats the stated stateless, no-Durable-Object lane design and adds a lease availability dependency to tools/list and other read-only requests.
packages/worker/src/mcp-auth.ts#L273-L293: Dispatch the modern branch outsidewithAccountWriteLease. Apply any required account-deletion protection at the specific mutating tool boundary.docs/contributing/architecture/request-lifecycle.md#L54-L63: Keep the no-Durable-Object statement only after the modern branch bypasses the lease. Otherwise document the account Durable Object hop.docs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.md#L32-L34: Keep the no-Durable-Object consequence only after the modern branch bypasses the lease. Otherwise revise the decision record.
📍 Affects 3 files
packages/worker/src/mcp-auth.ts#L273-L293(this comment)docs/contributing/architecture/request-lifecycle.md#L54-L63docs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.md#L32-L34
🤖 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 `@packages/worker/src/mcp-auth.ts` around lines 273 - 293, The modern branch in
the MCP dispatch must bypass withAccountWriteLease and call
handleStatelessMcpRequest directly; retain the lease-wrapped path only for
legacy requests, and enforce account-deletion protection at the relevant
mutating tool boundary. In packages/worker/src/mcp-auth.ts:273-293, update the
dispatch accordingly. In
docs/contributing/architecture/request-lifecycle.md:54-63 and
docs/contributing/decisions/0005-mcp-dual-lane-stateless-migration.md:32-34,
retain the no-Durable-Object claims only if this bypass is implemented;
otherwise revise them to document the account Durable Object hop.
… server fallback Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
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
3 participants
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.
Intent
MCP protocol revision
2026-07-28made the protocol stateless, and Cloudflare deprecated and feature-frozeMcpAgent— the Durable Object that hosts kody's/mcptoday. Adopt the new revision without breaking a single existing client, and make retirement of the old path a metrics decision instead of a guess: instrument which lane every request uses so we know exactly when legacy traffic is gone.Summary
/mcpbehind the existing route and auth: after bearer-token validation,handleMcpRequestclassifies each request with the SDK's ownisLegacyRequestpredicate. 2025-era requests keep the sessionfulMcpAgentDurable Object lane byte-identically;2026-07-28envelope requests are served by a per-request stateless SDK v2 server (packages/worker/src/mcp/stateless-lane.ts) withlegacy: 'reject'so exactly one lane owns each era. Both lanes deliberately stay insidewithAccountWriteLease— that lease is the deletion-safety boundary every kody surface takes (MCP, app, jobs, package invocations), not MCP session state; "stateless" here means no MCP session Durable Object.MCP_PROTOCOL_EVENTSAnalytics Engine dataset (packages/worker/src/mcp/protocol-metrics.ts). The retirement readout is a pure aggregate; the query is documented in the module header. No-op without the binding, never throws.registerToolsnow takes a shared registration surface (McpToolServer); the SDK v1/v2 generic mismatch is bridged at one documented seam (asMcpToolServer). The DO lane forwardsstate/setStatelive so session niceties (search preamble dedup, raw-fetch nudge accumulation) are unchanged; the stateless lane degrades those to per-call behavior, which the runners already handle.search/executenow advertise a deliberately looseoutputSchema(all fields optional, compound valuesunknown— server-side output validation can never reject a real response; covered by tests) on both lanes, and icons (2025-11-25 SEP-973) on the modern lane (SDK v1 ignores the config key).clientIdMetadataDocumentEnabled: trueon the OAuth provider — clients may present an HTTPS URL asclient_idwith no registration step; clients that do not use CIMD register via DCR (/oauth/register), which stays enabled per spec. A failed CIMD fetch returnsinvalid_client; any DCR retry is the client's own recovery. The requiredglobal_fetch_strictly_publiccompat flag was already set.server/discover, never fortools/call.Deliberately not in scope: the tasks extension (SDK v2 ships vocabulary only, no runtime; execute's idempotency-key +
run_getalready covers it), sampling/roots/MCP-logging (deprecated in 2026-07-28; kody never adopted them), and legacy-lane removal itself (that waits on the metrics this PR adds).Testing
npm run validate— all gates green except one pre-existing failure unrelated to this change:packages/worker/src/email/inbound-due-owners.workers.test.tsfails identically on unmodifiedmainin the same VM (time-dependent assertion; main CI passed today at 14:00 UTC).stateless-lane.mcp-e2e.test.ts): a real SDK v2 client pinned to 2026-07-28 completes OAuth, negotiates viaserver/discover, lists tools (asserting output schemas + icons), and runs a realsearchcall. Existing v1-client e2e tests keep passing untouched, proving the legacy lane is unaffected.mcp-auth.workers.test.ts: legacyinitializeroutes to the DO lane, a modern envelope request is served statelessly without touching the DO, and both record the expected Analytics Engine data points.protocol-metrics.node.test.ts) and output-schema safety (output-schemas.node.test.tsruns every structured-response shape through the advertised schemas exactly as the SDK validates them).System changes
System recap — extends mcp-server and mcp-oauth (medium risk)
Mode: recap · Base:
main@865304b0· Head:e806211cClassification: extends — the
/mcpsurface gains a second (stateless 2026-07-28) serving lane and per-request lane metrics; MCP OAuth gains CIMD client registration. No new primitive;primitives.yamlunchanged.Primitives touched
mcp-servermcp-oauthmcp-auth.ts; CIMD enabled beside DCRscheduled-cronindex.tsby root only; the actual edit there is the OAuth provider CIMD optionSystem map
Authenticated
/mcptraffic forks by protocol era into the existing Durable Object lane or the new stateless lane, and every request drops a lane data point into Analytics Engine.Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Change flow
Invariants
compact-mcp-surfaceupheld: both lanes register exactly the same two tools (search,execute) from one shared registration; no per-capability tools added.Summary by CodeRabbit
New Features
2026-07-28stateless MCP protocol alongside legacy traffic.Documentation
Tests