mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-08 03:07:53 +00:00
Use a much more lenient JSON parsing strategy for tools
This commit is contained in:
parent
873a763ea7
commit
245e6bc88d
3 changed files with 292 additions and 6 deletions
|
|
@ -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<Record<string, unknown>>(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<Record<ToolParamName, string>> = {}
|
||||
|
|
@ -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<Record<string, unknown>>(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("_")
|
||||
|
|
|
|||
193
src/utils/__tests__/safeParseJson.spec.ts
Normal file
193
src/utils/__tests__/safeParseJson.spec.ts
Normal file
|
|
@ -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<string[]>(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<string[]>(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<TestType>(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)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
81
src/utils/safeParseJson.ts
Normal file
81
src/utils/safeParseJson.ts
Normal file
|
|
@ -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<T> =
|
||||
| {
|
||||
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<T = unknown>(rawJson: string): SafeParseResult<T> {
|
||||
// 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,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue