Skip to content

Commit 499f6e2

Browse files
ndemiancclaude
andcommitted
fix(mcp): the startup chip's per-server counts must reflect EXPOSED tools
PR #31 review (Copilot). The chip built its per-server counts from handles[i].tools.length — the raw tools/list payload — while the run actually exposes built.tools, which is smaller after the per-server cap (MAX_TOOLS_PER_SERVER) and after junk specs are skipped. So a server listing 100 tools rendered "server (100)" even though only 64 were callable, and worse, that per-server number sat right next to an "allowed/total" denominator computed from built.tools.length — the chip contradicted itself. Per-server counts now derive from built.routes (the tools actually exposed) via a new pure helper, toolCountsByServer, so they reflect what the agent can call and SUM to the denominator. A server that exposed nothing shows "(0)" — honest, and a useful signal that it connected but contributed no usable tools. Verified: 41 tests (2 new). The key test reproduces the reviewer's scenario — a server 5 over the cap plus a server listing junk — and asserts the counts are the capped/filtered numbers AND that they sum to built.tools.length. Mutation-checked: a wrong per-tool increment fails, and dropping the malformed-entry guard fails. The agent.js consumption is one line reading the tested helper, covered by that test + reading (agent.js requires vscode). Full gate: 18 suites, 0 failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8acbe42 commit 499f6e2

3 files changed

Lines changed: 55 additions & 3 deletions

File tree

‎extensions/levelcode-ai/agent.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const providers = require('./providers/index');
1717
const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, looksReady } = require('./verify');
1818
const { classifyCommand, dangerLabel } = require('./commandSafety');
1919
const { loadProjectRules } = require('./projectRules');
20-
const { loadServerConfig, buildAgentTools, classifyMcpTool, explainMcpRefusal } = require('./mcpConfig');
20+
const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal } = require('./mcpConfig');
2121
const { connectAll, getServer } = require('./mcpClient');
2222

2323
const SYSTEM_BASE = [
@@ -549,7 +549,12 @@ async function setupMcp(ctx, wsFolders, dbg) {
549549
const route = built.routes.get(t.name);
550550
return classifyMcpTool(t.name, cfg.toolPolicy, route && route.annotations).approve === 'allow';
551551
}).length;
552-
const summary = handles.map((h) => h.name + ' (' + h.tools.length + ')').join(', ');
552+
// Per-server counts come from what was actually EXPOSED (built.routes), not the raw tools/list
553+
// length — a server capped at MAX_TOOLS_PER_SERVER or listing junk exposes fewer than it
554+
// advertised, and showing the raw number would contradict the allowed/total denominator below
555+
// (PR #31 review). A server that exposed nothing still shows "(0)": honest, and a useful signal.
556+
const perServer = toolCountsByServer(built.routes);
557+
const summary = handles.map((h) => h.name + ' (' + (perServer.get(h.name) || 0) + ')').join(', ');
553558
dbg('mcp.ready', { servers: handles.map((h) => h.name), tools: built.tools.length, allowed });
554559
ctx.post({ type: 'agentTool', icon: 'plug', text: '🔌 mcp · ' + summary + ' · ' + allowed + '/' + built.tools.length + ' allow-listed' });
555560
return built;

‎extensions/levelcode-ai/mcpConfig.js‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,26 @@ function buildAgentTools(servers, opts) {
334334
return { tools, routes, problems: assigned.problems };
335335
}
336336

