Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
9 changes: 9 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,12 @@ qdrant_storage/
plans/

roo-cli-*.tar.gz*

# Local dev scripts (not for CI)
/ci-fix-commit.ps1
/commit-and-push.ps1
/commit-message.txt
/resolve_conflicts.py

# AI session artifacts (timestamped session directories)
/docs/*_session_*/
2 changes: 1 addition & 1 deletion apps/vscode-e2e/src/fixtures/apply-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function addApplyDiffResultFixtures(mock: InstanceType<typeof LLMock>) {
},
{
toolCallId: "call_apply_diff_error_001",
expected: ["No sufficiently similar match found at line: 1", "This content does not exist"],
expected: ["Category: DIFF_MATCH_FAILED", "Pattern: EI/DIFF_MATCH_FAILED/001"],
result: "The apply_diff operation on `apply-diff-tool-fixture/error-handling.txt` was rejected - the search content did not match any content in the file, so it was not modified.",
id: "call_apply_diff_error_002",
},
Expand Down
3 changes: 3 additions & 0 deletions apps/vscode-e2e/src/suite/subtasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,9 @@ suite("Roo Code Subtasks", function () {
}
})

// NOTE: This test exercises parent-child resume via mock fixtures with overlapping
// predicates. If it flakes on CI, the fixture chain may need tighter matching.
// See debug-systemic report 190600 for root-cause analysis.
// Issue #560: interrupted child resumes and reports back to parent.
// Before the fix, cancelTask() severed the parent link, so the resumed child
// fell through to "Start New Task" instead of delegating back to the parent.
Expand Down
219 changes: 211 additions & 8 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,32 @@ type NativeArgsFor<TName extends ToolName> = TName extends keyof NativeToolArgs
*/
export type ToolCallStreamEvent = ApiStreamToolCallStartChunk | ApiStreamToolCallDeltaChunk | ApiStreamToolCallEndChunk

/**
* Discriminated union for parser failure kinds.
*
* - `json_syntax`: The arguments string could not be parsed as JSON.
* - `missing_required_arguments`: The JSON was valid but one or more required
* fields were absent (including the empty-object case).
* - `invalid_argument_shape`: The JSON was valid and required field names were
* present, but the structural shape did not match the tool schema (e.g. a
* field had the wrong type or the value could not be coerced).
*/
export type ParserFailureKind = "json_syntax" | "missing_required_arguments" | "invalid_argument_shape"

/**
* Typed, sanitized descriptor for a parser failure.
*
* IMPORTANT: This descriptor MUST NOT contain raw argument bodies, file paths,
* commands, task IDs, or secrets. It carries only structural facts needed for
* error classification and model guidance.
*/
export interface NativeToolParseFailure {
kind: ParserFailureKind
toolName?: string
missingParameters?: string[] // Known missing required field names from parser's tool contract
emptyArguments?: boolean // true if the input was {} or ""
}

/**
* Parser for native tool calls (OpenAI-style function calling).
* Converts native tool call format to ToolUse format for compatibility
Expand Down Expand Up @@ -73,6 +99,99 @@ export class NativeToolCallParser {
}
>()

/**
* Stores JSON.parse error messages keyed by tool call ID.
* When parseToolCall() catches a JSON.parse failure, it records the error
* here so presentAssistantMessage can retrieve it and route the signal to
* the INVALID_JSON_ARGUMENTS error-interception pattern instead of the
* generic PARAM_MISSING path.
*
* @deprecated Use {@link parseFailures} and {@link consumeParseFailure} for
* typed failure descriptors. This legacy string map is retained only as a
* compatibility wrapper for human diagnostics.
*/
private static parseErrors = new Map<string, string>()

/**
* Stores typed parser failure descriptors keyed by tool call ID.
* When parseToolCall() catches any failure (JSON syntax, missing required
* arguments, or invalid argument shape), it records a typed descriptor here
* so downstream consumers can classify the failure precisely instead of
* relying on raw error strings.
*/
private static parseFailures = new Map<string, NativeToolParseFailure>()

/**
* Required parameter names for each native tool, derived from
* {@link NativeToolArgs}. Used to classify missing-required-arguments
* failures with precise field names.
*/
private static readonly REQUIRED_PARAMETERS: Record<string, string[]> = {
access_mcp_resource: ["server_name", "uri"],
read_file: ["path"],
read_command_output: ["artifact_id"],
attempt_completion: ["result"],
execute_command: ["command"],
apply_diff: ["path", "diff"],
edit: ["file_path", "old_string", "new_string"],
search_and_replace: ["file_path", "old_string", "new_string"],
search_replace: ["file_path", "old_string", "new_string"],
edit_file: ["file_path", "old_string", "new_string"],
apply_patch: ["patch"],
list_files: ["path"],
new_task: ["mode", "message"],
ask_followup_question: ["question", "follow_up"],
codebase_search: ["query"],
generate_image: ["prompt", "path"],
run_slash_command: ["command"],
skill: ["skill"],
search_files: ["path", "regex"],
switch_mode: ["mode_slug", "reason"],
update_todo_list: ["todos"],
use_mcp_tool: ["server_name", "tool_name"],
write_to_file: ["path", "content"],
}

