fix(mcplocal): make the paginated-result contract usable by any MCP client
Some checks failed
CI/CD / lint (pull_request) Successful in 1m13s
CI/CD / test (pull_request) Successful in 1m25s
CI/CD / typecheck (pull_request) Successful in 3m2s
CI/CD / smoke (pull_request) Failing after 2m5s
CI/CD / build (pull_request) Successful in 5m1s
CI/CD / publish (pull_request) Has been skipped
Some checks failed
CI/CD / lint (pull_request) Successful in 1m13s
CI/CD / test (pull_request) Successful in 1m25s
CI/CD / typecheck (pull_request) Successful in 3m2s
CI/CD / smoke (pull_request) Failing after 2m5s
CI/CD / build (pull_request) Successful in 5m1s
CI/CD / publish (pull_request) Has been skipped
UniFi tools were unusable from non-Claude agents. A tool result over 2000 chars is replaced with a table of contents and re-read by calling the tool again with _resultId/_section, but those params were never declared on the tool's inputSchema — and unifi-network and my-grafana ship `additionalProperties: false`, so for a client that validates arguments the drill-down call was illegal and the data unreachable. The stub's own wording made it worse: "Use section parameter" names a param that does not exist; sending it fell through to the upstream, re-paginated, and minted a fresh _resultId. An unbounded loop. UniFi took the blame because it is the one server whose results always trip the threshold: get_devices was 11,718 chars, get_clients 136,696 across 19 pages with a 92-char page-1. Grafana and Gitea return compact results. - content-pipeline gains onToolsList, declaring _resultId/_section on every tool it can paginate (gate tools excluded — they are intercepted before the pipeline runs). additionalProperties stays false: a property listed in `properties` is already legal under it, so declaring is enough and the upstream keeps its typo protection. - createDefaultPlugin wired only the gate's onToolsList, so the pipeline's would have been dropped on the floor. Both now chain, gate first. - _resultId without _section re-shows the table of contents instead of forwarding an unknown argument to a strict upstream. - The stub names the tool, the live _resultId and a real section id. - Nested MCP envelopes are collapsed. A server fronting another MCP server returns the inner result wrapped in its own content/structuredContent pair, so the payload arrives two or three times over. A layer is peeled only when it carries nothing the inner value lacks, which keeps it lossless. Live against the sre project: get_devices 11,718 -> 5,210 chars and no longer paginates at all; get_clients 136,696 -> 62,358 across 8 pages instead of 19. - deploy/mcplocal.service shipped MCPLOCAL_MCPD_URL=http://10.0.0.194:3100, which is dead — every working machine had a hand-written drop-in. Points at the k8s ingress now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNXFvxanvM6uiFcb4Mp3xU
This commit is contained in:
109
src/mcplocal/tests/smoke/tool-drilldown.test.ts
Normal file
109
src/mcplocal/tests/smoke/tool-drilldown.test.ts
Normal file
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* Smoke tests: tool drill-down contract.
|
||||
*
|
||||
* A large tool result is replaced with a table of contents and re-read by
|
||||
* calling the same tool with _resultId + _section. Those two parameters must
|
||||
* appear in the tool's advertised inputSchema, because upstreams such as
|
||||
* unifi-network and my-grafana declare `additionalProperties: false` — a
|
||||
* client validating against the schema cannot send an undeclared parameter,
|
||||
* which used to make every paginated UniFi result unreadable.
|
||||
*
|
||||
* Requires: mcplocal running (localhost:3200) with a project whose servers are
|
||||
* reachable. Set SMOKE_PROJECT to target a specific project.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { SmokeMcpSession, isMcplocalRunning } from './mcp-client.js';
|
||||
|
||||
const PROJECT_NAME = process.env['SMOKE_PROJECT'] ?? 'smoke-data';
|
||||
/** HTTP-mode mcplocal authenticates every request with an McpToken. */
|
||||
const TOKEN = process.env['SMOKE_MCPTOKEN'];
|
||||
|
||||
interface Tool {
|
||||
name: string;
|
||||
inputSchema?: { type?: string; properties?: Record<string, unknown>; additionalProperties?: unknown };
|
||||
}
|
||||
|
||||
/** Tools served by the gate plugin — intercepted before the pipeline, so exempt. */
|
||||
const GATE_TOOLS = new Set(['begin_session', 'read_prompts', 'propose_prompt', 'propose_skill']);
|
||||
|
||||
describe('Smoke: tool drill-down contract', () => {
|
||||
let available = false;
|
||||
let session: SmokeMcpSession;
|
||||
let tools: Tool[] = [];
|
||||
|
||||
beforeAll(async () => {
|
||||
available = await isMcplocalRunning();
|
||||
if (!available) return;
|
||||
|
||||
session = new SmokeMcpSession(PROJECT_NAME, TOKEN);
|
||||
await session.initialize();
|
||||
await session.sendNotification('notifications/initialized');
|
||||
|
||||
// Open the gate if the project is gated, so the real catalog is visible.
|
||||
const gated = await session.send('tools/list') as { tools: Tool[] };
|
||||
if (gated.tools.some((t) => t.name === 'begin_session')) {
|
||||
await session.send('tools/call', {
|
||||
name: 'begin_session',
|
||||
arguments: { description: 'Verify the paginated tool-result drill-down contract' },
|
||||
}, 180_000);
|
||||
}
|
||||
|
||||
tools = ((await session.send('tools/list')) as { tools: Tool[] }).tools;
|
||||
}, 240_000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (session) await session.close();
|
||||
});
|
||||
|
||||
it('every upstream tool advertises _resultId and _section', async () => {
|
||||
if (!available) return;
|
||||
|
||||
const upstreamTools = tools.filter((t) => !GATE_TOOLS.has(t.name));
|
||||
if (upstreamTools.length === 0) {
|
||||
console.log(` No upstream tools in project "${PROJECT_NAME}" — skipping`);
|
||||
return;
|
||||
}
|
||||
|
||||
const missing = upstreamTools.filter((t) => {
|
||||
const props = t.inputSchema?.properties;
|
||||
return props === undefined || props['_resultId'] === undefined || props['_section'] === undefined;
|
||||
});
|
||||
|
||||
if (missing.length > 0) {
|
||||
console.log(` Missing drill-down params: ${missing.map((t) => t.name).join(', ')}`);
|
||||
}
|
||||
expect(missing).toEqual([]);
|
||||
console.log(` ${upstreamTools.length} tools carry the drill-down contract`);
|
||||
});
|
||||
|
||||
it('strict upstream schemas stay strict (params are declared, not permitted)', async () => {
|
||||
if (!available) return;
|
||||
|
||||
// Declaring the params in `properties` is what makes them legal under
|
||||
// `additionalProperties: false`; loosening it instead would drop the
|
||||
// upstream's own typo protection.
|
||||
const strict = tools.filter(
|
||||
(t) => !GATE_TOOLS.has(t.name) && t.inputSchema?.additionalProperties === false,
|
||||
);
|
||||
if (strict.length === 0) {
|
||||
console.log(' No strict-schema tools in this project — skipping');
|
||||
return;
|
||||
}
|
||||
|
||||
for (const t of strict) {
|
||||
expect(t.inputSchema!.properties!['_resultId']).toBeDefined();
|
||||
expect(t.inputSchema!.properties!['_section']).toBeDefined();
|
||||
}
|
||||
console.log(` ${strict.length} strict-schema tools keep additionalProperties: false`);
|
||||
});
|
||||
|
||||
it('gate tools do not advertise drill-down params', async () => {
|
||||
if (!available) return;
|
||||
|
||||
for (const t of tools.filter((x) => GATE_TOOLS.has(x.name))) {
|
||||
const props = t.inputSchema?.properties ?? {};
|
||||
expect(props['_resultId']).toBeUndefined();
|
||||
expect(props['_section']).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user