Merge pull request 'fix(mcplocal): serve OpenAI-safe tool names on the wire' (#124) from fix/wire-safe-tool-names into main

src/mcplocal/tests/mcp-endpoint-wire-names.test.ts

@@ -0,0 +1,100 @@
+import { describe, it, expect, vi } from 'vitest';
+import Fastify from 'fastify';
+import { registerMcpEndpoint } from '../src/http/mcp-endpoint.js';
+import type { McpRouter } from '../src/router.js';
+import type { JsonRpcRequest, JsonRpcResponse } from '../src/types.js';
+
+/**
+ * End-to-end over the Streamable HTTP transport: the /mcp endpoint must serve
+ * OpenAI-safe tool names on tools/list and map them back to the router's
+ * canonical `server/tool` names on tools/call (the librechat fetch_content
+ * incident, 2026-08-25).
+ */
+
+function parseSse(body: string): JsonRpcResponse {
+  const dataLine = body.split('\n').find((l) => l.startsWith('data: '));
+  if (!dataLine) throw new Error(`no SSE data line in: ${body}`);
+  return JSON.parse(dataLine.slice('data: '.length)) as JsonRpcResponse;
+}
+
+describe('registerMcpEndpoint wire names', () => {
+  it('lists wire-safe names and routes calls back to canonical names', async () => {
+    const routed: JsonRpcRequest[] = [];
+    const fakeRouter = {
+      route: vi.fn(async (req: JsonRpcRequest): Promise<JsonRpcResponse> => {
+        routed.push(req);
+        switch (req.method) {
+          case 'initialize':
+            return {
+              jsonrpc: '2.0',
+              id: req.id,
+              result: {
+                protocolVersion: '2024-11-05',
+                serverInfo: { name: 'test', version: '0' },
+                capabilities: { tools: {} },
+              },
+            };
+          case 'tools/list':
+            return {
+              jsonrpc: '2.0',
+              id: req.id,
+              result: { tools: [{ name: 'websearch/fetch_content', inputSchema: { type: 'object' } }] },
+            };
+          case 'tools/call':
+            return {
+              jsonrpc: '2.0',
+              id: req.id,
+              result: { content: [{ type: 'text', text: 'ok' }] },
+            };
+          default:
+            return { jsonrpc: '2.0', id: req.id, result: {} };
+        }
+      }),
+    } as unknown as McpRouter;
+
+    const app = Fastify();
+    registerMcpEndpoint(app, fakeRouter);
+    await app.ready();
+    try {
+      const headers = {
+        'content-type': 'application/json',
+        accept: 'application/json, text/event-stream',
+      };
+
+      const init = await app.inject({
+        method: 'POST',
+        url: '/mcp',
+        headers,
+        payload: { jsonrpc: '2.0', id: 1, method: 'initialize', params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 't', version: '0' } } },
+      });
+      expect(init.statusCode).toBe(200);
+      const sessionId = init.headers['mcp-session-id'] as string;
+      expect(sessionId).toBeTruthy();
+      const sessionHeaders = { ...headers, 'mcp-session-id': sessionId };
+
+      const list = await app.inject({
+        method: 'POST',
+        url: '/mcp',
+        headers: sessionHeaders,
+        payload: { jsonrpc: '2.0', id: 2, method: 'tools/list' },
+      });
+      const listResponse = parseSse(list.body);
+      const tools = (listResponse.result as { tools: Array<{ name: string }> }).tools;
+      expect(tools.map((t) => t.name)).toEqual(['websearch_fetch_content']);
+
+      const call = await app.inject({
+        method: 'POST',
+        url: '/mcp',
+        headers: sessionHeaders,
+        payload: { jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'websearch_fetch_content', arguments: { url: 'https://x' } } },
+      });
+      const callResponse = parseSse(call.body);
+      expect(callResponse.error).toBeUndefined();
+
+      const routedCall = routed.find((r) => r.method === 'tools/call');
+      expect(routedCall?.params?.['name']).toBe('websearch/fetch_content');
+    } finally {
+      await app.close();
+    }
+  });
+});

src/mcplocal/tests/project-mcp-endpoint-wire-names.test.ts

