diff --git a/changelog.d/fixes/6854-6854-mcp-toolcount.md b/changelog.d/fixes/6854-6854-mcp-toolcount.md new file mode 100644 index 0000000000..1e0ac6b1fe --- /dev/null +++ b/changelog.d/fixes/6854-6854-mcp-toolcount.md @@ -0,0 +1 @@ +- fix(mcp): de-duplicate `TOTAL_MCP_TOOL_COUNT` by tool name instead of double-counting collections (#6854) diff --git a/open-sse/mcp-server/server.ts b/open-sse/mcp-server/server.ts index a9428ba310..4d8b7be2de 100644 --- a/open-sse/mcp-server/server.ts +++ b/open-sse/mcp-server/server.ts @@ -37,6 +37,7 @@ import { oneproxyStatsInput, } from "./schemas/tools.ts"; import { startMcpHeartbeat } from "./runtimeHeartbeat.ts"; +import { countUniqueMcpTools } from "./toolCount.ts"; import { z } from "zod"; import { closeAuditDb, logToolCall } from "./audit.ts"; import { @@ -98,17 +99,18 @@ const MCP_ALLOWED_SCOPES = new Set( .map((s) => s.trim()) .filter(Boolean) ); -const TOTAL_MCP_TOOL_COUNT = - MCP_TOOLS.length + - Object.keys(memoryTools).length + - Object.keys(skillTools).length + - Object.keys(agentSkillTools).length + - Object.keys(githubSkillTools).length + - Object.keys(poolTools).length + - gamificationTools.length + - pluginTools.length + - notionTools.length + - obsidianTools.length; +const TOTAL_MCP_TOOL_COUNT = countUniqueMcpTools({ + MCP_TOOLS, + memoryTools, + skillTools, + agentSkillTools, + githubSkillTools, + poolTools, + gamificationTools, + pluginTools, + notionTools, + obsidianTools, +}); type JsonRecord = Record; diff --git a/open-sse/mcp-server/toolCount.ts b/open-sse/mcp-server/toolCount.ts new file mode 100644 index 0000000000..068adac6b6 --- /dev/null +++ b/open-sse/mcp-server/toolCount.ts @@ -0,0 +1,36 @@ +/** + * countUniqueMcpTools — de-duplicated MCP tool count. + * + * The various tool collections registered by the MCP server (array-shaped, e.g. + * `MCP_TOOLS`, and record-shaped, e.g. `memoryTools`) are not guaranteed disjoint by + * tool `name` — some tools (e.g. the agent-skills trio) are intentionally defined in + * both an array collection and a record collection for registration purposes. Summing + * `collection.length` / `Object.keys(collection).length` across all sources therefore + * double-counts any name that appears in more than one source. + * + * This helper unions every collection's tool names into a `Set` and returns the size + * of that set, so the reported tool count always reflects distinct, user-visible tool + * names regardless of how many internal collections a given tool happens to appear in. + */ + +type NamedTool = { name: string }; +type ToolCollection = readonly NamedTool[] | Readonly>; + +function collectionNames(collection: ToolCollection): string[] { + const items: NamedTool[] = Array.isArray(collection) + ? collection + : Object.values(collection as Record); + return items.map((item) => item.name); +} + +export function countUniqueMcpTools( + collectionsByLabel: Readonly> +): number { + const uniqueNames = new Set(); + for (const collection of Object.values(collectionsByLabel)) { + for (const name of collectionNames(collection)) { + uniqueNames.add(name); + } + } + return uniqueNames.size; +} diff --git a/tests/unit/mcp-tool-count-dedup-6854.test.ts b/tests/unit/mcp-tool-count-dedup-6854.test.ts new file mode 100644 index 0000000000..f578a761cb --- /dev/null +++ b/tests/unit/mcp-tool-count-dedup-6854.test.ts @@ -0,0 +1,83 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// Regression for #6854: TOTAL_MCP_TOOL_COUNT (open-sse/mcp-server/server.ts) was a +// plain additive sum across all registered tool collections. Three tools +// (omniroute_agent_skills_list/get/coverage) are intentionally defined in BOTH +// MCP_TOOLS (open-sse/mcp-server/schemas/tools.ts) and agentSkillTools +// (open-sse/mcp-server/tools/agentSkillTools.ts), so the additive sum reported 99 +// while only 96 distinct tool names actually exist. countUniqueMcpTools +// (open-sse/mcp-server/toolCount.ts) fixes this by unioning tool names into a Set +// before counting, so a tool present in multiple collections is only counted once. + +const { countUniqueMcpTools } = await import("../../open-sse/mcp-server/toolCount.ts"); +const { MCP_TOOLS } = await import("../../open-sse/mcp-server/schemas/tools.ts"); +const { memoryTools } = await import("../../open-sse/mcp-server/tools/memoryTools.ts"); +const { skillTools } = await import("../../open-sse/mcp-server/tools/skillTools.ts"); +const { agentSkillTools } = await import("../../open-sse/mcp-server/tools/agentSkillTools.ts"); +const { githubSkillTools } = await import("../../open-sse/mcp-server/tools/githubSkillTools.ts"); +const { poolTools } = await import("../../open-sse/mcp-server/tools/poolTools.ts"); +const { gamificationTools } = await import("../../open-sse/mcp-server/tools/gamificationTools.ts"); +const { pluginTools } = await import("../../open-sse/mcp-server/tools/pluginTools.ts"); +const { notionTools } = await import("../../open-sse/mcp-server/tools/notionTools.ts"); +const { obsidianTools } = await import("../../open-sse/mcp-server/tools/obsidianTools.ts"); + +type NamedTool = { name: string }; + +function namesOf(collection: readonly NamedTool[] | Record): string[] { + return Array.isArray(collection) + ? collection.map((t) => t.name) + : Object.values(collection).map((t) => t.name); +} + +test("#6854: countUniqueMcpTools de-duplicates tools registered in multiple collections", () => { + // The agent-skills trio is intentionally present in both MCP_TOOLS and agentSkillTools. + const mcpToolsNames = namesOf(MCP_TOOLS as unknown as NamedTool[]); + const agentSkillNames = namesOf(agentSkillTools as unknown as Record); + const overlap = mcpToolsNames.filter((n) => agentSkillNames.includes(n)); + assert.ok( + overlap.length > 0, + "expected MCP_TOOLS and agentSkillTools to still share the agent-skills tool names " + + "(if this fails because the overlap was removed instead, this test's premise no " + + "longer applies and it should be revisited)" + ); + + const collections = { + MCP_TOOLS: MCP_TOOLS as unknown as NamedTool[], + memoryTools: memoryTools as unknown as Record, + skillTools: skillTools as unknown as Record, + agentSkillTools: agentSkillTools as unknown as Record, + githubSkillTools: githubSkillTools as unknown as Record, + poolTools: poolTools as unknown as Record, + gamificationTools: gamificationTools as unknown as NamedTool[], + pluginTools: pluginTools as unknown as NamedTool[], + notionTools: notionTools as unknown as NamedTool[], + obsidianTools: obsidianTools as unknown as NamedTool[], + }; + + const total = countUniqueMcpTools(collections); + + // Independently compute the "true" unique count by unioning every collection's + // tool names into a Set — this must equal countUniqueMcpTools's own result AND + // must be strictly less than the naive additive sum whenever there is overlap. + const uniqueNames = new Set(); + for (const collection of Object.values(collections)) { + for (const name of namesOf(collection)) uniqueNames.add(name); + } + + const naiveAdditiveSum = Object.values(collections).reduce( + (sum, collection) => sum + namesOf(collection).length, + 0 + ); + + assert.equal(total, uniqueNames.size, "countUniqueMcpTools must equal the unique-name count"); + assert.equal( + total, + naiveAdditiveSum - overlap.length, + "unique count must be exactly the additive sum minus the double-counted overlap" + ); + assert.ok( + total < naiveAdditiveSum, + "unique count must be strictly less than the naive additive sum given a known overlap" + ); +});