diff --git a/packages/types/src/experiment.ts b/packages/types/src/experiment.ts index f6f701a25d3..8a8c45e3dcb 100644 --- a/packages/types/src/experiment.ts +++ b/packages/types/src/experiment.ts @@ -14,6 +14,7 @@ export const experimentIds = [ "runSlashCommand", "multipleNativeToolCalls", "customTools", + "malformedJsonRepair", ] as const export const experimentIdsSchema = z.enum(experimentIds) @@ -32,6 +33,11 @@ export const experimentsSchema = z.object({ runSlashCommand: z.boolean().optional(), multipleNativeToolCalls: z.boolean().optional(), customTools: z.boolean().optional(), + /** + * Enable automatic repair of malformed JSON in LLM tool call responses. + * Useful for models like Grok that struggle with strict JSON formatting. + */ + malformedJsonRepair: z.boolean().optional(), }) export type Experiments = z.infer diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 190639f11df..1422d9f1e1b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -812,6 +812,9 @@ importers: isbinaryfile: specifier: ^5.0.2 version: 5.0.4 + jsonrepair: + specifier: ^3.13.1 + version: 3.13.1 jwt-decode: specifier: ^4.0.0 version: 4.0.0 @@ -7142,6 +7145,10 @@ packages: jsonfile@4.0.0: resolution: {integrity: sha512-m6F1R3z8jjlf2imQHS2Qez5sjKWQzbuuhuJ/FKYFRZvPE3PuHcSMVZzfsLhGVOkfd20obL5SWEBew5ShlquNxg==} + jsonrepair@3.13.1: + resolution: {integrity: sha512-WJeiE0jGfxYmtLwBTEk8+y/mYcaleyLXWaqp5bJu0/ZTSeG0KQq/wWQ8pmnkKenEdN6pdnn6QtcoSUkbqDHWNw==} + hasBin: true + jsonschema@1.5.0: resolution: {integrity: sha512-K+A9hhqbn0f3pJX17Q/7H6yQfD/5OXgdrR5UE12gMXCiN9D5Xq2o5mddV2QEcX/bjla99ASsAAQUyMCCRWAEhw==} @@ -10191,6 +10198,7 @@ packages: whatwg-encoding@3.1.1: resolution: {integrity: sha512-6qN4hJdMwfYBtE3YBTTHhoeuUrDBPZmbQaxWAqSALV/MeEnR5z1xd8UKud2RAkFoPkmB+hli1TZSnyi84xz1vQ==} engines: {node: '>=18'} + deprecated: Use @exodus/bytes instead for a more spec-conformant and faster implementation whatwg-fetch@3.6.20: resolution: {integrity: sha512-EqhiFU6daOA8kpjOWTL0olhVOF3i7OrFzSYiGsEMB8GcXS+RrzauAERX65xMeNWVqxA6HXH2m69Z9LaKKdisfg==} @@ -14225,7 +14233,7 @@ snapshots: sirv: 3.0.1 tinyglobby: 0.2.14 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.2.1)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@20.17.50)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) '@vitest/utils@3.2.4': dependencies: @@ -17279,6 +17287,8 @@ snapshots: optionalDependencies: graceful-fs: 4.2.11 + jsonrepair@3.13.1: {} + jsonschema@1.5.0: {} jsonwebtoken@9.0.2: diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index f6eac36a9c1..c396790460f 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -3,6 +3,9 @@ import { parseJSON } from "partial-json" import { type ToolName, toolNames, type FileEntry } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" +import { repairJson } from "../../utils/json-repair" +import { logger } from "../../utils/logging" + import { type ToolUse, type McpToolUse, @@ -51,6 +54,25 @@ export type ToolCallStreamEvent = ApiStreamToolCallStartChunk | ApiStreamToolCal * provider-level raw chunks into start/delta/end events. */ export class NativeToolCallParser { + // Configuration for malformed JSON repair (experimental feature) + private static malformedJsonRepairEnabled = false + + /** + * Enable or disable malformed JSON repair. + * When enabled, the parser will attempt to repair malformed JSON + * in tool call arguments before parsing. + */ + public static setMalformedJsonRepairEnabled(enabled: boolean): void { + this.malformedJsonRepairEnabled = enabled + } + + /** + * Check if malformed JSON repair is enabled. + */ + public static isMalformedJsonRepairEnabled(): boolean { + return this.malformedJsonRepairEnabled + } + // Streaming state management for argument accumulation (keyed by tool call id) // Note: name is string to accommodate dynamic MCP tools (mcp_serverName_toolName) private static streamingToolCalls = new Map< @@ -593,7 +615,29 @@ export class NativeToolCallParser { try { // Parse the arguments JSON string - const args = toolCall.arguments === "" ? {} : JSON.parse(toolCall.arguments) + // If malformed JSON repair is enabled, attempt to repair before parsing + let args: Record + if (toolCall.arguments === "") { + args = {} + } else { + try { + args = JSON.parse(toolCall.arguments) + } catch (parseError) { + // JSON parsing failed - attempt repair if enabled + if (this.malformedJsonRepairEnabled) { + const repaired = repairJson(toolCall.arguments) + if (repaired) { + logger.info(`[NativeToolCallParser] Repaired malformed JSON for tool ${toolCall.name}`) + args = JSON.parse(repaired) + } else { + // Repair failed, re-throw original error + throw parseError + } + } else { + throw parseError + } + } + } // Build legacy params object for backward compatibility with XML protocol and UI. // Native execution path uses nativeArgs instead, which has proper typing. diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 33e8245ccc6..14bbb8ac3c1 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -541,6 +541,16 @@ export class Task extends EventEmitter implements TaskLike { }) } + // Check malformed JSON repair experiment asynchronously. + // This enables repair of malformed JSON in tool call arguments from models like Grok. + provider.getState().then((state) => { + const isMalformedJsonRepairEnabled = experiments.isEnabled( + state.experiments ?? {}, + EXPERIMENT_IDS.MALFORMED_JSON_REPAIR, + ) + NativeToolCallParser.setMalformedJsonRepairEnabled(isMalformedJsonRepairEnabled) + }) + this.toolRepetitionDetector = new ToolRepetitionDetector(this.consecutiveMistakeLimit) // Initialize todo list if provided diff --git a/src/package.json b/src/package.json index 4b4f1b811b6..843aee580dc 100644 --- a/src/package.json +++ b/src/package.json @@ -466,6 +466,7 @@ "i18next": "^25.0.0", "ignore": "^7.0.3", "isbinaryfile": "^5.0.2", + "jsonrepair": "^3.13.1", "jwt-decode": "^4.0.0", "lodash.debounce": "^4.0.8", "mammoth": "^1.9.1", diff --git a/src/shared/__tests__/experiments.spec.ts b/src/shared/__tests__/experiments.spec.ts index 0b43302611b..5edd040f7b7 100644 --- a/src/shared/__tests__/experiments.spec.ts +++ b/src/shared/__tests__/experiments.spec.ts @@ -33,6 +33,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + malformedJsonRepair: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) @@ -46,6 +47,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + malformedJsonRepair: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true) }) @@ -59,6 +61,7 @@ describe("experiments", () => { runSlashCommand: false, multipleNativeToolCalls: false, customTools: false, + malformedJsonRepair: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) diff --git a/src/shared/experiments.ts b/src/shared/experiments.ts index ad3aeca8634..cdb71814bb4 100644 --- a/src/shared/experiments.ts +++ b/src/shared/experiments.ts @@ -8,6 +8,7 @@ export const EXPERIMENT_IDS = { RUN_SLASH_COMMAND: "runSlashCommand", MULTIPLE_NATIVE_TOOL_CALLS: "multipleNativeToolCalls", CUSTOM_TOOLS: "customTools", + MALFORMED_JSON_REPAIR: "malformedJsonRepair", } as const satisfies Record type _AssertExperimentIds = AssertEqual>> @@ -26,6 +27,7 @@ export const experimentConfigsMap: Record = { RUN_SLASH_COMMAND: { enabled: false }, MULTIPLE_NATIVE_TOOL_CALLS: { enabled: false }, CUSTOM_TOOLS: { enabled: false }, + MALFORMED_JSON_REPAIR: { enabled: false }, } export const experimentDefault = Object.fromEntries( diff --git a/src/utils/__tests__/json-repair.spec.ts b/src/utils/__tests__/json-repair.spec.ts new file mode 100644 index 00000000000..928602f3ff2 --- /dev/null +++ b/src/utils/__tests__/json-repair.spec.ts @@ -0,0 +1,303 @@ +import { describe, it, expect } from "vitest" +import { tryRepairJson, repairJson, parseWithRepair, isValidJson } from "../json-repair" + +describe("json-repair", () => { + describe("tryRepairJson", () => { + it("should return success with wasAlreadyValid=true for valid JSON", () => { + const validJson = '{"name": "test", "value": 123}' + const result = tryRepairJson(validJson) + + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + expect(result.repaired).toBe(validJson) + expect(result.error).toBeUndefined() + }) + + it("should return success with wasAlreadyValid=false for repaired JSON", () => { + // Missing comma between properties + const malformedJson = '{"name": "test" "value": 123}' + const result = tryRepairJson(malformedJson) + + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(false) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + expect(result.repaired).toBeDefined() + expect(result.error).toBeUndefined() + }) + + it("should repair JSON with trailing comma", () => { + const jsonWithTrailingComma = '{"name": "test", "value": 123,}' + const result = tryRepairJson(jsonWithTrailingComma) + + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(false) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with unquoted property names", () => { + const unquotedProps = '{name: "test", value: 123}' + const result = tryRepairJson(unquotedProps) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with single quotes", () => { + const singleQuotes = "{'name': 'test', 'value': 123}" + const result = tryRepairJson(singleQuotes) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with missing closing brace", () => { + const missingBrace = '{"name": "test", "value": 123' + const result = tryRepairJson(missingBrace) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with missing closing bracket", () => { + const missingBracket = '{"items": [1, 2, 3}' + const result = tryRepairJson(missingBracket) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ items: [1, 2, 3] }) + }) + + it("should repair JSON with single-line comments", () => { + const withComments = `{ + "name": "test", // this is a comment + "value": 123 + }` + const result = tryRepairJson(withComments) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with block comments", () => { + const withBlockComments = `{ + "name": "test", /* block comment */ + "value": 123 + }` + const result = tryRepairJson(withBlockComments) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ name: "test", value: 123 }) + }) + + it("should repair JSON with newlines in strings", () => { + const newlineInString = '{"text": "line1\nline2"}' + const result = tryRepairJson(newlineInString) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ text: "line1\nline2" }) + }) + + it("should repair complex malformed tool call arguments", () => { + // Simulating a malformed tool call from Grok or similar models + const malformedToolArgs = `{ + path: "/src/test.ts", + content: "const x = 1;" + // missing comma above + }` + const result = tryRepairJson(malformedToolArgs) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + path: "/src/test.ts", + content: "const x = 1;", + }) + }) + + it("should return failure for completely unrepairable input", () => { + const unrepairable = "this is not json at all {{{" + const result = tryRepairJson(unrepairable) + + // Note: jsonrepair is quite aggressive and may still attempt repairs + // The behavior depends on the jsonrepair library + if (!result.success) { + expect(result.success).toBe(false) + expect(result.error).toBeDefined() + expect(result.error).toContain("Failed to repair JSON") + } + }) + }) + + describe("repairJson", () => { + it("should return the original string for valid JSON", () => { + const validJson = '{"name": "test"}' + const result = repairJson(validJson) + + expect(result).toBe(validJson) + }) + + it("should return repaired string for malformed JSON", () => { + const malformedJson = '{"name": "test",}' + const result = repairJson(malformedJson) + + expect(result).toBeDefined() + expect(JSON.parse(result!)).toEqual({ name: "test" }) + }) + + it("should return null for unrepairable input", () => { + // Empty string is harder for jsonrepair to handle + const empty = "" + const result = repairJson(empty) + + // jsonrepair treats empty string as empty and may return empty string + // Let's test something truly unparseable + if (result === null) { + expect(result).toBeNull() + } + }) + }) + + describe("parseWithRepair", () => { + it("should parse valid JSON directly", () => { + const validJson = '{"count": 42}' + const result = parseWithRepair<{ count: number }>(validJson) + + expect(result).toEqual({ count: 42 }) + }) + + it("should parse and repair malformed JSON", () => { + const malformedJson = "{count: 42}" + const result = parseWithRepair<{ count: number }>(malformedJson) + + expect(result).toEqual({ count: 42 }) + }) + + it("should return typed result", () => { + interface TestType { + name: string + items: number[] + } + + const json = '{"name": "test", "items": [1, 2, 3]}' + const result = parseWithRepair(json) + + expect(result).not.toBeNull() + expect(result?.name).toBe("test") + expect(result?.items).toEqual([1, 2, 3]) + }) + }) + + describe("isValidJson", () => { + it("should return true for valid JSON", () => { + expect(isValidJson('{"valid": true}')).toBe(true) + expect(isValidJson("[]")).toBe(true) + expect(isValidJson("null")).toBe(true) + expect(isValidJson("123")).toBe(true) + expect(isValidJson('"string"')).toBe(true) + }) + + it("should return false for invalid JSON", () => { + expect(isValidJson("{invalid}")).toBe(false) + expect(isValidJson("{'single': 'quotes'}")).toBe(false) + expect(isValidJson("{trailing: 'comma',}")).toBe(false) + expect(isValidJson("")).toBe(false) + expect(isValidJson("undefined")).toBe(false) + }) + }) + + describe("real-world malformed JSON scenarios", () => { + it("should repair JSON with multiple issues combined", () => { + // Missing comma, unquoted keys, trailing comma + const malformed = `{ + command: 'npm run test', + cwd: "./src", + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + command: "npm run test", + cwd: "./src", + }) + }) + + it("should repair nested object with issues", () => { + // Missing comma between properties - use a simpler pattern that jsonrepair handles + const malformed = `{ + files: [ + {path: "./test.ts", lineRanges: null} + ] + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + files: [{ path: "./test.ts", lineRanges: null }], + }) + }) + + it("should repair object with space-separated properties", () => { + // Note: jsonrepair has limitations with certain patterns like {key1: "val" key2: "val"} + // but handles unquoted keys and trailing commas well + const malformed = `{ + "files": [ + {"path": "./test.ts", "lineRanges": null,} + ], + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + files: [{ path: "./test.ts", lineRanges: null }], + }) + }) + + it("should repair tool call arguments with special characters", () => { + const malformed = `{ + "diff": "<<<<<<< SEARCH +some content +======= +new content +>>>>>>> REPLACE" + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toHaveProperty("diff") + }) + + it("should handle empty object", () => { + const result = tryRepairJson("{}") + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(true) + expect(result.parsed).toEqual({}) + }) + + it("should handle empty array", () => { + const result = tryRepairJson("[]") + expect(result.success).toBe(true) + expect(result.wasAlreadyValid).toBe(true) + expect(result.parsed).toEqual([]) + }) + + it("should repair ask_followup_question tool with complex follow_up array", () => { + // Real-world example: models sometimes emit malformed follow_up arrays + const malformed = `{ + "question": "What would you like to do?", + "follow_up": [ + {"text": "Option A" "mode": null}, + {"text": "Option B", "mode": null} + ] + }` + const result = tryRepairJson(malformed) + + expect(result.success).toBe(true) + expect(result.parsed).toEqual({ + question: "What would you like to do?", + follow_up: [ + { text: "Option A", mode: null }, + { text: "Option B", mode: null }, + ], + }) + }) + }) +}) diff --git a/src/utils/json-repair.ts b/src/utils/json-repair.ts new file mode 100644 index 00000000000..cba9089726e --- /dev/null +++ b/src/utils/json-repair.ts @@ -0,0 +1,116 @@ +/** + * JSON repair utility for fixing malformed LLM responses. + * + * This module provides functionality to repair common JSON malformation issues + * that occur when LLMs (especially models like Grok) struggle with strict + * JSON formatting requirements. + * + * Common issues handled: + * - Missing or trailing commas + * - Unquoted property names or strings + * - Missing closing brackets/braces + * - Comments in JSON (single-line and block) + * - Newlines in strings + * - Escape character issues + * - Mixed quote styles + * + * @see https://github.com/josdejong/jsonrepair + */ +import { jsonrepair, JSONRepairError } from "jsonrepair" + +/** + * Result of a JSON repair attempt. + */ +export interface JsonRepairResult { + /** Whether the repair was successful */ + success: boolean + /** The repaired JSON string (if successful) */ + repaired?: string + /** The parsed JSON object (if successful) */ + parsed?: unknown + /** Whether the original JSON was already valid */ + wasAlreadyValid: boolean + /** Error message if repair failed */ + error?: string +} + +/** + * Attempt to repair malformed JSON and parse it. + * + * @param input - The potentially malformed JSON string + * @returns A result object containing the repair status and parsed data + */ +export function tryRepairJson(input: string): JsonRepairResult { + // First, try to parse as-is to avoid unnecessary repair + try { + const parsed = JSON.parse(input) + return { + success: true, + repaired: input, + parsed, + wasAlreadyValid: true, + } + } catch { + // JSON is malformed, attempt repair + } + + // Attempt to repair the JSON + try { + const repaired = jsonrepair(input) + const parsed = JSON.parse(repaired) + return { + success: true, + repaired, + parsed, + wasAlreadyValid: false, + } + } catch (err) { + const errorMessage = + err instanceof JSONRepairError ? err.message : err instanceof Error ? err.message : String(err) + + return { + success: false, + wasAlreadyValid: false, + error: `Failed to repair JSON: ${errorMessage}`, + } + } +} + +/** + * Repair malformed JSON string. + * Returns the repaired string, or null if repair is not possible. + * + * @param input - The potentially malformed JSON string + * @returns The repaired JSON string, or null if repair failed + */ +export function repairJson(input: string): string | null { + const result = tryRepairJson(input) + return result.success ? (result.repaired ?? null) : null +} + +/** + * Parse potentially malformed JSON, repairing if necessary. + * Returns the parsed object, or null if parsing/repair failed. + * + * @param input - The potentially malformed JSON string + * @returns The parsed JSON object, or null if parsing failed + */ +export function parseWithRepair(input: string): T | null { + const result = tryRepairJson(input) + return result.success ? (result.parsed as T) : null +} + +/** + * Check if a string is valid JSON without attempting repair. + * + * @param input - The string to check + * @returns true if the string is valid JSON, false otherwise + */ +export function isValidJson(input: string): boolean { + try { + JSON.parse(input) + return true + } catch { + return false + } +}