@@ -0,0 +1,145 @@
+import { describe, it, expect, vi } from 'vitest';
+import Fastify from 'fastify';
+import { registerProjectMcpEndpoint } from '../src/http/project-mcp-endpoint.js';
+import type { McpRouter } from '../src/router.js';
+import type { JsonRpcRequest, JsonRpcResponse } from '../src/types.js';
+
+/**
+ * End-to-end wire-name test on the endpoint the librechat incident actually
+ * hit: /projects/:name/mcp. A fake `websearch` upstream serves a tool named
+ * `fetch_content`; the client must see `websearch_fetch_content` in
+ * tools/list, and calling that wire name must reach the upstream as plain
+ * `fetch_content` (router canonical `websearch/fetch_content`, prefix
+ * stripped on dispatch).
+ */
+
+const upstreamRequests: JsonRpcRequest[] = [];
+
+vi.mock('../src/discovery.js', () => ({
+  refreshProjectUpstreams: vi.fn(async (router: McpRouter) => {
+    router.addUpstream({
+      name: 'websearch',
+      send: async (req: JsonRpcRequest): Promise<JsonRpcResponse> => {
+        upstreamRequests.push(req);
+        if (req.method === 'tools/list') {
+          return {
+            jsonrpc: '2.0',
+            id: req.id,
+            result: { tools: [{ name: 'fetch_content', description: 'fetch a page', inputSchema: { type: 'object' } }] },
+          };
+        }
+        if (req.method === 'tools/call') {
+          return {
+            jsonrpc: '2.0',
+            id: req.id,
+            result: { content: [{ type: 'text', text: 'page body' }] },
+          };
+        }
+        return { jsonrpc: '2.0', id: req.id, result: {} };
+      },
+      close: async () => {},
+      isAlive: () => true,
+    });
+    return ['websearch'];
+  }),
+  // gated: false → no gate plugin, the full catalog is served at initialize
+  // (the chat-web configuration).
+  fetchProjectLlmConfig: vi.fn(async () => ({ gated: false, llmProvider: 'none' })),
+}));
+
+vi.mock('../src/http/config.js', async () => {
+  const actual = await vi.importActual<typeof import('../src/http/config.js')>('../src/http/config.js');
+  return { ...actual, loadProjectLlmOverride: vi.fn(() => undefined) };
+});
+
+function mockMcpdClient() {
+  const client: Record<string, unknown> = {
+    baseUrl: 'http://test:3100',
+    token: 'test-token',
+    get: vi.fn(async () => []),
+    post: vi.fn(async () => ({})),
+    put: vi.fn(),
+    delete: vi.fn(),
+    forward: vi.fn(async () => ({ status: 200, body: [] })),
+    withHeaders: vi.fn(),
+    withToken: vi.fn(),
+    withTimeout: vi.fn(),
+  };
+  (client.withHeaders as ReturnType<typeof vi.fn>).mockReturnValue(client);
+  (client.withToken as ReturnType<typeof vi.fn>).mockReturnValue(client);
+  (client.withTimeout as ReturnType<typeof vi.fn>).mockReturnValue(client);
+  return client;
+}
+
+function parseSse(body: string): JsonRpcResponse {
+  const dataLine = body.split('\n').find((l) => l.startsWith('data: '));
+  if (!dataLine) throw new Error(`no SSE data line in: ${body}`);
+  return JSON.parse(dataLine.slice('data: '.length)) as JsonRpcResponse;
+}
+
+describe('registerProjectMcpEndpoint wire names', () => {
+  it('serves wire-safe names on the project endpoint and dispatches calls upstream', async () => {
+    upstreamRequests.length = 0;
+    const app = Fastify();
+    registerProjectMcpEndpoint(app, mockMcpdClient() as never);
+    await app.ready();
+    try {
+      const headers = {
+        'content-type': 'application/json',
+        accept: 'application/json, text/event-stream',
+      };
+
+      const init = await app.inject({
+        method: 'POST',
+        url: '/projects/chat-web/mcp',
+        headers,
+        payload: { jsonrpc: '2.0', id: 1, method: 'initialize', params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 't', version: '0' } } },
+      });
+      expect(init.statusCode).toBe(200);
+      const sessionId = init.headers['mcp-session-id'] as string;
+      expect(sessionId).toBeTruthy();
+      const sessionHeaders = { ...headers, 'mcp-session-id': sessionId };
+
+      const list = await app.inject({
+        method: 'POST',
+        url: '/projects/chat-web/mcp',
+        headers: sessionHeaders,
+        payload: { jsonrpc: '2.0', id: 2, method: 'tools/list' },
+      });
+      const listResponse = parseSse(list.body);
+      const tools = (listResponse.result as { tools: Array<{ name: string }> }).tools;
+      const names = tools.map((t) => t.name);
+      expect(names).toContain('websearch_fetch_content');
+      // Every served name must be a valid OpenAI-style function name — the
+      // invariant the librechat incident violated.
+      for (const name of names) {
+        expect(name).toMatch(/^[A-Za-z0-9_.-]+$/);
+      }
+
+      const call = await app.inject({
+        method: 'POST',
+        url: '/projects/chat-web/mcp',
+        headers: sessionHeaders,
+        payload: { jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'websearch_fetch_content', arguments: { url: 'https://x' } } },
+      });
+      const callResponse = parseSse(call.body);
+      expect(callResponse.error).toBeUndefined();
+      expect((callResponse.result as { content: Array<{ text: string }> }).content[0]?.text).toBe('page body');
+
+      // The upstream saw the bare tool name — namespace stripped, not mangled.
+      const upstreamCall = upstreamRequests.find((r) => r.method === 'tools/call');
+      expect(upstreamCall?.params?.['name']).toBe('fetch_content');
+
+      // Legacy clients echoing the canonical slash name keep working.
+      const legacy = await app.inject({
+        method: 'POST',
+        url: '/projects/chat-web/mcp',
+        headers: sessionHeaders,
+        payload: { jsonrpc: '2.0', id: 4, method: 'tools/call', params: { name: 'websearch/fetch_content', arguments: { url: 'https://x' } } },
+      });
+      expect(parseSse(legacy.body).error).toBeUndefined();
+    } finally {
+      await app.close();
+    }
+  });
+});

