mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
TOTAL_MCP_TOOL_COUNT in open-sse/mcp-server/server.ts summed collection sizes additively, double-counting tools registered in more than one collection. The agent-skills trio (omniroute_agent_skills_list/get/coverage) is intentionally defined in both MCP_TOOLS (schemas/tools.ts) and agentSkillTools (tools/agentSkillTools.ts), inflating the reported count from 96 unique tools to 99. Replace the additive sum with countUniqueMcpTools() (new open-sse/mcp-server/toolCount.ts), which unions all collection tool names into a Set before counting, so any future overlap self-corrects instead of double-counting. Regression test: tests/unit/mcp-tool-count-dedup-6854.test.ts
This commit is contained in:
committed by
GitHub
parent
c089ca9d1a
commit
5c0a0d8db9
1
changelog.d/fixes/6854-6854-mcp-toolcount.md
Normal file
1
changelog.d/fixes/6854-6854-mcp-toolcount.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(mcp): de-duplicate `TOTAL_MCP_TOOL_COUNT` by tool name instead of double-counting collections (#6854)
|
||||
@@ -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<string, unknown>;
|
||||
|
||||
|
||||
36
open-sse/mcp-server/toolCount.ts
Normal file
36
open-sse/mcp-server/toolCount.ts
Normal file
@@ -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<Record<string, NamedTool>>;
|
||||
|
||||
function collectionNames(collection: ToolCollection): string[] {
|
||||
const items: NamedTool[] = Array.isArray(collection)
|
||||
? collection
|
||||
: Object.values(collection as Record<string, NamedTool>);
|
||||
return items.map((item) => item.name);
|
||||
}
|
||||
|
||||
export function countUniqueMcpTools(
|
||||
collectionsByLabel: Readonly<Record<string, ToolCollection>>
|
||||
): number {
|
||||
const uniqueNames = new Set<string>();
|
||||
for (const collection of Object.values(collectionsByLabel)) {
|
||||
for (const name of collectionNames(collection)) {
|
||||
uniqueNames.add(name);
|
||||
}
|
||||
}
|
||||
return uniqueNames.size;
|
||||
}
|
||||
83
tests/unit/mcp-tool-count-dedup-6854.test.ts
Normal file
83
tests/unit/mcp-tool-count-dedup-6854.test.ts
Normal file
@@ -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, NamedTool>): 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<string, NamedTool>);
|
||||
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<string, NamedTool>,
|
||||
skillTools: skillTools as unknown as Record<string, NamedTool>,
|
||||
agentSkillTools: agentSkillTools as unknown as Record<string, NamedTool>,
|
||||
githubSkillTools: githubSkillTools as unknown as Record<string, NamedTool>,
|
||||
poolTools: poolTools as unknown as Record<string, NamedTool>,
|
||||
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<string>();
|
||||
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"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user