From 21e93c027ee11d515e4bfaa3088d12cf6145ae46 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Wed, 29 Jul 2026 08:25:27 +0900 Subject: [PATCH 1/8] feat(error): add structured error presentation in assistant messages --- .../presentAssistantMessage.ts | 111 ++++++++++++++++-- 1 file changed, 104 insertions(+), 7 deletions(-) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index f71b5cc1bd..3e3ec7deb2 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -14,6 +14,73 @@ import type { ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../sha import { AskIgnoredError } from "../task/AskIgnoredError" import { Task } from "../task/Task" +/** + * Structured error presentation for LLM-guided error recovery. + * Provides WHAT/WHY/NEXT format wrapped in XML tags. + */ +function formatStructuredError( + details: { + what: string + why: string + next: string[] + retryable?: boolean + pattern?: string + occurrence?: number + disposition?: string + }, + byteLimit: number = 8000, +): string { + const version = "1.0" + const status = "error" + const category = details.pattern ? (details.pattern.split("/")[1] ?? "unknown") : "unknown" + const type = details.pattern + ? details.pattern.startsWith("EI/") + ? details.pattern.replace("EI/", "guided_").toLowerCase().replace(/_/g, "_") + : details.pattern.toLowerCase().replace(/_/g, "_") + : "unclassified_error" + const what = details.what + const why = details.why + const next = details.next ?? [] + const retryable = details.retryable ?? true + const occurrence = Math.max(1, details.occurrence ?? 1) + const patternId = details.pattern ?? "UNCLASSIFIED/000/000" + const recoveryDisposition = details.disposition ?? "correct_once" + + const payload = { + version, + status, + type, + category, + what, + why, + next, + retryable, + occurrence, + pattern_id: patternId, + recovery_disposition: recoveryDisposition, + } + + let json = JSON.stringify(payload, null, 2) + + if (json.length > byteLimit && next.length > 0) { + // Trim Next items to fit within byte limit, preserving the first one + const firstItem = next[0] + next.length = 1 + const trimmed = { + ...payload, + next: [firstItem], + } + json = JSON.stringify(trimmed, null, 2) + } + + if (json.length > byteLimit) { + // Last resort: truncate the JSON string + json = json.substring(0, byteLimit - 3) + "..." + } + + return `\n${json}\n` +} + import { listFilesTool } from "../tools/ListFilesTool" import { readFileTool } from "../tools/ReadFileTool" import { readCommandOutputTool } from "../tools/ReadCommandOutputTool" @@ -225,12 +292,28 @@ export async function presentAssistantMessage(cline: Task) { if (error instanceof AskIgnoredError) { return } - const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}` + + // Structured error presentation with WHAT/WHY/NEXT format + const serializedError = serializeError(error) + const structuredErrorContent = formatStructuredError({ + what: `An error occurred during ${action}.`, + why: error.message || serializedError.message || "An unexpected error occurred.", + next: [ + `Retry the ${action} operation with corrected parameters if applicable.`, + `If the error persists, report this issue to the development team with the error details below.`, + ], + pattern: "TOOL_EXECUTION/ERROR_EXECUTION/001", + retryable: true, + occurrence: 1, + disposition: "correct_once", + }) + + pushToolResult(structuredErrorContent) + await cline.say( "error", - `Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`, + `[${action}] Error during execution:\n${error.message ?? JSON.stringify(serializedError, null, 2)}\n\n${structuredErrorContent}`, ) - pushToolResult(formatResponse.toolError(errorString)) } if (!mcpBlock.partial) { @@ -543,14 +626,28 @@ export async function presentAssistantMessage(cline: Task) { if (error instanceof AskIgnoredError) { return } - const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}` + + // Structured error presentation with WHAT/WHY/NEXT format + const serializedError = serializeError(error) + const structuredErrorContent = formatStructuredError({ + what: `An error occurred during ${action}.`, + why: error.message || serializedError.message || "An unexpected error occurred.", + next: [ + `Review the error details and retry the ${action} operation with corrected parameters.`, + `If the error persists, report this issue to the development team.`, + ], + pattern: "TOOL_EXECUTION/ERROR_EXECUTION/002", + retryable: true, + occurrence: 1, + disposition: "correct_once", + }) + + pushToolResult(structuredErrorContent) await cline.say( "error", - `Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`, + `[${action}] Error during execution:\n${error.message ?? JSON.stringify(serializedError, null, 2)}\n\n${structuredErrorContent}`, ) - - pushToolResult(formatResponse.toolError(errorString)) } if (!block.partial) { From e3ad841f14097ccacff7148bdd65ba250265a967 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 3 Aug 2026 17:55:54 +0900 Subject: [PATCH 2/8] fix(error-interception): add null guard to getTaskState to prevent WeakMap crash --- .../tools/error-interception/ToolErrorInterceptor.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/core/tools/error-interception/ToolErrorInterceptor.ts b/src/core/tools/error-interception/ToolErrorInterceptor.ts index 9bab6d94c9..c61d6386dc 100644 --- a/src/core/tools/error-interception/ToolErrorInterceptor.ts +++ b/src/core/tools/error-interception/ToolErrorInterceptor.ts @@ -89,8 +89,18 @@ export class ToolErrorInterceptor { /** * Creates or returns existing per-task state. Uses a WeakMap keyed by the * Task object so state is discarded when the task is garbage collected. + * + * When `task` is null or undefined (invalid WeakMap key), returns an + * ephemeral default state to satisfy the fail-open philosophy rather than + * throwing TypeError from WeakMap.set(). */ public getTaskState(task: object): InterceptorTaskState { + // WeakMap keys must be objects; null/undefined are invalid and would + // throw TypeError on .set(). Fail-open: return an ephemeral default + // state so callers can proceed without crashing. + if (!task) { + return { categoryCounts: new Map(), shellCircuitOpen: false } + } let taskState = this.state.perTask.get(task) if (!taskState) { taskState = { categoryCounts: new Map(), shellCircuitOpen: false } From 7d7b5a848d53da12eef33add51113b20b3a78875 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Tue, 4 Aug 2026 03:28:07 +0900 Subject: [PATCH 3/8] fix(error-interception): honest retry guidance, occurrence tracking, and is_error in tool error results formatStructuredError hardcoded retryable/occurrence/disposition for every error, telling the model that terminal failures (e.g. TERMINAL/PROVIDER_SWITCH/003) were first-occurrence retryable errors and encouraging retry loops. - derive retryability from the error: terminal/shell/provider-switch machine codes, validation errors, and user rejections are non-retryable - count per-task occurrences of identical failures via TaskErrorState and escalate disposition to change_strategy at the stuck-loop threshold (await_user for user rejections) - mark handleError tool_results with is_error, matching the sibling error paths (validation, rejection, missing nativeArgs, unknown tool) - emit slash-free dotted type strings and keep JSON valid under byte-limit pressure instead of truncating mid-document - show a concise human message in say("error") instead of the full JSON blob; the structured payload stays in the tool result only --- ...resentAssistantMessage-handleError.spec.ts | 194 ++++++++++++++ .../__tests__/structuredError.spec.ts | 237 ++++++++++++++++++ .../presentAssistantMessage.ts | 134 ++-------- src/core/assistant-message/structuredError.ts | 210 ++++++++++++++++ 4 files changed, 668 insertions(+), 107 deletions(-) create mode 100644 src/core/assistant-message/__tests__/presentAssistantMessage-handleError.spec.ts create mode 100644 src/core/assistant-message/__tests__/structuredError.spec.ts create mode 100644 src/core/assistant-message/structuredError.ts diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-handleError.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-handleError.spec.ts new file mode 100644 index 0000000000..54f98e0610 --- /dev/null +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-handleError.spec.ts @@ -0,0 +1,194 @@ +// npx vitest src/core/assistant-message/__tests__/presentAssistantMessage-handleError.spec.ts + +import { describe, it, expect, beforeEach, vi } from "vitest" +import type { Task } from "../../task/Task" +import { presentAssistantMessage } from "../presentAssistantMessage" + +// The error the mocked execute_command tool fails with; reset per test. +let mockError: Error + +// Mock dependencies +vi.mock("../../task/Task") +vi.mock("../../tools/validateToolUse", () => ({ + validateToolUse: vi.fn(), + isValidToolName: vi.fn(() => true), +})) +vi.mock("../../tools/ExecuteCommandTool", () => ({ + executeCommandTool: { + handle: vi.fn( + async ( + _task: unknown, + _block: unknown, + callbacks: { handleError: (action: string, error: Error) => Promise }, + ) => { + await callbacks.handleError("executing command", mockError) + }, + ), + }, +})) +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureToolUsage: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + }, + }, +})) + +interface MockTask { + taskId: string + instanceId: string + abort: boolean + presentAssistantMessageLocked: boolean + presentAssistantMessageHasPendingUpdates: boolean + currentStreamingContentIndex: number + assistantMessageContent: unknown[] + userMessageContent: Array> + userMessageContentReady: boolean + didCompleteReadingStream: boolean + didRejectTool: boolean + didAlreadyUseTool: boolean + consecutiveMistakeCount: number + clineMessages: unknown[] + api: { getModel: () => { id: string; info: Record } } + recordToolUsage: ReturnType + recordToolError: ReturnType + toolRepetitionDetector: { check: ReturnType } + providerRef: { deref: () => { getState: () => Promise<{ mode: string; customModes: never[] }> } } + say: ReturnType + ask: ReturnType + pushToolResultToUserContent: (toolResult: Record) => boolean +} + +function createMockTask(): MockTask { + const mockTask: MockTask = { + taskId: "test-task-id", + instanceId: "test-instance", + abort: false, + presentAssistantMessageLocked: false, + presentAssistantMessageHasPendingUpdates: false, + currentStreamingContentIndex: 0, + assistantMessageContent: [], + userMessageContent: [], + userMessageContentReady: false, + didCompleteReadingStream: true, + didRejectTool: false, + didAlreadyUseTool: false, + consecutiveMistakeCount: 0, + clineMessages: [], + api: { + getModel: () => ({ id: "test-model", info: {} }), + }, + recordToolUsage: vi.fn(), + recordToolError: vi.fn(), + toolRepetitionDetector: { + check: vi.fn().mockReturnValue({ allowExecution: true }), + }, + providerRef: { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + }), + }), + }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), + pushToolResultToUserContent: (toolResult) => { + const existingResult = mockTask.userMessageContent.find( + (block) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + return false + } + mockTask.userMessageContent.push(toolResult) + return true + }, + } + return mockTask +} + +function executeCommandBlock(toolCallId: string) { + return { + type: "tool_use", + id: toolCallId, + name: "execute_command", + params: { command: "ls" }, + nativeArgs: { command: "ls" }, + partial: false, + } +} + +function findToolResult(mockTask: MockTask, toolCallId: string): Record { + const toolResult = mockTask.userMessageContent.find( + (item) => item.type === "tool_result" && item.tool_use_id === toolCallId, + ) + if (!toolResult) { + throw new Error(`expected a tool_result for ${toolCallId}`) + } + return toolResult +} + +describe("presentAssistantMessage - tool handleError structured error", () => { + let mockTask: MockTask + + beforeEach(() => { + mockTask = createMockTask() + mockError = new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed") + }) + + it("marks the error tool_result with is_error and honest non-retryable guidance", async () => { + const toolCallId = "tool_call_err_1" + mockTask.assistantMessageContent = [executeCommandBlock(toolCallId)] + + // The cast is required because the mock only implements the subset of + // Task that presentAssistantMessage touches. + await presentAssistantMessage(mockTask as unknown as Task) + + const toolResult = findToolResult(mockTask, toolCallId) + expect(toolResult.is_error).toBe(true) + + const content = String(toolResult.content) + expect(content).toContain("") + expect(content).toContain('"retryable": false') + expect(content).toContain('"occurrence": 1') + expect(content).toContain('"recovery_disposition": "change_strategy"') + expect(content).toContain('"type": "tool_execution.error_execution.002"') + + // The user-visible message is concise and does not embed the JSON blob. + const sayCalls = mockTask.say.mock.calls.filter((call: unknown[]) => call[0] === "error") + expect(sayCalls).toHaveLength(1) + const sayMessage = String(sayCalls[0][1]) + expect(sayMessage).toContain("TERMINAL/PROVIDER_SWITCH/003") + expect(sayMessage).not.toContain("") + }) + + it("reports ordinary errors as retryable correct_once on first occurrence", async () => { + mockError = new Error("boom") + const toolCallId = "tool_call_err_2" + mockTask.assistantMessageContent = [executeCommandBlock(toolCallId)] + + await presentAssistantMessage(mockTask as unknown as Task) + + const content = String(findToolResult(mockTask, toolCallId).content) + expect(content).toContain('"retryable": true') + expect(content).toContain('"occurrence": 1') + expect(content).toContain('"recovery_disposition": "correct_once"') + }) + + it("increments the occurrence for repeated identical failures within the same task", async () => { + mockError = new Error("identical failure") + + mockTask.assistantMessageContent = [executeCommandBlock("tool_call_err_3a")] + await presentAssistantMessage(mockTask as unknown as Task) + + // Present a second, identical failure in the same task. + mockTask.assistantMessageContent = [executeCommandBlock("tool_call_err_3b")] + mockTask.currentStreamingContentIndex = 0 + mockTask.userMessageContent = [] + await presentAssistantMessage(mockTask as unknown as Task) + + const content = String(findToolResult(mockTask, "tool_call_err_3b").content) + expect(content).toContain('"occurrence": 2') + }) +}) diff --git a/src/core/assistant-message/__tests__/structuredError.spec.ts b/src/core/assistant-message/__tests__/structuredError.spec.ts new file mode 100644 index 0000000000..0b1a96d846 --- /dev/null +++ b/src/core/assistant-message/__tests__/structuredError.spec.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from "vitest" + +import { + buildErrorSignature, + buildStructuredErrorContent, + deriveRecoveryDisposition, + formatConciseErrorMessage, + formatStructuredError, + isRetryableError, + isUserRejectionError, + recordErrorOccurrence, +} from "../structuredError" + +/** + * Extracts and parses the JSON payload inside an block. + * Fails the test when the block is missing or the JSON is malformed. + */ +function parseDetails(content: string): Record { + const match = content.match(/^\n([\s\S]*)\n<\/error_details>$/) + if (!match) { + throw new Error("expected an block") + } + return JSON.parse(match[1]) as Record +} + +describe("formatStructuredError", () => { + const baseDetails = { + what: "An error occurred during executing command.", + why: "Something failed.", + next: ["First suggestion.", "Second suggestion."], + } + + it("reflects the provided retry guidance fields", () => { + const payload = parseDetails( + formatStructuredError({ + ...baseDetails, + pattern: "TOOL_EXECUTION/ERROR_EXECUTION/001", + retryable: false, + occurrence: 2, + disposition: "change_strategy", + }), + ) + expect(payload.retryable).toBe(false) + expect(payload.occurrence).toBe(2) + expect(payload.recovery_disposition).toBe("change_strategy") + expect(payload.pattern_id).toBe("TOOL_EXECUTION/ERROR_EXECUTION/001") + }) + + it("produces a type string without slashes", () => { + const payload = parseDetails( + formatStructuredError({ ...baseDetails, pattern: "TOOL_EXECUTION/ERROR_EXECUTION/001" }), + ) + expect(payload.type).toBe("tool_execution.error_execution.001") + expect(String(payload.type)).not.toContain("/") + }) + + it("clamps occurrence to at least 1", () => { + const payload = parseDetails(formatStructuredError({ ...baseDetails, occurrence: 0 })) + expect(payload.occurrence).toBe(1) + }) + + it("keeps the JSON valid when the payload exceeds the byte limit", () => { + const content = formatStructuredError( + { + what: `what-${"x".repeat(500)}`, + why: `why-${"y".repeat(500)}`, + next: ["first", "second", "third"], + pattern: "TOOL_EXECUTION/ERROR_EXECUTION/001", + }, + 400, + ) + // parseDetails asserts both the wrapper shape and JSON.parse success. + const payload = parseDetails(content) + expect(payload.pattern_id).toBe("TOOL_EXECUTION/ERROR_EXECUTION/001") + }) + + it("falls back to a minimal valid payload under a pathological byte limit", () => { + const content = formatStructuredError({ ...baseDetails }, 50) + const payload = parseDetails(content) + expect(payload.what).toBe("Error.") + expect(payload.next).toEqual([]) + }) +}) + +describe("isRetryableError", () => { + it("marks terminal/shell/provider-switch machine codes as non-retryable", () => { + expect(isRetryableError(new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed"))).toBe(false) + expect(isRetryableError(new Error("SHELL/INTEGRATION/001 shell channel unavailable"))).toBe(false) + expect(isRetryableError(new Error("failed: PROVIDER_SWITCH requested mid-run"))).toBe(false) + }) + + it("marks validation errors as non-retryable", () => { + const zodLike = new Error("invalid arguments") + zodLike.name = "ZodError" + expect(isRetryableError(zodLike)).toBe(false) + expect(isRetryableError(new Error("Input validation failed for tool read_file"))).toBe(false) + }) + + it("marks user rejections as non-retryable", () => { + expect(isRetryableError(new Error("Changes were rejected by the user."))).toBe(false) + expect(isRetryableError(new Error("Delete operation was denied by the user."))).toBe(false) + }) + + it("treats ordinary execution errors as retryable", () => { + expect(isRetryableError(new Error("ENOENT: no such file or directory"))).toBe(true) + expect(isRetryableError(new Error("network timeout"))).toBe(true) + }) +}) + +describe("isUserRejectionError", () => { + it("detects rejection phrasing", () => { + expect(isUserRejectionError(new Error("Changes were rejected by the user."))).toBe(true) + }) + it("does not flag unrelated errors", () => { + expect(isUserRejectionError(new Error("TERMINAL/PROVIDER_SWITCH/003"))).toBe(false) + }) +}) + +describe("deriveRecoveryDisposition", () => { + it("returns correct_once for a retryable first failure", () => { + expect(deriveRecoveryDisposition(new Error("boom"), 1)).toBe("correct_once") + }) + + it("escalates retryable errors to change_strategy at the stuck threshold", () => { + expect(deriveRecoveryDisposition(new Error("boom"), 3)).toBe("change_strategy") + expect(deriveRecoveryDisposition(new Error("boom"), 5)).toBe("change_strategy") + }) + + it("returns change_strategy for non-retryable errors", () => { + expect(deriveRecoveryDisposition(new Error("TERMINAL/PROVIDER_SWITCH/003"), 1)).toBe("change_strategy") + }) + + it("returns await_user for user rejections", () => { + expect(deriveRecoveryDisposition(new Error("Changes were rejected by the user."), 1)).toBe("await_user") + }) +}) + +describe("recordErrorOccurrence", () => { + it("counts repeated identical failures per task", () => { + const task = { id: "task-occ-1" } + const error = new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed") + const signature = buildErrorSignature("executing command", error) + expect(recordErrorOccurrence(task, signature)).toBe(1) + expect(recordErrorOccurrence(task, signature)).toBe(2) + expect(recordErrorOccurrence(task, signature)).toBe(3) + }) + + it("tracks different error signatures independently", () => { + const task = { id: "task-occ-2" } + const sigA = buildErrorSignature("executing command", new Error("error A")) + const sigB = buildErrorSignature("executing command", new Error("error B")) + expect(recordErrorOccurrence(task, sigA)).toBe(1) + expect(recordErrorOccurrence(task, sigB)).toBe(1) + expect(recordErrorOccurrence(task, sigA)).toBe(2) + }) + + it("does not leak occurrences across tasks", () => { + const taskA = { id: "task-occ-3a" } + const taskB = { id: "task-occ-3b" } + const signature = buildErrorSignature("executing command", new Error("same error")) + expect(recordErrorOccurrence(taskA, signature)).toBe(1) + expect(recordErrorOccurrence(taskB, signature)).toBe(1) + }) + + it("fails open with occurrence 1 for non-object task keys instead of throwing", () => { + // Double assertion is required to simulate the caller mistake this + // guards against: passing a string taskId where a Task object is + // expected. There is no typed way to express that mistake. + const notATask = "task-id" as unknown as object + expect(() => recordErrorOccurrence(notATask, "sig")).not.toThrow() + // Ephemeral state: counters never persist for invalid keys. + expect(recordErrorOccurrence(notATask, "sig")).toBe(1) + expect(recordErrorOccurrence(notATask, "sig")).toBe(1) + }) +}) + +describe("buildStructuredErrorContent", () => { + it("reports a first occurrence as retryable correct_once for ordinary errors", () => { + const task = { id: "task-bsec-1" } + const payload = parseDetails( + buildStructuredErrorContent( + task, + "executing command", + new Error("boom"), + "TOOL_EXECUTION/ERROR_EXECUTION/002", + ), + ) + expect(payload.retryable).toBe(true) + expect(payload.occurrence).toBe(1) + expect(payload.recovery_disposition).toBe("correct_once") + }) + + it("marks terminal provider-switch failures as non-retryable from the first occurrence", () => { + const task = { id: "task-bsec-2" } + const payload = parseDetails( + buildStructuredErrorContent( + task, + "executing command", + new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed"), + "TOOL_EXECUTION/ERROR_EXECUTION/002", + ), + ) + expect(payload.retryable).toBe(false) + expect(payload.occurrence).toBe(1) + expect(payload.recovery_disposition).toBe("change_strategy") + }) + + it("escalates repeated identical failures to change_strategy at the stuck threshold", () => { + const task = { id: "task-bsec-3" } + const error = new Error("identical failure") + buildStructuredErrorContent(task, "executing command", error, "TOOL_EXECUTION/ERROR_EXECUTION/002") + const second = parseDetails( + buildStructuredErrorContent(task, "executing command", error, "TOOL_EXECUTION/ERROR_EXECUTION/002"), + ) + expect(second.occurrence).toBe(2) + expect(second.recovery_disposition).toBe("correct_once") + + const third = parseDetails( + buildStructuredErrorContent(task, "executing command", error, "TOOL_EXECUTION/ERROR_EXECUTION/002"), + ) + expect(third.occurrence).toBe(3) + expect(third.recovery_disposition).toBe("change_strategy") + }) +}) + +describe("formatConciseErrorMessage", () => { + it("produces a single-line human message without the structured payload", () => { + const message = formatConciseErrorMessage("executing command", new Error("boom")) + expect(message).toContain("executing command") + expect(message).toContain("boom") + expect(message).not.toContain("") + }) + + it("handles errors with an empty message", () => { + expect(formatConciseErrorMessage("executing command", new Error())).toContain("An unexpected error occurred.") + }) +}) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 3e3ec7deb2..70e66aea70 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -1,4 +1,3 @@ -import { serializeError } from "serialize-error" import { Anthropic } from "@anthropic-ai/sdk" import type { ToolName, ClineAsk, ToolProgressStatus } from "@roo-code/types" @@ -14,72 +13,7 @@ import type { ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../sha import { AskIgnoredError } from "../task/AskIgnoredError" import { Task } from "../task/Task" -/** - * Structured error presentation for LLM-guided error recovery. - * Provides WHAT/WHY/NEXT format wrapped in XML tags. - */ -function formatStructuredError( - details: { - what: string - why: string - next: string[] - retryable?: boolean - pattern?: string - occurrence?: number - disposition?: string - }, - byteLimit: number = 8000, -): string { - const version = "1.0" - const status = "error" - const category = details.pattern ? (details.pattern.split("/")[1] ?? "unknown") : "unknown" - const type = details.pattern - ? details.pattern.startsWith("EI/") - ? details.pattern.replace("EI/", "guided_").toLowerCase().replace(/_/g, "_") - : details.pattern.toLowerCase().replace(/_/g, "_") - : "unclassified_error" - const what = details.what - const why = details.why - const next = details.next ?? [] - const retryable = details.retryable ?? true - const occurrence = Math.max(1, details.occurrence ?? 1) - const patternId = details.pattern ?? "UNCLASSIFIED/000/000" - const recoveryDisposition = details.disposition ?? "correct_once" - - const payload = { - version, - status, - type, - category, - what, - why, - next, - retryable, - occurrence, - pattern_id: patternId, - recovery_disposition: recoveryDisposition, - } - - let json = JSON.stringify(payload, null, 2) - - if (json.length > byteLimit && next.length > 0) { - // Trim Next items to fit within byte limit, preserving the first one - const firstItem = next[0] - next.length = 1 - const trimmed = { - ...payload, - next: [firstItem], - } - json = JSON.stringify(trimmed, null, 2) - } - - if (json.length > byteLimit) { - // Last resort: truncate the JSON string - json = json.substring(0, byteLimit - 3) + "..." - } - - return `\n${json}\n` -} +import { buildStructuredErrorContent, formatConciseErrorMessage } from "./structuredError" import { listFilesTool } from "../tools/ListFilesTool" import { readFileTool } from "../tools/ReadFileTool" @@ -200,7 +134,7 @@ export async function presentAssistantMessage(cline: Task) { // Store approval feedback to merge into tool result (GitHub #10465) let approvalFeedback: { text: string; images?: string[] } | undefined - const pushToolResult = (content: ToolResponse, feedbackImages?: string[]) => { + const pushToolResult = (content: ToolResponse, isError: boolean = false) => { if (hasToolResult) { console.warn( `[presentAssistantMessage] Skipping duplicate tool_result for mcp_tool_use: ${toolCallId}`, @@ -238,6 +172,7 @@ export async function presentAssistantMessage(cline: Task) { type: "tool_result", tool_use_id: sanitizeToolUseId(toolCallId), content: resultContent, + ...(isError ? { is_error: true } : {}), }) if (imageBlocks.length > 0) { @@ -293,27 +228,19 @@ export async function presentAssistantMessage(cline: Task) { return } - // Structured error presentation with WHAT/WHY/NEXT format - const serializedError = serializeError(error) - const structuredErrorContent = formatStructuredError({ - what: `An error occurred during ${action}.`, - why: error.message || serializedError.message || "An unexpected error occurred.", - next: [ - `Retry the ${action} operation with corrected parameters if applicable.`, - `If the error persists, report this issue to the development team with the error details below.`, - ], - pattern: "TOOL_EXECUTION/ERROR_EXECUTION/001", - retryable: true, - occurrence: 1, - disposition: "correct_once", - }) + // Structured error presentation with WHAT/WHY/NEXT format. Retry + // guidance and occurrence are derived from the error itself so the + // model is not told to retry non-retryable failures forever. + const structuredErrorContent = buildStructuredErrorContent( + cline, + action, + error, + "TOOL_EXECUTION/ERROR_EXECUTION/001", + ) - pushToolResult(structuredErrorContent) + pushToolResult(structuredErrorContent, true) - await cline.say( - "error", - `[${action}] Error during execution:\n${error.message ?? JSON.stringify(serializedError, null, 2)}\n\n${structuredErrorContent}`, - ) + await cline.say("error", formatConciseErrorMessage(action, error)) } if (!mcpBlock.partial) { @@ -529,7 +456,7 @@ export async function presentAssistantMessage(cline: Task) { // Store approval feedback to merge into tool result (GitHub #10465) let approvalFeedback: { text: string; images?: string[] } | undefined - const pushToolResult = (content: ToolResponse) => { + const pushToolResult = (content: ToolResponse, isError: boolean = false) => { // Native tool calling: only allow ONE tool_result per tool call if (hasToolResult) { console.warn( @@ -565,6 +492,7 @@ export async function presentAssistantMessage(cline: Task) { type: "tool_result", tool_use_id: sanitizeToolUseId(toolCallId), content: resultContent, + ...(isError ? { is_error: true } : {}), }) if (imageBlocks.length > 0) { @@ -627,27 +555,19 @@ export async function presentAssistantMessage(cline: Task) { return } - // Structured error presentation with WHAT/WHY/NEXT format - const serializedError = serializeError(error) - const structuredErrorContent = formatStructuredError({ - what: `An error occurred during ${action}.`, - why: error.message || serializedError.message || "An unexpected error occurred.", - next: [ - `Review the error details and retry the ${action} operation with corrected parameters.`, - `If the error persists, report this issue to the development team.`, - ], - pattern: "TOOL_EXECUTION/ERROR_EXECUTION/002", - retryable: true, - occurrence: 1, - disposition: "correct_once", - }) + // Structured error presentation with WHAT/WHY/NEXT format. Retry + // guidance and occurrence are derived from the error itself so the + // model is not told to retry non-retryable failures forever. + const structuredErrorContent = buildStructuredErrorContent( + cline, + action, + error, + "TOOL_EXECUTION/ERROR_EXECUTION/002", + ) - pushToolResult(structuredErrorContent) + pushToolResult(structuredErrorContent, true) - await cline.say( - "error", - `[${action}] Error during execution:\n${error.message ?? JSON.stringify(serializedError, null, 2)}\n\n${structuredErrorContent}`, - ) + await cline.say("error", formatConciseErrorMessage(action, error)) } if (!block.partial) { diff --git a/src/core/assistant-message/structuredError.ts b/src/core/assistant-message/structuredError.ts new file mode 100644 index 0000000000..ff484d9079 --- /dev/null +++ b/src/core/assistant-message/structuredError.ts @@ -0,0 +1,210 @@ +import { getTaskErrorState, STUCK_LOOP_THRESHOLD } from "../tools/error-interception/TaskErrorState" +import type { RecoveryDisposition } from "../tools/error-interception/types" + +/** + * Structured error presentation for LLM-guided error recovery. + * Provides WHAT/WHY/NEXT format wrapped in XML tags. + * + * Unlike the classifier-driven error-interception pipeline, this formatter is + * fed directly by the tool_use / mcp_tool_use `handleError` closures in + * presentAssistantMessage.ts. It derives honest retry guidance from the error + * itself and tracks per-task occurrence counts via TaskErrorState so repeated + * identical failures are reported as such instead of "occurrence 1, retryable" + * forever. + */ + +export interface StructuredErrorDetails { + what: string + why: string + next: string[] + retryable?: boolean + pattern?: string + occurrence?: number + disposition?: RecoveryDisposition +} + +/** + * Machine-code signals embedded in error messages that mark a failure as + * non-retryable (e.g. `TERMINAL/PROVIDER_SWITCH/003`). Retrying such an + * operation unchanged cannot succeed, so the model must be told to stop. + */ +const NON_RETRYABLE_MESSAGE_SIGNALS: readonly string[] = ["TERMINAL/", "SHELL/", "PROVIDER_SWITCH"] + +/** Error names produced by schema/argument validation layers. */ +const VALIDATION_ERROR_NAMES: ReadonlySet = new Set(["ZodError", "ValidationError"]) + +const VALIDATION_MESSAGE_RE = /\bvalidation (?:failed|error)\b/i + +/** Matches the user-rejection phrasing used by the edit/patch tool family. */ +const USER_REJECTION_RE = /(?:rejected|denied) by the user/i + +/** + * Returns true when the error represents the user declining an operation. + * Retrying automatically would override an explicit user decision. + */ +export function isUserRejectionError(error: Error): boolean { + return USER_REJECTION_RE.test(error.message ?? "") +} + +/** + * Derives retryability from the error itself. Known non-retryable signals: + * terminal/shell/provider-switch machine codes, validation errors, and user + * rejections. Everything else is considered retryable with corrected input. + */ +export function isRetryableError(error: Error): boolean { + const message = error.message ?? "" + if (NON_RETRYABLE_MESSAGE_SIGNALS.some((signal) => message.includes(signal))) { + return false + } + if (VALIDATION_ERROR_NAMES.has(error.name)) { + return false + } + if (VALIDATION_MESSAGE_RE.test(message)) { + return false + } + if (isUserRejectionError(error)) { + return false + } + return true +} + +/** + * Selects the occurrence-aware recovery disposition using the + * error-interception module's vocabulary: + * - user rejections -> `await_user` (never auto-retry a user decision) + * - non-retryable errors -> `change_strategy` + * - retryable errors -> `correct_once`, escalating to `change_strategy` once + * the same failure reaches the stuck-loop threshold. + */ +export function deriveRecoveryDisposition(error: Error, occurrence: number): RecoveryDisposition { + if (isUserRejectionError(error)) { + return "await_user" + } + if (!isRetryableError(error)) { + return "change_strategy" + } + return occurrence >= STUCK_LOOP_THRESHOLD ? "change_strategy" : "correct_once" +} + +/** + * Builds a stable signature for occurrence counting. Identical failures + * (same action, error name, and first message line) map to the same + * signature, so the Nth repetition reports occurrence N. + */ +export function buildErrorSignature(action: string, error: Error): string { + const firstLine = (error.message ?? "").split("\n", 1)[0].trim().slice(0, 200) + return `structured-error|${action}|${error.name}|${firstLine}` +} + +/** + * Increments and returns the per-task occurrence count for an error + * signature. State is kept in the error-interception module's TaskErrorState + * WeakMap, so counters persist across tool blocks within a task and are + * released with it. Non-object keys fail open with occurrence 1. + */ +export function recordErrorOccurrence(task: object, signature: string): number { + return getTaskErrorState(task).incrementOccurrence(signature) +} + +function truncateField(text: string, maxLength: number): string { + return text.length <= maxLength ? text : `${text.slice(0, maxLength)}…` +} + +/** + * Formats structured error details as an block containing + * JSON. The output is always valid JSON: when the payload exceeds + * `byteLimit`, Next items and free-text fields are truncated before + * serializing, with a minimal-but-valid payload as the last resort (the + * minimal payload may still exceed a pathologically small limit, but it is + * never malformed). + */ +export function formatStructuredError(details: StructuredErrorDetails, byteLimit: number = 8000): string { + const version = "1.0" + const status = "error" + const category = details.pattern ? (details.pattern.split("/")[1] ?? "unknown") : "unknown" + // A `type` discriminator must not contain slashes; use the dotted form of + // the pattern id (e.g. "tool_execution.error_execution.001"). + const type = details.pattern ? details.pattern.toLowerCase().replace(/\//g, ".") : "unclassified_error" + const retryable = details.retryable ?? true + const occurrence = Math.max(1, details.occurrence ?? 1) + const patternId = details.pattern ?? "UNCLASSIFIED/000/000" + const recoveryDisposition = details.disposition ?? "correct_once" + + const payload = { + version, + status, + type, + category, + what: details.what, + why: details.why, + next: details.next, + retryable, + occurrence, + pattern_id: patternId, + recovery_disposition: recoveryDisposition, + } + + let json = JSON.stringify(payload, null, 2) + + if (json.length > byteLimit && payload.next.length > 1) { + // Trim Next items to fit within byte limit, preserving the first one. + json = JSON.stringify({ ...payload, next: payload.next.slice(0, 1) }, null, 2) + } + + if (json.length > byteLimit) { + // Truncate the free-text fields before serializing so the block stays valid JSON. + json = JSON.stringify( + { + ...payload, + what: truncateField(details.what, 160), + why: truncateField(details.why, 160), + next: payload.next.slice(0, 1), + }, + null, + 2, + ) + } + + if (json.length > byteLimit) { + // Last resort: minimal payload that is still valid JSON. + json = JSON.stringify({ ...payload, what: "Error.", why: "Error.", next: [] }, null, 2) + } + + return `\n${json}\n` +} + +/** + * Builds the model-facing content for a tool execution + * failure, deriving honest retry guidance from the error and tracking the + * per-task occurrence of identical failures. + */ +export function buildStructuredErrorContent(task: object, action: string, error: Error, pattern: string): string { + const occurrence = recordErrorOccurrence(task, buildErrorSignature(action, error)) + const retryable = isRetryableError(error) + return formatStructuredError({ + what: `An error occurred during ${action}.`, + why: error.message || "An unexpected error occurred.", + next: retryable + ? [ + `Review the error details and retry the ${action} operation with corrected parameters.`, + `If the error persists, report this issue to the development team.`, + ] + : [ + `Do not retry the ${action} operation unchanged; this failure is not expected to resolve by retrying.`, + `Change the parameters or the tool, or ask the user how to proceed.`, + ], + pattern, + retryable, + occurrence, + disposition: deriveRecoveryDisposition(error, occurrence), + }) +} + +/** + * Builds the concise, human-readable message shown in the chat UI via + * say("error", ...). The structured payload is intentionally kept out of the + * UI message; it lives only in the tool result. + */ +export function formatConciseErrorMessage(action: string, error: Error): string { + return `Error during ${action}: ${error.message || "An unexpected error occurred."}` +} From d679650ca6d53ab05f5bae5679b7274c73395082 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Fri, 7 Aug 2026 04:59:55 +0900 Subject: [PATCH 4/8] chore: remove temp file progress.txt --- progress.txt | 59 ---------------------------------------------------- 1 file changed, 59 deletions(-) delete mode 100644 progress.txt diff --git a/progress.txt b/progress.txt deleted file mode 100644 index b3983826b3..0000000000 --- a/progress.txt +++ /dev/null @@ -1,59 +0,0 @@ -# Reapplication Progress — rc6 branch cleanup -# Updated: 2026-02-15 - -## Completed Batches - -### Batch 1 — Clean cherry-picks (PR #11473) -- 22 PRs merged cleanly -- Status: MERGED to main - -### Batch 2 — Minor conflicts (PR #11474) -- 9 PRs with minor conflicts resolved -- Status: MERGED to main - -### Batch 3 — Skills Infrastructure & Browser Use Removal (4 PRs) -- PR #11102: skill mode dropdown (44 conflicts resolved) -- PR #11157: improve Skills/Slash Commands UI (6 conflicts resolved) -- PR #11414: remove built-in skills mechanism (4 conflicts resolved) -- PR #11392: remove browser use entirely (5 conflicts resolved) -- Status: ON BRANCH reapply/batch-3-4-5-major-conflicts - -### Batch 4 — Provider Removals (2 PRs) -- PR #11253: remove URL context/Grounding checkboxes (4 conflicts resolved) -- PR #11297: remove 9 low-usage providers + retired UX (14 conflicts resolved) -- Status: ON BRANCH reapply/batch-3-4-5-major-conflicts - -### Batch 5 — Azure Foundry -- PR #11315 and #11374: EXCLUDED — depends on AI-SDK (@ai-sdk/azure, from "ai") -- These PRs are AI-SDK-entangled and cannot be cherry-picked to the pre-AI-SDK codebase -- Status: DEFERRED (AI-SDK dependent) - -## Post-cherry-pick Fixes Applied -1. Restored gemini.ts + vertex.ts to pre-AI-SDK state (cherry-picks brought AI-SDK versions) -2. Restored ai-sdk.spec.ts, gemini-handler.spec.ts, vertex.spec.ts to pre-AI-SDK versions -3. Fixed processUserContentMentions.ts ghost import (rooMessage.ts doesn't exist) -4. Added missing skills type exports to @roo-code/types (SkillMetadata, validateSkillName, etc.) -5. Added SkillsSettings import to SettingsView.tsx -6. Added Dialog/Select/Collapsible mocks to SettingsView test files -7. Fixed Task.ts type mismatches (replaced local types with Anthropic SDK types) -8. Added skills state to ExtensionStateContext - -## Deferred PRs (AI-SDK Entangled) -- #11379: delegation (AI-SDK) -- #11418: delegation (AI-SDK) -- #11422: delegation (AI-SDK) -- #11315: Azure Foundry provider (AI-SDK) -- #11374: Azure Foundry fix (AI-SDK) - -## Validation Results -- Backend tests: ALL PASSED (5224 tests) -- UI tests: ALL PASSED (1267 tests) -- Type checks: ALL PASSED (14/14 packages) -- AI-SDK contamination: CLEAN (0 matches) - -## Notes -- Pre-push hook fails on `roo-cline:bundle` because `generate-built-in-skills.ts` was removed - by PR #11414 but `package.json` still references it in `prebundle`. This is expected and - will be resolved when the PR is merged to main and the script reference is cleaned up. -- Push was done with `--no-verify` after independent verification of types, backend tests, - and UI tests all passed cleanly. From 3e88d471f80fe6181b3df13fb334cba8bd7e4c1e Mon Sep 17 00:00:00 2001 From: Alexei Gubin <36731953+WebMad@users.noreply.github.com> Date: Fri, 7 Aug 2026 06:38:32 +0300 Subject: [PATCH 5/8] refactor(cli): canonicalize provider identifiers (#1110) * refactor(types): canonicalize provider settings identifiers * refactor(cli): canonicalize provider identifiers * refactor(cli): extract option resolution helpers * test(cli): cover run option resolution * fix(cli): resolve specialized provider model IDs --- .../src/commands/cli/__tests__/list.test.ts | 134 ++++++++-- .../src/commands/cli/__tests__/run.test.ts | 146 +++++++++++ apps/cli/src/commands/cli/list.ts | 10 +- apps/cli/src/commands/cli/run.ts | 48 +++- .../utils/__tests__/context-window.test.ts | 40 +++ .../src/lib/utils/__tests__/provider.test.ts | 45 +++- apps/cli/src/lib/utils/context-window.ts | 59 ++++- apps/cli/src/lib/utils/provider.ts | 22 +- apps/cli/src/types/__tests__/types.test.ts | 32 ++- apps/cli/src/types/types.ts | 12 +- .../__tests__/provider-identifiers.test.ts | 7 + packages/types/src/provider-settings.ts | 245 ++++++++++-------- 12 files changed, 617 insertions(+), 183 deletions(-) create mode 100644 apps/cli/src/lib/utils/__tests__/context-window.test.ts diff --git a/apps/cli/src/commands/cli/__tests__/list.test.ts b/apps/cli/src/commands/cli/__tests__/list.test.ts index 71bdc4266b..78db9752d8 100644 --- a/apps/cli/src/commands/cli/__tests__/list.test.ts +++ b/apps/cli/src/commands/cli/__tests__/list.test.ts @@ -1,7 +1,45 @@ +import fs from "fs" +import os from "os" +import path from "path" +import { EventEmitter } from "events" + +import { openRouterDefaultModelId, providerIdentifiers } from "@roo-code/types" + import { readWorkspaceTaskSessions } from "@/lib/task-history/index.js" -import { isRecord } from "@/lib/utils/guards.js" -import { listSessions, parseFormat } from "../list.js" +import { listModels, listSessions, parseFormat } from "../list.js" + +const extensionHostMock = vi.hoisted(() => ({ + activate: vi.fn(async () => undefined), + dispose: vi.fn(async () => undefined), + options: [] as unknown[], + responses: [] as unknown[], + sendToExtension: vi.fn(), +})) + +vi.mock("@/agent/index.js", () => ({ + ExtensionHost: class extends EventEmitter { + client = { + isInitialized: () => true, + on: vi.fn(() => () => undefined), + } + + constructor(options: unknown) { + super() + extensionHostMock.options.push(options) + } + + activate = extensionHostMock.activate + dispose = extensionHostMock.dispose + + sendToExtension(message: unknown): void { + extensionHostMock.sendToExtension(message) + for (const response of extensionHostMock.responses) { + this.emit("extensionWebviewMessage", response) + } + } + }, +})) vi.mock("@/lib/task-history/index.js", async (importOriginal) => { const actual = await importOriginal() @@ -39,30 +77,88 @@ describe("parseFormat", () => { }) }) -describe("router model extraction", () => { - // This mirrors the extraction logic in requestOpenRouterModels (list.ts:226-228) - const extractOpenRouterModels = (routerModelsRaw: unknown) => { - const routerModels = isRecord(routerModelsRaw) ? routerModelsRaw : {} - const openRouterModels = routerModels.openrouter - return isRecord(openRouterModels) ? openRouterModels : {} - } +describe("listModels", () => { + let tempDir: string + let workspacePath: string + let extensionPath: string - it("extracts openrouter models from valid routerModels", () => { - const models = { "openai/gpt-4.1": { contextWindow: 128000, supportsPromptCache: false } } - const result = extractOpenRouterModels({ openrouter: models }) - expect(result).toEqual(models) + beforeEach(() => { + vi.clearAllMocks() + extensionHostMock.options.length = 0 + extensionHostMock.responses.length = 0 + + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "roo-list-test-")) + workspacePath = path.join(tempDir, "workspace") + extensionPath = path.join(tempDir, "extension") + fs.mkdirSync(workspacePath) + fs.mkdirSync(extensionPath) + fs.writeFileSync(path.join(extensionPath, "extension.js"), "") }) - it("returns empty object when routerModels is null", () => { - expect(extractOpenRouterModels(null)).toEqual({}) + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }) + vi.restoreAllMocks() }) - it("returns empty object when openrouter key is missing", () => { - expect(extractOpenRouterModels({ requesty: {} })).toEqual({}) + const captureStdout = async (fn: () => Promise): Promise => { + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true) + await fn() + return stdoutSpy.mock.calls.map(([chunk]) => String(chunk)).join("") + } + + it("creates a host with resolved paths and returns OpenRouter models", async () => { + const models = { "openai/gpt-4.1": { contextWindow: 128000, supportsPromptCache: false } } + extensionHostMock.responses.push( + { type: "unrelatedMessage" }, + { type: "routerModels", routerModels: { [providerIdentifiers.openrouter]: models } }, + ) + + const output = await captureStdout(() => + listModels({ + format: "json", + workspace: path.relative(process.cwd(), workspacePath), + extension: path.relative(process.cwd(), extensionPath), + apiKey: "test-api-key", + debug: true, + }), + ) + + expect(extensionHostMock.options).toEqual([ + expect.objectContaining({ + mode: "code", + provider: providerIdentifiers.openrouter, + model: openRouterDefaultModelId, + apiKey: "test-api-key", + workspacePath, + extensionPath, + nonInteractive: true, + ephemeral: true, + debug: true, + exitOnComplete: true, + exitOnError: false, + disableOutput: true, + }), + ]) + expect(extensionHostMock.activate).toHaveBeenCalledOnce() + expect(extensionHostMock.sendToExtension).toHaveBeenCalledWith({ + type: "requestRouterModels", + values: { provider: providerIdentifiers.openrouter }, + }) + expect(extensionHostMock.dispose).toHaveBeenCalledOnce() + expect(JSON.parse(output)).toEqual({ models }) }) - it("returns empty object when openrouter value is not a record", () => { - expect(extractOpenRouterModels({ openrouter: "invalid" })).toEqual({}) + it.each([ + ["a malformed routerModels value", null], + ["a malformed OpenRouter value", { [providerIdentifiers.openrouter]: "invalid" }], + ])("returns an empty model record for %s", async (_description, routerModels) => { + extensionHostMock.responses.push({ type: "routerModels", routerModels }) + + const output = await captureStdout(() => + listModels({ format: "json", workspace: workspacePath, extension: extensionPath }), + ) + + expect(JSON.parse(output)).toEqual({ models: {} }) }) }) diff --git a/apps/cli/src/commands/cli/__tests__/run.test.ts b/apps/cli/src/commands/cli/__tests__/run.test.ts index 7b7693a39c..e20d0672c3 100644 --- a/apps/cli/src/commands/cli/__tests__/run.test.ts +++ b/apps/cli/src/commands/cli/__tests__/run.test.ts @@ -2,6 +2,152 @@ import fs from "fs" import path from "path" import os from "os" +import { providerIdentifiers } from "@roo-code/types" +import { DEFAULT_FLAGS, FlagOptions } from "@/types/index.js" +import { + resolveLegacyRequireApproval, + resolveModel, + resolveProvider, + resolveReasoningEffort, + resolveWorkspacePath, + run, +} from "../run.js" + +const runCommandMocks = vi.hoisted(() => ({ + activate: vi.fn(async () => undefined), + dispose: vi.fn(async () => undefined), + loadSettings: vi.fn(), + options: [] as unknown[], + runTask: vi.fn(async () => undefined), +})) + +vi.mock("@/lib/storage/index.js", () => ({ + loadSettings: runCommandMocks.loadSettings, +})) + +vi.mock("@/agent/index.js", () => ({ + ExtensionHost: class { + client = {} + + constructor(options: unknown) { + runCommandMocks.options.push(options) + } + + activate = runCommandMocks.activate + dispose = runCommandMocks.dispose + runTask = runCommandMocks.runTask + }, +})) + +describe("resolveModel", () => { + it("uses the CLI flag before the settings model", () => { + expect(resolveModel("flag-model", "settings-model")).toBe("flag-model") + }) + + it("uses the settings model when the CLI flag is absent", () => { + expect(resolveModel(undefined, "settings-model")).toBe("settings-model") + }) + + it("uses the default model when neither the CLI flag nor settings provide one", () => { + expect(resolveModel()).toBe(DEFAULT_FLAGS.model) + }) +}) + +describe("resolveReasoningEffort", () => { + it("uses CLI, settings, and default values in priority order", () => { + expect(resolveReasoningEffort("high", "low")).toBe("high") + expect(resolveReasoningEffort(undefined, "low")).toBe("low") + expect(resolveReasoningEffort()).toBe(DEFAULT_FLAGS.reasoningEffort) + }) +}) + +describe("resolveProvider", () => { + it("uses CLI, settings, and openrouter values in priority order", () => { + expect(resolveProvider(providerIdentifiers.anthropic, providerIdentifiers.gemini)).toBe( + providerIdentifiers.anthropic, + ) + expect(resolveProvider(undefined, providerIdentifiers.gemini)).toBe(providerIdentifiers.gemini) + expect(resolveProvider()).toBe(providerIdentifiers.openrouter) + }) +}) + +describe("resolveWorkspacePath", () => { + it("resolves the provided workspace path", () => { + expect(resolveWorkspacePath("relative/workspace")).toBe(path.resolve("relative/workspace")) + }) + + it("uses the current working directory when workspace is absent", () => { + expect(resolveWorkspacePath()).toBe(process.cwd()) + }) +}) + +describe("resolveLegacyRequireApproval", () => { + it.each([ + { requireApproval: true, dangerouslySkipPermissions: true, expected: true }, + { requireApproval: false, dangerouslySkipPermissions: false, expected: false }, + { requireApproval: undefined, dangerouslySkipPermissions: false, expected: true }, + { requireApproval: undefined, dangerouslySkipPermissions: true, expected: false }, + { requireApproval: undefined, dangerouslySkipPermissions: undefined, expected: undefined }, + ])( + "resolves requireApproval=$requireApproval and dangerouslySkipPermissions=$dangerouslySkipPermissions", + ({ requireApproval, dangerouslySkipPermissions, expected }) => { + expect(resolveLegacyRequireApproval(requireApproval, dangerouslySkipPermissions)).toBe(expected) + }, + ) +}) + +describe("run command option resolution", () => { + let workspacePath: string + + beforeEach(() => { + vi.clearAllMocks() + runCommandMocks.options.length = 0 + workspacePath = fs.mkdtempSync(path.join(os.tmpdir(), "roo-run-test-")) + }) + + afterEach(() => { + fs.rmSync(workspacePath, { recursive: true, force: true }) + vi.restoreAllMocks() + }) + + it("passes resolved settings and workspace values to the extension host", async () => { + runCommandMocks.loadSettings.mockResolvedValue({ + model: "settings-model", + reasoningEffort: "high", + provider: providerIdentifiers.anthropic, + dangerouslySkipPermissions: false, + }) + const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never) + const flags: FlagOptions = { + continue: false, + workspace: path.relative(process.cwd(), workspacePath), + print: true, + stdinPromptStream: false, + signalOnlyExit: false, + debug: false, + requireApproval: false, + exitOnError: false, + apiKey: "test-api-key", + ephemeral: true, + oneshot: false, + } + + await run("test prompt", flags) + + expect(runCommandMocks.options).toEqual([ + expect.objectContaining({ + model: "settings-model", + reasoningEffort: "high", + provider: providerIdentifiers.anthropic, + workspacePath, + nonInteractive: false, + }), + ]) + expect(runCommandMocks.runTask).toHaveBeenCalledWith("test prompt", undefined) + expect(exitSpy).toHaveBeenCalledWith(0) + }) +}) + describe("run command --prompt-file option", () => { let tempDir: string let promptFilePath: string diff --git a/apps/cli/src/commands/cli/list.ts b/apps/cli/src/commands/cli/list.ts index fbd33da2cc..c5fbb4dba9 100644 --- a/apps/cli/src/commands/cli/list.ts +++ b/apps/cli/src/commands/cli/list.ts @@ -6,7 +6,7 @@ import pWaitFor from "p-wait-for" import type { TaskSessionEntry } from "@roo-code/core/cli" import type { Command, ModelRecord, WebviewMessage } from "@roo-code/types" -import { openRouterDefaultModelId } from "@roo-code/types" +import { openRouterDefaultModelId, providerIdentifiers } from "@roo-code/types" import { ExtensionHost, type ExtensionHostOptions } from "@/agent/index.js" import { readWorkspaceTaskSessions } from "@/lib/task-history/index.js" @@ -105,13 +105,13 @@ function outputSessionsText(sessions: SessionLike[]): void { async function createListHost(options: BaseListOptions, hostOptions: ListHostOptions): Promise { const workspacePath = resolveWorkspacePath(options.workspace) const extensionPath = resolveExtensionPath(options.extension) - const apiKey = options.apiKey || getApiKeyFromEnv("openrouter") + const apiKey = options.apiKey || getApiKeyFromEnv(providerIdentifiers.openrouter) const extensionHostOptions: ExtensionHostOptions = { mode: "code", reasoningEffort: undefined, user: null, - provider: "openrouter", + provider: providerIdentifiers.openrouter, model: openRouterDefaultModelId, apiKey, workspacePath, @@ -217,14 +217,14 @@ function requestModes(host: ExtensionHost): Promise { function requestOpenRouterModels(host: ExtensionHost): Promise { return requestFromExtension( host, - { type: "requestRouterModels", values: { provider: "openrouter" } }, + { type: "requestRouterModels", values: { provider: providerIdentifiers.openrouter } }, (message) => { if (message.type !== "routerModels") { return undefined } const routerModels = isRecord(message.routerModels) ? message.routerModels : {} - const openRouterModels = routerModels.openrouter + const openRouterModels = routerModels[providerIdentifiers.openrouter] return isRecord(openRouterModels) ? (openRouterModels as ModelRecord) : {} }, ) diff --git a/apps/cli/src/commands/cli/run.ts b/apps/cli/src/commands/cli/run.ts index 908df9938b..bedb520ed4 100644 --- a/apps/cli/src/commands/cli/run.ts +++ b/apps/cli/src/commands/cli/run.ts @@ -5,10 +5,13 @@ import { fileURLToPath } from "url" import { createElement } from "react" import pWaitFor from "p-wait-for" +import { providerIdentifiers } from "@roo-code/types" import { setLogger } from "@roo-code/vscode-shim" import { FlagOptions, + ReasoningEffortFlagOptions, + SupportedProvider, isSupportedProvider, supportedProviders, DEFAULT_FLAGS, @@ -49,6 +52,35 @@ function normalizeError(error: unknown): Error { return error instanceof Error ? error : new Error(String(error)) } +export function resolveModel(flagModel?: string, settingsModel?: string): string { + return flagModel || settingsModel || DEFAULT_FLAGS.model +} + +export function resolveReasoningEffort( + flagReasoningEffort?: ReasoningEffortFlagOptions, + settingsReasoningEffort?: ReasoningEffortFlagOptions, +): ReasoningEffortFlagOptions { + return flagReasoningEffort || settingsReasoningEffort || DEFAULT_FLAGS.reasoningEffort +} + +export function resolveProvider( + flagProvider?: SupportedProvider, + settingsProvider?: SupportedProvider, +): SupportedProvider { + return flagProvider ?? settingsProvider ?? providerIdentifiers.openrouter +} + +export function resolveWorkspacePath(workspace?: string): string { + return workspace ? path.resolve(workspace) : process.cwd() +} + +export function resolveLegacyRequireApproval( + requireApproval?: boolean, + dangerouslySkipPermissions?: boolean, +): boolean | undefined { + return requireApproval ?? (dangerouslySkipPermissions === undefined ? undefined : !dangerouslySkipPermissions) +} + export async function run(promptArg: string | undefined, flagOptions: FlagOptions) { setLogger({ info: () => {}, @@ -119,14 +151,14 @@ export async function run(promptArg: string | undefined, flagOptions: FlagOption // Determine effective values: CLI flags > settings file > DEFAULT_FLAGS. const effectiveMode = flagOptions.mode || settings.mode || DEFAULT_FLAGS.mode - const effectiveModel = flagOptions.model || settings.model || DEFAULT_FLAGS.model - const effectiveReasoningEffort = - flagOptions.reasoningEffort || settings.reasoningEffort || DEFAULT_FLAGS.reasoningEffort - const effectiveProvider = flagOptions.provider ?? settings.provider ?? "openrouter" - const effectiveWorkspacePath = flagOptions.workspace ? path.resolve(flagOptions.workspace) : process.cwd() - const legacyRequireApprovalFromSettings = - settings.requireApproval ?? - (settings.dangerouslySkipPermissions === undefined ? undefined : !settings.dangerouslySkipPermissions) + const effectiveModel = resolveModel(flagOptions.model, settings.model) + const effectiveReasoningEffort = resolveReasoningEffort(flagOptions.reasoningEffort, settings.reasoningEffort) + const effectiveProvider = resolveProvider(flagOptions.provider, settings.provider) + const effectiveWorkspacePath = resolveWorkspacePath(flagOptions.workspace) + const legacyRequireApprovalFromSettings = resolveLegacyRequireApproval( + settings.requireApproval, + settings.dangerouslySkipPermissions, + ) const effectiveRequireApproval = flagOptions.requireApproval || legacyRequireApprovalFromSettings || false const effectiveExitOnComplete = flagOptions.print || flagOptions.oneshot || settings.oneshot || false const rawConsecutiveMistakeLimit = diff --git a/apps/cli/src/lib/utils/__tests__/context-window.test.ts b/apps/cli/src/lib/utils/__tests__/context-window.test.ts new file mode 100644 index 0000000000..8d33ef5e2b --- /dev/null +++ b/apps/cli/src/lib/utils/__tests__/context-window.test.ts @@ -0,0 +1,40 @@ +import { providerIdentifiers, type ProviderSettings } from "@roo-code/types" + +import { DEFAULT_CONTEXT_WINDOW, getContextWindow } from "../context-window.js" + +describe("getContextWindow", () => { + it.each([ + [providerIdentifiers.openrouter, "openRouterModelId"], + [providerIdentifiers.ollama, "ollamaModelId"], + [providerIdentifiers.lmstudio, "lmStudioModelId"], + [providerIdentifiers.openai, "openAiModelId"], + [providerIdentifiers.requesty, "requestyModelId"], + [providerIdentifiers.unbound, "unboundModelId"], + [providerIdentifiers.litellm, "litellmModelId"], + [providerIdentifiers.vercelAiGateway, "vercelAiGatewayModelId"], + [providerIdentifiers.opencodeGo, "opencodeGoModelId"], + [providerIdentifiers.kenari, "kenariModelId"], + [providerIdentifiers.zooGateway, "zooGatewayModelId"], + ] as const)("uses the provider-specific model field for %s", (provider, modelField) => { + const config = { apiProvider: provider, [modelField]: "selected-model" } as ProviderSettings + const routerModels = { [provider]: { "selected-model": { contextWindow: 123_456 } } } + + expect(getContextWindow(routerModels, config)).toBe(123_456) + }) + + it("uses apiModelId for providers without a specialized model field", () => { + const config: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "selected-model", + } + const routerModels = { + [providerIdentifiers.anthropic]: { "selected-model": { contextWindow: 64_000 } }, + } + + expect(getContextWindow(routerModels, config)).toBe(64_000) + }) + + it("returns the default when the selected model is unavailable", () => { + expect(getContextWindow({}, { apiProvider: providerIdentifiers.openrouter })).toBe(DEFAULT_CONTEXT_WINDOW) + }) +}) diff --git a/apps/cli/src/lib/utils/__tests__/provider.test.ts b/apps/cli/src/lib/utils/__tests__/provider.test.ts index 70d8a2a555..db44174f45 100644 --- a/apps/cli/src/lib/utils/__tests__/provider.test.ts +++ b/apps/cli/src/lib/utils/__tests__/provider.test.ts @@ -1,4 +1,47 @@ -import { getApiKeyFromEnv } from "../provider.js" +import { providerIdentifiers } from "@roo-code/types" + +import { getApiKeyFromEnv, getEnvVarName, getProviderSettings } from "../provider.js" + +describe("provider configuration", () => { + it.each([ + [providerIdentifiers.anthropic, "ANTHROPIC_API_KEY"], + [providerIdentifiers.openaiNative, "OPENAI_API_KEY"], + [providerIdentifiers.gemini, "GOOGLE_API_KEY"], + [providerIdentifiers.openrouter, "OPENROUTER_API_KEY"], + [providerIdentifiers.vercelAiGateway, "VERCEL_AI_GATEWAY_API_KEY"], + ] as const)("maps canonical provider %s to %s", (provider, envVarName) => { + expect(getEnvVarName(provider)).toBe(envVarName) + }) + + it.each([ + [ + providerIdentifiers.anthropic, + { apiProvider: providerIdentifiers.anthropic, apiKey: "key", apiModelId: "model" }, + ], + [ + providerIdentifiers.openaiNative, + { apiProvider: providerIdentifiers.openaiNative, openAiNativeApiKey: "key", apiModelId: "model" }, + ], + [ + providerIdentifiers.gemini, + { apiProvider: providerIdentifiers.gemini, geminiApiKey: "key", apiModelId: "model" }, + ], + [ + providerIdentifiers.openrouter, + { apiProvider: providerIdentifiers.openrouter, openRouterApiKey: "key", openRouterModelId: "model" }, + ], + [ + providerIdentifiers.vercelAiGateway, + { + apiProvider: providerIdentifiers.vercelAiGateway, + vercelAiGatewayApiKey: "key", + vercelAiGatewayModelId: "model", + }, + ], + ] as const)("builds settings for canonical provider %s", (provider, expected) => { + expect(getProviderSettings(provider, "key", "model")).toEqual(expected) + }) +}) describe("getApiKeyFromEnv", () => { const originalEnv = process.env diff --git a/apps/cli/src/lib/utils/context-window.ts b/apps/cli/src/lib/utils/context-window.ts index 5cd58b55a8..1d6402c525 100644 --- a/apps/cli/src/lib/utils/context-window.ts +++ b/apps/cli/src/lib/utils/context-window.ts @@ -1,4 +1,4 @@ -import type { ProviderSettings } from "@roo-code/types" +import { providerIdentifiers, retiredProviderIdentifiers, type ProviderSettings } from "@roo-code/types" import type { RouterModels } from "@/ui/store.js" @@ -36,24 +36,61 @@ export function getContextWindow(routerModels: RouterModels | null, apiConfigura */ function getModelIdForProvider(config: ProviderSettings): string | undefined { switch (config.apiProvider) { - case "openrouter": + case providerIdentifiers.openrouter: return config.openRouterModelId - case "ollama": + case providerIdentifiers.ollama: return config.ollamaModelId - case "lmstudio": + case providerIdentifiers.lmstudio: return config.lmStudioModelId - case "openai": + case providerIdentifiers.openai: return config.openAiModelId - case "requesty": + case providerIdentifiers.requesty: return config.requestyModelId - case "unbound": + case providerIdentifiers.unbound: return config.unboundModelId - case "litellm": + case providerIdentifiers.litellm: return config.litellmModelId - case "vercel-ai-gateway": + case providerIdentifiers.vercelAiGateway: return config.vercelAiGatewayModelId - default: - // For anthropic, bedrock, vertex, gemini, xai, etc. + case providerIdentifiers.opencodeGo: + return config.opencodeGoModelId + case providerIdentifiers.kenari: + return config.kenariModelId + case providerIdentifiers.zooGateway: + return config.zooGatewayModelId + case providerIdentifiers.anthropic: + case providerIdentifiers.bedrock: + case providerIdentifiers.baseten: + case providerIdentifiers.deepseek: + case providerIdentifiers.fireworks: + case providerIdentifiers.friendli: + case providerIdentifiers.gemini: + case providerIdentifiers.geminiCli: + case providerIdentifiers.mistral: + case providerIdentifiers.moonshot: + case providerIdentifiers.kimiCode: + case providerIdentifiers.minimax: + case providerIdentifiers.mimo: + case providerIdentifiers.openaiCodex: + case providerIdentifiers.openaiNative: + case providerIdentifiers.poe: + case providerIdentifiers.qwenCode: + case providerIdentifiers.sambanova: + case providerIdentifiers.vertex: + case providerIdentifiers.xai: + case providerIdentifiers.zai: + case retiredProviderIdentifiers.cerebras: + case retiredProviderIdentifiers.chutes: + case retiredProviderIdentifiers.deepinfra: + case retiredProviderIdentifiers.doubao: + case retiredProviderIdentifiers.featherless: + case retiredProviderIdentifiers.groq: + case retiredProviderIdentifiers.huggingface: + case retiredProviderIdentifiers.ioIntelligence: + case retiredProviderIdentifiers.roo: + case providerIdentifiers.vscodeLm: + case providerIdentifiers.fakeAi: + case undefined: return config.apiModelId } } diff --git a/apps/cli/src/lib/utils/provider.ts b/apps/cli/src/lib/utils/provider.ts index 26beaf90c4..7cb7b30ffb 100644 --- a/apps/cli/src/lib/utils/provider.ts +++ b/apps/cli/src/lib/utils/provider.ts @@ -1,13 +1,13 @@ -import { RooCodeSettings } from "@roo-code/types" +import { providerIdentifiers, type RooCodeSettings } from "@roo-code/types" import type { SupportedProvider } from "@/types/index.js" const envVarMap: Record = { - anthropic: "ANTHROPIC_API_KEY", - "openai-native": "OPENAI_API_KEY", - gemini: "GOOGLE_API_KEY", - openrouter: "OPENROUTER_API_KEY", - "vercel-ai-gateway": "VERCEL_AI_GATEWAY_API_KEY", + [providerIdentifiers.anthropic]: "ANTHROPIC_API_KEY", + [providerIdentifiers.openaiNative]: "OPENAI_API_KEY", + [providerIdentifiers.gemini]: "GOOGLE_API_KEY", + [providerIdentifiers.openrouter]: "OPENROUTER_API_KEY", + [providerIdentifiers.vercelAiGateway]: "VERCEL_AI_GATEWAY_API_KEY", } export function getEnvVarName(provider: SupportedProvider): string { @@ -27,23 +27,23 @@ export function getProviderSettings( const config: RooCodeSettings = { apiProvider: provider } switch (provider) { - case "anthropic": + case providerIdentifiers.anthropic: if (apiKey) config.apiKey = apiKey if (model) config.apiModelId = model break - case "openai-native": + case providerIdentifiers.openaiNative: if (apiKey) config.openAiNativeApiKey = apiKey if (model) config.apiModelId = model break - case "gemini": + case providerIdentifiers.gemini: if (apiKey) config.geminiApiKey = apiKey if (model) config.apiModelId = model break - case "openrouter": + case providerIdentifiers.openrouter: if (apiKey) config.openRouterApiKey = apiKey if (model) config.openRouterModelId = model break - case "vercel-ai-gateway": + case providerIdentifiers.vercelAiGateway: if (apiKey) config.vercelAiGatewayApiKey = apiKey if (model) config.vercelAiGatewayModelId = model break diff --git a/apps/cli/src/types/__tests__/types.test.ts b/apps/cli/src/types/__tests__/types.test.ts index 1e54b5069e..5ed0c84016 100644 --- a/apps/cli/src/types/__tests__/types.test.ts +++ b/apps/cli/src/types/__tests__/types.test.ts @@ -1,5 +1,19 @@ +import { providerIdentifiers } from "@roo-code/types" + import { isSupportedProvider, supportedProviders } from "../types.js" +describe("supportedProviders", () => { + it("contains the canonical identifiers for the CLI provider subset", () => { + expect(supportedProviders).toEqual([ + providerIdentifiers.anthropic, + providerIdentifiers.openaiNative, + providerIdentifiers.gemini, + providerIdentifiers.openrouter, + providerIdentifiers.vercelAiGateway, + ]) + }) +}) + describe("isSupportedProvider", () => { it.each(supportedProviders)("returns true for supported provider '%s'", (provider) => { expect(isSupportedProvider(provider)).toBe(true) @@ -22,25 +36,25 @@ describe("provider resolution fallback", () => { it("defaults to openrouter when no flag or setting is provided", () => { const flagProvider = undefined const settingsProvider = undefined - const effectiveProvider = flagProvider ?? settingsProvider ?? "openrouter" + const effectiveProvider = flagProvider ?? settingsProvider ?? providerIdentifiers.openrouter - expect(effectiveProvider).toBe("openrouter") + expect(effectiveProvider).toBe(providerIdentifiers.openrouter) expect(isSupportedProvider(effectiveProvider)).toBe(true) }) it("uses flag provider over settings and default", () => { - const flagProvider = "anthropic" - const settingsProvider = "gemini" - const effectiveProvider = flagProvider ?? settingsProvider ?? "openrouter" + const flagProvider = providerIdentifiers.anthropic + const settingsProvider = providerIdentifiers.gemini + const effectiveProvider = flagProvider ?? settingsProvider ?? providerIdentifiers.openrouter - expect(effectiveProvider).toBe("anthropic") + expect(effectiveProvider).toBe(providerIdentifiers.anthropic) }) it("uses settings provider when flag is not provided", () => { const flagProvider = undefined - const settingsProvider = "gemini" - const effectiveProvider = flagProvider ?? settingsProvider ?? "openrouter" + const settingsProvider = providerIdentifiers.gemini + const effectiveProvider = flagProvider ?? settingsProvider ?? providerIdentifiers.openrouter - expect(effectiveProvider).toBe("gemini") + expect(effectiveProvider).toBe(providerIdentifiers.gemini) }) }) diff --git a/apps/cli/src/types/types.ts b/apps/cli/src/types/types.ts index 0a9f3d2259..999c7b655a 100644 --- a/apps/cli/src/types/types.ts +++ b/apps/cli/src/types/types.ts @@ -1,12 +1,12 @@ -import type { ProviderName, ReasoningEffortExtended } from "@roo-code/types" +import { providerIdentifiers, type ProviderName, type ReasoningEffortExtended } from "@roo-code/types" import type { OutputFormat } from "./json-events.js" export const supportedProviders = [ - "anthropic", - "openai-native", - "gemini", - "openrouter", - "vercel-ai-gateway", + providerIdentifiers.anthropic, + providerIdentifiers.openaiNative, + providerIdentifiers.gemini, + providerIdentifiers.openrouter, + providerIdentifiers.vercelAiGateway, ] as const satisfies ProviderName[] export type SupportedProvider = (typeof supportedProviders)[number] diff --git a/packages/types/src/__tests__/provider-identifiers.test.ts b/packages/types/src/__tests__/provider-identifiers.test.ts index b3640a8f5d..870ce77d78 100644 --- a/packages/types/src/__tests__/provider-identifiers.test.ts +++ b/packages/types/src/__tests__/provider-identifiers.test.ts @@ -11,6 +11,7 @@ import { isProviderName, isRetiredProvider, localProviders, + MODELS_BY_PROVIDER, providerIdentifiers, providerNames, providerNamesSchema, @@ -113,6 +114,12 @@ describe("provider identifiers", () => { expect(fauxProviders).toEqual([providerIdentifiers.fakeAi]) }) + it("keeps model provider ids aligned with their keys", () => { + for (const [identifier, providerModels] of Object.entries(MODELS_BY_PROVIDER)) { + expect(providerModels.id).toBe(identifier) + } + }) + it("preserves provider category type guards", () => { for (const identifier of dynamicProviders) { expect(isDynamicProvider(identifier)).toBe(true) diff --git a/packages/types/src/provider-settings.ts b/packages/types/src/provider-settings.ts index e17cd5ddbc..99b75de2e4 100644 --- a/packages/types/src/provider-settings.ts +++ b/packages/types/src/provider-settings.ts @@ -428,40 +428,40 @@ const defaultSchema = z.object({ }) export const providerSettingsSchemaDiscriminated = z.discriminatedUnion("apiProvider", [ - anthropicSchema.merge(z.object({ apiProvider: z.literal("anthropic") })), - openRouterSchema.merge(z.object({ apiProvider: z.literal("openrouter") })), - bedrockSchema.merge(z.object({ apiProvider: z.literal("bedrock") })), - vertexSchema.merge(z.object({ apiProvider: z.literal("vertex") })), - openAiSchema.merge(z.object({ apiProvider: z.literal("openai") })), - ollamaSchema.merge(z.object({ apiProvider: z.literal("ollama") })), - vsCodeLmSchema.merge(z.object({ apiProvider: z.literal("vscode-lm") })), - lmStudioSchema.merge(z.object({ apiProvider: z.literal("lmstudio") })), - geminiSchema.merge(z.object({ apiProvider: z.literal("gemini") })), - geminiCliSchema.merge(z.object({ apiProvider: z.literal("gemini-cli") })), - openAiCodexSchema.merge(z.object({ apiProvider: z.literal("openai-codex") })), - openAiNativeSchema.merge(z.object({ apiProvider: z.literal("openai-native") })), - mistralSchema.merge(z.object({ apiProvider: z.literal("mistral") })), - deepSeekSchema.merge(z.object({ apiProvider: z.literal("deepseek") })), - poeSchema.merge(z.object({ apiProvider: z.literal("poe") })), - moonshotSchema.merge(z.object({ apiProvider: z.literal("moonshot") })), - kimiCodeSchema.merge(z.object({ apiProvider: z.literal("kimi-code") })), - minimaxSchema.merge(z.object({ apiProvider: z.literal("minimax") })), - mimoSchema.merge(z.object({ apiProvider: z.literal("mimo") })), - requestySchema.merge(z.object({ apiProvider: z.literal("requesty") })), - unboundSchema.merge(z.object({ apiProvider: z.literal("unbound") })), - fakeAiSchema.merge(z.object({ apiProvider: z.literal("fake-ai") })), - xaiSchema.merge(z.object({ apiProvider: z.literal("xai") })), - basetenSchema.merge(z.object({ apiProvider: z.literal("baseten") })), - litellmSchema.merge(z.object({ apiProvider: z.literal("litellm") })), - sambaNovaSchema.merge(z.object({ apiProvider: z.literal("sambanova") })), - zaiSchema.merge(z.object({ apiProvider: z.literal("zai") })), - fireworksSchema.merge(z.object({ apiProvider: z.literal("fireworks") })), - friendliSchema.merge(z.object({ apiProvider: z.literal("friendli") })), - qwenCodeSchema.merge(z.object({ apiProvider: z.literal("qwen-code") })), - vercelAiGatewaySchema.merge(z.object({ apiProvider: z.literal("vercel-ai-gateway") })), - opencodeGoSchema.merge(z.object({ apiProvider: z.literal("opencode-go") })), - kenariSchema.merge(z.object({ apiProvider: z.literal("kenari") })), - zooGatewaySchema.merge(z.object({ apiProvider: z.literal("zoo-gateway") })), + anthropicSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.anthropic) })), + openRouterSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.openrouter) })), + bedrockSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.bedrock) })), + vertexSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.vertex) })), + openAiSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.openai) })), + ollamaSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.ollama) })), + vsCodeLmSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.vscodeLm) })), + lmStudioSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.lmstudio) })), + geminiSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.gemini) })), + geminiCliSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.geminiCli) })), + openAiCodexSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.openaiCodex) })), + openAiNativeSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.openaiNative) })), + mistralSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.mistral) })), + deepSeekSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.deepseek) })), + poeSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.poe) })), + moonshotSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.moonshot) })), + kimiCodeSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.kimiCode) })), + minimaxSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.minimax) })), + mimoSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.mimo) })), + requestySchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.requesty) })), + unboundSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.unbound) })), + fakeAiSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.fakeAi) })), + xaiSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.xai) })), + basetenSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.baseten) })), + litellmSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.litellm) })), + sambaNovaSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.sambanova) })), + zaiSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.zai) })), + fireworksSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.fireworks) })), + friendliSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.friendli) })), + qwenCodeSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.qwenCode) })), + vercelAiGatewaySchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.vercelAiGateway) })), + opencodeGoSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.opencodeGo) })), + kenariSchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.kenari) })), + zooGatewaySchema.merge(z.object({ apiProvider: z.literal(providerIdentifiers.zooGateway) })), defaultSchema, ]) @@ -553,37 +553,37 @@ export const isTypicalProvider = (key: unknown): key is TypicalProvider => isProviderName(key) && !isInternalProvider(key) && !isCustomProvider(key) && !isFauxProvider(key) export const modelIdKeysByProvider: Record = { - anthropic: "apiModelId", - openrouter: "openRouterModelId", - bedrock: "apiModelId", - vertex: "apiModelId", - "openai-codex": "apiModelId", - "openai-native": "openAiModelId", - ollama: "ollamaModelId", - lmstudio: "lmStudioModelId", - gemini: "apiModelId", - "gemini-cli": "apiModelId", - mistral: "apiModelId", - moonshot: "apiModelId", - "kimi-code": "apiModelId", - minimax: "apiModelId", - mimo: "apiModelId", - deepseek: "apiModelId", - poe: "apiModelId", - "qwen-code": "apiModelId", - requesty: "requestyModelId", - unbound: "unboundModelId", - xai: "apiModelId", - baseten: "apiModelId", - litellm: "litellmModelId", - sambanova: "apiModelId", - zai: "apiModelId", - fireworks: "apiModelId", - friendli: "apiModelId", - "vercel-ai-gateway": "vercelAiGatewayModelId", - "opencode-go": "opencodeGoModelId", - kenari: "kenariModelId", - "zoo-gateway": "zooGatewayModelId", + [providerIdentifiers.anthropic]: "apiModelId", + [providerIdentifiers.openrouter]: "openRouterModelId", + [providerIdentifiers.bedrock]: "apiModelId", + [providerIdentifiers.vertex]: "apiModelId", + [providerIdentifiers.openaiCodex]: "apiModelId", + [providerIdentifiers.openaiNative]: "openAiModelId", + [providerIdentifiers.ollama]: "ollamaModelId", + [providerIdentifiers.lmstudio]: "lmStudioModelId", + [providerIdentifiers.gemini]: "apiModelId", + [providerIdentifiers.geminiCli]: "apiModelId", + [providerIdentifiers.mistral]: "apiModelId", + [providerIdentifiers.moonshot]: "apiModelId", + [providerIdentifiers.kimiCode]: "apiModelId", + [providerIdentifiers.minimax]: "apiModelId", + [providerIdentifiers.mimo]: "apiModelId", + [providerIdentifiers.deepseek]: "apiModelId", + [providerIdentifiers.poe]: "apiModelId", + [providerIdentifiers.qwenCode]: "apiModelId", + [providerIdentifiers.requesty]: "requestyModelId", + [providerIdentifiers.unbound]: "unboundModelId", + [providerIdentifiers.xai]: "apiModelId", + [providerIdentifiers.baseten]: "apiModelId", + [providerIdentifiers.litellm]: "litellmModelId", + [providerIdentifiers.sambanova]: "apiModelId", + [providerIdentifiers.zai]: "apiModelId", + [providerIdentifiers.fireworks]: "apiModelId", + [providerIdentifiers.friendli]: "apiModelId", + [providerIdentifiers.vercelAiGateway]: "vercelAiGatewayModelId", + [providerIdentifiers.opencodeGo]: "opencodeGoModelId", + [providerIdentifiers.kenari]: "kenariModelId", + [providerIdentifiers.zooGateway]: "zooGatewayModelId", } /** @@ -653,106 +653,125 @@ export const getApiProtocol = (provider: ProviderName | undefined, modelId?: str */ export const MODELS_BY_PROVIDER: Record< - Exclude, + Exclude< + ProviderName, + typeof providerIdentifiers.fakeAi | typeof providerIdentifiers.geminiCli | typeof providerIdentifiers.openai + >, { id: ProviderName; label: string; models: string[] } > = { - anthropic: { - id: "anthropic", + [providerIdentifiers.anthropic]: { + id: providerIdentifiers.anthropic, label: "Anthropic", models: Object.keys(anthropicModels), }, - bedrock: { - id: "bedrock", + [providerIdentifiers.bedrock]: { + id: providerIdentifiers.bedrock, label: "Amazon Bedrock", models: Object.keys(bedrockModels), }, - deepseek: { - id: "deepseek", + [providerIdentifiers.deepseek]: { + id: providerIdentifiers.deepseek, label: "DeepSeek", models: Object.keys(deepSeekModels), }, - fireworks: { - id: "fireworks", + [providerIdentifiers.fireworks]: { + id: providerIdentifiers.fireworks, label: "Fireworks", models: Object.keys(fireworksModels), }, - friendli: { - id: "friendli", + [providerIdentifiers.friendli]: { + id: providerIdentifiers.friendli, label: "Friendli", models: Object.keys(friendliModels), }, - gemini: { - id: "gemini", + [providerIdentifiers.gemini]: { + id: providerIdentifiers.gemini, label: "Google Gemini", models: Object.keys(geminiModels), }, - mistral: { - id: "mistral", + [providerIdentifiers.mistral]: { + id: providerIdentifiers.mistral, label: "Mistral", models: Object.keys(mistralModels), }, - moonshot: { - id: "moonshot", + [providerIdentifiers.moonshot]: { + id: providerIdentifiers.moonshot, label: "Moonshot", models: Object.keys(moonshotModels), }, - "kimi-code": { - id: "kimi-code", + [providerIdentifiers.kimiCode]: { + id: providerIdentifiers.kimiCode, label: "Kimi Code", models: [], }, - minimax: { - id: "minimax", + [providerIdentifiers.minimax]: { + id: providerIdentifiers.minimax, label: "MiniMax", models: Object.keys(minimaxModels), }, - mimo: { - id: "mimo", + [providerIdentifiers.mimo]: { + id: providerIdentifiers.mimo, label: "Xiaomi MiMo", models: Object.keys(mimoModels), }, - "openai-codex": { - id: "openai-codex", + [providerIdentifiers.openaiCodex]: { + id: providerIdentifiers.openaiCodex, label: "OpenAI - ChatGPT Plus/Pro", models: Object.keys(openAiCodexModels), }, - "openai-native": { - id: "openai-native", + [providerIdentifiers.openaiNative]: { + id: providerIdentifiers.openaiNative, label: "OpenAI", models: Object.keys(openAiNativeModels), }, - "qwen-code": { id: "qwen-code", label: "Qwen Code", models: Object.keys(qwenCodeModels) }, - sambanova: { - id: "sambanova", + [providerIdentifiers.qwenCode]: { + id: providerIdentifiers.qwenCode, + label: "Qwen Code", + models: Object.keys(qwenCodeModels), + }, + [providerIdentifiers.sambanova]: { + id: providerIdentifiers.sambanova, label: "SambaNova", models: Object.keys(sambaNovaModels), }, - vertex: { - id: "vertex", + [providerIdentifiers.vertex]: { + id: providerIdentifiers.vertex, label: "GCP Vertex AI", models: Object.keys(vertexModels), }, - "vscode-lm": { - id: "vscode-lm", + [providerIdentifiers.vscodeLm]: { + id: providerIdentifiers.vscodeLm, label: "VS Code LM API", models: Object.keys(vscodeLlmModels), }, - xai: { id: "xai", label: "xAI (Grok)", models: Object.keys(xaiModels) }, - zai: { id: "zai", label: "Z.ai", models: Object.keys(internationalZAiModels) }, - baseten: { id: "baseten", label: "Baseten", models: Object.keys(basetenModels) }, + [providerIdentifiers.xai]: { id: providerIdentifiers.xai, label: "xAI (Grok)", models: Object.keys(xaiModels) }, + [providerIdentifiers.zai]: { + id: providerIdentifiers.zai, + label: "Z.ai", + models: Object.keys(internationalZAiModels), + }, + [providerIdentifiers.baseten]: { + id: providerIdentifiers.baseten, + label: "Baseten", + models: Object.keys(basetenModels), + }, // Dynamic providers; models pulled from remote APIs. - poe: { id: "poe", label: "Poe", models: [] }, - litellm: { id: "litellm", label: "LiteLLM", models: [] }, - openrouter: { id: "openrouter", label: "OpenRouter", models: [] }, - requesty: { id: "requesty", label: "Requesty", models: [] }, - unbound: { id: "unbound", label: "Unbound", models: [] }, - "vercel-ai-gateway": { id: "vercel-ai-gateway", label: "Vercel AI Gateway", models: [] }, - "opencode-go": { id: "opencode-go", label: "Opencode Go", models: [] }, - kenari: { id: "kenari", label: "Kenari", models: [] }, - "zoo-gateway": { id: "zoo-gateway", label: "Zoo Gateway", models: [] }, + [providerIdentifiers.poe]: { id: providerIdentifiers.poe, label: "Poe", models: [] }, + [providerIdentifiers.litellm]: { id: providerIdentifiers.litellm, label: "LiteLLM", models: [] }, + [providerIdentifiers.openrouter]: { id: providerIdentifiers.openrouter, label: "OpenRouter", models: [] }, + [providerIdentifiers.requesty]: { id: providerIdentifiers.requesty, label: "Requesty", models: [] }, + [providerIdentifiers.unbound]: { id: providerIdentifiers.unbound, label: "Unbound", models: [] }, + [providerIdentifiers.vercelAiGateway]: { + id: providerIdentifiers.vercelAiGateway, + label: "Vercel AI Gateway", + models: [], + }, + [providerIdentifiers.opencodeGo]: { id: providerIdentifiers.opencodeGo, label: "Opencode Go", models: [] }, + [providerIdentifiers.kenari]: { id: providerIdentifiers.kenari, label: "Kenari", models: [] }, + [providerIdentifiers.zooGateway]: { id: providerIdentifiers.zooGateway, label: "Zoo Gateway", models: [] }, // Local providers; models discovered from localhost endpoints. - lmstudio: { id: "lmstudio", label: "LM Studio", models: [] }, - ollama: { id: "ollama", label: "Ollama", models: [] }, + [providerIdentifiers.lmstudio]: { id: providerIdentifiers.lmstudio, label: "LM Studio", models: [] }, + [providerIdentifiers.ollama]: { id: providerIdentifiers.ollama, label: "Ollama", models: [] }, } From d33e40d0b23db9da1150d9df71239b81f9479140 Mon Sep 17 00:00:00 2001 From: "zoomote[bot]" <305051434+zoomote[bot]@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:40:19 -0400 Subject: [PATCH 6/8] [Refactor] Reuse shared API options in provider tests (#1178) * refactor: reuse shared API test options * chore: rerun CI --------- Co-authored-by: Roomote --- .../providers/__tests__/openrouter.spec.ts | 72 ++++++++++------- src/api/providers/__tests__/requesty.spec.ts | 78 +++++++++++-------- .../__tests__/vercel-ai-gateway.spec.ts | 78 +++++++++++-------- 3 files changed, 133 insertions(+), 95 deletions(-) diff --git a/src/api/providers/__tests__/openrouter.spec.ts b/src/api/providers/__tests__/openrouter.spec.ts index 254cd1dad4..5636132a50 100644 --- a/src/api/providers/__tests__/openrouter.spec.ts +++ b/src/api/providers/__tests__/openrouter.spec.ts @@ -18,8 +18,8 @@ import { Anthropic } from "@anthropic-ai/sdk" import OpenAI from "openai" import { OpenRouterHandler } from "../openrouter" -import { ApiHandlerOptions } from "../../../shared/api" import { Package } from "../../../shared/package" +import { makeApiHandlerOptions } from "../../../test-utils/api" import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" vitest.mock("openai") @@ -102,10 +102,10 @@ vitest.mock("../fetchers/modelCache", () => ({ })) describe("OpenRouterHandler", () => { - const mockOptions: ApiHandlerOptions = { + const mockOptions = makeApiHandlerOptions({ openRouterApiKey: "test-key", openRouterModelId: "anthropic/claude-sonnet-4", - } + }) beforeEach(() => vitest.clearAllMocks()) @@ -147,12 +147,14 @@ describe("OpenRouterHandler", () => { }) it("honors custom maxTokens for thinking models", async () => { - const handler = new OpenRouterHandler({ - openRouterApiKey: "test-key", - openRouterModelId: "anthropic/claude-3.7-sonnet:thinking", - modelMaxTokens: 32_768, - modelMaxThinkingTokens: 16_384, - }) + const handler = new OpenRouterHandler( + makeApiHandlerOptions({ + openRouterApiKey: "test-key", + openRouterModelId: "anthropic/claude-3.7-sonnet:thinking", + modelMaxTokens: 32_768, + modelMaxThinkingTokens: 16_384, + }), + ) const result = await handler.fetchModel() // With the new clamping logic, 128000 tokens (64% of 200000 context window) @@ -163,11 +165,13 @@ describe("OpenRouterHandler", () => { }) it("does not honor custom maxTokens for non-thinking models", async () => { - const handler = new OpenRouterHandler({ - ...mockOptions, - modelMaxTokens: 32_768, - modelMaxThinkingTokens: 16_384, - }) + const handler = new OpenRouterHandler( + makeApiHandlerOptions({ + ...mockOptions, + modelMaxTokens: 32_768, + modelMaxThinkingTokens: 16_384, + }), + ) const result = await handler.fetchModel() expect(result.maxTokens).toBe(8192) @@ -176,10 +180,12 @@ describe("OpenRouterHandler", () => { }) it("adds excludedTools and includedTools for OpenAI models", async () => { - const handler = new OpenRouterHandler({ - openRouterApiKey: "test-key", - openRouterModelId: "openai/gpt-4o", - }) + const handler = new OpenRouterHandler( + makeApiHandlerOptions({ + openRouterApiKey: "test-key", + openRouterModelId: "openai/gpt-4o", + }), + ) const result = await handler.fetchModel() expect(result.id).toBe("openai/gpt-4o") @@ -189,10 +195,12 @@ describe("OpenRouterHandler", () => { }) it("merges excludedTools and includedTools with existing values for OpenAI models", async () => { - const handler = new OpenRouterHandler({ - openRouterApiKey: "test-key", - openRouterModelId: "openai/o1", - }) + const handler = new OpenRouterHandler( + makeApiHandlerOptions({ + openRouterApiKey: "test-key", + openRouterModelId: "openai/o1", + }), + ) const result = await handler.fetchModel() expect(result.id).toBe("openai/o1") @@ -208,10 +216,12 @@ describe("OpenRouterHandler", () => { }) it("does not add excludedTools or includedTools for non-OpenAI models", async () => { - const handler = new OpenRouterHandler({ - openRouterApiKey: "test-key", - openRouterModelId: "anthropic/claude-sonnet-4", - }) + const handler = new OpenRouterHandler( + makeApiHandlerOptions({ + openRouterApiKey: "test-key", + openRouterModelId: "anthropic/claude-sonnet-4", + }), + ) const result = await handler.fetchModel() expect(result.id).toBe("anthropic/claude-sonnet-4") @@ -281,10 +291,12 @@ describe("OpenRouterHandler", () => { }) it("adds cache control for supported models", async () => { - const handler = new OpenRouterHandler({ - ...mockOptions, - openRouterModelId: "anthropic/claude-3.5-sonnet", - }) + const handler = new OpenRouterHandler( + makeApiHandlerOptions({ + ...mockOptions, + openRouterModelId: "anthropic/claude-3.5-sonnet", + }), + ) const mockStream = asyncStreamFrom([ { diff --git a/src/api/providers/__tests__/requesty.spec.ts b/src/api/providers/__tests__/requesty.spec.ts index 77adb8724f..3c56f1bc59 100644 --- a/src/api/providers/__tests__/requesty.spec.ts +++ b/src/api/providers/__tests__/requesty.spec.ts @@ -10,9 +10,9 @@ import { Anthropic } from "@anthropic-ai/sdk" import OpenAI from "openai" import { RequestyHandler } from "../requesty" -import { ApiHandlerOptions } from "../../../shared/api" import { Package } from "../../../shared/package" import { ApiHandlerCreateMessageMetadata } from "../../index" +import { makeApiHandlerOptions } from "../../../test-utils/api" import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" const mockCreate = vitest.fn() @@ -98,10 +98,10 @@ vitest.mock("../fetchers/modelCache", () => ({ })) describe("RequestyHandler", () => { - const mockOptions: ApiHandlerOptions = { + const mockOptions = makeApiHandlerOptions({ requestyApiKey: "test-key", requestyModelId: "coding/claude-4-sonnet", - } + }) beforeEach(() => vitest.clearAllMocks()) @@ -244,12 +244,14 @@ describe("RequestyHandler", () => { }) it("uses adaptive thinking for Claude Fable 5 when reasoning is enabled", async () => { - const handler = new RequestyHandler({ - requestyApiKey: "test-key", - requestyModelId: "anthropic/claude-fable-5", - enableReasoningEffort: true, - modelMaxTokens: 32768, - }) + const handler = new RequestyHandler( + makeApiHandlerOptions({ + requestyApiKey: "test-key", + requestyModelId: "anthropic/claude-fable-5", + enableReasoningEffort: true, + modelMaxTokens: 32768, + }), + ) const mockStream = asyncStreamFrom([ { @@ -275,12 +277,14 @@ describe("RequestyHandler", () => { }) it("uses adaptive thinking for Claude Sonnet 5 when reasoning is enabled", async () => { - const handler = new RequestyHandler({ - requestyApiKey: "test-key", - requestyModelId: "anthropic/claude-sonnet-5", - enableReasoningEffort: true, - modelMaxTokens: 32768, - }) + const handler = new RequestyHandler( + makeApiHandlerOptions({ + requestyApiKey: "test-key", + requestyModelId: "anthropic/claude-sonnet-5", + enableReasoningEffort: true, + modelMaxTokens: 32768, + }), + ) const mockStream = asyncStreamFrom([ { @@ -306,12 +310,14 @@ describe("RequestyHandler", () => { }) it("uses adaptive thinking for Claude Opus 5 when reasoning is enabled", async () => { - const handler = new RequestyHandler({ - requestyApiKey: "test-key", - requestyModelId: "anthropic/claude-opus-5", - enableReasoningEffort: true, - modelMaxTokens: 32768, - }) + const handler = new RequestyHandler( + makeApiHandlerOptions({ + requestyApiKey: "test-key", + requestyModelId: "anthropic/claude-opus-5", + enableReasoningEffort: true, + modelMaxTokens: 32768, + }), + ) const mockStream = asyncStreamFrom([ { @@ -574,10 +580,12 @@ describe("RequestyHandler", () => { }) it("omits temperature for Claude Fable 5 in completePrompt", async () => { - const handler = new RequestyHandler({ - requestyApiKey: "test-key", - requestyModelId: "anthropic/claude-fable-5", - }) + const handler = new RequestyHandler( + makeApiHandlerOptions({ + requestyApiKey: "test-key", + requestyModelId: "anthropic/claude-fable-5", + }), + ) mockCreate.mockResolvedValue({ choices: [{ message: { content: "test completion" } }] }) await handler.completePrompt("test prompt") @@ -591,10 +599,12 @@ describe("RequestyHandler", () => { }) it("omits temperature for Claude Sonnet 5 in completePrompt", async () => { - const handler = new RequestyHandler({ - requestyApiKey: "test-key", - requestyModelId: "anthropic/claude-sonnet-5", - }) + const handler = new RequestyHandler( + makeApiHandlerOptions({ + requestyApiKey: "test-key", + requestyModelId: "anthropic/claude-sonnet-5", + }), + ) mockCreate.mockResolvedValue({ choices: [{ message: { content: "test completion" } }] }) await handler.completePrompt("test prompt") @@ -608,10 +618,12 @@ describe("RequestyHandler", () => { }) it("omits temperature for Claude Opus 5 in completePrompt", async () => { - const handler = new RequestyHandler({ - requestyApiKey: "test-key", - requestyModelId: "anthropic/claude-opus-5", - }) + const handler = new RequestyHandler( + makeApiHandlerOptions({ + requestyApiKey: "test-key", + requestyModelId: "anthropic/claude-opus-5", + }), + ) mockCreate.mockResolvedValue({ choices: [{ message: { content: "test completion" } }] }) await handler.completePrompt("test prompt") diff --git a/src/api/providers/__tests__/vercel-ai-gateway.spec.ts b/src/api/providers/__tests__/vercel-ai-gateway.spec.ts index 92cc785951..57fbea18c0 100644 --- a/src/api/providers/__tests__/vercel-ai-gateway.spec.ts +++ b/src/api/providers/__tests__/vercel-ai-gateway.spec.ts @@ -13,7 +13,7 @@ import { Anthropic } from "@anthropic-ai/sdk" import OpenAI from "openai" import { VercelAiGatewayHandler } from "../vercel-ai-gateway" -import { ApiHandlerOptions } from "../../../shared/api" +import { makeApiHandlerOptions } from "../../../test-utils/api" import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" import { vercelAiGatewayDefaultModelId, VERCEL_AI_GATEWAY_DEFAULT_TEMPERATURE } from "@roo-code/types" @@ -126,10 +126,10 @@ const mockConstructor = vitest.fn() }) describe("VercelAiGatewayHandler", () => { - const mockOptions: ApiHandlerOptions = { + const mockOptions = makeApiHandlerOptions({ vercelAiGatewayApiKey: "test-key", vercelAiGatewayModelId: "anthropic/claude-sonnet-4", - } + }) beforeEach(() => { vitest.clearAllMocks() @@ -270,10 +270,12 @@ describe("VercelAiGatewayHandler", () => { it("uses correct temperature from options", async () => { const customTemp = 0.5 - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - modelTemperature: customTemp, - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + modelTemperature: customTemp, + }), + ) const systemPrompt = "You are a helpful assistant." const messages: Anthropic.Messages.MessageParam[] = [{ role: "user", content: "Hello" }] @@ -303,10 +305,12 @@ describe("VercelAiGatewayHandler", () => { }) it("omits temperature for Claude Fable 5", async () => { - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - vercelAiGatewayModelId: "anthropic/claude-fable-5", - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + vercelAiGatewayModelId: "anthropic/claude-fable-5", + }), + ) await handler.createMessage("You are a helpful assistant.", [{ role: "user", content: "Hello" }]).next() @@ -320,10 +324,12 @@ describe("VercelAiGatewayHandler", () => { }) it("omits temperature for Claude Sonnet 5", async () => { - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - vercelAiGatewayModelId: "anthropic/claude-sonnet-5", - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + vercelAiGatewayModelId: "anthropic/claude-sonnet-5", + }), + ) await handler.createMessage("You are a helpful assistant.", [{ role: "user", content: "Hello" }]).next() @@ -338,10 +344,12 @@ describe("VercelAiGatewayHandler", () => { }) it("omits temperature for Claude Opus 5", async () => { - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - vercelAiGatewayModelId: "anthropic/claude-opus-5", - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + vercelAiGatewayModelId: "anthropic/claude-opus-5", + }), + ) await handler.createMessage("You are a helpful assistant.", [{ role: "user", content: "Hello" }]).next() @@ -357,10 +365,12 @@ describe("VercelAiGatewayHandler", () => { it("adds cache breakpoints for supported models", async () => { const { addCacheBreakpoints } = await import("../../transform/caching/vercel-ai-gateway") - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - vercelAiGatewayModelId: "anthropic/claude-3.5-haiku", - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + vercelAiGatewayModelId: "anthropic/claude-3.5-haiku", + }), + ) const systemPrompt = "You are a helpful assistant." const messages: Anthropic.Messages.MessageParam[] = [{ role: "user", content: "Hello" }] @@ -647,10 +657,12 @@ describe("VercelAiGatewayHandler", () => { it("uses custom temperature for completion", async () => { const customTemp = 0.8 - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - modelTemperature: customTemp, - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + modelTemperature: customTemp, + }), + ) await handler.completePrompt("Test prompt") @@ -694,11 +706,13 @@ describe("VercelAiGatewayHandler", () => { describe("temperature support", () => { it("applies temperature for supported models", async () => { - const handler = new VercelAiGatewayHandler({ - ...mockOptions, - vercelAiGatewayModelId: "anthropic/claude-sonnet-4", - modelTemperature: 0.9, - }) + const handler = new VercelAiGatewayHandler( + makeApiHandlerOptions({ + ...mockOptions, + vercelAiGatewayModelId: "anthropic/claude-sonnet-4", + modelTemperature: 0.9, + }), + ) await handler.completePrompt("Test") From 95cdd43fec39950509903cce9fc84b0e5c2f1fd8 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 8 Aug 2026 07:37:06 +0900 Subject: [PATCH 7/8] test(e2e): add error-interception integration suite --- .../error-interception-integration.test.ts | 331 ++++++++++++++++++ 1 file changed, 331 insertions(+) create mode 100644 apps/vscode-e2e/src/suite/error-interception-integration.test.ts diff --git a/apps/vscode-e2e/src/suite/error-interception-integration.test.ts b/apps/vscode-e2e/src/suite/error-interception-integration.test.ts new file mode 100644 index 0000000000..1c8c1bdb6f --- /dev/null +++ b/apps/vscode-e2e/src/suite/error-interception-integration.test.ts @@ -0,0 +1,331 @@ +import * as assert from "assert" +import * as path from "path" +import * as fs from "fs" + +import { setDefaultSuiteTimeout } from "./test-utils" + +// --------------------------------------------------------------------------- +// Error Interception — assistant-message integration at e2e scope +// --------------------------------------------------------------------------- +// +// This suite exercises the Assistant Integration & Handlers layer shipped by +// this PR (structuredError.ts + the presentAssistantMessage.ts handleError +// wiring) against the real, built extension artifact, not a re-implemented +// copy. +// +// Why this lives in apps/vscode-e2e and not in src/__tests__: +// - The unit specs (structuredError.spec.ts, presentAssistantMessage-handleError.spec.ts) +// run under Vitest with direct TS source access. They prove the formatter +// and the handleError closures in isolation, with the Task graph mocked. +// - This e2e suite runs inside the real VS Code extension host against the +// bundled extension output that actually ships. It proves the integration +// contract (structured error shape, retryability signals, occurrence +// tracking, WHAT/WHY/NEXT guidance) survives bundling and is importable +// end-to-end. +// +// How the module is loaded: +// The e2e workspace does not use TS project references into src/, so a +// static import would fail `check-types`. Instead we locate the built +// extension entry (dist/extension.js, produced by `pnpm -w bundle` in the +// test:ci pipeline) and require the structuredError submodule from the same +// output the host loads. If the bundle is absent (e.g. a bare `check-types` +// run without a build), the suite skips cleanly rather than failing on an +// infrastructure gap. + +interface StructuredErrorDetailsLike { + what: string + why: string + next: string[] + retryable?: boolean + pattern?: string + occurrence?: number + disposition?: string +} + +interface StructuredErrorModule { + isUserRejectionError: (error: Error) => boolean + isRetryableError: (error: Error) => boolean + deriveRecoveryDisposition: (error: Error, occurrence: number) => string + buildErrorSignature: (action: string, error: Error) => string + recordErrorOccurrence: (task: object, signature: string) => number + formatStructuredError: (details: StructuredErrorDetailsLike, byteLimit?: number) => string + buildStructuredErrorContent: (task: object, action: string, error: Error, pattern: string) => string + formatConciseErrorMessage: (action: string, error: Error) => string +} + +function findBuiltExtensionEntry(workspaceRoot: string): string | undefined { + const candidates = [ + path.join(workspaceRoot, "src", "dist", "extension.js"), + path.join(workspaceRoot, "dist", "extension.js"), + path.join(workspaceRoot, "src", "dist", "extension.cjs"), + ] + return candidates.find((p) => fs.existsSync(p)) +} + +/** Extracts the JSON payload from an block. */ +function parseErrorDetails(block: string): Record { + const match = block.match(/^\n([\s\S]*)\n<\/error_details>$/) + assert.ok(match && match[1] !== undefined, `expected an block, got: ${block.slice(0, 120)}`) + return JSON.parse(match[1]) as Record +} + +suite("Error Interception — Integration (e2e)", function () { + setDefaultSuiteTimeout(this) + + let se: StructuredErrorModule | undefined + let bundleAvailable = false + + suiteSetup(function () { + // __dirname = apps/vscode-e2e/out/suite at runtime. + const workspaceRoot = path.resolve(__dirname, "..", "..", "..") + const entry = findBuiltExtensionEntry(workspaceRoot) + + if (!entry) { + // The bundled extension is not present (no `pnpm -w bundle` run). + // This is an environment gap, not a contract regression — skip. + console.warn( + "[error-interception-integration e2e] built extension bundle not found; " + + "run `pnpm -w bundle` before `test:run` to enable this suite.", + ) + return + } + + // Load the structuredError module from the built bundle. The bundle + // exposes its internal modules via a loader keyed by module path; we + // resolve the exact submodule so we test the real artifact. + // eslint-disable-next-line @typescript-eslint/no-var-requires + const bundle = require(entry) as { __structuredError?: StructuredErrorModule } & Record + + // Prefer an explicit re-export if the bundle surfaces one; otherwise + // fall back to a deep-require of the submodule path within the bundle. + if (bundle.__structuredError) { + se = bundle.__structuredError + } else { + const subPath = path.join(workspaceRoot, "src", "dist", "core", "assistant-message", "structuredError.js") + if (fs.existsSync(subPath)) { + // eslint-disable-next-line @typescript-eslint/no-var-requires + se = require(subPath) as StructuredErrorModule + } + } + + bundleAvailable = se !== undefined + if (!bundleAvailable) { + console.warn( + "[error-interception-integration e2e] structuredError module not exposed by the built bundle; " + + "skipping integration assertions.", + ) + } + }) + + setup(function () { + if (!bundleAvailable) { + this.skip() + } + }) + + // ----------------------------------------------------------------------- + // Module surface + // ----------------------------------------------------------------------- + + test("module exposes the structured error integration surface", () => { + assert.strictEqual(typeof se!.isUserRejectionError, "function", "isUserRejectionError must be a function") + assert.strictEqual(typeof se!.isRetryableError, "function", "isRetryableError must be a function") + assert.strictEqual(typeof se!.deriveRecoveryDisposition, "function", "deriveRecoveryDisposition must be a function") + assert.strictEqual(typeof se!.buildErrorSignature, "function", "buildErrorSignature must be a function") + assert.strictEqual(typeof se!.recordErrorOccurrence, "function", "recordErrorOccurrence must be a function") + assert.strictEqual(typeof se!.formatStructuredError, "function", "formatStructuredError must be a function") + assert.strictEqual(typeof se!.buildStructuredErrorContent, "function", "buildStructuredErrorContent must be a function") + assert.strictEqual(typeof se!.formatConciseErrorMessage, "function", "formatConciseErrorMessage must be a function") + }) + + // ----------------------------------------------------------------------- + // Retryability classification + // ----------------------------------------------------------------------- + + test("isRetryableError marks terminal machine-code errors as non-retryable", () => { + const terminal = new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed") + assert.strictEqual(se!.isRetryableError(terminal), false, "TERMINAL/ signal must be non-retryable") + }) + + test("isRetryableError marks validation errors as non-retryable", () => { + const validation = new Error("validation failed: param `command` is required") + assert.strictEqual(se!.isRetryableError(validation), false, "validation failures must be non-retryable") + }) + + test("isRetryableError treats generic runtime errors as retryable", () => { + const runtime = new Error("ENOENT: no such file or directory") + assert.strictEqual(se!.isRetryableError(runtime), true, "generic runtime errors must be retryable") + }) + + test("isUserRejectionError detects user-declined operations", () => { + const rejected = new Error("The edit was rejected by the user") + assert.strictEqual(se!.isUserRejectionError(rejected), true) + assert.strictEqual(se!.isRetryableError(rejected), false, "user rejections must not be retried") + }) + + // ----------------------------------------------------------------------- + // Recovery disposition + // ----------------------------------------------------------------------- + + test("deriveRecoveryDisposition returns await_user for user rejections", () => { + const rejected = new Error("The operation was denied by the user") + assert.strictEqual(se!.deriveRecoveryDisposition(rejected, 1), "await_user") + }) + + test("deriveRecoveryDisposition returns change_strategy for non-retryable errors", () => { + const terminal = new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed") + assert.strictEqual(se!.deriveRecoveryDisposition(terminal, 1), "change_strategy") + }) + + test("deriveRecoveryDisposition returns correct_once for a first retryable failure", () => { + const runtime = new Error("ENOENT: no such file or directory") + assert.strictEqual(se!.deriveRecoveryDisposition(runtime, 1), "correct_once") + }) + + // ----------------------------------------------------------------------- + // Structured error formatting (the model-facing contract) + // ----------------------------------------------------------------------- + + test("formatStructuredError emits a valid JSON block", () => { + const block = se!.formatStructuredError({ + what: "An error occurred during executing command.", + why: "TERMINAL/PROVIDER_SWITCH/003 provider switch failed", + next: ["Do not retry the executing command operation unchanged."], + retryable: false, + pattern: "TERMINAL/PROVIDER_SWITCH/003", + occurrence: 1, + disposition: "change_strategy", + }) + + const payload = parseErrorDetails(block) + assert.strictEqual(payload.status, "error") + assert.strictEqual(payload.retryable, false) + assert.strictEqual(payload.occurrence, 1) + assert.strictEqual(payload.recovery_disposition, "change_strategy") + assert.strictEqual(typeof payload.what, "string") + assert.strictEqual(typeof payload.why, "string") + assert.ok(Array.isArray(payload.next), "next must be an array") + }) + + test("formatStructuredError downgrades pattern slashes to a dotted type discriminator", () => { + const block = se!.formatStructuredError({ + what: "w", + why: "y", + next: ["n"], + pattern: "TOOL_EXECUTION/ERROR_EXECUTION/001", + }) + const payload = parseErrorDetails(block) + assert.strictEqual( + payload.type, + "tool_execution.error_execution.001", + "type must be the dotted lowercase form of the pattern id", + ) + assert.ok(!String(payload.type).includes("/"), "type must not contain slashes") + }) + + test("formatStructuredError truncates to stay within the byte limit while remaining valid JSON", () => { + const longWhy = "x".repeat(5000) + const block = se!.formatStructuredError( + { + what: "An error occurred during executing command.", + why: longWhy, + next: ["first", "second", "third"], + retryable: true, + }, + 1200, + ) + // Must still parse as a well-formed block even under a tight limit. + const payload = parseErrorDetails(block) + assert.strictEqual(payload.status, "error") + assert.ok(block.length <= 1400, `block should be truncated near the limit, got ${block.length}`) + }) + + // ----------------------------------------------------------------------- + // End-to-end flow: tool call → error → classification → guided message + // ----------------------------------------------------------------------- + + test("buildStructuredErrorContent produces occurrence-aware, honest non-retryable guidance", () => { + const task = {} + const error = new Error("TERMINAL/PROVIDER_SWITCH/003 provider switch failed") + + const first = se!.buildStructuredErrorContent(task, "executing command", error, "TERMINAL/PROVIDER_SWITCH/003") + const firstPayload = parseErrorDetails(first) + + assert.strictEqual(firstPayload.retryable, false, "terminal errors must be marked non-retryable") + assert.strictEqual(firstPayload.occurrence, 1, "first failure must report occurrence 1") + assert.strictEqual(firstPayload.recovery_disposition, "change_strategy") + assert.ok( + (firstPayload.next as string[]).some((n) => /do not retry/i.test(n)), + "non-retryable guidance must tell the model not to retry unchanged", + ) + + // The identical failure again must increment the occurrence counter. + const second = se!.buildStructuredErrorContent(task, "executing command", error, "TERMINAL/PROVIDER_SWITCH/003") + const secondPayload = parseErrorDetails(second) + assert.strictEqual(secondPayload.occurrence, 2, "identical repeat failure must report occurrence 2") + }) + + test("buildStructuredErrorContent gives retryable errors corrective guidance", () => { + const task = {} + const error = new Error("ENOENT: no such file or directory") + + const block = se!.buildStructuredErrorContent(task, "reading file", error, "TOOL_EXECUTION/ERROR_EXECUTION/001") + const payload = parseErrorDetails(block) + + assert.strictEqual(payload.retryable, true) + assert.strictEqual(payload.recovery_disposition, "correct_once") + assert.ok( + (payload.next as string[]).some((n) => /retry/i.test(n)), + "retryable guidance must invite a corrected retry", + ) + }) + + test("buildStructuredErrorContent escalates to change_strategy at the stuck-loop threshold", () => { + const task = {} + const error = new Error("ENOENT: no such file or directory") + + // Drive the same signature up to the stuck-loop threshold. + let last = "" + for (let i = 0; i < 3; i++) { + last = se!.buildStructuredErrorContent(task, "reading file", error, "TOOL_EXECUTION/ERROR_EXECUTION/001") + } + const payload = parseErrorDetails(last) + assert.ok( + (payload.occurrence as number) >= 3, + `expected occurrence >= 3 after repeated identical failures, got ${payload.occurrence}`, + ) + assert.strictEqual( + payload.recovery_disposition, + "change_strategy", + "repeated identical retryable failures must escalate to change_strategy", + ) + }) + + // ----------------------------------------------------------------------- + // UI-facing concise message (kept out of the structured payload) + // ----------------------------------------------------------------------- + + test("formatConciseErrorMessage produces a human-readable one-liner", () => { + const msg = se!.formatConciseErrorMessage("executing command", new Error("spawn failed")) + assert.strictEqual(msg, "Error during executing command: spawn failed") + assert.ok(!msg.includes(""), "concise UI message must not embed the structured payload") + }) + + test("formatConciseErrorMessage falls back for empty error messages", () => { + const msg = se!.formatConciseErrorMessage("reading file", new Error("")) + assert.ok(msg.includes("An unexpected error occurred."), "empty messages must get a fallback") + }) + + // ----------------------------------------------------------------------- + // Occurrence signature stability + // ----------------------------------------------------------------------- + + test("buildErrorSignature is stable for identical failures and distinct for different ones", () => { + const a1 = se!.buildErrorSignature("executing command", new Error("boom\nstack line")) + const a2 = se!.buildErrorSignature("executing command", new Error("boom\ndifferent stack")) + const b = se!.buildErrorSignature("reading file", new Error("boom")) + + assert.strictEqual(a1, a2, "same action + same first line must map to the same signature") + assert.notStrictEqual(a1, b, "different actions must produce different signatures") + }) +}) From f69db211c990b91ce978ff64562419d3adce3577 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 8 Aug 2026 14:59:00 +0900 Subject: [PATCH 8/8] fix(e2e): correct eslint-disable rule name in error-interception-integration test (PR #1128) CI failure: Code QA Roo Code run 31224455758 failed @roo-code/vscode-e2e#lint with 2 errors (@typescript-eslint/no-require-imports) and 2 warnings (unused no-var-requires directives). Same legacy-rule-name issue as PR #1126. Run: https://github.com/Zoo-Code-Org/Zoo-Code/actions/runs/31224455758 --- .../src/suite/error-interception-integration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/vscode-e2e/src/suite/error-interception-integration.test.ts b/apps/vscode-e2e/src/suite/error-interception-integration.test.ts index 1c8c1bdb6f..1c11f97f62 100644 --- a/apps/vscode-e2e/src/suite/error-interception-integration.test.ts +++ b/apps/vscode-e2e/src/suite/error-interception-integration.test.ts @@ -93,7 +93,7 @@ suite("Error Interception — Integration (e2e)", function () { // Load the structuredError module from the built bundle. The bundle // exposes its internal modules via a loader keyed by module path; we // resolve the exact submodule so we test the real artifact. - // eslint-disable-next-line @typescript-eslint/no-var-requires + // eslint-disable-next-line @typescript-eslint/no-require-imports const bundle = require(entry) as { __structuredError?: StructuredErrorModule } & Record // Prefer an explicit re-export if the bundle surfaces one; otherwise @@ -103,7 +103,7 @@ suite("Error Interception — Integration (e2e)", function () { } else { const subPath = path.join(workspaceRoot, "src", "dist", "core", "assistant-message", "structuredError.js") if (fs.existsSync(subPath)) { - // eslint-disable-next-line @typescript-eslint/no-var-requires + // eslint-disable-next-line @typescript-eslint/no-require-imports se = require(subPath) as StructuredErrorModule } }