Fix: Correctly detect MCP tool errors (#14937)

Author: kevin-ramdass

packages/core/src/tools/mcp-tool.test.ts

@@ -223,6 +223,44 @@ describe('DiscoveredMCPTool', () => {
       },
     );
 
+    it('should return a structured error if MCP tool reports a top-level isError (spec compliant)', async () => {
+      const tool = new DiscoveredMCPTool(
+        mockCallableToolInstance,
+        serverName,
+        serverToolName,
+        baseDescription,
+        inputSchema,
+      );
+      const params = { param: 'isErrorTopLevelCase' };
+      const functionCall = {
+        name: serverToolName,
+        args: params,
+      };
+
+      // Spec compliant error response: { isError: true } at the top level of content (or response object in this mapping)
+      const errorResponse = { isError: true };
+      const mockMcpToolResponseParts: Part[] = [
+        {
+          functionResponse: {
+            name: serverToolName,
+            response: errorResponse,
+          },
+        },
+      ];
+      mockCallTool.mockResolvedValue(mockMcpToolResponseParts);
+      const expectedErrorMessage = `MCP tool '${serverToolName}' reported tool error for function call: ${safeJsonStringify(
+        functionCall,
+      )} with response: ${safeJsonStringify(mockMcpToolResponseParts)}`;
+      const invocation = tool.build(params);
+      const result = await invocation.execute(new AbortController().signal);
+
+      expect(result.error?.type).toBe(ToolErrorType.MCP_TOOL_ERROR);
+      expect(result.llmContent).toBe(expectedErrorMessage);
+      expect(result.returnDisplay).toContain(
+        `Error: MCP tool '${serverToolName}' reported an error.`,
+      );
+    });
+
     it.each([
       { isErrorValue: false, description: 'false (bool)' },
       { isErrorValue: 'false', description: '"false" (str)' },

packages/core/src/tools/mcp-tool.ts

@@ -133,6 +133,13 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation<
     }
 
     if (response) {
+      // Check for top-level isError (MCP Spec compliant)
+      const isErrorTop = (response as { isError?: boolean | string }).isError;
+      if (isErrorTop === true || isErrorTop === 'true') {
+        return true;
+      }
+
+      // Legacy check for nested error object (keep for backward compatibility if any tools rely on it)
       const error = (response as { error?: McpError })?.error;
       const isError = error?.isError;