337+
/**
338+
* Per-server counts of the tools ACTUALLY exposed, taken from the routes buildAgentTools emitted — i.e.
339+
* AFTER the per-server cap and junk-skipping. The startup chip uses this rather than the raw tools/list
340+
* length, so its per-server numbers reflect what the agent can really call and SUM to the run's real
341+
* total: a server that lists 100 tools but is capped to 64 must read `(64)`, not `(100)`, or the chip
342+
* contradicts its own allowed/total denominator (PR #31 review).
343+
*
344+
* @param {Map<string,{server:string}>} routes the routes map from buildAgentTools
345+
* @returns {Map<string, number>} server name → exposed tool count
346+
*/
347+
function toolCountsByServer(routes) {
348+
const counts = new Map();
349+
if (!routes || typeof routes.values !== 'function') { return counts; }
350+
for (const r of routes.values()) {
351+
if (!r || typeof r.server !== 'string') { continue; }
352+
counts.set(r.server, (counts.get(r.server) || 0) + 1);
353+
}
354+
return counts;
355+
}
356+
337357
// ---- 3. approval policy ----------------------------------------------------------------------
338358

339359
/**
@@ -391,6 +411,6 @@ function explainMcpRefusal(name, verdict) {
391411

392412
module.exports = {
393413
loadServerConfig, userScopedSetting, namespaceToolName, assignToolNames, buildAgentTools,
394-
classifyMcpTool, explainMcpRefusal,
414+
toolCountsByServer, classifyMcpTool, explainMcpRefusal,
395415
BUILTIN_TOOL_NAMES, MAX_TOOL_NAME, MAX_TOOL_DESC, MAX_SERVERS, MAX_TOOLS_PER_SERVER, WORKSPACE_CONFIG_PATH
396416
};

‎extensions/levelcode-ai/test/mcpConfig.test.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,33 @@ test('BUILD: the FALLBACK description is capped too — a giant tool name cannot
404404
assert.ok(b.tools[0].name.length <= M.MAX_TOOL_NAME, 'and the name still stays legal');
405405
});
406406

407+
test('BUILD: per-server exposed counts come from routes and SUM to the real total (chip honesty)', () => {
408+
// PR #31 review: the startup chip must not show a raw tools/list length that a cap or junk-skip made
409+
// untrue — e.g. "a (100)" next to a "/64 allow-listed" denominator. toolCountsByServer derives the
410+
// per-server numbers from what was actually exposed, so they reflect reality and sum to the total.
411+
const many = [];
412+
for (let i = 0; i < M.MAX_TOOLS_PER_SERVER + 5; i++) { many.push({ name: 'a' + i }); }
413+
const b = M.buildAgentTools([
414+
{ name: 'a', tools: many }, // over the cap
415+
{ name: 'b', tools: [{ name: 'one' }, null, { name: '' }, { name: 'two' }] } // 2 real + junk
416+
]);
417+
const counts = M.toolCountsByServer(b.routes);
418+
assert.strictEqual(counts.get('a'), M.MAX_TOOLS_PER_SERVER, 'a is capped, not its raw ' + many.length);
419+
assert.strictEqual(counts.get('b'), 2, 'b exposes only its 2 valid tools');
420+
421+
// The load-bearing invariant: the per-server counts sum to built.tools.length — the chip denominator.
422+
let sum = 0; for (const v of counts.values()) { sum += v; }
423+
assert.strictEqual(sum, b.tools.length, 'per-server counts must sum to the real total');
424+
});
425+
426+
test('BUILD: toolCountsByServer tolerates junk input without throwing', () => {
427+
assert.strictEqual(M.toolCountsByServer(null).size, 0);
428+
assert.strictEqual(M.toolCountsByServer(undefined).size, 0);
429+
assert.strictEqual(M.toolCountsByServer(new Map()).size, 0);
430+
// A routes-shaped map with a malformed entry is skipped, not counted.
431+
assert.strictEqual(M.toolCountsByServer(new Map([['x', { server: 's' }], ['y', null], ['z', {}]])).get('s'), 1);
432+
});
433+
407434
test('BUILD: annotations ride along so the policy can tighten on them', () => {
408435
const b = M.buildAgentTools([SRV('s', [{ name: 'rm', annotations: { destructiveHint: true } }])]);
409436
const route = b.routes.get('s__rm');

0 commit comments

Comments
 (0)