Skip to content

Commit 616cafc

Browse files
committed
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.
1 parent ed3a077 commit 616cafc

File tree

4 files changed

+377
-10
lines changed

4 files changed

+377
-10
lines changed

src/core/prompts/tools/index.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,8 @@ export function getToolDescriptionsForMode(
101101
// Add always available tools
102102
ALWAYS_AVAILABLE_TOOLS.forEach((tool) => tools.add(tool))
103103

104-
// Conditionally exclude codebase_search if feature is disabled or not configured
105-
if (
106-
!codeIndexManager ||
107-
!(codeIndexManager.isFeatureEnabled && codeIndexManager.isFeatureConfigured && codeIndexManager.isInitialized)
108-
) {
109-
tools.delete("codebase_search")
110-
}
104+
// Note: codebase_search is now always included in the tool list
105+
// The tool itself will check the indexing state at runtime and provide appropriate feedback
111106

112107
// Conditionally exclude update_todo_list if disabled in settings
113108
if (settings?.todoListEnabled === false) {
Lines changed: 344 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,344 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest"
2+
import { codebaseSearchTool } from "../codebaseSearchTool"
3+
import { CodeIndexManager } from "../../../services/code-index/manager"
4+
import { Task } from "../../task/Task"
5+
import { ToolUse } from "../../../shared/tools"
6+
7+
// Mock dependencies
8+
vi.mock("../../../services/code-index/manager")
9+
vi.mock("../../../utils/path", () => ({
10+
getWorkspacePath: vi.fn(() => "/test/workspace"),
11+
}))
12+
vi.mock("../../prompts/responses", () => ({
13+
formatResponse: {
14+
toolDenied: vi.fn(() => "Tool denied"),
15+
},
16+
}))
17+
vi.mock("vscode", () => ({
18+
workspace: {
19+
asRelativePath: vi.fn((path: string) => path.replace("/test/workspace/", "")),
20+
},
21+
}))
22+
23+
describe("codebaseSearchTool", () => {
24+
let mockTask: Task
25+
let mockAskApproval: any
26+
let mockHandleError: any
27+
let mockPushToolResult: any
28+
let mockRemoveClosingTag: any
29+
let mockCodeIndexManager: any
30+
31+
beforeEach(() => {
32+
vi.clearAllMocks()
33+
34+
// Setup mock task
35+
mockTask = {
36+
ask: vi.fn().mockResolvedValue(undefined),
37+
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
38+
consecutiveMistakeCount: 0,
39+
providerRef: {
40+
deref: vi.fn(() => ({
41+
context: {},
42+
})),
43+
},
44+
say: vi.fn().mockResolvedValue(undefined),
45+
} as any
46+
47+
// Setup mock functions
48+
mockAskApproval = vi.fn().mockResolvedValue(true)
49+
mockHandleError = vi.fn()
50+
mockPushToolResult = vi.fn()
51+
mockRemoveClosingTag = vi.fn((tag, value) => value)
52+
53+
// Setup mock CodeIndexManager
54+
mockCodeIndexManager = {
55+
isFeatureEnabled: true,
56+
isFeatureConfigured: true,
57+
isInitialized: true,
58+
state: "Indexed",
59+
searchIndex: vi.fn().mockResolvedValue([
60+
{
61+
score: 0.9,
62+
payload: {
63+
filePath: "/test/workspace/src/file.ts",
64+
startLine: 10,
65+
endLine: 20,
66+
codeChunk: "test code",
67+
},
68+
},
69+
]),
70+
}
71+
72+
vi.mocked(CodeIndexManager).getInstance = vi.fn((_context: any) => mockCodeIndexManager as any)
73+
})
74+
75+
describe("indexing state checks", () => {
76+
it("should provide feedback when indexing is in Standby state", async () => {
77+
mockCodeIndexManager.state = "Standby"
78+
79+
const block: ToolUse = {
80+
type: "tool_use",
81+
name: "codebase_search",
82+
params: { query: "test query" },
83+
partial: false,
84+
}
85+
86+
await codebaseSearchTool(
87+
mockTask,
88+
block,
89+
mockAskApproval,
90+
mockHandleError,
91+
mockPushToolResult,
92+
mockRemoveClosingTag,
93+
)
94+
95+
expect(mockPushToolResult).toHaveBeenCalledWith(
96+
expect.stringContaining("Semantic search is not available yet (currently Standby)"),
97+
)
98+
expect(mockPushToolResult).toHaveBeenCalledWith(
99+
expect.stringContaining("Code indexing has not started yet"),
100+
)
101+
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Please use file reading tools"))
102+
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
103+
})
104+
105+
it("should provide feedback when indexing is in progress", async () => {
106+
mockCodeIndexManager.state = "Indexing"
107+
108+
const block: ToolUse = {
109+
type: "tool_use",
110+
name: "codebase_search",
111+
params: { query: "test query" },
112+
partial: false,
113+
}
114+
115+
await codebaseSearchTool(
116+
mockTask,
117+
block,
118+
mockAskApproval,
119+
mockHandleError,
120+
mockPushToolResult,
121+
mockRemoveClosingTag,
122+
)
123+
124+
expect(mockPushToolResult).toHaveBeenCalledWith(
125+
expect.stringContaining("Semantic search is not available yet (currently Indexing)"),
126+
)
127+
expect(mockPushToolResult).toHaveBeenCalledWith(
128+
expect.stringContaining("Code indexing is currently in progress"),
129+
)
130+
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
131+
})
132+
133+
it("should provide feedback when indexing is in Error state", async () => {
134+
mockCodeIndexManager.state = "Error"
135+
136+
const block: ToolUse = {
137+
type: "tool_use",
138+
name: "codebase_search",
139+
params: { query: "test query" },
140+
partial: false,
141+
}
142+
143+
await codebaseSearchTool(
144+
mockTask,
145+
block,
146+
mockAskApproval,
147+
mockHandleError,
148+
mockPushToolResult,
149+
mockRemoveClosingTag,
150+
)
151+
152+
expect(mockPushToolResult).toHaveBeenCalledWith(
153+
expect.stringContaining("Semantic search is not available yet (currently Error)"),
154+
)
155+
expect(mockPushToolResult).toHaveBeenCalledWith(
156+
expect.stringContaining("Code indexing encountered an error"),
157+
)
158+
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
159+
})
160+
161+
it("should perform search when indexing is complete (Indexed state)", async () => {
162+
mockCodeIndexManager.state = "Indexed"
163+
164+
const block: ToolUse = {
165+
type: "tool_use",
166+
name: "codebase_search",
167+
params: { query: "test query" },
168+
partial: false,
169+
}
170+
171+
await codebaseSearchTool(
172+
mockTask,
173+
block,
174+
mockAskApproval,
175+
mockHandleError,
176+
mockPushToolResult,
177+
mockRemoveClosingTag,
178+
)
179+
180+
expect(mockCodeIndexManager.searchIndex).toHaveBeenCalledWith("test query", undefined)
181+
// Check that say was called with the search results
182+
expect(mockTask.say).toHaveBeenCalledWith("codebase_search_result", expect.stringContaining("test code"))
183+
// Check that pushToolResult was called with the formatted output
184+
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Query: test query"))
185+
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("test code"))
186+
expect(mockPushToolResult).not.toHaveBeenCalledWith(
187+
expect.stringContaining("Semantic search is not available"),
188+
)
189+
})
190+
})
191+
192+
describe("feature configuration checks", () => {
193+
it("should throw error when feature is disabled", async () => {
194+
mockCodeIndexManager.isFeatureEnabled = false
195+
196+
const block: ToolUse = {
197+
type: "tool_use",
198+
name: "codebase_search",
199+
params: { query: "test query" },
200+
partial: false,
201+
}
202+
203+
await codebaseSearchTool(
204+
mockTask,
205+
block,
206+
mockAskApproval,
207+
mockHandleError,
208+
mockPushToolResult,
209+
mockRemoveClosingTag,
210+
)
211+
212+
expect(mockHandleError).toHaveBeenCalledWith(
213+
"codebase_search",
214+
expect.objectContaining({
215+
message: "Code Indexing is disabled in the settings.",
216+
}),
217+
)
218+
})
219+
220+
it("should throw error when feature is not configured", async () => {
221+
mockCodeIndexManager.isFeatureConfigured = false
222+
223+
const block: ToolUse = {
224+
type: "tool_use",
225+
name: "codebase_search",
226+
params: { query: "test query" },
227+
partial: false,
228+
}
229+
230+
await codebaseSearchTool(
231+
mockTask,
232+
block,
233+
mockAskApproval,
234+
mockHandleError,
235+
mockPushToolResult,
236+
mockRemoveClosingTag,
237+
)
238+
239+
expect(mockHandleError).toHaveBeenCalledWith(
240+
"codebase_search",
241+
expect.objectContaining({
242+
message: "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
243+
}),
244+
)
245+
})
246+
})
247+
248+
describe("parameter validation", () => {
249+
it("should handle missing query parameter", async () => {
250+
const block: ToolUse = {
251+
type: "tool_use",
252+
name: "codebase_search",
253+
params: {},
254+
partial: false,
255+
}
256+
257+
await codebaseSearchTool(
258+
mockTask,
259+
block,
260+
mockAskApproval,
261+
mockHandleError,
262+
mockPushToolResult,
263+
mockRemoveClosingTag,
264+
)
265+
266+
expect(mockTask.sayAndCreateMissingParamError).toHaveBeenCalledWith("codebase_search", "query")
267+
expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error")
268+
})
269+
270+
it("should handle partial tool use", async () => {
271+
const block: ToolUse = {
272+
type: "tool_use",
273+
name: "codebase_search",
274+
params: { query: "test" },
275+
partial: true,
276+
}
277+
278+
await codebaseSearchTool(
279+
mockTask,
280+
block,
281+
mockAskApproval,
282+
mockHandleError,
283+
mockPushToolResult,
284+
mockRemoveClosingTag,
285+
)
286+
287+
expect(mockTask.ask).toHaveBeenCalled()
288+
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
289+
})
290+
})
291+
292+
describe("search results handling", () => {
293+
it("should handle empty search results", async () => {
294+
mockCodeIndexManager.searchIndex.mockResolvedValue([])
295+
296+
const block: ToolUse = {
297+
type: "tool_use",
298+
name: "codebase_search",
299+
params: { query: "test query" },
300+
partial: false,
301+
}
302+
303+
await codebaseSearchTool(
304+
mockTask,
305+
block,
306+
mockAskApproval,
307+
mockHandleError,
308+
mockPushToolResult,
309+
mockRemoveClosingTag,
310+
)
311+
312+
expect(mockPushToolResult).toHaveBeenCalledWith(
313+
'No relevant code snippets found for the query: "test query"',
314+
)
315+
})
316+
317+
it("should format search results correctly", async () => {
318+
const block: ToolUse = {
319+
type: "tool_use",
320+
name: "codebase_search",
321+
params: { query: "test query" },
322+
partial: false,
323+
}
324+
325+
await codebaseSearchTool(
326+
mockTask,
327+
block,
328+
mockAskApproval,
329+
mockHandleError,
330+
mockPushToolResult,
331+
mockRemoveClosingTag,
332+
)
333+
334+
// The tool should call pushToolResult with a single formatted string containing all results
335+
expect(mockPushToolResult).toHaveBeenCalledTimes(1)
336+
const resultString = mockPushToolResult.mock.calls[0][0]
337+
expect(resultString).toContain("Query: test query")
338+
expect(resultString).toContain("File path: src/file.ts")
339+
expect(resultString).toContain("Score: 0.9")
340+
expect(resultString).toContain("Lines: 10-20")
341+
expect(resultString).toContain("Code Chunk: test code")
342+
})
343+
})
344+
})

src/core/tools/codebaseSearchTool.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,33 @@ export async function codebaseSearchTool(
8282
throw new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).")
8383
}
8484

85+
// Check indexing state at runtime
86+
const indexingState = manager.state
87+
if (indexingState !== "Indexed") {
88+
let stateMessage = ""
89+
switch (indexingState) {
90+
case "Standby":
91+
stateMessage =
92+
"Code indexing has not started yet. Please wait for the initial indexing to complete."
93+
break
94+
case "Indexing":
95+
stateMessage =
96+
"Code indexing is currently in progress. Semantic search will be available once indexing is complete."
97+
break
98+
case "Error":
99+
stateMessage = "Code indexing encountered an error. Please check your configuration and try again."
100+
break
101+
default:
102+
stateMessage = `Code indexing is in an unexpected state: ${indexingState}`
103+
}
104+
105+
// Return informative message instead of throwing error
106+
pushToolResult(
107+
`Semantic search is not available yet (currently ${indexingState}).\n\n${stateMessage}\n\nPlease use file reading tools (read_file, search_files) for now.`,
108+
)
109+
return
110+
}
111+
85112
const searchResults: VectorStoreSearchResult[] = await manager.searchIndex(query, directoryPrefix)
86113

87114
// 3. Format and push results

0 commit comments

Comments
 (0)