feat(mcp): @orpc/mcp — serve one oRPC router as an MCP server (tools/resources/prompts)#1604
feat(mcp): @orpc/mcp — serve one oRPC router as an MCP server (tools/resources/prompts)#1604mi3lix9 wants to merge 13 commits into
Conversation
Adds the mcp() meta plugin (mirrors openapi()) plus a native MCPHandler that exposes opted-in procedures as MCP tools, resources, and prompts over stdio and Streamable HTTP (fetch/node). Reuses @orpc/json-schema converters for inputSchema/outputSchema and the procedure client for dispatch. - meta.ts: mcp(), mcp.tool/resource/prompt, getMCPMeta - registry.ts: opt-in router walk -> tools/resources/prompts definitions - content/error/uri-template: encoding + URI templates + error planes - adapters: standard dispatcher + fetch/node/stdio handlers - integration test: 15 passing; type:check + lint clean Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
Covers meta merge, content/error encoding, URI templates, registry classification, and fetch/node/stdio adapter round-trips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
- playground: annotate planet procedures with mcp.tool() and mount MCPHandler at /mcp over the same router (one API -> RPC + OpenAPI + MCP) - docs: add integrations/mcp.md + sidebar entry; README package list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
…, json body typing) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
…l/sdk client Drives the node MCPHandler over real HTTP with the canonical MCP SDK client (v1.29.0): initialize handshake, tools list/call (incl. in-band typed error), static + templated resources, and prompts. The SDK validates every response against its own schemas, proving real protocol compliance (not just self-consistency). Added @modelcontextprotocol/sdk as a dev dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
…idation) A live run with the official SDK client surfaced that returning an error payload in `structuredContent` makes clients reject the result (-32602) when the tool declares an `outputSchema` — the client validates structuredContent against the success schema. Errors now return only `content` + `isError: true`. Adds an SDK e2e regression: a schema-typed tool that errors must not violate its outputSchema. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces the @orpc/mcp package, which allows developers to expose oRPC routers as Model Context Protocol (MCP) servers supporting tools, resources, and prompts over Fetch, Node.js, and stdio transports. The changes include core registry and mapping logic, transport adapters, comprehensive test suites, and documentation. Feedback on the implementation highlights a high-severity security vulnerability in the Node.js adapter's readBody function, which lacks request size limits (exposing the server to memory exhaustion DoS) and can corrupt multi-byte UTF-8 characters. Additionally, a performance improvement is recommended to process JSON-RPC batch requests concurrently using Promise.all instead of sequentially.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
I don't like this approach. We should utilize the existing standard handler and standard request/response flow. For MCP-specific routes like |
…ed plugin Addresses PR middleapi#1604 review (@dinwwwh): MCP now uses oRPC's standard handler and request/response flow instead of a bespoke JSON-RPC dispatcher. - MCPHandlerCodec (StandardHandlerCodec): resolves tools/call, resources/read, prompts/get to a procedure and runs it through the standard pipeline. - MCPHandlerPlugin (StandardHandlerPlugin): auto-registered routing interceptor that early-responds for protocol routes (initialize, ping, list, completion) and frames the JSON-RPC envelope for procedure results. - fetch/node adapters extend FetchHandler/NodeHttpHandler; stdio drives the same StandardHandler via a synthesized request. One code path, all transports. Security & robustness (PR review + MCP SDK study): - Removes hand-rolled node readBody -> body reading/limits come from the adapter + BodyLimitHandlerPlugin (fixes DoS + multi-byte UTF-8 corruption). - Adds Origin/Host (DNS-rebinding) validation (enableDnsRebindingProtection + allowedOrigins/allowedHosts; missing Origin passes; reject -> 403). - stdio enforces a max message length. - Fixes a bug found via the official-SDK e2e: the body was read twice (resolveBody consumes the stream) -> memoize the parsed envelope per request. Drops JSON-RPC batching (incompatible with one-request/one-procedure; deprecated in the spec direction) -> rejected with -32600. 66 tests pass incl. the official @modelcontextprotocol/sdk e2e; type:check + lint clean. Docs + Next playground updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
…ed plugin Addresses PR middleapi#1604 review (@dinwwwh): MCP now uses oRPC's standard handler and request/response flow instead of a bespoke JSON-RPC dispatcher. - MCPHandlerCodec (StandardHandlerCodec): resolves tools/call, resources/read, prompts/get to a procedure and runs it through the standard pipeline. - MCPHandlerPlugin (StandardHandlerPlugin): auto-registered routing interceptor that early-responds for protocol routes (initialize, ping, list, completion) and frames the JSON-RPC envelope for procedure results. - fetch/node adapters extend FetchHandler/NodeHttpHandler; stdio drives the same StandardHandler via a synthesized request. One code path, all transports. Security & robustness (PR review + MCP SDK study): - Removes hand-rolled node readBody -> body reading/limits come from the adapter + BodyLimitHandlerPlugin (fixes DoS + multi-byte UTF-8 corruption). - Adds Origin/Host (DNS-rebinding) validation (enableDnsRebindingProtection + allowedOrigins/allowedHosts; missing Origin passes; reject -> 403). - stdio enforces a max message length. - Fixes a bug found via the official-SDK e2e: the body was read twice (resolveBody consumes the stream) -> memoize the parsed envelope per request. Drops JSON-RPC batching (incompatible with one-request/one-procedure; deprecated in the spec direction) -> rejected with -32600. 66 tests pass incl. the official @modelcontextprotocol/sdk e2e; type:check + lint clean. Docs + Next playground updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
|
Thanks for the steer @dinwwwh — reworked it around the standard handler as requested. Pushed in Architecture (per your feedback)
stdio drives the same Review findings addressed
Bonus: a real bug the e2e caughtI added an end-to-end test that drives the node handler with the official Status: 66 tests pass (incl. the official-SDK e2e), |
bf9665a to
0db6a51
Compare
…only
Removes the callable mcp({ ... }) form; the primitive is now always chosen via
mcp.tool() / mcp.resource() / mcp.prompt() (per PR feedback — one obvious way,
better DX). mcp.resource() is typed to require exactly one of uri | uriTemplate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
…on docs - Add opaque-cursor pagination to tools/list, resources/list, resources/templates/list, prompts/list via a configurable `pageSize` (default 100, so small catalogs are unchanged). Invalid cursor -> -32602. - Docs: Authorization is the app's job (context + middleware example); result pagination is the developer's job (tool input/output example); catalog pagination is built in. Reframe limitations (sessions/GET-SSE are being removed in the next MCP revision, so statelessness is intentional). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
Prose-led structure cloned from the other integration pages (Installation code-group, Setup, per-primitive sections, Serving, Authorization, Security, Limitations) with :::warning/tip callouts. Drops the pagination section (internal detail) and the meta-options/how-it-maps reference tables in favor of inline explanations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
packages/mcp/src/adapters/stdio/mcp-handler.test.ts (1)
43-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for
maxMessageLength.The suite never exercises the over-limit branch, so transport-level size-limit regressions—especially whitespace-padded inputs—can slip through unnoticed.
🤖 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/mcp/src/adapters/stdio/mcp-handler.test.ts` around lines 43 - 101, Add a regression test in mCPHandler (stdio) that exercises the maxMessageLength over-limit path. Extend the existing drive/createHandler test setup with an input whose payload exceeds the limit, including a whitespace-padded case, and assert the handler emits the expected transport error instead of processing the message. Use the existing helpers and response assertions in mcp-handler.test.ts to keep the test aligned with the current initialize/tools/list coverage.
🤖 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 `@apps/content/docs/integrations/mcp.md`:
- Around line 167-172: The multi-surface example is incomplete because
`MCPHandler` is instantiated without the schema converters that are required
when the router uses Zod-backed schemas. Update the `handlers` example to pass
the same converter setup used elsewhere on the page into `new
MCPHandler(router)` so the MCP surface matches the documented requirement.
In `@packages/mcp/src/adapters/standard/mcp-handler-plugin.ts`:
- Line 101: The page size handling in MCPHandlerPlugin currently accepts
fractional values, which later breaks pagination between the `slice`,
`nextCursor`, and `decodeCursor` flow. Update the `MCPHandlerPlugin` option
validation so `pageSize` is only accepted when it is a positive integer, and
fall back to `DEFAULT_LIST_PAGE_SIZE` otherwise; make the same validation
consistent anywhere `pageSize` is read or used in the affected pagination paths.
- Around line 297-313: The security check in checkSecurity currently fails open
when enableDnsRebindingProtection is on but allowedOrigins or allowedHosts is
undefined, because present Origin/Host headers are skipped instead of rejected.
Update checkSecurity in mcp-handler-plugin so that when DNS-rebinding protection
is enabled, any present Origin or Host value must be explicitly allowlisted, and
if the corresponding allowlist is undefined or the value is not included, return
false. Keep the existing behavior that returns true only when no protected
header is present or it matches the configured allowlist.
- Around line 143-157: The incoming JSON-RPC validation in mcp-handler-plugin.ts
is too permissive for payload.id, allowing non-primitive ids like objects or
booleans to reach response framing. Tighten the validation path in
isValidIncoming and the request handling around the id extraction so only valid
JSON-RPC id types are treated as requests; otherwise return Invalid Request
before calling frameProcedure or any response builder. If null ids are meant to
be supported, update frameProcedure and any related request types to accept that
explicitly and keep the validation consistent with the JSON-RPC rules.
In `@packages/mcp/src/adapters/stdio/mcp-handler.ts`:
- Around line 68-73: The DoS guard in mcp-handler’s line handling is checking
the trimmed content instead of the original buffered line, so oversized requests
padded with whitespace can bypass maxMessageLength. In the logic around the
line-processing block, validate the raw line length before calling trim(), and
use the existing INVALID_REQUEST response path to reject anything over the limit
while still allowing blank lines to be skipped after the length check.
In `@packages/mcp/src/content.ts`:
- Around line 4-6: `isPlainRecord` is too broad because it treats `Date` and
class instances as records, which can leak non-plain objects into
`structuredContent` when `hasOutputSchema` is used. Tighten `isPlainRecord` in
`content.ts` to accept only real plain objects (for example, by checking the
prototype is `Object.prototype` or `null`) and make sure the `hasOutputSchema`
path that builds `structuredContent` uses this stricter guard so only
object-shaped JSON-compatible values are passed through.
- Around line 23-25: The content serialization in content.ts is treating empty
content-block arrays as plain text because the Array.isArray guard requires
length > 0. Update the logic in the content conversion path so that any array
consisting entirely of valid content blocks, including [], is returned as {
content: output } rather than being stringified, and keep the existing
isContentBlock check in the same branch.
In `@packages/mcp/src/registry.ts`:
- Around line 101-107: The MCP schema conversion path is dropping composed
schemas from convertSchemas when inputSchemas or outputSchemas contain multiple
entries, because the current object-only checks reject the returned allOf
wrapper. Update registry.ts handling in the tool input/output and prompt
argument paths to preserve composed schemas by accepting allOf via
asObjectJsonSchema instead of only plain object schemas, and merge object
members from allOf when building prompt arguments so converted metadata is not
reduced to a generic object or emptied.
- Around line 86-112: The registry currently overwrites existing MCP entries
when names or URIs collide, which can silently expose the wrong
tool/prompt/resource. Add a uniqueness guard in the registration path around
registry.tools.set, registry.prompts.set, and registry.resources.set (for
example via a shared helper like setUnique) so duplicate meta.name,
defaultName(path), or fixed uri values throw an error instead of replacing the
earlier entry. Keep the existing ToolDefinition/PromptDefinition/resource
construction flow in registry.ts, but perform the duplicate check immediately
before inserting each entry.
In `@packages/mcp/src/sdk-compliance.test.ts`:
- Around line 82-86: The test setup in sdk-compliance.test.ts is binding the
server and client to different loopback families, which causes the connection to
fail. Update the server startup in the test so the `server.listen` call and the
`StreamableHTTPClientTransport` URL use the same host family, either both on
`127.0.0.1` or both on `::1`, and keep the `Client` connection logic aligned
with that choice.
In `@packages/mcp/src/types.ts`:
- Around line 11-46: `JSONRPCId` is narrower than the actual incoming envelope
shape, so `isValidIncoming()` can accept a request with a null `id` even though
`JSONRPCRequest` and `JSONRPCIncoming` do not allow it. Update the wire types in
`types.ts` to either include `null` in `JSONRPCId`/`JSONRPCRequest` or, if null
IDs should not be supported, tighten the `isValidIncoming()` guard used before
`frameProcedure()` to reject them so the runtime behavior matches the declared
contract.
In `@playgrounds/next/package.json`:
- Line 22: Move `@orpc/mcp` from devDependencies to dependencies in the package
manifest because runtime code in the Next playground imports it from route.ts
and planet.ts. Update the package.json entry so production installs include it,
and keep the change aligned with the existing package declaration structure.
In `@playgrounds/next/src/app/mcp/`[[...rest]]/route.ts:
- Around line 6-9: Enable the DNS-rebinding guard on the MCP App Router endpoint
by wiring the new Origin/Host validation into the route’s MCPHandler setup.
Update the handler initialization in the route that mounts MCPHandler so the
browser-facing endpoint enforces the guard for requests before they reach router
handling, using the same validation mechanism added elsewhere in this PR. Refer
to the MCPHandler construction and the route handler export in this file to
apply the check consistently.
---
Nitpick comments:
In `@packages/mcp/src/adapters/stdio/mcp-handler.test.ts`:
- Around line 43-101: Add a regression test in mCPHandler (stdio) that exercises
the maxMessageLength over-limit path. Extend the existing drive/createHandler
test setup with an input whose payload exceeds the limit, including a
whitespace-padded case, and assert the handler emits the expected transport
error instead of processing the message. Use the existing helpers and response
assertions in mcp-handler.test.ts to keep the test aligned with the current
initialize/tools/list coverage.
🪄 Autofix (Beta)
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
Run ID: 35a2800c-cb75-4064-9d0f-1fb6786edb6f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
README.mdapps/content/.vitepress/config.tsapps/content/docs/integrations/mcp.mdpackages/mcp/package.jsonpackages/mcp/src/adapters/fetch/index.tspackages/mcp/src/adapters/fetch/mcp-handler.test.tspackages/mcp/src/adapters/fetch/mcp-handler.tspackages/mcp/src/adapters/node/index.tspackages/mcp/src/adapters/node/mcp-handler.test.tspackages/mcp/src/adapters/node/mcp-handler.tspackages/mcp/src/adapters/standard/index.tspackages/mcp/src/adapters/standard/mcp-handler-codec.tspackages/mcp/src/adapters/standard/mcp-handler-plugin.tspackages/mcp/src/adapters/standard/mcp-pagination.test.tspackages/mcp/src/adapters/standard/utils.tspackages/mcp/src/adapters/stdio/index.tspackages/mcp/src/adapters/stdio/mcp-handler.test.tspackages/mcp/src/adapters/stdio/mcp-handler.tspackages/mcp/src/constants.tspackages/mcp/src/content.test.tspackages/mcp/src/content.tspackages/mcp/src/error.test.tspackages/mcp/src/error.tspackages/mcp/src/index.tspackages/mcp/src/meta.test.tspackages/mcp/src/meta.tspackages/mcp/src/registry.test.tspackages/mcp/src/registry.tspackages/mcp/src/sdk-compliance.test.tspackages/mcp/src/types.tspackages/mcp/src/uri-template.test.tspackages/mcp/src/uri-template.tspackages/mcp/tsconfig.jsonplaygrounds/next/package.jsonplaygrounds/next/src/app/mcp/[[...rest]]/route.tsplaygrounds/next/src/routers/planet.ts
There was a problem hiding this comment.
Important
@orpc/mcp is a strong foundation, but several spec-compliance and security gaps should be closed before this lands on main: Origin validation is off by default, the MCP lifecycle is not enforced, and a handful of wire-protocol details (MCP-Protocol-Version, the completion/complete capability, JSON-RPC id handling, error payloads) deviate from MCP 2025-11-25.
Reviewed changes — adds a new MCP package that serves an oRPC router as a 2025-11-25 MCP server over fetch/node HTTP and stdio, with opt-in mcp.tool / mcp.resource / mcp.prompt meta annotations.
- Adds
packages/mcpwithStandardHandler-basedMCPHandlerPlugin/MCPHandlerCodec. - Implements tools/resources/prompts list/call/read/get, lifecycle negotiation, pagination, and error mapping.
- Provides
fetch,node, andstdiotransport adapters and a Next.js playground route. - Adds docs, unit tests, and an e2e SDK compliance test driven by the official
@modelcontextprotocol/sdkclient.
⚠️ Lifecycle is not enforced
route() dispatches every method unconditionally, so a client can call tools/list, tools/call, etc. before initialize. The MCP lifecycle spec states that initialization MUST be the first interaction. The handler needs per-session handshake state, and non-lifecycle methods should be rejected until the handshake completes.
⚠️ DNS-rebinding protection is off by default
Streamable HTTP requires servers to validate the Origin header by default, yet enableDnsRebindingProtection defaults to false. Even when enabled, both allowedOrigins and allowedHosts are optional, so leaving either unset silently disables that half of the check. The inline comment on the default has more context.
⚠️ The MCP-Protocol-Version header is ignored
route() reads the JSON-RPC body but never inspects the MCP-Protocol-Version HTTP header. The spec requires validating it on subsequent HTTP requests (falling back to 2025-03-26 when absent, and returning 400 for unsupported values).
⚠️ Several protocol details don't match MCP 2025-11-25
completion/completeis answered, but thecompletionscapability is never advertised.resources/readwith a missing or non-stringurireturns-32002instead of-32602.- Out-of-bounds catalog cursors silently return an empty page instead of
-32602. - JSON-RPC
id: null/ object / array values flow through instead of being rejected or treated as notifications.
⚠️ Error responses can leak typed-error payloads
For resources and prompts, orpcError.toJSON() is placed verbatim in error.data, so any application data passed to ORPCError(..., { data }) is exposed to MCP clients. The inline comment points to the affected line.
⚠️ Registry can silently overwrite or drop definitions
registry.tools.set(name, ...), registry.resources.set(meta.uri, ...), and registry.prompts.set(name, ...) never detect collisions. Two procedures with the same explicit name (or default names that collide after defaultName truncation) hide each other. Separately, multi-schema outputs are merged into { allOf }, which isObjectJsonSchema rejects, so the tool outputSchema is silently dropped. Inline comments mark both sites.
⚠️ stdio transport lacks DoS and error boundaries
The stdio loop has no per-byte limit before readline buffers a line, no message-rate limit, no JSON parse-depth limit, and handler.handle rejections are unhandled. A malformed or hostile client can crash the reader or exhaust process memory. Inline comments point to the affected code.
ℹ️ Nitpicks
packages/mcp/src/adapters/standard/utils.ts:13-15exportsgetMessageId, but it is unused; either use it to normalize JSON-RPC ids or remove it.packages/mcp/src/adapters/fetch/mcp-handler.ts:39-43andnode/mcp-handler.ts:34-38spreadoptions(includingconverters) into bothStandardHandlerand the underlying transport handler, which leaks an internal option into the base handler contract.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ The docs rewrite is cleaner and more consistent with the rest of the site. Two small gaps worth a look.
Reviewed changes since last review — rewrites apps/content/docs/integrations/mcp.md in the standard docs style.
- Replaces the ad-hoc structure with Tools/Resources/Prompts/Serving/Authorization/Security sections.
- Updates install command tag from
@latestto@beta. - Tightens examples and removes the previous Pagination and Notes & limitations sections.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ness) Verified each CodeRabbit/pullfrog finding against the code; fixed the real ones: - security: checkSecurity no longer fails open — enabling DNS-rebinding protection without an allowlist now throws (was a silent no-op). - registry: throw on duplicate tool/prompt names and resource URIs/templates instead of silently overwriting. - registry: preserve composed (allOf) schemas from multiple .input()/.output() so tool inputSchema and prompt arguments are not dropped. - pagination: reject non-integer pageSize; reject stale/out-of-range cursors with -32602. - protocol: validate the JSON-RPC id type (reject object/boolean/null); resources/read with a missing/non-string uri returns -32602 (was -32002); removed the completion/complete stub (the capability was never advertised). - stdio: check raw line length before trimming (whitespace-padding bypass); wrap dispatch in try/catch so one failure cannot tear down the reader. - docs: the multi-surface example now passes converters. Adds 14 regression tests. Skipped findings (false positives / opinion / by design) are explained on the PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
There was a problem hiding this comment.
Important
dec3d6e closes most of the prior review feedback, but the DNS-rebinding guard still does not match its documented behavior and asObjectJsonSchema silently drops JSON Schema keywords when merging allOf.
Reviewed changes — the dec3d6e follow-up commit that hardens the MCP handler against prior review feedback.
- Fixed JSON-RPC id validation —
isValidIncomingnow rejectsnull/object/array ids. - Hardened catalog pagination —
pageSizemust be a positive integer; stale/out-of-range cursors return-32602. - Removed unadvertised
completion/complete— the stub route was deleted. - Added duplicate-name guards — registry build now throws on colliding tool/resource/prompt names.
- Preserved composed schemas —
asObjectJsonSchemamergesallOfobject members. - Improved stdio resilience — raw line length is checked before trimming;
handler.handleerrors no longer crash the reader. - Fixed
resources/readerror codes — missing/non-stringurireturns-32602. - Added
mcp-hardening.test.ts— covers id validation, pagination, DNS rebinding, error codes, and registry integrity.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
Resolve conflicts: - README.md: keep the @orpc/mcp entry plus main's corrected @orpc/next line. - pnpm-lock.yaml: take main's lockfile (already includes the @orpc/mcp importer); frozen-lockfile install passes. Note: the repo-wide `lint_and_typecheck` is red because of a pre-existing undici-types/FormData type error on main (origin/main e9cc1ee also fails it); @orpc/mcp itself type-checks clean, and lint + the full test suite pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwXhtSxKx3fpdk3b9mEttL
|
@dinwwwh please review the changes and tell me if we need any extra change, I have tested this on my machine with Claude Code and it works fine! |
|
I'm still getting familiar with MCP, so this PR is taking me a bit longer. |
Take your time bro, for me I used the official MCP website as a reference, and test it in Claude Code. |
| return jsonRpc(403, null, { error: { code: FORBIDDEN_ERROR, message: 'Origin not allowed' } }) | ||
| } | ||
|
|
||
| // 2. MCP uses HTTP POST. (GET SSE / DELETE sessions are not implemented yet.) |
There was a problem hiding this comment.
You can check the batch plugin to see how to handle batching or long-lived sessions. (In short, you can call for simple just ignore itnext multiple times)
Also, in oRPC, SSE is an async iterator (EventStreamBody) from Standard Server, so it works for both request and response bodies: https://github.com/middleapi/standardserver/#event-stream-body
| uri?: `${string}://${string}` | undefined | ||
|
|
||
| /** Resource-only: RFC 6570-style URI template (e.g. `planet://{id}`); vars map to input. */ | ||
| uriTemplate?: string | undefined |
There was a problem hiding this comment.
Why not merge uri and uriTemplate into one?
| return jsonRpc(200, id, { result }) | ||
| } | ||
| catch (error) { | ||
| const jsonRpcError = error instanceof JSONRPCError |
There was a problem hiding this comment.
- Why not handle errors in
mcp-handler-codecinstead of try/catch here? - Instead of relying on
JSONRPCError, why not just useORPCError?
| * Read + parse the JSON-RPC body of an MCP request (once per request). | ||
| * The returned promise rejects if the body is not valid JSON. | ||
| */ | ||
| export function readMCPPayload(request: StandardLazyRequest): Promise<unknown> { |
There was a problem hiding this comment.
WARNING: anti pattern, request instance can be changed during lifecycle
| @@ -0,0 +1,75 @@ | |||
| { | |||
| "name": "@orpc/mcp", | |||
There was a problem hiding this comment.
I think we should use the name @orpc/experimental-mcp. There's no way this will be stable anytime soon.

Summary
@orpc/mcpexposes an oRPC router as a Model Context Protocol server — the same procedures you serve over RPC and OpenAPI become MCP tools / resources / prompts, with the same types, validation, and middleware. Opt-in viamcp.tool/mcp.resource/mcp.prompt.Built on the standard handler (per review)
Reworked from the original standalone dispatcher to use oRPC's
StandardHandler(@dinwwwh's feedback):MCPHandlerCodecresolvestools/call/resources/read/prompts/getand runs them through the normal procedure pipeline (middleware, validation, context, plugins).MCPHandlerPlugin(auto-registered) is a routing interceptor that early-responds for the protocol routes (initialize,ping, thelistmethods,completion) and frames the JSON-RPC envelope.fetch/nodeextendFetchHandler/NodeHttpHandler;stdiodrives the same handler via a synthesized request. One code path, all transports — and anyStandardHandlerplugin (CORS, body-limit, OTel) composes.Review items addressed
readBodyis gone — body reading/limits now come from the adapter +BodyLimitHandlerPlugin(fixes the DoS + multi-byte UTF-8 corruption). Added optional Origin/DNS-rebinding validation. Both bot threads resolved.mcp.tool/resource/prompt(no callablemcp());mcp.resourceis typed to requireuri | uriTemplate.listmethod (opaque cursor, configurablepageSize, invalid cursor →-32602).Conformance — MCP
2025-11-25Implemented: lifecycle + capability negotiation,
ping, tools/resources/prompts (list/call/read/get), pagination, typed errors (in-band tool errors vs JSON-RPC),structuredContent/outputSchema.Deliberately not implemented because they are being removed/replaced in the next revision (checked against the draft changelog): sessions (
Mcp-Session-Id), theGETSSE channel,listChanged/subscribe. The stateless POST→response design is intentional and forward-aligned.Non-blocking follow-ups (SHOULD/MAY): honor
notifications/cancelled(currently a no-op),_meta/progress, binary resource contents, realcompletion/complete.Authorization
By design, authentication/authorization is the application's job via oRPC context + middleware (documented) — the package stays unopinionated about tokens, scopes, or OAuth.
Testing
Repo-wide CI gates pass with these changes:
type:check,lint, and 1961 tests — including an end-to-end suite driven by the official@modelcontextprotocol/sdkclient over real HTTP, plus unit/adapter/pagination coverage. Wired into the Next playground (/mcp) and documented atdocs/integrations/mcp.One decision for the maintainer
Positioning: keep as
@orpc/mcp, or ship as an experimental package while the MCP spec is mid-revision (the next revision is a large one)? Happy to rename either way.🤖 Generated with Claude Code
Summary by CodeRabbit
@orpc/mcpwith MCP server handlers for fetch/web, Node.js, and stdio.