Skip to content

Commit 0f7ba7c

Browse files
fix(server): make ToolTaskHandler.getTask/getTaskResult optional and actually invoke them
These handlers were defined on the interface but never invoked — three code paths bypassed them and called TaskStore directly: - TaskManager.handleGetTask - TaskManager.handleGetTaskPayload - McpServer.handleAutomaticTaskPolling The handlers exist to support proxying external job systems (AWS Step Functions, CI/CD pipelines) where the external system is the source of truth for task state. But every test/example implementation was pure boilerplate delegation to the store. This change makes the handlers optional: when omitted, TaskStore handles requests (zero boilerplate, previous de-facto behavior). When provided, they're invoked for tasks/get, tasks/result, and automatic polling. Dispatch logic is isolated in ExperimentalMcpServerTasks. Core gains a single setTaskOverrides() method on TaskManager — no changes to public TaskManagerOptions type. McpServer itself gains ~5 lines. Also drops the Args parameter from TaskRequestHandler since tool input arguments aren't available at tasks/get/tasks/result time. BREAKING CHANGE: TaskRequestHandler signature changed from (args, ctx) to (ctx). ToolTaskHandler.getTask and getTaskResult are now optional. Closes #1332 Co-authored-by: Luca Chang <lucalc@amazon.com>
1 parent 40174d2 commit 0f7ba7c

File tree

13 files changed

+320
-314
lines changed

13 files changed

+320
-314
lines changed
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@modelcontextprotocol/core": minor
3+
"@modelcontextprotocol/server": minor
4+
---
5+
6+
Make `ToolTaskHandler.getTask`/`getTaskResult` optional and actually invoke them
7+
8+
**Bug fix:** `getTask` and `getTaskResult` handlers registered via `registerToolTask` were never invoked — `tasks/get` and `tasks/result` requests always hit `TaskStore` directly.
9+
10+
**Breaking changes (experimental API):**
11+
12+
- `getTask` and `getTaskResult` are now **optional** on `ToolTaskHandler`. When omitted, `TaskStore` handles the requests (previous de-facto behavior).
13+
- `TaskRequestHandler` signature changed: handlers receive only `(ctx: TaskServerContext)`, not the tool's input arguments.
14+
15+
**Migration:** If your handlers just delegated to `ctx.task.store`, delete them. If you're proxying an external job system (Step Functions, CI/CD pipelines), keep them and drop the `args` parameter.

docs/migration-SKILL.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ Notes:
9696
| `ErrorCode.RequestTimeout` | `SdkErrorCode.RequestTimeout` |
9797
| `ErrorCode.ConnectionClosed` | `SdkErrorCode.ConnectionClosed` |
9898
| `StreamableHTTPError` | REMOVED (use `SdkError` with `SdkErrorCode.ClientHttp*`) |
99+
| `ToolTaskHandler.getTask(args, ctx)` | `ToolTaskHandler.getTask?(ctx)` — now optional, no args |
100+
| `ToolTaskHandler.getTaskResult(args, ctx)` | `ToolTaskHandler.getTaskResult?(ctx)` — now optional, no args |
99101

100102
All other symbols from `@modelcontextprotocol/sdk/types.js` retain their original names (e.g., `CallToolResultSchema`, `ListToolsResultSchema`, etc.).
101103

