From 245e6bc88d5a63954fe631d394e1656faf073c68 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Wed, 3 Dec 2025 01:51:51 -0500 Subject: [PATCH] Use a much more lenient JSON parsing strategy for tools --- .../assistant-message/NativeToolCallParser.ts | 24 ++- src/utils/__tests__/safeParseJson.spec.ts | 193 ++++++++++++++++++ src/utils/safeParseJson.ts | 81 ++++++++ 3 files changed, 292 insertions(+), 6 deletions(-) create mode 100644 src/utils/__tests__/safeParseJson.spec.ts create mode 100644 src/utils/safeParseJson.ts diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index ac95597779..36171ebff9 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -12,6 +12,7 @@ import type { ApiStreamToolCallDeltaChunk, ApiStreamToolCallEndChunk, } from "../../api/transform/stream" +import { safeParsePossiblyTabCorruptedJson } from "../../utils/safeParseJson" /** * Helper type to extract properly typed native arguments for a given tool. @@ -558,10 +559,16 @@ export class NativeToolCallParser { return null } - try { - // Parse the arguments JSON string - const args = JSON.parse(toolCall.arguments) + // Parse the arguments JSON string using safe parser that handles tab corruption + const parseResult = safeParsePossiblyTabCorruptedJson>(toolCall.arguments) + if (!parseResult.ok) { + console.error(`Failed to parse tool call arguments:`, parseResult.error) + console.error(`Error details:`, parseResult.error.message) + return null + } + const args = parseResult.value + try { // Build legacy params object for backward compatibility with XML protocol and UI. // Native execution path uses nativeArgs instead, which has proper typing. const params: Partial> = {} @@ -804,10 +811,15 @@ export class NativeToolCallParser { * The use_mcp_tool wrapper is only used in XML mode. */ public static parseDynamicMcpTool(toolCall: { id: string; name: string; arguments: string }): McpToolUse | null { - try { - // Parse the arguments - these are the actual tool arguments passed directly - const args = JSON.parse(toolCall.arguments || "{}") + // Parse the arguments using safe parser that handles tab corruption + const parseResult = safeParsePossiblyTabCorruptedJson>(toolCall.arguments || "{}") + if (!parseResult.ok) { + console.error(`Failed to parse dynamic MCP tool arguments:`, parseResult.error) + return null + } + const args = parseResult.value + try { // Extract server_name and tool_name from the tool name itself // Format: mcp_serverName_toolName const nameParts = toolCall.name.split("_") diff --git a/src/utils/__tests__/safeParseJson.spec.ts b/src/utils/__tests__/safeParseJson.spec.ts new file mode 100644 index 0000000000..1b8864d927 --- /dev/null +++ b/src/utils/__tests__/safeParseJson.spec.ts @@ -0,0 +1,193 @@ +import { safeParsePossiblyTabCorruptedJson, type SafeParseResult } from "../safeParseJson" + +describe("safeParsePossiblyTabCorruptedJson", () => { + describe("valid JSON without tabs", () => { + it("should parse valid JSON successfully", () => { + const input = '{"name":"test","value":123}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ name: "test", value: 123 }) + expect(result.repaired).toBe(false) + expect(result.source).toBe(input) + } + }) + + it("should handle escaped tabs in strings without repair", () => { + const input = '{"content":"line1\\tline2"}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ content: "line1\tline2" }) + expect(result.repaired).toBe(false) + } + }) + + it("should parse arrays correctly", () => { + const input = '["a","b","c"]' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual(["a", "b", "c"]) + expect(result.repaired).toBe(false) + } + }) + }) + + describe("JSON with raw tabs inside strings", () => { + it("should repair and parse JSON with raw tab in string value", () => { + // This JSON has a raw tab character inside the string value + const input = '{"content":"before\tafter"}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ content: "before\tafter" }) + expect(result.repaired).toBe(true) + expect(result.source).toBe('{"content":"before\\tafter"}') + } + }) + + it("should repair multiple raw tabs in the same string", () => { + const input = '{"content":"a\tb\tc\td"}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ content: "a\tb\tc\td" }) + expect(result.repaired).toBe(true) + } + }) + + it("should repair raw tabs in multiple string fields", () => { + const input = '{"field1":"has\ttab","field2":"also\ttab"}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ field1: "has\ttab", field2: "also\ttab" }) + expect(result.repaired).toBe(true) + } + }) + + it("should not modify tabs outside of string literals", () => { + // This tests that tabs used as whitespace in JSON structure are preserved + // Note: standard JSON.parse handles tabs as whitespace fine, this just ensures + // our repair doesn't break that + const input = '{\t"name"\t:\t"value"\t}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ name: "value" }) + expect(result.repaired).toBe(false) // No repair needed - tabs outside strings are valid + } + }) + }) + + describe("invalid JSON", () => { + it("should return error for completely invalid JSON", () => { + const input = "not json at all" + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(false) + if (!result.ok) { + expect(result.error).toBeInstanceOf(Error) + expect(result.repaired).toBe(false) // No repair attempted since no strings with tabs + } + }) + + it("should return error for unclosed strings", () => { + const input = '{"name":"unclosed' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(false) + if (!result.ok) { + expect(result.error).toBeInstanceOf(Error) + } + }) + + it("should successfully parse partial JSON after repair due to lenient parser", () => { + // This has a raw tab AND other syntax errors + // parseJSON is lenient and parses what it can, so this succeeds + const input = '{"content":"has\ttab", broken}' + const result = safeParsePossiblyTabCorruptedJson(input) + + // parseJSON successfully parses the valid portion after tab repair + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ content: "has\ttab" }) + expect(result.repaired).toBe(true) + } + }) + }) + + describe("edge cases", () => { + it("should handle empty object", () => { + const result = safeParsePossiblyTabCorruptedJson("{}") + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({}) + } + }) + + it("should handle empty string input", () => { + const result = safeParsePossiblyTabCorruptedJson("") + expect(result.ok).toBe(false) + }) + + it("should handle string with escaped backslash before tab", () => { + // The \\\t should be parsed as escaped backslash followed by tab + const input = '{"path":"C:\\\\folder\tname"}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.repaired).toBe(true) + } + }) + + it("should handle nested objects with tabs", () => { + const input = '{"outer":{"inner":"has\ttab"}}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual({ outer: { inner: "has\ttab" } }) + expect(result.repaired).toBe(true) + } + }) + + it("should handle arrays with tabs in string elements", () => { + const input = '["no tab","has\ttab","also\ttab"]' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value).toEqual(["no tab", "has\ttab", "also\ttab"]) + expect(result.repaired).toBe(true) + } + }) + }) + + describe("type parameter", () => { + it("should return correctly typed result", () => { + interface TestType { + name: string + count: number + } + const input = '{"name":"test","count":42}' + const result = safeParsePossiblyTabCorruptedJson(input) + + expect(result.ok).toBe(true) + if (result.ok) { + // TypeScript should know value is TestType + expect(result.value.name).toBe("test") + expect(result.value.count).toBe(42) + } + }) + }) +}) diff --git a/src/utils/safeParseJson.ts b/src/utils/safeParseJson.ts new file mode 100644 index 0000000000..9df8f9ab0c --- /dev/null +++ b/src/utils/safeParseJson.ts @@ -0,0 +1,81 @@ +import { parseJSON } from "partial-json" + +/** + * Replace raw TAB characters inside JSON string literals with \t + * without touching anything outside of quoted strings. + */ +function repairTabsInsideJsonStrings(rawJson: string): string { + return rawJson.replace( + /"(?:[^"\\]|\\.)*"/g, // match JSON string literals + (str) => str.replace(/\t/g, "\\t"), + ) +} + +export type SafeParseResult = + | { + ok: true + value: T + repaired: boolean + source: string // the JSON text that was successfully parsed + } + | { + ok: false + error: Error + repaired: boolean + source: string // last JSON text we attempted to parse + } + +/** + * Safely parse JSON that may contain raw TAB characters inside string values. + * - First tries JSON.parse(rawJson) for strict validation + * - If that fails, repairs raw tabs *inside strings* and tries again with parseJSON + * (which is more lenient with incomplete JSON from streaming) + */ +export function safeParsePossiblyTabCorruptedJson(rawJson: string): SafeParseResult { + // First attempt: strict parse to detect invalid JSON + try { + const value = JSON.parse(rawJson) as T + return { + ok: true, + value, + repaired: false, + source: rawJson, + } + } catch (originalError) { + // Second attempt: repair tabs *inside* string literals and use lenient parser + const repairedSource = repairTabsInsideJsonStrings(rawJson) + + // If nothing changed, don't bother retrying + if (repairedSource === rawJson) { + return { + ok: false, + error: originalError as Error, + repaired: false, + source: rawJson, + } + } + + try { + // Use parseJSON for extra leniency with the repaired source + const value = parseJSON(repairedSource) as T + return { + ok: true, + value, + repaired: true, + source: repairedSource, + } + } catch (repairedError) { + const combined = new Error( + `Failed to parse JSON even after repairing tabs.\n` + + `Original error: ${(originalError as Error).message}\n` + + `After repair: ${(repairedError as Error).message}`, + ) + return { + ok: false, + error: combined, + repaired: true, + source: repairedSource, + } + } + } +}