Fix deregistration bug in transport.off() (#1409)

This commit is contained in:
Steven Enamakel
2026-05-09 13:48:16 -07:00
committed by GitHub
parent cc6092e631
commit b30e7cc036
2 changed files with 35 additions and 8 deletions
+16 -7
View File
@@ -108,13 +108,22 @@ describe('SocketIOMCPTransportImpl — on / off', () => {
expect(handler).toHaveBeenCalledWith({ data: 42 });
});
// TODO: transport.off() passes the original handler reference to socket.off() instead of
// the wrapped handler registered via socket.on(). This means the wrapped handler may still
// fire after off() is called — the deregistration is a no-op in practice. Fix the bug in
// transport.ts first, then replace this todo with a behavioural assertion.
it.todo(
'off removes the handler (pending fix: off() passes original instead of wrapped handler)'
);
it('off removes the handler by resolving the wrapped handler', () => {
const socket = makeSocket();
const transport = new SocketIOMCPTransportImpl(socket as never);
const handler = vi.fn();
transport.on('tool_call', handler);
// Trigger via the raw socket to ensure it works
socket.trigger('mcp:tool_call', { data: 42 });
expect(handler).toHaveBeenCalledWith({ data: 42 });
expect(handler).toHaveBeenCalledTimes(1);
transport.off('tool_call', handler);
// Trigger again, the handler should NOT be called
socket.trigger('mcp:tool_call', { data: 84 });
expect(handler).toHaveBeenCalledTimes(1); // Still 1
});
});
describe('SocketIOMCPTransportImpl — request / response routing', () => {
+19 -1
View File
@@ -11,6 +11,7 @@ import type { MCPRequest, MCPResponse, SocketIOMCPTransport } from './types';
export class SocketIOMCPTransportImpl implements SocketIOMCPTransport {
private socket: Socket | null | undefined;
private requestHandlers = new Map<string | number, (response: MCPResponse) => void>();
private eventHandlers = new Map<string, Map<Function, Function>>();
private readonly eventPrefix = 'mcp:';
private responseHandler = (response: MCPResponse): void => {
mcpLog(
@@ -86,12 +87,29 @@ export class SocketIOMCPTransportImpl implements SocketIOMCPTransport {
mcpLog('Received event', createSafeLogData({ event: fullEvent }, data));
handler(data);
};
let handlersForEvent = this.eventHandlers.get(fullEvent);
if (!handlersForEvent) {
handlersForEvent = new Map();
this.eventHandlers.set(fullEvent, handlersForEvent);
}
handlersForEvent.set(handler, wrappedHandler);
this.socket.on(fullEvent, wrappedHandler);
}
off(event: string, handler: (data: unknown) => void): void {
if (!this.socket) return;
this.socket.off(`${this.eventPrefix}${event}`, handler);
const fullEvent = `${this.eventPrefix}${event}`;
const handlersForEvent = this.eventHandlers.get(fullEvent);
const wrappedHandler = handlersForEvent?.get(handler);
if (wrappedHandler) {
this.socket.off(fullEvent, wrappedHandler as any);
handlersForEvent?.delete(handler);
} else {
this.socket.off(fullEvent, handler);
}
}
async request(request: MCPRequest, timeoutMs = 30000): Promise<MCPResponse> {