From 2afa9f17bc44da8734046ea4bad3e5e7aa1403a9 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sun, 13 Sep 2026 02:13:48 +0300 Subject: [PATCH 1/4] refactor(code-index): extract manager registry --- src/__tests__/extension.spec.ts | 4 +- .../__tests__/registerCommands.spec.ts | 4 +- src/activate/registerCommands.ts | 4 +- src/core/prompts/system.ts | 4 +- src/core/task/build-tools.ts | 4 +- src/core/tools/CodebaseSearchTool.ts | 4 +- src/core/webview/ClineProvider.ts | 3 +- .../webview/__tests__/ClineProvider.spec.ts | 6 +- src/core/webview/webviewMessageHandler.ts | 4 +- src/extension.ts | 3 +- .../code-index-manager-registry.spec.ts | 120 ++++++++++++++++++ .../code-index/__tests__/manager.spec.ts | 15 ++- .../code-index/code-index-manager-registry.ts | 53 ++++++++ src/services/code-index/manager.ts | 55 +------- 14 files changed, 203 insertions(+), 80 deletions(-) create mode 100644 src/services/code-index/__tests__/code-index-manager-registry.spec.ts create mode 100644 src/services/code-index/code-index-manager-registry.ts diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index bb72d567dd..c6485f4abe 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -139,8 +139,8 @@ vi.mock("../services/mcp/McpServerManager", () => ({ }, })) -vi.mock("../services/code-index/manager", () => ({ - CodeIndexManager: { +vi.mock("../services/code-index/code-index-manager-registry", () => ({ + CodeIndexManagerRegistry: { getInstance: vi.fn().mockReturnValue(null), }, })) diff --git a/src/activate/__tests__/registerCommands.spec.ts b/src/activate/__tests__/registerCommands.spec.ts index 67a2b935ec..88e96f80be 100644 --- a/src/activate/__tests__/registerCommands.spec.ts +++ b/src/activate/__tests__/registerCommands.spec.ts @@ -67,8 +67,8 @@ vi.mock("../../core/config/importExport", () => ({ importSettingsWithFeedback: vi.fn(), })) -vi.mock("../../services/code-index/manager", () => ({ - CodeIndexManager: { +vi.mock("../../services/code-index/code-index-manager-registry", () => ({ + CodeIndexManagerRegistry: { getInstance: vi.fn(), }, })) diff --git a/src/activate/registerCommands.ts b/src/activate/registerCommands.ts index 692aabfd68..f062b56eab 100644 --- a/src/activate/registerCommands.ts +++ b/src/activate/registerCommands.ts @@ -10,7 +10,7 @@ import { ClineProvider } from "../core/webview/ClineProvider" import { ContextProxy } from "../core/config/ContextProxy" import { focusPanel } from "../utils/focusPanel" import { handleNewTask } from "./handleTask" -import { CodeIndexManager } from "../services/code-index/manager" +import { CodeIndexManagerRegistry } from "../services/code-index/code-index-manager-registry" import { importSettingsWithFeedback } from "../core/config/importExport" import { MdmService } from "../services/mdm/MdmService" import { registerRipgrepDiagnosticCommand } from "../services/ripgrep/diagnostic" @@ -227,7 +227,7 @@ export const openClineInNewTab = async ({ context, outputChannel }: Omit { throw new Error("Extension context is not available.") } - const manager = CodeIndexManager.getInstance(context) + const manager = CodeIndexManagerRegistry.getInstance(context) if (!manager) { throw new Error("CodeIndexManager is not available.") diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 495fe454b7..854c02899d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -91,6 +91,7 @@ import { McpServerManager } from "../../services/mcp/McpServerManager" import { MarketplaceManager } from "../../services/marketplace" import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckpointService" import { CodeIndexManager } from "../../services/code-index/manager" +import { CodeIndexManagerRegistry } from "../../services/code-index/code-index-manager-registry" import type { IndexProgressUpdate } from "../../services/code-index/interfaces/manager" import { MdmService } from "../../services/mdm/MdmService" import { SkillsManager } from "../../services/skills/SkillsManager" @@ -3307,7 +3308,7 @@ export class ClineProvider * @returns CodeIndexManager instance for the current workspace or the default one */ public getCurrentWorkspaceCodeIndexManager(): CodeIndexManager | undefined { - return CodeIndexManager.getInstance(this.context) + return CodeIndexManagerRegistry.getInstance(this.context) } /** diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index bfd4706dcc..97c4dd877e 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3225,7 +3225,7 @@ describe("webviewMessageHandler no-floating-promises coverage", () => { }) it("catches auto-enabled indexing failures and posts the resulting status", async () => { - const { CodeIndexManager } = await import("../../../services/code-index/manager") + const { CodeIndexManagerRegistry } = await import("../../../services/code-index/code-index-manager-registry") let workspaceEnabled = false const manager = createIndexManager({ setAutoEnableDefault: vi.fn().mockImplementation(async () => { @@ -3235,8 +3235,8 @@ describe("webviewMessageHandler no-floating-promises coverage", () => { }) Object.defineProperty(manager, "isWorkspaceEnabled", { get: () => workspaceEnabled }) const getAllInstances = vi - .spyOn(CodeIndexManager, "getAllInstances") - .mockReturnValue([manager] as unknown as ReturnType) + .spyOn(CodeIndexManagerRegistry, "getAllInstances") + .mockReturnValue([manager] as unknown as ReturnType) const provider = createProvider({ getCurrentWorkspaceCodeIndexManager: vi.fn().mockReturnValue(manager), }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 0dad65a480..34a35ea3ca 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -62,7 +62,7 @@ import { Package } from "../../shared/package" import { type RouterName, toRouterName } from "../../shared/api" import { MessageEnhancer } from "./messageEnhancer" -import { CodeIndexManager } from "../../services/code-index/manager" +import { CodeIndexManagerRegistry } from "../../services/code-index/code-index-manager-registry" import { checkExistKey } from "../../shared/checkExistApiConfig" import { getRouterRemovalMessage, getRouterUnavailableSignInMessage } from "../config/routerRemoval" import { experimentDefault } from "../../shared/experiments" @@ -3311,7 +3311,7 @@ export const webviewMessageHandler = async ( return } // Capture prior state for every manager before persisting the global change - const allManagers = CodeIndexManager.getAllInstances() + const allManagers = CodeIndexManagerRegistry.getAllInstances() const priorStates = new Map(allManagers.map((m) => [m, m.isWorkspaceEnabled])) await manager.setAutoEnableDefault(message.bool ?? true) // Apply stop/start to every affected manager diff --git a/src/extension.ts b/src/extension.ts index 0a78cd32ba..13bcf61666 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -35,6 +35,7 @@ import { openAiCodexOAuthManager } from "./integrations/openai-codex/oauth" import { kimiCodeOAuthManager } from "./integrations/kimi-code/oauth" import { McpServerManager } from "./services/mcp/McpServerManager" import { CodeIndexManager } from "./services/code-index/manager" +import { CodeIndexManagerRegistry } from "./services/code-index/code-index-manager-registry" import { MdmService } from "./services/mdm/MdmService" import { migrateSettings } from "./utils/migrateSettings" import { autoImportSettings } from "./utils/autoImportSettings" @@ -200,7 +201,7 @@ export async function activate(context: vscode.ExtensionContext) { if (vscode.workspace.workspaceFolders) { for (const folder of vscode.workspace.workspaceFolders) { - const manager = CodeIndexManager.getInstance(context, folder.uri.fsPath) + const manager = CodeIndexManagerRegistry.getInstance(context, folder.uri.fsPath) if (manager) { codeIndexManagers.push(manager) diff --git a/src/services/code-index/__tests__/code-index-manager-registry.spec.ts b/src/services/code-index/__tests__/code-index-manager-registry.spec.ts new file mode 100644 index 0000000000..fd5547f61c --- /dev/null +++ b/src/services/code-index/__tests__/code-index-manager-registry.spec.ts @@ -0,0 +1,120 @@ +import * as vscode from "vscode" +import { makeExtensionContext, makeTextDocument, makeTextEditor, makeUri } from "../../../test-utils/vscode" +import { CodeIndexManager } from "../manager" +import { CodeIndexManagerRegistry } from "../code-index-manager-registry" + +vi.mock("vscode", () => ({ + workspace: { workspaceFolders: undefined, getWorkspaceFolder: vi.fn() }, + window: { activeTextEditor: undefined }, + Uri: { file: vi.fn() }, +})) + +vi.mock("../manager", () => ({ + CodeIndexManager: vi.fn().mockImplementation(function () { + return { dispose: vi.fn() } + }), +})) + +describe("CodeIndexManagerRegistry", () => { + let context: vscode.ExtensionContext + let first: vscode.WorkspaceFolder + let second: vscode.WorkspaceFolder + + beforeEach(() => { + vi.clearAllMocks() + context = makeExtensionContext() + first = { uri: makeUri("/first"), name: "first", index: 0 } + second = { uri: makeUri("/second"), name: "second", index: 1 } + Object.defineProperty(vscode.workspace, "workspaceFolders", { configurable: true, value: [first, second] }) + Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: undefined }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(undefined) + vi.mocked(vscode.Uri.file).mockImplementation((value) => makeUri(value)) + }) + + afterEach(() => { + CodeIndexManagerRegistry.disposeAll() + vi.restoreAllMocks() + }) + + it.each([{ folders: undefined }, { folders: [] }])("returns no manager with folders=$folders", ({ folders }) => { + Object.defineProperty(vscode.workspace, "workspaceFolders", { configurable: true, value: folders }) + expect(CodeIndexManagerRegistry.getInstance(context)).toBeUndefined() + expect(CodeIndexManager).not.toHaveBeenCalled() + }) + + it("uses the first workspace when there is no active editor", () => { + CodeIndexManagerRegistry.getInstance(context) + expect(CodeIndexManager).toHaveBeenCalledWith("/first", first.uri, context) + }) + + it("prefers the active editor's workspace", () => { + const editor = makeTextEditor({ document: makeTextDocument({ uri: makeUri("/second/file.ts") }) }) + Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: editor }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(second) + CodeIndexManagerRegistry.getInstance(context) + expect(vscode.workspace.getWorkspaceFolder).toHaveBeenCalledWith(editor.document.uri) + expect(CodeIndexManager).toHaveBeenCalledWith("/second", second.uri, context) + }) + + it("falls back to the first workspace for an editor outside all folders", () => { + Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: makeTextEditor() }) + CodeIndexManagerRegistry.getInstance(context) + expect(CodeIndexManager).toHaveBeenCalledWith("/first", first.uri, context) + }) + + it("gives an explicit path priority over the active editor", () => { + Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: makeTextEditor() }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(first) + CodeIndexManagerRegistry.getInstance(context, "/second") + expect(CodeIndexManager).toHaveBeenCalledWith("/second", second.uri, context) + expect(vscode.workspace.getWorkspaceFolder).not.toHaveBeenCalled() + }) + + it("preserves the actual remote workspace URI", () => { + const uri = makeUri("/remote", { scheme: "vscode-remote", authority: "ssh-remote+host" }) + Object.defineProperty(vscode.workspace, "workspaceFolders", { + configurable: true, + value: [{ uri, name: "remote", index: 0 }], + }) + CodeIndexManagerRegistry.getInstance(context, "/remote") + expect(CodeIndexManager).toHaveBeenCalledWith("/remote", uri, context) + expect(vi.mocked(CodeIndexManager).mock.calls[0][1]).toBe(uri) + expect(vscode.Uri.file).not.toHaveBeenCalled() + }) + + it("constructs a file URI for an explicit path without open workspaces", () => { + Object.defineProperty(vscode.workspace, "workspaceFolders", { configurable: true, value: undefined }) + const uri = makeUri("/outside folder/#name") + vi.mocked(vscode.Uri.file).mockReturnValue(uri) + CodeIndexManagerRegistry.getInstance(context, uri.fsPath) + expect(vscode.Uri.file).toHaveBeenCalledWith(uri.fsPath) + expect(CodeIndexManager).toHaveBeenCalledWith(uri.fsPath, uri, context) + }) + + it("reuses the same path and keeps different paths isolated", () => { + const a = CodeIndexManagerRegistry.getInstance(context, "/first") + expect(CodeIndexManagerRegistry.getInstance(makeExtensionContext(), "/first")).toBe(a) + const b = CodeIndexManagerRegistry.getInstance(context, "/second") + expect(b).not.toBe(a) + expect(CodeIndexManager).toHaveBeenCalledTimes(2) + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([a, b]) + }) + + it("returns a snapshot that cannot mutate the cache", () => { + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) + const manager = CodeIndexManagerRegistry.getInstance(context) + CodeIndexManagerRegistry.getAllInstances().pop() + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([manager]) + }) + + it("disposes every manager, supports repeated cleanup and recreates instances", () => { + const a = CodeIndexManagerRegistry.getInstance(context, "/first")! + const b = CodeIndexManagerRegistry.getInstance(context, "/second")! + CodeIndexManagerRegistry.disposeAll() + CodeIndexManagerRegistry.disposeAll() + expect(a.dispose).toHaveBeenCalledTimes(1) + expect(b.dispose).toHaveBeenCalledTimes(1) + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) + expect(CodeIndexManagerRegistry.getInstance(context, "/first")).not.toBe(a) + }) +}) diff --git a/src/services/code-index/__tests__/manager.spec.ts b/src/services/code-index/__tests__/manager.spec.ts index ce52593ed5..16657a903c 100644 --- a/src/services/code-index/__tests__/manager.spec.ts +++ b/src/services/code-index/__tests__/manager.spec.ts @@ -1,4 +1,5 @@ import { CodeIndexManager } from "../manager" +import { CodeIndexManagerRegistry } from "../code-index-manager-registry" import { CodeIndexServiceFactory } from "../service-factory" import type { MockedClass } from "vitest" import * as path from "path" @@ -126,7 +127,7 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { beforeEach(() => { // Clear all instances before each test - CodeIndexManager.disposeAll() + CodeIndexManagerRegistry.disposeAll() const workspaceStateStore: Record = {} const globalStateStore: Record = {} @@ -160,11 +161,11 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { languageModelAccessInformation: {} as any, } - manager = CodeIndexManager.getInstance(mockContext)! + manager = CodeIndexManagerRegistry.getInstance(mockContext)! }) afterEach(() => { - CodeIndexManager.disposeAll() + CodeIndexManagerRegistry.disposeAll() }) describe("handleSettingsChange", () => { @@ -733,7 +734,7 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { }) it("should store enablement per folder URI, not per window", async () => { - CodeIndexManager.disposeAll() + CodeIndexManagerRegistry.disposeAll() const vscode = await import("vscode") @@ -764,8 +765,8 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { { uri: folderBUri, name: "folderB", index: 1 }, ] - const managerA = CodeIndexManager.getInstance(sharedContext as any, folderAPath)! - const managerB = CodeIndexManager.getInstance(sharedContext as any, folderBPath)! + const managerA = CodeIndexManagerRegistry.getInstance(sharedContext as any, folderAPath)! + const managerB = CodeIndexManagerRegistry.getInstance(sharedContext as any, folderBPath)! // Both start disabled (autoEnableDefault is false via globalState mock) expect(managerA.isWorkspaceEnabled).toBe(false) @@ -784,7 +785,7 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { expect(managerA.isWorkspaceEnabled).toBe(false) expect(managerB.isWorkspaceEnabled).toBe(true) - CodeIndexManager.disposeAll() + CodeIndexManagerRegistry.disposeAll() }) }) diff --git a/src/services/code-index/code-index-manager-registry.ts b/src/services/code-index/code-index-manager-registry.ts new file mode 100644 index 0000000000..4610bd5d36 --- /dev/null +++ b/src/services/code-index/code-index-manager-registry.ts @@ -0,0 +1,53 @@ +import * as vscode from "vscode" +import { CodeIndexManager } from "./manager" + +/** Resolves workspaces and owns their cached CodeIndexManager instances. */ +export class CodeIndexManagerRegistry { + private static instances = new Map() + + public static getInstance(context: vscode.ExtensionContext, workspacePath?: string): CodeIndexManager | undefined { + const folder = this.resolveWorkspaceFolder(workspacePath) + const resolvedPath = workspacePath || folder?.uri.fsPath + if (!resolvedPath) { + return undefined + } + + const existing = this.instances.get(resolvedPath) + if (existing) { + return existing + } + + // Preserve real workspace URIs, including remote schemes and authorities. + const folderUri = folder?.uri ?? vscode.Uri.file(resolvedPath) + const manager = new CodeIndexManager(resolvedPath, folderUri, context) + this.instances.set(resolvedPath, manager) + return manager + } + + public static getAllInstances(): CodeIndexManager[] { + return Array.from(this.instances.values()) + } + + public static disposeAll(): void { + for (const instance of this.instances.values()) { + instance.dispose() + } + this.instances.clear() + } + + private static resolveWorkspaceFolder(workspacePath?: string): vscode.WorkspaceFolder | undefined { + if (workspacePath) { + return vscode.workspace.workspaceFolders?.find((folder) => folder.uri.fsPath === workspacePath) + } + + const activeEditor = vscode.window.activeTextEditor + if (activeEditor) { + const folder = vscode.workspace.getWorkspaceFolder(activeEditor.document.uri) + if (folder) { + return folder + } + } + + return vscode.workspace.workspaceFolders?.[0] + } +} diff --git a/src/services/code-index/manager.ts b/src/services/code-index/manager.ts index dd36a32d88..967bb855b3 100644 --- a/src/services/code-index/manager.ts +++ b/src/services/code-index/manager.ts @@ -18,9 +18,6 @@ import { TelemetryService } from "@roo-code/telemetry" import { TelemetryEventName } from "@roo-code/types" export class CodeIndexManager { - // --- Singleton Implementation --- - private static instances = new Map() // Map workspace path to instance - // Specialized class instances private _configManager: CodeIndexConfigManager | undefined private readonly _stateManager: CodeIndexStateManager @@ -33,61 +30,11 @@ export class CodeIndexManager { // Flag to prevent race conditions during error recovery private _isRecoveringFromError = false - public static getInstance(context: vscode.ExtensionContext, workspacePath?: string): CodeIndexManager | undefined { - // Resolve the workspace folder to get both fsPath and the real URI - let folder: vscode.WorkspaceFolder | undefined - - if (workspacePath) { - folder = vscode.workspace.workspaceFolders?.find((f) => f.uri.fsPath === workspacePath) - } else { - const activeEditor = vscode.window.activeTextEditor - if (activeEditor) { - folder = vscode.workspace.getWorkspaceFolder(activeEditor.document.uri) - } - if (!folder) { - const workspaceFolders = vscode.workspace.workspaceFolders - if (!workspaceFolders || workspaceFolders.length === 0) { - return undefined - } - folder = workspaceFolders[0] - } - workspacePath = folder.uri.fsPath - } - - if (!CodeIndexManager.instances.has(workspacePath)) { - // folder may be undefined when workspacePath was provided but doesn't match - // any workspace folder (e.g. cwd passed from a tool). Fall back to file:// URI. - const folderUri = - folder?.uri ?? - ({ - fsPath: workspacePath, - scheme: "file", - authority: "", - path: workspacePath, - toString: () => `file://${workspacePath}`, - } as unknown as vscode.Uri) - CodeIndexManager.instances.set(workspacePath, new CodeIndexManager(workspacePath, folderUri, context)) - } - return CodeIndexManager.instances.get(workspacePath)! - } - - public static getAllInstances(): CodeIndexManager[] { - return Array.from(CodeIndexManager.instances.values()) - } - - public static disposeAll(): void { - for (const instance of CodeIndexManager.instances.values()) { - instance.dispose() - } - CodeIndexManager.instances.clear() - } - private readonly workspacePath: string private readonly _folderUri: vscode.Uri private readonly context: vscode.ExtensionContext - // Private constructor for singleton pattern - private constructor(workspacePath: string, folderUri: vscode.Uri, context: vscode.ExtensionContext) { + public constructor(workspacePath: string, folderUri: vscode.Uri, context: vscode.ExtensionContext) { this.workspacePath = workspacePath this._folderUri = folderUri this.context = context From 2dc6f28c201ec86df269ce4669a53f9299734a8e Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sun, 13 Sep 2026 02:21:37 +0300 Subject: [PATCH 2/4] test(task): mock code index registry in task suite --- src/core/task/__tests__/Task.spec.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 1bcacd459c..dfe410bdb6 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -131,6 +131,13 @@ vi.mock("p-wait-for", () => ({ default: vi.fn().mockImplementation(async () => Promise.resolve()), })) +// Task tests do not exercise indexing; keep workspace resolution and its cache out of this suite. +vi.mock("../../../services/code-index/code-index-manager-registry", () => ({ + CodeIndexManagerRegistry: { + getInstance: vi.fn().mockReturnValue(undefined), + }, +})) + vi.mock("vscode", () => { const mockDisposable = { dispose: vi.fn() } const mockEventEmitter = { event: vi.fn(), fire: vi.fn() } From 8637e486e88cf14b8b9c4e6d4e7969b93e7fc191 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sun, 13 Sep 2026 09:59:17 +0300 Subject: [PATCH 3/4] test(code-index): remove redundant context casts --- src/eslint-suppressions.json | 2 +- src/services/code-index/__tests__/manager.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index d90272962b..0e5207046c 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -1301,7 +1301,7 @@ }, "services/code-index/__tests__/manager.spec.ts": { "@typescript-eslint/no-explicit-any": { - "count": 89 + "count": 87 } }, "services/code-index/__tests__/orchestrator.spec.ts": { diff --git a/src/services/code-index/__tests__/manager.spec.ts b/src/services/code-index/__tests__/manager.spec.ts index 16657a903c..33ba6b0cd1 100644 --- a/src/services/code-index/__tests__/manager.spec.ts +++ b/src/services/code-index/__tests__/manager.spec.ts @@ -765,8 +765,8 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { { uri: folderBUri, name: "folderB", index: 1 }, ] - const managerA = CodeIndexManagerRegistry.getInstance(sharedContext as any, folderAPath)! - const managerB = CodeIndexManagerRegistry.getInstance(sharedContext as any, folderBPath)! + const managerA = CodeIndexManagerRegistry.getInstance(sharedContext, folderAPath)! + const managerB = CodeIndexManagerRegistry.getInstance(sharedContext, folderBPath)! // Both start disabled (autoEnableDefault is false via globalState mock) expect(managerA.isWorkspaceEnabled).toBe(false) From 8dcc696fb529d673d93bbf0411cb2c2c19044302 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sun, 13 Sep 2026 15:36:32 +0300 Subject: [PATCH 4/4] fix(code-index): search task workspace with full tool coverage --- src/core/tools/CodebaseSearchTool.ts | 2 +- .../__tests__/CodebaseSearchTool.spec.ts | 344 ++++++++++++++++++ .../CodebaseSearchTool.workspace.spec.ts | 133 +++++++ 3 files changed, 478 insertions(+), 1 deletion(-) create mode 100644 src/core/tools/__tests__/CodebaseSearchTool.spec.ts create mode 100644 src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts diff --git a/src/core/tools/CodebaseSearchTool.ts b/src/core/tools/CodebaseSearchTool.ts index afc5ee0dd0..fdf3bc4f38 100644 --- a/src/core/tools/CodebaseSearchTool.ts +++ b/src/core/tools/CodebaseSearchTool.ts @@ -57,7 +57,7 @@ export class CodebaseSearchTool extends BaseTool<"codebase_search"> { throw new Error("Extension context is not available.") } - const manager = CodeIndexManagerRegistry.getInstance(context) + const manager = CodeIndexManagerRegistry.getInstance(context, workspacePath) if (!manager) { throw new Error("CodeIndexManager is not available.") diff --git a/src/core/tools/__tests__/CodebaseSearchTool.spec.ts b/src/core/tools/__tests__/CodebaseSearchTool.spec.ts new file mode 100644 index 0000000000..26b3b95fab --- /dev/null +++ b/src/core/tools/__tests__/CodebaseSearchTool.spec.ts @@ -0,0 +1,344 @@ +import * as vscode from "vscode" + +import type { Task } from "../../task/Task" +import type { ClineProvider } from "../../webview/ClineProvider" +import type { CodeIndexManager } from "../../../services/code-index/manager" +import type { VectorStoreSearchResult } from "../../../services/code-index/interfaces" +import type { ToolUse } from "../../../shared/tools" +import { CodeIndexManagerRegistry } from "../../../services/code-index/code-index-manager-registry" +import { makeExtensionContext } from "../../../test-utils/vscode" +import { getWorkspacePath } from "../../../utils/path" +import { formatResponse } from "../../prompts/responses" +import type { ToolCallbacks } from "../BaseTool" +import { CodebaseSearchTool, codebaseSearchTool } from "../CodebaseSearchTool" + +vi.mock("vscode", () => ({ workspace: { asRelativePath: vi.fn() } })) +vi.mock("../../../utils/path", () => ({ getWorkspacePath: vi.fn() })) +vi.mock("../../../services/code-index/code-index-manager-registry", () => ({ + CodeIndexManagerRegistry: { getInstance: vi.fn() }, +})) + +describe("CodebaseSearchTool", () => { + const query = "find handlers" + let tool: CodebaseSearchTool + let task: Task + let context: vscode.ExtensionContext + let callbacks: ToolCallbacks + let manager: Pick + let deref: ReturnType> + + beforeEach(() => { + vi.resetAllMocks() + tool = new CodebaseSearchTool() + context = makeExtensionContext() + // Structural doubles expose only the provider/task/manager members consumed by the tool. + deref = vi.fn().mockReturnValue({ context } as ClineProvider) + const taskStub: Pick< + Task, + | "cwd" + | "providerRef" + | "consecutiveMistakeCount" + | "didToolFailInCurrentTurn" + | "sayAndCreateMissingParamError" + | "say" + | "ask" + > = { + cwd: "/task", + providerRef: { deref, [Symbol.toStringTag]: "WeakRef" }, + consecutiveMistakeCount: 3, + didToolFailInCurrentTurn: false, + sayAndCreateMissingParamError: vi + .fn() + .mockResolvedValue("missing query"), + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), + } + task = taskStub as Task + callbacks = { + askApproval: vi.fn().mockResolvedValue(true), + handleError: vi.fn().mockResolvedValue(undefined), + pushToolResult: vi.fn(), + } + manager = { + isFeatureEnabled: true, + isFeatureConfigured: true, + searchIndex: vi.fn().mockResolvedValue([]), + } + vi.mocked(CodeIndexManagerRegistry.getInstance).mockReturnValue(manager as CodeIndexManager) + vi.mocked(getWorkspacePath).mockReturnValue("/fallback") + vi.mocked(vscode.workspace.asRelativePath).mockReturnValue("src/result.ts") + }) + + afterEach(() => vi.restoreAllMocks()) + + function expectNoSearch() { + expect(manager.searchIndex).not.toHaveBeenCalled() + expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled() + expect(task.say).not.toHaveBeenCalled() + } + + function expectNoProviderAccess() { + expect(deref).not.toHaveBeenCalled() + expect(CodeIndexManagerRegistry.getInstance).not.toHaveBeenCalled() + expectNoSearch() + } + + function result(overrides: Partial = {}): VectorStoreSearchResult { + return { + id: "first", + score: 0.9, + payload: { filePath: "/task/src/result.ts", startLine: 2, endLine: 4, codeChunk: " \n first\n second \t" }, + ...overrides, + } + } + + it("exports a named tool instance", () => { + expect(codebaseSearchTool).toBeInstanceOf(CodebaseSearchTool) + expect(codebaseSearchTool.name).toBe("codebase_search") + }) + + it("reports missing workspace before even validating the query", async () => { + Object.defineProperty(task, "cwd", { value: "" }) + vi.mocked(getWorkspacePath).mockReturnValue("") + await tool.execute({ query: "" }, task, callbacks) + expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith( + "codebase_search", + new Error("Could not determine workspace path."), + ) + expect(callbacks.askApproval).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + expect(task.sayAndCreateMissingParamError).not.toHaveBeenCalled() + expect(task.consecutiveMistakeCount).toBe(3) + expect(task.didToolFailInCurrentTurn).toBe(false) + expectNoProviderAccess() + }) + + it("counts a missing query as a failed tool and forwards the missing-parameter response", async () => { + await tool.execute({ query: "" }, task, callbacks) + expect(task.consecutiveMistakeCount).toBe(4) + expect(task.didToolFailInCurrentTurn).toBe(true) + expect(task.sayAndCreateMissingParamError).toHaveBeenCalledExactlyOnceWith("codebase_search", "query") + expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith("missing query") + expect(callbacks.askApproval).not.toHaveBeenCalled() + expect(callbacks.handleError).not.toHaveBeenCalled() + expectNoProviderAccess() + }) + + it.each([undefined, "src", ""])("does not search after denied approval with path %j", async (path) => { + vi.mocked(callbacks.askApproval).mockResolvedValue(false) + await tool.execute({ query, path }, task, callbacks) + expect(callbacks.askApproval).toHaveBeenCalledExactlyOnceWith( + "tool", + JSON.stringify({ tool: "codebaseSearch", query, path, isOutsideWorkspace: false }), + ) + expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith(formatResponse.toolDenied()) + expect(task.consecutiveMistakeCount).toBe(3) + expect(task.didToolFailInCurrentTurn).toBe(false) + expect(callbacks.handleError).not.toHaveBeenCalled() + expectNoProviderAccess() + }) + + it.each(["provider", "context"])("reports a missing %s after approval", async (missing) => { + deref.mockReturnValue(missing === "provider" ? undefined : ({} as ClineProvider)) + await tool.execute({ query }, task, callbacks) + expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith( + "codebase_search", + new Error("Extension context is not available."), + ) + expect(task.consecutiveMistakeCount).toBe(0) + expect(CodeIndexManagerRegistry.getInstance).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + expectNoSearch() + }) + + it.each([ + ["missing", "CodeIndexManager is not available."], + ["disabled", "Code Indexing is disabled in the settings."], + ["unconfigured", "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL)."], + ])("reports a %s manager without searching", async (state, message) => { + if (state === "missing") vi.mocked(CodeIndexManagerRegistry.getInstance).mockReturnValue(undefined) + if (state === "disabled") Object.defineProperty(manager, "isFeatureEnabled", { value: false }) + if (state === "unconfigured") Object.defineProperty(manager, "isFeatureConfigured", { value: false }) + await tool.execute({ query }, task, callbacks) + expect(CodeIndexManagerRegistry.getInstance).toHaveBeenCalledExactlyOnceWith(context, "/task") + expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith("codebase_search", new Error(message)) + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + expect(task.consecutiveMistakeCount).toBe(0) + expectNoSearch() + }) + + it.each([undefined, "src", ""])( + "forwards directory prefix %j and resets mistakes before searching", + async (path) => { + vi.mocked(manager.searchIndex).mockImplementation(async () => { + expect(task.consecutiveMistakeCount).toBe(0) + return [] + }) + await tool.execute({ query, path }, task, callbacks) + expect(manager.searchIndex).toHaveBeenCalledExactlyOnceWith(query, path) + expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith( + `No relevant code snippets found for the query: "${query}"`, + ) + expect(callbacks.handleError).not.toHaveBeenCalled() + expect(task.say).not.toHaveBeenCalled() + expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled() + expect(getWorkspacePath).not.toHaveBeenCalled() + }, + ) + + it.each([null, undefined, false, 0, ""])( + "defensively handles a runtime-invalid falsy search response %j", + async (value) => { + // The manager promises an array. Deliberately violate that boundary to exercise the existing falsy guard. + vi.mocked(manager.searchIndex).mockResolvedValue(value as unknown as VectorStoreSearchResult[]) + await tool.execute({ query }, task, callbacks) + expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith( + `No relevant code snippets found for the query: "${query}"`, + ) + expect(callbacks.handleError).not.toHaveBeenCalled() + expect(task.say).not.toHaveBeenCalled() + expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled() + }, + ) + + it("preserves result order and metadata, relativizes paths without workspace prefixes and trims chunks", async () => { + vi.mocked(manager.searchIndex).mockResolvedValue([ + result(), + result({ + id: "second", + score: 0.5, + payload: { filePath: "/task/lib/other.ts", startLine: 10, endLine: 10, codeChunk: " \t " }, + }), + ]) + vi.mocked(vscode.workspace.asRelativePath) + .mockReturnValueOnce("src/result.ts") + .mockReturnValueOnce("lib/other.ts") + await tool.execute({ query }, task, callbacks) + expect(vscode.workspace.asRelativePath).toHaveBeenCalledTimes(2) + expect(vscode.workspace.asRelativePath).toHaveBeenNthCalledWith(1, "/task/src/result.ts", false) + expect(vscode.workspace.asRelativePath).toHaveBeenNthCalledWith(2, "/task/lib/other.ts", false) + expect(task.say).toHaveBeenCalledExactlyOnceWith( + "codebase_search_result", + JSON.stringify({ + tool: "codebaseSearch", + content: { + query, + results: [ + { + filePath: "src/result.ts", + score: 0.9, + startLine: 2, + endLine: 4, + codeChunk: "first\n second", + }, + { filePath: "lib/other.ts", score: 0.5, startLine: 10, endLine: 10, codeChunk: "" }, + ], + }, + }), + ) + expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith( + `Query: ${query}\nResults:\n\nFile path: src/result.ts\nScore: 0.9\nLines: 2-4\nCode Chunk: first\n second\n\nFile path: lib/other.ts\nScore: 0.5\nLines: 10-10\nCode Chunk: \n`, + ) + expect(task.say).toHaveBeenCalledBefore(vi.mocked(callbacks.pushToolResult)) + expect(callbacks.handleError).not.toHaveBeenCalled() + }) + + it.each([false, true])("skips absent payloads/file paths (include a valid result: %s)", async (includeValid) => { + // Missing filePath is invalid under Payload's type but explicitly guarded against at runtime. + const missingPath = { id: "malformed", score: 1, payload: { codeChunk: "ignored" } } as VectorStoreSearchResult + vi.mocked(manager.searchIndex).mockResolvedValue([ + result({ payload: undefined }), + result({ payload: null }), + missingPath, + ...(includeValid ? [result()] : []), + ]) + await tool.execute({ query }, task, callbacks) + expect(vscode.workspace.asRelativePath).toHaveBeenCalledTimes(includeValid ? 1 : 0) + expect(task.say).toHaveBeenCalledExactlyOnceWith( + "codebase_search_result", + JSON.stringify({ + tool: "codebaseSearch", + content: { + query, + results: includeValid + ? [ + { + filePath: "src/result.ts", + score: 0.9, + startLine: 2, + endLine: 4, + codeChunk: "first\n second", + }, + ] + : [], + }, + }), + ) + // A nonempty response whose entries are all skipped still emits an empty result header, not "No relevant...". + expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith( + `Query: ${query}\nResults:\n\n${includeValid ? "File path: src/result.ts\nScore: 0.9\nLines: 2-4\nCode Chunk: first\n second\n" : ""}`, + ) + expect(callbacks.handleError).not.toHaveBeenCalled() + }) + + it.each(["registry", "search", "say"])("forwards the original %s error without a tool result", async (source) => { + const error = new Error(`${source} failed`) + if (source === "registry") + vi.mocked(CodeIndexManagerRegistry.getInstance).mockImplementation(() => { + throw error + }) + if (source === "search") vi.mocked(manager.searchIndex).mockRejectedValue(error) + if (source === "say") { + vi.mocked(manager.searchIndex).mockResolvedValue([result()]) + vi.mocked(task.say).mockRejectedValue(error) + } + await tool.execute({ query }, task, callbacks) + expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith("codebase_search", error) + expect(vi.mocked(callbacks.handleError).mock.calls[0][1]).toBe(error) + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + if (source === "registry") expectNoSearch() + if (source === "search") expect(task.say).not.toHaveBeenCalled() + }) + + describe("handlePartial", () => { + it.each([ + { params: {}, partial: true }, + { params: { query }, partial: true }, + { params: { path: "src" }, partial: false }, + { params: { query, path: "src" }, partial: true }, + { params: { query: "", path: "" }, partial: false }, + ])("sends the supplied optional fields and partial flag: %j", async ({ params, partial }) => { + const block: ToolUse<"codebase_search"> = { type: "tool_use", name: "codebase_search", params, partial } + await tool.handlePartial(task, block) + expect(task.ask).toHaveBeenCalledExactlyOnceWith( + "tool", + JSON.stringify({ + tool: "codebaseSearch", + ...params, + isOutsideWorkspace: false, + }), + partial, + ) + expectNoProviderAccess() + expect(callbacks.askApproval).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + expect(task.consecutiveMistakeCount).toBe(3) + }) + + it("swallows a rejected partial ask without searching or reporting a tool error", async () => { + vi.mocked(task.ask).mockRejectedValue(new Error("superseded partial message")) + await expect( + tool.handlePartial(task, { + type: "tool_use", + name: "codebase_search", + params: { query }, + partial: true, + }), + ).resolves.toBeUndefined() + expect(task.ask).toHaveBeenCalledOnce() + expect(callbacks.handleError).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + expectNoProviderAccess() + }) + }) +}) diff --git a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts new file mode 100644 index 0000000000..1d656a73a7 --- /dev/null +++ b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts @@ -0,0 +1,133 @@ +import * as vscode from "vscode" +import type { Task } from "../../task/Task" +import type { ClineProvider } from "../../webview/ClineProvider" +import type { ToolCallbacks } from "../BaseTool" +import { CodebaseSearchTool } from "../CodebaseSearchTool" +import { CodeIndexManagerRegistry } from "../../../services/code-index/code-index-manager-registry" +import { getWorkspacePath } from "../../../utils/path" +import { makeExtensionContext, makeTextDocument, makeTextEditor, makeUri } from "../../../test-utils/vscode" + +vi.mock("vscode", () => ({ + workspace: { workspaceFolders: undefined, getWorkspaceFolder: vi.fn(), asRelativePath: vi.fn() }, + window: { activeTextEditor: undefined }, + Uri: { file: vi.fn() }, +})) +vi.mock("../../../utils/path", () => ({ getWorkspacePath: vi.fn() })) +vi.mock("../../../services/code-index/manager", () => ({ + CodeIndexManager: vi.fn().mockImplementation(function (workspacePath: string) { + return { + isFeatureEnabled: true, + isFeatureConfigured: true, + searchIndex: vi.fn().mockResolvedValue([ + { + score: 0.9, + payload: { + filePath: `${workspacePath}/src/result.ts`, + startLine: 2, + endLine: 4, + codeChunk: " match ", + }, + }, + ]), + dispose: vi.fn(), + } + }), +})) + +describe("CodebaseSearchTool workspace selection", () => { + const first = { uri: makeUri("/first"), name: "first", index: 0 } + const second = { uri: makeUri("/second"), name: "second", index: 1 } + let task: Task + let callbacks: ToolCallbacks + + beforeEach(() => { + vi.clearAllMocks() + Object.defineProperty(vscode.workspace, "workspaceFolders", { configurable: true, value: [first, second] }) + Object.defineProperty(vscode.window, "activeTextEditor", { + configurable: true, + value: makeTextEditor({ document: makeTextDocument({ uri: makeUri("/first/editor.ts") }) }), + }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(first) + vi.mocked(vscode.workspace.asRelativePath).mockImplementation((value) => + typeof value === "string" ? value : value.fsPath, + ) + vi.mocked(vscode.Uri.file).mockImplementation((value) => makeUri(value)) + vi.mocked(getWorkspacePath).mockReturnValue(first.uri.fsPath) + // Only the provider context and task members consumed by this tool are needed. + const provider = { context: makeExtensionContext() } as ClineProvider + const taskStub: Pick = { + cwd: second.uri.fsPath, + providerRef: new WeakRef(provider), + consecutiveMistakeCount: 0, + say: vi.fn().mockResolvedValue(undefined), + } + task = taskStub as Task + callbacks = { + askApproval: vi.fn().mockResolvedValue(true), + handleError: vi.fn().mockResolvedValue(undefined), + pushToolResult: vi.fn(), + } + }) + + afterEach(() => { + CodeIndexManagerRegistry.disposeAll() + vi.restoreAllMocks() + }) + + it("searches the task workspace despite an active editor in another root", async () => { + const context = task.providerRef.deref()!.context + const editorManager = CodeIndexManagerRegistry.getInstance(context, first.uri.fsPath)! + const taskManager = CodeIndexManagerRegistry.getInstance(context, second.uri.fsPath)! + + await new CodebaseSearchTool().execute({ query: "find match", path: "src" }, task, callbacks) + + expect(callbacks.handleError).not.toHaveBeenCalled() + expect(taskManager.searchIndex).toHaveBeenCalledWith("find match", "src") + expect(editorManager.searchIndex).not.toHaveBeenCalled() + expect(getWorkspacePath).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).toHaveBeenCalledWith( + expect.stringContaining("File path: /second/src/result.ts"), + ) + expect(task.say).toHaveBeenCalledWith( + "codebase_search_result", + JSON.stringify({ + tool: "codebaseSearch", + content: { + query: "find match", + results: [ + { filePath: "/second/src/result.ts", score: 0.9, startLine: 2, endLine: 4, codeChunk: "match" }, + ], + }, + }), + ) + }) + + it.each(["", " "])("uses the resolved fallback workspace when task cwd is %j", async (cwd) => { + Object.defineProperty(task, "cwd", { value: cwd }) + vi.mocked(getWorkspacePath).mockReturnValue(second.uri.fsPath) + + await new CodebaseSearchTool().execute({ query: "fallback" }, task, callbacks) + + expect(getWorkspacePath).toHaveBeenCalled() + expect(callbacks.handleError).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).toHaveBeenCalledWith( + expect.stringContaining("File path: /second/src/result.ts"), + ) + }) + + it("reports a missing workspace before requesting approval or creating a manager", async () => { + Object.defineProperty(task, "cwd", { value: "" }) + vi.mocked(getWorkspacePath).mockReturnValue("") + Object.defineProperty(vscode.workspace, "workspaceFolders", { value: undefined }) + Object.defineProperty(vscode.window, "activeTextEditor", { value: undefined }) + + await new CodebaseSearchTool().execute({ query: "match" }, task, callbacks) + + expect(callbacks.handleError).toHaveBeenCalledWith( + "codebase_search", + new Error("Could not determine workspace path."), + ) + expect(callbacks.askApproval).not.toHaveBeenCalled() + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) + }) +})