-
Notifications
You must be signed in to change notification settings - Fork 281
fix: align codebase search readiness across mode filters #1630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
edelauna
merged 4 commits into
Zoo-Code-Org:main
from
WebMad:fix/codebase-search-readiness
Sep 19, 2026
+362
−1
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
663923c
test(code-index): readiness coverage
WebMad 7a219bc
test(code-index): clarify search readiness scenarios
WebMad 64dbc30
test(tools): cover codebase search mode permissions directly
WebMad 847c7e8
test(build-tools-readiness): add liveness check and extract callable …
edelauna File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
160 changes: 160 additions & 0 deletions
160
src/core/prompts/tools/__tests__/codebase-search-readiness.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| import type OpenAI from "openai" | ||
| import { toolNamesSchema, type ModeConfig } from "@roo-code/types" | ||
| import type { CodeIndexManager } from "../../../../services/code-index/manager" | ||
| import { filterNativeToolsForMode } from "../filter-tools-for-mode" | ||
| import { resolveEffectiveToolPolicy } from "../effective-tool-policy" | ||
| import { getNativeTools } from "../native-tools" | ||
|
|
||
| const tools = toolNamesSchema.enum | ||
| const ordinaryReadTools = [tools.read_file, tools.list_files, tools.search_files] | ||
| type Readiness = Pick<CodeIndexManager, "isFeatureEnabled" | "isFeatureConfigured" | "isInitialized"> | ||
|
|
||
| function makeManager(flags: Readiness): CodeIndexManager { | ||
| // These consumers only read the public readiness getters, not manager services. | ||
| return flags as CodeIndexManager | ||
| } | ||
|
|
||
| function toolNames(definitions: OpenAI.Chat.ChatCompletionTool[]) { | ||
| return definitions.flatMap((tool) => ("function" in tool ? [tool.function.name] : [])) | ||
| } | ||
|
|
||
| describe("codebase_search readiness", () => { | ||
| it("excludes search without a manager but retains ordinary read tools", () => { | ||
| const policy = resolveEffectiveToolPolicy({ mode: "code" }) | ||
| const filtered = filterNativeToolsForMode(getNativeTools(), "code", [], {}) | ||
|
|
||
| expect(policy.tools).not.toContain(tools.codebase_search) | ||
| expect(toolNames(filtered)).not.toContain(tools.codebase_search) | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(policy.tools).toContain(tool) | ||
| expect(toolNames(filtered)).toContain(tool) | ||
| } | ||
| }) | ||
|
|
||
| it.each([ | ||
| { isFeatureEnabled: false, isFeatureConfigured: false, isInitialized: false }, | ||
| { isFeatureEnabled: false, isFeatureConfigured: false, isInitialized: true }, | ||
| { isFeatureEnabled: false, isFeatureConfigured: true, isInitialized: false }, | ||
| { isFeatureEnabled: false, isFeatureConfigured: true, isInitialized: true }, | ||
| { isFeatureEnabled: true, isFeatureConfigured: false, isInitialized: false }, | ||
| { isFeatureEnabled: true, isFeatureConfigured: false, isInitialized: true }, | ||
| { isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: false }, | ||
| ])( | ||
| "excludes search for enabled=$isFeatureEnabled, configured=$isFeatureConfigured, initialized=$isInitialized", | ||
| (flags) => { | ||
| const manager = makeManager(flags) | ||
| const policy = resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager }) | ||
| const filtered = filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager) | ||
|
|
||
| expect(policy.tools).not.toContain(tools.codebase_search) | ||
| expect(toolNames(filtered)).not.toContain(tools.codebase_search) | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(policy.tools).toContain(tool) | ||
| expect(toolNames(filtered)).toContain(tool) | ||
| } | ||
| }, | ||
| ) | ||
|
|
||
| it("includes search when all three readiness conditions are met", () => { | ||
| const manager = makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }) | ||
| const policy = resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager }) | ||
| const filtered = filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager) | ||
|
|
||
| expect(policy.tools).toContain(tools.codebase_search) | ||
| expect(toolNames(filtered)).toContain(tools.codebase_search) | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(policy.tools).toContain(tool) | ||
| expect(toolNames(filtered)).toContain(tool) | ||
| } | ||
| }) | ||
|
|
||
| it.each(["isFeatureEnabled", "isFeatureConfigured", "isInitialized"] as const)( | ||
| "rereads %s when the same manager becomes unavailable and recovers", | ||
| (flag) => { | ||
| const flags = { isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true } | ||
| const manager = makeManager(flags) | ||
|
|
||
| expect(resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager }).tools).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| expect(toolNames(filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager))).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
|
|
||
| flags[flag] = false | ||
|
|
||
| expect(resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager }).tools).not.toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| expect(toolNames(filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager))).not.toContain( | ||
| tools.codebase_search, | ||
| ) | ||
|
|
||
| flags[flag] = true | ||
|
|
||
| expect(resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager }).tools).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| expect(toolNames(filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager))).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| }, | ||
| ) | ||
|
|
||
| it("does not reuse readiness from another manager or a missing manager", () => { | ||
| const ready = makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }) | ||
| const disabled = makeManager({ isFeatureEnabled: false, isFeatureConfigured: true, isInitialized: true }) | ||
|
|
||
| expect(resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: ready }).tools).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| expect(toolNames(filterNativeToolsForMode(getNativeTools(), "code", [], {}, ready))).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
|
|
||
| for (const manager of [disabled, undefined]) { | ||
| expect(resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager }).tools).not.toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| expect(toolNames(filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager))).not.toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| } | ||
|
|
||
| expect(resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: ready }).tools).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| expect(toolNames(filterNativeToolsForMode(getNativeTools(), "code", [], {}, ready))).toContain( | ||
| tools.codebase_search, | ||
| ) | ||
| }) | ||
|
|
||
| it("does not grant read permissions merely because the manager is ready", () => { | ||
| const manager = makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }) | ||
| const mode: ModeConfig = { slug: "no-read", name: "No read", roleDefinition: "No reading", groups: ["command"] } | ||
| const policy = resolveEffectiveToolPolicy({ mode: mode.slug, customModes: [mode], codeIndexManager: manager }) | ||
| const filtered = filterNativeToolsForMode(getNativeTools(), mode.slug, [mode], {}, manager) | ||
|
|
||
| for (const tool of [tools.codebase_search, ...ordinaryReadTools]) { | ||
| expect(policy.tools).not.toContain(tool) | ||
| expect(toolNames(filtered)).not.toContain(tool) | ||
| } | ||
| // The mode remains usable; this is not an accidentally empty result. | ||
| expect(policy.tools).toContain(tools.execute_command) | ||
| expect(toolNames(filtered)).toContain(tools.execute_command) | ||
| }) | ||
|
|
||
| it("honors explicit search disabling without disabling ordinary read tools", () => { | ||
| const manager = makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }) | ||
| const disabledTools = [tools.codebase_search] | ||
| const policy = resolveEffectiveToolPolicy({ mode: "code", codeIndexManager: manager, disabledTools }) | ||
| const filtered = filterNativeToolsForMode(getNativeTools(), "code", [], {}, manager, { disabledTools }) | ||
|
|
||
| expect(policy.tools).not.toContain(tools.codebase_search) | ||
| expect(toolNames(filtered)).not.toContain(tools.codebase_search) | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(policy.tools).toContain(tool) | ||
| expect(toolNames(filtered)).toContain(tool) | ||
| } | ||
| }) | ||
| }) | ||
179 changes: 179 additions & 0 deletions
179
src/core/task/__tests__/build-tools-readiness.integration.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| import type OpenAI from "openai" | ||
| import { toolNamesSchema } from "@roo-code/types" | ||
| import type { CodeIndexManager } from "../../../services/code-index/manager" | ||
| import { CodeIndexManagerRegistry } from "../../../services/code-index/code-index-manager-registry" | ||
| import { makeExtensionContext } from "../../../test-utils/vscode" | ||
| import type { ClineProvider } from "../../webview/ClineProvider" | ||
| import { buildNativeToolsArrayWithRestrictions } from "../build-tools" | ||
|
|
||
| vi.mock("../../../services/code-index/code-index-manager-registry", () => ({ | ||
| CodeIndexManagerRegistry: { getOrCreate: vi.fn() }, | ||
| })) | ||
|
|
||
| const tools = toolNamesSchema.enum | ||
| const ordinaryReadTools = [tools.read_file, tools.list_files, tools.search_files] | ||
|
|
||
| function toolNames(definitions: OpenAI.Chat.ChatCompletionTool[]) { | ||
| return definitions.flatMap((tool) => ("function" in tool ? [tool.function.name] : [])) | ||
| } | ||
|
|
||
| function makeManager(flags: Pick<CodeIndexManager, "isFeatureEnabled" | "isFeatureConfigured" | "isInitialized">) { | ||
| // The real filter only consumes these public readiness getters, not manager services. | ||
| return flags as CodeIndexManager | ||
| } | ||
|
|
||
| describe.each([ | ||
| { strategy: "filtered definitions", includeAllToolsWithRestrictions: false }, | ||
| { strategy: "all definitions with an allowlist", includeAllToolsWithRestrictions: true }, | ||
| ])("task readiness with $strategy", ({ includeAllToolsWithRestrictions }) => { | ||
| beforeEach(() => vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockReset()) | ||
|
|
||
| function makeOptions() { | ||
| const context = makeExtensionContext() | ||
| // Only context and getMcpHub are needed; constructing a webview provider is unrelated to this test. | ||
| const provider = { context, getMcpHub: () => undefined } as ClineProvider | ||
| return { | ||
| provider, | ||
| cwd: "/tasks/ready", | ||
| mode: "code", | ||
| customModes: [], | ||
| experiments: {}, | ||
| apiConfiguration: {}, | ||
| includeAllToolsWithRestrictions, | ||
| } | ||
| } | ||
|
|
||
| function callable(result: Awaited<ReturnType<typeof buildNativeToolsArrayWithRestrictions>>) { | ||
| return includeAllToolsWithRestrictions ? result.allowedFunctionNames : toolNames(result.tools) | ||
| } | ||
|
|
||
| it("uses the task context and cwd without leaking readiness between workspaces", async () => { | ||
| const options = makeOptions() | ||
| const ready = makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }) | ||
| const unready = makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: false }) | ||
| const managers = new Map([ | ||
| ["/tasks/ready", ready], | ||
| ["/tasks/unready", unready], | ||
| ]) | ||
| vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockImplementation((_context, cwd) => managers.get(cwd ?? "")) | ||
|
|
||
| const first = await buildNativeToolsArrayWithRestrictions(options) | ||
| expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenLastCalledWith(options.provider.context, "/tasks/ready") | ||
| expect(callable(first)).toContain(tools.codebase_search) | ||
|
|
||
| const other = await buildNativeToolsArrayWithRestrictions({ ...options, cwd: "/tasks/unready" }) | ||
| expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenLastCalledWith( | ||
| options.provider.context, | ||
| "/tasks/unready", | ||
| ) | ||
| if (includeAllToolsWithRestrictions) { | ||
| expect(other.allowedFunctionNames).toBeDefined() | ||
| expect(other.allowedFunctionNames).not.toContain(tools.codebase_search) | ||
| // Keep definitions for historical calls, while forbidding new calls. | ||
| expect(toolNames(other.tools)).toContain(tools.codebase_search) | ||
| } else { | ||
| expect(other.allowedFunctionNames).toBeUndefined() | ||
| expect(toolNames(other.tools)).not.toContain(tools.codebase_search) | ||
| } | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(callable(other)).toContain(tool) | ||
| } | ||
|
|
||
| const restored = await buildNativeToolsArrayWithRestrictions(options) | ||
| expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenLastCalledWith(options.provider.context, "/tasks/ready") | ||
| expect(callable(restored)).toContain(tools.codebase_search) | ||
| expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenCalledTimes(3) | ||
| }) | ||
|
|
||
| it("omits search without a manager while retaining ordinary read tools", async () => { | ||
| vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockReturnValue(undefined) | ||
| const result = await buildNativeToolsArrayWithRestrictions({ ...makeOptions(), cwd: "/tasks/missing" }) | ||
|
|
||
| if (includeAllToolsWithRestrictions) { | ||
| expect(result.allowedFunctionNames).toBeDefined() | ||
| expect(result.allowedFunctionNames).not.toContain(tools.codebase_search) | ||
| expect(toolNames(result.tools)).toContain(tools.codebase_search) | ||
| } else { | ||
| expect(result.allowedFunctionNames).toBeUndefined() | ||
| expect(toolNames(result.tools)).not.toContain(tools.codebase_search) | ||
| } | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(callable(result)).toContain(tool) | ||
| } | ||
| }) | ||
|
|
||
| it.each(["isFeatureEnabled", "isFeatureConfigured", "isInitialized"] as const)( | ||
| "rereads %s on subsequent builds with the same manager", | ||
| async (flag) => { | ||
| const options = makeOptions() | ||
| const flags = { isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true } | ||
| vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockReturnValue(makeManager(flags)) | ||
|
|
||
| const initial = await buildNativeToolsArrayWithRestrictions(options) | ||
| expect(callable(initial)).toContain(tools.codebase_search) | ||
|
|
||
| flags[flag] = false | ||
| const unavailable = await buildNativeToolsArrayWithRestrictions(options) | ||
| if (includeAllToolsWithRestrictions) { | ||
| expect(unavailable.allowedFunctionNames).toBeDefined() | ||
| expect(unavailable.allowedFunctionNames).not.toContain(tools.codebase_search) | ||
| expect(toolNames(unavailable.tools)).toContain(tools.codebase_search) | ||
| } else { | ||
| expect(unavailable.allowedFunctionNames).toBeUndefined() | ||
| expect(toolNames(unavailable.tools)).not.toContain(tools.codebase_search) | ||
| } | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(callable(unavailable)).toContain(tool) | ||
| } | ||
|
|
||
| flags[flag] = true | ||
| const recovered = await buildNativeToolsArrayWithRestrictions(options) | ||
| expect(callable(recovered)).toContain(tools.codebase_search) | ||
| }, | ||
| ) | ||
|
|
||
| it("does not grant read tools to a command-only mode even with a ready manager", async () => { | ||
| vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockReturnValue( | ||
| makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }), | ||
| ) | ||
| const result = await buildNativeToolsArrayWithRestrictions({ | ||
| ...makeOptions(), | ||
| mode: "no-read", | ||
| customModes: [{ slug: "no-read", name: "No read", roleDefinition: "No reading", groups: ["command"] }], | ||
| }) | ||
|
|
||
| if (includeAllToolsWithRestrictions) { | ||
| expect(result.allowedFunctionNames).toBeDefined() | ||
| expect(toolNames(result.tools)).toContain(tools.codebase_search) | ||
| } else { | ||
| expect(result.allowedFunctionNames).toBeUndefined() | ||
| } | ||
| const callableSet = callable(result) | ||
| for (const tool of [tools.codebase_search, ...ordinaryReadTools]) { | ||
| expect(callableSet).not.toContain(tool) | ||
| } | ||
| expect(callableSet).toContain(tools.execute_command) | ||
| }) | ||
|
|
||
| it("honors disabledTools with a ready manager without disabling ordinary read tools", async () => { | ||
| vi.mocked(CodeIndexManagerRegistry.getOrCreate).mockReturnValue( | ||
| makeManager({ isFeatureEnabled: true, isFeatureConfigured: true, isInitialized: true }), | ||
| ) | ||
| const result = await buildNativeToolsArrayWithRestrictions({ | ||
| ...makeOptions(), | ||
| disabledTools: [tools.codebase_search], | ||
| }) | ||
|
|
||
| if (includeAllToolsWithRestrictions) { | ||
| expect(result.allowedFunctionNames).toBeDefined() | ||
| expect(result.allowedFunctionNames).not.toContain(tools.codebase_search) | ||
| expect(toolNames(result.tools)).toContain(tools.codebase_search) | ||
| } else { | ||
| expect(result.allowedFunctionNames).toBeUndefined() | ||
| expect(toolNames(result.tools)).not.toContain(tools.codebase_search) | ||
| } | ||
| for (const tool of ordinaryReadTools) { | ||
| expect(callable(result)).toContain(tool) | ||
| } | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.