Skip to content

Commit a606fb1

Browse files
KKonstantinovKKonstantinov
authored
SPEC COMPLIANCE: Remove loose/passthrough types not allowed/defined by MCP spec + Task types (#1242)
1 parent 4d6c3b8 commit a606fb1

File tree

14 files changed

+866
-488
lines changed

14 files changed

+866
-488
lines changed

‎src/client/index.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,8 @@ import {
2828
ListToolsResultSchema,
2929
type LoggingLevel,
3030
McpError,
31-
type Notification,
3231
type ReadResourceRequest,
3332
ReadResourceResultSchema,
34-
type Request,
35-
type Result,
3633
type ServerCapabilities,
3734
SUPPORTED_PROTOCOL_VERSIONS,
3835
type SubscribeRequest,
@@ -48,7 +45,10 @@ import {
4845
ResourceListChangedNotificationSchema,
4946
ListChangedOptions,
5047
ListChangedOptionsBaseSchema,
51-
type ListChangedHandlers
48+
type ListChangedHandlers,
49+
type Request,
50+
type Notification,
51+
type Result
5252
} from '../types.js';
5353
import { AjvJsonSchemaValidator } from '../validation/ajv-provider.js';
5454
import type { JsonSchemaType, JsonSchemaValidator, jsonSchemaValidator } from '../validation/types.js';
@@ -368,14 +368,14 @@ export class Client<
368368
}
369369

370370
const { params } = validatedRequest.data;
371-
const mode = params.mode ?? 'form';
371+
params.mode = params.mode ?? 'form';
372372
const { supportsFormMode, supportsUrlMode } = getSupportedElicitationModes(this._capabilities.elicitation);
373373

374-
if (mode === 'form' && !supportsFormMode) {
374+
if (params.mode === 'form' && !supportsFormMode) {
375375
throw new McpError(ErrorCode.InvalidParams, 'Client does not support form-mode elicitation requests');
376376
}
377377

378-
if (mode === 'url' && !supportsUrlMode) {
378+
if (params.mode === 'url' && !supportsUrlMode) {
379379
throw new McpError(ErrorCode.InvalidParams, 'Client does not support URL-mode elicitation requests');
380380
}
381381

@@ -404,9 +404,9 @@ export class Client<
404404
}
405405

406406
const validatedResult = validationResult.data;
407-
const requestedSchema = mode === 'form' ? (params.requestedSchema as JsonSchemaType) : undefined;
407+
const requestedSchema = params.mode === 'form' ? (params.requestedSchema as JsonSchemaType) : undefined;
408408

409-
if (mode === 'form' && validatedResult.action === 'accept' && validatedResult.content && requestedSchema) {
409+
if (params.mode === 'form' && validatedResult.action === 'accept' && validatedResult.content && requestedSchema) {
410410
if (this._capabilities.elicitation?.form?.applyDefaults) {
411411
try {
412412
applyElicitationDefaults(requestedSchema, validatedResult.content);

‎src/client/streamableHttp.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Transport, FetchLike, createFetchWithInit, normalizeHeaders } from '../shared/transport.js';
2-
import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResponse, JSONRPCMessage, JSONRPCMessageSchema } from '../types.js';
2+
import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResultResponse, JSONRPCMessage, JSONRPCMessageSchema } from '../types.js';
33
import { auth, AuthResult, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js';
44
import { EventSourceParserStream } from 'eventsource-parser/stream';
55

@@ -350,7 +350,7 @@ export class StreamableHTTPClientTransport implements Transport {
350350
if (!event.event || event.event === 'message') {
351351
try {
352352
const message = JSONRPCMessageSchema.parse(JSON.parse(event.data));
353-
if (isJSONRPCResponse(message)) {
353+
if (isJSONRPCResultResponse(message)) {
354354
// Mark that we received a response - no need to reconnect for this request
355355
receivedResponse = true;
356356
if (replayMessageId !== undefined) {

‎src/examples/server/simpleTaskInteractive.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ import {
3434
ListToolsRequestSchema,
3535
CallToolRequestSchema,
3636
GetTaskRequestSchema,
37-
GetTaskPayloadRequestSchema
37+
GetTaskPayloadRequestSchema,
38+
GetTaskPayloadResult
3839
} from '../../types.js';
3940
import { TaskMessageQueue, QueuedMessage, QueuedRequest, isTerminal, CreateTaskOptions } from '../../experimental/tasks/interfaces.js';
4041
import { InMemoryTaskStore } from '../../experimental/tasks/stores/in-memory.js';
@@ -618,7 +619,7 @@ const createServer = (): Server => {
618619
});
619620

620621
// Handle tasks/result
621-
server.setRequestHandler(GetTaskPayloadRequestSchema, async (request, extra): Promise<Result> => {
622+
server.setRequestHandler(GetTaskPayloadRequestSchema, async (request, extra): Promise<GetTaskPayloadResult> => {
622623
const { taskId } = request.params;
623624
console.log(`[Server] tasks/result called for task ${taskId}`);
624625
return taskResultHandler.handle(taskId, server, extra.sessionId ?? '');

‎src/experimental/tasks/interfaces.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,18 @@
55

66
import {
77
Task,
8-
Request,
98
RequestId,
109
Result,
1110
JSONRPCRequest,
1211
JSONRPCNotification,
13-
JSONRPCResponse,
14-
JSONRPCError,
12+
JSONRPCResultResponse,
13+
JSONRPCErrorResponse,
1514
ServerRequest,
1615
ServerNotification,
1716
CallToolResult,
1817
GetTaskResult,
19-
ToolExecution
18+
ToolExecution,
19+
Request
2020
} from '../../types.js';
2121
import { CreateTaskResult } from './types.js';
2222
import type { RequestHandlerExtra, RequestTaskStore } from '../../shared/protocol.js';
@@ -124,13 +124,13 @@ export interface QueuedNotification extends BaseQueuedMessage {
124124
export interface QueuedResponse extends BaseQueuedMessage {
125125
type: 'response';
126126
/** The actual JSONRPC response */
127-
message: JSONRPCResponse;
127+
message: JSONRPCResultResponse;
128128
}
129129

130130
export interface QueuedError extends BaseQueuedMessage {
131131
type: 'error';
132132
/** The actual JSONRPC error */
133-
message: JSONRPCError;
133+
message: JSONRPCErrorResponse;
134134
}
135135

136136
/**

‎src/experimental/tasks/stores/in-memory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* @experimental
66
*/
77

8-
import { Task, Request, RequestId, Result } from '../../../types.js';
8+
import { Task, RequestId, Result, Request } from '../../../types.js';
99
import { TaskStore, isTerminal, TaskMessageQueue, QueuedMessage, CreateTaskOptions } from '../interfaces.js';
1010
import { randomBytes } from 'node:crypto';
1111

‎src/server/index.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@ import {
2626
LoggingLevelSchema,
2727
type LoggingMessageNotification,
2828
McpError,
29-
type Notification,
30-
type Request,
3129
type ResourceUpdatedNotification,
32-
type Result,
3330
type ServerCapabilities,
3431
type ServerNotification,
3532
type ServerRequest,
@@ -40,7 +37,10 @@ import {
4037
type ToolUseContent,
4138
CallToolRequestSchema,
4239
CallToolResultSchema,
43-
CreateTaskResultSchema
40+
CreateTaskResultSchema,
41+
type Request,
42+
type Notification,
43+
type Result
4444
} from '../types.js';
4545
import { AjvJsonSchemaValidator } from '../validation/ajv-provider.js';
4646
import type { JsonSchemaType, jsonSchemaValidator } from '../validation/types.js';

‎src/server/streamableHttp.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ import {
44
MessageExtraInfo,
55
RequestInfo,
66
isInitializeRequest,
7-
isJSONRPCError,
87
isJSONRPCRequest,
9-
isJSONRPCResponse,
8+
isJSONRPCResultResponse,
109
JSONRPCMessage,
1110
JSONRPCMessageSchema,
1211
RequestId,
1312
SUPPORTED_PROTOCOL_VERSIONS,
14-
DEFAULT_NEGOTIATED_PROTOCOL_VERSION
13+
DEFAULT_NEGOTIATED_PROTOCOL_VERSION,
14+
isJSONRPCErrorResponse
1515
} from '../types.js';
1616
import getRawBody from 'raw-body';
1717
import contentType from 'content-type';
@@ -871,7 +871,7 @@ export class StreamableHTTPServerTransport implements Transport {
871871

872872
async send(message: JSONRPCMessage, options?: { relatedRequestId?: RequestId }): Promise<void> {
873873
let requestId = options?.relatedRequestId;
874-
if (isJSONRPCResponse(message) || isJSONRPCError(message)) {
874+
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
875875
// If the message is a response, use the request ID from the message
876876
requestId = message.id;
877877
}
@@ -881,7 +881,7 @@ export class StreamableHTTPServerTransport implements Transport {
881881
// Those will be sent via dedicated response SSE streams
882882
if (requestId === undefined) {
883883
// For standalone SSE streams, we can only send requests and notifications
884-
if (isJSONRPCResponse(message) || isJSONRPCError(message)) {
884+
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
885885
throw new Error('Cannot send a response on a standalone SSE stream unless resuming a previous client request');
886886
}
887887

@@ -924,7 +924,7 @@ export class StreamableHTTPServerTransport implements Transport {
924924
}
925925
}
926926

927-
if (isJSONRPCResponse(message) || isJSONRPCError(message)) {
927+
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
928928
this._requestResponseMap.set(requestId, message);
929929
const relatedIds = Array.from(this._requestToStreamMapping.entries())
930930
.filter(([_, streamId]) => this._streamMapping.get(streamId) === response)

‎src/shared/protocol.ts‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,20 @@ import {
1313
ListTasksResultSchema,
1414
CancelTaskRequestSchema,
1515
CancelTaskResultSchema,
16-
isJSONRPCError,
16+
isJSONRPCErrorResponse,
1717
isJSONRPCRequest,
18-
isJSONRPCResponse,
18+
isJSONRPCResultResponse,
1919
isJSONRPCNotification,
20-
JSONRPCError,
20+
JSONRPCErrorResponse,
2121
JSONRPCNotification,
2222
JSONRPCRequest,
2323
JSONRPCResponse,
2424
McpError,
25-
Notification,
2625
PingRequestSchema,
2726
Progress,
2827
ProgressNotification,
2928
ProgressNotificationSchema,
3029
RELATED_TASK_META_KEY,
31-
Request,
3230
RequestId,
3331
Result,
3432
ServerCapabilities,
@@ -41,7 +39,11 @@ import {
4139
CancelledNotification,
4240
Task,
4341
TaskStatusNotification,
44-
TaskStatusNotificationSchema
42+
TaskStatusNotificationSchema,
43+
Request,
44+
Notification,
45+
JSONRPCResultResponse,
46+
isTaskAugmentedRequestParams
4547
} from '../types.js';
4648
import { Transport, TransportSendOptions } from './transport.js';
4749
import { AuthInfo } from '../server/auth/types.js';
@@ -324,7 +326,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
324326
> = new Map();
325327
private _requestHandlerAbortControllers: Map<RequestId, AbortController> = new Map();
326328
private _notificationHandlers: Map<string, (notification: JSONRPCNotification) => Promise<void>> = new Map();
327-
private _responseHandlers: Map<number, (response: JSONRPCResponse | Error) => void> = new Map();
329+
private _responseHandlers: Map<number, (response: JSONRPCResultResponse | Error) => void> = new Map();
328330
private _progressHandlers: Map<number, ProgressCallback> = new Map();
329331
private _timeoutInfo: Map<number, TimeoutInfo> = new Map();
330332
private _pendingDebouncedNotifications = new Set<string>();
@@ -335,7 +337,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
335337
private _taskStore?: TaskStore;
336338
private _taskMessageQueue?: TaskMessageQueue;
337339

338-
private _requestResolvers: Map<RequestId, (response: JSONRPCResponse | Error) => void> = new Map();
340+
private _requestResolvers: Map<RequestId, (response: JSONRPCResultResponse | Error) => void> = new Map();
339341

340342
/**
341343
* Callback for when the connection is closed for any reason.
@@ -408,18 +410,18 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
408410
const requestId = message.id;
409411

410412
// Lookup resolver in _requestResolvers map
411-
const resolver = this._requestResolvers.get(requestId);
413+
const resolver = this._requestResolvers.get(requestId as RequestId);
412414

413415
if (resolver) {
414416
// Remove resolver from map after invocation
415-
this._requestResolvers.delete(requestId);
417+
this._requestResolvers.delete(requestId as RequestId);
416418

417419
// Invoke resolver with response or error
418420
if (queuedMessage.type === 'response') {
419-
resolver(message as JSONRPCResponse);
421+
resolver(message as JSONRPCResultResponse);
420422
} else {
421423
// Convert JSONRPCError to McpError
422-
const errorMessage = message as JSONRPCError;
424+
const errorMessage = message as JSONRPCErrorResponse;
423425
const error = new McpError(
424426
errorMessage.error.code,
425427
errorMessage.error.message,
@@ -546,6 +548,9 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
546548
}
547549

548550
private async _oncancel(notification: CancelledNotification): Promise<void> {
551+
if (!notification.params.requestId) {
552+
return;
553+
}
549554
// Handle request cancellation
550555
const controller = this._requestHandlerAbortControllers.get(notification.params.requestId);
551556
controller?.abort(notification.params.reason);
@@ -616,7 +621,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
616621
const _onmessage = this._transport?.onmessage;
617622
this._transport.onmessage = (message, extra) => {
618623
_onmessage?.(message, extra);
619-
if (isJSONRPCResponse(message) || isJSONRPCError(message)) {
624+
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
620625
this._onresponse(message);
621626
} else if (isJSONRPCRequest(message)) {
622627
this._onrequest(message, extra);
@@ -675,7 +680,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
675680
const relatedTaskId = request.params?._meta?.[RELATED_TASK_META_KEY]?.taskId;
676681

677682
if (handler === undefined) {
678-
const errorResponse: JSONRPCError = {
683+
const errorResponse: JSONRPCErrorResponse = {
679684
jsonrpc: '2.0',
680685
id: request.id,
681686
error: {
@@ -706,7 +711,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
706711
const abortController = new AbortController();
707712
this._requestHandlerAbortControllers.set(request.id, abortController);
708713

709-
const taskCreationParams = request.params?.task;
714+
const taskCreationParams = isTaskAugmentedRequestParams(request.params) ? request.params.task : undefined;
710715
const taskStore = this._taskStore ? this.requestTaskStore(request, capturedTransport?.sessionId) : undefined;
711716

712717
const fullExtra: RequestHandlerExtra<SendRequestT, SendNotificationT> = {
@@ -791,7 +796,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
791796
return;
792797
}
793798

794-
const errorResponse: JSONRPCError = {
799+
const errorResponse: JSONRPCErrorResponse = {
795800
jsonrpc: '2.0',
796801
id: request.id,
797802
error: {
@@ -852,14 +857,14 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
852857
handler(params);
853858
}
854859

855-
private _onresponse(response: JSONRPCResponse | JSONRPCError): void {
860+
private _onresponse(response: JSONRPCResponse | JSONRPCErrorResponse): void {
856861
const messageId = Number(response.id);
857862

858863
// Check if this is a response to a queued request
859864
const resolver = this._requestResolvers.get(messageId);
860865
if (resolver) {
861866
this._requestResolvers.delete(messageId);
862-
if (isJSONRPCResponse(response)) {
867+
if (isJSONRPCResultResponse(response)) {
863868
resolver(response);
864869
} else {
865870
const error = new McpError(response.error.code, response.error.message, response.error.data);
@@ -879,7 +884,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
879884

880885
// Keep progress handler alive for CreateTaskResult responses
881886
let isTaskResponse = false;
882-
if (isJSONRPCResponse(response) && response.result && typeof response.result === 'object') {
887+
if (isJSONRPCResultResponse(response) && response.result && typeof response.result === 'object') {
883888
const result = response.result as Record<string, unknown>;
884889
if (result.task && typeof result.task === 'object') {
885890
const task = result.task as Record<string, unknown>;
@@ -894,7 +899,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
894899
this._progressHandlers.delete(messageId);
895900
}
896901

897-
if (isJSONRPCResponse(response)) {
902+
if (isJSONRPCResultResponse(response)) {
898903
handler(response);
899904
} else {
900905
const error = McpError.fromError(response.error.code, response.error.message, response.error.data);
@@ -1191,7 +1196,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
11911196
const relatedTaskId = relatedTask?.taskId;
11921197
if (relatedTaskId) {
11931198
// Store the response resolver for this request so responses can be routed back
1194-
const responseResolver = (response: JSONRPCResponse | Error) => {
1199+
const responseResolver = (response: JSONRPCResultResponse | Error) => {
11951200
const handler = this._responseHandlers.get(messageId);
11961201
if (handler) {
11971202
handler(response);

0 commit comments

Comments
 (0)