Skip to content

Commit e672c72

Browse files
committed
fix: Move notification calls from internal _createRegisteredTool to public methods
The _createRegisteredTool() method was calling setToolRequestHandlers() and sendToolListChanged() internally, which caused issues when registering tools after server connection due to capability registration restrictions. This moves the notification responsibility to the public registerTool() and tool() methods, making tool registration consistent and allowing registration after connection when capabilities have already been established. Fixes memory leak warnings caused by rapid tool registration after connection.
1 parent a1608a6 commit e672c72

File tree

2 files changed

+128
-5
lines changed

2 files changed

+128
-5
lines changed

src/server/mcp.test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4290,4 +4290,120 @@ describe("elicitInput()", () => {
42904290
text: "No booking made. Original date not available."
42914291
}]);
42924292
});
4293+
4294+
/***
4295+
* Test: Bulk Tool Registration - Memory Leak Prevention
4296+
*/
4297+
test("should handle multiple tool registrations without memory leak", async () => {
4298+
const mcpServer = new McpServer({
4299+
name: "test server",
4300+
version: "1.0",
4301+
});
4302+
4303+
// Register first tool before connection (should work fine)
4304+
mcpServer.tool("tool1", async () => ({
4305+
content: [{ type: "text", text: "Tool 1" }]
4306+
}));
4307+
4308+
const notifications: Notification[] = []
4309+
const client = new Client({
4310+
name: "test client",
4311+
version: "1.0",
4312+
});
4313+
client.fallbackNotificationHandler = async (notification) => {
4314+
notifications.push(notification)
4315+
}
4316+
4317+
const [clientTransport, serverTransport] =
4318+
InMemoryTransport.createLinkedPair();
4319+
4320+
await Promise.all([
4321+
client.connect(clientTransport),
4322+
mcpServer.connect(serverTransport),
4323+
]);
4324+
4325+
// Clear any initial notifications
4326+
notifications.length = 0;
4327+
4328+
// This should work since capabilities were already registered during first tool
4329+
mcpServer.tool("tool2", async () => ({
4330+
content: [{ type: "text", text: "Tool 2" }]
4331+
}));
4332+
4333+
// Yield event loop to let notifications process
4334+
await new Promise(process.nextTick);
4335+
4336+
// Should have sent exactly one notification for the second tool
4337+
expect(notifications).toHaveLength(1);
4338+
expect(notifications[0]).toMatchObject({
4339+
method: "notifications/tools/list_changed",
4340+
});
4341+
4342+
// Verify both tools are registered
4343+
const toolsResult = await client.request(
4344+
{ method: "tools/list" },
4345+
ListToolsResultSchema,
4346+
);
4347+
expect(toolsResult.tools).toHaveLength(2);
4348+
expect(toolsResult.tools.map(t => t.name).sort()).toEqual(["tool1", "tool2"]);
4349+
});
4350+
4351+
/***
4352+
* Test: registerTool() should work after connection (fixed behavior)
4353+
*/
4354+
test("registerTool() should work after connection", async () => {
4355+
const mcpServer = new McpServer({
4356+
name: "test server",
4357+
version: "1.0",
4358+
});
4359+
4360+
// Register first tool before connection to establish capabilities
4361+
mcpServer.tool("tool1", async () => ({
4362+
content: [{ type: "text", text: "Tool 1" }]
4363+
}));
4364+
4365+
const notifications: Notification[] = []
4366+
const client = new Client({
4367+
name: "test client",
4368+
version: "1.0",
4369+
});
4370+
client.fallbackNotificationHandler = async (notification) => {
4371+
notifications.push(notification)
4372+
}
4373+
4374+
const [clientTransport, serverTransport] =
4375+
InMemoryTransport.createLinkedPair();
4376+
4377+
await Promise.all([
4378+
client.connect(clientTransport),
4379+
mcpServer.connect(serverTransport),
4380+
]);
4381+
4382+
// Clear any initial notifications
4383+
notifications.length = 0;
4384+
4385+
// This should now work after the fix
4386+
mcpServer.registerTool("test2", {
4387+
description: "Test tool 2"
4388+
}, async () => ({
4389+
content: [{ type: "text", text: "Test 2" }]
4390+
}));
4391+
4392+
// Yield event loop to let notifications process
4393+
await new Promise(process.nextTick);
4394+
4395+
// Should have sent exactly one notification
4396+
expect(notifications).toHaveLength(1);
4397+
expect(notifications[0]).toMatchObject({
4398+
method: "notifications/tools/list_changed",
4399+
});
4400+
4401+
// Verify both tools are registered
4402+
const toolsResult = await client.request(
4403+
{ method: "tools/list" },
4404+
ListToolsResultSchema,
4405+
);
4406+
expect(toolsResult.tools).toHaveLength(2);
4407+
expect(toolsResult.tools.map(t => t.name).sort()).toEqual(["test2", "tool1"]);
4408+
});
42934409
});

src/server/mcp.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -803,9 +803,6 @@ export class McpServer {
803803
};
804804
this._registeredTools[name] = registeredTool;
805805

806-
this.setToolRequestHandlers();
807-
this.sendToolListChanged()
808-
809806
return registeredTool
810807
}
811808

@@ -914,7 +911,12 @@ export class McpServer {
914911
}
915912
const callback = rest[0] as ToolCallback<ZodRawShape | undefined>;
916913

917-
return this._createRegisteredTool(name, undefined, description, inputSchema, outputSchema, annotations, callback)
914+
const result = this._createRegisteredTool(name, undefined, description, inputSchema, outputSchema, annotations, callback);
915+
916+
this.setToolRequestHandlers();
917+
this.sendToolListChanged();
918+
919+
return result;
918920
}
919921

920922
/**
@@ -937,7 +939,7 @@ export class McpServer {
937939

938940
const { title, description, inputSchema, outputSchema, annotations } = config;
939941

940-
return this._createRegisteredTool(
942+
const result = this._createRegisteredTool(
941943
name,
942944
title,
943945
description,
@@ -946,6 +948,11 @@ export class McpServer {
946948
annotations,
947949
cb as ToolCallback<ZodRawShape | undefined>
948950
);
951+
952+
this.setToolRequestHandlers();
953+
this.sendToolListChanged();
954+
955+
return result;
949956
}
950957

951958
/**

0 commit comments

Comments
 (0)