Skip to content

Commit dc09b1d

Browse files
fix: address review — guard cancel() once response received (abort race during async result validation); add core to changeset; setNotificationHandler JSDoc notes 2nd arg
1 parent 34a4fd5 commit dc09b1d

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

.changeset/custom-methods-minimal.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
---
2+
'@modelcontextprotocol/core': minor
23
'@modelcontextprotocol/client': minor
34
'@modelcontextprotocol/server': minor
45
---

packages/core/src/shared/protocol.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,12 @@ export abstract class Protocol<ContextT extends BaseContext> {
884884
};
885885
}
886886

887+
let responseReceived = false;
888+
887889
const cancel = (reason: unknown) => {
890+
if (responseReceived) {
891+
return;
892+
}
888893
this._progressHandlers.delete(messageId);
889894

890895
this._transport
@@ -910,6 +915,7 @@ export abstract class Protocol<ContextT extends BaseContext> {
910915
if (options?.signal?.aborted) {
911916
return;
912917
}
918+
responseReceived = true;
913919

914920
if (response instanceof Error) {
915921
return reject(response);
@@ -1143,7 +1149,8 @@ export abstract class Protocol<ContextT extends BaseContext> {
11431149
* For spec methods, pass `(method, handler)`; the notification is parsed with the
11441150
* spec schema. For custom (non-spec) methods, pass `(method, schemas, handler)`;
11451151
* `params` are validated against `schemas.params` and the handler receives the
1146-
* parsed params object directly.
1152+
* parsed params object directly. The raw notification is passed as the second
1153+
* argument; `_meta` is recoverable via `notification.params?._meta`.
11471154
*/
11481155
setNotificationHandler<M extends NotificationMethod>(
11491156
method: M,

packages/core/test/shared/customMethods.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest';
22
import { z } from 'zod/v4';
33

44
import { Protocol } from '../../src/shared/protocol.js';
5-
import type { BaseContext, JSONRPCRequest, Result } from '../../src/exports/public/index.js';
5+
import type { BaseContext, JSONRPCRequest, Result, StandardSchemaV1 } from '../../src/exports/public/index.js';
66
import { ProtocolError, ProtocolErrorCode } from '../../src/types/index.js';
77
import { InMemoryTransport } from '../../src/util/inMemory.js';
88

@@ -150,6 +150,34 @@ describe('Protocol custom-method support', () => {
150150
code: ProtocolErrorCode.InternalError
151151
});
152152
});
153+
154+
it('returns the result (and sends no cancellation) if the signal aborts during async result-schema validation', async () => {
155+
const [a, b] = await pair();
156+
b.setRequestHandler('acme/echo', { params: z.object({}) }, async () => ({ echoed: 'ok' }));
157+
158+
const cancelled: unknown[] = [];
159+
b.setNotificationHandler('notifications/cancelled', n => {
160+
cancelled.push(n);
161+
});
162+
163+
const ac = new AbortController();
164+
const AsyncEcho: StandardSchemaV1<unknown, { echoed: string }> = {
165+
'~standard': {
166+
version: 1,
167+
vendor: 'test',
168+
validate: value =>
169+
new Promise(r => {
170+
ac.abort();
171+
setTimeout(() => r({ value: value as { echoed: string } }), 0);
172+
})
173+
}
174+
};
175+
176+
const result = await a.request({ method: 'acme/echo', params: {} }, AsyncEcho, { signal: ac.signal });
177+
expect(result).toEqual({ echoed: 'ok' });
178+
await new Promise(r => setTimeout(r, 0));
179+
expect(cancelled).toHaveLength(0);
180+
});
153181
});
154182

155183
describe('ctx.mcpReq.send schema overload', () => {

0 commit comments

Comments
 (0)