src/mcplocal/tests/wire-names.test.ts

@@ -0,0 +1,168 @@
+import { describe, it, expect, vi } from 'vitest';
+import { sanitizeWireName, WireNameCodec, routeWithWireNames } from '../src/util/wire-names.js';
+import type { JsonRpcRequest, JsonRpcResponse } from '../src/types.js';
+
+describe('sanitizeWireName', () => {
+  it('replaces slashes with underscores', () => {
+    expect(sanitizeWireName('websearch/fetch_content')).toBe('websearch_fetch_content');
+    expect(sanitizeWireName('all/websearch/fetch_content')).toBe('all_websearch_fetch_content');
+  });
+
+  it('keeps names that are already OpenAI-safe', () => {
+    expect(sanitizeWireName('begin_session')).toBe('begin_session');
+    expect(sanitizeWireName('my-grafana.tool')).toBe('my-grafana.tool');
+  });
+
+  it('replaces every character outside [A-Za-z0-9_.-]', () => {
+    expect(sanitizeWireName('a b:c/d')).toBe('a_b_c_d');
+  });
+});
+
+describe('WireNameCodec', () => {
+  it('round-trips a namespaced tool name', () => {
+    const codec = new WireNameCodec();
+    const wire = codec.encodeName('websearch/fetch_content');
+    expect(wire).toBe('websearch_fetch_content');
+    expect(codec.decodeName(wire)).toBe('websearch/fetch_content');
+  });
+
+  it('is stable across repeated encodes', () => {
+    const codec = new WireNameCodec();
+    expect(codec.encodeName('searxng/web_url_read')).toBe('searxng_web_url_read');
+    expect(codec.encodeName('searxng/web_url_read')).toBe('searxng_web_url_read');
+  });
+
+  it('passes unknown inbound names through unchanged', () => {
+    const codec = new WireNameCodec();
+    // Legacy client echoing the slash form, or a virtual tool never listed.
+    expect(codec.decodeName('websearch/fetch_content')).toBe('websearch/fetch_content');
+    expect(codec.decodeName('begin_session')).toBe('begin_session');
+  });
+
+  it('suffixes on collision, first registration wins the plain name', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    try {
+      const codec = new WireNameCodec();
+      expect(codec.encodeName('foo_bar/baz')).toBe('foo_bar_baz');
+      expect(codec.encodeName('foo/bar_baz')).toBe('foo_bar_baz_2');
+      // Both decode back to their own presented names.
+      expect(codec.decodeName('foo_bar_baz')).toBe('foo_bar/baz');
+      expect(codec.decodeName('foo_bar_baz_2')).toBe('foo/bar_baz');
+      // And stay stable.
+      expect(codec.encodeName('foo/bar_baz')).toBe('foo_bar_baz_2');
+      expect(warn).toHaveBeenCalledOnce();
+    } finally {
+      warn.mockRestore();
+    }
+  });
+
+  it('encodes tools/list responses and leaves other fields intact', () => {
+    const codec = new WireNameCodec();
+    const response: JsonRpcResponse = {
+      jsonrpc: '2.0',
+      id: 1,
+      result: {
+        tools: [
+          { name: 'websearch/fetch_content', description: 'fetch', inputSchema: { type: 'object' } },
+          { name: 'begin_session', description: 'gate' },
+        ],
+      },
+    };
+    const encoded = codec.encodeToolsList(response);
+    const tools = (encoded.result as { tools: Array<{ name: string; description?: string }> }).tools;
+    expect(tools.map((t) => t.name)).toEqual(['websearch_fetch_content', 'begin_session']);
+    expect(tools[0]?.description).toBe('fetch');
+    // Original response object is not mutated.
+    const originalTools = (response.result as { tools: Array<{ name: string }> }).tools;
+    expect(originalTools[0]?.name).toBe('websearch/fetch_content');
+  });
+
+  it('returns error and non-list responses unchanged', () => {
+    const codec = new WireNameCodec();
+    const err: JsonRpcResponse = { jsonrpc: '2.0', id: 1, error: { code: -32603, message: 'boom' } };
+    expect(codec.encodeToolsList(err)).toBe(err);
+    const other: JsonRpcResponse = { jsonrpc: '2.0', id: 1, result: { content: [] } };
+    expect(codec.encodeToolsList(other)).toBe(other);
+  });
+
+  it('decodes tools/call requests for known wire names only', () => {
+    const codec = new WireNameCodec();
+    codec.encodeName('websearch/fetch_content');
+
+    const known: JsonRpcRequest = {
+      jsonrpc: '2.0',
+      id: 2,
+      method: 'tools/call',
+      params: { name: 'websearch_fetch_content', arguments: { url: 'https://x' } },
+    };
+    const decoded = codec.decodeToolCall(known);
+    expect(decoded.params?.['name']).toBe('websearch/fetch_content');
+    expect(decoded.params?.['arguments']).toEqual({ url: 'https://x' });
+    // Original request object is not mutated.
+    expect(known.params?.['name']).toBe('websearch_fetch_content');
+
+    const unknown: JsonRpcRequest = {
+      jsonrpc: '2.0',
+      id: 3,
+      method: 'tools/call',
+      params: { name: 'not_listed', arguments: {} },
+    };
+    expect(codec.decodeToolCall(unknown)).toBe(unknown);
+  });
+});
+
+describe('routeWithWireNames', () => {
+  const listResponse: JsonRpcResponse = {
+    jsonrpc: '2.0',
+    id: 1,
+    result: { tools: [{ name: 'websearch/fetch_content' }, { name: 'searxng/web_url_read' }] },
+  };
+
+  it('serves wire-safe names on tools/list and maps tools/call back', async () => {
+    const codec = new WireNameCodec();
+    const seen: JsonRpcRequest[] = [];
+    const route = async (req: JsonRpcRequest): Promise<JsonRpcResponse> => {
+      seen.push(req);
+      if (req.method === 'tools/list') return listResponse;
+      return { jsonrpc: '2.0', id: req.id, result: { content: [{ type: 'text', text: 'ok' }] } };
+    };
+
+    const listed = await routeWithWireNames(codec, route, { jsonrpc: '2.0', id: 1, method: 'tools/list' });
+    const names = (listed.result as { tools: Array<{ name: string }> }).tools.map((t) => t.name);
+    expect(names).toEqual(['websearch_fetch_content', 'searxng_web_url_read']);
+
+    // The exact scenario from the librechat incident: the model echoes the
+    // wire name; the router must receive the canonical name.
+    await routeWithWireNames(codec, route, {
+      jsonrpc: '2.0',
+      id: 2,
+      method: 'tools/call',
+      params: { name: 'websearch_fetch_content', arguments: { url: 'https://x' } },
+    });
+    expect(seen[1]?.params?.['name']).toBe('websearch/fetch_content');
+  });
+
+  it('keeps legacy slash-name calls working', async () => {
+    const codec = new WireNameCodec();
+    const seen: JsonRpcRequest[] = [];
+    const route = async (req: JsonRpcRequest): Promise<JsonRpcResponse> => {
+      seen.push(req);
+      return { jsonrpc: '2.0', id: req.id, result: {} };
+    };
+    await routeWithWireNames(codec, route, {
+      jsonrpc: '2.0',
+      id: 1,
+      method: 'tools/call',
+      params: { name: 'websearch/fetch_content', arguments: {} },
+    });
+    expect(seen[0]?.params?.['name']).toBe('websearch/fetch_content');
+  });
+
+  it('does not touch other methods', async () => {
+    const codec = new WireNameCodec();
+    const init: JsonRpcRequest = { jsonrpc: '2.0', id: 1, method: 'initialize', params: {} };
+    const response: JsonRpcResponse = { jsonrpc: '2.0', id: 1, result: { protocolVersion: '2024-11-05' } };
+    const out = await routeWithWireNames(codec, async () => response, init);
+    expect(out).toBe(response);
+  });
+});