diff --git a/webview-ui/src/components/chat/McpExecution.tsx b/webview-ui/src/components/chat/McpExecution.tsx index 9e48552fdc8..00d69ec5b95 100644 --- a/webview-ui/src/components/chat/McpExecution.tsx +++ b/webview-ui/src/components/chat/McpExecution.tsx @@ -60,33 +60,113 @@ export const McpExecution = ({ // Only need expanded state for response section (like command output) const [isResponseExpanded, setIsResponseExpanded] = useState(false) - // Try to parse JSON and return both the result and formatted text - const tryParseJson = useCallback((text: string): { isJson: boolean; formatted: string } => { - if (!text) return { isJson: false, formatted: "" } + const looksLikeJson = useCallback((value: string): boolean => { + const trimmed = value.trim() + return (trimmed.startsWith("{") && trimmed.endsWith("}")) || (trimmed.startsWith("[") && trimmed.endsWith("]")) + }, []) + + const looksLikeEscapedJsonBlob = useCallback( + (value: string): boolean => { + // Gate the "unescape blob" fallback strictly to avoid mutating arbitrary strings + // that merely contain backslashes. + // Examples we want to handle: + // - `{\"id\":1}` + // - `[{\"id\":1}]` + const trimmed = value.trim() + if (!looksLikeJson(trimmed)) return false + return /^\{\\"/.test(trimmed) || /^\[\s*\{\\"/.test(trimmed) + }, + [looksLikeJson], + ) + const tryParseJsonValue = useCallback((value: string): unknown | undefined => { try { - const parsed = JSON.parse(text) - return { - isJson: true, - formatted: JSON.stringify(parsed, null, 2), - } + return JSON.parse(value) + } catch { + return undefined + } + }, []) + + const tryUnescapeJsonBlob = useCallback((value: string): string | undefined => { + // Some MCP servers return JSON "blobs" with escaped quotes/backslashes but WITHOUT + // wrapping the value in a JSON string literal (e.g. `[{\"id\":1}]`). + // + // Avoid manual `replaceAll("\\\\", "\\")` / `replaceAll('\\"', '"')` transformations, + // which can accidentally double-unescape sequences. Instead, ask the JSON parser to + // interpret escape sequences by wrapping the blob into a JSON string literal. + try { + // Intentionally do NOT escape backslashes, so sequences like `\"` and `\\` are + // interpreted as escapes inside the JSON string literal. + // We only escape raw newlines/tabs so the wrapper remains valid JSON. + const jsonStringLiteral = `"${value + .replaceAll("\n", "\\n") + .replaceAll("\r", "\\r") + .replaceAll("\t", "\\t")}"` + const decoded = JSON.parse(jsonStringLiteral) + return typeof decoded === "string" ? decoded : undefined } catch { + return undefined + } + }, []) + + // Try to parse JSON and return both the result and formatted text. + // Handles: + // - minified JSON + // - double-encoded JSON (JSON string containing JSON) + // - escaped JSON blobs (eg `[{"id":1}]`) + const tryParseJson = useCallback( + (text: string): { isJson: boolean; formatted: string } => { + if (!text) return { isJson: false, formatted: "" } + + const trimmed = text.trim() + let parsed: unknown | undefined = tryParseJsonValue(trimmed) + + // If initial parse fails, try un-escaping common "JSON encoded as a string blob" patterns. + if (parsed === undefined && looksLikeEscapedJsonBlob(trimmed)) { + const unescaped = tryUnescapeJsonBlob(trimmed) + if (unescaped !== undefined) { + parsed = tryParseJsonValue(unescaped) + } + } + + // If we parsed a string that itself looks like JSON, try parsing again (double-encoded). + for (let i = 0; i < 2; i++) { + if (typeof parsed === "string" && looksLikeJson(parsed)) { + parsed = tryParseJsonValue(parsed) + continue + } + break + } + + // Only treat as JSON when we end up with a non-string JSON value. + if (parsed !== undefined && typeof parsed !== "string") { + return { + isJson: true, + formatted: JSON.stringify(parsed, null, 2), + } + } + return { isJson: false, formatted: text, } - } - }, []) + }, + [looksLikeEscapedJsonBlob, looksLikeJson, tryParseJsonValue, tryUnescapeJsonBlob], + ) // Only parse response data when expanded AND complete to avoid parsing partial JSON const responseData = useMemo(() => { if (!isResponseExpanded) { return { isJson: false, formatted: responseText } } - // Only try to parse JSON if the response is complete - if (status && status.status === "completed") { + + // If we have streaming status, only parse when completed. + // If we have no status, this is typically a non-streaming "ask" payload, so treat as complete. + const shouldParse = status ? status.status === "completed" : true + if (shouldParse) { return tryParseJson(responseText) } + // For partial responses, just return as-is without parsing return { isJson: false, formatted: responseText } }, [responseText, isResponseExpanded, tryParseJson, status]) @@ -125,6 +205,7 @@ export const McpExecution = ({ const formattedResponseText = responseData.formatted const formattedArgumentsText = argumentsData.formatted const responseIsJson = responseData.isJson + const rawResponseText = responseText const onToggleResponseExpand = useCallback(() => { setIsResponseExpanded(!isResponseExpanded) @@ -277,7 +358,7 @@ export const McpExecution = ({ "mt-1 pt-1": !isArguments && (useMcpServer?.type === "use_mcp_tool" || (toolName && serverName)), })}> - + )} @@ -285,6 +366,7 @@ export const McpExecution = ({ {isJson ? ( - + ) : ( )} diff --git a/webview-ui/src/components/chat/__tests__/McpExecution.spec.tsx b/webview-ui/src/components/chat/__tests__/McpExecution.spec.tsx new file mode 100644 index 00000000000..ced39c2a74c --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/McpExecution.spec.tsx @@ -0,0 +1,147 @@ +import React from "react" +import { render, screen, fireEvent } from "@testing-library/react" + +import { McpExecution } from "../McpExecution" + +vi.mock("react-use", () => ({ + useEvent: vi.fn(), +})) + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})) + +// CodeBlock is heavy (shiki); mock it to just print its props. +vi.mock("../../common/CodeBlock", () => ({ + default: ({ source, rawSource, language }: { source: string; rawSource?: string; language: string }) => + !source || source.length === 0 ? null : ( +
+ {source} +
+ ), +})) + +vi.mock("../Markdown", () => ({ + Markdown: ({ markdown }: { markdown: string }) =>
{markdown}
, +})) + +vi.mock("../../mcp/McpToolRow", () => ({ + default: ({ tool }: { tool: { name: string } }) =>
{tool.name}
, +})) + +describe("McpExecution", () => { + it("pretty prints minified JSON response when expanded (no streaming status)", () => { + render( + , + ) + + // Expand response + fireEvent.click(screen.getByRole("button")) + + const block = screen.getByTestId("code-block-json") + expect(block).toHaveTextContent('"id": 1') + expect(block).toHaveTextContent('"title": "PR"') + // raw output preserved for copy + expect(block.getAttribute("data-raw")).toBe('[{"id":1,"title":"PR"}]') + }) + + it("pretty prints escaped JSON blobs when expanded", () => { + // Example: JSON content returned as an escaped JSON blob string (common in some MCP servers) + const escapedBlob = '[{\\"id\\":1,\\"title\\":\\"PR\\"}]' + + render( + , + ) + + fireEvent.click(screen.getByRole("button")) + + const block = screen.getByTestId("code-block-json") + expect(block).toHaveTextContent('"id": 1') + expect(block).toHaveTextContent('"title": "PR"') + expect(block.getAttribute("data-raw")).toBe(escapedBlob) + }) + + it("pretty prints double-encoded JSON when expanded", () => { + // Example: response is a JSON string whose content is JSON + const doubleEncoded = JSON.stringify('[{"id":1,"title":"PR"}]') + + render( + , + ) + + fireEvent.click(screen.getByRole("button")) + + const block = screen.getByTestId("code-block-json") + expect(block).toHaveTextContent('"id": 1') + expect(block).toHaveTextContent('"title": "PR"') + expect(block.getAttribute("data-raw")).toBe(doubleEncoded) + }) + + it("renders non-JSON response as Markdown", () => { + render( + , + ) + + fireEvent.click(screen.getByRole("button")) + + expect(screen.getByTestId("markdown")).toHaveTextContent("not json") + }) +})