From 7af6fd03064385d1b5a299c4468769c0d2b5b659 Mon Sep 17 00:00:00 2001
From: Gergo Magyar <gergomagyar@icloud.com>
Date: Sun, 3 May 2026 08:48:49 +0100
Subject: [PATCH] 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.
---
 gitnexus/src/mcp/tools.ts         |  2 ++
 gitnexus/test/unit/server.test.ts | 22 +++++++++++++++-------
 gitnexus/test/unit/tools.test.ts  |  5 ++++-
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts
index fd139a936..a85298c04 100644
--- a/gitnexus/src/mcp/tools.ts
+++ b/gitnexus/src/mcp/tools.ts
@@ -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',
diff --git a/gitnexus/test/unit/server.test.ts b/gitnexus/test/unit/server.test.ts
index 01fc44d9f..e762c58ee 100644
--- a/gitnexus/test/unit/server.test.ts
+++ b/gitnexus/test/unit/server.test.ts
@@ -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();
     }
   });
 });
diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts
index e3a646c77..a9ce5cf25 100644
--- a/gitnexus/test/unit/tools.test.ts
+++ b/gitnexus/test/unit/tools.test.ts
@@ -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));
     }
   });