docs/migration.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,37 @@ import { JSONRPCError, ResourceReference, isJSONRPCError } from '@modelcontextpr
478478
import { JSONRPCErrorResponse, ResourceTemplateReference, isJSONRPCErrorResponse } from '@modelcontextprotocol/server';
479479
```
480480

481+
### `ToolTaskHandler.getTask` and `getTaskResult` are now optional (experimental)
482+
483+
`getTask` and `getTaskResult` are now optional on `ToolTaskHandler`. When omitted, `tasks/get` and `tasks/result` are served directly from the configured `TaskStore`. Their signature has also changed — they no longer receive the tool's input arguments (which aren't available at `tasks/get`/`tasks/result` time).
484+
485+
If your handlers just delegated to the store, delete them:
486+
487+
**Before:**
488+
489+
```typescript
490+
server.experimental.tasks.registerToolTask('long-task', config, {
491+
createTask: async (args, ctx) => { /* ... */ },
492+
getTask: async (args, ctx) => ctx.task.store.getTask(ctx.task.id),
493+
getTaskResult: async (args, ctx) => ctx.task.store.getTaskResult(ctx.task.id)
494+
});
495+
```
496+
497+
**After:**
498+
499+
```typescript
500+
server.experimental.tasks.registerToolTask('long-task', config, {
501+
createTask: async (args, ctx) => { /* ... */ }
502+
});
503+
```
504+
505+
Keep them if you're proxying an external job system (AWS Step Functions, CI/CD pipelines, etc.) — the new signature takes only `ctx`:
506+
507+
```typescript
508+
getTask: async (ctx) => describeStepFunctionExecution(ctx.task.id),
509+
getTaskResult: async (ctx) => getStepFunctionOutput(ctx.task.id)
510+
```
511+
481512
### Request handler context types
482513

483514
The `RequestHandlerExtra` type has been replaced with a structured context type hierarchy using nested groups:

examples/server/src/simpleStreamableHttp.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -480,13 +480,6 @@ const getServer = () => {
480480
return {
481481
task
482482
};
483-
},
484-
async getTask(_args, ctx) {
485-
return await ctx.task.store.getTask(ctx.task.id);
486-
},
487-
async getTaskResult(_args, ctx) {
488-
const result = await ctx.task.store.getTaskResult(ctx.task.id);
489-
return result as CallToolResult;
490483
}
491484
}
492485
);
@@ -588,13 +581,6 @@ const getServer = () => {
588581
})();
589582

590583
return { task };
591-
},
592-
async getTask(_args, ctx) {
593-
return await ctx.task.store.getTask(ctx.task.id);
594-
},
595-
async getTaskResult(_args, ctx) {
596-
const result = await ctx.task.store.getTaskResult(ctx.task.id);
597-
return result as CallToolResult;
598584
}
599585
}
600586
);

packages/core/src/shared/taskManager.ts

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,16 @@ export type TaskContext = {
154154
requestedTtl?: number | null;
155155
};
156156

157+
/**
158+
* Overrides for `tasks/get` and `tasks/result` lookups. Consulted before
159+
* the configured {@linkcode TaskStore}; return `undefined` to fall through.
160+
* @internal
161+
*/
162+
export type TaskLookupOverrides = {
163+
getTask?: (taskId: string, ctx: BaseContext) => Promise<Task | undefined>;
164+
getTaskResult?: (taskId: string, ctx: BaseContext) => Promise<Result | undefined>;
165+
};
166+
157167
export type TaskManagerOptions = {
158168
/**
159169
* Task storage implementation. Required for handling incoming task requests (server-side).
@@ -199,20 +209,30 @@ export class TaskManager {
199209
private _requestResolvers: Map<RequestId, (response: JSONRPCResultResponse | Error) => void> = new Map();
200210
private _options: TaskManagerOptions;
201211
private _host?: TaskManagerHost;
212+
private _overrides?: TaskLookupOverrides;
202213

203214
constructor(options: TaskManagerOptions) {
204215
this._options = options;
205216
this._taskStore = options.taskStore;
206217
this._taskMessageQueue = options.taskMessageQueue;
207218
}
208219

220+
/**
221+
* Installs per-task lookup overrides consulted before the {@linkcode TaskStore}.
222+
* Used by McpServer to dispatch to per-tool `getTask`/`getTaskResult` handlers.
223+
* @internal
224+
*/
225+
setTaskOverrides(overrides: TaskLookupOverrides): void {
226+
this._overrides = overrides;
227+
}
228+
209229
bind(host: TaskManagerHost): void {
210230
this._host = host;
211231

212232
if (this._taskStore) {
213233
host.registerHandler('tasks/get', async (request, ctx) => {
214234
const params = request.params as { taskId: string };
215-
const task = await this.handleGetTask(params.taskId, ctx.sessionId);
235+
const task = await this.handleGetTask(params.taskId, ctx);
216236
// Per spec: tasks/get responses SHALL NOT include related-task metadata
217237
// as the taskId parameter is the source of truth
218238
return {
@@ -222,7 +242,7 @@ export class TaskManager {
222242

223243
host.registerHandler('tasks/result', async (request, ctx) => {
224244
const params = request.params as { taskId: string };
225-
return await this.handleGetTaskPayload(params.taskId, ctx.sessionId, ctx.mcpReq.signal, async message => {
245+
return await this.handleGetTaskPayload(params.taskId, ctx, async message => {
226246
// Send the message on the response stream by passing the relatedRequestId
227247
// This tells the transport to write the message to the tasks/result response stream
228248
await host.sendOnResponseStream(message, ctx.mcpReq.id);
@@ -362,8 +382,11 @@ export class TaskManager {
362382

363383
// -- Handler bodies (delegated from Protocol's registered handlers) --
364384

365-
private async handleGetTask(taskId: string, sessionId?: string): Promise<Task> {
366-
const task = await this._requireTaskStore.getTask(taskId, sessionId);
385+
private async handleGetTask(taskId: string, ctx: BaseContext): Promise<Task> {
386+
const override = await this._overrides?.getTask?.(taskId, ctx);
387+
if (override !== undefined) return override;
388+
389+
const task = await this._requireTaskStore.getTask(taskId, ctx.sessionId);
367390
if (!task) {
368391
throw new ProtocolError(ProtocolErrorCode.InvalidParams, 'Failed to retrieve task: Task not found');
369392
}
@@ -372,10 +395,12 @@ export class TaskManager {
372395

373396
private async handleGetTaskPayload(
374397
taskId: string,
375-
sessionId: string | undefined,
376-
signal: AbortSignal,
398+
ctx: BaseContext,
377399
sendOnResponseStream: (message: JSONRPCNotification | JSONRPCRequest) => Promise<void>
378400
): Promise<Result> {
401+
const sessionId = ctx.sessionId;
402+
const signal = ctx.mcpReq.signal;
403+
379404
const handleTaskResult = async (): Promise<Result> => {
380405
if (this._taskMessageQueue) {
381406
let queuedMessage: QueuedMessage | undefined;
@@ -404,17 +429,15 @@ export class TaskManager {
404429
}
405430
}
406431

407-
const task = await this._requireTaskStore.getTask(taskId, sessionId);
408-
if (!task) {
409-
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Task not found: ${taskId}`);
410-
}
432+
const task = await this.handleGetTask(taskId, ctx);
411433

