Skip to content

Commit 2e2d0b5

Browse files
cameroncookecodex
andcommitted
fix(mcp): Avoid leaked idle request tracking
Avoid inflating the MCP idle in-flight counter when a client reuses a pending JSON-RPC request id. Also clear the pending request if downstream message handling throws synchronously before a response can be sent. Refs #394 Co-Authored-By: Codex <noreply@openai.com>
1 parent 6ca2dd7 commit 2e2d0b5

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

src/server/__tests__/server.test.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,20 @@ describe('MCP server transport request lifecycle instrumentation', () => {
101101
expect(onRequestCompleted).not.toHaveBeenCalled();
102102
});
103103

104-
it('completes duplicate responses only once', async () => {
104+
it('ignores duplicate request IDs until the pending request completes', async () => {
105+
const onRequestStarted = vi.fn();
105106
const onRequestCompleted = vi.fn();
106-
const { transport } = await createStartedInstrumentedTransport({ onRequestCompleted });
107+
const { transport } = await createStartedInstrumentedTransport({
108+
onRequestStarted,
109+
onRequestCompleted,
110+
});
107111

108112
transport.onmessage?.({ jsonrpc: '2.0', id: '1', method: 'initialize' });
113+
transport.onmessage?.({ jsonrpc: '2.0', id: '1', method: 'tools/list' });
109114
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
110115
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
111116

117+
expect(onRequestStarted).toHaveBeenCalledTimes(1);
112118
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
113119
});
114120

@@ -124,4 +130,26 @@ describe('MCP server transport request lifecycle instrumentation', () => {
124130

125131
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
126132
});
133+
134+
it('marks completion when downstream message handling throws synchronously', async () => {
135+
const onRequestStarted = vi.fn();
136+
const onRequestCompleted = vi.fn();
137+
const transport = new TestTransport();
138+
const downstreamOnMessage = vi.fn(() => {
139+
throw new Error('handler failed');
140+
});
141+
instrumentMcpRequestLifecycle(transport, { onRequestStarted, onRequestCompleted });
142+
143+
transport.onmessage = downstreamOnMessage;
144+
await transport.start();
145+
146+
expect(() => {
147+
transport.onmessage?.({ jsonrpc: '2.0', id: '1', method: 'tools/list' });
148+
}).toThrow('handler failed');
149+
150+
expect(onRequestStarted).toHaveBeenCalledTimes(1);
151+
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
152+
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
153+
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
154+
});
127155
});

src/server/request-lifecycle.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,25 @@ export function instrumentMcpRequestLifecycle(
4747
onMessageWrapped = true;
4848
const downstreamOnMessage = transport.onmessage;
4949
transport.onmessage = (message, extra) => {
50+
let startedRequestId: string | null = null;
51+
5052
if (isJSONRPCRequest(message)) {
51-
pendingRequestIds.add(requestIdKey(message.id));
52-
observer.onRequestStarted?.();
53+
const requestId = requestIdKey(message.id);
54+
if (!pendingRequestIds.has(requestId)) {
55+
pendingRequestIds.add(requestId);
56+
startedRequestId = requestId;
57+
observer.onRequestStarted?.();
58+
}
5359
}
5460

55-
downstreamOnMessage(message, extra);
61+
try {
62+
downstreamOnMessage(message, extra);
63+
} catch (error) {
64+
if (startedRequestId !== null && pendingRequestIds.delete(startedRequestId)) {
65+
observer.onRequestCompleted?.();
66+
}
67+
throw error;
68+
}
5669
};
5770
};
5871

0 commit comments

Comments
 (0)