feat(ai-mcp): forward clientOptions to the MCP SDK client - #1070
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe MCP client API now accepts optional MCP SDK ChangesMCP client options
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant E2ETest
participant MCPRoute
participant MCPClient
participant MCPValidator
E2ETest->>MCPRoute: Send rejectStructuredOutput=true
MCPRoute->>MCPClient: Create client with clientOptions
MCPClient->>MCPValidator: Validate structured tool output
MCPValidator-->>MCPClient: Return rejection error
MCPClient-->>E2ETest: Emit TOOL_CALL_RESULT with validator error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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.
Actionable comments posted: 1
🤖 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/ai-mcp/src/client.ts`:
- Around line 110-117: The MCPClientImpl restart metadata currently omits
clientOptions, causing reconstructed clients to lose custom settings such as
jsonSchemaValidator. Update MCPClientImpl.getInfo() to retain clientOptions in
its restart descriptor, then have createMcpAppCallHandler forward that
descriptor value when constructing the replacement Client.
🪄 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: af5c54ac-22d7-4488-9b6a-1e089d912033
📒 Files selected for processing (7)
.changeset/mcp-client-options.mdpackages/ai-mcp/src/client.tspackages/ai-mcp/src/types.tspackages/ai-mcp/tests/client.test.tspackages/ai-mcp/tests/helpers/in-memory-server.tstesting/e2e/src/routes/api.mcp-test.tstesting/e2e/tests/mcp.spec.ts
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/ai-mcp/src/pool.ts`:
- Line 155: Update the public return type of MCPClients.getServers() to include
the optional clientOptions field alongside transport and prefix, matching the
descriptor returned by the implementation and allowing TypeScript consumers to
access or forward it.
In `@packages/ai-mcp/tests/client.test.ts`:
- Line 353: Update the assertion for client.getInfo() so it verifies the
returned object shape omits clientOptions entirely, rather than merely checking
that its value is undefined. Preserve the existing getInfo() expectations while
asserting the expected object directly.
🪄 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: ff221a24-28d6-46a8-afc9-32a594d28beb
📒 Files selected for processing (5)
packages/ai-mcp/src/apps/call-handler.tspackages/ai-mcp/src/apps/session-store.tspackages/ai-mcp/src/client.tspackages/ai-mcp/src/pool.tspackages/ai-mcp/tests/client.test.ts
`MCPClients.getServers()` returns `clientOptions` since the previous commit, but its declared return type did not, so a TypeScript consumer could not read or forward it. `createMcpAppCallHandler` reads pools through exactly this method — the value was there at runtime while the public type said it was not. Also pins the omission contract in the `getInfo()` test. `toBeUndefined()` passes whether the key is absent or present-and-undefined, and the implementation deliberately omits it so a descriptor round-trips unchanged; `toStrictEqual` is what tells the two apart. Both reported by CodeRabbit on TanStack#1070.
`createMCPClient` and `createMCPClientFromTransport` built their SDK `Client`
with `new Client({ name, version })` and no second argument, so nothing in
`ClientOptions` was reachable from this package.
The one that matters is `jsonSchemaValidator`. The SDK validates a tool's
`structuredContent` against its declared `outputSchema`, and its default AJV
provider compiles each schema by building JavaScript source and handing it to
`new Function`. Edge runtimes forbid that: on Cloudflare Workers, connecting to
any MCP server whose tools declare an `outputSchema` fails with `Error
compiling schema` — AJV's wrapper around `Code generation from strings
disallowed for this context`.
That fails a whole run rather than one tool call. Validators are built in
`cacheToolMetadata` during `tools/list`, so the throw lands in discovery, before
the model has seen a single tool.
The SDK already ships the fix — `CfWorkerJsonSchemaValidator`, backed by the
optional peer `@cfworker/json-schema` — and its `ClientOptions` docblock
documents installing it. It was simply not installable through this package.
```ts
import { CfWorkerJsonSchemaValidator } from '@modelcontextprotocol/sdk/validation/cfworker'
const mcp = await createMCPClient({
transport: { type: 'http', url: 'https://mcp.example.com/mcp' },
clientOptions: { jsonSchemaValidator: new CfWorkerJsonSchemaValidator() },
})
```
Additive and optional: omitting it keeps the SDK's defaults, which the third
unit test pins. `createMCPClients` picks it up through `MCPClientOptions` for
free.
E2E installs a validator that REFUSES everything, which is what makes the
pass-through observable — the mock server's `get_guitar_price` returns
`structuredContent` the default provider accepts, so an option dropped on the
floor would leave the run indistinguishable from the existing test.
`createMcpAppCallHandler` rebuilds a client per call from `getInfo()`, and that descriptor carried only `transport` and `prefix`. So a client created with `clientOptions` served widget tool calls through a REBUILT client that had none — back on the SDK's AJV validator, which is the failure the option exists to avoid, reintroduced for every MCP Apps call on an edge runtime. `getInfo()` now reports the options the client was built with, the descriptor carries them, and the handler forwards them. `MCPClients.getServers()` reports them the same way, since the handler reads pools through it. Optional on the return type rather than required: a hand-rolled `MCPClient` would otherwise stop compiling, and it is omitted entirely when the client was built without options, so a descriptor round-trips unchanged.
`MCPClients.getServers()` returns `clientOptions` since the previous commit, but its declared return type did not, so a TypeScript consumer could not read or forward it. `createMcpAppCallHandler` reads pools through exactly this method — the value was there at runtime while the public type said it was not. Also pins the omission contract in the `getInfo()` test. `toBeUndefined()` passes whether the key is absent or present-and-undefined, and the implementation deliberately omits it so a descriptor round-trips unchanged; `toStrictEqual` is what tells the two apart. Both reported by CodeRabbit on TanStack#1070.
fad5f41 to
5586c27
Compare
|
View your CI Pipeline Execution ↗ for commit 5586c27
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-codex
@tanstack/ai-cohere
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-daytona
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-isolate-quickjs-bun
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
Changes
createMCPClientandcreateMCPClientFromTransportnow acceptclientOptions, forwarded verbatim to the MCP SDK'sClient. They previously built it asnew Client({ name, version })with no second argument, so nothing inClientOptionswas reachable from this package.The option that motivated this is
jsonSchemaValidator.The SDK validates a tool's
structuredContentagainst its declaredoutputSchema, and its default AJV provider compiles each schema by building JavaScript source and handing it tonew Function. Edge runtimes forbid that. On Cloudflare Workers, connecting to any MCP server whose tools declare anoutputSchemafails withError compiling schema: const schema0 = scope.schema[0]; return function validate0(data, …— AJV's wrapper aroundCode generation from strings disallowed for this context.It takes down a whole run rather than one tool call: validators are built in
cacheToolMetadataduringtools/list, so the throw lands in discovery, before the model has seen a single tool. In practice that meanschat({ mcp })fails on the first turn for every operator who has an MCP server connected.The SDK already ships the fix —
CfWorkerJsonSchemaValidator, backed by the optional peer@cfworker/json-schema— and its ownClientOptionsdocblock documents installing it. It was simply not installable through this package.Additive and optional — omitting it keeps the SDK's defaults.
createMCPClientspicks it up throughMCPClientOptionsfor free.Found while running an operator agent on Cloudflare Workers against Sentry / Stripe / Cloudflare Observability MCP servers, all of which declare
outputSchemaon their tools.Test plan
Unit —
packages/ai-mcp/tests/client.test.ts, against a new in-memory server helper whose tool declares anoutputSchemaand returnsstructuredContent(the only path that reaches a validator):forwards a custom jsonSchemaValidator to the SDK client— a recording provider is consulted, and the assertion is ontools()rather thancallTool, which is where the SDK actually builds validators.accepts clientOptions through createMCPClient— same, through the public factory.falls back to the SDK default when no clientOptions are given— the payload still round-trips, so the addition changes nothing for existing callers.E2E —
testing/e2e/tests/mcp.spec.ts+ an opt-in flag onapi.mcp-test:clientOptions reaches the SDK client — a custom validator changes the outcomeSame mock server, same aimock fixture, one
forwardedPropsflag. The route installs a validator that REFUSES everything throughclientOptions.get_guitar_pricereturnsstructuredContentthe default AJV provider accepts, so an option dropped on the floor would leave the run indistinguishable from the existing test. With it wired,TOOL_CALL_RESULTcarriesMCP error -32602: Structured content does not match the tool's output schema: rejected by the injected validatorand not the payload.The negative assertion is scoped to the tool result rather than the transcript on purpose: the fixture's final answer is a recorded script that names the price whether or not the tool succeeded.
Reproduce:
pnpm test:pr—@tanstack/ai-mcpis clean acrosstest:lib/test:types/test:eslint/test:build. Two unrelated tasks fail on this machine and fail identically on a cleanmain(verified by stashing):@tanstack/ai-sandbox-local-process:test:lib(3 reaper-conformance cases shelling out tostat -c '%Y %n', which is GNU syntax and not what macOSstataccepts) and one case in@tanstack/ai:test:lib.pnpm formatrun; no lockfile changes (no dependencies added).Checklist
pnpm run test:pr.🚀 Release Impact
minoron@tanstack/ai-mcp— new opt-in public API, backwards compatible.Summary by CodeRabbit
New Features
Bug Fixes