412434
if (!isTerminal(task.status)) {
413435
await this._waitForTaskUpdate(task.pollInterval, signal);
414436
return await handleTaskResult();
415437
}
416438

417-
const result = await this._requireTaskStore.getTaskResult(taskId, sessionId);
439+
const override = await this._overrides?.getTaskResult?.(taskId, ctx);
440+
const result = override ?? (await this._requireTaskStore.getTaskResult(taskId, sessionId));
418441
await this._clearTaskQueue(taskId);
419442

420443
return {

packages/server/src/experimental/tasks/interfaces.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type {
1313
TaskServerContext
1414
} from '@modelcontextprotocol/core';
1515

16-
import type { BaseToolCallback } from '../../server/mcp.js';
16+
import type { AnyToolHandler, BaseToolCallback } from '../../server/mcp.js';
1717

1818
// ============================================================================
1919
// Task Handler Types (for registerToolTask)
@@ -30,19 +30,27 @@ export type CreateTaskRequestHandler<
3030

3131
/**
3232
* Handler for task operations (`get`, `getResult`).
33+
*
34+
* Receives only the context (no tool arguments — they are not available at
35+
* `tasks/get` or `tasks/result` time). Access the task ID via `ctx.task.id`.
36+
*
3337
* @experimental
3438
*/
35-
export type TaskRequestHandler<SendResultT extends Result, Args extends StandardSchemaWithJSON | undefined = undefined> = BaseToolCallback<
36-
SendResultT,
37-
TaskServerContext,
38-
Args
39-
>;
39+
export type TaskRequestHandler<SendResultT extends Result> = (ctx: TaskServerContext) => SendResultT | Promise<SendResultT>;
4040

4141
/**
4242
* Interface for task-based tool handlers.
4343
*
44-
* Task-based tools split a long-running operation into three phases:
45-
* `createTask`, `getTask`, and `getTaskResult`.
44+
* Task-based tools create a task on `tools/call` and by default let the SDK's
45+
* {@linkcode TaskStore} handle subsequent `tasks/get` and `tasks/result` requests.
46+
*
47+
* Provide `getTask` and `getTaskResult` to override the default lookups — useful
48+
* when proxying an external job system (e.g., AWS Step Functions, CI/CD pipelines)
49+
* where the external system is the source of truth for task state.
50+
*
51+
* **Note:** the taskId → tool mapping used to dispatch `getTask`/`getTaskResult`
52+
* is held in-memory and does not survive server restarts or span multiple
53+
* instances. In those scenarios, requests fall through to the `TaskStore`.
4654
*
4755
* @see {@linkcode @modelcontextprotocol/server!experimental/tasks/mcpServer.ExperimentalMcpServerTasks#registerToolTask | registerToolTask} for registration.
4856
* @experimental
@@ -56,11 +64,23 @@ export interface ToolTaskHandler<Args extends StandardSchemaWithJSON | undefined
5664
*/
5765
createTask: CreateTaskRequestHandler<CreateTaskResult, Args>;
5866
/**
59-
* Handler for `tasks/get` requests.
67+
* Optional handler for `tasks/get` requests. When omitted, the configured
68+
* {@linkcode TaskStore} is consulted directly.
6069
*/
61-
getTask: TaskRequestHandler<GetTaskResult, Args>;
70+
getTask?: TaskRequestHandler<GetTaskResult>;
6271
/**
63-
* Handler for `tasks/result` requests.
72+
* Optional handler for `tasks/result` requests. When omitted, the configured
73+
* {@linkcode TaskStore} is consulted directly.
6474
*/
65-
getTaskResult: TaskRequestHandler<CallToolResult, Args>;
75+
getTaskResult?: TaskRequestHandler<CallToolResult>;
76+
}
77+
78+
/**
79+
* Type guard for {@linkcode ToolTaskHandler}.
80+
* @experimental
81+
*/
82+
export function isToolTaskHandler(
83+
handler: AnyToolHandler<StandardSchemaWithJSON | undefined>
84+
): handler is ToolTaskHandler<StandardSchemaWithJSON | undefined> {
85+
return 'createTask' in handler;
6686
}

