feat: host-side MCP client (@tanstack/ai-mcp)#700
Conversation
…duplicate detection
…op redundant lazy cast
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@examples/ts-react-chat/src/lib/mcp-servers.ts`:
- Around line 11-29: The three transport factories (everythingTransport,
memoryTransport, sequentialThinkingTransport) call npx with unpinned package
names; change each args entry from '`@modelcontextprotocol/server-`...' to an
exact version string like '`@modelcontextprotocol/server-memory`@<exact-version>'
(and similarly for server-everything and server-sequential-thinking) so npx
installs a deterministic release. Locate the args arrays in everythingTransport,
memoryTransport, and sequentialThinkingTransport and replace the package tokens
with the exact versions used by this project (pull versions from package.json or
the lockfile) before committing.
In `@examples/ts-react-chat/src/routes/api.mcp-chat.ts`:
- Around line 51-95: The route can leak an MCP client if Promise.all rejects
partway; change the client creation so you track and clean up any
successfully-created clients on failure (e.g. use Promise.allSettled on
createMCPClient(...) for the two clients or create them sequentially), inspect
results, and if any creation failed call dispose() (or the client's teardown
method) on each fulfilled client before throwing/returning; ensure the
successful clients are only passed into chat({ mcp: { clients: [...] } }) after
you've validated both were created so MCPManager.discover()/dispose() inside
chat runs correctly.
In `@examples/ts-react-chat/src/routes/api.mcp-manual.ts`:
- Around line 23-24: Reorder the imported symbols so named exports are
alphabetized per the project's sort-imports rule: in the import from
'`@tanstack/ai-mcp`' ensure the members are sorted (createMCPClient,
mcpPromptToMessages, mcpResourceToContentPart) and in the import from
'`@tanstack/ai`' ensure (ModelMessage, StreamChunk) are sorted; update the import
lines that declare createMCPClient, mcpResourceToContentPart,
mcpPromptToMessages and the import that declares StreamChunk, ModelMessage so
their member lists are alphabetically ordered to satisfy ESLint.
- Around line 69-158: The MCP client created by createMCPClient(...) can leak if
an error happens after creation (e.g., during chat(...) or
toServerSentEventsResponse(...)); ensure client.close() is always called on
error paths by moving client declaration into the outer scope and adding a
finally or ensuring the outer catch closes the client if it exists and is not
already closed (reference createMCPClient, client.tools, client.close,
closeMcpOnDrain, chat, toServerSentEventsResponse, abortController); keep
existing early-close where client.tools() fails and still close in the outer
catch/finally so the stdio MCP process is not leaked.
In `@examples/ts-react-chat/src/routes/mcp-demo.tsx`:
- Around line 115-120: The code passes remarkGfm into the rehypePlugins array,
so GFM parsing won't run; update the component props to remove remarkGfm from
rehypePlugins and instead add it to remarkPlugins (i.e., keep rehypePlugins as
[rehypeRaw, rehypeSanitize, rehypeHighlight] and add
remarkPlugins={[remarkGfm]}), ensuring the component uses the remarkPlugins prop
with remarkGfm.
🪄 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: c11c09ae-c0ba-444e-bb41-dcde07e19dca
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
examples/ts-react-chat/package.jsonexamples/ts-react-chat/src/lib/mcp-servers.tsexamples/ts-react-chat/src/routeTree.gen.tsexamples/ts-react-chat/src/routes/api.mcp-chat.tsexamples/ts-react-chat/src/routes/api.mcp-manual.tsexamples/ts-react-chat/src/routes/api.mcp-pool.tsexamples/ts-react-chat/src/routes/mcp-demo.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/ts-react-chat/src/routes/api.mcp-manual.ts (1)
151-156:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't return raw backend error messages to the browser.
This endpoint is directly callable from the demo UI, so echoing
error.messagehere can expose MCP/OpenAI/stdio internals to any caller. Keep the detailed diagnostics inconsole.errorand return a fixed 500 payload instead.Suggested fix
return new Response( - JSON.stringify({ error: error.message || 'An error occurred' }), + JSON.stringify({ error: 'Internal server error' }), { status: 500, headers: { 'Content-Type': 'application/json' }, }, )🤖 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 `@examples/ts-react-chat/src/routes/api.mcp-manual.ts` around lines 151 - 156, The handler currently returns JSON.stringify({ error: error.message || 'An error occurred' }) which exposes internal backend errors to the browser; instead call console.error(error) (or processLogger.error) to record diagnostics and replace the response body with a fixed generic payload such as JSON.stringify({ error: 'Internal Server Error' }) while keeping status: 500 and Content-Type: 'application/json' in the new Response(...) call so no raw error.message is echoed to clients.
🤖 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.
Outside diff comments:
In `@examples/ts-react-chat/src/routes/api.mcp-manual.ts`:
- Around line 151-156: The handler currently returns JSON.stringify({ error:
error.message || 'An error occurred' }) which exposes internal backend errors to
the browser; instead call console.error(error) (or processLogger.error) to
record diagnostics and replace the response body with a fixed generic payload
such as JSON.stringify({ error: 'Internal Server Error' }) while keeping status:
500 and Content-Type: 'application/json' in the new Response(...) call so no raw
error.message is echoed to clients.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd79d535-7a22-4b28-91a0-5e991ce866a8
📒 Files selected for processing (4)
examples/ts-react-chat/src/routes/api.mcp-chat.tsexamples/ts-react-chat/src/routes/api.mcp-manual.tsexamples/ts-react-chat/src/routes/api.mcp-pool.tsexamples/ts-react-chat/src/routes/mcp-demo.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/ts-react-chat/src/routes/api.mcp-pool.ts
- examples/ts-react-chat/src/routes/api.mcp-chat.ts
- examples/ts-react-chat/src/routes/mcp-demo.tsx
# Conflicts: # packages/ai/skills/ai-core/tool-calling/SKILL.md # packages/ai/src/activities/chat/index.ts # pnpm-lock.yaml # testing/e2e/package.json
…p-alive vs close)
…-mcp-prop # Conflicts: # examples/ts-react-chat/src/lib/mcp-providers.ts
…t in examples
Review fixes:
- docs: correct chat() signature (adapter: openaiText('gpt-5.5'), no top-level model)
- docs/skills: fix close-before-consume lifecycle samples — close MCP clients in
middleware terminal hooks (onFinish/onAbort/onError), not try/finally or
await using around a streaming return
- skills: repair truncated/unbalanced code fence corrupting the Provider Skills
section in tool-calling SKILL.md
- example: settle parallel createMCPClient calls in api.mcp-chat.ts and close the
connected sibling on partial failure (no leaked stdio subprocess)
- example: probe capabilities in api.mcp-status.ts instead of catch-all-ing
resources()/prompts()
- ai-mcp: name the tool in MCP isError throws; short-circuit already-aborted
signals; pool.tools() now attributes the failing server by config key
- ai(chat): correct MCPConnectionPolicy JSDoc ('close' = when the run ends;
'keep-alive' = never closed by chat())
- types: collapse redundant AutomaticDescriptor; reword DescribedTool/codegen
claims to name-only typing (args stay untyped on discovery; Mode 2 types args)
New coverage (ai-mcp 30 → 46 tests):
- isError path, abort→callTool forwarding, already-aborted short-circuit,
structuredContent preference, mcpContentToTanstack branches
- replace tautological duplicate-name test with one exercising the real guard
- connect-failure wrapping, double-close idempotency, bound-defs prefix,
pool tools() failure attribution, stdio smoke test, authProvider forwarding
- types.test-d.ts pins the descriptor name-literal guarantee
Docs additions:
- Authentication section (headers + OAuth authProvider + finishAuth caveat) in
mcp.md and the ai-mcp skill
- all MCP route examples converted from Next.js App Router to TanStack Start
(createFileRoute + server handlers); no Next.js references remain in PR files
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…licit binding Newer MCP servers (e.g. @modelcontextprotocol/server-everything's simulate-research-query) mark tools with `execution.taskSupport: 'required'` (experimental MCP tasks). Plain `callTool` — what @tanstack/ai-mcp uses — is rejected by the server with -32600, so offering these tools to the model guarantees a failed tool call. - tools(): auto-discovery now filters task-required tools via the new `requiresTaskExecution()` guard — the model is never offered a tool that cannot succeed - tools([defs]): explicitly binding a task-required tool throws the new `MCPTaskRequiredToolError` (exported) with guidance pointing at the SDK's tasks API - tests: in-memory server helper registering a real task-required tool; discovery-exclusion + binding-throws coverage (48 tests) - e2e: in-process MCP server registers a task-required tool; spec asserts it never reaches the discovered tool list - docs: Mode 1 exclusion callout, Mode 2 error mention, Error Reference row in mcp.md; matching error-list updates in the ai-mcp skill Actual task-based execution support (client.experimental.tasks.callToolStream) is tracked in #704. Also includes import-order/lint cleanup in examples/ts-react-chat api.mcp-manual.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- rename mcp-chat.md → mcp-managed.md ("Managed MCP with chat()") and
mcp-with-chat.md → mcp-manual.md ("Manual MCP: typed tools, resources &
prompts") — the old slugs differed only by a preposition while the real
distinction is managed vs manual lifecycle; new names match the vocabulary
already used by the e2e/example routes (api.mcp-managed-test, api.mcp-manual)
- update all ids, cross-links, link texts, and docs/config.json nav labels
- Quick Start (mcp.md) now leads with the managed mcp option (zero lifecycle
code) instead of the manual middleware-close pattern; Lifecycle section
opens with the managed escape hatch before the manual rules
- mcp-managed.md: keep one full route example (plus keep-warm, where module
vs handler placement is the point); convert the other five examples to
focused fragments showing just the client setup + chat() call; drop the
repeated 8-line body-validation boilerplate (449 → 306 lines)
- reorder sidebar/frontmatter to adoption order:
mcp → mcp-managed → mcp-manual → mcp-codegen
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
||
| /** Load mcp.config.ts (via jiti) or mcp.config.json from cwd. */ | ||
| export async function loadConfig(cwd: string): Promise<MCPCodegenConfig> { | ||
| const { existsSync } = await import('node:fs') |
There was a problem hiding this comment.
I had to look into why you had these dynamic imports. I get it. It's the one package for the bin and the lib. If you'd imported at module level it would have added node and other dependencies into the library. It looks weird, but I'm happy with it. It's clever to be able to include the cli in the same package.
…ples - emit.ts: guarantee valid/unique generated identifiers, escape string literals via JSON.stringify when emitting TypeScript - introspect.ts: move connect() inside try/finally, guard close() from masking the original error, drain cursor-paginated list endpoints - prompts.ts: never produce undefined content (JSON.stringify(x ?? null)) - manager.ts: await onDiscoveryError so async handlers fail fast - chat: combine caller + middleware abort signals so ctx.abort() reaches running tools via ctx.abortSignal (regression test added) - mcp routes (e2e + example): close MCP client on non-stream error paths, bridge request aborts during setup - mcp-demo/threads: pass remark-gfm via remarkPlugins (not rehypePlugins) - tests: fix import/order lint errors, assert pool cleanup on failure, cover hostile-name escaping and interface collisions in emit Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tombeckenham
left a comment
There was a problem hiding this comment.
This is great work. Really. I added a few small changes and cleaned up the docs. Ship it!
Summary
Adds
@tanstack/ai-mcp— a host-side Model Context Protocol client — so a server-sidechat()/ agent loop can discover and run tools (and read resources/prompts) from external MCP servers, across any adapter, with optional generated end-to-end types.Built on the official
@modelcontextprotocol/sdk. The runtime stays edge-deployable (Streamable HTTP isnode:-free); the Node-only stdio transport is isolated behind a@tanstack/ai-mcp/stdiosubpath, and the codegen CLI's heavy deps are bundled into the bin only (never the library).How it works
An MCP client turns a server into ready-to-spread
ServerTool[]; you spread them intochat({ tools }). TanStack AI never knows MCP is involved.What's included
createMCPClient— connect to one server. Transports: Streamable HTTP (edge-safe), SSE, and stdio (via@tanstack/ai-mcp/stdio), plus a user-supplied-transport escape hatch.createMCPClients— multi-server pool: parallel connect, auto-prefixed tool names (collision-free by default), close-all, typed per-server access.client.tools()— auto-discovery (argsunknown).client.tools([toolDefinition(...)])— bind TanStacktoolDefinition()s → typed + runtime-validated, allowlisted (reuses the existing tool primitive; no parallel schema system).createMCPClient<GeneratedServer>(...)— generated end-to-end types vianpx @tanstack/ai-mcp generate+mcp.config.ts(emits per-server descriptors + a combinedMCPServersmap; pure types, zero runtime cost).resources()/readResource()/resourceTemplates(),prompts()/getPrompt(), plusmcpResourceToContentPart/mcpPromptToMessagesconverters for seedingchat().close()+[Symbol.asyncDispose](await using);chat()never closes the client (warm reuse supported).@tanstack/ai) — adds an optionalabortSignaltoToolExecutionContextand threads the chat run's signal through tool execution, so long-running tools (e.g. MCPcallTool) cancel with the run. Additive/backward-compatible.Testing & docs
testing/e2e): a real in-process MCP server + chat route + Playwright spec proving an MCP tool executes insidechat()and its result reaches the streamed transcript.docs/tools/mcp.md(+ nav/cross-links), updatedtool-callingskill + a newai-mcpskill, and a changeset (minor:@tanstack/ai-mcp,@tanstack/ai).Notes for reviewers
examples/typecheck failures (ts-solid-chat/ts-react-chat/ts-code-mode-web, missingfetcherfrom docs(chat): document thefetcherchat transport for server functions #681) are unrelated to this PR and excluded from thetest:prgate.🤖 Generated with Claude Code
Summary by CodeRabbit