diff --git a/.github/workflows/cli-release.yml b/.github/workflows/cli-release.yml index 2a6a57c6f8..7cd5973703 100644 --- a/.github/workflows/cli-release.yml +++ b/.github/workflows/cli-release.yml @@ -96,13 +96,11 @@ jobs: type: 'module', dependencies: { '@inkjs/ui': pkg.dependencies['@inkjs/ui'], - '@trpc/client': pkg.dependencies['@trpc/client'], 'commander': pkg.dependencies.commander, 'fuzzysort': pkg.dependencies.fuzzysort, 'ink': pkg.dependencies.ink, 'p-wait-for': pkg.dependencies['p-wait-for'], 'react': pkg.dependencies.react, - 'superjson': pkg.dependencies.superjson, 'zustand': pkg.dependencies.zustand } }; diff --git a/.github/workflows/code-qa.yml b/.github/workflows/code-qa.yml index 0f1cf91873..4ef64cadf1 100644 --- a/.github/workflows/code-qa.yml +++ b/.github/workflows/code-qa.yml @@ -76,8 +76,12 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Setup Node.js and pnpm uses: ./.github/actions/setup-node-pnpm + - name: Verify Knip guardrails + run: pnpm test:knip-policy - name: Run knip checks run: pnpm knip + - name: Run production Knip checks + run: pnpm knip:production compile: runs-on: ubuntu-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 01825e27ab..61bb417bf2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,6 +119,7 @@ pnpm install - Follow the ESLint and TypeScript best practices. - Write clear, descriptive commits that reference issues (for example, `Fixes #123`). Follow the [Writing Style](#writing-style). - Test your change fully (`npm test`). +- Run both dead-code checks: `pnpm knip` and `pnpm knip:production`. See [Dead-code checks before review](docs/development-quality-checks.md) for findings, exceptions, and verification. - Rebase onto the latest `main` branch before you submit. ### Submitting a Pull Request diff --git a/apps/cli/package.json b/apps/cli/package.json index 8bc04a5ce9..57beadf3a6 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -26,24 +26,21 @@ "@roo-code/core": "workspace:^", "@roo-code/types": "workspace:^", "@roo-code/vscode-shim": "workspace:^", - "@trpc/client": "^11.18.0", "@vscode/ripgrep": "^1.15.9", "commander": "^12.1.0", - "execa": "^9.5.2", "fuzzysort": "^3.1.0", "ink": "^6.6.0", "p-wait-for": "^5.0.2", "react": "^19.1.0", - "superjson": "^2.2.6", "zustand": "^5.0.14" }, "devDependencies": { "@roo-code/config-eslint": "workspace:^", - "@trpc/server": "11.18.0", "@roo-code/config-typescript": "workspace:^", "@types/node": "22.20.1", "@types/react": "18.3.31", "@vitest/coverage-v8": "4.1.9", + "execa": "^9.5.2", "ink-testing-library": "4.0.0", "rimraf": "6.0.1", "tsup": "8.5.1", diff --git a/apps/cli/scripts/build.sh b/apps/cli/scripts/build.sh index c9c95fe898..a542de1b8f 100755 --- a/apps/cli/scripts/build.sh +++ b/apps/cli/scripts/build.sh @@ -151,13 +151,11 @@ create_tarball() { type: 'module', dependencies: { '@inkjs/ui': pkg.dependencies['@inkjs/ui'], - '@trpc/client': pkg.dependencies['@trpc/client'], 'commander': pkg.dependencies.commander, 'fuzzysort': pkg.dependencies.fuzzysort, 'ink': pkg.dependencies.ink, 'p-wait-for': pkg.dependencies['p-wait-for'], 'react': pkg.dependencies.react, - 'superjson': pkg.dependencies.superjson, 'zustand': pkg.dependencies.zustand } }; diff --git a/apps/cli/src/agent/__tests__/extension-host.test.ts b/apps/cli/src/agent/__tests__/extension-host.test.ts index 5e848e5a7e..82019755a7 100644 --- a/apps/cli/src/agent/__tests__/extension-host.test.ts +++ b/apps/cli/src/agent/__tests__/extension-host.test.ts @@ -35,7 +35,6 @@ function createTestHost({ }: Partial = {}): ExtensionHost { return new ExtensionHost({ mode, - user: null, provider, model, workspacePath: "/test/workspace", @@ -112,7 +111,6 @@ describe("ExtensionHost", () => { mode: "code", workspacePath: "/my/workspace", extensionPath: "/my/extension", - user: null, apiKey: "test-key", provider: providerIdentifiers.openrouter, model: "test-model", diff --git a/apps/cli/src/agent/agent-state.ts b/apps/cli/src/agent/agent-state.ts index d1451d62fd..7105679cc0 100644 --- a/apps/cli/src/agent/agent-state.ts +++ b/apps/cli/src/agent/agent-state.ts @@ -436,28 +436,3 @@ export function detectAgentState(messages: ClineMessage[]): AgentStateInfo { description: getStateDescription(AgentLoopState.RUNNING), } } - -/** - * Quick check: Is the agent waiting for user input? - * - * This is a convenience function for simple use cases where you just need - * to know if user action is required. - */ -export function isAgentWaitingForInput(messages: ClineMessage[]): boolean { - return detectAgentState(messages).isWaitingForInput -} - -/** - * Quick check: Is the agent actively running (not waiting)? - */ -export function isAgentRunning(messages: ClineMessage[]): boolean { - const state = detectAgentState(messages) - return state.isRunning && !state.isWaitingForInput -} - -/** - * Quick check: Is content currently streaming? - */ -export function isContentStreaming(messages: ClineMessage[]): boolean { - return detectAgentState(messages).isStreaming -} diff --git a/apps/cli/src/agent/extension-client.ts b/apps/cli/src/agent/extension-client.ts index c2d77dfdd9..c868663433 100644 --- a/apps/cli/src/agent/extension-client.ts +++ b/apps/cli/src/agent/extension-client.ts @@ -539,21 +539,10 @@ export class ExtensionClient { // Factory Functions // ============================================================================= -/** - * Create a new ExtensionClient instance. - * - * This is a convenience function that creates a client with default settings. - * - * @param sendMessage - Function to send messages to the extension - * @returns A new ExtensionClient instance - */ -export function createClient(sendMessage: (message: WebviewMessage) => void): ExtensionClient { - return new ExtensionClient({ sendMessage }) -} - /** * Create a mock client for testing. * + * @internal * The mock client captures all sent messages for verification. * * @returns An object with the client and captured messages diff --git a/apps/cli/src/agent/extension-host.ts b/apps/cli/src/agent/extension-host.ts index a8c37e2c40..ca8e5b8a10 100644 --- a/apps/cli/src/agent/extension-host.ts +++ b/apps/cli/src/agent/extension-host.ts @@ -27,7 +27,6 @@ import { createVSCodeAPI, IExtensionHost, ExtensionHostEventMap, setRuntimeConfi import { DebugLogger, setDebugLogEnabled } from "@roo-code/core/cli" import { DEFAULT_FLAGS, type SupportedProvider } from "@/types/index.js" -import type { User } from "@/lib/sdk/index.js" import { getProviderSettings } from "@/lib/utils/provider.js" import { createEphemeralStorageDir } from "@/lib/storage/index.js" @@ -67,7 +66,6 @@ export interface ExtensionHostOptions { mode: string reasoningEffort?: ReasoningEffortExtended | "unspecified" | "disabled" consecutiveMistakeLimit?: number - user: User | null provider: SupportedProvider apiKey?: string model: string diff --git a/apps/cli/src/agent/message-processor.ts b/apps/cli/src/agent/message-processor.ts index f841932dcf..5f4e1bff93 100644 --- a/apps/cli/src/agent/message-processor.ts +++ b/apps/cli/src/agent/message-processor.ts @@ -404,29 +404,6 @@ export class MessageProcessor { // Message Validation Helpers // ============================================================================= -/** - * Check if a message is a valid ClineMessage. - * Useful for validating messages before processing. - */ -export function isValidClineMessage(message: unknown): message is ClineMessage { - if (!message || typeof message !== "object") { - return false - } - - const msg = message as Record - - // Required fields - if (typeof msg.ts !== "number") { - return false - } - - if (msg.type !== "ask" && msg.type !== "say") { - return false - } - - return true -} - /** * Check if a message is a valid ExtensionMessage. */ @@ -464,19 +441,3 @@ export function parseExtensionMessage(json: string): ExtensionMessage | undefine return undefined } } - -/** - * Parse the text field of an api_req_started message. - * Returns undefined if parsing fails or text is not present. - */ -export function parseApiReqStartedText(message: ClineMessage): { cost?: number } | undefined { - if (message.say !== "api_req_started" || !message.text) { - return undefined - } - - try { - return JSON.parse(message.text) - } catch { - return undefined - } -} diff --git a/apps/cli/src/agent/state-store.ts b/apps/cli/src/agent/state-store.ts index 68dcfc4069..a57da55a98 100644 --- a/apps/cli/src/agent/state-store.ts +++ b/apps/cli/src/agent/state-store.ts @@ -387,29 +387,3 @@ export class StateStore { // ============================================================================= // Singleton Store (optional convenience) // ============================================================================= - -let defaultStore: StateStore | null = null - -/** - * Get the default singleton store instance. - * Useful for simple applications that don't need multiple stores. - */ -export function getDefaultStore(): StateStore { - if (!defaultStore) { - defaultStore = new StateStore() - } - - return defaultStore -} - -/** - * Reset the default store instance. - * Useful for testing or when you need a fresh start. - */ -export function resetDefaultStore(): void { - if (defaultStore) { - defaultStore.reset() - } - - defaultStore = null -} diff --git a/apps/cli/src/commands/cli/list.ts b/apps/cli/src/commands/cli/list.ts index c5fbb4dba9..1ab08cbf91 100644 --- a/apps/cli/src/commands/cli/list.ts +++ b/apps/cli/src/commands/cli/list.ts @@ -110,7 +110,6 @@ async function createListHost(options: BaseListOptions, hostOptions: ListHostOpt const extensionHostOptions: ExtensionHostOptions = { mode: "code", reasoningEffort: undefined, - user: null, provider: providerIdentifiers.openrouter, model: openRouterDefaultModelId, apiKey, diff --git a/apps/cli/src/commands/cli/run.ts b/apps/cli/src/commands/cli/run.ts index bedb520ed4..5d45e9be6f 100644 --- a/apps/cli/src/commands/cli/run.ts +++ b/apps/cli/src/commands/cli/run.ts @@ -196,7 +196,6 @@ export async function run(promptArg: string | undefined, flagOptions: FlagOption mode: effectiveMode, reasoningEffort: effectiveReasoningEffort === "unspecified" ? undefined : effectiveReasoningEffort, consecutiveMistakeLimit: effectiveConsecutiveMistakeLimit, - user: null, provider: effectiveProvider, model: effectiveModel, workspacePath: effectiveWorkspacePath, diff --git a/apps/cli/src/lib/sdk/client.ts b/apps/cli/src/lib/sdk/client.ts deleted file mode 100644 index ff60e798ef..0000000000 --- a/apps/cli/src/lib/sdk/client.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { createTRPCProxyClient, httpBatchLink } from "@trpc/client" -import superjson from "superjson" - -import type { User, Org } from "./types.js" - -export interface ClientConfig { - url: string - authToken: string -} - -export interface RooClient { - auth: { - me: { - query: () => Promise<{ type: "user"; user: User } | { type: "org"; org: Org } | null> - } - } -} - -export const createClient = ({ url, authToken }: ClientConfig): RooClient => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return createTRPCProxyClient({ - links: [ - httpBatchLink({ - url: `${url}/trpc`, - transformer: superjson, - headers: () => (authToken ? { Authorization: `Bearer ${authToken}` } : {}), - }), - ], - }) as unknown as RooClient -} diff --git a/apps/cli/src/lib/sdk/index.ts b/apps/cli/src/lib/sdk/index.ts deleted file mode 100644 index f45970d7d1..0000000000 --- a/apps/cli/src/lib/sdk/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export * from "./types.js" -export * from "./client.js" diff --git a/apps/cli/src/lib/sdk/types.ts b/apps/cli/src/lib/sdk/types.ts deleted file mode 100644 index 9f0511bb2c..0000000000 --- a/apps/cli/src/lib/sdk/types.ts +++ /dev/null @@ -1,31 +0,0 @@ -export interface User { - id: string - name: string - email: string - imageUrl: string | null - entity: { - id: string - username: string | null - image_url: string - last_name: string - first_name: string - email_addresses: { email_address: string }[] - public_metadata: Record - } - publicMetadata: Record - stripeCustomerId: string | null - lastSyncAt: string - deletedAt: string | null - createdAt: string - updatedAt: string -} - -export interface Org { - id: string - name: string - slug: string - imageUrl: string | null - createdAt: string - updatedAt: string - deletedAt: string | null -} diff --git a/apps/cli/src/lib/storage/credentials.ts b/apps/cli/src/lib/storage/credentials.ts index b687111c16..9b1b355f1e 100644 --- a/apps/cli/src/lib/storage/credentials.ts +++ b/apps/cli/src/lib/storage/credentials.ts @@ -12,6 +12,9 @@ export interface Credentials { orgId?: string } +/** + * @internal + */ export async function saveToken(token: string, options?: { userId?: string; orgId?: string }): Promise { await fs.mkdir(getConfigDir(), { recursive: true }) diff --git a/apps/cli/src/lib/storage/settings.ts b/apps/cli/src/lib/storage/settings.ts index 86a2d9243e..512a4f3b4c 100644 --- a/apps/cli/src/lib/storage/settings.ts +++ b/apps/cli/src/lib/storage/settings.ts @@ -35,6 +35,9 @@ export async function saveSettings(settings: Partial): Promise { await saveSettings({ onboardingProviderChoice: undefined }) } diff --git a/apps/cli/src/lib/task-history/index.ts b/apps/cli/src/lib/task-history/index.ts index 3be2d45d4c..0ef0a3eb8d 100644 --- a/apps/cli/src/lib/task-history/index.ts +++ b/apps/cli/src/lib/task-history/index.ts @@ -7,6 +7,9 @@ import { arePathsEqual } from "@/lib/utils/path.js" const DEFAULT_CLI_TASK_STORAGE_PATH = path.join(os.homedir(), ".vscode-mock", "global-storage") +/** + * @internal + */ export function getDefaultCliTaskStoragePath(): string { return DEFAULT_CLI_TASK_STORAGE_PATH } diff --git a/apps/cli/src/ui/App.tsx b/apps/cli/src/ui/App.tsx index ede7c83170..c00af46683 100644 --- a/apps/cli/src/ui/App.tsx +++ b/apps/cli/src/ui/App.tsx @@ -79,7 +79,6 @@ function AppInner({ createExtensionHost, ...extensionHostOptions }: TUIAppProps) continueSession, workspacePath, extensionPath, - user, provider, apiKey, model, @@ -181,7 +180,6 @@ function AppInner({ createExtensionHost, ...extensionHostOptions }: TUIAppProps) continueSession, mode, reasoningEffort, - user, provider, apiKey, model, diff --git a/apps/cli/src/ui/components/Header.tsx b/apps/cli/src/ui/components/Header.tsx index 040e275918..3ed2a180c8 100644 --- a/apps/cli/src/ui/components/Header.tsx +++ b/apps/cli/src/ui/components/Header.tsx @@ -19,7 +19,6 @@ interface HeaderProps extends ExtensionHostOptions { function Header({ workspacePath, - user, provider, model, mode, @@ -46,7 +45,6 @@ function Header({ {ASCII_ROO} - {user && Welcome back, {user.name}} cwd:{" "} {workspacePath.startsWith(homeDir) ? workspacePath.replace(homeDir, "~") : workspacePath} diff --git a/apps/cli/src/ui/components/Icon.tsx b/apps/cli/src/ui/components/Icon.tsx index 2fdec76a7f..1b9738167a 100644 --- a/apps/cli/src/ui/components/Icon.tsx +++ b/apps/cli/src/ui/components/Icon.tsx @@ -103,6 +103,8 @@ export function isNerdFontSupported(): boolean { /** * Reset the Nerd Font detection cache (useful for testing). + * + * @internal */ export function resetNerdFontCache(): void { nerdFontSupported = null @@ -164,6 +166,8 @@ export function Icon({ name, useNerdFont, width = 2, color, ...textProps }: Icon /** * Get the raw icon character (useful for string concatenation). + * + * @internal */ export function getIconChar(name: IconName, useNerdFont?: boolean): string { const iconDef = ICONS[name] diff --git a/apps/cli/src/ui/components/TodoChangeDisplay.tsx b/apps/cli/src/ui/components/TodoChangeDisplay.tsx deleted file mode 100644 index f41049122a..0000000000 --- a/apps/cli/src/ui/components/TodoChangeDisplay.tsx +++ /dev/null @@ -1,142 +0,0 @@ -import { memo } from "react" -import { Box, Text } from "ink" - -import type { TodoItem } from "@roo-code/types" - -import * as theme from "../theme.js" - -/** - * Status icons for TODO items using Unicode characters - */ -const STATUS_ICONS = { - completed: "✓", - in_progress: "→", - pending: "○", -} as const - -/** - * Get the color for a TODO status - */ -function getStatusColor(status: TodoItem["status"]): string { - switch (status) { - case "completed": - return theme.successColor - case "in_progress": - return theme.warningColor - case "pending": - default: - return theme.dimText - } -} - -interface TodoChangeDisplayProps { - /** Previous TODO list for comparison */ - previousTodos: TodoItem[] - /** New TODO list */ - newTodos: TodoItem[] -} - -/** - * TodoChangeDisplay component for CLI - * - * Shows only the items that changed between two TODO lists. - * Used for compact inline display in the chat history. - * - * Visual example: - * ``` - * ☑ TODO Updated - * ✓ Design architecture [completed] - * → Implement core logic [started] - * ``` - */ -function TodoChangeDisplay({ previousTodos, newTodos }: TodoChangeDisplayProps) { - if (!newTodos || newTodos.length === 0) { - return null - } - - const isInitialState = previousTodos.length === 0 - - // Determine which todos to display - let todosToDisplay: TodoItem[] - - if (isInitialState) { - // For initial state, show all todos - todosToDisplay = newTodos - } else { - // For updates, only show changes (completed or started items) - todosToDisplay = newTodos.filter((newTodo) => { - if (newTodo.status === "completed") { - const previousTodo = previousTodos.find((p) => p.id === newTodo.id || p.content === newTodo.content) - return !previousTodo || previousTodo.status !== "completed" - } - if (newTodo.status === "in_progress") { - const previousTodo = previousTodos.find((p) => p.id === newTodo.id || p.content === newTodo.content) - return !previousTodo || previousTodo.status !== "in_progress" - } - return false - }) - } - - // If no changes to display, show nothing - if (todosToDisplay.length === 0) { - return null - } - - // Calculate progress for summary - const totalCount = newTodos.length - const completedCount = newTodos.filter((t) => t.status === "completed").length - - return ( - - {/* Header with progress summary */} - - - ☑ TODO {isInitialState ? "List" : "Updated"} - - - {" "} - ({completedCount}/{totalCount}) - - - - {/* Changed items */} - - {todosToDisplay.map((todo, index) => { - const icon = STATUS_ICONS[todo.status] || STATUS_ICONS.pending - const color = getStatusColor(todo.status) - - // Determine what changed - const previousTodo = previousTodos.find((p) => p.id === todo.id || p.content === todo.content) - let changeLabel: string | null = null - - if (isInitialState) { - // Don't show labels for initial state - changeLabel = null - } else if (!previousTodo) { - changeLabel = "new" - } else if (todo.status === "completed" && previousTodo.status !== "completed") { - changeLabel = "done" - } else if (todo.status === "in_progress" && previousTodo.status !== "in_progress") { - changeLabel = "started" - } - - return ( - - - {icon} {todo.content} - - {changeLabel && ( - - {" "} - [{changeLabel}] - - )} - - ) - })} - - - ) -} - -export default memo(TodoChangeDisplay) diff --git a/apps/cli/src/ui/components/__tests__/TodoChangeDisplay.test.tsx b/apps/cli/src/ui/components/__tests__/TodoChangeDisplay.test.tsx deleted file mode 100644 index a26c66e3df..0000000000 --- a/apps/cli/src/ui/components/__tests__/TodoChangeDisplay.test.tsx +++ /dev/null @@ -1,149 +0,0 @@ -import { render } from "ink-testing-library" - -import type { TodoItem } from "@roo-code/types" - -import TodoChangeDisplay from "../TodoChangeDisplay.js" - -describe("TodoChangeDisplay", () => { - it("renders all todos for initial state (no previous todos)", () => { - const newTodos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "completed" }, - { id: "2", content: "Task 2", status: "in_progress" }, - { id: "3", content: "Task 3", status: "pending" }, - ] - - const { lastFrame } = render() - const output = lastFrame() - - // Check header shows "List" for initial state - expect(output).toContain("TODO List") - - // All items should be shown - expect(output).toContain("Task 1") - expect(output).toContain("Task 2") - expect(output).toContain("Task 3") - - // Progress should be shown - expect(output).toContain("(1/3)") - }) - - it("shows only changed items when previous todos exist", () => { - const previousTodos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "pending" }, - { id: "2", content: "Task 2", status: "pending" }, - { id: "3", content: "Task 3", status: "pending" }, - ] - - const newTodos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "completed" }, // Changed to completed - { id: "2", content: "Task 2", status: "in_progress" }, // Changed to in_progress - { id: "3", content: "Task 3", status: "pending" }, // No change - ] - - const { lastFrame } = render() - const output = lastFrame() - - // Header should say "Updated" - expect(output).toContain("TODO Updated") - - // Only changed items should be shown - expect(output).toContain("Task 1") - expect(output).toContain("Task 2") - - // Unchanged item should NOT be shown - // Note: We can check if "Task 3" appears but since rendering is compact, - // we'll check for change labels instead - expect(output).toContain("[done]") - expect(output).toContain("[started]") - }) - - it("returns null when no todos provided", () => { - const { lastFrame } = render() - expect(lastFrame()).toBe("") - }) - - it("returns null when no changes detected", () => { - const todos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "completed" }, - { id: "2", content: "Task 2", status: "pending" }, - ] - - const { lastFrame } = render() - // No changes means nothing to display - expect(lastFrame()).toBe("") - }) - - it("shows [new] label for newly added items", () => { - const previousTodos: TodoItem[] = [{ id: "1", content: "Task 1", status: "completed" }] - - const newTodos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "completed" }, - { id: "2", content: "New Task", status: "in_progress" }, // New item - ] - - const { lastFrame } = render() - const output = lastFrame() - - expect(output).toContain("New Task") - expect(output).toContain("[new]") - }) - - it("displays correct status icons", () => { - const newTodos: TodoItem[] = [ - { id: "1", content: "Completed task", status: "completed" }, - { id: "2", content: "In progress task", status: "in_progress" }, - { id: "3", content: "Pending task", status: "pending" }, - ] - - const { lastFrame } = render() - const output = lastFrame() - - // Check status icons - expect(output).toContain("✓") // completed - expect(output).toContain("→") // in_progress - expect(output).toContain("○") // pending - }) - - it("shows progress summary in header", () => { - const newTodos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "completed" }, - { id: "2", content: "Task 2", status: "completed" }, - { id: "3", content: "Task 3", status: "pending" }, - { id: "4", content: "Task 4", status: "pending" }, - ] - - const { lastFrame } = render() - const output = lastFrame() - - // 2 out of 4 completed - expect(output).toContain("(2/4)") - }) - - it("does not show labels for initial state items", () => { - const newTodos: TodoItem[] = [ - { id: "1", content: "Task 1", status: "in_progress" }, - { id: "2", content: "Task 2", status: "pending" }, - ] - - const { lastFrame } = render() - const output = lastFrame() - - // Initial state should not have change labels like [done], [started], [new] - expect(output).not.toContain("[done]") - expect(output).not.toContain("[started]") - expect(output).not.toContain("[new]") - }) - - it("handles matching by content when ids differ", () => { - const previousTodos: TodoItem[] = [{ id: "old-1", content: "Same content task", status: "pending" }] - - const newTodos: TodoItem[] = [{ id: "new-1", content: "Same content task", status: "completed" }] - - const { lastFrame } = render() - const output = lastFrame() - - // Should recognize as the same task that changed status - expect(output).toContain("Same content task") - expect(output).toContain("[done]") - }) -}) diff --git a/apps/cli/src/ui/components/tools/utils.ts b/apps/cli/src/ui/components/tools/utils.ts index 484125dbb2..af4972cbac 100644 --- a/apps/cli/src/ui/components/tools/utils.ts +++ b/apps/cli/src/ui/components/tools/utils.ts @@ -32,16 +32,6 @@ export function sanitizeContent(text: string): string { return text.replace(/\t/g, " ").replace(/\r/g, "") } -/** - * Format diff stats as a colored string representation - */ -export function formatDiffStats(stats: { added: number; removed: number }): { added: string; removed: string } { - return { - added: `+${stats.added}`, - removed: `-${stats.removed}`, - } -} - /** * Get a friendly display name for a tool */ @@ -146,28 +136,6 @@ export function getToolIconName(toolName: string): IconName { return iconNames[toolName] || "gear" } -/** - * Format a file path for display, optionally with workspace indicator - */ -export function formatPath(path: string, isOutsideWorkspace?: boolean, isProtected?: boolean): string { - let result = path - const badges: string[] = [] - - if (isOutsideWorkspace) { - badges.push("outside workspace") - } - - if (isProtected) { - badges.push("protected") - } - - if (badges.length > 0) { - result += ` (${badges.join(", ")})` - } - - return result -} - /** * Parse diff content into structured hunks for rendering */ diff --git a/apps/cli/src/ui/hooks/useExtensionHost.ts b/apps/cli/src/ui/hooks/useExtensionHost.ts index 235c7c5aa8..35ff758c11 100644 --- a/apps/cli/src/ui/hooks/useExtensionHost.ts +++ b/apps/cli/src/ui/hooks/useExtensionHost.ts @@ -69,7 +69,6 @@ export function useExtensionHost({ continueSession, mode, reasoningEffort, - user, provider, apiKey, model, @@ -107,7 +106,6 @@ export function useExtensionHost({ const host = createExtensionHost({ mode, - user, reasoningEffort, provider, apiKey, diff --git a/apps/cli/src/ui/theme.ts b/apps/cli/src/ui/theme.ts index bce18f7629..f857a28be8 100644 --- a/apps/cli/src/ui/theme.ts +++ b/apps/cli/src/ui/theme.ts @@ -38,12 +38,6 @@ const hardcore = { // Title and branding colors export const titleColor = hardcore.orange // Orange for title -export const welcomeText = hardcore.text // Standard text -export const asciiColor = hardcore.cyan // Cyan for ASCII art - -// Tips section colors -export const tipsHeader = hardcore.orange // Orange for tips headers -export const tipsText = hardcore.subtext0 // Subtle text for tips // Header text colors (for messages) export const userHeader = hardcore.purple // Purple for user header @@ -61,9 +55,6 @@ export const thinkingText = hardcore.overlay2 // Subtle gray for thinking text export const borderColor = hardcore.surface1 // Surface color for borders export const borderColorActive = hardcore.purple // Active/focused border color export const dimText = hardcore.overlay1 // Dim text -export const promptColor = hardcore.overlay2 // Prompt indicator -export const promptColorActive = hardcore.cyan // Active prompt color -export const placeholderColor = hardcore.overlay0 // Placeholder text // Status colors export const successColor = hardcore.green // Green for success diff --git a/docs/development-quality-checks.md b/docs/development-quality-checks.md new file mode 100644 index 0000000000..5de20077bf --- /dev/null +++ b/docs/development-quality-checks.md @@ -0,0 +1,70 @@ +# Dead-code checks before review + +Run these commands before requesting review: + +```sh +pnpm knip +pnpm knip:production +pnpm test:knip-policy +``` + +The `knip` job in Code QA runs all three commands. Both analyzer runs must finish +without errors. The default run checks the graph that includes tests and development +dependencies. The production run checks runtime entry points without test callers +or development dependencies. A helper with passing unit tests can still be unused +by the product; the second run catches that case. + +Unused files, dependencies, exports, exported types, enum members, namespace +exports, and duplicate exports are errors. Duplicate exports means exporting the +same declaration more than once; it does not mean similar implementations in +different modules. Class-member analysis remains off pending a separate audit. +Exports used inside their own file retain the existing exemption. + +The policy tests run the installed Knip CLI against small temporary projects using +the extension workspace configuration. They verify successful analysis as well as +nonzero exit codes and specific findings for unused exports, unused types, +duplicate exports, unlisted dependencies, and production files referenced only by +tests. A workflow test verifies that CI runs both modes without allowing failure. + +## Handling findings + +First look for an existing implementation and real callers. Remove unused code or +integrate the existing helper rather than adding a second implementation. Deleting +an export can expose an unused local declaration to ESLint, so run lint and type +checks after cleanup. + +For an intentional direct-test seam, put `@internal` and the reason next to the +export. Knip still follows test imports in default mode; production mode excludes +that export from its public-surface findings. This annotation does not make an +otherwise unused file reachable. Reserve `@public` for a deliberate public API. +Do not annotate a new scaffold just to make the checks pass. + +Entry patterns describe real runtime entry points. Browser component fixtures are +development entries because Playwright loads them outside the normal application +graph. They are excluded from the production project so their imports cannot make +otherwise unused application helpers appear reachable. Shared `src/test-utils` +files are excluded only in production mode. + +Two existing source files have explicit production-only exceptions: + +| File | Reason | +| ------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/utils/versionToken.ts` | A staged compare-and-swap helper from epic #1375 with direct tests; it has no runtime caller yet. Remove this exception when the write guard consumes it, or remove the helper if that work is abandoned. | +| `src/core/tools/helpers/toolResultFormatting.ts` | Existing display helper with direct tests and no runtime callers. Retained as existing debt rather than expanding this PR's removal scope. Remove the exception when integrated or delete the helper and its tests. | + +`resolveModelWithAbort` also currently has only contract-test callers. Its explicit +`@internal` annotation retains the existing shared cancellation helper for provider +adoption. This is existing debt, not evidence that providers already use it. + +Review additions to exceptions and annotations as changes to the gate itself. +The repository's existing CODEOWNERS includes these files. Requiring that approval +and the `knip` status check is a GitHub ruleset setting; changing this workflow does +not configure branch protection or prevent an approved change to its own policy. + +## What needs other checks + +Knip cannot prove that a new helper should reuse an existing abstraction, that a PR +matches its issue, or that a test detects a behavioral regression. Use focused +contract tests and the existing changed-code mutation checks for those questions. +Keep new architectural ESLint rules and clone detection scoped to demonstrated +failure patterns and validate their false positives before making them required. diff --git a/knip.json b/knip.json index db102031eb..a0ea16566e 100644 --- a/knip.json +++ b/knip.json @@ -1,14 +1,36 @@ { "$schema": "https://unpkg.com/knip@5/schema.json", - "ignore": ["**/__tests__/**", "apps/vscode-e2e/**", "scripts/**", "apps/cli/scripts/**"], + "ignore": [ + "apps/vscode-e2e/**", + "scripts/**", + "apps/cli/scripts/**", + "**/__tests__/**!", + "packages/core/src/custom-tools/__tests__/fixtures/**", + "packages/core/src/custom-tools/__tests__/fixtures-override/**", + "src/integrations/misc/__tests__/performance/**", + "src/integrations/terminal/__tests__/setupTerminalTests.ts", + "src/integrations/terminal/__tests__/streamUtils/index.ts", + "src/services/tree-sitter/__tests__/fixtures/sample-python.ts", + "src/services/tree-sitter/__tests__/helpers.ts" + ], "ignoreDependencies": ["lint-staged"], "ignoreExportsUsedInFile": true, "playwright": false, "playwright-ct": false, "workspaces": { "src": { - "entry": ["extension.ts", "extension/api.ts", "workers/countTokens.ts"], - "project": ["**/*.ts", "!**/__tests__/**"], + "entry": ["extension.ts!", "extension/api.ts!", "workers/countTokens.ts!"], + "project": [ + "**/*.ts!", + "!**/__tests__/**!", + "!vitest.setup.ts!", + "!utils/vitest-verbosity.ts!", + "!utils/TaskSemaphore.ts!", + "!utils/WorkspacePathResolver.ts!", + "!test-utils/**!", + "!utils/versionToken.ts!", + "!core/tools/helpers/toolResultFormatting.ts!" + ], "ignoreDependencies": [ "@roo-code/config-typescript", "@types/node-cache", @@ -20,8 +42,15 @@ ] }, "webview-ui": { - "entry": ["src/index.tsx"], - "project": ["src/**/*.{ts,tsx}", "../src/shared/*.ts"], + "entry": ["src/index.tsx!", "src/**/*.visual.tsx", "src/**/*.visual.{fixture,i18n}.{ts,tsx}"], + "project": [ + "src/**/*.{ts,tsx}!", + "../src/shared/*.ts!", + "!src/utils/test-utils.tsx!", + "!src/vite-plugins/**!", + "!src/**/*.visual.tsx!", + "!src/**/*.visual.{fixture,i18n}.{ts,tsx}!" + ], "ignoreDependencies": [ "@roo-code/config-typescript", "@types/katex", @@ -36,38 +65,39 @@ ] }, "apps/cli": { - "project": ["src/**/*.ts", "src/**/*.tsx"], + "ignore": ["eslint.config.mjs", "scripts/**"], + "project": ["src/**/*.ts!", "src/**/*.tsx!"], "ignoreDependencies": ["@vscode/ripgrep"] }, + "apps/vscode-nightly": { + "ignore": ["esbuild.mjs"] + }, "packages/config-typescript": { "ignoreUnresolved": ["vitest/globals"] }, + "packages/types": { + "project": ["src/**/*.ts!", "!src/roomodes-schema.ts!"] + }, "packages/core": { - "project": ["src/**/*.ts"], + "project": ["src/**/*.ts!"], "ignoreDependencies": ["esbuild"] }, "packages/vscode-shim": { - "project": ["src/**/*.ts"] + "project": ["src/**/*.ts!"] }, "packages/cloud": { - "project": ["src/**/*.ts"], + "project": ["src/**/*.ts!"], "ignoreDependencies": ["@types/vscode"] }, "packages/telemetry": { - "project": ["src/**/*.ts"], + "project": ["src/**/*.ts!"], "ignoreDependencies": ["@types/vscode"] }, - "packages/{build,ipc,types}": { - "project": ["src/**/*.ts"] + "packages/{build,ipc}": { + "project": ["src/**/*.ts!"] } }, "rules": { - "classMembers": "off", - "duplicates": "warn", - "enumMembers": "warn", - "exports": "warn", - "nsExports": "warn", - "types": "warn", - "nsTypes": "warn" + "classMembers": "off" } } diff --git a/package.json b/package.json index 26598c4caf..0f440e1ec1 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,9 @@ "code-server:install": "node scripts/code-server.js", "changeset:version": "cp CHANGELOG.md src/CHANGELOG.md && changeset version && cp -vf src/CHANGELOG.md .", "knip": "knip", - "npm:publish:types": "pnpm --filter @roo-code/types npm:publish" + "npm:publish:types": "pnpm --filter @roo-code/types npm:publish", + "knip:production": "knip --production", + "test:knip-policy": "node --test scripts/knip-policy.test.mjs" }, "devDependencies": { "@changesets/cli": "2.31.0", diff --git a/packages/types/src/provider-settings/moonshot.ts b/packages/types/src/provider-settings/moonshot.ts index 96292872f3..6d5a99e6cd 100644 --- a/packages/types/src/provider-settings/moonshot.ts +++ b/packages/types/src/provider-settings/moonshot.ts @@ -20,6 +20,3 @@ export const moonshotProviderDefinition = createProviderDefinition({ moonshotApiKey: z.string().optional(), }, }) - -export const kimiCodeAuthMethodSchema = z.enum(["oauth", "api-key"]) -export type KimiCodeAuthMethod = z.infer diff --git a/packages/types/src/provider-settings/sambanova.ts b/packages/types/src/provider-settings/sambanova.ts index 257883747b..1067e01391 100644 --- a/packages/types/src/provider-settings/sambanova.ts +++ b/packages/types/src/provider-settings/sambanova.ts @@ -17,7 +17,3 @@ export const sambaNovaProviderDefinition = createProviderDefinition({ sambaNovaApiKey: z.string().optional(), }, }) - -export const zaiApiLineSchema = z.enum(["international_coding", "china_coding", "international_api", "china_api"]) - -export type ZaiApiLine = z.infer diff --git a/packages/types/src/task.ts b/packages/types/src/task.ts index f36c69ad0d..d243bffbb1 100644 --- a/packages/types/src/task.ts +++ b/packages/types/src/task.ts @@ -102,7 +102,6 @@ export enum TaskStatus { Interactive = "interactive", Resumable = "resumable", Idle = "idle", - None = "none", } export const taskMetadataSchema = z.object({ diff --git a/packages/vscode-shim/src/types.ts b/packages/vscode-shim/src/types.ts index d21a99c43d..8280772110 100644 --- a/packages/vscode-shim/src/types.ts +++ b/packages/vscode-shim/src/types.ts @@ -67,37 +67,6 @@ export interface ISelection extends IRange { isReversed: boolean } -/** - * Represents a line of text in a document - */ -export interface TextLine { - text: string - range: IRange - rangeIncludingLineBreak: IRange - firstNonWhitespaceCharacterIndex: number - isEmptyOrWhitespace: boolean -} - -/** - * Represents a text document - */ -export interface TextDocument { - uri: IUri - fileName: string - languageId: string - version: number - isDirty: boolean - isClosed: boolean - lineCount: number - getText(range?: IRange): string - lineAt(line: number): TextLine - offsetAt(position: IPosition): number - positionAt(offset: number): IPosition - save(): Thenable - validateRange(range: IRange): IRange - validatePosition(position: IPosition): IPosition -} - /** * Configuration target for settings */ @@ -107,26 +76,6 @@ export enum ConfigurationTarget { WorkspaceFolder = 3, } -/** - * Workspace folder representation - */ -export interface WorkspaceFolder { - uri: IUri - name: string - index: number -} - -/** - * Workspace configuration interface - */ -export interface WorkspaceConfiguration { - get(section: string): T | undefined - get(section: string, defaultValue: T): T - has(section: string): boolean - inspect(section: string): ConfigurationInspect | undefined - update(section: string, value: unknown, configurationTarget?: ConfigurationTarget): Thenable -} - /** * Configuration inspection result */ @@ -165,6 +114,9 @@ export interface SecretStorageChangeEvent { key: string } +export type { TextDocument, TextLine, WorkspaceFolder, CancellationToken } from "./interfaces/document.js" +export type { WorkspaceConfiguration } from "./interfaces/workspace.js" + /** * Represents an extension */ @@ -222,16 +174,10 @@ export enum ExtensionMode { */ export type Event = (listener: (e: T) => void, thisArgs?: unknown, disposables?: Disposable[]) => Disposable -/** - * Cancellation token for async operations - */ -export interface CancellationToken { - isCancellationRequested: boolean - onCancellationRequested: Event -} - /** * File system file type enum + * + * @public */ export enum FileType { Unknown = 0, @@ -273,6 +219,8 @@ export enum ViewColumn { /** * UI Kind enum + * + * @public */ export enum UIKind { Desktop = 1, diff --git a/packages/vscode-shim/src/utils/paths.ts b/packages/vscode-shim/src/utils/paths.ts index 948c25429e..37b4def249 100644 --- a/packages/vscode-shim/src/utils/paths.ts +++ b/packages/vscode-shim/src/utils/paths.ts @@ -46,10 +46,7 @@ export function ensureDirectoryExists(dirPath: string): void { } } -/** - * Initialize workspace directories - */ -export function initializeWorkspace(workspacePath: string): void { +function initializeWorkspace(workspacePath: string): void { const dirs = [getGlobalStorageDir(), getWorkspaceStorageDir(workspacePath), getLogsDir()] for (const dir of dirs) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 97a4be7f9e..adde200ccc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -93,18 +93,12 @@ importers: '@roo-code/vscode-shim': specifier: workspace:^ version: link:../../packages/vscode-shim - '@trpc/client': - specifier: ^11.18.0 - version: 11.18.0(@trpc/server@11.18.0(typescript@5.9.3))(typescript@5.9.3) '@vscode/ripgrep': specifier: ^1.15.9 version: 1.18.0 commander: specifier: ^12.1.0 version: 12.1.0 - execa: - specifier: ^9.5.2 - version: 9.6.1 fuzzysort: specifier: ^3.1.0 version: 3.1.0 @@ -117,9 +111,6 @@ importers: react: specifier: ^19.1.0 version: 19.2.7 - superjson: - specifier: ^2.2.6 - version: 2.2.6 zustand: specifier: ^5.0.14 version: 5.0.14(@types/react@18.3.31)(react@19.2.7) @@ -130,9 +121,6 @@ importers: '@roo-code/config-typescript': specifier: workspace:^ version: link:../../packages/config-typescript - '@trpc/server': - specifier: 11.18.0 - version: 11.18.0(typescript@5.9.3) '@types/node': specifier: 22.20.1 version: 22.20.1 @@ -142,6 +130,9 @@ importers: '@vitest/coverage-v8': specifier: 4.1.9 version: 4.1.9(vitest@4.1.11) + execa: + specifier: ^9.5.2 + version: 9.6.1 ink-testing-library: specifier: 4.0.0 version: 4.0.0(@types/react@18.3.31) @@ -587,9 +578,6 @@ importers: say: specifier: ^0.16.0 version: 0.16.0 - semver-compare: - specifier: ^1.0.0 - version: 1.0.0 serialize-error: specifier: ^12.0.0 version: 12.0.0 @@ -630,9 +618,6 @@ importers: specifier: 3.25.76 version: 3.25.76 devDependencies: - '@ai-sdk/openai-compatible': - specifier: 3.0.70 - version: 3.0.70(zod@3.25.76) '@roo-code/build': specifier: workspace:^ version: link:../packages/build @@ -663,9 +648,6 @@ importers: '@types/ps-tree': specifier: 1.1.6 version: 1.1.6 - '@types/semver-compare': - specifier: 1.0.3 - version: 1.0.3 '@types/vscode': specifier: 1.100.0 version: 1.100.0 @@ -750,9 +732,6 @@ importers: '@roo-code/types': specifier: workspace:^ version: link:../packages/types - '@tailwindcss/vite': - specifier: ^4.0.0 - version: 4.3.2(vite@8.1.0(@types/node@22.20.1)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.22.4)(yaml@2.9.0)) '@tanstack/react-query': specifier: ^5.68.0 version: 5.101.2(react@18.3.1) @@ -907,6 +886,9 @@ importers: '@roo-code/config-typescript': specifier: workspace:^ version: link:../packages/config-typescript + '@tailwindcss/vite': + specifier: ^4.0.0 + version: 4.3.2(vite@8.1.0(@types/node@22.20.1)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.22.4)(yaml@2.9.0)) '@testing-library/jest-dom': specifier: 6.9.1 version: 6.9.1 @@ -3283,19 +3265,6 @@ packages: '@textlint/types@15.7.1': resolution: {integrity: sha512-Vye/GmFNBTgVzZFtIFJTmLB+s2A7oIADxNG6r9UhfPuY+Czv0z5G3xeyFZZudPlfxURsKUyPIU5XsjOFqVp33A==} - '@trpc/client@11.18.0': - resolution: {integrity: sha512-wOqeg3Fvl25V1ZisQhUD3K8G60ZJDlSGJNSyeXrLH24xAo5w6GSR2Kzb1cSNY9Y+IQ2YZvYGZstBU+V/ulo/ow==} - hasBin: true - peerDependencies: - '@trpc/server': 11.18.0 - typescript: '>=5.7.2' - - '@trpc/server@11.18.0': - resolution: {integrity: sha512-JAvXOuNTxgXjIDfQaOvDq1j66LMNfDJUH1IU7Slfn8EvRv2EkH6ehu3A7zpYhjO0syHHiYg77v2lG2JFJgvw7Q==} - hasBin: true - peerDependencies: - typescript: '>=5.7.2' - '@turbo/darwin-64@2.10.0': resolution: {integrity: sha512-EwvHThXzpY0KGd1/NAmuewI5D+aVa3Rl/OlxE36yfjUKb/+ySrfJrSlEFt8aD1OXwnnaHnQnPKHFndor0Zxlsg==} cpu: [x64] @@ -3557,9 +3526,6 @@ packages: '@types/sarif@2.1.7': resolution: {integrity: sha512-kRz0VEkJqWLf1LLVN4pT1cg1Z9wAuvI6L97V3m2f5B76Tg8d413ddvLBPTEHAZJlnn4XSvu0FkZtViCQGVyrXQ==} - '@types/semver-compare@1.0.3': - resolution: {integrity: sha512-mVZkB2QjXmZhh+MrtwMlJ8BqUnmbiSkpd88uOWskfwB8yitBT0tBRAKt+41VRgZD9zr9Sc+Xs02qGgvzd1Rq/Q==} - '@types/stack-utils@2.0.3': resolution: {integrity: sha512-9aEbYZ3TbYMznPdcdr3SmIrLXwC/AKZXQeCf9Pgao5CKb8CyHuEX5jzWPTkvregvhRJHcpRO6BFoGW9ycaOkYw==} @@ -5953,10 +5919,6 @@ packages: resolution: {integrity: sha512-mfcwb6IzQyOKTs84CQMrOwW4gQcaTOAWJ0zzJCl2WSPDrWk/OzDaImWFH3djXhb24g4eudZfLRozAvPGw4d9hQ==} engines: {node: '>= 0.4'} - is-what@5.5.0: - resolution: {integrity: sha512-oG7cgbmg5kLYae2N5IVd3jm2s+vldjxJzK1pcu9LfpGuQ93MQSzo0okvRna+7y5ifrD+20FE8FvjusyGaz14fw==} - engines: {node: '>=18'} - is-windows@1.0.2: resolution: {integrity: sha512-eXK1UInq2bPmjyX6e3VHIzMLobc4J94i4AWn+Hpq3OU5KkrRC96OAcR3PRJ/pGu6m8TRnBHP9dkXQVsT/COVIA==} engines: {node: '>=0.10.0'} @@ -8133,10 +8095,6 @@ packages: engines: {node: '>=16 || 14 >=14.17'} hasBin: true - superjson@2.2.6: - resolution: {integrity: sha512-H+ue8Zo4vJmV2nRjpx86P35lzwDT3nItnIsocgumgr0hHMQ+ZGq5vrERg9kJBo5AWGmxZDhzDo+WVIJqkB0cGA==} - engines: {node: '>=16'} - supports-color@7.2.0: resolution: {integrity: sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==} engines: {node: '>=8'} @@ -11567,15 +11525,6 @@ snapshots: dependencies: '@textlint/ast-node-types': 15.7.1 - '@trpc/client@11.18.0(@trpc/server@11.18.0(typescript@5.9.3))(typescript@5.9.3)': - dependencies: - '@trpc/server': 11.18.0(typescript@5.9.3) - typescript: 5.9.3 - - '@trpc/server@11.18.0(typescript@5.9.3)': - dependencies: - typescript: 5.9.3 - '@turbo/darwin-64@2.10.0': optional: true @@ -11855,8 +11804,6 @@ snapshots: '@types/sarif@2.1.7': {} - '@types/semver-compare@1.0.3': {} - '@types/stack-utils@2.0.3': {} '@types/stacktrace-js@2.0.3': @@ -14566,8 +14513,6 @@ snapshots: call-bound: 1.0.4 get-intrinsic: 1.3.0 - is-what@5.5.0: {} - is-windows@1.0.2: {} is-wsl@3.1.0: @@ -17264,10 +17209,6 @@ snapshots: pirates: 4.0.7 ts-interface-checker: 0.1.13 - superjson@2.2.6: - dependencies: - copy-anything: 4.0.5 - supports-color@7.2.0: dependencies: has-flag: 4.0.0 diff --git a/scripts/code-qa-workflow.test.mjs b/scripts/code-qa-workflow.test.mjs index 40ccc66348..3f83d1129e 100644 --- a/scripts/code-qa-workflow.test.mjs +++ b/scripts/code-qa-workflow.test.mjs @@ -116,3 +116,23 @@ describe("platform unit-test workflow", () => { assert.match(config, /maxWorkers: isWindowsCI \? 2 : undefined/) }) }) + +describe("Knip quality gate", () => { + it("runs analyzer challenge tests and both dependency graphs without allowing failure", () => { + const manifest = JSON.parse(fs.readFileSync(path.join(repositoryRoot, "package.json"), "utf8")) + assert.equal(manifest.scripts.knip, "knip") + assert.equal(manifest.scripts["knip:production"], "knip --production") + assert.equal(manifest.scripts["test:knip-policy"], "node --test scripts/knip-policy.test.mjs") + for (const [stepName, command] of [ + ["Verify Knip guardrails", "pnpm test:knip-policy"], + ["Run knip checks", "pnpm knip"], + ["Run production Knip checks", "pnpm knip:production"], + ]) { + assert.match(workflowStep(stepName), new RegExp(`run: ${command}\\s*$`)) + assert.doesNotMatch(workflowStep(stepName), /continue-on-error|if:/) + } + const job = workflow.match(/\n knip:\n(?[\s\S]*?)\n compile:/)?.groups?.body + assert.ok(job, "missing knip job") + assert.doesNotMatch(job, /continue-on-error|if:/) + }) +}) diff --git a/scripts/knip-policy.test.mjs b/scripts/knip-policy.test.mjs new file mode 100644 index 0000000000..f7ea6b1e6a --- /dev/null +++ b/scripts/knip-policy.test.mjs @@ -0,0 +1,193 @@ +import assert from "node:assert/strict" +import { spawnSync } from "node:child_process" +import { mkdtempSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import path from "node:path" +import { fileURLToPath } from "node:url" +import { test } from "node:test" + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") +const cli = path.join(root, "node_modules/knip/bin/knip.js") +const config = JSON.parse(readFileSync(path.join(root, "knip.json"), "utf8")) + +// Exercise the installed analyzer and the repository's extension graph, rather +// than asserting that a configuration flag merely appears in a JSON file. +function fixture(files, run, includeWebview = false) { + const directory = mkdtempSync(path.join(tmpdir(), "zoo-knip-policy-")) + try { + const write = (name, contents) => { + const filename = path.join(directory, name) + mkdirSync(path.dirname(filename), { recursive: true }) + writeFileSync(filename, contents) + } + write("package.json", JSON.stringify({ name: "knip-policy-fixture", private: true })) + write("pnpm-workspace.yaml", `packages:\n - src\n${includeWebview ? " - webview-ui\n" : ""}`) + write( + "src/package.json", + JSON.stringify({ + name: "fixture-extension", + scripts: { test: "vitest run" }, + devDependencies: { vitest: "4.1.11" }, + }), + ) + write( + "knip.json", + JSON.stringify({ + ...config, + workspaces: { + src: config.workspaces.src, + ...(includeWebview ? { "webview-ui": config.workspaces["webview-ui"] } : {}), + }, + }), + ) + if (includeWebview) { + write("webview-ui/package.json", JSON.stringify({ name: "fixture-webview", private: true })) + write("webview-ui/src/index.tsx", 'console.log("application entry")\n') + } + write("src/extension.ts", 'import { live } from "./helper"\nconsole.log(live)\n') + write("src/helper.ts", "export const live = 1\n") + write( + "src/vitest.config.ts", + 'import { defineConfig } from "vitest/config"\nexport default defineConfig({ test: {} })\n', + ) + symlinkSync(path.join(root, "src/node_modules"), path.join(directory, "src/node_modules"), "junction") + for (const [name, contents] of Object.entries(files)) write(name, contents) + run((...flags) => { + const result = spawnSync(process.execPath, [cli, "--no-progress", "--reporter", "json", ...flags], { + cwd: directory, + encoding: "utf8", + timeout: 30_000, + }) + assert.ifError(result.error) + assert.equal(result.signal, null, result.stderr) + assert.ok(result.status === 0 || result.status === 1, result.stderr) + return { status: result.status, report: JSON.parse(result.stdout) } + }) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +} + +function reported(result, category, name) { + return result.report.issues.some((issue) => issue[category]?.some((item) => item.name === name)) +} + +test("unused exports fail both Knip modes", () => { + fixture({ "src/helper.ts": "export const live = 1\nexport const unused = 2\n" }, (knip) => { + for (const flags of [[], ["--production"]]) { + const result = knip(...flags) + assert.equal(result.status, 1) + assert.ok(reported(result, "exports", "unused")) + } + }) +}) + +test("tests cannot make an otherwise unused production export pass the production check", () => { + fixture( + { + "src/helper.ts": "export const live = 1\nexport const testOnly = 2\n", + "src/__tests__/helper.spec.ts": 'import { testOnly } from "../helper"\nconsole.log(testOnly)\n', + }, + (knip) => { + const development = knip() + assert.equal(development.status, 0, JSON.stringify(development.report)) + const production = knip("--production") + assert.equal(production.status, 1) + assert.ok(reported(production, "exports", "testOnly")) + }, + ) +}) + +test("an explicit internal test seam remains usable in tests without becoming public production API", () => { + fixture( + { + "src/helper.ts": "export const live = 1\n/** @internal */\nexport const testOnly = 2\n", + "src/__tests__/helper.spec.ts": 'import { testOnly } from "../helper"\nconsole.log(testOnly)\n', + }, + (knip) => { + const development = knip() + assert.equal(development.status, 0, JSON.stringify(development.report)) + assert.equal(knip("--production").status, 0) + }, + ) +}) + +test("a production file imported only by tests is still reported as unused", () => { + fixture( + { + "src/scaffold.ts": "export const scaffold = 1\n", + "src/__tests__/scaffold.spec.ts": 'import { scaffold } from "../scaffold"\nconsole.log(scaffold)\n', + }, + (knip) => { + const development = knip() + assert.equal(development.status, 0, JSON.stringify(development.report)) + const production = knip("--production") + assert.equal(production.status, 1) + assert.ok(production.report.files.includes("src/scaffold.ts")) + }, + ) +}) + +test("duplicate exports remain errors", () => { + fixture( + { + "src/helper.ts": "export const live = 1\nexport default live\n", + "src/extension.ts": 'import alias, { live } from "./helper"\nconsole.log(live, alias)\n', + }, + (knip) => { + for (const flags of [[], ["--production"]]) { + const result = knip(...flags) + assert.equal(result.status, 1) + assert.ok(result.report.issues.some((issue) => issue.duplicates.length > 0)) + } + }, + ) +}) + +test("unused type exports remain errors", () => { + fixture({ "src/helper.ts": "export const live = 1\nexport interface UnusedType { value: string }\n" }, (knip) => { + for (const flags of [[], ["--production"]]) { + const result = knip(...flags) + assert.equal(result.status, 1) + assert.ok(reported(result, "types", "UnusedType")) + } + }) +}) + +test("unlisted runtime dependencies remain errors", () => { + fixture( + { + "src/extension.ts": + 'import "unlisted-knip-policy-dependency"\nimport { live } from "./helper"\nconsole.log(live)\n', + }, + (knip) => { + for (const flags of [[], ["--production"]]) { + const result = knip(...flags) + assert.equal(result.status, 1) + assert.ok(reported(result, "unlisted", "unlisted-knip-policy-dependency")) + } + }, + ) +}) + +test("visual entries cannot hide helpers unused by the production application", () => { + fixture( + { + "webview-ui/src/scene.visual.tsx": 'import { sceneOnly } from "./scene-helper"\nconsole.log(sceneOnly)\n', + "webview-ui/src/scene-helper.ts": 'export const sceneOnly = "scene"\n', + "webview-ui/src/scene.visual.fixture.tsx": + 'import { fixtureOnly } from "./fixture-helper"\nconsole.log(fixtureOnly)\n', + "webview-ui/src/fixture-helper.ts": 'export const fixtureOnly = "fixture"\n', + }, + (knip) => { + const development = knip() + assert.equal(development.status, 0, JSON.stringify(development.report)) + const production = knip("--production") + assert.equal(production.status, 1) + assert.ok(production.report.files.includes("webview-ui/src/scene-helper.ts")) + assert.ok(production.report.files.includes("webview-ui/src/fixture-helper.ts")) + assert.ok(!production.report.files.some((file) => file.includes(".visual."))) + }, + true, + ) +}) diff --git a/src/api/providers/__tests__/openai-compatible.spec.ts b/src/api/providers/__tests__/openai-compatible.spec.ts deleted file mode 100644 index 78a5a43576..0000000000 --- a/src/api/providers/__tests__/openai-compatible.spec.ts +++ /dev/null @@ -1,517 +0,0 @@ -// Use vi.hoisted to define mock functions that can be referenced in hoisted vi.mock() calls -const { mockStreamText, mockGenerateText } = vi.hoisted(() => ({ - mockStreamText: vi.fn(), - mockGenerateText: vi.fn(), -})) - -vi.mock("ai", async (importOriginal) => { - const actual = await importOriginal() - return { - ...actual, - streamText: mockStreamText, - generateText: mockGenerateText, - } -}) - -vi.mock("@ai-sdk/openai-compatible", () => ({ - createOpenAICompatible: vi.fn(function () { - // Return a function that returns a mock language model - return vi.fn(() => ({ - modelId: "test-model", - provider: "openai-compatible", - })) - }), -})) - -import type { Anthropic } from "@anthropic-ai/sdk" - -import { OpenAICompatibleHandler, OpenAICompatibleConfig } from "../openai-compatible" -import type { ApiHandlerOptions } from "../../../shared/api" -import type { ApiStreamChunk } from "../../../api/transform/stream" -import { APICallError, type LanguageModel } from "ai" - -/** Error shape produced when the upstream API responds with a 4xx/5xx status. */ -type StatusedError = Error & { status: number } - -// Concrete implementation for testing -class TestOpenAICompatibleHandler extends OpenAICompatibleHandler { - constructor(options: ApiHandlerOptions, config: OpenAICompatibleConfig) { - super(options, config) - } - - override getModel() { - return { - id: this.config.modelId, - info: this.config.modelInfo, - maxTokens: this.config.modelMaxTokens, - temperature: this.config.temperature, - } - } -} - -describe("OpenAICompatibleHandler", () => { - let handler: TestOpenAICompatibleHandler - let mockOptions: ApiHandlerOptions - let mockConfig: OpenAICompatibleConfig - - const systemPrompt = "You are a helpful assistant." - const messages: Anthropic.Messages.MessageParam[] = [ - { - role: "user", - content: [{ type: "text", text: "Hello!" }], - }, - ] - - beforeEach(() => { - mockOptions = { - apiModelId: "test-model", - apiKey: "test-api-key", - } - mockConfig = { - providerName: "TestProvider", - baseURL: "https://api.test.com/v1", - apiKey: "test-api-key", - modelId: "test-model", - modelInfo: { - maxTokens: 8192, - contextWindow: 128000, - supportsImages: false, - supportsPromptCache: true, - }, - } - handler = new TestOpenAICompatibleHandler(mockOptions, mockConfig) - vi.clearAllMocks() - }) - - describe("constructor", () => { - it("should initialize with provided options and config", () => { - expect(handler).toBeInstanceOf(TestOpenAICompatibleHandler) - expect(handler.getModel().id).toBe(mockConfig.modelId) - }) - }) - - describe("createMessage", () => { - it("should handle streaming responses", async () => { - async function* mockFullStream() { - yield { type: "text-delta", text: "Test response" } - } - - const mockUsage = Promise.resolve({ - inputTokens: 10, - outputTokens: 5, - details: {}, - raw: {}, - }) - - mockStreamText.mockReturnValue({ - fullStream: mockFullStream(), - usage: mockUsage, - }) - - const stream = handler.createMessage(systemPrompt, messages) - const chunks: ApiStreamChunk[] = [] - for await (const chunk of stream) { - chunks.push(chunk) - } - - expect(chunks.length).toBeGreaterThan(0) - const textChunks = chunks.filter((chunk) => chunk.type === "text") - expect(textChunks).toHaveLength(1) - expect(textChunks[0].text).toBe("Test response") - }) - - it("should map cache read and reasoning tokens from ai v7 usage details", async () => { - async function* mockFullStream() { - yield { type: "text-delta", text: "ok" } - } - - mockStreamText.mockReturnValue({ - fullStream: mockFullStream(), - usage: Promise.resolve({ - inputTokens: 20, - outputTokens: 10, - inputTokenDetails: { cacheReadTokens: 7 }, - outputTokenDetails: { reasoningTokens: 4 }, - raw: {}, - }), - }) - - const chunks: ApiStreamChunk[] = [] - for await (const chunk of handler.createMessage(systemPrompt, messages)) { - chunks.push(chunk) - } - - expect(chunks.find((chunk) => chunk.type === "usage")).toEqual({ - type: "usage", - inputTokens: 20, - outputTokens: 10, - cacheReadTokens: 7, - reasoningTokens: 4, - }) - }) - - it("should handle multiple stream parts and yield usage metrics", async () => { - async function* mockFullStream() { - yield { type: "text-delta", text: "First part" } - yield { type: "text-delta", text: "Second part" } - yield { type: "tool-call", toolCallId: "123", toolName: "test_tool", input: "{}" } - } - - const mockUsage = Promise.resolve({ - inputTokens: 20, - outputTokens: 10, - details: {}, - raw: {}, - }) - - mockStreamText.mockReturnValue({ - fullStream: mockFullStream(), - usage: mockUsage, - }) - - const stream = handler.createMessage(systemPrompt, messages) - const chunks: ApiStreamChunk[] = [] - for await (const chunk of stream) { - chunks.push(chunk) - } - - expect(chunks.length).toBeGreaterThan(2) - const textChunks = chunks.filter((chunk) => chunk.type === "text") - expect(textChunks).toHaveLength(2) - expect(textChunks[0].text).toBe("First part") - expect(textChunks[1].text).toBe("Second part") - - const toolChunks = chunks.filter((chunk) => chunk.type === "tool_call") - expect(toolChunks).toHaveLength(1) - expect(toolChunks[0].id).toBe("123") - expect(toolChunks[0].name).toBe("test_tool") - expect(toolChunks[0].arguments).toBe("{}") - - const usageChunks = chunks.filter((chunk) => chunk.type === "usage") - expect(usageChunks).toHaveLength(1) - }) - - it("should handle stream without usage metrics", async () => { - async function* mockFullStream() { - yield { type: "text-delta", text: "Test response" } - } - - mockStreamText.mockReturnValue({ - fullStream: mockFullStream(), - usage: Promise.resolve(undefined), - }) - - const stream = handler.createMessage(systemPrompt, messages) - const chunks: ApiStreamChunk[] = [] - for await (const chunk of stream) { - chunks.push(chunk) - } - - expect(chunks.length).toBeGreaterThan(0) - const usageChunks = chunks.filter((chunk) => chunk.type === "usage") - expect(usageChunks).toHaveLength(0) - }) - - it("should handle tool-call events in stream", async () => { - async function* mockFullStream() { - yield { type: "text-delta", text: "Calling tool" } - yield { type: "tool-input-start", id: "tc_1", toolName: "read_file" } - yield { type: "tool-input-delta", id: "tc_1", delta: '{"path":"test.ts"}' } - yield { type: "tool-input-end", id: "tc_1" } - } - - const mockUsage = Promise.resolve({ - inputTokens: 15, - outputTokens: 8, - details: {}, - raw: {}, - }) - - mockStreamText.mockReturnValue({ - fullStream: mockFullStream(), - usage: mockUsage, - }) - - const stream = handler.createMessage(systemPrompt, messages) - const chunks: ApiStreamChunk[] = [] - for await (const chunk of stream) { - chunks.push(chunk) - } - - const toolChunks = chunks.filter( - (chunk) => - chunk.type === "tool_call_start" || - chunk.type === "tool_call_delta" || - chunk.type === "tool_call_end", - ) - expect(toolChunks.map((chunk) => chunk.type)).toEqual([ - "tool_call_start", - "tool_call_delta", - "tool_call_end", - ]) - expect(toolChunks[0]).toMatchObject({ type: "tool_call_start", id: "tc_1", name: "read_file" }) - expect(toolChunks[1]).toMatchObject({ type: "tool_call_delta", id: "tc_1", delta: '{"path":"test.ts"}' }) - expect(toolChunks[2]).toMatchObject({ type: "tool_call_end", id: "tc_1" }) - }) - - // Test 1: createMessage() with a real AI SDK APICallError 429 (the AI SDK - // exposes the HTTP status as statusCode, not status) → verify the wrapped error - // surfaces .status === 429 and the provider name in its message. - it("should throw error with .status 429 when API returns 429", async () => { - const rateLimitError = new APICallError({ - message: "Rate limited", - statusCode: 429, - url: "https://test.invalid/v1/chat/completions", - requestBodyValues: {}, - }) - - mockStreamText.mockReturnValue({ - // eslint-disable-next-line require-yield - fullStream: (async function* () { - throw rateLimitError - })(), - usage: Promise.resolve({ inputTokens: 0, outputTokens: 0, details: {}, raw: {} }), - }) - - let thrownError: StatusedError | undefined - try { - for await (const chunk of handler.createMessage(systemPrompt, messages)) { - void chunk // Use void to satisfy no-unused-expressions rule - } - } catch (e) { - thrownError = e as StatusedError - } - - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected createMessage to throw") - } - expect(thrownError.status).toBe(429) - expect(thrownError.message).toContain("TestProvider") - }) - - // Test 2: createMessage() with mock 500 response → verify error is properly tagged - it("should throw error with .status 500 and provider name when API returns 500", async () => { - const serverError = Object.assign(new Error("Internal Server Error"), { status: 500 }) - - mockStreamText.mockReturnValue({ - // eslint-disable-next-line require-yield - fullStream: (async function* () { - throw serverError - })(), - usage: Promise.resolve({ inputTokens: 0, outputTokens: 0, details: {}, raw: {} }), - }) - - let thrownError: StatusedError | undefined - try { - for await (const chunk of handler.createMessage(systemPrompt, messages)) { - void chunk - } - } catch (e) { - thrownError = e as StatusedError - } - - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected createMessage to throw") - } - expect(thrownError.status).toBe(500) - expect(thrownError.message).toContain("TestProvider") - }) - - it("should surface a streamed error part through handleOpenAIError with status and provider name", async () => { - const rateLimitError = Object.assign(new Error("Rate limited"), { status: 429 }) - - mockStreamText.mockReturnValue({ - fullStream: (async function* () { - yield { type: "text-delta", text: "partial" } - yield { type: "error", error: rateLimitError } - })(), - usage: Promise.resolve({ inputTokens: 0, outputTokens: 0, details: {}, raw: {} }), - }) - - const chunks: ApiStreamChunk[] = [] - let thrownError: StatusedError | undefined - try { - for await (const chunk of handler.createMessage(systemPrompt, messages)) { - chunks.push(chunk) - } - } catch (e) { - thrownError = e as StatusedError - } - - // The text delta before the error part is still yielded; the error part itself - // must throw the wrapped error instead of being emitted as a chunk. - expect(chunks).toEqual([{ type: "text", text: "partial" }]) - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected createMessage to throw") - } - expect(thrownError.status).toBe(429) - // Single wrap: exactly one provider prefix, so a redundant re-wrap inside the - // stream loop would duplicate it and fail this exact-message assertion. - expect(thrownError.message).toBe("TestProvider completion error: Rate limited") - }) - - it("should wrap a synchronous streamText() failure with status and provider name", async () => { - const rateLimitError = Object.assign(new Error("Rate limited"), { status: 429 }) - - mockStreamText.mockImplementation(() => { - throw rateLimitError - }) - - let thrownError: StatusedError | undefined - try { - // The generator body runs on the first next(): the synchronous streamText() - // throw must surface here as a wrapped error with status and provider name. - await handler.createMessage(systemPrompt, messages).next() - } catch (e) { - thrownError = e as StatusedError - } - - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected createMessage to throw") - } - expect(thrownError.status).toBe(429) - expect(thrownError.message).toContain("TestProvider") - }) - - // The usage read happens after the stream drains: a rejection there must still be - // wrapped once with status and provider name by the same catch path. - it("should wrap a result.usage failure with status and provider name", async () => { - const usageError = Object.assign(new Error("usage failed"), { status: 500 }) - - mockStreamText.mockReturnValue({ - fullStream: (async function* () { - yield { type: "text-delta", text: "done" } - })(), - usage: Promise.reject(usageError), - }) - - const chunks: ApiStreamChunk[] = [] - let thrownError: StatusedError | undefined - try { - for await (const chunk of handler.createMessage(systemPrompt, messages)) { - chunks.push(chunk) - } - } catch (e) { - thrownError = e as StatusedError - } - - expect(chunks).toEqual([{ type: "text", text: "done" }]) - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected createMessage to throw") - } - expect(thrownError.status).toBe(500) - expect(thrownError.message).toBe("TestProvider completion error: usage failed") - }) - }) - - describe("completePrompt", () => { - it("should complete a prompt using generateText", async () => { - mockGenerateText.mockResolvedValue({ - text: "Test completion", - }) - - const result = await handler.completePrompt("Test prompt") - - expect(result).toBe("Test completion") - expect(mockGenerateText).toHaveBeenCalledWith( - expect.objectContaining({ - prompt: "Test prompt", - }), - ) - }) - - // The configured temperature must reach generateText unchanged: the `?? 0` fallback must - // not degrade to a boolean-and (which would hand generateText 0 or undefined instead of the - // configured value). - it("passes the configured temperature through to generateText", async () => { - mockConfig.temperature = 0.7 - const handlerWithTemp = new TestOpenAICompatibleHandler(mockOptions, mockConfig) - mockGenerateText.mockResolvedValue({ text: "ok" }) - - await handlerWithTemp.completePrompt("Test prompt") - - expect(mockGenerateText).toHaveBeenCalledWith( - expect.objectContaining({ - prompt: "Test prompt", - temperature: 0.7, - }), - ) - }) - - // Test 3: completePrompt() with mock 4xx/5xx → verify error carries .status and provider name - it("should throw error with .status and provider name when generateText throws 400", async () => { - const badRequestError = Object.assign(new Error("Bad Request"), { status: 400 }) - - mockGenerateText.mockRejectedValue(badRequestError) - - await expect(handler.completePrompt("Test prompt")).rejects.toThrow("TestProvider") - - let thrownError: StatusedError | undefined - try { - await handler.completePrompt("Test prompt") - } catch (e) { - thrownError = e as StatusedError - } - - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected completePrompt to throw") - } - expect(thrownError.status).toBe(400) - expect(thrownError.message).toContain("TestProvider") - }) - - it("should throw error with .status and provider name when generateText throws 500", async () => { - const serverError = Object.assign(new Error("Internal Server Error"), { status: 500 }) - - mockGenerateText.mockRejectedValue(serverError) - - let thrownError: StatusedError | undefined - try { - await handler.completePrompt("Test prompt") - } catch (e) { - thrownError = e as StatusedError - } - - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected completePrompt to throw") - } - expect(thrownError.status).toBe(500) - expect(thrownError.message).toContain("TestProvider") - }) - - // The language-model lookup fails before generateText() is ever reached: the - // completePrompt try block must wrap that failure with status and provider name. - it("should wrap a getLanguageModel() failure with status and provider name", async () => { - const modelError = Object.assign(new Error("model unavailable"), { status: 401 }) - - class FailingModelHandler extends TestOpenAICompatibleHandler { - override getLanguageModel(): LanguageModel { - throw modelError - } - } - const failingHandler = new FailingModelHandler(mockOptions, mockConfig) - - let thrownError: StatusedError | undefined - try { - await failingHandler.completePrompt("Test prompt") - } catch (e) { - thrownError = e as StatusedError - } - - expect(thrownError).toBeInstanceOf(Error) - if (!thrownError) { - throw new Error("Expected completePrompt to throw") - } - expect(thrownError.status).toBe(401) - expect(thrownError.message).toContain("TestProvider") - expect(mockGenerateText).not.toHaveBeenCalled() - }) - }) -}) diff --git a/src/api/providers/fetchers/__tests__/versionedSettings.spec.ts b/src/api/providers/fetchers/__tests__/versionedSettings.spec.ts deleted file mode 100644 index cf45a1b5bf..0000000000 --- a/src/api/providers/fetchers/__tests__/versionedSettings.spec.ts +++ /dev/null @@ -1,263 +0,0 @@ -import { - compareSemver, - meetsMinimumVersion, - findHighestMatchingVersion, - resolveVersionedSettings, - type VersionedSettings, -} from "../versionedSettings" - -describe("versionedSettings", () => { - describe("compareSemver", () => { - it("should return 0 for equal versions", () => { - expect(compareSemver("1.0.0", "1.0.0")).toBe(0) - expect(compareSemver("3.36.4", "3.36.4")).toBe(0) - expect(compareSemver("0.0.1", "0.0.1")).toBe(0) - }) - - it("should return positive when first version is greater", () => { - expect(compareSemver("2.0.0", "1.0.0")).toBeGreaterThan(0) - expect(compareSemver("1.1.0", "1.0.0")).toBeGreaterThan(0) - expect(compareSemver("1.0.1", "1.0.0")).toBeGreaterThan(0) - expect(compareSemver("3.36.5", "3.36.4")).toBeGreaterThan(0) - expect(compareSemver("3.37.0", "3.36.4")).toBeGreaterThan(0) - expect(compareSemver("4.0.0", "3.36.4")).toBeGreaterThan(0) - }) - - it("should return negative when first version is smaller", () => { - expect(compareSemver("1.0.0", "2.0.0")).toBeLessThan(0) - expect(compareSemver("1.0.0", "1.1.0")).toBeLessThan(0) - expect(compareSemver("1.0.0", "1.0.1")).toBeLessThan(0) - expect(compareSemver("3.36.3", "3.36.4")).toBeLessThan(0) - expect(compareSemver("3.35.0", "3.36.4")).toBeLessThan(0) - expect(compareSemver("2.0.0", "3.36.4")).toBeLessThan(0) - }) - - it("should handle pre-release versions by ignoring pre-release suffix", () => { - expect(compareSemver("3.36.4-beta.1", "3.36.4")).toBe(0) - expect(compareSemver("3.36.4-rc.2", "3.36.4")).toBe(0) - expect(compareSemver("3.36.5-alpha", "3.36.4")).toBeGreaterThan(0) - expect(compareSemver("3.36.3-beta", "3.36.4")).toBeLessThan(0) - }) - - it("should handle edge cases", () => { - expect(compareSemver("0.0.0", "0.0.0")).toBe(0) - expect(compareSemver("10.20.30", "10.20.30")).toBe(0) - expect(compareSemver("10.0.0", "9.99.99")).toBeGreaterThan(0) - }) - }) - - describe("meetsMinimumVersion", () => { - it("should return true when current version equals minimum", () => { - expect(meetsMinimumVersion("3.36.4", "3.36.4")).toBe(true) - }) - - it("should return true when current version exceeds minimum", () => { - expect(meetsMinimumVersion("3.36.4", "3.36.5")).toBe(true) - expect(meetsMinimumVersion("3.36.4", "3.37.0")).toBe(true) - expect(meetsMinimumVersion("3.36.4", "4.0.0")).toBe(true) - }) - - it("should return false when current version is below minimum", () => { - expect(meetsMinimumVersion("3.36.4", "3.36.3")).toBe(false) - expect(meetsMinimumVersion("3.36.4", "3.35.0")).toBe(false) - expect(meetsMinimumVersion("3.36.4", "2.0.0")).toBe(false) - }) - }) - - describe("findHighestMatchingVersion", () => { - it("should return undefined when no versions match", () => { - const versionedSettings: VersionedSettings = { - "4.0.0": { includedTools: ["apply_diff"] }, - "5.0.0": { includedTools: ["apply_diff", "search_replace"] }, - } - - const result = findHighestMatchingVersion(versionedSettings, "3.36.4") - expect(result).toBeUndefined() - }) - - it("should return the exact version when it matches", () => { - const versionedSettings: VersionedSettings = { - "3.36.4": { includedTools: ["apply_diff"] }, - "3.35.0": { includedTools: ["search_replace"] }, - } - - const result = findHighestMatchingVersion(versionedSettings, "3.36.4") - expect(result).toBe("3.36.4") - }) - - it("should return the highest version that is <= current version", () => { - const versionedSettings: VersionedSettings = { - "3.37.0": { includedTools: ["future_tool"] }, - "3.36.4": { includedTools: ["apply_diff"] }, - "3.35.0": { includedTools: ["search_replace"] }, - "3.34.0": { includedTools: ["basic_tool"] }, - } - - // Current version is 3.36.5, should match 3.36.4 (highest <= 3.36.5) - const result = findHighestMatchingVersion(versionedSettings, "3.36.5") - expect(result).toBe("3.36.4") - }) - - it("should handle single version", () => { - const versionedSettings: VersionedSettings = { - "3.35.0": { includedTools: ["search_replace"] }, - } - - expect(findHighestMatchingVersion(versionedSettings, "3.36.4")).toBe("3.35.0") - expect(findHighestMatchingVersion(versionedSettings, "3.34.0")).toBeUndefined() - }) - - it("should handle empty versionedSettings", () => { - const versionedSettings: VersionedSettings = {} - - const result = findHighestMatchingVersion(versionedSettings, "3.36.4") - expect(result).toBeUndefined() - }) - }) - - describe("resolveVersionedSettings", () => { - const currentVersion = "3.36.4" - - it("should return settings for exact version match", () => { - const versionedSettings: VersionedSettings = { - "3.36.4": { - includedTools: ["search_replace"], - excludedTools: ["apply_diff"], - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, currentVersion) - - expect(resolved).toEqual({ - includedTools: ["search_replace"], - excludedTools: ["apply_diff"], - }) - }) - - it("should return settings for highest matching version", () => { - const versionedSettings: VersionedSettings = { - "4.0.0": { - includedTools: ["future_tool"], - }, - "3.36.0": { - includedTools: ["search_replace"], - excludedTools: ["apply_diff"], - }, - "3.35.0": { - includedTools: ["old_tool"], - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, currentVersion) - - expect(resolved).toEqual({ - includedTools: ["search_replace"], - excludedTools: ["apply_diff"], - }) - }) - - it("should return empty object when no versions match", () => { - const versionedSettings: VersionedSettings = { - "4.0.0": { - includedTools: ["future_tool"], - }, - "3.37.0": { - includedTools: ["newer_tool"], - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, currentVersion) - - expect(resolved).toEqual({}) - }) - - it("should handle empty versionedSettings", () => { - const resolved = resolveVersionedSettings({}, currentVersion) - expect(resolved).toEqual({}) - }) - - it("should handle versioned boolean values", () => { - const versionedSettings: VersionedSettings = { - "3.36.0": { - supportsReasoningEffort: true, - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, currentVersion) - - expect(resolved).toEqual({ - supportsReasoningEffort: true, - }) - }) - - it("should handle versioned null values", () => { - const versionedSettings: VersionedSettings = { - "3.36.0": { - defaultTemperature: null, - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, currentVersion) - - expect(resolved).toEqual({ - defaultTemperature: null, - }) - }) - - it("should handle versioned nested objects", () => { - const versionedSettings: VersionedSettings = { - "3.36.0": { - complexSetting: { nested: { deeply: true } }, - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, currentVersion) - - expect(resolved).toEqual({ - complexSetting: { nested: { deeply: true } }, - }) - }) - - it("should use all settings from the matching version", () => { - const versionedSettings: VersionedSettings = { - "3.36.4": { - includedTools: ["search_replace", "apply_diff"], - excludedTools: ["write_to_file"], - supportsReasoningEffort: true, - description: "Updated model", - }, - "3.35.0": { - includedTools: ["search_replace"], - description: "Old model", - }, - } - - const resolved = resolveVersionedSettings(versionedSettings, "3.36.4") - - expect(resolved).toEqual({ - includedTools: ["search_replace", "apply_diff"], - excludedTools: ["write_to_file"], - supportsReasoningEffort: true, - description: "Updated model", - }) - }) - - it("should handle multiple versions and select correct one", () => { - const versionedSettings: VersionedSettings = { - "3.38.0": { feature: "very_new" }, - "3.37.0": { feature: "new" }, - "3.36.0": { feature: "current" }, - "3.35.0": { feature: "old" }, - "3.34.0": { feature: "very_old" }, - } - - // Test different current versions - expect(resolveVersionedSettings(versionedSettings, "3.40.0")).toEqual({ feature: "very_new" }) - expect(resolveVersionedSettings(versionedSettings, "3.37.5")).toEqual({ feature: "new" }) - expect(resolveVersionedSettings(versionedSettings, "3.36.5")).toEqual({ feature: "current" }) - expect(resolveVersionedSettings(versionedSettings, "3.35.5")).toEqual({ feature: "old" }) - expect(resolveVersionedSettings(versionedSettings, "3.34.5")).toEqual({ feature: "very_old" }) - expect(resolveVersionedSettings(versionedSettings, "3.33.0")).toEqual({}) - }) - }) -}) diff --git a/src/api/providers/fetchers/modelEndpointCache.ts b/src/api/providers/fetchers/modelEndpointCache.ts index 7a00cfce4f..23919f6cd0 100644 --- a/src/api/providers/fetchers/modelEndpointCache.ts +++ b/src/api/providers/fetchers/modelEndpointCache.ts @@ -103,6 +103,3 @@ export const getModelEndpoints = async ( return modelProviders ?? {} } - -export const flushModelProviders = async (router: RouterName, modelId: string) => - memoryCache.del(getCacheKey(router, modelId)) diff --git a/src/api/providers/fetchers/openrouter.ts b/src/api/providers/fetchers/openrouter.ts index 426ba89d8c..a8a780c8a3 100644 --- a/src/api/providers/fetchers/openrouter.ts +++ b/src/api/providers/fetchers/openrouter.ts @@ -41,10 +41,6 @@ const modelRouterBaseModelSchema = z.object({ export type OpenRouterBaseModel = z.infer -/** - * OpenRouterModel - */ - export const openRouterModelSchema = modelRouterBaseModelSchema.extend({ id: z.string(), architecture: openRouterArchitectureSchema.optional(), @@ -52,10 +48,8 @@ export const openRouterModelSchema = modelRouterBaseModelSchema.extend({ supported_parameters: z.array(z.string()).optional(), }) -export type OpenRouterModel = z.infer - /** - * OpenRouterModelEndpoint + * @internal */ export const openRouterModelEndpointSchema = modelRouterBaseModelSchema.extend({ @@ -63,7 +57,9 @@ export const openRouterModelEndpointSchema = modelRouterBaseModelSchema.extend({ tag: z.string().optional(), }) -export type OpenRouterModelEndpoint = z.infer +/** + * @internal + */ /** * OpenRouterModelsResponse diff --git a/src/api/providers/fetchers/versionedSettings.ts b/src/api/providers/fetchers/versionedSettings.ts deleted file mode 100644 index 87f51cc880..0000000000 --- a/src/api/providers/fetchers/versionedSettings.ts +++ /dev/null @@ -1,117 +0,0 @@ -import cmp from "semver-compare" - -import { Package } from "../../../shared/package" - -/** - * Type for versioned settings where the version is the key. - * Each version key maps to a settings object that should be used - * when the current plugin version is >= that version. - * - * Example API response: - * ``` - * { - * settings: { - * includedTools: ['search_replace'] // Plain value for old clients - * }, - * versionedSettings: { - * '3.36.4': { - * includedTools: ['search_replace', 'apply_diff'], // Enhanced value for 3.36.4+ - * excludedTools: ['write_to_file'], - * }, - * '3.35.0': { - * includedTools: ['search_replace'], // Value for 3.35.0 - 3.36.3 - * }, - * } - * } - * ``` - * - * The resolver will find the highest version key that is <= the current plugin version - * and use those settings. If no version matches, falls back to plain `settings`. - */ -export type VersionedSettings = Record> - -/** - * Compares two semantic version strings using semver-compare. - * - * @param version1 First version string (e.g., "3.36.4") - * @param version2 Second version string (e.g., "3.36.0") - * @returns -1 if version1 < version2, 0 if equal, 1 if version1 > version2 - */ -export function compareSemver(version1: string, version2: string): number { - // Handle pre-release versions by stripping the suffix - // semver-compare doesn't handle pre-release properly - const stripPrerelease = (v: string): string => v.split("-")[0] - return cmp(stripPrerelease(version1), stripPrerelease(version2)) -} - -/** - * Checks if the current plugin version meets or exceeds the required minimum version. - * - * @param minPluginVersion The minimum required version - * @param currentVersion The current plugin version (defaults to Package.version) - * @returns true if current version >= minPluginVersion - */ -export function meetsMinimumVersion(minPluginVersion: string, currentVersion: string = Package.version): boolean { - return compareSemver(currentVersion, minPluginVersion) >= 0 -} - -/** - * Finds the highest version from versionedSettings that is <= the current plugin version. - * - * @param versionedSettings The versioned settings object with version keys - * @param currentVersion The current plugin version (defaults to Package.version) - * @returns The highest matching version key, or undefined if none match - */ -export function findHighestMatchingVersion( - versionedSettings: VersionedSettings, - currentVersion: string = Package.version, -): string | undefined { - const versions = Object.keys(versionedSettings) - - if (versions.length === 0) { - return undefined - } - - // Filter to versions that are <= currentVersion - const matchingVersions = versions.filter((version) => meetsMinimumVersion(version, currentVersion)) - - if (matchingVersions.length === 0) { - return undefined - } - - // Sort in descending order and return the highest - matchingVersions.sort((a, b) => compareSemver(b, a)) - return matchingVersions[0] -} - -/** - * Resolves versioned settings by finding the highest version that is <= the current - * plugin version and returning those settings. - * - * The versionedSettings structure uses version numbers as keys: - * ``` - * versionedSettings: { - * '3.36.4': { includedTools: ['search_replace'], excludedTools: ['apply_diff'] }, - * '3.35.0': { includedTools: ['search_replace'] }, - * } - * ``` - * - * This function finds the highest version key that is <= currentVersion and returns - * the corresponding settings object. If no version matches, returns an empty object. - * - * @param versionedSettings The versioned settings object with version keys - * @param currentVersion The current plugin version (defaults to Package.version) - * @returns The settings object for the highest matching version, or empty object if none match - */ -export function resolveVersionedSettings>( - versionedSettings: VersionedSettings, - currentVersion: string = Package.version, -): Partial { - const matchingVersion = findHighestMatchingVersion(versionedSettings, currentVersion) - - if (!matchingVersion) { - return {} - } - - return versionedSettings[matchingVersion] as Partial -} diff --git a/src/api/providers/index.ts b/src/api/providers/index.ts index 0153df5fc8..e9beb183e2 100644 --- a/src/api/providers/index.ts +++ b/src/api/providers/index.ts @@ -13,7 +13,6 @@ export { MistralHandler } from "./mistral" export { OpenAiCodexHandler } from "./openai-codex" export { OpenAiNativeHandler } from "./openai-native" export { OpenAiHandler } from "./openai" -export { OpenAICompatibleHandler } from "./openai-compatible" export type { OpenAICompatibleConfig } from "./openai-compatible" export { OpenRouterHandler } from "./openrouter" export { PoeHandler } from "./poe" diff --git a/src/api/providers/lm-studio.ts b/src/api/providers/lm-studio.ts index 57bf9adefa..a8f93738e5 100644 --- a/src/api/providers/lm-studio.ts +++ b/src/api/providers/lm-studio.ts @@ -1,6 +1,5 @@ import { Anthropic } from "@anthropic-ai/sdk" import OpenAI from "openai" -import axios from "axios" import { type ModelInfo, @@ -235,7 +234,7 @@ export class LmStudioHandler extends BaseProvider implements SingleCompletionHan async completePrompt(prompt: string, _options?: CompletePromptOptions): Promise { try { // Create params object with optional draft model - const params: any = { + const params: OpenAI.Chat.ChatCompletionCreateParamsNonStreaming & { draft_model?: string } = { model: this.getModel().id, messages: [{ role: "user", content: prompt }], temperature: this.options.modelTemperature ?? LMSTUDIO_DEFAULT_TEMPERATURE, @@ -261,17 +260,3 @@ export class LmStudioHandler extends BaseProvider implements SingleCompletionHan } } } - -export async function getLmStudioModels(baseUrl = "http://localhost:1234") { - try { - if (!URL.canParse(baseUrl)) { - return [] - } - - const response = await axios.get(`${baseUrl}/v1/models`) - const modelsArray = response.data?.data?.map((model: any) => model.id) || [] - return [...new Set(modelsArray)] - } catch { - return [] - } -} diff --git a/src/api/providers/openai-compatible.ts b/src/api/providers/openai-compatible.ts index 7f1d6b18c3..b46e2a7be9 100644 --- a/src/api/providers/openai-compatible.ts +++ b/src/api/providers/openai-compatible.ts @@ -1,27 +1,9 @@ -/** - * OpenAI-compatible provider base class using Vercel AI SDK. - * This provides a parallel implementation to OpenAiHandler using @ai-sdk/openai-compatible. - */ - -import { Anthropic } from "@anthropic-ai/sdk" -import OpenAI from "openai" -import { createOpenAICompatible } from "@ai-sdk/openai-compatible" -import { streamText, generateText, LanguageModel, ToolSet } from "ai" - import type { ModelInfo } from "@roo-code/types" -import type { ApiHandlerOptions } from "../../shared/api" - -import { convertToAiSdkMessages, convertToolsForAiSdk, processAiSdkStreamPart } from "../transform/ai-sdk" -import { ApiStream, ApiStreamUsageChunk } from "../transform/stream" - -import { DEFAULT_HEADERS } from "./constants" -import { handleOpenAIError } from "./utils/error-handler" -import { BaseProvider } from "./base-provider" -import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" - /** * Configuration options for creating an OpenAI-compatible provider. + * + * @public */ export interface OpenAICompatibleConfig { /** Provider name for identification */ @@ -43,186 +25,3 @@ export interface OpenAICompatibleConfig { /** Temperature setting */ temperature?: number } - -/** - * Base class for OpenAI-compatible API providers using Vercel AI SDK. - * Extends BaseProvider and implements SingleCompletionHandler. - */ -export abstract class OpenAICompatibleHandler extends BaseProvider implements SingleCompletionHandler { - protected options: ApiHandlerOptions - protected config: OpenAICompatibleConfig - protected provider: ReturnType - - constructor(options: ApiHandlerOptions, config: OpenAICompatibleConfig) { - super() - this.options = options - this.config = config - - // Create the OpenAI-compatible provider using AI SDK - this.provider = createOpenAICompatible({ - name: config.providerName, - baseURL: config.baseURL, - apiKey: config.apiKey, - headers: { - ...DEFAULT_HEADERS, - ...(config.headers || {}), - }, - }) - } - - /** - * Get the language model for the configured model ID. - */ - protected getLanguageModel(): LanguageModel { - return this.provider(this.config.modelId) - } - - /** - * Get the model information. Must be implemented by subclasses. - */ - abstract override getModel(): { id: string; info: ModelInfo; maxTokens?: number; temperature?: number } - - /** - * Process usage metrics from the AI SDK response. - * Can be overridden by subclasses to handle provider-specific usage formats. - */ - protected processUsageMetrics(usage: { - inputTokens?: number - outputTokens?: number - inputTokenDetails?: { cacheReadTokens?: number } - outputTokenDetails?: { reasoningTokens?: number } - raw?: Record - }): ApiStreamUsageChunk { - return { - type: "usage", - inputTokens: usage.inputTokens || 0, - outputTokens: usage.outputTokens || 0, - cacheReadTokens: usage.inputTokenDetails?.cacheReadTokens, - reasoningTokens: usage.outputTokenDetails?.reasoningTokens, - } - } - - /** - * Map OpenAI tool_choice to AI SDK toolChoice format. - */ - protected mapToolChoice( - toolChoice: OpenAI.Chat.ChatCompletionCreateParams["tool_choice"], - ): "auto" | "none" | "required" | { type: "tool"; toolName: string } | undefined { - if (!toolChoice) { - return undefined - } - - // Handle string values - if (typeof toolChoice === "string") { - switch (toolChoice) { - case "auto": - return "auto" - case "none": - return "none" - case "required": - return "required" - default: - return "auto" - } - } - - // Handle object values (OpenAI ChatCompletionNamedToolChoice format) - if (typeof toolChoice === "object" && "type" in toolChoice) { - if (toolChoice.type === "function" && "function" in toolChoice && toolChoice.function?.name) { - return { type: "tool", toolName: toolChoice.function.name } - } - } - - return undefined - } - - /** - * Get the max tokens parameter to include in the request. - */ - protected getMaxOutputTokens(): number | undefined { - const modelInfo = this.config.modelInfo - const maxTokens = this.config.modelMaxTokens || modelInfo.maxTokens - - return maxTokens ?? undefined - } - - /** - * Create a message stream using the AI SDK. - */ - override async *createMessage( - systemPrompt: string, - messages: Anthropic.Messages.MessageParam[], - metadata?: ApiHandlerCreateMessageMetadata, - ): ApiStream { - const model = this.getModel() - const languageModel = this.getLanguageModel() - - // Convert messages to AI SDK format - const aiSdkMessages = convertToAiSdkMessages(messages) - - // Convert tools to OpenAI format first, then to AI SDK format - const openAiTools = this.convertToolsForOpenAI(metadata?.tools) - const aiSdkTools = convertToolsForAiSdk(openAiTools) as ToolSet | undefined - - // Build the request options - const requestOptions: Parameters[0] = { - model: languageModel, - system: systemPrompt, - messages: aiSdkMessages, - temperature: model.temperature ?? this.config.temperature ?? 0, - maxOutputTokens: this.getMaxOutputTokens(), - tools: aiSdkTools, - toolChoice: this.mapToolChoice(metadata?.tool_choice), - } - - // Use streamText for streaming responses - try { - const result = streamText(requestOptions) - - // Process the full stream to get all events - for await (const part of result.fullStream) { - // A streamed provider error arrives as an `error` part rather than a throw: - // re-throw the raw part error so the single catch below wraps it exactly once - // with status and provider name (an extra wrap here would duplicate the message - // prefix and log the error twice). Abort parts are a normal termination and fall - // through to processAiSdkStreamPart's ignore case. - if (part.type === "error") { - throw part.error - } - - // Use the processAiSdkStreamPart utility to convert stream parts - for (const chunk of processAiSdkStreamPart(part)) { - yield chunk - } - } - - // Yield usage metrics at the end - const usage = await result.usage - if (usage) { - yield this.processUsageMetrics(usage) - } - } catch (error) { - throw handleOpenAIError(error, this.config.providerName) - } - } - - /** - * Complete a prompt using the AI SDK generateText. - */ - async completePrompt(prompt: string, _options?: CompletePromptOptions): Promise { - try { - const languageModel = this.getLanguageModel() - - const { text } = await generateText({ - model: languageModel, - prompt, - maxOutputTokens: this.getMaxOutputTokens(), - temperature: this.config.temperature ?? 0, - }) - - return text - } catch (error) { - throw handleOpenAIError(error, this.config.providerName) - } - } -} diff --git a/src/api/providers/utils/abort-signal.ts b/src/api/providers/utils/abort-signal.ts index cbb7a13aaf..d2cc7b8c24 100644 --- a/src/api/providers/utils/abort-signal.ts +++ b/src/api/providers/utils/abort-signal.ts @@ -53,17 +53,6 @@ export function throwIfAborted(signal?: AbortSignal): void { throw abortError } -/** - * Request options this series passes to the OpenAI SDK call. The SDK's - * `RequestOptions` declares `signal` as `AbortSignal | null | undefined`, - * which does not satisfy the builder's base constraint, so the builder is - * typed with only the options this series sets. The built config is still - * assignable to the SDK's `RequestOptions`. - */ -export type OpenAiRequestOptions = { - signal?: AbortSignal -} - /** * Whether a failure indicates an aborted request: the caller's signal fired, * an `Error` carries a native abort error name (`AbortError`, @@ -139,9 +128,11 @@ export function rejectOnAbort(pending: Promise, signal: AbortSignal, provi * resolve. Abort failures from the lookup itself are normalized to the * provider AbortError; any other resolution failure propagates unchanged. * - * Providers pass their own resolution step, so the entry guard, the race, and - * the normalization logic exist once and are exercised through every - * provider's spec. + * Accepts a provider-specific resolution step so the entry guard, race, and + * normalization can share one implementation. Direct contract tests cover + * this helper; providers do not currently call it. + * + * @internal Shared cancellation helper retained with direct contract tests for provider adoption. */ export async function resolveModelWithAbort( fetchModel: () => Promise, diff --git a/src/api/providers/utils/image-generation.ts b/src/api/providers/utils/image-generation.ts index add00688e9..44aab4e61f 100644 --- a/src/api/providers/utils/image-generation.ts +++ b/src/api/providers/utils/image-generation.ts @@ -174,6 +174,8 @@ export async function generateImageWithProvider(options: ImageGenerationOptions) /** * Generate an image using OpenAI's Images API (/v1/images/generations) * Supports BFL models (Flux) with provider-specific options for image editing + * + * @internal */ export async function generateImageWithImagesApi(options: ImagesApiOptions): Promise { const { baseURL, authToken, model, prompt, inputImage, outputFormat = "png" } = options diff --git a/src/api/providers/vscode-lm.ts b/src/api/providers/vscode-lm.ts index 5dee3f2acf..7842326b2f 100644 --- a/src/api/providers/vscode-lm.ts +++ b/src/api/providers/vscode-lm.ts @@ -513,7 +513,11 @@ export function middleOutTruncate(text: string, maxChars: number): string { return `${head}${buildMarker(removed)}${tail}` } -/** Estimated character cost of a whole conversation, using the same accounting as truncation. */ +/** + * Estimated character cost of a whole conversation, using the same accounting as truncation. + * + * @internal Exported for focused implementation tests. + */ export function estimateMessagesChars(messages: Anthropic.Messages.MessageParam[]): number { return messages.reduce((sum, message) => sum + estimateContentChars(message.content), 0) } @@ -701,6 +705,8 @@ function parseLeakedInvokeParams( * * `validTools` may be a bare name set (no schemas, so every parameter stays a literal string) or a * map from tool name to its parameter schema, which enables typed conversion. + * + * @internal Exported for focused implementation tests. */ export function extractLeakedToolCalls( text: string, diff --git a/src/api/transform/cache-strategy/types.ts b/src/api/transform/cache-strategy/types.ts index 2b5d5736c9..4215295d6a 100644 --- a/src/api/transform/cache-strategy/types.ts +++ b/src/api/transform/cache-strategy/types.ts @@ -19,14 +19,6 @@ export interface ModelInfo { cachableFields: Array<"system" | "messages" | "tools"> } -/** - * Cache point definition - */ -export interface CachePoint { - /** Type of cache point */ - type: "default" -} - /** * Result of cache strategy application */ diff --git a/src/api/transform/minimax-format.ts b/src/api/transform/minimax-format.ts index 32a32a4437..ed2028ed4c 100644 --- a/src/api/transform/minimax-format.ts +++ b/src/api/transform/minimax-format.ts @@ -100,19 +100,3 @@ export function mergeEnvironmentDetailsForMiniMax( return result } - -/** - * @deprecated Use mergeEnvironmentDetailsForMiniMax instead. This function extracted - * environment_details to the system prompt, but the new approach merges them into - * tool_result content like r1-format does with mergeToolResultText. - */ -export function extractEnvironmentDetailsForMiniMax(messages: Anthropic.Messages.MessageParam[]): { - messages: Anthropic.Messages.MessageParam[] - extractedSystemContent: string[] -} { - // For backwards compatibility, just return the merged messages with empty extracted content - return { - messages: mergeEnvironmentDetailsForMiniMax(messages), - extractedSystemContent: [], - } -} diff --git a/src/api/transform/reasoning.ts b/src/api/transform/reasoning.ts index 0cf0bf05cd..a2cc11aef3 100644 --- a/src/api/transform/reasoning.ts +++ b/src/api/transform/reasoning.ts @@ -68,6 +68,9 @@ export const getOpenRouterReasoning = ({ : undefined : undefined +/** + * @internal + */ export const getRooReasoning = ({ model, reasoningEffort, diff --git a/src/api/transform/vscode-lm-format.ts b/src/api/transform/vscode-lm-format.ts index 8e56d7a60f..8400145132 100644 --- a/src/api/transform/vscode-lm-format.ts +++ b/src/api/transform/vscode-lm-format.ts @@ -236,6 +236,9 @@ export function convertToVsCodeLmMessages( return vsCodeLmMessages } +/** + * @internal + */ export function convertToAnthropicRole(vsCodeLmMessageRole: vscode.LanguageModelChatMessageRole): string | null { switch (vsCodeLmMessageRole) { case vscode.LanguageModelChatMessageRole.Assistant: diff --git a/src/core/auto-approval/commands.ts b/src/core/auto-approval/commands.ts index 9d7289c265..4d28e0fe1b 100644 --- a/src/core/auto-approval/commands.ts +++ b/src/core/auto-approval/commands.ts @@ -113,94 +113,6 @@ export function findLongestPrefixMatch(command: string, prefixes: string[]): str return longestMatch } -/** - * Check if a single command should be auto-approved. - * Returns true only for commands that explicitly match the allowlist - * and either don't match the denylist or have a longer allowlist match. - * - * Special handling for wildcards: "*" in allowlist allows any command, - * but denylist can still block specific commands. - */ -export function isAutoApprovedSingleCommand( - command: string, - allowedCommands: string[], - deniedCommands?: string[], -): boolean { - if (!command) { - return true - } - - // If no allowlist configured, nothing can be auto-approved - if (!allowedCommands?.length) { - return false - } - - // Check if wildcard is present in allowlist - const hasWildcard = allowedCommands.some((cmd) => cmd.toLowerCase() === "*") - - // If no denylist provided (undefined), use simple allowlist logic - if (deniedCommands === undefined) { - const trimmedCommand = command.trim().toLowerCase() - - return allowedCommands.some((prefix) => { - const lowerPrefix = prefix.toLowerCase() - // Handle wildcard "*" - it matches any command - return lowerPrefix === "*" || trimmedCommand.startsWith(lowerPrefix) - }) - } - - // Find longest matching prefix in both lists - const longestDeniedMatch = findLongestPrefixMatch(command, deniedCommands) - const longestAllowedMatch = findLongestPrefixMatch(command, allowedCommands) - - // Special case: if wildcard is present and no denylist match, auto-approve - if (hasWildcard && !longestDeniedMatch) { - return true - } - - // Must have an allowlist match to be auto-approved - if (!longestAllowedMatch) { - return false - } - - // If no denylist match, auto-approve - if (!longestDeniedMatch) { - return true - } - - // Both have matches - allowlist must be longer to auto-approve - return longestAllowedMatch.length > longestDeniedMatch.length -} - -/** - * Check if a single command should be auto-denied. - * Returns true only for commands that explicitly match the denylist - * and either don't match the allowlist or have a longer denylist match. - */ -export function isAutoDeniedSingleCommand( - command: string, - allowedCommands: string[], - deniedCommands?: string[], -): boolean { - if (!command) return false - - // If no denylist configured, nothing can be auto-denied - if (!deniedCommands?.length) return false - - // Find longest matching prefix in both lists - const longestDeniedMatch = findLongestPrefixMatch(command, deniedCommands) - const longestAllowedMatch = findLongestPrefixMatch(command, allowedCommands || []) - - // Must have a denylist match to be auto-denied - if (!longestDeniedMatch) return false - - // If no allowlist match, auto-deny - if (!longestAllowedMatch) return true - - // Both have matches - denylist must be longer or equal to auto-deny - return longestDeniedMatch.length >= longestAllowedMatch.length -} - /** * Command approval decision types */ diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 0db0e9c9ea..9882039ad3 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -295,6 +295,8 @@ export const importSettings = async ({ providerSettingsManager, contextProxy, cu /** * Import settings from a specific file + * + * @internal * @param options - Import options containing managers and proxy * @param fileUri - URI of the file to import from * @returns Promise resolving to import result diff --git a/src/core/message-queue/MessageQueueService.ts b/src/core/message-queue/MessageQueueService.ts index 85a2192217..340fad6087 100644 --- a/src/core/message-queue/MessageQueueService.ts +++ b/src/core/message-queue/MessageQueueService.ts @@ -4,12 +4,6 @@ import { v4 as uuidv4 } from "uuid" import { QueuedMessage } from "@roo-code/types" -export interface MessageQueueState { - messages: QueuedMessage[] - isProcessing: boolean - isPaused: boolean -} - export interface QueueEvents { stateChanged: [messages: QueuedMessage[]] } diff --git a/src/core/prompts/tools/filter-tools-for-mode.ts b/src/core/prompts/tools/filter-tools-for-mode.ts index bdb9408fdc..3921ee79fb 100644 --- a/src/core/prompts/tools/filter-tools-for-mode.ts +++ b/src/core/prompts/tools/filter-tools-for-mode.ts @@ -72,7 +72,11 @@ export function filterNativeToolsForMode( customModes: ModeConfig[] | undefined, experiments: Record | undefined, codeIndexManager?: CodeIndexManager, - settings?: Record, + settings?: { + todoListEnabled?: boolean + disabledTools?: string[] + modelInfo?: Pick + }, mcpHub?: McpHub, allowedMcpServers?: string[], ): OpenAI.Chat.ChatCompletionTool[] { @@ -82,6 +86,7 @@ export function filterNativeToolsForMode( // set — including the protocol-tool rule: a disabledTools entry naming // attempt_completion is ignored, while a model-profile excludedTools entry // keeps it out. + // Tool policy reads only includedTools and excludedTools from model metadata. const modelInfo = settings?.modelInfo as ModelInfo | undefined const policy = resolveEffectiveToolPolicy({ diff --git a/src/core/prompts/tools/native-tools/index.ts b/src/core/prompts/tools/native-tools/index.ts index 758914d2d6..d5f75b590c 100644 --- a/src/core/prompts/tools/native-tools/index.ts +++ b/src/core/prompts/tools/native-tools/index.ts @@ -70,6 +70,3 @@ export function getNativeTools(options: NativeToolsOptions = {}): OpenAI.Chat.Ch writeToFile, ] satisfies OpenAI.Chat.ChatCompletionTool[] } - -// Backward compatibility: export default tools with line ranges enabled -export const nativeTools = getNativeTools() diff --git a/src/core/prompts/tools/native-tools/read_file.ts b/src/core/prompts/tools/native-tools/read_file.ts index 35d6a0d53a..d0851b9a78 100644 --- a/src/core/prompts/tools/native-tools/read_file.ts +++ b/src/core/prompts/tools/native-tools/read_file.ts @@ -152,8 +152,3 @@ export function createReadFileTool(options: ReadFileToolOptions = {}): OpenAI.Ch }, } satisfies OpenAI.Chat.ChatCompletionTool } - -/** - * Default read_file tool with all parameters - */ -export const read_file = createReadFileTool() diff --git a/src/core/task/build-tools.ts b/src/core/task/build-tools.ts index 9d395eaa21..c5ee7a163c 100644 --- a/src/core/task/build-tools.ts +++ b/src/core/task/build-tools.ts @@ -66,11 +66,6 @@ function getToolName(tool: OpenAI.Chat.ChatCompletionTool): string { * @param options - Configuration options for building the tools * @returns Array of filtered native and MCP tools */ -export async function buildNativeToolsArray(options: BuildToolsOptions): Promise { - const result = await buildNativeToolsArrayWithRestrictions(options) - return result.tools -} - /** * Builds the complete tools array for native protocol requests with optional mode restrictions. * When includeAllToolsWithRestrictions is true, returns ALL tools but also provides diff --git a/src/core/tools/EditTool.ts b/src/core/tools/EditTool.ts index b121fcc096..895b929ed2 100644 --- a/src/core/tools/EditTool.ts +++ b/src/core/tools/EditTool.ts @@ -274,4 +274,3 @@ function escapeRegExp(input: string): string { } export const editTool = new EditTool() -export const searchAndReplaceTool = editTool // alias for backward compat diff --git a/src/core/tools/SearchAndReplaceTool.ts b/src/core/tools/SearchAndReplaceTool.ts deleted file mode 100644 index 1ce22aa079..0000000000 --- a/src/core/tools/SearchAndReplaceTool.ts +++ /dev/null @@ -1,2 +0,0 @@ -// Deprecated: Use EditTool instead. This file exists only for backward compatibility. -export { EditTool as SearchAndReplaceTool, searchAndReplaceTool } from "./EditTool" diff --git a/src/core/tools/UpdateTodoListTool.ts b/src/core/tools/UpdateTodoListTool.ts index 4317696da6..08edec2ed5 100644 --- a/src/core/tools/UpdateTodoListTool.ts +++ b/src/core/tools/UpdateTodoListTool.ts @@ -111,45 +111,6 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { } } -export function addTodoToTask(cline: Task, content: string, status: TodoStatus = "pending", id?: string): TodoItem { - const todo: TodoItem = { - id: id ?? crypto.randomUUID(), - content, - status, - } - if (!cline.todoList) cline.todoList = [] - cline.todoList.push(todo) - return todo -} - -export function updateTodoStatusForTask(cline: Task, id: string, nextStatus: TodoStatus): boolean { - if (!cline.todoList) return false - const idx = cline.todoList.findIndex((t) => t.id === id) - if (idx === -1) return false - const current = cline.todoList[idx] - if ( - (current.status === "pending" && nextStatus === "in_progress") || - (current.status === "in_progress" && nextStatus === "completed") || - current.status === nextStatus - ) { - cline.todoList[idx] = { ...current, status: nextStatus } - return true - } - return false -} - -export function removeTodoFromTask(cline: Task, id: string): boolean { - if (!cline.todoList) return false - const idx = cline.todoList.findIndex((t) => t.id === id) - if (idx === -1) return false - cline.todoList.splice(idx, 1) - return true -} - -export function getTodoListForTask(cline: Task): TodoItem[] | undefined { - return cline.todoList?.slice() -} - export async function setTodoListForTask(cline?: Task, todos?: TodoItem[]) { if (cline === undefined) return cline.todoList = Array.isArray(todos) ? todos : [] diff --git a/src/core/tools/__tests__/searchAndReplaceTool.spec.ts b/src/core/tools/__tests__/searchAndReplaceTool.spec.ts deleted file mode 100644 index 53d3ee1125..0000000000 --- a/src/core/tools/__tests__/searchAndReplaceTool.spec.ts +++ /dev/null @@ -1,13 +0,0 @@ -// Deprecated: Tests for the old SearchAndReplaceTool. -// Full edit tool tests are in editTool.spec.ts. -// This file only verifies the backward-compatible re-export. - -import { searchAndReplaceTool } from "../SearchAndReplaceTool" -import { editTool } from "../EditTool" - -describe("SearchAndReplaceTool re-export", () => { - it("exports searchAndReplaceTool as an alias for editTool", () => { - expect(searchAndReplaceTool).toBeDefined() - expect(searchAndReplaceTool).toBe(editTool) - }) -}) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 18c255afe4..0c0498ba8a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -143,10 +143,6 @@ import { PendingEditOperationStore, type PendingEditOperationInput } from "./Pen * https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/customSidebarViewProvider.ts */ -export type ClineProviderEvents = { - clineCreated: [cline: Task] -} - type DelegatedChildContext = TaskExecutionContext function runDelegationTransition( diff --git a/src/core/webview/checkpointRestoreHandler.ts b/src/core/webview/checkpointRestoreHandler.ts index c7e0fb9708..bf276e46bc 100644 --- a/src/core/webview/checkpointRestoreHandler.ts +++ b/src/core/webview/checkpointRestoreHandler.ts @@ -3,7 +3,6 @@ import { ClineProvider } from "./ClineProvider" import { saveTaskMessages } from "../task-persistence" import * as vscode from "vscode" import pWaitFor from "p-wait-for" -import { t } from "../../i18n" export interface CheckpointRestoreConfig { provider: ClineProvider @@ -86,19 +85,3 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore throw error } } - -/** - * Common checkpoint restore validation and initialization utility. - * This can be used by any checkpoint restore flow that needs to wait for initialization. - */ -export async function waitForClineInitialization(provider: ClineProvider, timeoutMs: number = 3000): Promise { - try { - await pWaitFor(() => provider.getCurrentTask()?.isInitialized === true, { - timeout: timeoutMs, - }) - return true - } catch { - vscode.window.showErrorMessage(t("common:errors.checkpoint_timeout")) - return false - } -} diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index f1b2590f7f..3153c00f55 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -369,11 +369,6 @@ "count": 8 } }, - "api/providers/lm-studio.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 2 - } - }, "api/providers/mimo.ts": { "@typescript-eslint/no-explicit-any": { "count": 5 @@ -754,11 +749,6 @@ "count": 14 } }, - "core/prompts/tools/filter-tools-for-mode.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 1 - } - }, "core/prompts/tools/native-tools/__tests__/converters.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 2 @@ -1174,11 +1164,6 @@ "count": 1 } }, - "integrations/misc/__tests__/line-counter.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 2 - } - }, "integrations/misc/__tests__/open-file.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 11 diff --git a/src/i18n/index.ts b/src/i18n/index.ts index 7242e8fee9..a98fce9720 100644 --- a/src/i18n/index.ts +++ b/src/i18n/index.ts @@ -9,15 +9,6 @@ export function initializeI18n(language: string): void { i18next.changeLanguage(language) } -/** - * Get the current language - * - * @returns The current language code - */ -export function getCurrentLanguage(): string { - return i18next.language -} - /** * Change the current language * diff --git a/src/integrations/misc/__tests__/line-counter.spec.ts b/src/integrations/misc/__tests__/line-counter.spec.ts deleted file mode 100644 index 68011cdc2c..0000000000 --- a/src/integrations/misc/__tests__/line-counter.spec.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest" -import { countFileLines, countFileLinesAndTokens } from "../line-counter" -import { countTokens } from "../../../utils/countTokens" -import { Readable } from "stream" - -// Mock dependencies -vi.mock("fs", () => ({ - default: { - promises: { - access: vi.fn(), - }, - constants: { - F_OK: 0, - }, - createReadStream: vi.fn(), - }, - createReadStream: vi.fn(), -})) - -vi.mock("../../../utils/countTokens", () => ({ - countTokens: vi.fn(), -})) - -const mockCountTokens = vi.mocked(countTokens) - -// Get the mocked fs module -const fs = await import("fs") -const mockCreateReadStream = vi.mocked(fs.createReadStream) -const mockFsAccess = vi.mocked(fs.default.promises.access) - -describe("line-counter", () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - describe("countFileLinesAndTokens", () => { - it("should count lines and tokens without budget limit", async () => { - // Create a proper readable stream - const mockStream = new Readable({ - read() { - this.push("line1\n") - this.push("line2\n") - this.push("line3\n") - this.push(null) // End of stream - }, - }) - - mockCreateReadStream.mockReturnValue(mockStream as any) - mockFsAccess.mockResolvedValue(undefined) - - // Mock token counting - simulate ~10 tokens per chunk - mockCountTokens.mockResolvedValue(30) - - const result = await countFileLinesAndTokens("/test/file.txt") - - expect(result.lineCount).toBe(3) - expect(result.tokenEstimate).toBe(30) - expect(result.complete).toBe(true) - }) - - it("should handle tokenizer errors with conservative estimate", async () => { - // Create a proper readable stream - const mockStream = new Readable({ - read() { - this.push("line1\n") - this.push(null) - }, - }) - - mockCreateReadStream.mockReturnValue(mockStream as any) - mockFsAccess.mockResolvedValue(undefined) - - // Simulate tokenizer error - mockCountTokens.mockRejectedValue(new Error("unreachable")) - - const result = await countFileLinesAndTokens("/test/file.txt") - - // Should still complete with conservative token estimate (content.length) - expect(result.lineCount).toBe(1) - expect(result.tokenEstimate).toBeGreaterThan(0) - expect(result.complete).toBe(true) - }) - - it("should throw error for non-existent files", async () => { - mockFsAccess.mockRejectedValue(new Error("ENOENT")) - - await expect(countFileLinesAndTokens("/nonexistent/file.txt")).rejects.toThrow("File not found") - }) - }) - - describe("countFileLines", () => { - it("should throw error for non-existent files", async () => { - mockFsAccess.mockRejectedValue(new Error("ENOENT")) - - await expect(countFileLines("/nonexistent/file.txt")).rejects.toThrow("File not found") - }) - }) -}) diff --git a/src/integrations/misc/__tests__/read-lines.spec.ts b/src/integrations/misc/__tests__/read-lines.spec.ts deleted file mode 100644 index 14456d24f1..0000000000 --- a/src/integrations/misc/__tests__/read-lines.spec.ts +++ /dev/null @@ -1,132 +0,0 @@ -import { promises as fs } from "fs" -import path from "path" -import { readLines } from "../read-lines" - -describe("nthline", () => { - const testFile = path.join(__dirname, "test.txt") - - beforeAll(async () => { - // Create a test file with numbered lines - const content = Array.from({ length: 10 }, (_, i) => `Line ${i + 1}`).join("\n") - await fs.writeFile(testFile, content) - }) - - afterAll(async () => { - await fs.unlink(testFile) - }) - - describe("readLines function", () => { - it("should read lines from start when from_line is not provided", async () => { - const lines = await readLines(testFile, 2) - // Expect lines with trailing newline because it exists in the file at that point - const expected = ["Line 1", "Line 2", "Line 3"].join("\n") + "\n" - expect(lines).toEqual(expected) - }) - - it("should read a range of lines from a file", async () => { - const lines = await readLines(testFile, 3, 1) - // Expect lines with trailing newline because it exists in the file at that point - const expected = ["Line 2", "Line 3", "Line 4"].join("\n") + "\n" - expect(lines).toEqual(expected) - }) - - it("should read lines when to_line equals from_line", async () => { - const lines = await readLines(testFile, 2, 2) - // Expect line with trailing newline because it exists in the file at that point - const expected = "Line 3\n" - expect(lines).toEqual(expected) - }) - - it("should throw error for negative to_line", async () => { - await expect(readLines(testFile, -3)).rejects.toThrow( - "startLine (0) must be less than or equal to endLine (-3)", - ) - }) - - it("should handle negative from_line by clamping to 0", async () => { - const lines = await readLines(testFile, 3, -1) - expect(lines).toEqual(["Line 1", "Line 2", "Line 3", "Line 4"].join("\n") + "\n") - }) - - it("should floor non-integer line numbers", async () => { - const linesWithNonIntegerStart = await readLines(testFile, 3, 1.5) - expect(linesWithNonIntegerStart).toEqual(["Line 2", "Line 3", "Line 4"].join("\n") + "\n") - - const linesWithNonIntegerEnd = await readLines(testFile, 3.5) - expect(linesWithNonIntegerEnd).toEqual(["Line 1", "Line 2", "Line 3", "Line 4"].join("\n") + "\n") - }) - - it("should throw error when from_line > to_line", async () => { - await expect(readLines(testFile, 1, 3)).rejects.toThrow( - "startLine (3) must be less than or equal to endLine (1)", - ) - }) - - it("should return partial range if file ends before to_line", async () => { - const lines = await readLines(testFile, 15, 8) - expect(lines).toEqual(["Line 9", "Line 10"].join("\n")) - }) - - it("should throw error if from_line is beyond file length", async () => { - await expect(readLines(testFile, 20, 15)).rejects.toThrow("does not exist") - }) - - // Helper function to create a temporary file, run a test, and clean up - async function withTempFile(filename: string, content: string, testFn: (filepath: string) => Promise) { - const filepath = path.join(__dirname, filename) - await fs.writeFile(filepath, content) - try { - await testFn(filepath) - } finally { - await fs.unlink(filepath) - } - } - - it("should handle empty files", async () => { - await withTempFile("empty.txt", "", async (filepath) => { - await expect(readLines(filepath, 0, 0)).rejects.toThrow("does not exist") - }) - }) - - it("should handle files with only one line without carriage return", async () => { - await withTempFile("single-line-no-cr.txt", "Single line", async (filepath) => { - const lines = await readLines(filepath, 0, 0) - expect(lines).toEqual("Single line") - }) - }) - - it("should handle files with only one line with carriage return", async () => { - await withTempFile("single-line-with-cr.txt", "Single line\n", async (filepath) => { - const lines = await readLines(filepath, 0, 0) - expect(lines).toEqual("Single line\n") - }) - }) - - it("should read the entire file when no startLine or endLine is specified", async () => { - const content = await readLines(testFile) - expect(content).toEqual(Array.from({ length: 10 }, (_, i) => `Line ${i + 1}`).join("\n")) - }) - - it("should handle files with different line endings", async () => { - await withTempFile("mixed-endings.txt", "Line 1\rLine 2\r\nLine 3\n", async (filepath) => { - const lines = await readLines(filepath, 2) - expect(lines).toEqual("Line 1\rLine 2\r\nLine 3\n") - }) - }) - - it("should handle files with Unicode characters", async () => { - await withTempFile("unicode.txt", "Line 1 😀\nLine 2 你好\nLine 3 こんにちは\n", async (filepath) => { - const lines = await readLines(filepath, 1) - expect(lines).toEqual("Line 1 😀\nLine 2 你好\n") - }) - }) - - it("should handle files containing only carriage returns", async () => { - await withTempFile("cr-only.txt", "\n\n\n\n\n", async (filepath) => { - // Read lines 1-3 (second, third, and fourth lines) - const lines = await readLines(filepath, 3, 1) - expect(lines).toEqual("\n\n\n") - }) - }) - }) -}) diff --git a/src/integrations/misc/export-markdown.ts b/src/integrations/misc/export-markdown.ts index a400834c7b..c9488f94d5 100644 --- a/src/integrations/misc/export-markdown.ts +++ b/src/integrations/misc/export-markdown.ts @@ -92,7 +92,6 @@ export function formatContentBlockToMarkdown(block: ExtendedContentBlock): strin } case "tool_result": { // For now we're not doing tool name lookup since we don't use tools anymore - // const toolName = findToolName(block.tool_use_id, messages) const toolName = "Tool" if (typeof block.content === "string") { return `[${toolName}${block.is_error ? " (Error)" : ""}]\n${block.content}` @@ -113,16 +112,3 @@ export function formatContentBlockToMarkdown(block: ExtendedContentBlock): strin return `[Unexpected content type: ${block.type}]` } } - -export function findToolName(toolCallId: string, messages: Anthropic.MessageParam[]): string { - for (const message of messages) { - if (Array.isArray(message.content)) { - for (const block of message.content) { - if (block.type === "tool_use" && block.id === toolCallId) { - return block.name - } - } - } - } - return "Unknown Tool" -} diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index aa645dda67..c038f7b338 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -135,6 +135,8 @@ export async function extractTextFromFile(filePath: string): Promise { /** * Extracts formatted text exclusively from supplied bytes, without pathname I/O. * filePath is only format/diagnostic metadata, never a source of file contents. + * + * @internal Exported for focused implementation tests. */ export async function extractTextFromBuffer(source: Buffer, filePath: string): Promise { const fileExtension = path.extname(filePath).toLowerCase() @@ -384,6 +386,8 @@ export function applyRunLengthEncoding(content: string): string { * * @param input The terminal output to process * @returns The processed terminal output with carriage returns (\r) handled + * + * @internal */ export function processCarriageReturns(input: string): string { // Quick check: if no carriage returns (\r), return the original input @@ -435,6 +439,8 @@ export function processCarriageReturns(input: string): string { * * @param input The terminal output to process * @returns The processed output with backspaces handled + * + * @internal */ export function processBackspaces(input: string): string { let output = "" diff --git a/src/integrations/misc/line-counter.ts b/src/integrations/misc/line-counter.ts deleted file mode 100644 index 8a54db0263..0000000000 --- a/src/integrations/misc/line-counter.ts +++ /dev/null @@ -1,167 +0,0 @@ -import fs, { createReadStream } from "fs" -import { createInterface } from "readline" -import { countTokens } from "../../utils/countTokens" -import { Anthropic } from "@anthropic-ai/sdk" - -/** - * Efficiently counts lines in a file using streams without loading the entire file into memory - * - * @param filePath - Path to the file to count lines in - * @returns A promise that resolves to the number of lines in the file - */ -export async function countFileLines(filePath: string): Promise { - // Check if file exists - try { - await fs.promises.access(filePath, fs.constants.F_OK) - } catch { - throw new Error(`File not found: ${filePath}`) - } - - return new Promise((resolve, reject) => { - let lineCount = 0 - - const readStream = createReadStream(filePath) - const rl = createInterface({ - input: readStream, - crlfDelay: Infinity, - }) - - rl.on("line", () => { - lineCount++ - }) - - rl.on("close", () => { - resolve(lineCount) - }) - - rl.on("error", (err) => { - reject(err) - }) - - readStream.on("error", (err) => { - reject(err) - }) - }) -} - -export interface LineAndTokenCountResult { - /** Total number of lines counted */ - lineCount: number - /** Estimated token count */ - tokenEstimate: number - /** Whether the full file was scanned (false if early exit occurred) */ - complete: boolean -} - -export interface LineAndTokenCountOptions { - /** Maximum tokens allowed before early exit. If undefined, scans entire file */ - budgetTokens?: number - /** Number of lines to buffer before running token estimation (default: 256) */ - chunkLines?: number -} - -/** - * Efficiently counts lines and estimates tokens in a file using streams with incremental token estimation. - * Processes file in chunks to avoid memory issues and can early-exit when budget is exceeded. - * - * @param filePath - Path to the file to analyze - * @param options - Configuration options for counting - * @returns A promise that resolves to line count, token estimate, and completion status - */ -export async function countFileLinesAndTokens( - filePath: string, - options: LineAndTokenCountOptions = {}, -): Promise { - const { budgetTokens, chunkLines = 256 } = options - - // Check if file exists - try { - await fs.promises.access(filePath, fs.constants.F_OK) - } catch { - throw new Error(`File not found: ${filePath}`) - } - - return new Promise((resolve, reject) => { - let lineCount = 0 - let tokenEstimate = 0 - let lineBuffer: string[] = [] - let complete = true - let isProcessing = false - let shouldClose = false - - const readStream = createReadStream(filePath) - const rl = createInterface({ - input: readStream, - crlfDelay: Infinity, - }) - - const processBuffer = async () => { - if (lineBuffer.length === 0) return - - const bufferText = lineBuffer.join("\n") - lineBuffer = [] // Clear buffer before processing - - try { - const contentBlocks: Anthropic.Messages.ContentBlockParam[] = [{ type: "text", text: bufferText }] - const chunkTokens = await countTokens(contentBlocks) - tokenEstimate += chunkTokens - } catch { - // On tokenizer error, use conservative estimate: 2 char ≈ 1 token - tokenEstimate += Math.ceil(bufferText.length / 2) - } - - // Check if we've exceeded budget - if (budgetTokens !== undefined && tokenEstimate > budgetTokens) { - complete = false - shouldClose = true - rl.close() - readStream.destroy() - } - } - - rl.on("line", (line) => { - lineCount++ - lineBuffer.push(line) - - // Process buffer when it reaches chunk size - if (lineBuffer.length >= chunkLines && !isProcessing) { - isProcessing = true - rl.pause() - processBuffer() - .then(() => { - isProcessing = false - if (!shouldClose) { - rl.resume() - } - }) - .catch((err) => { - isProcessing = false - reject(err) - }) - } - }) - - rl.on("close", async () => { - // Wait for any ongoing processing to complete - while (isProcessing) { - await new Promise((r) => setTimeout(r, 10)) - } - - // Process any remaining lines in buffer - try { - await processBuffer() - resolve({ lineCount, tokenEstimate, complete }) - } catch (err) { - reject(err) - } - }) - - rl.on("error", (err) => { - reject(err) - }) - - readStream.on("error", (err) => { - reject(err) - }) - }) -} diff --git a/src/integrations/misc/read-lines.ts b/src/integrations/misc/read-lines.ts deleted file mode 100644 index 5a5eda9f83..0000000000 --- a/src/integrations/misc/read-lines.ts +++ /dev/null @@ -1,116 +0,0 @@ -/** - * credits @BorisChumichev - * - * https://github.com/BorisChumichev/node-nthline - * - * This module extend functionality of reading lines from a file - * Now you can read a range of lines from a file - */ -import { createReadStream } from "fs" - -const outOfRangeError = (filepath: string, n: number) => { - return new RangeError(`Line with index ${n} does not exist in '${filepath}'. Note that line indexing is zero-based`) -} - -/** - * Reads a range of lines from a file. - * - * @param filepath - Path to the file to read - * @param endLine - Optional. The line number to stop reading at (inclusive). If undefined, reads to the end of file. - * @param startLine - Optional. The line number to start reading from (inclusive). If undefined, starts from line 0. - * @returns Promise resolving to a string containing the read lines joined with newlines - * @throws {RangeError} If line numbers are invalid or out of range - */ -export function readLines(filepath: string, endLine?: number, startLine?: number): Promise { - return new Promise((resolve, reject) => { - // Reject if startLine is defined but not a number - if (startLine !== undefined && typeof startLine !== "number") { - return reject(new RangeError(`Invalid startLine: ${startLine}. Line numbers must be numbers.`)) - } - - // Force startLine to be an integer and clamp to 0 if negative - if (startLine !== undefined) { - startLine = Math.max(0, Math.floor(startLine)) - } - - // Reject if endLine is defined but not a number - if (endLine !== undefined && typeof endLine !== "number") { - return reject(new RangeError(`Invalid endLine: ${endLine}. Line numbers must be numbers.`)) - } - - // Force endLine to be an integer - if (endLine !== undefined) { - endLine = Math.floor(endLine) - } - - const effectiveStartLine = startLine === undefined ? 0 : startLine - - // Check startLine and endLine relationship - if (endLine !== undefined && effectiveStartLine > endLine) { - return reject( - new RangeError(`startLine (${effectiveStartLine}) must be less than or equal to endLine (${endLine})`), - ) - } - - // Set up stream - const input = createReadStream(filepath) - let buffer = "" - let lineCount = 0 - let result = "" - - // Handle errors - input.on("error", reject) - - // Process data chunks directly - input.on("data", (chunk) => { - // Add chunk to buffer - buffer += chunk.toString() - - let pos = 0 - let nextNewline = buffer.indexOf("\n", pos) - - // Process complete lines in the buffer - while (nextNewline !== -1) { - // If we're in the target range, add this line to the result - if (lineCount >= effectiveStartLine && (endLine === undefined || lineCount <= endLine)) { - result += buffer.substring(pos, nextNewline + 1) // Include the newline - } - - // Move position and increment line counter - pos = nextNewline + 1 - lineCount++ - - // If we've reached the end line, we can stop - if (endLine !== undefined && lineCount > endLine) { - input.destroy() - resolve(result) - return - } - - // Find next newline - nextNewline = buffer.indexOf("\n", pos) - } - - // Trim buffer - keep only the incomplete line - buffer = buffer.substring(pos) - }) - - // Handle end of file - input.on("end", () => { - // Process any remaining data in buffer (last line without newline) - if (buffer.length > 0) { - if (lineCount >= effectiveStartLine && (endLine === undefined || lineCount <= endLine)) { - result += buffer - } - lineCount++ - } - - // Check if we found any lines in the requested range - if (lineCount <= effectiveStartLine) { - reject(outOfRangeError(filepath, effectiveStartLine)) - } else { - resolve(result) - } - }) - }) -} diff --git a/src/package.json b/src/package.json index 0d6933f025..9c5182f316 100644 --- a/src/package.json +++ b/src/package.json @@ -528,7 +528,6 @@ "safe-stable-stringify": "^2.5.0", "sanitize-filename": "^1.6.4", "say": "^0.16.0", - "semver-compare": "^1.0.0", "serialize-error": "^12.0.0", "shell-quote": "^1.8.2", "simple-git": "^3.27.0", @@ -544,7 +543,6 @@ "zod": "3.25.76" }, "devDependencies": { - "@ai-sdk/openai-compatible": "3.0.70", "@roo-code/build": "workspace:^", "@roo-code/config-eslint": "workspace:^", "@roo-code/config-typescript": "workspace:^", @@ -555,7 +553,6 @@ "@types/node-cache": "4.2.5", "@types/proper-lockfile": "4.1.4", "@types/ps-tree": "1.1.6", - "@types/semver-compare": "1.0.3", "@types/vscode": "1.100.0", "@vitest/coverage-v8": "4.1.9", "@vscode/vsce": "3.9.2", diff --git a/src/services/code-index/interfaces/manager.ts b/src/services/code-index/interfaces/manager.ts index cdda7a7053..cab376e1b6 100644 --- a/src/services/code-index/interfaces/manager.ts +++ b/src/services/code-index/interfaces/manager.ts @@ -1,79 +1,3 @@ -import { VectorStoreSearchResult } from "./vector-store" -import * as vscode from "vscode" - -/** - * Interface for the code index manager - */ -export interface ICodeIndexManager { - /** - * Event emitted when progress is updated - */ - onProgressUpdate: vscode.Event<{ - systemStatus: IndexingState - fileStatuses: Record - message?: string - }> - - /** - * Current state of the indexing process - */ - readonly state: IndexingState - - /** - * Whether the code indexing feature is enabled - */ - readonly isFeatureEnabled: boolean - - /** - * Whether the code indexing feature is configured - */ - readonly isFeatureConfigured: boolean - - /** - * Loads configuration from storage - */ - loadConfiguration(): Promise - - /** - * Starts the indexing process - */ - startIndexing(): Promise - - /** - * Stops any in-progress indexing operation and the file watcher - */ - stopIndexing(): void - - /** - * Stops the file watcher - */ - stopWatcher(): void - - /** - * Clears the index data - */ - clearIndexData(): Promise - - /** - * Searches the index - * @param query Query string - * @param limit Maximum number of results to return - * @returns Promise resolving to search results - */ - searchIndex(query: string, limit: number): Promise - - /** - * Gets the current status of the indexing system - * @returns Current status information - */ - getCurrentStatus(): { systemStatus: IndexingState; fileStatuses: Record; message?: string } - - /** - * Disposes of resources used by the manager - */ - dispose(): void -} - export type IndexingState = "Standby" | "Indexing" | "Indexed" | "Error" | "Stopping" export type EmbedderProvider = | "openai" @@ -85,10 +9,3 @@ export type EmbedderProvider = | "bedrock" | "openrouter" | "semble" - -export interface IndexProgressUpdate { - systemStatus: IndexingState - message?: string - processedBlockCount?: number - totalBlockCount?: number -} diff --git a/src/services/code-index/semble/semble-downloader.ts b/src/services/code-index/semble/semble-downloader.ts index 6a326b2150..83a419bd9f 100644 --- a/src/services/code-index/semble/semble-downloader.ts +++ b/src/services/code-index/semble/semble-downloader.ts @@ -69,6 +69,8 @@ export function isSembleSupportedPlatform(platform?: string, arch?: string): boo /** * Returns the list of supported platform-arch keys (e.g. "linux-x64", "darwin-arm64"). + * + * @internal */ export function getSembleSupportedPlatforms(): string[] { return Object.keys(SEMBLE_ARCHIVES) @@ -143,6 +145,8 @@ export async function downloadSemble(storageDir: string): Promise { const info = getArchiveInfo() diff --git a/src/services/code-index/shared/validation-helpers.ts b/src/services/code-index/shared/validation-helpers.ts index 92a32d8578..025d004aa1 100644 --- a/src/services/code-index/shared/validation-helpers.ts +++ b/src/services/code-index/shared/validation-helpers.ts @@ -53,16 +53,6 @@ export interface HttpError extends Error { } } -/** - * Common error types that can occur during embedder validation - */ -export interface ValidationError { - status?: number - message?: string - name?: string - code?: string -} - /** * Maps HTTP status codes to appropriate error messages */ diff --git a/src/services/command/built-in-commands.ts b/src/services/command/built-in-commands.ts index 0b40367199..c8342f95c5 100644 --- a/src/services/command/built-in-commands.ts +++ b/src/services/command/built-in-commands.ts @@ -319,6 +319,8 @@ export async function getBuiltInCommand(name: string): Promise { return Object.keys(BUILT_IN_COMMANDS) diff --git a/src/services/destructive-command-guard/manager.ts b/src/services/destructive-command-guard/manager.ts index 84338713b4..bcd12f6396 100644 --- a/src/services/destructive-command-guard/manager.ts +++ b/src/services/destructive-command-guard/manager.ts @@ -24,10 +24,12 @@ export function getDcgArchiveInfo(platform = process.platform, arch = process.ar return DCG_ARCHIVES[`${platform}-${arch}`] } +/** @internal Exported for focused implementation tests. */ export function isDcgSupportedPlatform(platform = process.platform, arch = process.arch): boolean { return getDcgArchiveInfo(platform, arch) !== undefined } +/** @internal Exported for focused implementation tests. */ export function getDcgBinaryPath( storageDir: string, platform = process.platform, @@ -37,10 +39,12 @@ export function getDcgBinaryPath( return info ? path.join(storageDir, "destructive-command-guard", info.binary) : undefined } +/** @internal Exported for focused implementation tests. */ export function isTrustedDownloadUrl(url: string): boolean { return isTrustedHttpsUrl(url, DCG_TRUSTED_DOWNLOAD_DOMAINS) } +/** @internal Exported for focused implementation tests. */ export function resolveTrustedRedirect(url: string, location: string | undefined, redirectsRemaining: number): string { return resolveManagedBinaryRedirect(url, location, redirectsRemaining, { name: "DCG", diff --git a/src/services/managed-binary/archive.ts b/src/services/managed-binary/archive.ts index be2f29a133..9b463064c9 100644 --- a/src/services/managed-binary/archive.ts +++ b/src/services/managed-binary/archive.ts @@ -80,6 +80,7 @@ export async function extractZipArchive(archivePath: string, destination: string await runProcess("unzip", ["-o", archivePath, "-d", destination]) } +/** @internal Exported for focused implementation tests. */ export async function extractSingleFileZipArchive( archivePath: string, destination: string, @@ -108,6 +109,7 @@ export async function extractSingleFileZipArchive( await runPowerShell(script) } +/** @internal Exported for focused implementation tests. */ export async function extractSingleFileTarXzArchive( archivePath: string, destination: string, diff --git a/src/services/roo-config/index.ts b/src/services/roo-config/index.ts index f9874b0dff..5f5f8c3fa5 100644 --- a/src/services/roo-config/index.ts +++ b/src/services/roo-config/index.ts @@ -398,6 +398,8 @@ export async function getAgentsDirectoriesForCwd(cwd: string): Promise * merged: "global content\n\n# Project-specific rules (override global):\n\nproject content" * } * ``` + * + * @internal */ export async function loadConfiguration( relativePath: string, @@ -439,6 +441,3 @@ export async function loadConfiguration( merged: merged || "", } } - -// Export with backward compatibility alias -export const loadRooConfiguration: typeof loadConfiguration = loadConfiguration diff --git a/src/services/tree-sitter/index.ts b/src/services/tree-sitter/index.ts index 241094699d..029e374609 100644 --- a/src/services/tree-sitter/index.ts +++ b/src/services/tree-sitter/index.ts @@ -22,6 +22,8 @@ export function getMinComponentLines(): number { /** * Set the minimum number of lines for a component (for testing) + * + * @internal */ export function setMinComponentLines(value: number): void { currentMinComponentLines = value @@ -163,7 +165,7 @@ Parsing files using tree-sitter 1. Parse the file content into an AST (Abstract Syntax Tree) using the appropriate language grammar (set of rules that define how the components of a language like keywords, expressions, and statements can be combined to create valid programs). 2. Create a query using a language-specific query string, and run it against the AST's root node to capture specific syntax elements. - - We use tag queries to identify named entities in a program, and then use a syntax capture to label the entity and its name. A notable example of this is GitHub's search-based code navigation. + - We use tag queries to identify named entities in a program, and then use a syntax capture to label the entity and its name. A notable example of this is GitHub's search-based code navigation. - Our custom tag queries are based on tree-sitter's default tag queries, but modified to only capture definitions. 3. Sort the captures by their position in the file, output the name of the definition, and format by i.e. adding "|----\n" for gaps between captured sections. diff --git a/src/services/tree-sitter/markdownParser.ts b/src/services/tree-sitter/markdownParser.ts index bd16ea32f7..e082563916 100644 --- a/src/services/tree-sitter/markdownParser.ts +++ b/src/services/tree-sitter/markdownParser.ts @@ -190,6 +190,8 @@ export function parseMarkdown(content: string): QueryCapture[] { * @param captures - The array of query captures * @param minSectionLines - Minimum number of lines for a section to be included * @returns A formatted string with headers and section line ranges + * + * @internal */ export function formatMarkdownCaptures(captures: QueryCapture[], minSectionLines: number = 4): string | null { if (captures.length === 0) { diff --git a/src/services/tree-sitter/queries/scala.ts b/src/services/tree-sitter/queries/scala.ts index d56d8c8832..b8a763336c 100644 --- a/src/services/tree-sitter/queries/scala.ts +++ b/src/services/tree-sitter/queries/scala.ts @@ -1,3 +1,6 @@ +/** + * @internal + */ export const scalaQuery = ` ; Classes (class_definition diff --git a/src/shared/context-mentions.ts b/src/shared/context-mentions.ts index 34562dc2fc..cfb955be4f 100644 --- a/src/shared/context-mentions.ts +++ b/src/shared/context-mentions.ts @@ -67,44 +67,6 @@ export const mentionRegexGlobal = new RegExp(mentionRegex.source, "g") // Regex to match command mentions like /command-name anywhere in text export const commandRegexGlobal = /(?:^|\s)\/([a-zA-Z0-9_.-]+)(?=\s|$)/g -export interface MentionSuggestion { - type: "file" | "folder" | "git" | "problems" - label: string - description?: string - value: string - icon?: string -} - -export interface GitMentionSuggestion extends MentionSuggestion { - type: "git" - hash: string - shortHash: string - subject: string - author: string - date: string -} - -export function formatGitSuggestion(commit: { - hash: string - shortHash: string - subject: string - author: string - date: string -}): GitMentionSuggestion { - return { - type: "git", - label: commit.subject, - description: `${commit.shortHash} by ${commit.author} on ${commit.date}`, - value: commit.hash, - icon: "$(git-commit)", // VSCode git commit icon - hash: commit.hash, - shortHash: commit.shortHash, - subject: commit.subject, - author: commit.author, - date: commit.date, - } -} - // Helper function to unescape paths with backslash-escaped spaces export function unescapeSpaces(path: string): string { return path.replace(/\\ /g, " ") diff --git a/src/shared/modes.ts b/src/shared/modes.ts index a94aa47ed0..17ff6108b4 100644 --- a/src/shared/modes.ts +++ b/src/shared/modes.ts @@ -58,14 +58,6 @@ export function getModeBySlug(slug: string, customModes?: ModeConfig[]): ModeCon return modes.find((mode) => mode.slug === slug) } -export function getModeConfig(slug: string, customModes?: ModeConfig[]): ModeConfig { - const mode = getModeBySlug(slug, customModes) - if (!mode) { - throw new Error(`No mode found for slug: ${slug}`) - } - return mode -} - // Get all available modes, with custom modes overriding built-in modes export function getAllModes(customModes?: ModeConfig[]): ModeConfig[] { if (!customModes?.length) { @@ -90,11 +82,6 @@ export function getAllModes(customModes?: ModeConfig[]): ModeConfig[] { return allModes } -// Check if a mode is custom or an override -export function isCustomMode(slug: string, customModes?: ModeConfig[]): boolean { - return !!customModes?.some((mode) => mode.slug === slug) -} - /** * Find a mode by its slug, don't fall back to built-in modes */ diff --git a/src/shared/parse-command.ts b/src/shared/parse-command.ts index 6b2fb4d267..edaebcb81a 100644 --- a/src/shared/parse-command.ts +++ b/src/shared/parse-command.ts @@ -102,6 +102,8 @@ function unterminatedQuoteMessage(quoteType: QuoteType, openIndex: number, comma * opener line's newline through the line that is exactly the terminator word * (after stripping leading tabs for `<<-`). If no terminator line is found the * heredoc is unterminated. + * + * @internal */ export function findUnterminatedQuote(command: string): UnterminatedQuote | null { return scanTopLevelQuotes(command).unterminatedQuote diff --git a/src/shared/tools.ts b/src/shared/tools.ts index a1daf27d08..b9a56d7d3d 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -33,8 +33,6 @@ export type HandleError = (action: string, error: Error) => Promise export type PushToolResult = (content: ToolResponse) => void -export type AskFinishSubTaskApproval = () => Promise - export interface TextContent { type: "text" content: string @@ -184,96 +182,18 @@ export interface McpToolUse { partial: boolean } -export interface ExecuteCommandToolUse extends ToolUse<"execute_command"> { - name: "execute_command" - // Pick, "command"> makes "command" required, but Partial<> makes it optional - params: Partial, "command" | "cwd" | "timeout">> -} - -export interface ReadFileToolUse extends ToolUse<"read_file"> { - name: "read_file" - params: Partial< - Pick< - Record, - | "args" - | "path" - | "start_line" - | "end_line" - | "mode" - | "offset" - | "limit" - | "indentation" - | "anchor_line" - | "max_levels" - | "include_siblings" - | "include_header" - > - > -} - -export interface WriteToFileToolUse extends ToolUse<"write_to_file"> { - name: "write_to_file" - params: Partial, "path" | "content">> -} - -export interface CodebaseSearchToolUse extends ToolUse<"codebase_search"> { - name: "codebase_search" - params: Partial, "query" | "path">> -} - -export interface SearchFilesToolUse extends ToolUse<"search_files"> { - name: "search_files" - params: Partial, "path" | "regex" | "file_pattern">> -} - -export interface ListFilesToolUse extends ToolUse<"list_files"> { - name: "list_files" - params: Partial, "path" | "recursive">> -} - -export interface UseMcpToolToolUse extends ToolUse<"use_mcp_tool"> { - name: "use_mcp_tool" - params: Partial, "server_name" | "tool_name" | "arguments">> -} - -export interface AccessMcpResourceToolUse extends ToolUse<"access_mcp_resource"> { - name: "access_mcp_resource" - params: Partial, "server_name" | "uri">> -} - -export interface AskFollowupQuestionToolUse extends ToolUse<"ask_followup_question"> { - name: "ask_followup_question" - params: Partial, "question" | "follow_up">> -} - +/** + * @public + */ export interface AttemptCompletionToolUse extends ToolUse<"attempt_completion"> { name: "attempt_completion" params: Partial, "result">> } -export interface SwitchModeToolUse extends ToolUse<"switch_mode"> { - name: "switch_mode" - params: Partial, "mode_slug" | "reason">> -} - -export interface NewTaskToolUse extends ToolUse<"new_task"> { - name: "new_task" - params: Partial, "mode" | "message" | "todos">> -} - -export interface RunSlashCommandToolUse extends ToolUse<"run_slash_command"> { - name: "run_slash_command" - params: Partial, "command" | "args">> -} - -export interface SkillToolUse extends ToolUse<"skill"> { - name: "skill" - params: Partial, "skill" | "args">> -} - -export interface GenerateImageToolUse extends ToolUse<"generate_image"> { - name: "generate_image" - params: Partial, "prompt" | "path" | "image">> +/** @internal Type used by tool-argument regression tests. */ +export interface WriteToFileToolUse extends ToolUse<"write_to_file"> { + name: "write_to_file" + params: Partial, "path" | "content">> } // Define tool group configuration @@ -283,34 +203,6 @@ export type ToolGroupConfig = { customTools?: readonly string[] // Opt-in only tools - only available when explicitly included via model's includedTools } -export const TOOL_DISPLAY_NAMES: Record = { - execute_command: "run commands", - read_file: "read files", - read_command_output: "read command output", - write_to_file: "write files", - apply_diff: "apply changes", - edit: "edit files", - search_and_replace: "apply changes using search and replace", - search_replace: "apply single search and replace", - edit_file: "edit files using search and replace", - apply_patch: "apply patches using codex format", - search_files: "search files", - list_files: "list files", - use_mcp_tool: "use mcp tools", - access_mcp_resource: "access mcp resources", - ask_followup_question: "ask questions", - attempt_completion: "complete tasks", - switch_mode: "switch modes", - new_task: "create new task", - codebase_search: "codebase search", - update_todo_list: "update todo list", - run_slash_command: "run slash command", - skill: "load skill", - generate_image: "generate images", - custom_tool: "use custom tools", - invalid_tool_call: "invalid tool call", -} as const - // Define available tool groups. export const TOOL_GROUPS: Record = { read: { diff --git a/src/utils/TaskSemaphore.ts b/src/utils/TaskSemaphore.ts index 15db11c2e1..09cf43b023 100644 --- a/src/utils/TaskSemaphore.ts +++ b/src/utils/TaskSemaphore.ts @@ -1,3 +1,4 @@ +// Retained for in-flight work: #355 (parallelizable tasks), #368 (TaskScheduler), #373, #375 import { Semaphore } from "async-mutex" /** diff --git a/src/utils/WorkspacePathResolver.ts b/src/utils/WorkspacePathResolver.ts index c11fd3f780..42536991d7 100644 --- a/src/utils/WorkspacePathResolver.ts +++ b/src/utils/WorkspacePathResolver.ts @@ -1,3 +1,4 @@ +// Retained for in-flight work: PR #436, issues #390, #724 import * as path from "path" import * as fs from "fs/promises" diff --git a/src/utils/config.ts b/src/utils/config.ts index 9f750bb915..fee3adbc73 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -16,6 +16,8 @@ export type InjectableConfigType = * Uses VSCode env:name pattern: https://code.visualstudio.com/docs/reference/variables-reference#_environment-variables * * Does not mutate original object + * + * @internal */ export async function injectEnv(config: C, notFoundValue: any = "") { return injectVariables(config, { env: process.env }, notFoundValue) diff --git a/src/utils/networkProxy.ts b/src/utils/networkProxy.ts index 835887334e..bf6ca37b80 100644 --- a/src/utils/networkProxy.ts +++ b/src/utils/networkProxy.ts @@ -329,6 +329,8 @@ function updateProxyEnvVars(config: ProxyConfig): void { /** * Check if a proxy is currently configured and active. + * + * @internal */ export function isProxyEnabled(): boolean { const config = getProxyConfig() @@ -338,6 +340,8 @@ export function isProxyEnabled(): boolean { /** * Check if we're running in debug mode. + * + * @internal */ export function isDebugMode(): boolean { if (!extensionContext) { diff --git a/webview-ui/package.json b/webview-ui/package.json index faadd1ddf8..40be8a186b 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -34,7 +34,6 @@ "@radix-ui/react-slot": "^1.1.2", "@radix-ui/react-tooltip": "^1.1.8", "@roo-code/types": "workspace:^", - "@tailwindcss/vite": "^4.0.0", "@tanstack/react-query": "^5.68.0", "@vscode/codicons": "^0.0.45", "@vscode/webview-ui-toolkit": "^1.4.0", @@ -99,6 +98,7 @@ "@types/react-dom": "18.3.7", "@types/stacktrace-js": "2.0.3", "@types/vscode-webview": "1.57.5", + "@tailwindcss/vite": "^4.0.0", "@vitejs/plugin-react": "5.2.0", "@vitest/coverage-v8": "4.1.9", "@vitest/ui": "4.1.9", diff --git a/webview-ui/playwright/AppProviders.tsx b/webview-ui/playwright/AppProviders.tsx index f8a274cd9f..906c34ec47 100644 --- a/webview-ui/playwright/AppProviders.tsx +++ b/webview-ui/playwright/AppProviders.tsx @@ -4,7 +4,7 @@ import { I18nextProvider } from "react-i18next" import { providerIdentifiers } from "@roo-code/types" import { ExtensionStateContextProvider } from "@/context/ExtensionStateContext" -import { TranslationProvider } from "@/i18n/TranslationContext" +import TranslationProvider from "@/i18n/TranslationContext" import i18next, { loadTranslations } from "@/i18n/setup" import { TooltipProvider } from "@/components/ui/tooltip" import { TranslationContext as PlaywrightTranslationContext } from "@src/i18n/TranslationContext" diff --git a/webview-ui/src/components/chat/BatchListFilesPermission.tsx b/webview-ui/src/components/chat/BatchListFilesPermission.tsx deleted file mode 100644 index a5d08c244b..0000000000 --- a/webview-ui/src/components/chat/BatchListFilesPermission.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import { memo } from "react" - -import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock" -import { PathTooltip } from "../ui/PathTooltip" - -interface DirPermissionItem { - path: string - key: string -} - -interface BatchListFilesPermissionProps { - dirs: DirPermissionItem[] - ts: number -} - -export const BatchListFilesPermission = memo(({ dirs = [], ts }: BatchListFilesPermissionProps) => { - if (!dirs?.length) { - return null - } - - return ( -
-
- {dirs.map((dir, index) => { - return ( -
- - - - - {dir.path} - - -
- - -
- ) - })} -
-
- ) -}) - -BatchListFilesPermission.displayName = "BatchListFilesPermission" diff --git a/webview-ui/src/components/chat/CheckpointRestoreDialog.tsx b/webview-ui/src/components/chat/CheckpointRestoreDialog.tsx index b862d250be..34a99c3afa 100644 --- a/webview-ui/src/components/chat/CheckpointRestoreDialog.tsx +++ b/webview-ui/src/components/chat/CheckpointRestoreDialog.tsx @@ -72,12 +72,3 @@ export const CheckpointRestoreDialog: React.FC = ( ) } - -// Export convenience components for backward compatibility -export const EditMessageWithCheckpointDialog: React.FC> = (props) => ( - -) - -export const DeleteMessageWithCheckpointDialog: React.FC> = (props) => ( - -) diff --git a/webview-ui/src/components/chat/ErrorRow.tsx b/webview-ui/src/components/chat/ErrorRow.tsx index 4ee1a1d129..26dc795bd3 100644 --- a/webview-ui/src/components/chat/ErrorRow.tsx +++ b/webview-ui/src/components/chat/ErrorRow.tsx @@ -74,7 +74,7 @@ export interface ErrorRowProps { /** * Unified error display component for all error types in the chat */ -export const ErrorRow = memo( +const ErrorRow = memo( ({ type, title, diff --git a/webview-ui/src/components/chat/ProfileViolationWarning.tsx b/webview-ui/src/components/chat/ProfileViolationWarning.tsx index 5248e480b3..fdaaa1bb64 100644 --- a/webview-ui/src/components/chat/ProfileViolationWarning.tsx +++ b/webview-ui/src/components/chat/ProfileViolationWarning.tsx @@ -1,7 +1,7 @@ import React from "react" import { useAppTranslation } from "@/i18n/TranslationContext" -export const ProfileViolationWarning: React.FC = () => { +const ProfileViolationWarning: React.FC = () => { const { t } = useAppTranslation() return ( diff --git a/webview-ui/src/components/chat/SlashCommandItemSimple.tsx b/webview-ui/src/components/chat/SlashCommandItemSimple.tsx deleted file mode 100644 index d395834abd..0000000000 --- a/webview-ui/src/components/chat/SlashCommandItemSimple.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import React from "react" - -import type { Command } from "@roo-code/types" - -interface SlashCommandItemSimpleProps { - command: Command - onClick?: (command: Command) => void -} - -export const SlashCommandItemSimple: React.FC = ({ command, onClick }) => { - return ( -
onClick?.(command)}> - {/* Command name */} -
-
- /{command.name} - {command.description && ( -
- {command.description} -
- )} -
-
-
- ) -} diff --git a/webview-ui/src/components/chat/TooManyToolsWarning.tsx b/webview-ui/src/components/chat/TooManyToolsWarning.tsx deleted file mode 100644 index 697fad19ae..0000000000 --- a/webview-ui/src/components/chat/TooManyToolsWarning.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import React, { useCallback } from "react" -import { useAppTranslation } from "@/i18n/TranslationContext" -import { useTooManyTools } from "@src/hooks/useTooManyTools" -import WarningRow from "./WarningRow" - -/** - * Displays a warning when the user has too many MCP tools enabled. - * LLMs get confused when offered too many tools, which can lead to errors. - * - * The warning is shown when: - * - The total number of enabled tools across all enabled MCP servers exceeds the threshold - * - * @example - * - */ -export const TooManyToolsWarning: React.FC = () => { - const { t } = useAppTranslation() - const { isOverThreshold, title, message } = useTooManyTools() - - const handleOpenMcpSettings = useCallback(() => { - window.postMessage({ type: "action", action: "settingsButtonClicked", values: { section: "mcp" } }, "*") - }, []) - - // Don't show warning if under threshold - if (!isOverThreshold) { - return null - } - - return ( - - ) -} - -export default TooManyToolsWarning diff --git a/webview-ui/src/components/chat/WarningRow.tsx b/webview-ui/src/components/chat/WarningRow.tsx index 3fe4e90076..4f18f6a88b 100644 --- a/webview-ui/src/components/chat/WarningRow.tsx +++ b/webview-ui/src/components/chat/WarningRow.tsx @@ -30,7 +30,7 @@ export interface WarningRowProps { * onAction={() => openSettings()} * /> */ -export const WarningRow: React.FC = ({ title, message, docsURL, actionText, onAction }) => { +const WarningRow: React.FC = ({ title, message, docsURL, actionText, onAction }) => { const { t } = useAppTranslation() return ( diff --git a/webview-ui/src/components/chat/__tests__/BatchListFilesPermission.spec.tsx b/webview-ui/src/components/chat/__tests__/BatchListFilesPermission.spec.tsx deleted file mode 100644 index f2a81936c8..0000000000 --- a/webview-ui/src/components/chat/__tests__/BatchListFilesPermission.spec.tsx +++ /dev/null @@ -1,103 +0,0 @@ -import { render, screen } from "@/utils/test-utils" - -import { TranslationProvider } from "@/i18n/__mocks__/TranslationContext" - -import { BatchListFilesPermission } from "../BatchListFilesPermission" - -describe("BatchListFilesPermission", () => { - const mockDirs = [ - { - key: "apps/cli", - path: "apps/cli", - }, - { - key: "apps/vscode-e2e", - path: "apps/vscode-e2e", - }, - { - key: "packages/core", - path: "packages/core", - }, - ] - - beforeEach(() => { - vi.clearAllMocks() - }) - - it("renders directory list correctly", () => { - render( - - - , - ) - - expect(screen.getByText("apps/cli")).toBeInTheDocument() - expect(screen.getByText("apps/vscode-e2e")).toBeInTheDocument() - expect(screen.getByText("packages/core")).toBeInTheDocument() - }) - - it("renders nothing when dirs array is empty", () => { - const { container } = render( - - - , - ) - - expect(container.firstChild).toBeNull() - }) - - it("re-renders when timestamp changes", () => { - const { rerender } = render( - - - , - ) - - expect(screen.getByText("apps/cli")).toBeInTheDocument() - - rerender( - - - , - ) - - expect(screen.getByText("apps/cli")).toBeInTheDocument() - }) - - it("renders all directories in a single container", () => { - render( - - - , - ) - - // All directories should be within a single bordered container - const container = screen.getByText("apps/cli").closest(".border.border-border.rounded-md") - expect(container).toBeInTheDocument() - - // All 3 dirs should be inside this container - expect(container?.querySelectorAll(".flex.items-center.gap-2")).toHaveLength(mockDirs.length) - }) - - it("renders a single directory", () => { - const singleDir = [ - { - key: "apps/cli", - path: "apps/cli", - }, - ] - - render( - - - , - ) - - expect(screen.getByText("apps/cli")).toBeInTheDocument() - - // Single directory should still be rendered inside the container - const bordered = screen.getByText("apps/cli").closest(".border.border-border.rounded-md") - expect(bordered).toBeInTheDocument() - expect(bordered?.querySelectorAll(".flex.items-center.gap-2")).toHaveLength(1) - }) -}) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx index 56b008b862..14323cf634 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx @@ -83,8 +83,6 @@ vi.mock("@src/components/welcome/RooTips", nullDefaultModule) vi.mock("../Announcement", nullDefaultModule) vi.mock("./TaskHeader", () => ({ default: () =>
})) vi.mock("./ProfileViolationWarning", nullDefaultModule) -vi.mock("../common/DismissibleUpsell", nullDefaultModule) - vi.mock("./CheckpointWarning", () => ({ CheckpointWarning: () => null })) vi.mock("./QueuedMessages", () => ({ QueuedMessages: () => null })) vi.mock("./WorktreeSelector", () => ({ WorktreeSelector: () => null })) diff --git a/webview-ui/src/components/chat/__tests__/ErrorRow.spec.tsx b/webview-ui/src/components/chat/__tests__/ErrorRow.spec.tsx index 612c6c117b..d60cf9c272 100644 --- a/webview-ui/src/components/chat/__tests__/ErrorRow.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ErrorRow.spec.tsx @@ -3,7 +3,7 @@ import React from "react" import { render, screen, fireEvent } from "@/utils/test-utils" import { vscode } from "@/utils/vscode" -import { ErrorRow } from "../ErrorRow" +import ErrorRow from "../ErrorRow" // Mock vscode webview messaging vi.mock("@/utils/vscode", () => ({ diff --git a/webview-ui/src/components/chat/__tests__/SlashCommandItemSimple.spec.tsx b/webview-ui/src/components/chat/__tests__/SlashCommandItemSimple.spec.tsx deleted file mode 100644 index 197b0c60e0..0000000000 --- a/webview-ui/src/components/chat/__tests__/SlashCommandItemSimple.spec.tsx +++ /dev/null @@ -1,163 +0,0 @@ -import type { Command } from "@roo-code/types" - -import { render, screen, fireEvent } from "@/utils/test-utils" - -import { SlashCommandItemSimple } from "../SlashCommandItemSimple" - -describe("SlashCommandItemSimple", () => { - const mockCommand: Command = { - name: "test-command", - description: "Test command description", - source: "global", - filePath: "/path/to/command.md", - } - - const mockOnClick = vi.fn() - - beforeEach(() => { - vi.clearAllMocks() - }) - - it("renders command name with slash prefix", () => { - render() - - expect(screen.getByText("/test-command")).toBeInTheDocument() - }) - - it("renders command description when provided", () => { - render() - - expect(screen.getByText("Test command description")).toBeInTheDocument() - }) - - it("does not render description when not provided", () => { - const commandWithoutDescription: Command = { - ...mockCommand, - description: undefined, - } - - render() - - expect(screen.queryByText("Test command description")).not.toBeInTheDocument() - }) - - it("calls onClick handler when clicked", () => { - render() - - // The outer div is the clickable element - const commandElement = screen.getByText("/test-command").closest("div.px-4") - expect(commandElement).toBeInTheDocument() - - fireEvent.click(commandElement!) - - expect(mockOnClick).toHaveBeenCalledTimes(1) - expect(mockOnClick).toHaveBeenCalledWith(mockCommand) - }) - - it("does not throw error when onClick is not provided", () => { - render() - - const commandElement = screen.getByText("/test-command").closest("div.px-4") - expect(commandElement).toBeInTheDocument() - - // Should not throw error - expect(() => fireEvent.click(commandElement!)).not.toThrow() - }) - - it("applies hover styles", () => { - render() - - // The outer div has the hover styles - const commandElement = screen.getByText("/test-command").closest("div.px-4") - expect(commandElement).toHaveClass("hover:bg-vscode-list-hoverBackground") - }) - - it("applies cursor pointer style", () => { - render() - - const commandElement = screen.getByText("/test-command").closest("div.px-4") - expect(commandElement).toHaveClass("cursor-pointer") - }) - - it("renders with correct layout classes", () => { - render() - - const commandElement = screen.getByText("/test-command").closest("div.px-4") - expect(commandElement).toHaveClass("px-4", "py-2", "text-sm", "flex", "items-center") - }) - - it("renders command name with correct text color", () => { - render() - - const nameElement = screen.getByText("/test-command") - expect(nameElement).toHaveClass("text-vscode-foreground") - }) - - it("renders description with correct text styling", () => { - render() - - const descriptionElement = screen.getByText("Test command description") - expect(descriptionElement).toHaveClass("text-xs", "text-vscode-descriptionForeground", "truncate", "mt-0.5") - }) - - it("handles built-in commands correctly", () => { - const builtInCommand: Command = { - ...mockCommand, - source: "built-in", - } - - render() - - expect(screen.getByText("/test-command")).toBeInTheDocument() - expect(screen.getByText("Test command description")).toBeInTheDocument() - - // Should still be clickable - const commandElement = screen.getByText("/test-command").closest("div.px-4") - fireEvent.click(commandElement!) - expect(mockOnClick).toHaveBeenCalledWith(builtInCommand) - }) - - it("handles project commands correctly", () => { - const projectCommand: Command = { - ...mockCommand, - source: "project", - } - - render() - - expect(screen.getByText("/test-command")).toBeInTheDocument() - expect(screen.getByText("Test command description")).toBeInTheDocument() - - // Should still be clickable - const commandElement = screen.getByText("/test-command").closest("div.px-4") - fireEvent.click(commandElement!) - expect(mockOnClick).toHaveBeenCalledWith(projectCommand) - }) - - it("truncates long command names", () => { - const longNameCommand: Command = { - ...mockCommand, - name: "this-is-a-very-long-command-name-that-should-be-truncated-in-the-ui", - } - - render() - - const nameElement = screen.getByText("/this-is-a-very-long-command-name-that-should-be-truncated-in-the-ui") - expect(nameElement).toHaveClass("truncate") - }) - - it("truncates long descriptions", () => { - const longDescriptionCommand: Command = { - ...mockCommand, - description: - "This is a very long description that should be truncated in the UI to prevent overflow and maintain a clean layout", - } - - render() - - const descriptionElement = screen.getByText( - "This is a very long description that should be truncated in the UI to prevent overflow and maintain a clean layout", - ) - expect(descriptionElement).toHaveClass("truncate") - }) -}) diff --git a/webview-ui/src/components/chat/__tests__/TooManyToolsWarning.spec.tsx b/webview-ui/src/components/chat/__tests__/TooManyToolsWarning.spec.tsx deleted file mode 100644 index 85560201da..0000000000 --- a/webview-ui/src/components/chat/__tests__/TooManyToolsWarning.spec.tsx +++ /dev/null @@ -1,296 +0,0 @@ -import { render, screen, fireEvent } from "@/utils/test-utils" -import { MAX_MCP_TOOLS_THRESHOLD } from "@roo-code/types" - -import { TooManyToolsWarning } from "../TooManyToolsWarning" - -// Mock vscode webview messaging -vi.mock("@/utils/vscode", () => ({ - vscode: { - postMessage: vi.fn(), - }, -})) - -// Mock ExtensionState context with variable mcpServers -const mockMcpServers = vi.fn() - -vi.mock("@/context/ExtensionStateContext", () => ({ - useExtensionState: () => ({ - mcpServers: mockMcpServers(), - }), -})) - -// Mock i18n TranslationContext -vi.mock("@/i18n/TranslationContext", () => ({ - useAppTranslation: () => ({ - t: (key: string, params?: Record) => { - if (key === "chat:tooManyTools.title") { - return "Too many tools enabled" - } - if (key === "chat:tooManyTools.toolsPart") { - const count = params?.count ?? 0 - return count === 1 ? `${count} tool` : `${count} tools` - } - if (key === "chat:tooManyTools.serversPart") { - const count = params?.count ?? 0 - return count === 1 ? `${count} MCP server` : `${count} MCP servers` - } - if (key === "chat:tooManyTools.messageTemplate") { - return `You have ${params?.tools} enabled via ${params?.servers}. Such a high number can confuse the model and lead to errors. Try to keep it below ${params?.threshold}.` - } - if (key === "chat:tooManyTools.openMcpSettings") { - return "Open MCP Settings" - } - if (key === "chat:apiRequest.errorMessage.docs") { - return "Docs" - } - return key - }, - }), -})) - -describe("TooManyToolsWarning", () => { - beforeEach(() => { - vi.clearAllMocks() - mockMcpServers.mockReturnValue([]) - }) - - it("does not render when there are no MCP servers", () => { - mockMcpServers.mockReturnValue([]) - - const { container } = render() - - expect(container.firstChild).toBeNull() - }) - - it("does not render when tool count is below threshold", () => { - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools: [ - { name: "tool1", enabledForPrompt: true }, - { name: "tool2", enabledForPrompt: true }, - ], - }, - ]) - - const { container } = render() - - expect(container.firstChild).toBeNull() - }) - - it("does not render when tool count equals threshold", () => { - // Create tools to exactly match threshold - const tools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD }, (_, i) => ({ - name: `tool${i}`, - enabledForPrompt: true, - })) - - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools, - }, - ]) - - const { container } = render() - - expect(container.firstChild).toBeNull() - }) - - it("renders warning when tool count exceeds threshold", () => { - // Create more tools than the threshold - const tools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD + 10 }, (_, i) => ({ - name: `tool${i}`, - enabledForPrompt: true, - })) - - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools, - }, - ]) - - render() - - expect(screen.getByText("Too many tools enabled")).toBeInTheDocument() - expect( - screen.getByText( - `You have ${MAX_MCP_TOOLS_THRESHOLD + 10} tools enabled via 1 MCP server. Such a high number can confuse the model and lead to errors. Try to keep it below ${MAX_MCP_TOOLS_THRESHOLD}.`, - ), - ).toBeInTheDocument() - }) - - it("ignores disabled servers", () => { - // Create tools across two servers, one disabled - const tools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD + 10 }, (_, i) => ({ - name: `tool${i}`, - enabledForPrompt: true, - })) - - mockMcpServers.mockReturnValue([ - { - name: "disabledServer", - status: "connected", - disabled: true, // This server is disabled - tools, - }, - { - name: "enabledServer", - status: "connected", - disabled: false, - tools: [{ name: "tool1", enabledForPrompt: true }], // Only 1 tool - }, - ]) - - const { container } = render() - - // Should not render because only 1 tool is on enabled server - expect(container.firstChild).toBeNull() - }) - - it("ignores disconnected servers", () => { - const tools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD + 10 }, (_, i) => ({ - name: `tool${i}`, - enabledForPrompt: true, - })) - - mockMcpServers.mockReturnValue([ - { - name: "disconnectedServer", - status: "disconnected", // Not connected - disabled: false, - tools, - }, - ]) - - const { container } = render() - - expect(container.firstChild).toBeNull() - }) - - it("ignores disabled tools", () => { - // Create tools with some disabled - const enabledTools = Array.from({ length: 20 }, (_, i) => ({ - name: `enabledTool${i}`, - enabledForPrompt: true, - })) - const disabledTools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD + 10 }, (_, i) => ({ - name: `disabledTool${i}`, - enabledForPrompt: false, // These are disabled - })) - - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools: [...enabledTools, ...disabledTools], - }, - ]) - - const { container } = render() - - // Should not render because only 20 tools are enabled - expect(container.firstChild).toBeNull() - }) - - it("treats tools with undefined enabledForPrompt as enabled", () => { - // Create tools without enabledForPrompt set (default behavior is enabled) - const tools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD + 5 }, (_, i) => ({ - name: `tool${i}`, - // enabledForPrompt is undefined, which means enabled by default - })) - - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools, - }, - ]) - - render() - - expect(screen.getByText("Too many tools enabled")).toBeInTheDocument() - }) - - it("counts tools across multiple servers", () => { - // Create tools across multiple servers - const tools1 = Array.from({ length: 35 }, (_, i) => ({ - name: `server1tool${i}`, - enabledForPrompt: true, - })) - const tools2 = Array.from({ length: 30 }, (_, i) => ({ - name: `server2tool${i}`, - enabledForPrompt: true, - })) - - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools: tools1, - }, - { - name: "server2", - status: "connected", - disabled: false, - tools: tools2, - }, - ]) - - render() - - // 35 + 30 = 65 tools > 60 threshold - expect(screen.getByText("Too many tools enabled")).toBeInTheDocument() - expect( - screen.getByText( - `You have 65 tools enabled via 2 MCP servers. Such a high number can confuse the model and lead to errors. Try to keep it below ${MAX_MCP_TOOLS_THRESHOLD}.`, - ), - ).toBeInTheDocument() - }) - - it("renders MCP settings link and opens settings when clicked", () => { - const mockWindowPostMessage = vi.spyOn(window, "postMessage") - - // Create more tools than the threshold - const tools = Array.from({ length: MAX_MCP_TOOLS_THRESHOLD + 10 }, (_, i) => ({ - name: `tool${i}`, - enabledForPrompt: true, - })) - - mockMcpServers.mockReturnValue([ - { - name: "server1", - status: "connected", - disabled: false, - tools, - }, - ]) - - render() - - // Verify the link is rendered - const settingsLink = screen.getByText("Open MCP Settings") - expect(settingsLink).toBeInTheDocument() - - // Click the link and verify it posts the message - fireEvent.click(settingsLink) - - expect(mockWindowPostMessage).toHaveBeenCalledWith( - { type: "action", action: "settingsButtonClicked", values: { section: "mcp" } }, - "*", - ) - - mockWindowPostMessage.mockRestore() - }) -}) diff --git a/webview-ui/src/components/chat/__tests__/WarningRow.spec.tsx b/webview-ui/src/components/chat/__tests__/WarningRow.spec.tsx index 38eae810a0..1bcf5d771a 100644 --- a/webview-ui/src/components/chat/__tests__/WarningRow.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/WarningRow.spec.tsx @@ -1,7 +1,7 @@ import { render, screen, fireEvent } from "@/utils/test-utils" import { vscode } from "@/utils/vscode" -import { WarningRow } from "../WarningRow" +import WarningRow from "../WarningRow" // Mock vscode webview messaging vi.mock("@/utils/vscode", () => ({ diff --git a/webview-ui/src/components/common/DismissibleUpsell.tsx b/webview-ui/src/components/common/DismissibleUpsell.tsx deleted file mode 100644 index 6f0e1ad2dc..0000000000 --- a/webview-ui/src/components/common/DismissibleUpsell.tsx +++ /dev/null @@ -1,177 +0,0 @@ -import { memo, ReactNode, useEffect, useState, useRef } from "react" -import { vscode } from "@src/utils/vscode" -import { useAppTranslation } from "@src/i18n/TranslationContext" -import { telemetryClient } from "@src/utils/TelemetryClient" -import { TelemetryEventName } from "@roo-code/types" - -interface DismissibleUpsellProps { - /** Required unique identifier for this upsell */ - upsellId: string - /** Optional CSS class name for styling */ - className?: string - /** Optional Icon component */ - icon?: ReactNode - /** Content to display inside the upsell */ - children: ReactNode - /** Visual variant of the upsell */ - variant?: "default" | "banner" - /** Optional callback when upsell is dismissed */ - onDismiss?: () => void - /** Optional callback when upsell is clicked */ - onClick?: () => void - /** Whether clicking the upsell should also dismiss it (default: false) */ - dismissOnClick?: boolean -} - -const DismissIcon = () => ( - -) - -const DismissibleUpsell = memo( - ({ - upsellId, - className, - icon, - children, - variant = "default", - onDismiss, - onClick, - dismissOnClick = false, - }: DismissibleUpsellProps) => { - const { t } = useAppTranslation() - const [isVisible, setIsVisible] = useState(false) - const isMountedRef = useRef(true) - - useEffect(() => { - // Track mounted state - isMountedRef.current = true - - // Request the current list of dismissed upsells from the extension - vscode.postMessage({ type: "getDismissedUpsells" }) - - // Listen for the response - const handleMessage = (event: MessageEvent) => { - // Only update state if component is still mounted - if (!isMountedRef.current) return - - const message = event.data - // Add null/undefined check for message - if (message && message.type === "dismissedUpsells" && Array.isArray(message.list)) { - // Check if this upsell has been dismissed - if (!message.list.includes(upsellId)) { - setIsVisible(true) - } - } - } - - window.addEventListener("message", handleMessage) - return () => { - isMountedRef.current = false - window.removeEventListener("message", handleMessage) - } - }, [upsellId]) - - const handleDismiss = () => { - // Track telemetry for dismissal - telemetryClient.capture(TelemetryEventName.UPSELL_DISMISSED, { - upsellId: upsellId, - }) - - // First notify the extension to persist the dismissal - // This ensures the message is sent even if the component unmounts quickly - vscode.postMessage({ - type: "dismissUpsell", - upsellId: upsellId, - }) - - // Then hide the upsell - setIsVisible(false) - - // Call the optional callback - onDismiss?.() - } - - // Don't render if not visible - if (!isVisible) { - return null - } - - const variants = { - banner: { - container: - "p-2 bg-vscode-badge-background/80 text-vscode-badge-foreground border-vscode-dropdown-border border", - button: "text-vscode-badge-foreground", - }, - default: { - container: "bg-vscode-notifications-background text-vscode-notifications-foreground", - button: "text-vscode-notifications-foreground", - }, - } - // Build container classes based on variant and presence of click handler - const containerClasses = [ - "relative flex items-start justify-between gap-2", - "text-sm", - variants[variant].container, - onClick && "cursor-pointer hover:opacity-90 transition-opacity duration-200", - className, - ] - .filter(Boolean) - .join(" ") - - // Build button classes based on variant - const buttonClasses = [ - "flex items-center justify-center", - "rounded", - "bg-transparent", - "border-none", - "cursor-pointer", - "hover:opacity-50 transition-opacity duration-200", - variants[variant].button, - "focus:outline focus:outline-1 focus:outline-vscode-focusBorder focus:outline-offset-1", - ].join(" ") - - return ( -
{ - // Track telemetry for click - if (onClick) { - telemetryClient.capture(TelemetryEventName.UPSELL_CLICKED, { - upsellId: upsellId, - }) - } - - // Call the onClick handler if provided - onClick?.() - // Also dismiss if dismissOnClick is true - if (dismissOnClick) { - handleDismiss() - } - }}> - {icon && icon} -
{children}
- -
- ) - }, -) - -DismissibleUpsell.displayName = "DismissibleUpsell" - -export default DismissibleUpsell diff --git a/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx b/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx deleted file mode 100644 index 69e4e94a4f..0000000000 --- a/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx +++ /dev/null @@ -1,625 +0,0 @@ -import { render, screen, fireEvent, waitFor, act } from "@testing-library/react" -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" -import DismissibleUpsell from "../DismissibleUpsell" -import { TelemetryEventName } from "@roo-code/types" - -// Mock the vscode API -const mockPostMessage = vi.fn() -vi.mock("@src/utils/vscode", () => ({ - vscode: { - postMessage: (message: any) => mockPostMessage(message), - }, -})) - -// Mock telemetryClient -const mockCapture = vi.fn() -vi.mock("@src/utils/TelemetryClient", () => ({ - telemetryClient: { - capture: (eventName: string, properties?: Record) => mockCapture(eventName, properties), - }, -})) - -// Mock the translation hook -vi.mock("@src/i18n/TranslationContext", () => ({ - useAppTranslation: () => ({ - t: (key: string) => { - const translations: Record = { - "common:dismiss": "Dismiss", - "common:dismissAndDontShowAgain": "Dismiss and don't show again", - } - return translations[key] || key - }, - }), -})) - -describe("DismissibleUpsell", () => { - beforeEach(() => { - mockPostMessage.mockClear() - mockCapture.mockClear() - vi.clearAllTimers() - }) - - afterEach(() => { - vi.clearAllTimers() - }) - - // Helper function to make the component visible - const makeUpsellVisible = () => { - const messageEvent = new MessageEvent("message", { - data: { - type: "dismissedUpsells", - list: [], // Empty list means no upsells are dismissed - }, - }) - window.dispatchEvent(messageEvent) - } - - it("renders children content", async () => { - render( - -
Test content
- , - ) - - // Component starts hidden, make it visible - makeUpsellVisible() - - // Wait for component to become visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - }) - - it("requests dismissed upsells list on mount", () => { - render( - -
Test content
- , - ) - - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "getDismissedUpsells", - }) - }) - - it("hides the upsell when dismiss button is clicked and tracks telemetry", async () => { - const onDismiss = vi.fn() - const { container } = render( - -
Test content
- , - ) - - // Make component visible first - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - // Find and click the dismiss button - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - fireEvent.click(dismissButton) - - // Check that telemetry was tracked - expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_DISMISSED, { - upsellId: "test-upsell", - }) - - // Check that the dismiss message was sent BEFORE hiding - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "test-upsell", - }) - - // Check that the component is no longer visible - await waitFor(() => { - expect(container.firstChild).toBeNull() - }) - - // Check that the callback was called - expect(onDismiss).toHaveBeenCalled() - }) - - it("hides the upsell if it's in the dismissed list", async () => { - const { container } = render( - -
Test content
- , - ) - - // Component starts hidden by default - expect(container.firstChild).toBeNull() - - // Simulate receiving a message that this upsell is dismissed - const messageEvent = new MessageEvent("message", { - data: { - type: "dismissedUpsells", - list: ["test-upsell", "other-upsell"], - }, - }) - window.dispatchEvent(messageEvent) - - // Check that the component remains hidden - await waitFor(() => { - expect(container.firstChild).toBeNull() - }) - }) - - it("remains visible if not in the dismissed list", async () => { - render( - -
Test content
- , - ) - - // Simulate receiving a message that doesn't include this upsell - const messageEvent = new MessageEvent("message", { - data: { - type: "dismissedUpsells", - list: ["other-upsell"], - }, - }) - window.dispatchEvent(messageEvent) - - // Check that the component is still visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - }) - - it("applies the className prop to the container", async () => { - const { container } = render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(container.firstChild).not.toBeNull() - }) - - expect(container.firstChild).toHaveClass("custom-class") - }) - - it("dismiss button has proper accessibility attributes", async () => { - render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - expect(dismissButton).toHaveAttribute("aria-label", "Dismiss") - expect(dismissButton).toHaveAttribute("title", "Dismiss and don't show again") - }) - - // New edge case tests - it("handles multiple rapid dismissals of the same component", async () => { - const onDismiss = vi.fn() - render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - - // Click multiple times rapidly - fireEvent.click(dismissButton) - fireEvent.click(dismissButton) - fireEvent.click(dismissButton) - - // Should only send one message - expect(mockPostMessage).toHaveBeenCalledTimes(2) // 1 for getDismissedUpsells, 1 for dismissUpsell - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "test-upsell", - }) - - // Callback should only be called once - expect(onDismiss).toHaveBeenCalledTimes(1) - }) - - it("does not update state after component unmounts", async () => { - const { unmount } = render( - -
Test content
- , - ) - - // Unmount the component - unmount() - - // Simulate receiving a message after unmount - const messageEvent = new MessageEvent("message", { - data: { - type: "dismissedUpsells", - list: ["test-upsell"], - }, - }) - - // This should not cause any errors - act(() => { - window.dispatchEvent(messageEvent) - }) - - // No errors should be thrown - expect(true).toBe(true) - }) - - it("handles invalid/malformed messages gracefully", async () => { - render( - -
Test content
- , - ) - - // First make it visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - // Send various malformed messages - const malformedMessages = [ - { type: "dismissedUpsells", list: null }, - { type: "dismissedUpsells", list: "not-an-array" }, - { type: "dismissedUpsells" }, // missing list - { type: "wrongType", list: ["test-upsell"] }, - null, - undefined, - "string-message", - ] - - malformedMessages.forEach((data) => { - const messageEvent = new MessageEvent("message", { data }) - window.dispatchEvent(messageEvent) - }) - - // Component should still be visible - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - it("ensures message is sent before component unmounts on dismiss", async () => { - const { unmount } = render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - fireEvent.click(dismissButton) - - // Message should be sent immediately - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "test-upsell", - }) - - // Unmount immediately after clicking - unmount() - - // Message was already sent before unmount - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "test-upsell", - }) - }) - - it("uses separate id and className props correctly", async () => { - const { container } = render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(container.firstChild).not.toBeNull() - }) - - // className should be applied to the container - expect(container.firstChild).toHaveClass("styling-class") - - // When dismissed, should use the id, not className - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - fireEvent.click(dismissButton) - - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "unique-id", - }) - }) - - it("calls onClick when the container is clicked and tracks telemetry", async () => { - const onClick = vi.fn() - render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - // Click on the container (not the dismiss button) - const container = screen.getByText("Test content").parentElement as HTMLElement - fireEvent.click(container) - - expect(onClick).toHaveBeenCalledTimes(1) - - // Check that telemetry was tracked - expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_CLICKED, { - upsellId: "test-upsell", - }) - }) - - it("does not call onClick when dismiss button is clicked", async () => { - const onClick = vi.fn() - const onDismiss = vi.fn() - render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - // Click the dismiss button - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - fireEvent.click(dismissButton) - - // onClick should not be called, but onDismiss should - expect(onClick).not.toHaveBeenCalled() - expect(onDismiss).toHaveBeenCalledTimes(1) - }) - - it("adds cursor-pointer class when onClick is provided", async () => { - const { container, rerender } = render( - {}}> -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(container.firstChild).not.toBeNull() - }) - - // Should have cursor-pointer when onClick is provided - expect(container.firstChild).toHaveClass("cursor-pointer") - - // Re-render without onClick - rerender( - -
Test content
- , - ) - - // Should not have cursor-pointer when onClick is not provided - expect(container.firstChild).not.toHaveClass("cursor-pointer") - }) - - it("handles both onClick and onDismiss independently", async () => { - const onClick = vi.fn() - const onDismiss = vi.fn() - const { container } = render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - // Click on the container - const containerDiv = screen.getByText("Test content").parentElement as HTMLElement - fireEvent.click(containerDiv) - expect(onClick).toHaveBeenCalledTimes(1) - expect(onDismiss).not.toHaveBeenCalled() - - // Reset mocks - onClick.mockClear() - onDismiss.mockClear() - - // Click the dismiss button - const dismissButton = screen.getByRole("button", { name: /dismiss/i }) - fireEvent.click(dismissButton) - - // Only onDismiss should be called - expect(onClick).not.toHaveBeenCalled() - expect(onDismiss).toHaveBeenCalledTimes(1) - - // Component should be hidden after dismiss - await waitFor(() => { - expect(container.firstChild).toBeNull() - }) - }) - - it("dismisses when clicked if dismissOnClick is true and tracks both telemetry events", async () => { - const onClick = vi.fn() - const onDismiss = vi.fn() - const { container } = render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - const containerDiv = screen.getByText("Test content").parentElement as HTMLElement - fireEvent.click(containerDiv) - - expect(onClick).toHaveBeenCalledTimes(1) - expect(onDismiss).toHaveBeenCalledTimes(1) - - // Check that both telemetry events were tracked - expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_CLICKED, { - upsellId: "test-upsell", - }) - expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_DISMISSED, { - upsellId: "test-upsell", - }) - - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "test-upsell", - }) - - await waitFor(() => { - expect(container.firstChild).toBeNull() - }) - }) - - it("dismisses on container click when dismissOnClick is true and no onClick is provided; tracks only dismissal", async () => { - const onDismiss = vi.fn() - const { container } = render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - // Click on the container (not the dismiss button) - const containerDiv = screen.getByText("Test content").parentElement as HTMLElement - fireEvent.click(containerDiv) - - // onDismiss should be called - expect(onDismiss).toHaveBeenCalledTimes(1) - - // Telemetry: only dismissal should be tracked - expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_DISMISSED, { - upsellId: "test-upsell", - }) - expect(mockCapture).not.toHaveBeenCalledWith(TelemetryEventName.UPSELL_CLICKED, expect.anything()) - - // Dismiss message should be sent - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "dismissUpsell", - upsellId: "test-upsell", - }) - - // Component should be hidden - await waitFor(() => { - expect(container.firstChild).toBeNull() - }) - }) - it("does not dismiss when clicked if dismissOnClick is false", async () => { - const onClick = vi.fn() - const onDismiss = vi.fn() - render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - const containerDiv = screen.getByText("Test content").parentElement as HTMLElement - fireEvent.click(containerDiv) - - expect(onClick).toHaveBeenCalledTimes(1) - expect(onDismiss).not.toHaveBeenCalled() - - expect(mockPostMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "dismissUpsell" })) - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - it("does not dismiss when clicked if dismissOnClick is not provided (defaults to false)", async () => { - const onClick = vi.fn() - const onDismiss = vi.fn() - render( - -
Test content
- , - ) - - // Make component visible - makeUpsellVisible() - - // Wait for component to be visible - await waitFor(() => { - expect(screen.getByText("Test content")).toBeInTheDocument() - }) - - const containerDiv = screen.getByText("Test content").parentElement as HTMLElement - fireEvent.click(containerDiv) - - expect(onClick).toHaveBeenCalledTimes(1) - expect(onDismiss).not.toHaveBeenCalled() - expect(screen.getByText("Test content")).toBeInTheDocument() - }) -}) diff --git a/webview-ui/src/components/settings/useSettingsSearch.ts b/webview-ui/src/components/settings/useSettingsSearch.ts index 9b2c4648c5..8937157f32 100644 --- a/webview-ui/src/components/settings/useSettingsSearch.ts +++ b/webview-ui/src/components/settings/useSettingsSearch.ts @@ -72,35 +72,6 @@ export function useSearchIndexRegistry(getSectionLabel: (section: SectionName) = return { contextValue, index } } -/** - * Scan the DOM for searchable settings within a container. - * This is called once on mount to build the index. - */ -export function scanDOMForSearchableSettings( - container: Element, - getSectionLabel: (section: SectionName) => string, -): SearchableSettingData[] { - const settings: SearchableSettingData[] = [] - const elements = container.querySelectorAll("[data-searchable]") - - elements.forEach((el) => { - const settingId = el.getAttribute("data-setting-id") - const section = el.getAttribute("data-setting-section") as SectionName | null - const label = el.getAttribute("data-setting-label") - - if (settingId && section && label) { - settings.push({ - settingId, - section, - label, - sectionLabel: getSectionLabel(section), - }) - } - }) - - return settings -} - interface UseSettingsSearchOptions { index: SearchableSettingData[] } diff --git a/webview-ui/src/components/settings/utils/__tests__/headers.test.ts b/webview-ui/src/components/settings/utils/__tests__/headers.spec.ts similarity index 100% rename from webview-ui/src/components/settings/utils/__tests__/headers.test.ts rename to webview-ui/src/components/settings/utils/__tests__/headers.spec.ts diff --git a/webview-ui/src/components/settings/utils/__tests__/organizationFilters.test.ts b/webview-ui/src/components/settings/utils/__tests__/organizationFilters.spec.ts similarity index 95% rename from webview-ui/src/components/settings/utils/__tests__/organizationFilters.test.ts rename to webview-ui/src/components/settings/utils/__tests__/organizationFilters.spec.ts index 84183cd3c9..d5e32b7b75 100644 --- a/webview-ui/src/components/settings/utils/__tests__/organizationFilters.test.ts +++ b/webview-ui/src/components/settings/utils/__tests__/organizationFilters.spec.ts @@ -35,12 +35,14 @@ describe("organizationFilters", () => { allowAll: false, providers: { anthropic: { allowAll: true }, + openai: { allowAll: false, models: ["gpt-4o"] }, gemini: { allowAll: true }, }, } const result = filterProviders(mockProviders, allowList) - expect(result).toHaveLength(2) + expect(result).toHaveLength(3) expect(result).toContainEqual({ value: "anthropic", label: "Anthropic" }) + expect(result).toContainEqual({ value: "openai", label: "OpenAI" }) expect(result).toContainEqual({ value: "gemini", label: "Gemini" }) }) }) diff --git a/webview-ui/src/components/ui/select-dropdown.tsx b/webview-ui/src/components/ui/select-dropdown.tsx index 5258123e04..288d81b16f 100644 --- a/webview-ui/src/components/ui/select-dropdown.tsx +++ b/webview-ui/src/components/ui/select-dropdown.tsx @@ -11,7 +11,6 @@ import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui" import { StandardTooltip } from "@/components/ui" export enum DropdownOptionType { - ITEM = "item", SEPARATOR = "separator", SHORTCUT = "shortcut", ACTION = "action", diff --git a/webview-ui/src/hooks/useTooManyTools.ts b/webview-ui/src/hooks/useTooManyTools.ts index ba18b10a70..75a3d834c6 100644 --- a/webview-ui/src/hooks/useTooManyTools.ts +++ b/webview-ui/src/hooks/useTooManyTools.ts @@ -20,7 +20,7 @@ export interface TooManyToolsInfo { /** * Hook that calculates tool counts and provides localized warning messages. - * Used by TooManyToolsWarning components in both chat and MCP settings views. + * Used by MCP settings views to summarize enabled tool counts. * * @returns Tool count information and localized messages * diff --git a/webview-ui/src/i18n/TranslationContext.tsx b/webview-ui/src/i18n/TranslationContext.tsx index 17f1545065..f2950ae2f8 100644 --- a/webview-ui/src/i18n/TranslationContext.tsx +++ b/webview-ui/src/i18n/TranslationContext.tsx @@ -13,7 +13,7 @@ export const TranslationContext = createContext<{ }) // Translation provider component -export const TranslationProvider: React.FC<{ children: ReactNode }> = ({ children }) => { +const TranslationProvider: React.FC<{ children: ReactNode }> = ({ children }) => { // Initialize with default configuration const { i18n } = useTranslation() // Get the extension state directly - it already contains all state properties diff --git a/webview-ui/src/oauth/urls.ts b/webview-ui/src/oauth/urls.ts index dca51a87b2..a756254267 100644 --- a/webview-ui/src/oauth/urls.ts +++ b/webview-ui/src/oauth/urls.ts @@ -9,6 +9,7 @@ export function getOpenRouterAuthUrl(uriScheme?: string) { return `https://openrouter.ai/auth?callback_url=${getCallbackUrl(providerIdentifiers.openrouter, uriScheme)}` } +/** @internal Exported for focused implementation tests. */ export function getRequestyAuthUrl(uriScheme?: string) { return `https://app.requesty.ai/oauth/authorize?callback_url=${getCallbackUrl(providerIdentifiers.requesty, uriScheme)}` } diff --git a/webview-ui/src/utils/__tests__/path-mentions.test.ts b/webview-ui/src/utils/__tests__/path-mentions.spec.ts similarity index 93% rename from webview-ui/src/utils/__tests__/path-mentions.test.ts rename to webview-ui/src/utils/__tests__/path-mentions.spec.ts index 1163324503..b0e84ec935 100644 --- a/webview-ui/src/utils/__tests__/path-mentions.test.ts +++ b/webview-ui/src/utils/__tests__/path-mentions.spec.ts @@ -79,10 +79,14 @@ describe("Path Mentions Utilities", () => { expect(convertToMentionPath(absPath, MOCK_CWD_POSIX)).toBe("@/src/normal.ts") }) - it("should add leading slash if missing after cwd removal", () => { - const absPath = "/Users/test/projectfile.txt" // Edge case: file directly in project root + it("should not convert external paths that share the cwd prefix", () => { + const absPath = "/Users/test/projectfile.txt" const cwd = "/Users/test/project" - expect(convertToMentionPath(absPath, cwd)).toBe("@/file.txt") // Should still add '/' + expect(convertToMentionPath(absPath, cwd)).toBe("/Users/test/projectfile.txt") + }) + + it("should preserve the leading slash for the cwd itself", () => { + expect(convertToMentionPath(MOCK_CWD_POSIX, MOCK_CWD_POSIX)).toBe("@/") }) it("should handle cwd with trailing slash", () => { diff --git a/webview-ui/src/utils/format.ts b/webview-ui/src/utils/format.ts index 29c7a2c966..cd3521aed2 100644 --- a/webview-ui/src/utils/format.ts +++ b/webview-ui/src/utils/format.ts @@ -13,6 +13,9 @@ export function formatLargeNumber(num: number): string { return num.toString() } +/** + * @internal + */ export const formatDate = (timestamp: number) => { const date = new Date(timestamp) const locale = i18next.language || "en" diff --git a/webview-ui/src/utils/path-mentions.ts b/webview-ui/src/utils/path-mentions.ts index 4ad25b9b56..ec380e0603 100644 --- a/webview-ui/src/utils/path-mentions.ts +++ b/webview-ui/src/utils/path-mentions.ts @@ -65,7 +65,7 @@ export function convertToMentionPath(path: string, cwd?: string): string { const lowerPath = normalizedPath.toLowerCase() const lowerCwd = normalizedCwd.toLowerCase() - if (lowerPath.startsWith(lowerCwd)) { + if (lowerPath === lowerCwd || lowerPath.startsWith(`${lowerCwd}/`)) { let relativePath = normalizedPath.substring(normalizedCwd.length) // Ensure there's a slash after the @ symbol when we create the mention path relativePath = relativePath.startsWith("/") ? relativePath : "/" + relativePath diff --git a/webview-ui/src/utils/sourceMapUtils.ts b/webview-ui/src/utils/sourceMapUtils.ts index 8e779576a6..64478ec7af 100644 --- a/webview-ui/src/utils/sourceMapUtils.ts +++ b/webview-ui/src/utils/sourceMapUtils.ts @@ -165,6 +165,8 @@ export function enhanceErrorWithSourceMaps(error: Error, componentStack?: string /** * Parse a stack trace string into structured stack frames * This is kept for backward compatibility with tests + * + * @internal */ export async function parseStackTrace(stack: string): Promise { if (!stack) return []