packages/server/src/experimental/tasks/mcpServer.ts

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,25 @@
55
* @experimental
66
*/
77

8-
import type { StandardSchemaWithJSON, TaskToolExecution, ToolAnnotations, ToolExecution } from '@modelcontextprotocol/core';
8+
import type {
9+
BaseContext,
10+
CallToolResult,
11+
GetTaskResult,
12+
ServerContext,
13+
StandardSchemaWithJSON,
14+
TaskManager,
15+
TaskServerContext,
16+
TaskToolExecution,
17+
ToolAnnotations,
18+
ToolExecution
19+
} from '@modelcontextprotocol/core';
920

1021
import type { AnyToolHandler, McpServer, RegisteredTool } from '../../server/mcp.js';
1122
import type { ToolTaskHandler } from './interfaces.js';
23+
import { isToolTaskHandler } from './interfaces.js';
1224

1325
/**
14-
* Internal interface for accessing {@linkcode McpServer}'s private _createRegisteredTool method.
26+
* Internal interface for accessing {@linkcode McpServer}'s private members.
1527
* @internal
1628
*/
1729
interface McpServerInternal {
@@ -26,6 +38,7 @@ interface McpServerInternal {
2638
_meta: Record<string, unknown> | undefined,
2739
handler: AnyToolHandler<StandardSchemaWithJSON | undefined>
2840
): RegisteredTool;
41+
_registeredTools: { [name: string]: RegisteredTool };
2942
}
3043

