fix(server): send protocol errors on the wire with a single "MCP error <code>:" prefix (#659)

changelog.d/659.fixed.md

@@ -0,0 +1 @@
+**Protocol errors reach SDK clients with a single `MCP error <code>:` prefix** — every `McpError` thrown from a request handler (argument validation, an unknown tool/prompt/resource, `expectedContent` on an attach session, …) arrived at `@modelcontextprotocol/sdk` clients as `MCP error -32602: MCP error -32602: …`: the SDK copies the thrown error's `.message` — which its own constructor has already prefixed — verbatim onto the wire, and the client-side constructor prefixes it again (the dev proxy had been hiding it with a dedupe). The tools/call, prompt and resource handlers now convert at the JSON-RPC boundary (`WireMcpError`: same class, code and data, bare `.message`), so the wire carries the reason once; in-process throw sites and logs are unchanged (#659)

src/errors/debug-errors.ts

@@ -15,10 +15,11 @@ import { ErrorCode as McpErrorCode } from '@modelcontextprotocol/sdk/types.js';
  * Base for mcp-debugger's typed errors.
  *
  * McpError's constructor bakes `MCP error <code>: ` into `.message`, which is
- * right on the JSON-RPC path (the SDK derives the wire error from it) and wrong
- * inside a tool result envelope, where it reads like a transport failure
- * (issue #647). The SDK keeps no copy of the plain text, so this base records
- * it as `detail`; tool result envelopes and getErrorMessage() report that.
+ * right for logs and in-process callers and wrong inside a tool result
+ * envelope, where it reads like a transport failure (issue #647). The SDK
+ * keeps no copy of the plain text, so this base records it as `detail`; tool
+ * result envelopes and getErrorMessage() report that, and the JSON-RPC
+ * boundary sends it on the wire (toWireError, issue #659).
  */
 export abstract class DebugError extends McpError {
   /** The message without the SDK's `MCP error <code>: ` prefix. */
@@ -168,6 +169,51 @@ export class DebugSessionCreationError extends DebugError {
   }
 }
 
+/**
+ * The plain text of an McpError: a DebugError's recorded `detail`, otherwise
+ * `.message` with the single `MCP error <code>: ` prefix its constructor added.
+ * Only the error's own code is stripped, and only once — a message that
+ * legitimately begins with that text for another reason is left alone.
+ */
+export function mcpErrorDetail(error: McpError): string {
+  if (error instanceof DebugError) {
+    return error.detail;
+  }
+  const prefix = `MCP error ${error.code}: `;
+  return error.message.startsWith(prefix) ? error.message.slice(prefix.length) : error.message;
+}
+
+/**
+ * The McpError shape the JSON-RPC boundary throws (issue #659).
+ *
+ * The SDK's Protocol copies a thrown error's `.code` and `.message` verbatim
+ * into the JSON-RPC error, and the client-side McpError constructor prefixes
+ * `MCP error <code>: ` again — so an ordinary McpError thrown from a request
+ * handler reaches SDK clients as `MCP error -32602: MCP error -32602: …`.
+ * This subclass keeps the class (instanceof, `.code`, `.data`) and resets
+ * `.message` to the bare detail so the wire carries the text once. It is for
+ * the request-handler boundary only; everything upstream keeps throwing
+ * McpError / DebugError, whose prefixed `.message` is right in logs.
+ */
+export class WireMcpError extends McpError {
+  constructor(code: McpErrorCode, detail: string, data?: unknown) {
+    super(code, detail, data);
+    this.message = detail;
+  }
+}
+
+/** Convert an McpError for the JSON-RPC boundary; a WireMcpError passes through. */
+export function toWireError(error: McpError): WireMcpError {
+  if (error instanceof WireMcpError) {
+    return error;
+  }
+  const wire = new WireMcpError(error.code, mcpErrorDetail(error), error.data);
+  if (error.stack) {
+    wire.stack = error.stack;
+  }
+  return wire;
+}
+
 /**
  * Helper to extract a user-facing error message safely. A DebugError reports
  * its plain `detail` rather than the prefixed McpError message (issue #647).

src/server/output-resources.ts

@@ -13,10 +13,10 @@ import {
   ReadResourceRequestSchema,
   SubscribeRequestSchema,
   UnsubscribeRequestSchema,
-  ErrorCode as McpErrorCode,
-  McpError
+  ErrorCode as McpErrorCode
 } from '@modelcontextprotocol/sdk/types.js';
 import { ILogger, type IFileSystem } from '@debugmcp/shared';
+import { WireMcpError } from '../errors/debug-errors.js';
 import type { SessionManager } from '../session/session-manager.js';
 import { proxyLogPathFor } from '../proxy/session-log-layout.js';
 import { readProxyLogTail } from '../session/launch/proxy-failure-diagnostics.js';
@@ -162,11 +162,11 @@ export function registerResourceHandlers(
     const sessionId = outputSessionId ?? proxyLogSessionId;
     const session = sessionId ? sessionManager.getSession(sessionId) : undefined;
     if (!session) {
-      throw new McpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`);
+      throw new WireMcpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`);
     }
     if (proxyLogSessionId) {
       if (!session.logDir) {
-        throw new McpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`);
+        throw new WireMcpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`);
       }
       const text = await readProxyLogTail(
         fileSystem,
@@ -190,7 +190,7 @@ export function registerResourceHandlers(
     const uri = request.params.uri;
     const sessionId = parseOutputResourceUri(uri);
     if (!sessionId || !sessionManager.getSession(sessionId)) {
-      throw new McpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`);
+      throw new WireMcpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`);
     }
     notifier.subscribe(uri);
     return {};

src/server/prompts.ts

@@ -8,10 +8,10 @@ import { Server } from '@modelcontextprotocol/sdk/server/index.js';
 import {
   ListPromptsRequestSchema,
   GetPromptRequestSchema,
-  ErrorCode as McpErrorCode,
-  McpError
+  ErrorCode as McpErrorCode
 } from '@modelcontextprotocol/sdk/types.js';
 import { IEnvironment } from '@debugmcp/shared';
+import { WireMcpError } from '../errors/debug-errors.js';
 import { buildDebuggingWorkflowPrompt } from '../skill-content.js';
 import { getBpAddressingMode } from '../utils/bp-addressing.js';
 
@@ -28,7 +28,7 @@ export function registerPromptHandlers(server: Server, environment: IEnvironment
 
   server.setRequestHandler(GetPromptRequestSchema, async (request) => {
     if (request.params.name !== promptDescriptor.name) {
-      throw new McpError(McpErrorCode.InvalidParams, `Unknown prompt: ${request.params.name}`);
+      throw new WireMcpError(McpErrorCode.InvalidParams, `Unknown prompt: ${request.params.name}`);
     }
     return {
       description: promptDescriptor.description,

src/server/tool-dispatch.ts

@@ -12,6 +12,7 @@ import {
   McpError,
   ServerResult
 } from '@modelcontextprotocol/sdk/types.js';
+import { toWireError } from '../errors/debug-errors.js';
 import { coerceToolArguments, ToolArguments } from './tool-arguments.js';
 import { extractPayloadSuccess, sanitizeRequest } from './tool-result.js';
 import { buildToolDefinitions, isToolName } from './tool-schemas.js';
@@ -85,8 +86,13 @@ export function registerToolHandlers(server: Server, ctx: ToolContext): void {
           timestamp: Date.now()
         });
 
-        if (error instanceof McpError) throw error;
-        throw new McpError(McpErrorCode.InternalError, `Failed to execute tool ${toolName}: ${errorMessage}`);
+        // The JSON-RPC boundary: the SDK copies .message onto the wire and the
+        // client prefixes it again, so send the bare detail (issue #659).
+        throw toWireError(
+          error instanceof McpError
+            ? error
+            : new McpError(McpErrorCode.InternalError, `Failed to execute tool ${toolName}: ${errorMessage}`)
+        );
       }
     }
   );

tests/core/unit/errors/debug-errors.test.ts

@@ -1,7 +1,8 @@
 /**
  * The typed error hierarchy keeps two views of one message (issue #647):
- * `.message` carries the SDK's `MCP error <code>: ` prefix for the JSON-RPC
- * path, `.detail` is the plain text tool result envelopes report.
+ * `.message` carries the SDK's `MCP error <code>: ` prefix for logs and
+ * in-process callers, `.detail` is the plain text tool result envelopes report
+ * and the JSON-RPC boundary sends on the wire (issue #659).
  */
 import { describe, it, expect } from 'vitest';
 import { McpError } from '@modelcontextprotocol/sdk/types.js';
@@ -16,7 +17,10 @@ import {
   SessionNotFoundError,
   SessionTerminatedError,
   UnsupportedFeatureError,
-  UnsupportedLanguageError
+  UnsupportedLanguageError,
+  WireMcpError,
+  mcpErrorDetail,
+  toWireError
 } from '../../../../src/errors/debug-errors.js';
 
 describe('DebugError hierarchy', () => {
@@ -99,3 +103,33 @@ describe('DebugError hierarchy', () => {
     expect(getErrorMessage('text')).toBe('text');
   });
 });
+
+describe('the JSON-RPC boundary shape (issue #659)', () => {
+  it('mcpErrorDetail strips exactly one prefix, matching the error\'s own code', () => {
+    expect(mcpErrorDetail(new McpError(McpErrorCode.InvalidParams, 'bad arg'))).toBe('bad arg');
+    expect(mcpErrorDetail(new SessionNotFoundError('s1'))).toBe('Session not found: s1');
+    // A detail that itself starts with a prefix (a client re-wrapping a wire
+    // message) keeps that one: only the layer this error added comes off.
+    expect(mcpErrorDetail(new McpError(McpErrorCode.InvalidParams, 'MCP error -32602: nested')))
+      .toBe('MCP error -32602: nested');
+  });
+
+  it('WireMcpError is an McpError whose .message is the bare detail', () => {
+    const error = new WireMcpError(McpErrorCode.InvalidParams, 'bad arg', { k: 1 });
+    expect(error).toBeInstanceOf(McpError);
+    expect(error.code).toBe(McpErrorCode.InvalidParams);
+    expect(error.message).toBe('bad arg');
+    expect(error.data).toEqual({ k: 1 });
+  });
+
+  it('toWireError keeps code, data and stack and is idempotent', () => {
+    const source = new ProxyNotRunningError('s1', 'pause');
+    const wire = toWireError(source);
+    expect(wire).toBeInstanceOf(WireMcpError);
+    expect(wire.code).toBe(source.code);
+    expect(wire.data).toEqual(source.data);
+    expect(wire.stack).toBe(source.stack);
+    expect(wire.message).toBe('Cannot pause: no active proxy for session s1');
+    expect(toWireError(wire)).toBe(wire);
+  });
+});

tests/core/unit/server/mcp-wire-errors.test.ts

@@ -0,0 +1,138 @@
+/**
+ * Protocol errors as an SDK client sees them (issue #659).
+ *
+ * The SDK's Protocol copies a thrown error's `.message` verbatim into the
+ * JSON-RPC error and the client-side McpError constructor prefixes
+ * `MCP error <code>: ` again, so an McpError thrown from a request handler
+ * used to reach clients doubled. These tests drive the real handlers through
+ * a real Server/Client pair over the SDK's in-memory transport and pin the
+ * exact client-side message: one prefix, then the reason.
+ */
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { Client } from '@modelcontextprotocol/sdk/client/index.js';
+import { Server } from '@modelcontextprotocol/sdk/server/index.js';
+import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
+import { McpError, ErrorCode as McpErrorCode } from '@modelcontextprotocol/sdk/types.js';
+import { registerToolHandlers } from '../../../../src/server/tool-dispatch.js';
+import { registerPromptHandlers } from '../../../../src/server/prompts.js';
+import { registerResourceHandlers } from '../../../../src/server/output-resources.js';
+import type { ToolContext } from '../../../../src/server/tool-context.js';
+import type { SessionManager } from '../../../../src/session/session-manager.js';
+import type { OutputResourceNotifier } from '../../../../src/server/output-resources.js';
+import { SessionNotFoundError } from '../../../../src/errors/debug-errors.js';
+
+const logger = { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() };
+const environment = {
+  get: (): string | undefined => undefined,
+  getAll: () => ({}),
+  getCurrentWorkingDirectory: () => '/'
+};
+
+function fakeContext(overrides: Partial<ToolContext> = {}): ToolContext {
+  return {
+    logger,
+    environment,
+    getSessionName: () => 'sess',
+    getSupportedLanguagesAsync: async () => ['mock'],
+    validateSession: (sessionId: string) => {
+      throw new SessionNotFoundError(sessionId);
+    },
+    ...overrides
+  } as unknown as ToolContext;
+}
+
+async function connect(server: Server): Promise<Client> {
+  const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+  const client = new Client({ name: 'wire-test', version: '0.0.0' });
+  await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
+  return client;
+}
+
+async function caught(promise: Promise<unknown>): Promise<McpError> {
+  try {
+    await promise;
+  } catch (error) {
+    expect(error).toBeInstanceOf(McpError);
+    return error as McpError;
+  }
+  throw new Error('expected the request to be rejected');
+}
+
+describe('protocol errors reach SDK clients with a single MCP error prefix', () => {
+  let server: Server;
+  let client: Client;
+
+  beforeEach(() => {
+    server = new Server(
+      { name: 'wire-test-server', version: '0.0.0' },
+      { capabilities: { tools: {}, prompts: {}, resources: { subscribe: true } } }
+    );
+  });
+
+  afterEach(async () => {
+    await client?.close();
+    await server.close();
+  });
+
+  it('argument validation (tools/call)', async () => {
+    registerToolHandlers(server, fakeContext());
+    client = await connect(server);
+
+    const error = await caught(client.callTool({ name: 'set_breakpoint', arguments: { file: '/x.py', line: 1 } }));
+    expect(error.code).toBe(McpErrorCode.InvalidParams);
+    expect(error.message).toBe(`MCP error ${McpErrorCode.InvalidParams}: Missing required parameter: sessionId`);
+  });
+
+  it('a handler rejection that is not an McpError is wrapped once as InternalError', async () => {
+    registerToolHandlers(
+      server,
+      fakeContext({
+        getAdapterRegistry: () => {
+          throw new Error('registry exploded');
+        }
+      })
+    );
+    client = await connect(server);
+
+    const error = await caught(client.callTool({ name: 'list_supported_languages', arguments: {} }));
+    expect(error.code).toBe(McpErrorCode.InternalError);
+    expect(error.message).toBe(
+      `MCP error ${McpErrorCode.InternalError}: Failed to list supported languages: registry exploded`
+    );
+  });
+
+  it('an unknown tool', async () => {
+    registerToolHandlers(server, fakeContext());
+    client = await connect(server);
+
+    const error = await caught(client.callTool({ name: 'no_such_tool', arguments: {} }));
+    expect(error.code).toBe(McpErrorCode.MethodNotFound);
+    expect(error.message).toBe(`MCP error ${McpErrorCode.MethodNotFound}: Unknown tool: no_such_tool`);
+  });
+
+  it('an unknown prompt', async () => {
+    registerPromptHandlers(server, environment);
+    client = await connect(server);
+
+    const error = await caught(client.getPrompt({ name: 'nope' }));
+    expect(error.code).toBe(McpErrorCode.InvalidParams);
+    expect(error.message).toBe(`MCP error ${McpErrorCode.InvalidParams}: Unknown prompt: nope`);
+  });
+
+  it('an unknown resource (read and subscribe)', async () => {
+    const sessionManager = {
+      getAllSessions: () => [],
+      getSession: () => undefined
+    } as unknown as SessionManager;
+    const notifier = { subscribe: vi.fn(), unsubscribe: vi.fn() } as unknown as OutputResourceNotifier;
+    registerResourceHandlers(server, sessionManager, notifier, { readTail: vi.fn() });
+    client = await connect(server);
+
+    const uri = 'debug://sessions/ghost/output';
+    for (const request of [client.readResource({ uri }), client.subscribeResource({ uri })]) {
+      const error = await caught(request);
+      expect(error.code).toBe(McpErrorCode.InvalidParams);
+      expect(error.message).toBe(`MCP error ${McpErrorCode.InvalidParams}: Unknown resource: ${uri}`);
+    }
+  });
+});