From cce793927a150ca71fb211cac48361233978b56b Mon Sep 17 00:00:00 2001
From: KooshaPari <koosha@example.com>
Date: Sat, 12 Sep 2026 21:35:34 -0700
Subject: [PATCH] fix: prevent double-prefixed McpError message when
 reconstructing from wire (fixes #2786)

McpError's constructor prefixes messages with 'MCP error NNNN: ', and the
server serializes this prefixed message as error.message. When the client
reconstructs the error via fromError, the already-prefixed message gets
the prefix added again, resulting in messages like:
  'MCP error -32601: MCP error -32601: Unknown tool: nope'

Fixed by stripping the prefix in fromError before passing to the
constructor, so deserialized errors get exactly one prefix. Also changed
the queued-response path in protocol.ts to use fromError consistently
instead of new McpError.
---
 src/shared/protocol.ts       |  2 +-
 src/types.ts                 | 17 +++++++++++++++--
 test/shared/protocol.test.ts | 26 ++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts
index 2637be65bc..2b5f9bd7dd 100644
--- a/src/shared/protocol.ts
+++ b/src/shared/protocol.ts
@@ -892,7 +892,7 @@ export abstract class Protocol<SendRequestT extends Request, SendNotificationT e
             if (isJSONRPCResultResponse(response)) {
                 resolver(response);
             } else {
-                const error = new McpError(response.error.code, response.error.message, response.error.data);
+                const error = McpError.fromError(response.error.code, response.error.message, response.error.data);
                 resolver(error);
             }
             return;
diff --git a/src/types.ts b/src/types.ts
index 835eac89f8..b74d8f05dd 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -2314,20 +2314,33 @@ export class McpError extends Error {
         this.name = 'McpError';
     }
 
+    /**
+     * Strip the "MCP error NNNN: " prefix that McpError's constructor adds,
+     * so that fromError does not double-prefix when reconstructing an error
+     * whose message already carries the prefix from the wire.
+     */
+    private static _stripErrorPrefix(message: string): string {
+        return message.replace(/^MCP error -?\d+: /, '');
+    }
+
     /**
      * Factory method to create the appropriate error type based on the error code and data
      */
     static fromError(code: number, message: string, data?: unknown): McpError {
+        // Strip the "MCP error NNNN: " prefix if present, to avoid double-prefixing
+        // when the message comes from a serialized McpError on the wire.
+        const cleanMessage = McpError._stripErrorPrefix(message);
+
         // Check for specific error types
         if (code === ErrorCode.UrlElicitationRequired && data) {
             const errorData = data as { elicitations?: unknown[] };
             if (errorData.elicitations) {
-                return new UrlElicitationRequiredError(errorData.elicitations as ElicitRequestURLParams[], message);
+                return new UrlElicitationRequiredError(errorData.elicitations as ElicitRequestURLParams[], cleanMessage);
             }
         }
 
         // Default to generic McpError
-        return new McpError(code, message, data);
+        return new McpError(code, cleanMessage, data);
     }
 }
 
diff --git a/test/shared/protocol.test.ts b/test/shared/protocol.test.ts
index 733146f29b..9585a9e2e9 100644
--- a/test/shared/protocol.test.ts
+++ b/test/shared/protocol.test.ts
@@ -5403,6 +5403,32 @@ describe('Error handling for missing resolvers', () => {
         });
     });
 
+    describe('McpError.fromError should not double-prefix', () => {
+        it('should not double-prefix when message already has MCP error prefix', () => {
+            // Simulate the message as it arrives from the wire (already prefixed by server's McpError constructor)
+            const prefixedMessage = 'MCP error -32601: Unknown tool: nope';
+            const error = McpError.fromError(ErrorCode.MethodNotFound, prefixedMessage);
+
+            // Should have exactly one prefix, not two
+            expect(error.message).toBe('MCP error -32601: Unknown tool: nope');
+            expect(error.message).not.toContain('MCP error -32601: MCP error');
+        });
+
+        it('should add prefix when message does not have it', () => {
+            // Direct usage without prefix (e.g., internal code creating an error)
+            const error = McpError.fromError(ErrorCode.InternalError, 'Something went wrong');
+
+            // Should have the prefix added
+            expect(error.message).toBe('MCP error -32603: Something went wrong');
+        });
+
+        it('should handle negative error codes correctly', () => {
+            const prefixedMessage = 'MCP error -32600: Invalid request';
+            const error = McpError.fromError(ErrorCode.InvalidRequest, prefixedMessage);
+            expect(error.message).toBe('MCP error -32600: Invalid request');
+        });
+    });
+
     describe('Response and error message routing integration', () => {
         it('should handle mixed response and error messages in queue', async () => {
             await protocol.connect(transport);