3144
/**
@@ -39,14 +52,63 @@ interface McpServerInternal {
3952
* @experimental
4053
*/
4154
export class ExperimentalMcpServerTasks {
55+
/**
56+
* Maps taskId → toolName for tasks whose handlers define custom
57+
* `getTask` or `getTaskResult`. In-memory only; after a server restart
58+
* or on a different instance, lookups fall through to the TaskStore.
59+
*/
60+
private _taskToTool = new Map<string, string>();
61+
4262
constructor(private readonly _mcpServer: McpServer) {}
4363

64+
/** @internal */
65+
_installOverrides(taskManager: TaskManager): void {
66+
taskManager.setTaskOverrides({
67+
getTask: (taskId, ctx) => this._dispatch(taskId, ctx, 'getTask'),
68+
getTaskResult: (taskId, ctx) => this._dispatch(taskId, ctx, 'getTaskResult')
69+
});
70+
}
71+
72+
/** @internal */
73+
_recordTask(taskId: string, toolName: string): void {
74+
const tool = (this._mcpServer as unknown as McpServerInternal)._registeredTools[toolName];
75+
if (tool && isToolTaskHandler(tool.handler) && (tool.handler.getTask || tool.handler.getTaskResult)) {
76+
this._taskToTool.set(taskId, toolName);
77+
}
78+
}
79+
80+
private async _dispatch<M extends 'getTask' | 'getTaskResult'>(
81+
taskId: string,
82+
ctx: BaseContext,
83+
method: M
84+
): Promise<(M extends 'getTask' ? GetTaskResult : CallToolResult) | undefined> {
85+
const toolName = this._taskToTool.get(taskId);
86+
if (!toolName) return undefined;
87+
88+
const tool = (this._mcpServer as unknown as McpServerInternal)._registeredTools[toolName];
89+
if (!tool || !isToolTaskHandler(tool.handler)) return undefined;
90+
91+
const handler = tool.handler[method];
92+
if (!handler) return undefined;
93+
94+
const serverCtx = ctx as ServerContext;
95+
if (!serverCtx.task?.store) return undefined;
96+
97+
const taskCtx: TaskServerContext = {
98+
...serverCtx,
99+
task: { ...serverCtx.task, id: taskId, store: serverCtx.task.store }
100+
};
101+
102+
return handler(taskCtx) as M extends 'getTask' ? GetTaskResult : CallToolResult;
103+
}
104+
44105
/**
45106
* Registers a task-based tool with a config object and handler.
46107
*
47108
* Task-based tools support long-running operations that can be polled for status
48-
* and results. The handler must implement {@linkcode ToolTaskHandler.createTask | createTask}, {@linkcode ToolTaskHandler.getTask | getTask}, and {@linkcode ToolTaskHandler.getTaskResult | getTaskResult}
49-
* methods.
109+
* and results. The handler implements {@linkcode ToolTaskHandler.createTask | createTask}
110+
* to start the task; subsequent `tasks/get` and `tasks/result` requests are served
111+
* from the configured {@linkcode TaskStore}.
50112
*
51113
* @example
52114
* ```typescript
@@ -59,19 +121,13 @@ export class ExperimentalMcpServerTasks {
59121
* const task = await ctx.task.store.createTask({ ttl: 300000 });
60122
* startBackgroundWork(task.taskId, args);
61123
* return { task };
62-
* },
63-
* getTask: async (args, ctx) => {
64-
* return ctx.task.store.getTask(ctx.task.id);
65-
* },
66-
* getTaskResult: async (args, ctx) => {
67-
* return ctx.task.store.getTaskResult(ctx.task.id);
68124
* }
69125
* });
70126
* ```
71127
*
72128
* @param name - The tool name
73129
* @param config - Tool configuration (description, schemas, etc.)
74-
* @param handler - Task handler with {@linkcode ToolTaskHandler.createTask | createTask}, {@linkcode ToolTaskHandler.getTask | getTask}, {@linkcode ToolTaskHandler.getTaskResult | getTaskResult} methods
130+
* @param handler - Task handler with {@linkcode ToolTaskHandler.createTask | createTask}
75131
* @returns {@linkcode server/mcp.RegisteredTool | RegisteredTool} for managing the tool's lifecycle
76132
*
77133
* @experimental

0 commit comments

Comments
 (0)