Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f419205
feat(error-interception): add deterministic error interception middle…
k1yt Jul 24, 2026
f5bb527
fix(error-interception): address CodeRabbit review findings
k1yt Jul 24, 2026
6bd6ec2
fix: update e2e fixture and add coverage tests for Codecov
k1yt Jul 24, 2026
7d45ce1
test: add 3 targeted coverage tests for 80% Codecov threshold
k1yt Jul 24, 2026
4e29301
test: add 13 targeted tests for 80%+ Codecov patch coverage
k1yt Jul 24, 2026
37b9b1c
feat(error-interception): add INVALID_JSON_ARGUMENTS pattern for conc…
k1yt Jul 25, 2026
0271915
fix(error-interception): add logging to silent error paths
Jul 25, 2026
5b800dc
feat(error-interception): improve AI guidance quality for 4 patterns
Jul 25, 2026
f81d1fb
fix(error-interception): show errors to user in UI alongside AI guidance
Jul 26, 2026
9d3e65d
feat(error-interception): user-friendly error UI with structured deta…
Jul 26, 2026
d525554
fix(error-interception): add non-null assertion in test to satisfy TS…
Jul 26, 2026
3f5497e
fix(error-interception): update stale test assertion for unknown tool…
Jul 26, 2026
a10a145
fix(error-interception): rebase onto upstream/main and fix eslint no-…
Jul 26, 2026
3d9964e
fix(error-interception): address PR review findings and improve guidance
Jul 26, 2026
fefbe54
fix: resolve CI lint and test failures for PR #1009
Jul 27, 2026
321da70
fix(e2e): update apply-diff fixture to match <error_details> format +…
Jul 27, 2026
cc4008d
fix: correct PushToolResult type in integration test
Jul 27, 2026
5c8c495
docs: add flaky-test note for interrupted-child E2E
Jul 27, 2026
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 @@ -688,6 +688,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