Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/types/src/experiment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const experimentIds = [
"runSlashCommand",
"multipleNativeToolCalls",
"customTools",
"llmResponseRepair",
] as const

export const experimentIdsSchema = z.enum(experimentIds)
Expand All @@ -32,6 +33,7 @@ export const experimentsSchema = z.object({
runSlashCommand: z.boolean().optional(),
multipleNativeToolCalls: z.boolean().optional(),
customTools: z.boolean().optional(),
llmResponseRepair: z.boolean().optional(),
})

export type Experiments = z.infer<typeof experimentsSchema>
Expand Down
81 changes: 79 additions & 2 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
ApiStreamToolCallEndChunk,
} from "../../api/transform/stream"
import { MCP_TOOL_PREFIX, MCP_TOOL_SEPARATOR, parseMcpToolName } from "../../utils/mcp-name"
import { repairToolCallJson } from "../../utils/repair-json"

/**
* Helper type to extract properly typed native arguments for a given tool.
Expand Down Expand Up @@ -51,6 +52,28 @@ export type ToolCallStreamEvent = ApiStreamToolCallStartChunk | ApiStreamToolCal
* provider-level raw chunks into start/delta/end events.
*/
export class NativeToolCallParser {
/**
* Configuration for LLM response repair feature.
* When enabled, attempts to repair malformed JSON from LLM responses.
* @see Issue #10481
*/
private static repairEnabled = false

/**
* Enable or disable the LLM response repair feature.
* This should be called when the experimental setting changes.
*/
public static setRepairEnabled(enabled: boolean): void {
this.repairEnabled = enabled
}

/**
* Check if the repair feature is enabled.
*/
public static isRepairEnabled(): boolean {
return this.repairEnabled
}
Comment on lines +55 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setRepairEnabled method is defined but never called anywhere in the codebase. This means the experimental setting llmResponseRepair will have no effect - even if a user enables it, repairEnabled will always remain false. Other experiments in the codebase use a different pattern: they check experiments.isEnabled(state?.experiments ?? {}, EXPERIMENT_IDS.XXX) at runtime when the feature is needed. Either wire up setRepairEnabled() to be called when the experimental setting changes (e.g., in the extension activation or state update path), or refactor to check the experiment state dynamically at runtime like other experiments do.

Fix it with Roo Code or mention @roomote and request a fix.


// 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<
Expand Down Expand Up @@ -593,7 +616,39 @@ export class NativeToolCallParser {

try {
// Parse the arguments JSON string
const args = toolCall.arguments === "" ? {} : JSON.parse(toolCall.arguments)
let args: Record<string, any>
let wasRepaired = false

if (toolCall.arguments === "") {
args = {}
} else {
try {
args = JSON.parse(toolCall.arguments)
} catch (parseError) {
// If repair is enabled, attempt to fix malformed JSON
if (this.repairEnabled) {
const repairResult = repairToolCallJson(toolCall.arguments)
if (repairResult.parsed !== undefined) {
args = repairResult.parsed
wasRepaired = true
console.log(
`[NativeToolCallParser] Successfully repaired malformed JSON for tool '${resolvedName}'`,
)
} else {
// Repair failed, re-throw original error
throw parseError
}
} else {
// Repair not enabled, re-throw original error
throw parseError
}
}
}

// Log repair for debugging (can be removed in production)
if (wasRepaired) {
console.log(`[NativeToolCallParser] Original JSON: ${toolCall.arguments.substring(0, 200)}...`)
}

// Build legacy params object for backward compatibility with XML protocol and UI.
// Native execution path uses nativeArgs instead, which has proper typing.
Expand Down Expand Up @@ -863,7 +918,29 @@ export class NativeToolCallParser {
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 || "{}")
let args: Record<string, any>
const argsString = toolCall.arguments || "{}"

try {
args = JSON.parse(argsString)
} catch (parseError) {
// If repair is enabled, attempt to fix malformed JSON
if (this.repairEnabled) {
const repairResult = repairToolCallJson(argsString)
if (repairResult.parsed !== undefined) {
args = repairResult.parsed
console.log(
`[NativeToolCallParser] Successfully repaired malformed JSON for MCP tool '${toolCall.name}'`,
)
} else {
// Repair failed, re-throw original error
throw parseError
}
} else {
// Repair not enabled, re-throw original error
throw parseError
}
}

// Extract server_name and tool_name from the tool name itself
// Format: mcp--serverName--toolName (using -- separator)
Expand Down
3 changes: 3 additions & 0 deletions src/shared/__tests__/experiments.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe("experiments", () => {
runSlashCommand: false,
multipleNativeToolCalls: false,
customTools: false,
llmResponseRepair: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand All @@ -46,6 +47,7 @@ describe("experiments", () => {
runSlashCommand: false,
multipleNativeToolCalls: false,
customTools: false,
llmResponseRepair: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
})
Expand All @@ -59,6 +61,7 @@ describe("experiments", () => {
runSlashCommand: false,
multipleNativeToolCalls: false,
customTools: false,
llmResponseRepair: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand Down
2 changes: 2 additions & 0 deletions src/shared/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const EXPERIMENT_IDS = {
RUN_SLASH_COMMAND: "runSlashCommand",
MULTIPLE_NATIVE_TOOL_CALLS: "multipleNativeToolCalls",
CUSTOM_TOOLS: "customTools",
LLM_RESPONSE_REPAIR: "llmResponseRepair",
} as const satisfies Record<string, ExperimentId>

type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
Expand All @@ -26,6 +27,7 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
RUN_SLASH_COMMAND: { enabled: false },
MULTIPLE_NATIVE_TOOL_CALLS: { enabled: false },
CUSTOM_TOOLS: { enabled: false },
LLM_RESPONSE_REPAIR: { enabled: false },
}

export const experimentDefault = Object.fromEntries(
Expand Down
Loading
Loading