Repository navigation
feat: Tasks extension for server and client (ext/tasks subpaths) - #2782
mattzcarey wants to merge 12 commits into
Conversation
🦋 Changeset detectedLatest commit: 8980fb8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
mattzcarey
changed the title
774c3f9 to
b04649e
Compare
mattzcarey
changed the title
mattzcarey
changed the title
46c22c6 to
b6f148b
Compare
b6f148b to
100decd
Compare
claude
Bot
added
the
v2
100decd to
92e5587
Compare
92e5587 to
1e5cffb
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 optional notes from this repository's REVIEW.md or CLAUDE.md checks were not posted as comments, over this review's limit for such notes; they are on this commit's check card.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| * comes back as `{ kind: 'result' }` for tools that answer synchronously. | ||
| */ | ||
| async callTool(params: CallToolRequestParams, options?: RequestOptions): Promise<CallToolOutcome> { | ||
| const raw = await this.client.request({ method: 'tools/call', params }, callToolOutcomeSchema, options); |
There was a problem hiding this comment.
🔴 Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_MISMATCH recovery, and output-schema validation of synchronous results. The method at tasksClientExtension.ts:81 issues a bare client.request for tools/call, so against a SEP-2243 server (including createMcpHandler) a tool with x-mcp-header arguments fails where client.callTool succeeds, and a { kind: 'result' } with bad structuredContent passes unchecked. Fix: route the request through the same pre/post logic as Client.callTool (header scan, one-refresh retry, outputSchema check) when the result is not a task, or document that tasks.callTool is a raw send without those behaviours.
Why this was flagged
A user installs TasksClientExtension and calls tasks.callTool({ name, arguments }) for a tool whose inputSchema declares an x-mcp-header parameter, against this SDK's own createMcpHandler. TasksClientExtension.callTool (tasksClientExtension.ts:81) calls this.client.request({ method: 'tools/call', params }, callToolOutcomeSchema, options) and nothing else. Client.callTool at client.ts:2358-2383 computes Mcp-Param-* headers via _resolveXMcpHeaderScan and retries once on HEADER_MISMATCH at client.ts:2423-2454; the extension does neither, so the server rejects the call with HEADER_MISMATCH and the user gets an error that the plain client.callTool path does not produce. Likewise for a tool with outputSchema that answers synchronously, Client.callTool at client.ts:2457-2492 rejects missing or invalid structuredContent; the extension returns { kind: 'result' } with the unchecked payload. docs/clients/tasks.md:26 describes tasks.callTool as the way to call any tool that may become a task.
Verification: Triggers whenever a tools/call goes through TasksClientExtension.callTool for a tool with an x-mcp-header parameter against createMcpHandler, or for a tool with an outputSchema. packages/client/src/ext/tasks/tasksClientExtension.ts:81 is the entire send path, so the call is rejected outright with no recovery, where client.callTool recovers.
There was a problem hiding this comment.
3 optional suggestions (nits or notes on pre-existing code) were found and not posted.
Still open from earlier reviews (10):
- 🔴
docs/servers/tasks.md:41—Operators following the documented task pattern get a server process crash when a client cancels a task that is waiting… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:68—Hosts that install TasksClientExtension but still use plain client.callTool get a silent empty result when a tool answe… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:81—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:113—Client users following the documented waitFor pattern are prompted again on every poll while a task waits for input, so… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:107—Operators who set a task retention longer than about 24.8 days get the task purged about 1 ms after creation instead of… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:79—Users who share an OAuth client can read, answer and cancel each other's tasks once this merges, which the base cannot… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:142—Clients whose tasks/update answers are discarded by the protocol lift get a success ack while the task stays stuck in i… - Also unresolved: 3 minor or pre-existing.
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
| async create(ctx: ServerContext, options?: CreateTaskOptions): Promise<TaskToolResult> { | ||
| requireTasksExtension(ctx, `Tool "${ctx.mcpReq.method}" executes as a task and`); | ||
| const principal = principalOf(ctx); | ||
| const task = await this.store.create({ | ||
| ttlMs: options?.ttlMs === undefined ? this.#defaultTtlMs : options.ttlMs, | ||
| pollIntervalMs: options?.pollIntervalMs ?? this.#defaultPollIntervalMs, | ||
| ...(principal !== undefined && { principal }), | ||
| ...(options?.context !== undefined && { context: options.context }) | ||
| }); | ||
| const result: CreateTaskResult = { resultType: 'task', ...task }; | ||
| return result as TaskToolResult; |
There was a problem hiding this comment.
🔴 Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task handle, while the work keeps running unseen. tasks.create at tasksExtension.ts:113-123 returns a handle with no structuredContent, and McpServer's output validation at packages/server/src/server/mcp.ts:407-411 rejects it; the catch at mcp.ts:324 turns that into a text error result, so the taskId never reaches the client. Fix: let a resultType: 'task' handle bypass tool output validation the same way isInputRequiredResult does at mcp.ts:395, or make tasks.create fail loudly for such tools; docs/servers/tasks.md:67 only notes the limitation in passing.
Why this was flagged
A server registers server.registerTool('report', { inputSchema, outputSchema: z.object({ pages: z.number() }) }, async (args, ctx) => { const task = await tasks.create(ctx); void run(store.handle(task.taskId)); return task; }). tasks.create at packages/server/src/ext/tasks/tasksExtension.ts:116-123 creates the task in the store and returns { resultType: 'task', taskId, status, ... } with no structuredContent. McpServer's tools/call handler at packages/server/src/server/mcp.ts:308 calls validateToolOutput; at mcp.ts:389 the tool has an outputSchema, mcp.ts:395 only exempts isInputRequiredResult, and mcp.ts:407-411 throws Output validation error: Tool report has an output schema but no structured content was provided. The catch at mcp.ts:316-324 is not -32042 or -32021, so createToolError answers { content: [{ type: 'text', text: 'Output validation error: ...' }], isError: true }. The task record and its void run(...) work stay alive in the store until TTL with nobody able to learn the taskId. The base branch has no tasks extension, so no task is orphaned there.
Verification: docs/servers/tasks.md:67 says "Task tools are ordinary tools without outputSchema", but nothing enforces it. create at packages/server/src/ext/tasks/tasksExtension.ts:116-123 returns { resultType: 'task', ...task } with no structuredContent; at mcp.ts:395 only isInputRequiredResult short-circuits, so it reaches mcp.ts:407-411 and throws, and the catch at mcp.ts:317-324 turns that into createToolError(...).
| const task = await this.store.create({ | ||
| ttlMs: options?.ttlMs === undefined ? this.#defaultTtlMs : options.ttlMs, | ||
| pollIntervalMs: options?.pollIntervalMs ?? this.#defaultPollIntervalMs, | ||
| ...(principal !== undefined && { principal }), | ||
| ...(options?.context !== undefined && { context: options.context }) | ||
| }); |
There was a problem hiding this comment.
🔴 Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task then sits in input_required until its TTL. tasksExtension.ts:116-121 records nothing about the creating request's client capabilities, and InMemoryTaskStore.requireInput (inMemoryStore.ts:170-180) publishes any inputRequests without a check, whereas the in-band seam at server.ts:711-723 refuses the same requests with -32021. Fix: capture the request's declared capabilities in create (from ctx.mcpReq.envelope[CLIENT_CAPABILITIES_META_KEY]) and have requireInput reject (or fail the task) when requiredClientCapabilitiesForInputRequest is not covered, mirroring the tools/call input_required gate.
Why this was flagged
A host builds new Client(info, { extensions: [new TasksClientExtension()] }) with no elicitation capability. A tool handler calls tasks.create(ctx) (packages/server/src/ext/tasks/tasksExtension.ts:113-124), which passes only ttlMs, pollIntervalMs, principal and context to the store; the envelope's client capabilities are dropped. The work then calls handle.requireInput({ approve: { method: 'elicitation/create', ... } }); inMemoryStore.ts:170-180 sets status to input_required and stores the requests with no capability check, and tasks/get at inMemoryStore.ts:235-241 returns them to the client. On the base branch the only way to ask the client for input is an input_required tool result, and packages/server/src/server/server.ts:711-723 refuses that with MissingRequiredClientCapabilityError (-32021) when the request's envelope does not declare the capability, so a non-elicitation client never receives an elicitation. After merging, such a client receives an elicitation it cannot fulfil and the task stays input_required until the 24h default TTL purge at inMemoryStore.ts:107, after which the client gets -32602 'Task not found'.
Verification: At TasksExtension.create (packages/server/src/ext/tasks/tasksExtension.ts:116-121) nothing from ctx.mcpReq.envelope[CLIENT_CAPABILITIES_META_KEY] is recorded. requireInput at inMemoryStore.ts:170-180 sets status = 'input_required' with no check on the request kinds. The in-band path enforces this with -32021; the tasks path does not.
| { params: updateTaskHandlerParamsSchema }, | ||
| async (params, ctx): Promise<UpdateTaskResult> => { | ||
| requireTasksExtension(ctx, 'tasks/update'); | ||
| const inputResponses = (ctx.mcpReq.inputResponses ?? params.inputResponses ?? {}) as InputResponses; |
There was a problem hiding this comment.
🔴 Server work that awaits requireInput receives unvalidated client JSON presented as typed ElicitResult/CreateMessageResult answers, so a malformed tasks/update can crash the detached work. tasksExtension.ts:139 casts ctx.mcpReq.inputResponses to InputResponses and the store resolves those raw objects from requireInput (inMemoryStore.ts:55) without checking them against the pending request's method. Fix: in tasks/update, validate every accepted entry against the response schema of the matching pending request's method (the 2026 codec's validateInputResponse) and answer -32602 on a mismatch.
Why this was flagged
A task is in input_required with a sampling or elicitation key; the client sends tasks/update with inputResponses: { q: {} }. partitionInputResponses at packages/core-internal/src/shared/inputRequiredEngine.ts:50-54 accepts any plain object without method/result, so the entry reaches the handler at packages/server/src/ext/tasks/tasksExtension.ts:139, which casts it as InputResponses. InMemoryTaskStore.update at packages/server/src/ext/tasks/inMemoryStore.ts:126-132 stores the object as-is and resolves requireInput (typed Promise<InputResponses> at inMemoryStore.ts:55) with it. Server code written to that type, e.g. answers.q.content.text, throws TypeError inside the void-started work, surfacing as an unhandled rejection. The core SDK's own path types ctx.mcpReq.inputResponses as Record<string, unknown> (packages/core-internal/src/shared/protocol.ts:394) and says the values are not re-validated and must be treated as untrusted; the extension drops that guard by asserting the typed union. The base branch has no tasks extension.
Verification: tasksExtension.ts:139 does (ctx.mcpReq.inputResponses ?? params.inputResponses ?? {}) as InputResponses. inMemoryStore.ts:124-126 stores the raw entry for any key present in pending.requests with no check against the pending request's method, and :132 hands it to the detached work via requireInput(): Promise<InputResponses> (inMemoryStore.ts:55). Base branch has no tasks extension, so this is new.
There was a problem hiding this comment.
The new check in tasks/update (tasksExtension.ts:180-189) only refuses entries the protocol layer already dropped, i.e. non-objects and wrapped {method, result} envelopes, plus a missing inputResponses. A bare but wrong-shaped entry such as inputResponses: { q: {} } still passes partitionInputResponses (inputRequiredEngine.ts:50), is still cast as InputResponses at tasksExtension.ts:187, and InMemoryTaskStore.update (inMemoryStore.ts:127-133) still stores it and resolves requireInput with it, so work reading answers.q.content.text still throws in the detached task. Closing this needs each accepted entry validated against the response schema for the matching pending request's method (e.g. the 2026 codec's validateInputResponse) with -32602 on a mismatch, which means the handler or the store has to look up the pending requests before accepting the answers.
1e5cffb to
ff1836d
Compare
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 18 findings from earlier reviews are still open above.
2 optional suggestions (nits or notes on pre-existing code) were found and not posted.
Still open from earlier reviews (18):
- 🔴
docs/servers/tasks.md:41—Operators following the documented task pattern get a server process crash when a client cancels a task that is waiting… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:89—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:121—Client users following the documented waitFor pattern are prompted again on every poll while a task waits for input, so… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:107—Operators who set a task retention longer than about 24.8 days get the task purged about 1 ms after creation instead of… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:176—Server work that calls handle.requireInput while an earlier requireInput on the same task is still outstanding hangs fo… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:195—Server work that settles a task with complete or fail while a requireInput is still outstanding leaves that requireInpu… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:78—Users who share an OAuth client can read, answer and cancel each other's tasks once this merges, which the base cannot… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:120—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:122—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 5 minor or pre-existing, 4 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
4b9ede4 to
38e3016
Compare
There was a problem hiding this comment.
5 other optional suggestions (nits or notes on pre-existing code) were found and not posted.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Still open from earlier reviews (18):
- 🔴
docs/servers/tasks.md:41—Operators following the documented task pattern get a server process crash when a client cancels a task that is waiting… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:89—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:121—Client users following the documented waitFor pattern are prompted again on every poll while a task waits for input, so… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:107—Operators who set a task retention longer than about 24.8 days get the task purged about 1 ms after creation instead of… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:176—Server work that calls handle.requireInput while an earlier requireInput on the same task is still outstanding hangs fo… - 🔴
packages/server/src/ext/tasks/inMemoryStore.ts:195—Server work that settles a task with complete or fail while a requireInput is still outstanding leaves that requireInpu… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:78—Users who share an OAuth client can read, answer and cancel each other's tasks once this merges, which the base cannot… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:120—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:122—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 5 minor or pre-existing, 4 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
| * plain async function driving the store's writer handle. | ||
| */ | ||
| import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; | ||
| import { TasksClientExtension } from '@modelcontextprotocol/client/ext/tasks'; |
There was a problem hiding this comment.
🟡 nit (optional): Maintainers get Tasks end-to-end tests that run on one transport and one era, outside the suite that tracks user-visible behaviour. Both e2e files sit in packages/server/test/ext/tasks, and test/e2e/requirements.ts lists none of their behaviours, unlike the Skills extension's. Fix: move them to test/e2e/scenarios with verifies() ids in requirements.ts, which also removes the server's new @ modelcontextprotocol/client devDependency (packages/server/package.json:181) and five tsconfig paths, covering the 2 sites listed. Same instruction at 2 sites (tasks.e2e.test.ts:9, withSkills.e2e.test.ts:8).
Why this was flagged
The diff adds packages/server/test/ext/tasks/tasks.e2e.test.ts and withSkills.e2e.test.ts, which drive a real Client against createMcpHandler through a fetch-wired StreamableHTTPClientTransport only (tasks.e2e.test.ts:82-84). test/e2e/requirements.ts has no entry mentioning tasks, while the Skills extension has skills:* entries at test/e2e/requirements.ts:3394-3434, proved with verifies() at test/e2e/scenarios/skills.test.ts:73 over that suite's default transport list. So stdio, which examples/ext-tasks/README.md advertises, and the 2025-era path have no Tasks test, and the behaviour manifest does not show Tasks at all. To host the tests in the server package the diff adds @ modelcontextprotocol/client to its devDependencies (packages/server/package.json:181) and five paths entries (packages/server/tsconfig.json:25-29). Nothing fails at runtime, and no safeguard applies because the coverage check only reads ids already in requirements.ts. The base branch has no Tasks tests.
Verification: CLAUDE.md:11 reads: "test/e2e/requirements.ts lists those behaviours." The diff adds end-to-end tests of user-visible Tasks behaviour but lists none of it in that file. packages/server/test/ext/tasks/tasks.e2e.test.ts:82-84 wires only new StreamableHTTPClientTransport. Nothing breaks, the gap is test coverage.
Finding ID: f-ac448db3
38e3016 to
c19f4b6
Compare
There was a problem hiding this comment.
5 other optional suggestions (nits or notes on pre-existing code) were found and not posted.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Still open from earlier reviews (13):
- 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:89—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:79—Users who share an OAuth client can read, answer and cancel each other's tasks once this merges, which the base cannot… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:121—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:123—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 6 minor or pre-existing, 3 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
52fde7c to
4846fbc
Compare
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 12 findings from earlier reviews are still open above.
5 optional suggestions (nits or notes on pre-existing code) were found and not posted.
Still open from earlier reviews (12):
- 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:89—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:152—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:154—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 6 minor or pre-existing, 3 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
4846fbc to
8cd563c
Compare
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 6 findings from earlier reviews are still open above.
4 optional suggestions (nits or notes on pre-existing code) were found and not posted.
Still open from earlier reviews (6):
- 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:90—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:152—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:154—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 2 minor or pre-existing, 1 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
@modelcontextprotocol/server/ext/tasks adds TasksExtension(store): it advertises io.modelcontextprotocol/tasks, serves tasks/get, tasks/update and tasks/cancel, refuses requests that did not declare the extension (-32021), and gives a tool handler tasks.create(ctx) to answer tools/call with a task handle. TaskStore is the four-method interface a server implements over its own task state; InMemoryTaskStore is the in-process reference, with a writer handle (status, requireInput, complete, fail, signal). @modelcontextprotocol/client/ext/tasks adds TasksClientExtension: callTool, get, update, cancel and waitFor, which polls tasks/get until a terminal status. Wire types and schemas live at @modelcontextprotocol/core-internal/ext/tasks. examples/ext-tasks is a minimal in-process server and client: a bake_cake tool on TasksExtension over an InMemoryTaskStore reports progress, asks for input (input_required) and honours cancellation; the client follows it with TasksClientExtension.waitFor, answers through tasks.update, and cancels a second task.
InMemoryTaskStore passed ttlMs straight to setTimeout, which fires almost at once for any delay above 2^31-1 ms (about 24.8 days), so such a task was purged right after creation and its signal aborted. Wait the TTL out in steps of at most that delay.
handle.complete and handle.fail left an outstanding requireInput promise pending until the TTL purge, or forever with ttlMs: null, and a second requireInput replaced the first wait so the first never settled. Reject the pending wait when the task settles, refuse a second concurrent wait, and drop the abort listener once a wait resolves.
- TasksExtension goes in McpServerOptions.extensions; ServerOptions has no extensions option. - The requester half ships in this PR as TasksClientExtension from @modelcontextprotocol/client/ext/tasks, not @modelcontextprotocol/ext-tasks. - The server example fired the work with void and no catch; requireInput rejects on cancel or purge, so that crashed Node with an unhandled rejection. Settle failures on the task instead, here and in the TSDoc. - The schema header pointed at a vendored test fixture that does not exist.
The client guide answers input_required inside an async onUpdate, but waitFor called it without awaiting. Polling went on at the task's interval while the user was being asked, so the same unanswered input request came back and was asked again on every poll, and a rejection from the callback was unhandled. Await the callback before the next poll and let its rejection reject waitFor.
@modelcontextprotocol/core-internal is private, so users cannot import the wire types from it; they come from the server and client ext/tasks subpaths.
…6-07-28 only The task principal was the OAuth client id, so every user of a shared or public client could read, answer and cancel the others' tasks. The default is now the client id plus the token subject at authInfo.extra.sub when the verifier sets one, and TasksExtensionOptions.principal picks any identity. On 2025-11-25 the capability was advertised, then every tasks call failed with -32021 asking for an envelope that era cannot carry, and the core legacy tasks/get and tasks/cancel got the extension's handlers. The extension's methods now answer -32601 there, and tasks.create refuses with a -32021 that names the era.
everythingServer now installs TasksExtension over one in-memory store shared by the per-request servers, and registers the tools the tasks-* scenarios call by name: greet, slow_compute, failing_job, protocol_error_job, confirm_delete, multi_input and test_tool_with_task. All nine tasks-* server scenarios pass, so they leave the expected-failures baseline.
…ndles - tasks/update acked a request whose inputResponses the protocol dropped as not bare results, or that had none, leaving the task waiting. It now answers -32602 naming the refused keys, or the missing field; an empty object is still a valid partial update. - The tools/call middleware refused a task handle minted outside create after the task was already running, so the task leaked until its TTL, or forever with ttlMs: null. It now cancels the task before refusing.
TTL expiry and close() aborted the same signal as tasks/cancel with no reason, and a pending requireInput rejected with "was cancelled" in every case. The signal now carries an Error naming cancellation, expiry or store close, and the wait rejects with it.
…-type headers The TasksClientExtension example built a client on the default legacy handshake, where tasks.callTool is refused with -32021. It now negotiates, and the header says the extension needs a 2026-07-28 connection. The wire-type and schema headers keep provenance only.
8cd563c to
5356ed6
Compare
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 6 findings from earlier reviews are still open above.
3 optional suggestions (nits or notes on pre-existing code) were found and not posted.
Still open from earlier reviews (6):
- 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:90—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:152—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:154—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 2 minor or pre-existing, 1 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
…ol/core/ext/tasks
The Tasks schemas, types and constants lived in the private core-internal,
so code implementing either side without the SDK's server or client had no
published import, and the changeset had pointed at a private package. They
now live in core/ext/tasks, like core/ext/skills: the zod schemas and
constants move unchanged, and the types are inferred from the schemas, so
there is one definition. core-internal/ext/tasks is gone.
The core types describe inputRequests structurally ({ method, params }), as
core cannot import the SDK's MRTR unions. The server and client subpaths
re-export core's definitions, plus the precise InputRequest(s) and
InputResponse(s) types that requireInput and update take.
There was a problem hiding this comment.
4 other optional suggestions (nits or notes on pre-existing code) were found and not posted.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Still open from earlier reviews (6):
- 🔴
packages/client/src/ext/tasks/tasksClientExtension.ts:90—Clients calling tools through tasks.callTool lose what client.callTool provides: Mcp-Param-* header mirroring, HEADER_M… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:152—Clients that never declared elicitation, sampling or roots can be handed those requests through a task and the task the… - 🔴
packages/server/src/ext/tasks/tasksExtension.ts:154—Servers that declare an outputSchema on a task-returning tool send clients an isError tool result instead of the task h… - Also unresolved: 2 minor or pre-existing, 1 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
| ctx.setRequestHandler('tasks/get', { params: getTaskParamsSchema }, async (params, requestCtx): Promise<GetTaskResult> => { | ||
| requireModernRequest(requestCtx); | ||
| requireTasksExtension(requestCtx, 'tasks/get'); | ||
| const task = await this.store.get(params.taskId, { principal: this.#principal(requestCtx) }); |
There was a problem hiding this comment.
🟡 (optional) A token without a tool's required scope can read that tool's task result, and answer or cancel the task, when it shares the task's principal and knows the taskId. Per-tool scopeChallenge runs only for tools/call; tasks/get, tasks/update and tasks/cancel (tasksExtension.ts:169, :190, :199) check the principal alone. Fix: make all three task methods refuse a token that the creating tool's scopeChallenge would challenge, for example by recording the tool name at creation and re-running its check, or state in the docs that task access ignores scopes. On the base branch the output only returns on the scope-checked call.
Why this was flagged
A server registers a tool with scopeChallenge: requireScopes('reports:send') whose handler returns tasks.create(ctx). Token T1 with that scope calls it: the preflight at packages/server/src/server/createMcpHandler.ts:854-866 passes and the task is bound to defaultTaskPrincipal (tasksExtension.ts:98-103), which is the client id plus extra.sub and carries no scopes. Token T2 of the same client and subject but without reports:send then sends tasks/get, tasks/update or tasks/cancel with the taskId. resolveScopeChallenge (mcp.ts:217-260) handles only tools/call, resources/read and prompts/get and returns nothing for tasks/*, so no challenge is issued; the handlers pass only { principal } and #lookup (inMemoryStore.ts:218-223) compares that string. T2 receives the gated tool's result, can answer its requireInput approval, or cancel it. On the base branch a gated tool's output is only returned in the tools/call response that passed the scope check.
Verification: McpServer.resolveScopeChallenge at packages/server/src/server/mcp.ts:217-261 switches only on tools/call, resources/read and prompts/get; default: { return; } at :257-259 means tasks/* never produces a challenge. On the base a scoped tool's result is reachable only through the gated tools/call.
Finding ID: f-df7abc51
Adds the Tasks extension (
io.modelcontextprotocol/tasks, SEP-2663) as@modelcontextprotocol/server/ext/tasksand@modelcontextprotocol/client/ext/tasks. Stacked on #3066 and #2820; review the top commit.The SDK handles the wire protocol only. The server is responsible for running tasks:
TasksExtensionis a{ id, setup(ctx) }extension. It advertises the capability, servestasks/get,tasks/updateandtasks/cancelthroughctx.setRequestHandler, rejects requests that did not declare the extension (-32021), ties each task to the request principal, and gives toolstasks.create(ctx)to return a task handle.TaskStoreis four methods (create,get,update,cancel) that a server implements over its own state.InMemoryTaskStoreis the in-process reference implementation, with a writerhandle(status,requireInput,complete,fail,signal).TasksClientExtensionacceptsresultType: "task"ontools/callthroughctx.acceptResultType, wrapscallTool,get,updateandcancel, and addswaitFor, which polls at the server'spollIntervalMsuntil the task reaches a terminal state.@modelcontextprotocol/core-internal/ext/tasks.examples/ext-tasksis a minimal in-process server and client: abake_caketool reports progress, asks for input (input_required) and honours cancellation, and the client follows it withwaitFor, answers throughtasks.update, and cancels a second task.Notes for review:
inputResponsesis reserved: the protocol layer lifts it out of request params, so thetasks/updatehandler reads it fromctx.mcpReq.inputResponses.client.callToolstill rejects a task handle ([v2] Tasks extension: tools/call rejects CreateTaskResult but accepts an omitted discriminator as complete #2637). Only the extension declaresresultType: "task".notifications/tasks(subscriptions/listen cannot carry extension notifications (blocks notifications/tasks) #2569) is not implemented. Clients poll.Refs #2189, #2598.
pnpm -r typecheck,pnpm -r lintand the client and server suites ran, including an end-to-end test of a realClientagainst the statelesscreateMcpHandler(completion,input_required→tasks/update, failure, cancel,-32602,-32021). A second end-to-end test passes Skills and Tasks together inextensionson oneMcpServer: the client lists and reads a skill, then follows a tool's task to completion. The example runs end to end over stdio.