mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-25 13:10:13 +00:00
feat(mcp-tools): sort tool catalog alphabetically for a stable surface
MCP clients render tools in the order the server lists them, and the SDK lists them in registration order. Tools are registered across three thematic modules (sessions/search/admin), so the raw catalog interleaves get_* / list_* / query_* / search_* and looks random to the user. Add sortToolCatalog() at the end of registerTools(): it re-keys the SDK registry alphabetically once, after all modules have registered. A plain name sort groups the catalog by verb prefix, and any tool added to any module in the future is sorted in automatically. Sorting only reorders the keys; handler lookup is by name, so behaviour is unchanged. Add a backend-free unit test locking in the order for both roles, the prefix grouping, and that GA-only tools stay hidden from tenant users. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,57 @@
|
||||
/**
|
||||
* Unit tests for the tool catalog ordering.
|
||||
*
|
||||
* MCP clients render tools in the exact order the server lists them, and the SDK
|
||||
* lists them in registration order. registerTools() sorts the catalog after all
|
||||
* modules have registered so the surface is stable and grouped by verb prefix
|
||||
* (get_* / list_* / query_* / search_*). These tests lock that in — they need no
|
||||
* backend token (registerTools only wires handlers, it never calls the API).
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
|
||||
import { registerTools } from '../tools.js';
|
||||
|
||||
function registeredToolNames(ga: boolean): string[] {
|
||||
const server = new McpServer({ name: 'test', version: '0.0.0' });
|
||||
// knowledgeBase / eventTypeIndex are optional — pass undefined so this stays a
|
||||
// pure unit test with no search-provider or backend dependency.
|
||||
registerTools(server, undefined, undefined, ga);
|
||||
const internal = server as unknown as { _registeredTools: Record<string, unknown> };
|
||||
return Object.keys(internal._registeredTools);
|
||||
}
|
||||
|
||||
describe('tool catalog ordering', () => {
|
||||
it('lists tools in alphabetical order for a Global Admin', () => {
|
||||
const names = registeredToolNames(true);
|
||||
expect(names.length).toBeGreaterThan(0);
|
||||
expect(names).toEqual([...names].sort());
|
||||
});
|
||||
|
||||
it('lists tools in alphabetical order for a tenant user', () => {
|
||||
const names = registeredToolNames(false);
|
||||
expect(names.length).toBeGreaterThan(0);
|
||||
expect(names).toEqual([...names].sort());
|
||||
});
|
||||
|
||||
it('groups tools by verb prefix as a side effect of the sort', () => {
|
||||
const names = registeredToolNames(true);
|
||||
// Once sorted, every tool sharing a prefix is contiguous: the index of the
|
||||
// last get_* is below the index of the first list_*, etc.
|
||||
const lastIndexOfPrefix = (p: string) =>
|
||||
names.reduce((acc, n, i) => (n.startsWith(p) ? i : acc), -1);
|
||||
const firstIndexOfPrefix = (p: string) => names.findIndex((n) => n.startsWith(p));
|
||||
|
||||
expect(lastIndexOfPrefix('get_')).toBeLessThan(firstIndexOfPrefix('list_'));
|
||||
expect(lastIndexOfPrefix('list_')).toBeLessThan(firstIndexOfPrefix('query_'));
|
||||
expect(lastIndexOfPrefix('query_')).toBeLessThan(firstIndexOfPrefix('search_'));
|
||||
});
|
||||
|
||||
it('hides Global-Admin-only tools from a tenant user', () => {
|
||||
const gaNames = registeredToolNames(true);
|
||||
const tenantNames = registeredToolNames(false);
|
||||
expect(tenantNames.length).toBeLessThan(gaNames.length);
|
||||
// list_tenants is GA-only — it must not leak into the tenant catalog.
|
||||
expect(gaNames).toContain('list_tenants');
|
||||
expect(tenantNames).not.toContain('list_tenants');
|
||||
});
|
||||
});
|
||||
@@ -19,4 +19,34 @@ export function registerTools(
|
||||
registerSessionTools(server, ga);
|
||||
registerSearchTools(server, knowledgeBase, eventTypeIndex, ga);
|
||||
registerAdminTools(server, ga);
|
||||
sortToolCatalog(server);
|
||||
}
|
||||
|
||||
/**
|
||||
* MCP clients render the tool catalog in the exact order the server lists it,
|
||||
* and the SDK lists tools in registration order (it iterates `Object.entries`
|
||||
* over its internal registry). Because our tools are registered across three
|
||||
* thematic modules, that raw order is effectively random to the user — `get_*`,
|
||||
* `search_*` and `query_*` end up interleaved.
|
||||
*
|
||||
* Re-key the internal registry alphabetically once, after all modules have
|
||||
* registered. A plain name sort groups the catalog by verb prefix
|
||||
* (`get_*` → `list_*` → `query_*` → `search_*`), and any tool added to any
|
||||
* module in the future is sorted in automatically — no per-module ordering to
|
||||
* keep in sync. Sorting only reorders the keys; handler lookup is still by name,
|
||||
* so behaviour is unchanged.
|
||||
*
|
||||
* Note: `_registeredTools` is an SDK-internal field with no public reorder API.
|
||||
* Touching it is contained to this one helper and is stable across SDK versions
|
||||
* (it is the core tool registry, a plain insertion-ordered object).
|
||||
*/
|
||||
function sortToolCatalog(server: McpServer): void {
|
||||
const internal = server as unknown as { _registeredTools: Record<string, unknown> };
|
||||
const registry = internal._registeredTools;
|
||||
if (!registry) return;
|
||||
const sorted: Record<string, unknown> = {};
|
||||
for (const name of Object.keys(registry).sort()) {
|
||||
sorted[name] = registry[name];
|
||||
}
|
||||
internal._registeredTools = sorted;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user