test(mcp): address PR #1127 review follow-ups

- Replace private `_requestHandlers` SDK access in server.test.ts with
  `Client` + `InMemoryTransport.createLinkedPair()` for the tools/list
  annotation propagation test. The new path uses supported public APIs
  and surfaces SDK changes loudly instead of silently degrading.
- Extract `OPEN_WORLD_READ_ONLY_TOOLS` set in tools.test.ts so future
  read-only open-world tools can be added without rewriting the
  invariant; preserves the current "only `query` is open-world" guard.
- Add inline rationale on `group_sync` annotations explaining the
  conservative `idempotentHint: false` (writes contracts.json on every
  call even when output is deterministic).

No runtime behavior change. Annotations themselves and tools/list shape
are unchanged.
This commit is contained in:
Gergo Magyar 2026-05-03 08:48:49 +01:00
parent fe6ff3abad
commit 7af6fd0306
3 changed files with 21 additions and 8 deletions

View file

@ -531,6 +531,8 @@ WHEN TO USE: Discover groups before group_sync. Optional "name" returns a single
description: `Rebuild the Contract Registry (contracts.json) for a group: extract HTTP contracts, apply manifest links, exact-match cross-links.
WHEN TO USE: After changing group.yaml or re-indexing member repos.`,
// Writes contracts.json on every call; conservatively non-idempotent
// even though output is deterministic for identical input.
annotations: DESTRUCTIVE_TOOL_ANNOTATIONS,
inputSchema: {
type: 'object',

View file

@ -13,6 +13,8 @@
* directly through the MCP Server's handler dispatch.
*/
import { describe, it, expect, vi } from 'vitest';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
import { createMCPServer } from '../../src/mcp/server.js';
import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js';
@ -57,16 +59,22 @@ describe('createMCPServer', () => {
it('tools/list response includes tool annotations', async () => {
const backend = createMockBackend();
const server = createMCPServer(backend);
const handler = (server as any)._requestHandlers.get('tools/list');
const client = new Client({ name: 'test-client', version: '0.0.0' });
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
expect(typeof handler).toBe('function');
try {
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
const response = await handler({ method: 'tools/list', params: {} }, {});
expect(response.tools).toHaveLength(GITNEXUS_TOOLS.length);
const response = await client.listTools();
expect(response.tools).toHaveLength(GITNEXUS_TOOLS.length);
for (const tool of response.tools) {
const definition = GITNEXUS_TOOLS.find((t) => t.name === tool.name)!;
expect(tool.annotations).toEqual(definition.annotations);
for (const tool of response.tools) {
const definition = GITNEXUS_TOOLS.find((t) => t.name === tool.name)!;
expect(tool.annotations).toEqual(definition.annotations);
}
} finally {
await client.close();
await server.close();
}
});
});

View file

@ -12,6 +12,9 @@ import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js';
const GROUP_TOOLS = new Set(['group_list', 'group_sync']);
const MUTATING_TOOLS = new Set(['rename', 'group_sync']);
// Read-only tools that legitimately reach external systems. Add a tool name
// here when introducing a read-only tool that needs openWorldHint: true.
const OPEN_WORLD_READ_ONLY_TOOLS = new Set(['query']);
describe('GITNEXUS_TOOLS', () => {
it('exports all tools (7 base + 3 route/tool/shape + 1 api_impact + 2 group)', () => {
@ -64,7 +67,7 @@ describe('GITNEXUS_TOOLS', () => {
expect(tool.annotations.readOnlyHint).toBe(true);
expect(tool.annotations.destructiveHint).toBe(false);
expect(tool.annotations.idempotentHint).toBe(true);
expect(tool.annotations.openWorldHint).toBe(tool.name === 'query');
expect(tool.annotations.openWorldHint).toBe(OPEN_WORLD_READ_ONLY_TOOLS.has(tool.name));
}
});