feat(ai-mcp): support task-based tool execution#940
Conversation
|
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:
📝 WalkthroughWalkthroughMCP task-required tools are discovered and executed through the SDK’s experimental task stream. Optional-task tools continue using standard calls, with capability gating, abort handling, refreshed discovery, end-to-end coverage, a task execution example, and updated documentation. ChangesMCP task execution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ChatClient
participant TasksChatRoute
participant MCPClient
participant TasksMCPServer
ChatClient->>TasksChatRoute: POST chat request
TasksChatRoute->>MCPClient: create task-aware MCP client
MCPClient->>TasksMCPServer: listTools()
MCPClient->>TasksMCPServer: tasks.callToolStream()
TasksMCPServer-->>MCPClient: task status and terminal result
MCPClient-->>TasksChatRoute: tool execution output
TasksChatRoute-->>ChatClient: server-sent events
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ai-mcp/tests/helpers/in-memory-server.ts (1)
72-99: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDuplicated
registerToolTaskboilerplate; verify SDK actually invokesgetTask/getTaskResult. Both sites register a task-required tool with an identicalcreateTask(synchronously stores a'completed'result)/getTask/getTaskResulttriple. BecausecreateTaskcompletes the task synchronously, these tests can pass even if the SDK never callsgetTask/getTaskResult— which is exactly the bug fixed by@modelcontextprotocol/sdkPR#1335for the v1.x line ("registerToolTask's getTask and getTaskResult handlers not being invoked").
packages/ai-mcp/tests/helpers/in-memory-server.ts#L72-L99: confirm the installed@modelcontextprotocol/sdkversion actually calls these handlers (e.g., add a spy/counter assertion), and consider extracting a sharedregisterSyncTaskTool(server, name, config, resultBuilder)helper to de-duplicate this pattern.testing/e2e/src/routes/api.mcp-server.ts#L63-L93: same verification and de-duplication applies here; this block is otherwise identical to the one inin-memory-server.ts.🤖 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/ai-mcp/tests/helpers/in-memory-server.ts` around lines 72 - 99, In packages/ai-mcp/tests/helpers/in-memory-server.ts#L72-L99 and testing/e2e/src/routes/api.mcp-server.ts#L63-L93, extract the duplicated registerToolTask setup into a shared registerSyncTaskTool helper, then add spy or counter assertions proving the installed SDK invokes getTask and getTaskResult; preserve the synchronous completed-result behavior and apply the shared helper at both sites.
🤖 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/tests/tools.test.ts`:
- Line 246: Replace the expect.anything() matcher in the callMcpTool non-task
branch assertion with an exact options object asserting { signal: undefined },
preserving verification that the forwarded signal shape is constructed
correctly.
---
Nitpick comments:
In `@packages/ai-mcp/tests/helpers/in-memory-server.ts`:
- Around line 72-99: In
packages/ai-mcp/tests/helpers/in-memory-server.ts#L72-L99 and
testing/e2e/src/routes/api.mcp-server.ts#L63-L93, extract the duplicated
registerToolTask setup into a shared registerSyncTaskTool helper, then add spy
or counter assertions proving the installed SDK invokes getTask and
getTaskResult; preserve the synchronous completed-result behavior and apply the
shared helper at both sites.
🪄 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: 185d35bd-5add-4a50-8afb-cfdd8f1d5c7c
📒 Files selected for processing (17)
.changeset/mcp-task-execution.mddocs/config.jsondocs/tools/mcp-manual.mddocs/tools/mcp.mdpackages/ai-mcp/README.mdpackages/ai-mcp/skills/ai-mcp/SKILL.mdpackages/ai-mcp/src/client.tspackages/ai-mcp/src/errors.tspackages/ai-mcp/src/tools.tspackages/ai-mcp/tests/client.test.tspackages/ai-mcp/tests/helpers/in-memory-server.tspackages/ai-mcp/tests/tools.test.tstesting/e2e/src/routes/api.mcp-apps-server.tstesting/e2e/src/routes/api.mcp-server.tstesting/e2e/src/routes/api.mcp-status-test.tstesting/e2e/tests/mcp-apps.spec.tstesting/e2e/tests/mcp-status.spec.ts
|
Review follow-up on the task-fixture nitpick: I verified the suggested callback counters against the installed I therefore did not add impossible callback assertions or extract a shared helper across the |
|
can you create an example in ts-react-chat so i cna manually test this out? |
mcp-providers.ts exported both the provider metadata (used by the
/mcp-demo and /mcp-apps pages) and resolveTextAdapter, so the pages
dragged every provider SDK into the browser bundle. @anthropic-ai/sdk
sits in vite's optimizeDeps.exclude, which serves its CJS dependency
standardwebhooks un-prebundled and crashes both pages in dev ("does not
provide an export named 'Webhook'").
Split adapter resolution into a server-only mcp-provider-adapters.ts and
keep mcp-providers.ts as client-safe metadata. Also bump the Gemini
example model to gemini-3.5-flash — gemini-2.5-flash now 404s for new
users.
Manual test surface for task-based tool execution. An in-process
Streamable HTTP MCP server (/api/mcp-tasks-server) exposes a single
appraise_guitar_collection tool with execution.taskSupport: 'required',
and /api/mcp-tasks-chat runs it through chat({ mcp }) so the task flow
(create task → poll → fetch result) drives the call instead of plain
tools/call.
The appraisal takes ~4s, so the tool call visibly sits pending in the
UI while the client polls every 500ms, then the result lands and the
model summarizes it. Try: "Appraise my guitar collection: strat, tele,
jazzmaster".
|
Done — added a Tasks mode to the ts-react-chat MCP demo. To test:
You should see the tool call sit pending for ~4s while the client polls, then the result lands and the model summarizes it ($1400/guitar → $4200 for that prompt). How it works: the demo hosts an in-process Streamable HTTP MCP server at Heads up: the first of the two new commits fixes a pre-existing dev crash on |
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 `@examples/ts-react-chat/src/routes/api.mcp-tasks-chat.ts`:
- Around line 45-86: Prevent MCP resource leaks when stream setup fails: in
examples/ts-react-chat/src/routes/api.mcp-tasks-chat.ts lines 45-86, keep the
client accessible outside the try block and close it in catch; in
examples/ts-react-chat/src/routes/api.mcp-chat.ts lines 46-113, close all
connected clients with Promise.allSettled in catch; in
examples/ts-react-chat/src/routes/api.mcp-pool.ts lines 52-96, keep the pool
accessible outside the try block and close it in catch. Ensure cleanup occurs
when chat or response initialization throws, without disrupting existing abort
and error responses.
🪄 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: f58b9551-3a75-4f0a-afa0-1de28f600cb0
📒 Files selected for processing (10)
examples/ts-react-chat/src/lib/mcp-provider-adapters.tsexamples/ts-react-chat/src/lib/mcp-providers.tsexamples/ts-react-chat/src/routeTree.gen.tsexamples/ts-react-chat/src/routes/api.mcp-apps-chat.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/api.mcp-tasks-chat.tsexamples/ts-react-chat/src/routes/api.mcp-tasks-server.tsexamples/ts-react-chat/src/routes/mcp-demo.tsx
…nnect chat() only takes ownership of the clients once the SSE stream is consumed, so a synchronous throw between connect and returning the response (adapter resolution, capability validation) leaked stdio subprocesses / HTTP sessions. Hoist the client/pool handle out of the try and close it defensively in catch — close() is idempotent, and the abort/error responses are unchanged.
- Gate task execution on the server declaring the tasks capability for
tools/call: auto-discovery skips task-required tools the server cannot
execute, and binding one explicitly throws MCPTaskRequiredToolError
(revived from its deprecated state).
- Follow tools/list pagination when discovering tool definitions.
- Pass the task param to callToolStream explicitly instead of relying on
the SDK's internal metadata cache.
- callTool: lazy discovery fetches tools/list raw (doesn't arm the SDK's
output-schema validators, keeping direct calls validation-free as
before), tolerates a failing tools/list by falling back to a plain
call, and no longer re-lists on every call for unlisted tool names.
- callTool accepts an optional { signal }; aborting a task-required call
now sends a best-effort tasks/cancel for the in-flight remote task.
- Invalidate the cached definitions on tools/list_changed so execution
modes are re-discovered.
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/client.ts`:
- Around line 291-300: Update the direct tool-call flow in the client method
containing taskRequired so it throws MCPTaskRequiredToolError when the resolved
definition requires task execution but serverSupportsTaskCalls(this.#client) is
false, rather than falling back to callMcpTool. Preserve ordinary calls for
non-task-required tools and add a regression test using
makeServerWithUnsupportedTaskTool.
In `@packages/ai-mcp/src/tools.ts`:
- Around line 83-93: Remove the abort-triggered tasks.cancelTask call and the
related taskId tracking from the stream handling in tools.ts; local abort must
only stop waiting without cancelling server-side work. Update the corresponding
client behavior in the client.ts task flow and revise the affected client tests
to assert no remote cancellation occurs on abort.
🪄 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: 24e38f8f-f489-42c4-a4a9-3a27f49f08ab
📒 Files selected for processing (10)
.changeset/mcp-task-execution.mddocs/config.jsondocs/tools/mcp-manual.mddocs/tools/mcp.mdpackages/ai-mcp/src/client.tspackages/ai-mcp/src/errors.tspackages/ai-mcp/src/tools.tspackages/ai-mcp/tests/client.test.tspackages/ai-mcp/tests/helpers/in-memory-server.tspackages/ai-mcp/tests/tools.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/config.json
- packages/ai-mcp/tests/tools.test.ts
- docs/tools/mcp-manual.md
… server cannot execute Throw MCPTaskRequiredToolError (mirroring the tools([...defs]) binding guard) instead of sending a plain tools/call the server rejects with an opaque -32600.
2 participants
Summary
execution.taskSupport: 'required'through the SDK's experimentalcallToolStreamflowMCPClient.callTool(), and MCP Apps callscallTool, preserve the deprecated task-required error export, and document abort semanticsOn abort, the client stops waiting for the task stream but does not cancel a remote task that has already been created. Task progress events and public task-management methods remain out of scope for this initial implementation.
Closes #704
Test plan
pnpm test:prpnpm --filter @tanstack/ai-e2e test:e2e(349 passed, 1 gated skip)Summary by CodeRabbit