-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(mcp): pretty-print tool responses in UI #11115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,33 +60,75 @@ | |
| // 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 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 | ||
| } | ||
| }, []) | ||
|
|
||
| // 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 && (trimmed.includes('\\"') || trimmed.includes("\\\\"))) { | ||
| const unescaped = trimmed.replaceAll("\\\\", "\\").replaceAll('\\"', '"') | ||
| 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, | ||
| } | ||
| } | ||
| }, []) | ||
| }, | ||
| [looksLikeJson, tryParseJsonValue], | ||
| ) | ||
|
|
||
| // 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 +167,7 @@ | |
| const formattedResponseText = responseData.formatted | ||
| const formattedArgumentsText = argumentsData.formatted | ||
| const responseIsJson = responseData.isJson | ||
| const rawResponseText = responseText | ||
|
|
||
| const onToggleResponseExpand = useCallback(() => { | ||
| setIsResponseExpanded(!isResponseExpanded) | ||
|
|
@@ -277,14 +320,15 @@ | |
| "mt-1 pt-1": | ||
| !isArguments && (useMcpServer?.type === "use_mcp_tool" || (toolName && serverName)), | ||
| })}> | ||
| <CodeBlock source={formattedArgumentsText} language="json" /> | ||
| <CodeBlock source={formattedArgumentsText} rawSource={argumentsText} language="json" /> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Response section - collapsible like command output */} | ||
| <ResponseContainer | ||
| isExpanded={isResponseExpanded} | ||
| response={formattedResponseText} | ||
| rawResponse={rawResponseText} | ||
| isJson={responseIsJson} | ||
| hasArguments={!!(isArguments || useMcpServer?.arguments || argumentsText)} | ||
| isPartial={status ? status.status !== "completed" : false} | ||
|
|
@@ -299,12 +343,14 @@ | |
| const ResponseContainerInternal = ({ | ||
| isExpanded, | ||
| response, | ||
| rawResponse, | ||
| isJson, | ||
| hasArguments, | ||
| isPartial = false, | ||
| }: { | ||
| isExpanded: boolean | ||
| response: string | ||
| rawResponse: string | ||
| isJson: boolean | ||
| hasArguments?: boolean | ||
| isPartial?: boolean | ||
|
|
@@ -327,7 +373,7 @@ | |
| "max-h-96 overflow-y-auto mt-1 pt-1": !hasArguments, | ||
| })}> | ||
| {isJson ? ( | ||
| <CodeBlock source={response} language="json" /> | ||
| <CodeBlock source={response} rawSource={rawResponse} language="json" /> | ||
| ) : ( | ||
| <Markdown markdown={response} partial={isPartial} /> | ||
| )} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 : ( | ||
| <div data-testid={`code-block-${language}`} data-raw={rawSource ?? ""}> | ||
| {source} | ||
| </div> | ||
| ), | ||
| })) | ||
|
|
||
| vi.mock("../Markdown", () => ({ | ||
| Markdown: ({ markdown }: { markdown: string }) => <div data-testid="markdown">{markdown}</div>, | ||
| })) | ||
|
|
||
| vi.mock("../../mcp/McpToolRow", () => ({ | ||
| default: ({ tool }: { tool: { name: string } }) => <div data-testid="mcp-tool-row">{tool.name}</div>, | ||
| })) | ||
|
|
||
| describe("McpExecution", () => { | ||
| it("pretty prints minified JSON response when expanded (no streaming status)", () => { | ||
| render( | ||
| <McpExecution | ||
| executionId="1" | ||
| serverName="github" | ||
| toolName="list_pull_requests" | ||
| isArguments={true} | ||
| useMcpServer={ | ||
| { | ||
| type: "use_mcp_tool", | ||
| serverName: "github", | ||
| toolName: "list_pull_requests", | ||
| arguments: "{}", | ||
| response: '[{"id":1,"title":"PR"}]', | ||
| } as any | ||
| } | ||
| />, | ||
| ) | ||
|
|
||
| // 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( | ||
| <McpExecution | ||
| executionId="1" | ||
| serverName="github" | ||
| toolName="list_pull_requests" | ||
| isArguments={true} | ||
| useMcpServer={ | ||
| { | ||
| type: "use_mcp_tool", | ||
| serverName: "github", | ||
| toolName: "list_pull_requests", | ||
| arguments: "{}", | ||
| response: escapedBlob, | ||
| } as any | ||
| } | ||
| />, | ||
| ) | ||
|
|
||
| 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( | ||
| <McpExecution | ||
| executionId="1" | ||
| serverName="github" | ||
| toolName="list_pull_requests" | ||
| isArguments={true} | ||
| useMcpServer={ | ||
| { | ||
| type: "use_mcp_tool", | ||
| serverName: "github", | ||
| toolName: "list_pull_requests", | ||
| arguments: "{}", | ||
| response: doubleEncoded, | ||
| } as any | ||
| } | ||
| />, | ||
| ) | ||
|
|
||
| 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( | ||
| <McpExecution | ||
| executionId="1" | ||
| serverName="github" | ||
| toolName="list_pull_requests" | ||
| isArguments={true} | ||
| useMcpServer={ | ||
| { | ||
| type: "use_mcp_tool", | ||
| serverName: "github", | ||
| toolName: "list_pull_requests", | ||
| arguments: "{}", | ||
| response: "not json", | ||
| } as any | ||
| } | ||
| />, | ||
| ) | ||
|
|
||
| fireEvent.click(screen.getByRole("button")) | ||
|
|
||
| expect(screen.getByTestId("markdown")).toHaveTextContent("not json") | ||
| }) | ||
| }) |
Uh oh!
There was an error while loading. Please reload this page.