Skip to content

Commit 8bd4fc5

Browse files
committed
fix: add runtime indexing state check to codebase_search tool (#5662)
- Check if indexing is complete (state === 'Indexed') before searching - Provide clear error message with current indexing state - Guide users to use alternative tools while indexing - Add comprehensive test coverage for all indexing states This ensures semantic search is only used when the index is ready, preventing confusion and failed search attempts during indexing.
1 parent f648e4c commit 8bd4fc5

File tree

2 files changed

+394
-0
lines changed

2 files changed

+394
-0
lines changed
Lines changed: 386 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,386 @@
1+
// npx vitest src/core/tools/__tests__/codebaseSearchTool.spec.ts
2+
3+
import { describe, it, expect, vi, beforeEach } from "vitest"
4+
import * as path from "path"
5+
import { codebaseSearchTool } from "../codebaseSearchTool"
6+
import { Task } from "../../task/Task"
7+
import { CodeIndexManager } from "../../../services/code-index/manager"
8+
import { ToolUse } from "../../../shared/tools"
9+
import { formatResponse } from "../../prompts/responses"
10+
11+
// Mock vscode
12+
vi.mock("vscode", () => ({
13+
workspace: {
14+
asRelativePath: vi.fn().mockImplementation((filePath) => {
15+
// Simple mock that just returns the path without workspace prefix
16+
return filePath.replace("/test/workspace/", "")
17+
}),
18+
},
19+
}))
20+
21+
// Mock dependencies
22+
vi.mock("../../../utils/path", () => ({
23+
getWorkspacePath: vi.fn().mockReturnValue("/test/workspace"),
24+
}))
25+
26+
vi.mock("../../../services/code-index/manager")
27+
28+
vi.mock("../../prompts/responses", () => ({
29+
formatResponse: {
30+
toolDenied: vi.fn().mockReturnValue("Tool denied"),
31+
},
32+
}))
33+
34+
describe("codebaseSearchTool", () => {
35+
let mockCline: Task
36+
let mockAskApproval: any
37+
let mockHandleError: any
38+
let mockPushToolResult: any
39+
let mockRemoveClosingTag: any
40+
let mockCodeIndexManager: any
41+
42+
beforeEach(() => {
43+
vi.clearAllMocks()
44+
45+
// Mock Cline/Task
46+
mockCline = {
47+
ask: vi.fn().mockResolvedValue(undefined),
48+
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
49+
say: vi.fn().mockResolvedValue(undefined),
50+
consecutiveMistakeCount: 0,
51+
providerRef: {
52+
deref: vi.fn().mockReturnValue({
53+
context: {},
54+
}),
55+
},
56+
} as any
57+
58+
// Mock callback functions
59+
mockAskApproval = vi.fn().mockResolvedValue(true)
60+
mockHandleError = vi.fn().mockResolvedValue(undefined)
61+
mockPushToolResult = vi.fn()
62+
mockRemoveClosingTag = vi.fn().mockImplementation((tag, value) => value)
63+
64+
// Mock CodeIndexManager instance
65+
mockCodeIndexManager = {
66+
isFeatureEnabled: true,
67+
isFeatureConfigured: true,
68+
state: "Indexed",
69+
searchIndex: vi.fn().mockResolvedValue([
70+
{
71+
score: 0.9,
72+
payload: {
73+
filePath: "/test/workspace/src/example.ts",
74+
startLine: 10,
75+
endLine: 20,
76+
codeChunk: "function example() { return 'test'; }",
77+
},
78+
},
79+
]),
80+
}
81+
82+
// Mock CodeIndexManager.getInstance
83+
vi.mocked(CodeIndexManager).getInstance = vi.fn().mockReturnValue(mockCodeIndexManager)
84+
})
85+
86+
describe("indexing state checks", () => {
87+
it("should throw error when indexing state is 'Indexing'", async () => {
88+
// Arrange
89+
mockCodeIndexManager.state = "Indexing"
90+
const block: ToolUse = {
91+
type: "tool_use",
92+
name: "codebase_search",
93+
params: {
94+
query: "test query",
95+
},
96+
partial: false,
97+
}
98+
99+
// Act
100+
await codebaseSearchTool(
101+
mockCline,
102+
block,
103+
mockAskApproval,
104+
mockHandleError,
105+
mockPushToolResult,
106+
mockRemoveClosingTag,
107+
)
108+
109+
// Assert
110+
expect(mockHandleError).toHaveBeenCalledWith(
111+
"codebase_search",
112+
expect.objectContaining({
113+
message: expect.stringContaining("Semantic search isn't ready yet (currently Indexing)"),
114+
}),
115+
)
116+
})
117+
118+
it("should throw error when indexing state is 'Standby'", async () => {
119+
// Arrange
120+
mockCodeIndexManager.state = "Standby"
121+
const block: ToolUse = {
122+
type: "tool_use",
123+
name: "codebase_search",
124+
params: {
125+
query: "test query",
126+
},
127+
partial: false,
128+
}
129+
130+
// Act
131+
await codebaseSearchTool(
132+
mockCline,
133+
block,
134+
mockAskApproval,
135+
mockHandleError,
136+
mockPushToolResult,
137+
mockRemoveClosingTag,
138+
)
139+
140+
// Assert
141+
expect(mockHandleError).toHaveBeenCalledWith(
142+
"codebase_search",
143+
expect.objectContaining({
144+
message: expect.stringContaining("Semantic search isn't ready yet (currently Standby)"),
145+
}),
146+
)
147+
})
148+
149+
it("should throw error when indexing state is 'Error'", async () => {
150+
// Arrange
151+
mockCodeIndexManager.state = "Error"
152+
const block: ToolUse = {
153+
type: "tool_use",
154+
name: "codebase_search",
155+
params: {
156+
query: "test query",
157+
},
158+
partial: false,
159+
}
160+
161+
// Act
162+
await codebaseSearchTool(
163+
mockCline,
164+
block,
165+
mockAskApproval,
166+
mockHandleError,
167+
mockPushToolResult,
168+
mockRemoveClosingTag,
169+
)
170+
171+
// Assert
172+
expect(mockHandleError).toHaveBeenCalledWith(
173+
"codebase_search",
174+
expect.objectContaining({
175+
message: expect.stringContaining("Semantic search isn't ready yet (currently Error)"),
176+
}),
177+
)
178+
})
179+
180+
it("should proceed with search when indexing state is 'Indexed'", async () => {
181+
// Arrange
182+
mockCodeIndexManager.state = "Indexed"
183+
const block: ToolUse = {
184+
type: "tool_use",
185+
name: "codebase_search",
186+
params: {
187+
query: "test query",
188+
},
189+
partial: false,
190+
}
191+
192+
// Act
193+
await codebaseSearchTool(
194+
mockCline,
195+
block,
196+
mockAskApproval,
197+
mockHandleError,
198+
mockPushToolResult,
199+
mockRemoveClosingTag,
200+
)
201+
202+
// Assert
203+
expect(mockHandleError).not.toHaveBeenCalled()
204+
expect(mockCodeIndexManager.searchIndex).toHaveBeenCalledWith("test query", undefined)
205+
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("test query"))
206+
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("example.ts"))
207+
})
208+
})
209+
210+
describe("feature availability checks", () => {
211+
it("should throw error when feature is disabled", async () => {
212+
// Arrange
213+
mockCodeIndexManager.isFeatureEnabled = false
214+
const block: ToolUse = {
215+
type: "tool_use",
216+
name: "codebase_search",
217+
params: {
218+
query: "test query",
219+
},
220+
partial: false,
221+
}
222+
223+
// Act
224+
await codebaseSearchTool(
225+
mockCline,
226+
block,
227+
mockAskApproval,
228+
mockHandleError,
229+
mockPushToolResult,
230+
mockRemoveClosingTag,
231+
)
232+
233+
// Assert
234+
expect(mockHandleError).toHaveBeenCalledWith(
235+
"codebase_search",
236+
expect.objectContaining({
237+
message: "Code Indexing is disabled in the settings.",
238+
}),
239+
)
240+
})
241+
242+
it("should throw error when feature is not configured", async () => {
243+
// Arrange
244+
mockCodeIndexManager.isFeatureConfigured = false
245+
const block: ToolUse = {
246+
type: "tool_use",
247+
name: "codebase_search",
248+
params: {
249+
query: "test query",
250+
},
251+
partial: false,
252+
}
253+
254+
// Act
255+
await codebaseSearchTool(
256+
mockCline,
257+
block,
258+
mockAskApproval,
259+
mockHandleError,
260+
mockPushToolResult,
261+
mockRemoveClosingTag,
262+
)
263+
264+
// Assert
265+
expect(mockHandleError).toHaveBeenCalledWith(
266+
"codebase_search",
267+
expect.objectContaining({
268+
message: "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
269+
}),
270+
)
271+
})
272+
})
273+
274+
describe("parameter validation", () => {
275+
it("should handle missing query parameter", async () => {
276+
// Arrange
277+
const block: ToolUse = {
278+
type: "tool_use",
279+
name: "codebase_search",
280+
params: {},
281+
partial: false,
282+
}
283+
284+
// Act
285+
await codebaseSearchTool(
286+
mockCline,
287+
block,
288+
mockAskApproval,
289+
mockHandleError,
290+
mockPushToolResult,
291+
mockRemoveClosingTag,
292+
)
293+
294+
// Assert
295+
expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("codebase_search", "query")
296+
expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error")
297+
})
298+
299+
it("should handle user denial", async () => {
300+
// Arrange
301+
mockAskApproval.mockResolvedValue(false)
302+
const block: ToolUse = {
303+
type: "tool_use",
304+
name: "codebase_search",
305+
params: {
306+
query: "test query",
307+
},
308+
partial: false,
309+
}
310+
311+
// Act
312+
await codebaseSearchTool(
313+
mockCline,
314+
block,
315+
mockAskApproval,
316+
mockHandleError,
317+
mockPushToolResult,
318+
mockRemoveClosingTag,
319+
)
320+
321+
// Assert
322+
expect(mockPushToolResult).toHaveBeenCalledWith("Tool denied")
323+
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
324+
})
325+
})
326+
327+
describe("search results handling", () => {
328+
it("should handle empty search results", async () => {
329+
// Arrange
330+
mockCodeIndexManager.searchIndex.mockResolvedValue([])
331+
const block: ToolUse = {
332+
type: "tool_use",
333+
name: "codebase_search",
334+
params: {
335+
query: "nonexistent query",
336+
},
337+
partial: false,
338+
}
339+
340+
// Act
341+
await codebaseSearchTool(
342+
mockCline,
343+
block,
344+
mockAskApproval,
345+
mockHandleError,
346+
mockPushToolResult,
347+
mockRemoveClosingTag,
348+
)
349+
350+
// Assert
351+
expect(mockPushToolResult).toHaveBeenCalledWith(
352+
'No relevant code snippets found for the query: "nonexistent query"',
353+
)
354+
})
355+
356+
it("should handle search with directory prefix", async () => {
357+
// Arrange
358+
const block: ToolUse = {
359+
type: "tool_use",
360+
name: "codebase_search",
361+
params: {
362+
query: "test query",
363+
path: "src/components",
364+
},
365+
partial: false,
366+
}
367+
368+
// Act
369+
await codebaseSearchTool(
370+
mockCline,
371+
block,
372+
mockAskApproval,
373+
mockHandleError,
374+
mockPushToolResult,
375+
mockRemoveClosingTag,
376+
)
377+
378+
// Assert
379+
// The path gets normalized in the tool, so we need to check for the normalized version
380+
expect(mockCodeIndexManager.searchIndex).toHaveBeenCalledWith(
381+
"test query",
382+
path.normalize("src/components"),
383+
)
384+
})
385+
})
386+
})

0 commit comments

Comments
 (0)