/**
* Retrieve and remove the typed parse failure descriptor for a given tool
* call ID. Returns undefined if no failure was recorded or if it was
* already consumed.
*
* Atomic consume-and-delete, matching the lifecycle of the legacy
* {@link consumeParseError} string side channel.
*/
public static consumeParseFailure(toolCallId: string): NativeToolParseFailure | undefined {
const failure = NativeToolCallParser.parseFailures.get(toolCallId)
if (failure !== undefined) {
NativeToolCallParser.parseFailures.delete(toolCallId)
}
return failure
}

/**
* Retrieve and remove the parse error for a given tool call ID.
* Returns undefined if no parse error was recorded.
*
* @deprecated Compatibility wrapper. New production code should use
* {@link consumeParseFailure} for typed failure descriptors. This method
* returns the string representation for human diagnostics only.
*/
public static consumeParseError(toolCallId: string): string | undefined {
const error = NativeToolCallParser.parseErrors.get(toolCallId)
if (error !== undefined) {
NativeToolCallParser.parseErrors.delete(toolCallId)
}
return error
}

/**
* Check whether a parse error was recorded for a given tool call ID
* without consuming it.
*/
public static hasParseError(toolCallId: string): boolean {
return NativeToolCallParser.parseErrors.has(toolCallId)
}

private static coerceOptionalBoolean(value: unknown): boolean | undefined {
if (typeof value === "boolean") {
return value
Expand Down Expand Up @@ -1003,11 +1122,43 @@ export class NativeToolCallParser {
// Native-only: core tools must always have typed nativeArgs.
// If we couldn't construct it, the model produced an invalid tool call payload.
if (!nativeArgs && !customToolRegistry.has(resolvedName)) {
throw new Error(
`[NativeToolCallParser] Invalid arguments for tool '${resolvedName}'. ` +
`Native tool calls require a valid JSON payload matching the tool schema. ` +
`Received: ${JSON.stringify(args)}`,
)
// Classify the failure precisely so the catch block can store a
// typed descriptor instead of a raw error string.
//
// If args is not a plain object (e.g. a primitive, array, or null),
// the structural shape is fundamentally wrong.
const isPlainObject = typeof args === "object" && args !== null && !Array.isArray(args)

if (!isPlainObject) {
throw {
__parserFailureKind: "invalid_argument_shape" as const,
toolName: resolvedName as string,
missingParameters: [],
emptyArguments: false,
}
}

const required = NativeToolCallParser.REQUIRED_PARAMETERS[resolvedName as string] ?? []
const missing = required.filter((p) => args[p] === undefined)
const isEmpty = Object.keys(args).length === 0

if (missing.length > 0) {
throw {
__parserFailureKind: "missing_required_arguments" as const,
toolName: resolvedName as string,
missingParameters: missing,
emptyArguments: isEmpty,
}
}

// Required fields are present but the structural shape didn't match
// any known pattern in the switch above.
throw {
__parserFailureKind: "invalid_argument_shape" as const,
toolName: resolvedName as string,
missingParameters: [],
emptyArguments: isEmpty,
}
}

const result: ToolUse<TName> = {
Expand All @@ -1030,15 +1181,67 @@ export class NativeToolCallParser {

return result
} catch (error) {
console.error(
`Failed to parse tool call arguments: ${error instanceof Error ? error.message : String(error)}`,
)
// Determine whether this is a JSON.parse syntax failure or a
// post-parse structural failure (missing required arguments or
// invalid argument shape). The structural failures are thrown as
// tagged objects with __parserFailureKind; JSON.parse failures are
// standard SyntaxError instances.
const failure = NativeToolCallParser.classifyParseFailure(error, resolvedName as string)

const errorMessage = error instanceof Error ? error.message : String(error)

console.error(`Failed to parse tool call arguments: ${errorMessage}`)

console.error(`Tool call: ${JSON.stringify(toolCall, null, 2)}`)

// Store the legacy string error for backward compatibility with
// existing callers of consumeParseError().
NativeToolCallParser.parseErrors.set(toolCall.id, errorMessage)

// Store the typed failure descriptor for new callers that use
// consumeParseFailure().
NativeToolCallParser.parseFailures.set(toolCall.id, failure)

return null
}
}

/**
* Classify a caught error from parseToolCall() into a typed
* {@link NativeToolParseFailure} descriptor.
*
* - If the error is a tagged object with `__parserFailureKind`, it was
* thrown by the structural validation logic and carries precise metadata.
* - Otherwise, the error originated from JSON.parse (a SyntaxError) and is
* classified as `json_syntax`.
*/
private static classifyParseFailure(error: unknown, toolName: string): NativeToolParseFailure {
// Check for tagged structural failure objects thrown by the validation
// logic above. These are not Error instances — they are plain objects
// with a __parserFailureKind discriminator.
if (typeof error === "object" && error !== null && "__parserFailureKind" in error) {
const tagged = error as {
__parserFailureKind: ParserFailureKind
toolName?: string
missingParameters?: string[]
emptyArguments?: boolean
}
return {
kind: tagged.__parserFailureKind,
toolName: tagged.toolName ?? toolName,
missingParameters: tagged.missingParameters,
emptyArguments: tagged.emptyArguments,
}
}

// Any other error (SyntaxError from JSON.parse, or unexpected runtime
// error) is classified as a JSON syntax failure.
return {
kind: "json_syntax",
toolName,
}
}

/**
* Parse dynamic MCP tools (named mcp--serverName--toolName).
* These are generated dynamically by getMcpServerTools() and are returned
Expand Down
Loading
Loading