From 616cafc3a1e1a85d19970c997eb9bceb9ef7aede Mon Sep 17 00:00:00 2001 From: MuriloFP Date: Wed, 23 Jul 2025 11:10:48 -0300 Subject: [PATCH] fix: handle codebase search indexing state properly (#5662) - Always show codebase_search tool in the tool list - Add runtime state checking with user-friendly feedback - Provide clear messages for each indexing state (Standby, Indexing, Error) - Suggest alternative tools when semantic search is unavailable - Add comprehensive tests for the new behavior This ensures Roo doesn't try to use semantic search when indexing is incomplete and provides clear feedback to users about the current state. --- src/core/prompts/tools/index.ts | 9 +- .../__tests__/codebaseSearchTool.spec.ts | 344 ++++++++++++++++++ src/core/tools/codebaseSearchTool.ts | 27 ++ src/services/code-index/search-service.ts | 7 +- 4 files changed, 377 insertions(+), 10 deletions(-) create mode 100644 src/core/tools/__tests__/codebaseSearchTool.spec.ts diff --git a/src/core/prompts/tools/index.ts b/src/core/prompts/tools/index.ts index 9f4af7f312..be3bb3ccd1 100644 --- a/src/core/prompts/tools/index.ts +++ b/src/core/prompts/tools/index.ts @@ -101,13 +101,8 @@ export function getToolDescriptionsForMode( // Add always available tools ALWAYS_AVAILABLE_TOOLS.forEach((tool) => tools.add(tool)) - // Conditionally exclude codebase_search if feature is disabled or not configured - if ( - !codeIndexManager || - !(codeIndexManager.isFeatureEnabled && codeIndexManager.isFeatureConfigured && codeIndexManager.isInitialized) - ) { - tools.delete("codebase_search") - } + // Note: codebase_search is now always included in the tool list + // The tool itself will check the indexing state at runtime and provide appropriate feedback // Conditionally exclude update_todo_list if disabled in settings if (settings?.todoListEnabled === false) { diff --git a/src/core/tools/__tests__/codebaseSearchTool.spec.ts b/src/core/tools/__tests__/codebaseSearchTool.spec.ts new file mode 100644 index 0000000000..b3b5d0d8d9 --- /dev/null +++ b/src/core/tools/__tests__/codebaseSearchTool.spec.ts @@ -0,0 +1,344 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { codebaseSearchTool } from "../codebaseSearchTool" +import { CodeIndexManager } from "../../../services/code-index/manager" +import { Task } from "../../task/Task" +import { ToolUse } from "../../../shared/tools" + +// Mock dependencies +vi.mock("../../../services/code-index/manager") +vi.mock("../../../utils/path", () => ({ + getWorkspacePath: vi.fn(() => "/test/workspace"), +})) +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + toolDenied: vi.fn(() => "Tool denied"), + }, +})) +vi.mock("vscode", () => ({ + workspace: { + asRelativePath: vi.fn((path: string) => path.replace("/test/workspace/", "")), + }, +})) + +describe("codebaseSearchTool", () => { + let mockTask: Task + let mockAskApproval: any + let mockHandleError: any + let mockPushToolResult: any + let mockRemoveClosingTag: any + let mockCodeIndexManager: any + + beforeEach(() => { + vi.clearAllMocks() + + // Setup mock task + mockTask = { + ask: vi.fn().mockResolvedValue(undefined), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"), + consecutiveMistakeCount: 0, + providerRef: { + deref: vi.fn(() => ({ + context: {}, + })), + }, + say: vi.fn().mockResolvedValue(undefined), + } as any + + // Setup mock functions + mockAskApproval = vi.fn().mockResolvedValue(true) + mockHandleError = vi.fn() + mockPushToolResult = vi.fn() + mockRemoveClosingTag = vi.fn((tag, value) => value) + + // Setup mock CodeIndexManager + mockCodeIndexManager = { + isFeatureEnabled: true, + isFeatureConfigured: true, + isInitialized: true, + state: "Indexed", + searchIndex: vi.fn().mockResolvedValue([ + { + score: 0.9, + payload: { + filePath: "/test/workspace/src/file.ts", + startLine: 10, + endLine: 20, + codeChunk: "test code", + }, + }, + ]), + } + + vi.mocked(CodeIndexManager).getInstance = vi.fn((_context: any) => mockCodeIndexManager as any) + }) + + describe("indexing state checks", () => { + it("should provide feedback when indexing is in Standby state", async () => { + mockCodeIndexManager.state = "Standby" + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Semantic search is not available yet (currently Standby)"), + ) + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Code indexing has not started yet"), + ) + expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Please use file reading tools")) + expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled() + }) + + it("should provide feedback when indexing is in progress", async () => { + mockCodeIndexManager.state = "Indexing" + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Semantic search is not available yet (currently Indexing)"), + ) + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Code indexing is currently in progress"), + ) + expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled() + }) + + it("should provide feedback when indexing is in Error state", async () => { + mockCodeIndexManager.state = "Error" + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Semantic search is not available yet (currently Error)"), + ) + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Code indexing encountered an error"), + ) + expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled() + }) + + it("should perform search when indexing is complete (Indexed state)", async () => { + mockCodeIndexManager.state = "Indexed" + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCodeIndexManager.searchIndex).toHaveBeenCalledWith("test query", undefined) + // Check that say was called with the search results + expect(mockTask.say).toHaveBeenCalledWith("codebase_search_result", expect.stringContaining("test code")) + // Check that pushToolResult was called with the formatted output + expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Query: test query")) + expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("test code")) + expect(mockPushToolResult).not.toHaveBeenCalledWith( + expect.stringContaining("Semantic search is not available"), + ) + }) + }) + + describe("feature configuration checks", () => { + it("should throw error when feature is disabled", async () => { + mockCodeIndexManager.isFeatureEnabled = false + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockHandleError).toHaveBeenCalledWith( + "codebase_search", + expect.objectContaining({ + message: "Code Indexing is disabled in the settings.", + }), + ) + }) + + it("should throw error when feature is not configured", async () => { + mockCodeIndexManager.isFeatureConfigured = false + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockHandleError).toHaveBeenCalledWith( + "codebase_search", + expect.objectContaining({ + message: "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).", + }), + ) + }) + }) + + describe("parameter validation", () => { + it("should handle missing query parameter", async () => { + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: {}, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockTask.sayAndCreateMissingParamError).toHaveBeenCalledWith("codebase_search", "query") + expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error") + }) + + it("should handle partial tool use", async () => { + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test" }, + partial: true, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockTask.ask).toHaveBeenCalled() + expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled() + }) + }) + + describe("search results handling", () => { + it("should handle empty search results", async () => { + mockCodeIndexManager.searchIndex.mockResolvedValue([]) + + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockPushToolResult).toHaveBeenCalledWith( + 'No relevant code snippets found for the query: "test query"', + ) + }) + + it("should format search results correctly", async () => { + const block: ToolUse = { + type: "tool_use", + name: "codebase_search", + params: { query: "test query" }, + partial: false, + } + + await codebaseSearchTool( + mockTask, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + // The tool should call pushToolResult with a single formatted string containing all results + expect(mockPushToolResult).toHaveBeenCalledTimes(1) + const resultString = mockPushToolResult.mock.calls[0][0] + expect(resultString).toContain("Query: test query") + expect(resultString).toContain("File path: src/file.ts") + expect(resultString).toContain("Score: 0.9") + expect(resultString).toContain("Lines: 10-20") + expect(resultString).toContain("Code Chunk: test code") + }) + }) +}) diff --git a/src/core/tools/codebaseSearchTool.ts b/src/core/tools/codebaseSearchTool.ts index 236b066306..a5e2baaaea 100644 --- a/src/core/tools/codebaseSearchTool.ts +++ b/src/core/tools/codebaseSearchTool.ts @@ -82,6 +82,33 @@ export async function codebaseSearchTool( throw new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).") } + // Check indexing state at runtime + const indexingState = manager.state + if (indexingState !== "Indexed") { + let stateMessage = "" + switch (indexingState) { + case "Standby": + stateMessage = + "Code indexing has not started yet. Please wait for the initial indexing to complete." + break + case "Indexing": + stateMessage = + "Code indexing is currently in progress. Semantic search will be available once indexing is complete." + break + case "Error": + stateMessage = "Code indexing encountered an error. Please check your configuration and try again." + break + default: + stateMessage = `Code indexing is in an unexpected state: ${indexingState}` + } + + // Return informative message instead of throwing error + pushToolResult( + `Semantic search is not available yet (currently ${indexingState}).\n\n${stateMessage}\n\nPlease use file reading tools (read_file, search_files) for now.`, + ) + return + } + const searchResults: VectorStoreSearchResult[] = await manager.searchIndex(query, directoryPrefix) // 3. Format and push results diff --git a/src/services/code-index/search-service.ts b/src/services/code-index/search-service.ts index a56f5cc674..a8361587db 100644 --- a/src/services/code-index/search-service.ts +++ b/src/services/code-index/search-service.ts @@ -34,10 +34,11 @@ export class CodeIndexSearchService { const minScore = this.configManager.currentSearchMinScore const maxResults = this.configManager.currentSearchMaxResults + // Note: State checking is now handled in the codebaseSearchTool + // This allows the tool to provide more user-friendly feedback const currentState = this.stateManager.getCurrentStatus().systemStatus - if (currentState !== "Indexed" && currentState !== "Indexing") { - // Allow search during Indexing too - throw new Error(`Code index is not ready for search. Current state: ${currentState}`) + if (currentState === "Error") { + throw new Error(`Code index is in error state. Please check your configuration.`) } try {