diff --git a/src/core/tools/CodebaseSearchTool.ts b/src/core/tools/CodebaseSearchTool.ts index 004c61f9f8..6084dd6b74 100644 --- a/src/core/tools/CodebaseSearchTool.ts +++ b/src/core/tools/CodebaseSearchTool.ts @@ -63,12 +63,21 @@ export class CodebaseSearchTool extends BaseTool<"codebase_search"> { throw new Error("CodeIndexManager is not available.") } + // Settings defaults are not evidence that a fresh manager is explicitly disabled. + // Initialization belongs to the manager's owner, not the search tool. + if (!manager.isConfigurationLoaded) { + throw new Error("Code Indexing configuration has not been loaded for this workspace.") + } + if (!manager.isFeatureEnabled) { throw new Error("Code Indexing is disabled in the settings.") } if (!manager.isFeatureConfigured) { throw new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).") } + if (!manager.isInitialized) { + throw new Error("Code Indexing is not initialized for this workspace.") + } const searchResults: VectorStoreSearchResult[] = await manager.searchIndex(query, directoryPrefix) diff --git a/src/core/tools/__tests__/CodebaseSearchTool.spec.ts b/src/core/tools/__tests__/CodebaseSearchTool.spec.ts index 4b0a6ed01d..bc88d44ec1 100644 --- a/src/core/tools/__tests__/CodebaseSearchTool.spec.ts +++ b/src/core/tools/__tests__/CodebaseSearchTool.spec.ts @@ -25,7 +25,15 @@ describe("CodebaseSearchTool", () => { let task: Task let context: vscode.ExtensionContext let callbacks: ToolCallbacks - let manager: Pick + let manager: Pick< + CodeIndexManager, + | "isFeatureEnabled" + | "isFeatureConfigured" + | "isConfigurationLoaded" + | "isInitialized" + | "initialize" + | "searchIndex" + > let deref: ReturnType> beforeEach(() => { @@ -61,8 +69,11 @@ describe("CodebaseSearchTool", () => { pushToolResult: vi.fn(), } manager = { + isConfigurationLoaded: true, isFeatureEnabled: true, isFeatureConfigured: true, + isInitialized: true, + initialize: vi.fn().mockResolvedValue({ requiresRestart: false }), searchIndex: vi.fn().mockResolvedValue([]), } vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockReturnValue(manager as CodeIndexManager) @@ -70,7 +81,10 @@ describe("CodebaseSearchTool", () => { vi.mocked(vscode.workspace.asRelativePath).mockReturnValue("src/result.ts") }) - afterEach(() => vi.restoreAllMocks()) + afterEach(() => { + expect(manager.initialize).not.toHaveBeenCalled() + vi.restoreAllMocks() + }) function result(overrides: Partial = {}): VectorStoreSearchResult { return { @@ -174,7 +188,27 @@ describe("CodebaseSearchTool", () => { expect(task.say).not.toHaveBeenCalled() }) - it("reports disabled indexing without searching", async () => { + it("reports configuration that has never loaded before checking settings or services", async () => { + Object.defineProperties(manager, { + isConfigurationLoaded: { value: false }, + isFeatureEnabled: { value: false }, + isFeatureConfigured: { value: false }, + isInitialized: { value: false }, + }) + + await tool.execute({ query }, task, callbacks) + + expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith( + toolNamesSchema.enum.codebase_search, + new Error("Code Indexing configuration has not been loaded for this workspace."), + ) + expect(manager.initialize).not.toHaveBeenCalled() + expect(manager.searchIndex).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + }) + + it.each([true, false])("reports disabled indexing without searching (initialized: %s)", async (initialized) => { + Object.defineProperty(manager, "isInitialized", { value: initialized }) Object.defineProperty(manager, "isFeatureEnabled", { value: false }) await tool.execute({ query }, task, callbacks) @@ -190,20 +224,38 @@ describe("CodebaseSearchTool", () => { expect(task.say).not.toHaveBeenCalled() }) - it("reports missing index configuration without searching", async () => { - Object.defineProperty(manager, "isFeatureConfigured", { value: false }) + it.each([true, false])( + "reports missing index configuration without searching (initialized: %s)", + async (initialized) => { + Object.defineProperty(manager, "isInitialized", { value: initialized }) + Object.defineProperty(manager, "isFeatureConfigured", { value: false }) + + await tool.execute({ query }, task, callbacks) + + expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith( + toolNamesSchema.enum.codebase_search, + new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL)."), + ) + expect(callbacks.pushToolResult).not.toHaveBeenCalled() + expect(task.consecutiveMistakeCount).toBe(0) + expect(manager.searchIndex).not.toHaveBeenCalled() + expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled() + expect(task.say).not.toHaveBeenCalled() + }, + ) + + it("reports configured but unready services without initializing or searching", async () => { + Object.defineProperty(manager, "isInitialized", { value: false }) await tool.execute({ query }, task, callbacks) + expect(manager.initialize).not.toHaveBeenCalled() expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith( toolNamesSchema.enum.codebase_search, - new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL)."), + new Error("Code Indexing is not initialized for this workspace."), ) - expect(callbacks.pushToolResult).not.toHaveBeenCalled() - expect(task.consecutiveMistakeCount).toBe(0) expect(manager.searchIndex).not.toHaveBeenCalled() - expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled() - expect(task.say).not.toHaveBeenCalled() + expect(callbacks.pushToolResult).not.toHaveBeenCalled() }) it.each([undefined, "src", ""])( @@ -214,6 +266,7 @@ describe("CodebaseSearchTool", () => { return [] }) await tool.execute({ query, path }, task, callbacks) + expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenCalledExactlyOnceWith(context, "/task") expect(manager.searchIndex).toHaveBeenCalledExactlyOnceWith(query, path) expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith( `No relevant code snippets found for the query: "${query}"`, @@ -253,6 +306,8 @@ describe("CodebaseSearchTool", () => { .mockReturnValueOnce("src/result.ts") .mockReturnValueOnce("lib/other.ts") await tool.execute({ query }, task, callbacks) + expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenCalledExactlyOnceWith(context, "/task") + expect(manager.searchIndex).toHaveBeenCalledExactlyOnceWith(query, undefined) 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) diff --git a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts index 5cfabb4c43..03789f7d40 100644 --- a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts +++ b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts @@ -19,6 +19,9 @@ vi.mock("../../../services/code-index/manager", () => ({ CodeIndexManager: vi.fn().mockImplementation(function (workspacePath: string) { let initialized = false return { + get isConfigurationLoaded() { + return initialized + }, get isInitialized() { return initialized }, @@ -184,7 +187,7 @@ describe("CodebaseSearchTool workspace selection", () => { expect(callbacks.pushToolResult).toHaveBeenCalledTimes(3) }) - it("creates a manager for an external task path without initializing it", async () => { + it("reports a fresh external task manager as uninitialized without initializing or searching", async () => { Object.defineProperty(task, "cwd", { value: "/external-task" }) await new CodebaseSearchTool().execute({ query: "external match" }, task, callbacks) @@ -201,9 +204,24 @@ describe("CodebaseSearchTool workspace selection", () => { expect(getWorkspacePath).not.toHaveBeenCalled() expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith( toolNamesSchema.enum.codebase_search, - new Error("Code Indexing is disabled in the settings."), + new Error("Code Indexing configuration has not been loaded for this workspace."), ) expect(callbacks.pushToolResult).not.toHaveBeenCalled() expect(task.say).not.toHaveBeenCalled() }) + + it("searches an initialized external manager while background indexing is ongoing", async () => { + Object.defineProperty(task, "cwd", { value: "/external-task" }) + const manager = CodeIndexManagerRegistry.getOrCreate(provider.context, task.cwd)! + await manager.initialize(provider.contextProxy) + vi.mocked(manager.initialize).mockClear() + Object.defineProperty(manager, "state", { get: () => "Indexing" }) + + await new CodebaseSearchTool().execute({ query: "external match" }, task, callbacks) + + expect(manager.state).toBe("Indexing") + expect(manager.initialize).not.toHaveBeenCalled() + expect(manager.searchIndex).toHaveBeenCalledExactlyOnceWith("external match", undefined) + expect(callbacks.handleError).not.toHaveBeenCalled() + }) }) diff --git a/src/services/code-index/__tests__/config-manager.spec.ts b/src/services/code-index/__tests__/config-manager.spec.ts index 8b41dfe1d6..71b6e79e61 100644 --- a/src/services/code-index/__tests__/config-manager.spec.ts +++ b/src/services/code-index/__tests__/config-manager.spec.ts @@ -115,6 +115,67 @@ describe("CodeIndexConfigManager", () => { }) describe("loadConfiguration", () => { + it("does not mark the synchronous constructor snapshot as loaded", () => { + expect(configManager.isConfigurationLoaded).toBe(false) + }) + + it("marks disabled configuration loaded only after secrets refresh completes", async () => { + let finishRefresh!: () => void + mockContextProxy.refreshSecrets.mockReturnValue( + new Promise((resolve) => { + finishRefresh = resolve + }), + ) + + const loading = configManager.loadConfiguration() + expect(configManager.isConfigurationLoaded).toBe(false) + finishRefresh() + await loading + expect(configManager.isConfigurationLoaded).toBe(true) + expect(configManager.isFeatureEnabled).toBe(false) + }) + + it("marks enabled but unconfigured settings as loaded", async () => { + mockContextProxy.getGlobalState.mockReturnValue({ codebaseIndexEnabled: true }) + await configManager.loadConfiguration() + expect(configManager.isConfigurationLoaded).toBe(true) + expect(configManager.isFeatureEnabled).toBe(true) + expect(configManager.isFeatureConfigured).toBe(false) + }) + + it.each(["secrets", "configuration", "restart", "payload"] as const)( + "retains the last successful load state when %s fails", + async (stage) => { + const error = new Error(`${stage} failed`) + const failNextLoad = () => { + if (stage === "secrets") { + mockContextProxy.refreshSecrets.mockRejectedValueOnce(error) + } else if (stage === "configuration") { + mockContextProxy.getGlobalState.mockImplementationOnce(() => { + throw error + }) + } else if (stage === "restart") { + vi.spyOn(configManager, "doesConfigChangeRequireRestart").mockImplementationOnce(() => { + throw error + }) + } else { + vi.spyOn(configManager, "currentSearchMinScore", "get").mockImplementationOnce(() => { + throw error + }) + } + } + + failNextLoad() + await expect(configManager.loadConfiguration()).rejects.toThrow(error) + expect(configManager.isConfigurationLoaded).toBe(false) + await configManager.loadConfiguration() + expect(configManager.isConfigurationLoaded).toBe(true) + failNextLoad() + await expect(configManager.loadConfiguration()).rejects.toThrow(error) + expect(configManager.isConfigurationLoaded).toBe(true) + }, + ) + it("should load default configuration when no state exists", async () => { mockContextProxy.getGlobalState.mockReturnValue(undefined) mockContextProxy.getSecret.mockReturnValue(undefined) diff --git a/src/services/code-index/__tests__/manager.spec.ts b/src/services/code-index/__tests__/manager.spec.ts index 9faf06627e..952e205fdd 100644 --- a/src/services/code-index/__tests__/manager.spec.ts +++ b/src/services/code-index/__tests__/manager.spec.ts @@ -1,3 +1,5 @@ +import { ContextProxy } from "../../../core/config/ContextProxy" +import { makeExtensionContext } from "../../../test-utils/vscode" import { CodeIndexManager } from "../manager" import { CodeIndexManagerRegistry } from "../code-index-manager-registry" import { CodeIndexServiceFactory } from "../service-factory" @@ -168,6 +170,66 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { CodeIndexManagerRegistry.disposeAll() }) + describe("configuration readiness", () => { + it("does not regain readiness when a detached configuration finishes loading", async () => { + let finishRefresh!: () => void + const contextProxy = new ContextProxy(makeExtensionContext()) + vi.spyOn(contextProxy, "getGlobalState").mockReturnValue({ codebaseIndexEnabled: false }) + vi.spyOn(contextProxy, "getSecret").mockReturnValue(undefined) + vi.spyOn(contextProxy, "refreshSecrets").mockReturnValue( + new Promise((resolve) => { + finishRefresh = resolve + }), + ) + + const initialization = manager.initialize(contextProxy) + expect(manager.isConfigurationLoaded).toBe(false) + await manager.recoverFromError() + finishRefresh() + await initialization + expect(manager.isConfigurationLoaded).toBe(false) + }) + + it("keeps failed configuration unloaded until a settings refresh succeeds", async () => { + const error = new Error("secrets refresh failed") + const contextProxy = new ContextProxy(makeExtensionContext()) + vi.spyOn(contextProxy, "getGlobalState").mockReturnValue({ codebaseIndexEnabled: false }) + vi.spyOn(contextProxy, "getSecret").mockReturnValue(undefined) + vi.spyOn(contextProxy, "refreshSecrets").mockRejectedValueOnce(error).mockResolvedValue(undefined) + + await expect(manager.initialize(contextProxy)).rejects.toThrow(error) + expect(manager.isConfigurationLoaded).toBe(false) + await manager.handleSettingsChange() + expect(manager.isConfigurationLoaded).toBe(true) + expect(manager.isInitialized).toBe(false) + }) + + it("marks disabled configuration loaded only after secrets refresh completes", async () => { + let finishRefresh!: () => void + const refresh = new Promise((resolve) => { + finishRefresh = resolve + }) + const contextProxy = new ContextProxy(makeExtensionContext()) + vi.spyOn(contextProxy, "getGlobalState").mockReturnValue({ codebaseIndexEnabled: false }) + vi.spyOn(contextProxy, "getSecret").mockReturnValue(undefined) + vi.spyOn(contextProxy, "refreshSecrets").mockReturnValue(refresh) + + expect(manager.isConfigurationLoaded).toBe(false) + const initialization = manager.initialize(contextProxy) + expect(contextProxy.refreshSecrets).toHaveBeenCalledOnce() + expect(manager.isConfigurationLoaded).toBe(false) + finishRefresh() + await initialization + + expect(manager.isConfigurationLoaded).toBe(true) + expect(manager.isFeatureEnabled).toBe(false) + expect(manager.isInitialized).toBe(false) + + await manager.recoverFromError() + expect(manager.isConfigurationLoaded).toBe(false) + }) + }) + describe("handleSettingsChange", () => { it("should log background indexing failures", async () => { const indexingError = new Error("indexing startup failed") diff --git a/src/services/code-index/config-manager.ts b/src/services/code-index/config-manager.ts index ce65d0407b..5b2b2c63e2 100644 --- a/src/services/code-index/config-manager.ts +++ b/src/services/code-index/config-manager.ts @@ -11,6 +11,13 @@ import { providerIdentifiers } from "@roo-code/types/provider-identifiers" * Handles loading, validating, and providing access to configuration values. */ export class CodeIndexConfigManager { + private _isConfigurationLoaded = false + + /** Whether at least one full asynchronous configuration load has succeeded. */ + public get isConfigurationLoaded(): boolean { + return this._isConfigurationLoaded + } + private codebaseIndexEnabled: boolean = false private embedderProvider: EmbedderProvider = providerIdentifiers.openai private modelId?: string @@ -207,7 +214,7 @@ export class CodeIndexConfigManager { const requiresRestart = this.doesConfigChangeRequireRestart(previousConfigSnapshot) - return { + const result = { configSnapshot: previousConfigSnapshot, currentConfig: { isConfigured: this.isConfigured(), @@ -228,6 +235,8 @@ export class CodeIndexConfigManager { }, requiresRestart, } + this._isConfigurationLoaded = true + return result } /** diff --git a/src/services/code-index/manager.ts b/src/services/code-index/manager.ts index fd3e6b0553..ee181b17d6 100644 --- a/src/services/code-index/manager.ts +++ b/src/services/code-index/manager.ts @@ -103,6 +103,11 @@ export class CodeIndexManager { return this._configManager?.isFeatureConfigured ?? false } + /** Whether configuration loading has succeeded, independently of service or indexing readiness. */ + public get isConfigurationLoaded(): boolean { + return this._configManager?.isConfigurationLoaded ?? false + } + public get isInitialized(): boolean { try { this.assertInitialized()