Skip to content

Commit 8dcc696

Browse files
committed
fix(code-index): search task workspace with full tool coverage
1 parent 8637e48 commit 8dcc696

3 files changed

Lines changed: 478 additions & 1 deletion

File tree

‎src/core/tools/CodebaseSearchTool.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export class CodebaseSearchTool extends BaseTool<"codebase_search"> {
5757
throw new Error("Extension context is not available.")
5858
}
5959

60-
const manager = CodeIndexManagerRegistry.getInstance(context)
60+
const manager = CodeIndexManagerRegistry.getInstance(context, workspacePath)
6161

6262
if (!manager) {
6363
throw new Error("CodeIndexManager is not available.")
Lines changed: 344 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,344 @@
1+
import * as vscode from "vscode"
2+
3+
import type { Task } from "../../task/Task"
4+
import type { ClineProvider } from "../../webview/ClineProvider"
5+
import type { CodeIndexManager } from "../../../services/code-index/manager"
6+
import type { VectorStoreSearchResult } from "../../../services/code-index/interfaces"
7+
import type { ToolUse } from "../../../shared/tools"
8+
import { CodeIndexManagerRegistry } from "../../../services/code-index/code-index-manager-registry"
9+
import { makeExtensionContext } from "../../../test-utils/vscode"
10+
import { getWorkspacePath } from "../../../utils/path"
11+
import { formatResponse } from "../../prompts/responses"
12+
import type { ToolCallbacks } from "../BaseTool"
13+
import { CodebaseSearchTool, codebaseSearchTool } from "../CodebaseSearchTool"
14+
15+
vi.mock("vscode", () => ({ workspace: { asRelativePath: vi.fn() } }))
16+
vi.mock("../../../utils/path", () => ({ getWorkspacePath: vi.fn() }))
17+
vi.mock("../../../services/code-index/code-index-manager-registry", () => ({
18+
CodeIndexManagerRegistry: { getInstance: vi.fn() },
19+
}))
20+
21+
describe("CodebaseSearchTool", () => {
22+
const query = "find handlers"
23+
let tool: CodebaseSearchTool
24+
let task: Task
25+
let context: vscode.ExtensionContext
26+
let callbacks: ToolCallbacks
27+
let manager: Pick<CodeIndexManager, "isFeatureEnabled" | "isFeatureConfigured" | "searchIndex">
28+
let deref: ReturnType<typeof vi.fn<Task["providerRef"]["deref"]>>
29+
30+
beforeEach(() => {
31+
vi.resetAllMocks()
32+
tool = new CodebaseSearchTool()
33+
context = makeExtensionContext()
34+
// Structural doubles expose only the provider/task/manager members consumed by the tool.
35+
deref = vi.fn<Task["providerRef"]["deref"]>().mockReturnValue({ context } as ClineProvider)
36+
const taskStub: Pick<
37+
Task,
38+
| "cwd"
39+
| "providerRef"
40+
| "consecutiveMistakeCount"
41+
| "didToolFailInCurrentTurn"
42+
| "sayAndCreateMissingParamError"
43+
| "say"
44+
| "ask"
45+
> = {
46+
cwd: "/task",
47+
providerRef: { deref, [Symbol.toStringTag]: "WeakRef" },
48+
consecutiveMistakeCount: 3,
49+
didToolFailInCurrentTurn: false,
50+
sayAndCreateMissingParamError: vi
51+
.fn<Task["sayAndCreateMissingParamError"]>()
52+
.mockResolvedValue("missing query"),
53+
say: vi.fn<Task["say"]>().mockResolvedValue(undefined),
54+
ask: vi.fn<Task["ask"]>().mockResolvedValue({ response: "yesButtonClicked" }),
55+
}
56+
task = taskStub as Task
57+
callbacks = {
58+
askApproval: vi.fn<ToolCallbacks["askApproval"]>().mockResolvedValue(true),
59+
handleError: vi.fn<ToolCallbacks["handleError"]>().mockResolvedValue(undefined),
60+
pushToolResult: vi.fn<ToolCallbacks["pushToolResult"]>(),
61+
}
62+
manager = {
63+
isFeatureEnabled: true,
64+
isFeatureConfigured: true,
65+
searchIndex: vi.fn<CodeIndexManager["searchIndex"]>().mockResolvedValue([]),
66+
}
67+
vi.mocked(CodeIndexManagerRegistry.getInstance).mockReturnValue(manager as CodeIndexManager)
68+
vi.mocked(getWorkspacePath).mockReturnValue("/fallback")
69+
vi.mocked(vscode.workspace.asRelativePath).mockReturnValue("src/result.ts")
70+
})
71+
72+
afterEach(() => vi.restoreAllMocks())
73+
74+
function expectNoSearch() {
75+
expect(manager.searchIndex).not.toHaveBeenCalled()
76+
expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled()
77+
expect(task.say).not.toHaveBeenCalled()
78+
}
79+
80+
function expectNoProviderAccess() {
81+
expect(deref).not.toHaveBeenCalled()
82+
expect(CodeIndexManagerRegistry.getInstance).not.toHaveBeenCalled()
83+
expectNoSearch()
84+
}
85+
86+
function result(overrides: Partial<VectorStoreSearchResult> = {}): VectorStoreSearchResult {
87+
return {
88+
id: "first",
89+
score: 0.9,
90+
payload: { filePath: "/task/src/result.ts", startLine: 2, endLine: 4, codeChunk: " \n first\n second \t" },
91+
...overrides,
92+
}
93+
}
94+
95+
it("exports a named tool instance", () => {
96+
expect(codebaseSearchTool).toBeInstanceOf(CodebaseSearchTool)
97+
expect(codebaseSearchTool.name).toBe("codebase_search")
98+
})
99+
100+
it("reports missing workspace before even validating the query", async () => {
101+
Object.defineProperty(task, "cwd", { value: "" })
102+
vi.mocked(getWorkspacePath).mockReturnValue("")
103+
await tool.execute({ query: "" }, task, callbacks)
104+
expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith(
105+
"codebase_search",
106+
new Error("Could not determine workspace path."),
107+
)
108+
expect(callbacks.askApproval).not.toHaveBeenCalled()
109+
expect(callbacks.pushToolResult).not.toHaveBeenCalled()
110+
expect(task.sayAndCreateMissingParamError).not.toHaveBeenCalled()
111+
expect(task.consecutiveMistakeCount).toBe(3)
112+
expect(task.didToolFailInCurrentTurn).toBe(false)
113+
expectNoProviderAccess()
114+
})
115+
116+
it("counts a missing query as a failed tool and forwards the missing-parameter response", async () => {
117+
await tool.execute({ query: "" }, task, callbacks)
118+
expect(task.consecutiveMistakeCount).toBe(4)
119+
expect(task.didToolFailInCurrentTurn).toBe(true)
120+
expect(task.sayAndCreateMissingParamError).toHaveBeenCalledExactlyOnceWith("codebase_search", "query")
121+
expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith("missing query")
122+
expect(callbacks.askApproval).not.toHaveBeenCalled()
123+
expect(callbacks.handleError).not.toHaveBeenCalled()
124+
expectNoProviderAccess()
125+
})
126+
127+
it.each([undefined, "src", ""])("does not search after denied approval with path %j", async (path) => {
128+
vi.mocked(callbacks.askApproval).mockResolvedValue(false)
129+
await tool.execute({ query, path }, task, callbacks)
130+
expect(callbacks.askApproval).toHaveBeenCalledExactlyOnceWith(
131+
"tool",
132+
JSON.stringify({ tool: "codebaseSearch", query, path, isOutsideWorkspace: false }),
133+
)
134+
expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith(formatResponse.toolDenied())
135+
expect(task.consecutiveMistakeCount).toBe(3)
136+
expect(task.didToolFailInCurrentTurn).toBe(false)
137+
expect(callbacks.handleError).not.toHaveBeenCalled()
138+
expectNoProviderAccess()
139+
})
140+
141+
it.each(["provider", "context"])("reports a missing %s after approval", async (missing) => {
142+
deref.mockReturnValue(missing === "provider" ? undefined : ({} as ClineProvider))
143+
await tool.execute({ query }, task, callbacks)
144+
expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith(
145+
"codebase_search",
146+
new Error("Extension context is not available."),
147+
)
148+
expect(task.consecutiveMistakeCount).toBe(0)
149+
expect(CodeIndexManagerRegistry.getInstance).not.toHaveBeenCalled()
150+
expect(callbacks.pushToolResult).not.toHaveBeenCalled()
151+
expectNoSearch()
152+
})
153+
154+
it.each([
155+
["missing", "CodeIndexManager is not available."],
156+
["disabled", "Code Indexing is disabled in the settings."],
157+
["unconfigured", "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL)."],
158+
])("reports a %s manager without searching", async (state, message) => {
159+
if (state === "missing") vi.mocked(CodeIndexManagerRegistry.getInstance).mockReturnValue(undefined)
160+
if (state === "disabled") Object.defineProperty(manager, "isFeatureEnabled", { value: false })
161+
if (state === "unconfigured") Object.defineProperty(manager, "isFeatureConfigured", { value: false })
162+
await tool.execute({ query }, task, callbacks)
163+
expect(CodeIndexManagerRegistry.getInstance).toHaveBeenCalledExactlyOnceWith(context, "/task")
164+
expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith("codebase_search", new Error(message))
165+
expect(callbacks.pushToolResult).not.toHaveBeenCalled()
166+
expect(task.consecutiveMistakeCount).toBe(0)
167+
expectNoSearch()
168+
})
169+
170+
it.each([undefined, "src", ""])(
171+
"forwards directory prefix %j and resets mistakes before searching",
172+
async (path) => {
173+
vi.mocked(manager.searchIndex).mockImplementation(async () => {
174+
expect(task.consecutiveMistakeCount).toBe(0)
175+
return []
176+
})
177+
await tool.execute({ query, path }, task, callbacks)
178+
expect(manager.searchIndex).toHaveBeenCalledExactlyOnceWith(query, path)
179+
expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith(
180+
`No relevant code snippets found for the query: "${query}"`,
181+
)
182+
expect(callbacks.handleError).not.toHaveBeenCalled()
183+
expect(task.say).not.toHaveBeenCalled()
184+
expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled()
185+
expect(getWorkspacePath).not.toHaveBeenCalled()
186+
},
187+
)
188+
189+
it.each([null, undefined, false, 0, ""])(
190+
"defensively handles a runtime-invalid falsy search response %j",
191+
async (value) => {
192+
// The manager promises an array. Deliberately violate that boundary to exercise the existing falsy guard.
193+
vi.mocked(manager.searchIndex).mockResolvedValue(value as unknown as VectorStoreSearchResult[])
194+
await tool.execute({ query }, task, callbacks)
195+
expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith(
196+
`No relevant code snippets found for the query: "${query}"`,
197+
)
198+
expect(callbacks.handleError).not.toHaveBeenCalled()
199+
expect(task.say).not.toHaveBeenCalled()
200+
expect(vscode.workspace.asRelativePath).not.toHaveBeenCalled()
201+
},
202+
)
203+
204+
it("preserves result order and metadata, relativizes paths without workspace prefixes and trims chunks", async () => {
205+
vi.mocked(manager.searchIndex).mockResolvedValue([
206+
result(),
207+
result({
208+
id: "second",
209+
score: 0.5,
210+
payload: { filePath: "/task/lib/other.ts", startLine: 10, endLine: 10, codeChunk: " \t " },
211+
}),
212+
])
213+
vi.mocked(vscode.workspace.asRelativePath)
214+
.mockReturnValueOnce("src/result.ts")
215+
.mockReturnValueOnce("lib/other.ts")
216+
await tool.execute({ query }, task, callbacks)
217+
expect(vscode.workspace.asRelativePath).toHaveBeenCalledTimes(2)
218+
expect(vscode.workspace.asRelativePath).toHaveBeenNthCalledWith(1, "/task/src/result.ts", false)
219+
expect(vscode.workspace.asRelativePath).toHaveBeenNthCalledWith(2, "/task/lib/other.ts", false)
220+
expect(task.say).toHaveBeenCalledExactlyOnceWith(
221+
"codebase_search_result",
222+
JSON.stringify({
223+
tool: "codebaseSearch",
224+
content: {
225+
query,
226+
results: [
227+
{
228+
filePath: "src/result.ts",
229+
score: 0.9,
230+
startLine: 2,
231+
endLine: 4,
232+
codeChunk: "first\n second",
233+
},
234+
{ filePath: "lib/other.ts", score: 0.5, startLine: 10, endLine: 10, codeChunk: "" },
235+
],
236+
},
237+
}),
238+
)
239+
expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith(
240+
`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`,
241+
)
242+
expect(task.say).toHaveBeenCalledBefore(vi.mocked(callbacks.pushToolResult))
243+
expect(callbacks.handleError).not.toHaveBeenCalled()
244+
})
245+
246+
it.each([false, true])("skips absent payloads/file paths (include a valid result: %s)", async (includeValid) => {
247+
// Missing filePath is invalid under Payload's type but explicitly guarded against at runtime.
248+
const missingPath = { id: "malformed", score: 1, payload: { codeChunk: "ignored" } } as VectorStoreSearchResult
249+
vi.mocked(manager.searchIndex).mockResolvedValue([
250+
result({ payload: undefined }),
251+
result({ payload: null }),
252+
missingPath,
253+
...(includeValid ? [result()] : []),
254+
])
255+
await tool.execute({ query }, task, callbacks)
256+
expect(vscode.workspace.asRelativePath).toHaveBeenCalledTimes(includeValid ? 1 : 0)
257+
expect(task.say).toHaveBeenCalledExactlyOnceWith(
258+
"codebase_search_result",
259+
JSON.stringify({
260+
tool: "codebaseSearch",
261+
content: {
262+
query,
263+
results: includeValid
264+
? [
265+
{
266+
filePath: "src/result.ts",
267+
score: 0.9,
268+
startLine: 2,
269+
endLine: 4,
270+
codeChunk: "first\n second",
271+
},
272+
]
273+
: [],
274+
},
275+
}),
276+
)
277+
// A nonempty response whose entries are all skipped still emits an empty result header, not "No relevant...".
278+
expect(callbacks.pushToolResult).toHaveBeenCalledExactlyOnceWith(
279+
`Query: ${query}\nResults:\n\n${includeValid ? "File path: src/result.ts\nScore: 0.9\nLines: 2-4\nCode Chunk: first\n second\n" : ""}`,
280+
)
281+
expect(callbacks.handleError).not.toHaveBeenCalled()
282+
})
283+
284+
it.each(["registry", "search", "say"])("forwards the original %s error without a tool result", async (source) => {
285+
const error = new Error(`${source} failed`)
286+
if (source === "registry")
287+
vi.mocked(CodeIndexManagerRegistry.getInstance).mockImplementation(() => {
288+
throw error
289+
})
290+
if (source === "search") vi.mocked(manager.searchIndex).mockRejectedValue(error)
291+
if (source === "say") {
292+
vi.mocked(manager.searchIndex).mockResolvedValue([result()])
293+
vi.mocked(task.say).mockRejectedValue(error)
294+
}
295+
await tool.execute({ query }, task, callbacks)
296+
expect(callbacks.handleError).toHaveBeenCalledExactlyOnceWith("codebase_search", error)
297+
expect(vi.mocked(callbacks.handleError).mock.calls[0][1]).toBe(error)
298+
expect(callbacks.pushToolResult).not.toHaveBeenCalled()
299+
if (source === "registry") expectNoSearch()
300+
if (source === "search") expect(task.say).not.toHaveBeenCalled()
301+
})
302+
303+
describe("handlePartial", () => {
304+
it.each([
305+
{ params: {}, partial: true },
306+
{ params: { query }, partial: true },
307+
{ params: { path: "src" }, partial: false },
308+
{ params: { query, path: "src" }, partial: true },
309+
{ params: { query: "", path: "" }, partial: false },
310+
])("sends the supplied optional fields and partial flag: %j", async ({ params, partial }) => {
311+
const block: ToolUse<"codebase_search"> = { type: "tool_use", name: "codebase_search", params, partial }
312+
await tool.handlePartial(task, block)
313+
expect(task.ask).toHaveBeenCalledExactlyOnceWith(
314+
"tool",
315+
JSON.stringify({
316+
tool: "codebaseSearch",
317+
...params,
318+
isOutsideWorkspace: false,
319+
}),
320+
partial,
321+
)
322+
expectNoProviderAccess()
323+
expect(callbacks.askApproval).not.toHaveBeenCalled()
324+
expect(callbacks.pushToolResult).not.toHaveBeenCalled()
325+
expect(task.consecutiveMistakeCount).toBe(3)
326+
})
327+
328+
it("swallows a rejected partial ask without searching or reporting a tool error", async () => {
329+
vi.mocked(task.ask).mockRejectedValue(new Error("superseded partial message"))
330+
await expect(
331+
tool.handlePartial(task, {
332+
type: "tool_use",
333+
name: "codebase_search",
334+
params: { query },
335+
partial: true,
336+
}),
337+
).resolves.toBeUndefined()
338+
expect(task.ask).toHaveBeenCalledOnce()
339+
expect(callbacks.handleError).not.toHaveBeenCalled()
340+
expect(callbacks.pushToolResult).not.toHaveBeenCalled()
341+
expectNoProviderAccess()
342+
})
343+
})
344+
})

0 commit comments

Comments
 (0)