From 0d332ee9a565c9d903ec5e9a894d6c8b19be3fcf Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Tue, 22 Sep 2026 09:31:06 +0200 Subject: [PATCH 1/5] fix: exempt attempt_completion from disabledTools A disabledTools entry naming attempt_completion removed the completion tool from prompt generation and API declarations, and runtime validation rejected every call, so a task could never finish. The effective tool policy now partitions such entries out of the user's disabledTools list once, at policy entry, before any filtering step, and surfaces the ignored entries as a single in-task notice per new task. Prompt generation, API declarations, and runtime validation all derive from the same resolved policy, so they cannot disagree about which tools are callable. Stored configuration is left byte-untouched. The other always-available tools remain blockable, and a model-profile excludedTools entry still strips attempt_completion. Code comments that stated the old precedence are corrected (comment-only). Issue: #1640 --- packages/types/src/global-settings.ts | 5 +- packages/types/src/message.ts | 2 + ...resentAssistantMessage-custom-tool.spec.ts | 23 +- .../presentAssistantMessage.ts | 7 +- .../__tests__/effective-tool-policy.spec.ts | 126 +++++++++-- .../__tests__/filter-tools-for-mode.spec.ts | 18 ++ .../prompts/tools/effective-tool-policy.ts | 85 ++++++-- .../prompts/tools/filter-tools-for-mode.ts | 5 +- src/core/task/Task.ts | 21 ++ ...Task.ignored-disabled-tools-notice.spec.ts | 206 ++++++++++++++++++ src/core/task/__tests__/build-tools.spec.ts | 32 ++- webview-ui/src/components/chat/ChatRow.tsx | 12 + .../IgnoredDisabledToolsNotice.spec.tsx | 60 +++++ webview-ui/src/i18n/locales/ca/chat.json | 4 + webview-ui/src/i18n/locales/de/chat.json | 4 + webview-ui/src/i18n/locales/en/chat.json | 4 + webview-ui/src/i18n/locales/es/chat.json | 4 + webview-ui/src/i18n/locales/fr/chat.json | 4 + webview-ui/src/i18n/locales/hi/chat.json | 4 + webview-ui/src/i18n/locales/id/chat.json | 4 + webview-ui/src/i18n/locales/it/chat.json | 4 + webview-ui/src/i18n/locales/ja/chat.json | 4 + webview-ui/src/i18n/locales/ko/chat.json | 4 + webview-ui/src/i18n/locales/nl/chat.json | 4 + webview-ui/src/i18n/locales/pl/chat.json | 4 + webview-ui/src/i18n/locales/pt-BR/chat.json | 4 + webview-ui/src/i18n/locales/ru/chat.json | 4 + webview-ui/src/i18n/locales/tr/chat.json | 4 + webview-ui/src/i18n/locales/vi/chat.json | 4 + webview-ui/src/i18n/locales/zh-CN/chat.json | 4 + webview-ui/src/i18n/locales/zh-TW/chat.json | 4 + 31 files changed, 598 insertions(+), 76 deletions(-) create mode 100644 src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts create mode 100644 webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index 692798d00d..12e1c90726 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -285,7 +285,10 @@ export const globalSettingsSchema = z.object({ /** * List of native tool names to globally disable. - * Tools in this list will be excluded from prompt generation and rejected at execution time. + * Tools in this list are excluded from prompt generation and rejected at + * execution time. The tool the task loop completes through + * (attempt_completion) cannot be disabled this way: an entry naming it is + * ignored at policy resolution. */ disabledTools: z.array(toolNamesSchema).optional(), }) diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index 01d7962266..47cc1ec796 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -140,6 +140,7 @@ export function isNonBlockingAsk(ask: ClineAsk): ask is NonBlockingAsk { * - `condense_context_error`: Error occurred during context condensation * - `codebase_search_result`: Results from searching the codebase * - `too_many_tools_warning`: Warning that too many MCP tools are enabled, which may confuse the LLM + * - `ignored_disabled_tools_warning`: Warning that a `disabledTools` entry names a protocol tool, which user settings cannot disable */ export const clineSays = [ "error", @@ -170,6 +171,7 @@ export const clineSays = [ "codebase_search_result", "user_edit_todos", "too_many_tools_warning", + "ignored_disabled_tools_warning", "tool", ] as const diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts index aa278e077d..e4b4839ccb 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts @@ -420,11 +420,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { }) }) - it("marks a disabled attempt_completion as blocked and answers it with an error tool_result", async () => { - // An explicit disabledTools entry outranks the always-available class, - // so a disabled attempt_completion reaches the validator like any - // other tool; its rejection must surface as the standard validation- - // error tool_result instead of completing the task. + it("ignores a disabled attempt_completion entry and executes the completion normally", async () => { mockTask.assistantMessageContent = [ { type: "tool_use", @@ -449,29 +445,24 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { }), } - // Mirror the real validator's rejection for a requirement that maps - // to false (validateToolUse.spec pins the predicate itself). - vi.mocked(validateToolUse).mockImplementationOnce(() => { - throw new Error('Tool "attempt_completion" is not allowed in code mode.') - }) - await presentAssistantMessage(mockTask) const validateToolUseMock = vi.mocked(validateToolUse) expect(validateToolUseMock).toHaveBeenCalled() const toolRequirements = validateToolUseMock.mock.calls[0][3] - expect(toolRequirements).toMatchObject({ attempt_completion: false }) + // Absent, not merely un-false: the validator is never told the + // completion tool was disabled. + expect(toolRequirements).not.toHaveProperty("attempt_completion") const errorToolResults = mockTask.userMessageContent.filter((block: unknown) => { const b = block as { type?: string; is_error?: boolean } return b.type === "tool_result" && b.is_error }) - expect(errorToolResults).toHaveLength(1) - expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(errorToolResults).toHaveLength(0) + expect(mockTask.consecutiveMistakeCount).toBe(0) - // The completion handler must not run for the rejected call. const { attemptCompletionTool } = await import("../../tools/AttemptCompletionTool") - expect(attemptCompletionTool.handle).not.toHaveBeenCalled() + expect(attemptCompletionTool.handle).toHaveBeenCalled() }) it("treats a model-excluded attempt_completion as blocked and answers it with an error tool_result", async () => { diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 546c43c06c..6da178f333 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -608,9 +608,10 @@ export async function presentAssistantMessage(cline: Task) { const isCustomTool = Boolean(stateExperiments?.customTools && customToolRegistry.has(block.name)) try { - // Build requirements through the shared policy module so every suppressed - // entry — disabled tools, and an excluded or disabled protocol tool — reaches - // the validator, which checks them before the always-available class. See + // Build requirements through the shared policy module so every entry that + // carries disabling weight — non-protocol disabled tools, and every excluded + // tool, protocol entries included on that leg — reaches the validator, which + // checks them before the always-available class. See // `buildToolRequirements` in effective-tool-policy.ts. const toolRequirements = buildToolRequirements(disabledTools, modelInfo?.info) diff --git a/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts b/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts index 8e2c3cb80f..115141c489 100644 --- a/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts +++ b/src/core/prompts/tools/__tests__/effective-tool-policy.spec.ts @@ -8,6 +8,7 @@ import { resolveToolAlias, buildToolRequirements, isToolDisabledOrExcluded, + partitionDisabledToolsForProtocol, } from "../effective-tool-policy" import { getModeBySlug, defaultModeSlug } from "../../../../shared/modes" import type { CodeIndexManager } from "../../../../services/code-index/manager" @@ -124,12 +125,12 @@ describe("resolveEffectiveToolPolicy - disabledTools", () => { expect(policy.tools.has("write_to_file")).toBe(false) }) - it("removes a protocol tool listed in disabledTools", () => { + it("ignores a disabledTools entry naming a protocol tool", () => { expect( policyFor(["read", "edit", "command"], { disabledTools: [...PROTOCOL_TOOLS] }).tools.has( "attempt_completion", ), - ).toBe(false) + ).toBe(true) }) it("keeps the protocol tool when it is neither disabled nor excluded", () => { @@ -139,6 +140,61 @@ describe("resolveEffectiveToolPolicy - disabledTools", () => { ), ).toBe(true) }) + + it("retains the protocol tool even under an otherwise maximally restricted configuration", () => { + const policy = policyFor([], { disabledTools: ["attempt_completion", "read_file"], todoListEnabled: false }) + expect(policy.tools.has("attempt_completion")).toBe(true) + expect(policy.tools.has("read_file")).toBe(false) + expect(policy.tools.has("update_todo_list")).toBe(false) + }) + + it("still strips other entries that name the always-available tools", () => { + // The exemption covers only the protocol entry, never its always-available siblings. + const policy = policyFor(["read", "edit", "command"], { + disabledTools: ["attempt_completion", "ask_followup_question", "switch_mode", "execute_command"], + }) + expect(policy.tools.has("attempt_completion")).toBe(true) + expect(policy.tools.has("ask_followup_question")).toBe(false) + expect(policy.tools.has("switch_mode")).toBe(false) + expect(policy.tools.has("execute_command")).toBe(false) + }) + + it("still strips the protocol tool when only excludedTools names it, beside unrelated disables", () => { + const policy = policyFor(["read", "edit", "command"], { + disabledTools: ["attempt_completion", "execute_command"], + modelInfo: modelInfo({ excludedTools: ["attempt_completion"] }), + }) + expect(policy.tools.has("attempt_completion")).toBe(false) + expect(policy.tools.has("execute_command")).toBe(false) + }) + + it("still strips the protocol tool when both lists name it", () => { + // With a user disable and a model exclusion naming the same tool, the + // model exclusion still suppresses it in the effective policy. + const policy = policyFor(["read", "edit", "command"], { + disabledTools: ["attempt_completion"], + modelInfo: modelInfo({ excludedTools: ["attempt_completion"] }), + }) + expect(policy.tools.has("attempt_completion")).toBe(false) + }) + + it("keeps prompt advertisement and the execution gate agreeing about the protocol tool", () => { + // Agreement by construction: for every combination of the two lists + // naming the completion tool, the policy advertises it exactly when the + // runtime requirements do not reject it. + const cases: Array<[string[] | undefined, string[] | undefined]> = [ + [undefined, undefined], + [["attempt_completion"], undefined], + [undefined, ["attempt_completion"]], + [["attempt_completion"], ["attempt_completion"]], + ] + for (const [disabledTools, excludedTools] of cases) { + const model = modelInfo(excludedTools ? { excludedTools } : undefined) + const policy = policyFor(["read", "edit", "command"], { disabledTools, modelInfo: model }) + const requirements = buildToolRequirements(disabledTools, model) + expect(policy.tools.has("attempt_completion")).toBe(requirements.attempt_completion !== false) + } + }) }) describe("resolveEffectiveToolPolicy - model customization", () => { @@ -358,9 +414,9 @@ describe("buildToolRequirements", () => { expect(reqs).toEqual({ write_file: false, write_to_file: false }) }) - it("maps a disabled protocol tool to false like any other tool", () => { + it("omits a disabled protocol tool while mapping the other entries", () => { const reqs = buildToolRequirements([...PROTOCOL_TOOLS, "ask_followup_question", "switch_mode"]) - expect(reqs).toEqual({ attempt_completion: false, ask_followup_question: false, switch_mode: false }) + expect(reqs).toEqual({ ask_followup_question: false, switch_mode: false }) }) it("adds alias + canonical for real aliases", () => { @@ -368,19 +424,16 @@ describe("buildToolRequirements", () => { expect(Object.keys(reqs).sort()).toEqual(["write_file", "write_to_file"].sort()) }) - it("keeps protocol-tool and regular entries together in a mixed list", () => { - // An explicit protocol-tool disable reaches the validator beside the - // regular tools in the same list. + it("drops the protocol-tool entry from a mixed list and keeps the regular ones", () => { expect(buildToolRequirements(["attempt_completion", "write_file"])).toEqual({ - attempt_completion: false, write_file: false, write_to_file: false, }) }) it("maps a protocol tool excluded by the model to false", () => { - // A model excludedTools entry suppresses attempt_completion just as a - // disabledTools entry does, so the execution gate sees it too. + // A model excludedTools entry suppresses attempt_completion at the + // execution gate; only the user disabledTools leg exempts it. const reqs = buildToolRequirements(undefined, modelInfo({ excludedTools: ["attempt_completion"] })) expect(reqs).toEqual({ attempt_completion: false }) }) @@ -393,11 +446,24 @@ describe("buildToolRequirements", () => { it("returns an empty map for a model customization without exclusions", () => { expect(buildToolRequirements(undefined, modelInfo())).toEqual({}) }) + + it("omits the protocol entry from disabledTools while keeping the model exclusion of the same tool", () => { + // An ignored user disable does not shield the tool from a model exclusion. + expect( + buildToolRequirements( + ["attempt_completion", "write_file"], + modelInfo({ excludedTools: ["attempt_completion"] }), + ), + ).toEqual({ + write_file: false, + write_to_file: false, + attempt_completion: false, + }) + }) }) describe("resolveToolAlias", () => { it("resolves every registered alias to its canonical tool", () => { - // Exercises the module-load ALIAS_TO_CANONICAL map for both registered aliases. expect(resolveToolAlias("write_file")).toBe("write_to_file") expect(resolveToolAlias("search_and_replace")).toBe("edit") }) @@ -422,6 +488,21 @@ describe("PROTOCOL_TOOLS", () => { it("lists the single protocol tool by canonical name", () => { expect([...PROTOCOL_TOOLS]).toEqual(["attempt_completion"]) }) + + it("partitions disabledTools entries naming a protocol tool into ignored", () => { + expect(partitionDisabledToolsForProtocol(undefined)).toEqual({ effective: [], ignored: [] }) + expect(partitionDisabledToolsForProtocol([])).toEqual({ effective: [], ignored: [] }) + expect(partitionDisabledToolsForProtocol(["execute_command", "attempt_completion", "web_fetch"])).toEqual({ + effective: ["execute_command", "web_fetch"], + ignored: ["attempt_completion"], + }) + }) + + it("de-duplicates repeated protocol-tool entries in the ignored list", () => { + expect( + partitionDisabledToolsForProtocol(["attempt_completion", "attempt_completion", "execute_command"]), + ).toEqual({ effective: ["execute_command"], ignored: ["attempt_completion"] }) + }) }) describe("resolveEffectiveToolPolicy - edit restriction edge cases", () => { @@ -662,20 +743,21 @@ describe("resolveEffectiveToolPolicy - MCP capability flags", () => { }) describe("resolveEffectiveToolPolicy - protocol tool honoring (fresh module)", () => { - // A disabled/excluded protocol tool must stay out of the effective set even - // when aliased: the re-add consults the same alias-resolved predicate as the - // exclusion steps, so an alias in disabledTools suppresses the canonical tool. + // The exemption consults the same alias-resolved names as the exclusion + // steps, so an alias of a protocol tool in disabledTools is ignored just + // like the canonical name, while a model excludedTools entry still + // suppresses it. async function freshResolve() { vi.resetModules() const mod = await import("../effective-tool-policy") return mod.resolveEffectiveToolPolicy } - it("suppresses the protocol tool when disabledTools lists an alias of it", async () => { + it("ignores a disabledTools alias of the protocol tool", async () => { // Reset first, then register a temporary alias of attempt_completion, and // only then load a fresh resolver: its module-load alias map (and with it - // the re-add gate) is built from the shared alias table as it stands at - // import time, so the suppression becomes reachable only through alias + // the exemption) is built from the shared alias table as it stands at + // import time, so the ignored entry is reachable only through alias // resolution, not a literal name match. vi.resetModules() const toolsMod = await import("../../../../shared/tools") @@ -686,7 +768,7 @@ describe("resolveEffectiveToolPolicy - protocol tool honoring (fresh module)", ( mod .resolveEffectiveToolPolicy({ mode: "code", disabledTools: ["wp4_attempt_alias"] }) .tools.has("attempt_completion"), - ).toBe(false) + ).toBe(true) // Sanity: the injected alias actually resolves through the fresh module. expect(mod.resolveToolAlias("wp4_attempt_alias")).toBe("attempt_completion") } finally { @@ -702,10 +784,10 @@ describe("resolveEffectiveToolPolicy - protocol tool honoring (fresh module)", ( }) it("pins the protocol list and re-adds an unlisted tool independently of the always-available roster", async () => { - // Two positive controls for the suppression test above, on a fresh module: - // the exported protocol list is pinned, and with attempt_completion - // stripped from the always-available roster the unlisted tool must STILL - // be callable — so the re-add step, not the roster, is what guarantees it. + // Two positive controls on a fresh module: the exported protocol list is + // pinned, and with attempt_completion stripped from the always-available + // roster the tool must STILL be callable — so the re-add step, not the + // roster, is what guarantees it. vi.resetModules() const toolsMod = await import("../../../../shared/tools") const mod = await import("../effective-tool-policy") diff --git a/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts b/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts index 11198caff9..3332b82c22 100644 --- a/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts +++ b/src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts @@ -89,6 +89,24 @@ describe("filterNativeToolsForMode - disabledTools", () => { expect(resultNames).not.toContain("search_and_replace") expect(resultNames).not.toContain("edit") }) + + it("keeps the attempt_completion declaration when disabledTools lists it under a maximally restricted mode", () => { + const restrictedMode: ModeConfig = { + slug: "control-only", + name: "Control Only", + roleDefinition: "", + groups: [], + } + const tools = [makeTool("read_file"), makeTool("attempt_completion")] + + const result = filterNativeToolsForMode(tools, "control-only", [restrictedMode], undefined, undefined, { + disabledTools: ["attempt_completion", "read_file"], + }) + + const names = result.map((t) => ("function" in t && t.function ? t.function.name : "")) + expect(names).toContain("attempt_completion") + expect(names).not.toContain("read_file") + }) }) describe("filterNativeToolsForMode - settings round-trips", () => { diff --git a/src/core/prompts/tools/effective-tool-policy.ts b/src/core/prompts/tools/effective-tool-policy.ts index 534f646882..1796eb9441 100644 --- a/src/core/prompts/tools/effective-tool-policy.ts +++ b/src/core/prompts/tools/effective-tool-policy.ts @@ -16,11 +16,12 @@ type EffectiveMcpHub = { * Canonical tool names that participate in the task-completion protocol. * * The effective tool policy re-adds these after the mode/permission filters, so a - * mode that grants no groups still advertises them — but a `disabledTools` entry - * or a model `excludedTools` entry takes precedence: honoring an explicit - * restriction takes priority over the re-add, and the runtime validator rejects - * execution of a tool so restricted (see `buildToolRequirements` and the - * requirements-before-always-available precedence in `validateToolUse.ts`). + * mode that grants no groups still advertises them. A user `disabledTools` entry + * cannot suppress them: such entries are ignored at policy entry (see + * `partitionDisabledToolsForProtocol`), because the task loop can only exit + * through the completion tool and configuration must not be able to close that + * route. A model-profile `excludedTools` entry still suppresses the re-add and + * reaches the runtime requirements, so prompt and validator agree on it too. * * `attempt_completion` is the only tool with no coherent prompt state when absent * (the task loop can only exit through it), so it is the sole protocol entry. @@ -110,6 +111,37 @@ export function isToolDisabledOrExcluded( return Boolean(disabledTools?.some(isSuppressed)) || Boolean(modelInfo?.excludedTools?.some(isSuppressed)) } +/** + * Partitions a raw user `disabledTools` list into entries that carry disabling + * weight and entries that name a protocol tool. + * + * Protocol tools are exempt from user disabling: the task loop exits only + * through `attempt_completion`, so a configuration entry must not be able to + * close that route. `effective` is what every suppression consumer must act + * on, while `ignored` surfaces the stripped entries so callers can report + * what was ignored instead of re-deriving the predicate elsewhere. Entries + * are matched after alias resolution. + * + * @param disabledTools The user's disabled-tools list (may contain aliases). + * @returns Order-preserving `effective` entries, plus the de-duplicated + * `ignored` entries that name a protocol tool. + */ +export function partitionDisabledToolsForProtocol(disabledTools: string[] | undefined): { + effective: string[] + ignored: string[] +} { + const effective: string[] = [] + const ignored: string[] = [] + for (const entry of disabledTools ?? []) { + if (PROTOCOL_TOOLS.includes(resolveToolAlias(entry))) { + ignored.push(entry) + } else { + effective.push(entry) + } + } + return { effective, ignored: [...new Set(ignored)] } +} + export interface EffectiveToolPolicyInput { mode: string customModes?: ModeConfig[] @@ -198,8 +230,9 @@ function hasAnyMcpResources(mcpHub: EffectiveMcpHub, allowedServers?: string[]): * * This is the single source of truth shared by prompt generation, API tool * construction, runtime validation, and preview. The numbered steps below (1-10) - * compute the allowed tool set; step 11 re-adds `PROTOCOL_TOOLS` unless an - * explicit disable/exclude suppresses them. + * compute the allowed tool set; step 11 re-adds `PROTOCOL_TOOLS` unless the + * model's `excludedTools` suppresses them — `disabledTools` entries naming a + * protocol tool are partitioned out at entry and carry no disabling weight. * * The returned policy is deterministic for a given input and free of side * effects. @@ -221,6 +254,10 @@ export function resolveEffectiveToolPolicy(input: EffectiveToolPolicyInput): Eff allowedMcpServers, } = input + // A disabledTools entry naming a protocol tool carries no disabling weight; + // partition it out once at entry so no later step can act on it. + const { effective: effectiveDisabledTools } = partitionDisabledToolsForProtocol(disabledTools) + // 1. Resolve mode config with default-slug fallback (existing behavior). const modeSlug = mode ?? defaultModeSlug const modeConfig = getModeBySlug(modeSlug, customModes) || getModeBySlug(defaultModeSlug, customModes)! @@ -290,9 +327,9 @@ export function resolveEffectiveToolPolicy(input: EffectiveToolPolicyInput): Eff allowedToolNames.delete("run_slash_command") } - // 9. Drop disabledTools entries (alias-resolved). - if (disabledTools?.length) { - for (const toolName of disabledTools) { + // 9. Drop effective disabledTools entries (alias-resolved). + if (effectiveDisabledTools.length) { + for (const toolName of effectiveDisabledTools) { allowedToolNames.delete(resolveToolAlias(toolName)) } } @@ -314,15 +351,14 @@ export function resolveEffectiveToolPolicy(input: EffectiveToolPolicyInput): Eff allowedToolNames.delete("use_mcp_tool") } - // 11. Protocol guarantee: re-add every protocol tool that neither the user's - // disabledTools nor the model's excludedTools suppresses, so the logical - // set and the runtime validator agree in both directions: an unlisted - // protocol tool stays callable — this re-add, not the always-available - // roster, is what guarantees it — while a suppressed one stays out of the - // prompt, the declarations, and (via buildToolRequirements) execution, - // having been removed by steps 4 and 9. + // 11. Protocol guarantee: re-add every protocol tool that the effective + // disabledTools list and the model's excludedTools do not suppress. + // After the entry partition only an excludedTools entry can suppress + // one, and `buildToolRequirements` partitions the same entries out of + // the runtime requirements, so the logical set and the validator + // cannot disagree about the completion tool in either direction. for (const tool of PROTOCOL_TOOLS) { - if (!isToolDisabledOrExcluded(tool, disabledTools, modelInfo)) { + if (!isToolDisabledOrExcluded(tool, effectiveDisabledTools, modelInfo)) { allowedToolNames.add(resolveToolAlias(tool)) } } @@ -340,10 +376,14 @@ export function resolveEffectiveToolPolicy(input: EffectiveToolPolicyInput): Eff /** * Builds the runtime `toolRequirements` map (tool name → false) from every entry - * in the user and model exclusion lists. + * that carries disabling weight: the user's `disabledTools` minus protocol-tool + * entries, plus every model `excludedTools` entry. + * * A requirements entry outranks the always-available class in `validateToolUse`, - * so every disabled or model-excluded tool is rejected at execution with the - * standard validation error tool_result, matching its removal from the policy. + * so every tool mapped here is rejected at execution with the standard + * validation error tool_result, matching its removal from the policy. A + * `disabledTools` entry naming a protocol tool is deliberately absent — the tool + * stays callable, matching its retention in the policy. * * @param disabledTools The raw disabled-tools list (may contain aliases). * @param modelInfo The model customization whose `excludedTools` may suppress a @@ -352,7 +392,8 @@ export function resolveEffectiveToolPolicy(input: EffectiveToolPolicyInput): Eff */ export function buildToolRequirements(disabledTools?: string[], modelInfo?: ModelInfo): Record { const requirements: Record = {} - for (const toolName of [...(disabledTools ?? []), ...(modelInfo?.excludedTools ?? [])]) { + const { effective } = partitionDisabledToolsForProtocol(disabledTools) + for (const toolName of [...effective, ...(modelInfo?.excludedTools ?? [])]) { const canonical = resolveToolAlias(toolName) requirements[toolName] = false requirements[canonical] = false diff --git a/src/core/prompts/tools/filter-tools-for-mode.ts b/src/core/prompts/tools/filter-tools-for-mode.ts index 45ccb39c5d..bdb9408fdc 100644 --- a/src/core/prompts/tools/filter-tools-for-mode.ts +++ b/src/core/prompts/tools/filter-tools-for-mode.ts @@ -79,8 +79,9 @@ export function filterNativeToolsForMode( // Resolve the single, request-scoped effective tool policy. The filter below // consumes only its `tools` set (plus alias renames from model customization), // so prompt generation and API tool construction agree on the logical allowed - // set — including the protocol-tool rule: unlisted, attempt_completion is - // advertised; listed in disabledTools/excludedTools, it is not. + // set — including the protocol-tool rule: a disabledTools entry naming + // attempt_completion is ignored, while a model-profile excludedTools entry + // keeps it out. const modelInfo = settings?.modelInfo as ModelInfo | undefined const policy = resolveEffectiveToolPolicy({ diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 55798437c3..3c2b90e9f9 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -97,6 +97,7 @@ import { getTaskDirectoryPath } from "../../utils/storage" // prompts import { formatResponse } from "../prompts/responses" import { SYSTEM_PROMPT } from "../prompts/system" +import { partitionDisabledToolsForProtocol } from "../prompts/tools/effective-tool-policy" import { buildNativeToolsArrayWithRestrictions } from "./build-tools" // core modules @@ -2263,6 +2264,26 @@ export class Task extends EventEmitter implements TaskLike { { isNonInteractive: true }, ) } + + // Surface once per new task which `disabledTools` entries carry no + // disabling weight (protocol tools are exempt from user disabling). + // Detection reads the raw list, so the notice also fires when a + // model-profile `excludedTools` entry suppresses the tool through a + // separate route. `startTask` runs exactly once per new task, which + // is what bounds this to a single notice. + const disabledToolsState = await this.providerRef.deref()?.getState() + const { ignored } = partitionDisabledToolsForProtocol(disabledToolsState?.disabledTools) + if (ignored.length > 0) { + await this.say( + "ignored_disabled_tools_warning", + JSON.stringify({ ignoredTools: ignored }), + undefined, + undefined, + undefined, + undefined, + { isNonInteractive: true }, + ) + } this.isInitialized = true const imageBlocks: Anthropic.ImageBlockParam[] = formatResponse.imageBlocks(images) diff --git a/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts b/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts new file mode 100644 index 0000000000..60eaae08c8 --- /dev/null +++ b/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts @@ -0,0 +1,206 @@ +// npx vitest run core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts + +import * as vscode from "vscode" + +import type { ModelInfo, ProviderSettings } from "@roo-code/types" +import { providerIdentifiers } from "@roo-code/types/provider-identifiers" + +import { Task } from "../Task" +import { ClineProvider } from "../../webview/ClineProvider" + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + hasInstance: vi.fn().mockReturnValue(true), + createInstance: vi.fn(), + get instance() { + return { + captureTaskCreated: vi.fn(), + captureTaskRestarted: vi.fn(), + captureModeSwitch: vi.fn(), + captureConversationMessage: vi.fn(), + captureLlmCompletion: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + captureCodeActionUsed: vi.fn(), + setProvider: vi.fn(), + } + }, + }, +})) + +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + const mockEventEmitter = { event: vi.fn(), fire: vi.fn() } + const mockTextDocument = { uri: { fsPath: "/mock/workspace/path/file.ts" } } + const mockTextEditor = { document: mockTextDocument } + const mockTab = { input: { uri: { fsPath: "/mock/workspace/path/file.ts" } } } + const mockTabGroup = { tabs: [mockTab] } + + return { + TabInputTextDiff: vi.fn(), + CodeActionKind: { + QuickFix: { value: "quickfix" }, + RefactorRewrite: { value: "refactor.rewrite" }, + }, + window: { + createTextEditorDecorationType: vi.fn().mockReturnValue({ + dispose: vi.fn(), + }), + visibleTextEditors: [mockTextEditor], + tabGroups: { + all: [mockTabGroup], + close: vi.fn(), + onDidChangeTabs: vi.fn(() => ({ dispose: vi.fn() })), + }, + showErrorMessage: vi.fn(), + }, + workspace: { + getConfiguration: vi.fn(() => ({ get: (_k: string, d: unknown) => d })), + workspaceFolders: [ + { + uri: { fsPath: "/mock/workspace/path" }, + name: "mock-workspace", + index: 0, + }, + ], + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), + fs: { + stat: vi.fn().mockResolvedValue({ type: 1 }), + }, + onDidSaveTextDocument: vi.fn(() => mockDisposable), + }, + env: { + uriScheme: "vscode", + language: "en", + }, + EventEmitter: vi.fn().mockImplementation(function () { + return mockEventEmitter + }), + Disposable: { + from: vi.fn(), + }, + TabInputText: vi.fn(), + version: "1.85.0", + } +}) + +vi.mock("../../environment/getEnvironmentDetails", () => ({ + getEnvironmentDetails: vi.fn().mockResolvedValue(""), +})) + +vi.mock("../../ignore/RooIgnoreController") + +vi.mock("p-wait-for", () => ({ + default: vi.fn().mockImplementation(async () => Promise.resolve()), +})) + +vi.mock("delay", () => ({ + __esModule: true, + default: vi.fn().mockResolvedValue(undefined), +})) + +type StartTaskAccess = { + startTask: (task?: string, images?: string[]) => Promise + getEnabledMcpToolsCount: () => Promise<{ enabledToolCount: number; enabledServerCount: number }> + initiateTaskLoop: (userContent: unknown[]) => Promise +} + +// The private-member double assertion is the established Task-spec pattern +// (see Task.spec.ts getTaskTestAccess): the spies below must reach private +// startup collaborators without widening their production visibility. +function getStartTaskAccess(task: Task): StartTaskAccess { + return task as unknown as StartTaskAccess +} + +type ProviderStateShaped = Partial>> & { + disabledTools?: string[] +} + +async function startTaskWithDisabledTools(disabledTools: string[] | undefined, profileExcludedTools?: string[]) { + const providerState = { disabledTools } as ProviderStateShaped + const mockProvider = { + context: { + globalStorageUri: { fsPath: "/test/storage" }, + }, + getState: vi.fn().mockResolvedValue(providerState), + log: vi.fn(), + on: vi.fn(), + off: vi.fn(), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + // ClineProvider's surface is too broad to type this fixture fully; double assertion is the last resort (AGENTS.md) — only task-scoped fields are read. + } as unknown as ClineProvider + + const apiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + + const task = new Task({ + provider: mockProvider, + apiConfiguration, + task: "example task", + startTask: false, + }) + + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + // The model-profile suppression route is fed through the model info, never + // the provider state, so the fixture can vary it independently of the list. + const modelInfo: ModelInfo = { + contextWindow: 50_000, + maxTokens: 1024, + supportsPromptCache: false, + excludedTools: profileExcludedTools, + } + vi.spyOn(task.api, "getModel").mockReturnValue({ + id: "claude-3-5-sonnet-20241022", + info: modelInfo, + }) + const taskAccess = getStartTaskAccess(task) + vi.spyOn(taskAccess, "getEnabledMcpToolsCount").mockResolvedValue({ + enabledToolCount: 0, + enabledServerCount: 0, + }) + vi.spyOn(taskAccess, "initiateTaskLoop").mockResolvedValue(undefined) + + await taskAccess.startTask("example task") + + return saySpy.mock.calls.filter(([type]) => type === "ignored_disabled_tools_warning") +} + +describe("Task - ignored disabled-tools notice", () => { + it("notifies exactly once when disabledTools lists a tool that cannot be disabled", async () => { + const noticeCalls = await startTaskWithDisabledTools(["execute_command", "attempt_completion"]) + + expect(noticeCalls).toHaveLength(1) + const [, text, , , , , options] = noticeCalls[0] + expect(JSON.parse(text as string)).toEqual({ ignoredTools: ["attempt_completion"] }) + expect(options).toEqual({ isNonInteractive: true }) + }) + + it("stays silent when the user list is absent or names only ordinary tools", async () => { + expect(await startTaskWithDisabledTools(undefined)).toHaveLength(0) + expect(await startTaskWithDisabledTools([])).toHaveLength(0) + expect(await startTaskWithDisabledTools(["execute_command", "read_file"])).toHaveLength(0) + }) + + it("stays silent when only the model profile excludes the completion tool", async () => { + const noticeCalls = await startTaskWithDisabledTools(["execute_command"], ["attempt_completion"]) + + expect(noticeCalls).toHaveLength(0) + }) + + it("still notifies when the completion tool is user-disabled and profile-excluded", async () => { + // The notice keys on the raw user list, not the effective policy. + const noticeCalls = await startTaskWithDisabledTools(["attempt_completion"], ["attempt_completion"]) + + expect(noticeCalls).toHaveLength(1) + expect(JSON.parse(noticeCalls[0][1] as string)).toEqual({ ignoredTools: ["attempt_completion"] }) + }) +}) diff --git a/src/core/task/__tests__/build-tools.spec.ts b/src/core/task/__tests__/build-tools.spec.ts index 65990932a2..9e9c397c1f 100644 --- a/src/core/task/__tests__/build-tools.spec.ts +++ b/src/core/task/__tests__/build-tools.spec.ts @@ -2,9 +2,10 @@ // // Gemini `includeAllToolsWithRestrictions` path: with the flag on, `tools` // contains ALL declarations while `allowedFunctionNames` is derived from the -// resolver-filtered set, so every `disabledTools`/`excludedTools` entry — -// protocol tools included — leaves the callable allowlist while the -// declarations stay advertised. +// resolver-filtered set, so every `disabledTools`/`excludedTools` entry leaves +// the callable allowlist while the declarations stay advertised — except a +// `disabledTools` entry naming a protocol tool, which the resolver ignores, so +// the completion tool stays callable. import type OpenAI from "openai" import type * as vscode from "vscode" @@ -65,7 +66,7 @@ function toolNames(tools: OpenAI.Chat.ChatCompletionTool[]): string[] { describe("buildNativeToolsArrayWithRestrictions — Gemini includeAllToolsWithRestrictions", () => { const provider = makeProvider() - it("sends all declarations but restricts allowedFunctionNames (protocol tool follows the allowlist once disabled)", async () => { + it("sends all declarations but restricts allowedFunctionNames (disabled protocol tool entry is ignored)", async () => { const result = await buildNativeToolsArrayWithRestrictions({ provider, cwd: "/test/path", @@ -77,20 +78,31 @@ describe("buildNativeToolsArrayWithRestrictions — Gemini includeAllToolsWithRe includeAllToolsWithRestrictions: true, }) - // All tools are still advertised (declarations), including the two - // disabled ones. expect(toolNames(result.tools)).toContain("execute_command") expect(toolNames(result.tools)).toContain("attempt_completion") - // The logical set (allowedFunctionNames) honors the policy for both: - // an explicit disable of a protocol tool leaves the callable allowlist - // just like any other tool. - expect(result.allowedFunctionNames).not.toContain("attempt_completion") + expect(result.allowedFunctionNames).toContain("attempt_completion") expect(result.allowedFunctionNames).not.toContain("execute_command") // Anchor: code mode still grants read_file, so the allowlist is populated. expect(result.allowedFunctionNames).toContain("read_file") }) + it("default path (flag omitted) keeps the protocol-tool declaration while omitting disabled regular tools", async () => { + const result = await buildNativeToolsArrayWithRestrictions({ + provider, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: {}, + apiConfiguration: undefined, + disabledTools: ["execute_command", "attempt_completion"], + }) + + expect(toolNames(result.tools)).toContain("attempt_completion") + expect(toolNames(result.tools)).not.toContain("execute_command") + expect(result.allowedFunctionNames).toBeUndefined() + }) + it("flows mode filtering through the resolver into allowedFunctionNames", async () => { const customModes: ModeConfig[] = [ { diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 952322084f..e90a7f56c7 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -1590,6 +1590,18 @@ export const ChatRowContent = ({ /> ) } + case "ignored_disabled_tools_warning": { + const ignoredData = safeJsonParse<{ ignoredTools: string[] }>(message.text || "{}") + if (!ignoredData?.ignoredTools) return null + return ( + + ) + } default: return ( <> diff --git a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx new file mode 100644 index 0000000000..cffba0ac4d --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx @@ -0,0 +1,60 @@ +import React from "react" + +import { renderWithExtensionState, screen } from "@/utils/test-utils" +import type { ClineMessage } from "@roo-code/types" + +import { ChatRowContent } from "../ChatRow" + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, options?: Record) => { + const map: Record = { + "chat:ignoredDisabledTools.title": "Disabled tool ignored", + "chat:ignoredDisabledTools.messageTemplate": + "The following tools cannot be disabled and will remain available: {{tools}}.", + } + const template = map[key] ?? key + if (!options) return template + return template.replace("{{tools}}", String(options.tools)) + }, + i18n: { + exists: () => false, + }, + }), + Trans: ({ children }: { children?: React.ReactNode }) => <>{children}, + initReactI18next: { type: "3rdParty", init: () => {} }, +})) + +function renderChatRow(message: ClineMessage) { + return renderWithExtensionState( + {}} + onSuggestionClick={() => {}} + onBatchFileResponse={() => {}} + onFollowUpUnmount={() => {}} + isFollowUpAnswered={false} + />, + ) +} + +describe("ChatRow - ignored disabled-tools notice", () => { + it("renders a warning row naming the ignored tool entry", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + text: JSON.stringify({ ignoredTools: ["attempt_completion"] }), + } + + renderChatRow(message) + + expect(screen.getByText("Disabled tool ignored")).toBeInTheDocument() + expect( + screen.getByText("The following tools cannot be disabled and will remain available: attempt_completion."), + ).toBeInTheDocument() + }) +}) diff --git a/webview-ui/src/i18n/locales/ca/chat.json b/webview-ui/src/i18n/locales/ca/chat.json index 53bc81a127..95ddc2fcd1 100644 --- a/webview-ui/src/i18n/locales/ca/chat.json +++ b/webview-ui/src/i18n/locales/ca/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Tens {{tools}} habilitades via {{servers}}. Un nombre tant alt pot confondre el model i portar a errors. Intenta mantenir-lo per sota de {{threshold}}.", "openMcpSettings": "Obrir configuració de MCP" }, + "ignoredDisabledTools": { + "title": "Eina desactivada ignorada", + "messageTemplate": "Les eines següents no es poden desactivar i continuaran disponibles: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/de/chat.json b/webview-ui/src/i18n/locales/de/chat.json index 3300742d0f..397d3f3997 100644 --- a/webview-ui/src/i18n/locales/de/chat.json +++ b/webview-ui/src/i18n/locales/de/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Du hast {{tools}} über {{servers}} aktiviert. Eine so hohe Anzahl kann das Modell verwirren und zu Fehlern führen. Versuche, es unter {{threshold}} zu halten.", "openMcpSettings": "MCP-Einstellungen öffnen" }, + "ignoredDisabledTools": { + "title": "Deaktiviertes Tool ignoriert", + "messageTemplate": "Die folgenden Tools können nicht deaktiviert werden und bleiben verfügbar: {{tools}}." + }, "readCommandOutput": { "title": "Zoo las Befehlsausgabe" }, diff --git a/webview-ui/src/i18n/locales/en/chat.json b/webview-ui/src/i18n/locales/en/chat.json index 21eedce9c9..080ba71916 100644 --- a/webview-ui/src/i18n/locales/en/chat.json +++ b/webview-ui/src/i18n/locales/en/chat.json @@ -476,6 +476,10 @@ "messageTemplate": "You have {{tools}} enabled via {{servers}}. Such a high number can confuse the model and lead to errors. Try to keep it below {{threshold}}.", "openMcpSettings": "Open MCP Settings" }, + "ignoredDisabledTools": { + "title": "Disabled tool ignored", + "messageTemplate": "The following tools cannot be disabled and will remain available: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/es/chat.json b/webview-ui/src/i18n/locales/es/chat.json index 66cac35673..ea0b368218 100644 --- a/webview-ui/src/i18n/locales/es/chat.json +++ b/webview-ui/src/i18n/locales/es/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Tienes {{tools}} habilitadas a través de {{servers}}. Un número tan alto puede confundir al modelo y llevar a errores. Intenta mantenerlo por debajo de {{threshold}}.", "openMcpSettings": "Abrir configuración de MCP" }, + "ignoredDisabledTools": { + "title": "Herramienta deshabilitada ignorada", + "messageTemplate": "Las siguientes herramientas no se pueden deshabilitar y seguirán disponibles: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/fr/chat.json b/webview-ui/src/i18n/locales/fr/chat.json index 2a599e03fc..b07a2f5a7c 100644 --- a/webview-ui/src/i18n/locales/fr/chat.json +++ b/webview-ui/src/i18n/locales/fr/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Tu as {{tools}} activés via {{servers}}. Un nombre aussi élevé peut confondre le modèle et entraîner des erreurs. Essaie de le maintenir en dessous de {{threshold}}.", "openMcpSettings": "Ouvrir les paramètres MCP" }, + "ignoredDisabledTools": { + "title": "Outil désactivé ignoré", + "messageTemplate": "Les outils suivants ne peuvent pas être désactivés et resteront disponibles : {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/hi/chat.json b/webview-ui/src/i18n/locales/hi/chat.json index fa9bb62592..a5e4f56232 100644 --- a/webview-ui/src/i18n/locales/hi/chat.json +++ b/webview-ui/src/i18n/locales/hi/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "आपके पास {{servers}} के माध्यम से {{tools}} सक्षम हैं। इतनी अधिक संख्या मॉडल को भ्रमित कर सकती है और त्रुटियों का कारण बन सकती है। इसे {{threshold}} से नीचे रखने का प्रयास करें।", "openMcpSettings": "MCP सेटिंग्स खोलें" }, + "ignoredDisabledTools": { + "title": "निष्क्रिय टूल अनदेखा किया गया", + "messageTemplate": "ये टूल निष्क्रिय नहीं किए जा सकते और उपलब्ध रहेंगे: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/id/chat.json b/webview-ui/src/i18n/locales/id/chat.json index 378314b3a5..48871e9479 100644 --- a/webview-ui/src/i18n/locales/id/chat.json +++ b/webview-ui/src/i18n/locales/id/chat.json @@ -494,6 +494,10 @@ "messageTemplate": "Anda memiliki {{tools}} diaktifkan melalui {{servers}}. Jumlah yang begitu besar dapat membingungkan model dan menyebabkan kesalahan. Cobalah untuk menjaganya di bawah {{threshold}}.", "openMcpSettings": "Buka Pengaturan MCP" }, + "ignoredDisabledTools": { + "title": "Tool nonaktif diabaikan", + "messageTemplate": "Tool berikut tidak dapat dinonaktifkan dan akan tetap tersedia: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/it/chat.json b/webview-ui/src/i18n/locales/it/chat.json index 2b307241ab..c51641be7d 100644 --- a/webview-ui/src/i18n/locales/it/chat.json +++ b/webview-ui/src/i18n/locales/it/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Hai {{tools}} abilitate via {{servers}}. Un numero così alto può confondere il modello e portare a errori. Prova a mantenerlo sotto {{threshold}}.", "openMcpSettings": "Apri impostazioni MCP" }, + "ignoredDisabledTools": { + "title": "Strumento disabilitato ignorato", + "messageTemplate": "I seguenti strumenti non possono essere disabilitati e rimarranno disponibili: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/ja/chat.json b/webview-ui/src/i18n/locales/ja/chat.json index 0e3d2a99d9..58a1d24ba2 100644 --- a/webview-ui/src/i18n/locales/ja/chat.json +++ b/webview-ui/src/i18n/locales/ja/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "{{servers}}経由で{{tools}}が有効になっています。このような高い数は、モデルを混乱させてエラーを引き起こす可能性があります。{{threshold}}以下に保つようにしてください。", "openMcpSettings": "MCP 設定を開く" }, + "ignoredDisabledTools": { + "title": "無効化されたツールは無視されました", + "messageTemplate": "次のツールは無効化できず、引き続き利用可能です: {{tools}}。" + }, "readCommandOutput": { "title": "Zooがコマンド出力を読み込みました" }, diff --git a/webview-ui/src/i18n/locales/ko/chat.json b/webview-ui/src/i18n/locales/ko/chat.json index 1ec6e8beb1..6c2c460c2a 100644 --- a/webview-ui/src/i18n/locales/ko/chat.json +++ b/webview-ui/src/i18n/locales/ko/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "{{servers}}를 통해 {{tools}}가 활성화되어 있습니다. 이렇게 많은 수의 도구는 모델을 혼동시키고 오류를 유발할 수 있습니다. {{threshold}} 이하로 유지하도록 노력하세요.", "openMcpSettings": "MCP 설정 열기" }, + "ignoredDisabledTools": { + "title": "비활성화된 도구 무시됨", + "messageTemplate": "다음 도구는 비활성화할 수 없으며 계속 사용할 수 있습니다: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/nl/chat.json b/webview-ui/src/i18n/locales/nl/chat.json index 293a0c8ab8..65dbdc2b75 100644 --- a/webview-ui/src/i18n/locales/nl/chat.json +++ b/webview-ui/src/i18n/locales/nl/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Je hebt {{tools}} ingeschakeld via {{servers}}. Zoveel tools kunnen het model verwarren en tot fouten leiden. Probeer dit onder {{threshold}} te houden.", "openMcpSettings": "MCP-instellingen openen" }, + "ignoredDisabledTools": { + "title": "Uitgeschakelde tool genegeerd", + "messageTemplate": "De volgende tools kunnen niet worden uitgeschakeld en blijven beschikbaar: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/pl/chat.json b/webview-ui/src/i18n/locales/pl/chat.json index efabde84ce..d3bf01ae30 100644 --- a/webview-ui/src/i18n/locales/pl/chat.json +++ b/webview-ui/src/i18n/locales/pl/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Masz {{tools}} włączonych przez {{servers}}. Taka duża liczba może zamieszać model i prowadzić do błędów. Staraj się, aby była poniżej {{threshold}}.", "openMcpSettings": "Otwórz ustawienia MCP" }, + "ignoredDisabledTools": { + "title": "Zignorowano wyłączane narzędzie", + "messageTemplate": "Następujących narzędzi nie można wyłączyć i pozostaną dostępne: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/pt-BR/chat.json b/webview-ui/src/i18n/locales/pt-BR/chat.json index b88e869ad7..16e02ddd02 100644 --- a/webview-ui/src/i18n/locales/pt-BR/chat.json +++ b/webview-ui/src/i18n/locales/pt-BR/chat.json @@ -488,6 +488,10 @@ "messageTemplate": "Você tem {{tools}} habilitadas via {{servers}}. Um número tão alto pode confundir o modelo e levar a erros. Tente mantê-lo abaixo de {{threshold}}.", "openMcpSettings": "Abrir Configurações MCP" }, + "ignoredDisabledTools": { + "title": "Ferramenta desativada ignorada", + "messageTemplate": "As seguintes ferramentas não podem ser desativadas e permanecerão disponíveis: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/ru/chat.json b/webview-ui/src/i18n/locales/ru/chat.json index 93f61926e7..7f7164cf43 100644 --- a/webview-ui/src/i18n/locales/ru/chat.json +++ b/webview-ui/src/i18n/locales/ru/chat.json @@ -489,6 +489,10 @@ "messageTemplate": "У тебя включено {{tools}} через {{servers}}. Такое большое количество может сбить модель с толку и привести к ошибкам. Постарайся держать это ниже {{threshold}}.", "openMcpSettings": "Открыть настройки MCP" }, + "ignoredDisabledTools": { + "title": "Отключённый инструмент проигнорирован", + "messageTemplate": "Следующие инструменты нельзя отключить, и они останутся доступными: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/tr/chat.json b/webview-ui/src/i18n/locales/tr/chat.json index 708a602213..deb7ca2fc2 100644 --- a/webview-ui/src/i18n/locales/tr/chat.json +++ b/webview-ui/src/i18n/locales/tr/chat.json @@ -489,6 +489,10 @@ "messageTemplate": "{{servers}} üzerinden {{tools}} etkinleştirilmiş durumda. Bu kadar fazlası modeli kafası karışabilir ve hatalara neden olabilir. {{threshold}} altında tutmaya çalış.", "openMcpSettings": "MCP Ayarlarını Aç" }, + "ignoredDisabledTools": { + "title": "Devre dışı bırakılan araç yok sayıldı", + "messageTemplate": "Aşağıdaki araçlar devre dışı bırakılamaz ve kullanılabilir kalır: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/vi/chat.json b/webview-ui/src/i18n/locales/vi/chat.json index 50db8f8e22..fcb1c5df37 100644 --- a/webview-ui/src/i18n/locales/vi/chat.json +++ b/webview-ui/src/i18n/locales/vi/chat.json @@ -489,6 +489,10 @@ "messageTemplate": "Bạn đã bật {{tools}} qua {{servers}}. Số lượng lớn như vậy có thể khiến mô hình bối rối và dẫn đến lỗi. Cố gắng giữ nó dưới {{threshold}}.", "openMcpSettings": "Mở cài đặt MCP" }, + "ignoredDisabledTools": { + "title": "Đã bỏ qua công cụ bị tắt", + "messageTemplate": "Các công cụ sau không thể bị tắt và sẽ vẫn khả dụng: {{tools}}." + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/zh-CN/chat.json b/webview-ui/src/i18n/locales/zh-CN/chat.json index 8aa277b843..6740b025e6 100644 --- a/webview-ui/src/i18n/locales/zh-CN/chat.json +++ b/webview-ui/src/i18n/locales/zh-CN/chat.json @@ -489,6 +489,10 @@ "messageTemplate": "你通过 {{servers}} 启用了 {{tools}}。这么多数量会混淆模型并导致错误。建议将其保持在 {{threshold}} 以下。", "openMcpSettings": "打开 MCP 设置" }, + "ignoredDisabledTools": { + "title": "已忽略被禁用的工具", + "messageTemplate": "以下工具无法被禁用,将继续可用:{{tools}}。" + }, "readCommandOutput": { "title": "Zoo read command output" }, diff --git a/webview-ui/src/i18n/locales/zh-TW/chat.json b/webview-ui/src/i18n/locales/zh-TW/chat.json index 284fe1ad18..a6299f97c8 100644 --- a/webview-ui/src/i18n/locales/zh-TW/chat.json +++ b/webview-ui/src/i18n/locales/zh-TW/chat.json @@ -479,6 +479,10 @@ "messageTemplate": "您已啟用 {{tools}}(透過 {{servers}})。這麼多的工具可能會混淆模型並導致錯誤。請嘗試保持在 {{threshold}} 以下。", "openMcpSettings": "開啟 MCP 設定" }, + "ignoredDisabledTools": { + "title": "已忽略被停用的工具", + "messageTemplate": "以下工具無法被停用,將繼續可用:{{tools}}。" + }, "readCommandOutput": { "title": "Zoo read command output" }, From e57573769956dc4e5663290e8fa121238e0ce29c Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Tue, 22 Sep 2026 09:56:47 +0200 Subject: [PATCH 2/5] test: cover missing- and corrupt-text branches of the disabled-tools notice The patch-coverage check flagged two untested branches in the ChatRow case that renders the ignored disabled-tools notice: the row renders nothing when the message carries no text, and when the stored payload is not valid JSON. Add spec cases for both so every render path of the notice is covered. --- .../IgnoredDisabledToolsNotice.spec.tsx | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx index cffba0ac4d..3124dc71c4 100644 --- a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx @@ -57,4 +57,31 @@ describe("ChatRow - ignored disabled-tools notice", () => { screen.getByText("The following tools cannot be disabled and will remain available: attempt_completion."), ).toBeInTheDocument() }) + + it("renders nothing when the notice message text is missing", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + } + + const { container } = renderChatRow(message) + + expect(container.firstChild).toBeNull() + expect(screen.queryByText("Disabled tool ignored")).not.toBeInTheDocument() + }) + + it("renders nothing when the notice payload is not valid JSON", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + text: "{not valid json", + } + + const { container } = renderChatRow(message) + + expect(container.firstChild).toBeNull() + expect(screen.queryByText("Disabled tool ignored")).not.toBeInTheDocument() + }) }) From ade659df89dfc4ca8aecabca70000343546223cc Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Tue, 22 Sep 2026 17:44:47 +0200 Subject: [PATCH 3/5] Strengthen exemption tests and guard the ignored-tools notice render Follow-up to PR review feedback on the attempt_completion exemption. presentAssistantMessage-custom-tool.spec.ts asserted only that attemptCompletionTool.handle was called for a disabled attempt_completion entry. The assertion now also pins the arguments: the task, the attempt_completion tool-use block, and the callback object including its toolCallId. ChatRow rendered the ignored disabled-tools warning by calling join on the parsed ignoredTools field after only a truthy check, so a corrupted persisted payload (a string, or an array containing non-strings) threw during render. The field is now treated as unknown and the row renders nothing unless it is a non-empty array of strings. Three regression tests in IgnoredDisabledToolsNotice.spec.tsx cover those payload shapes. The translation mock in IgnoredDisabledToolsNotice.spec.tsx now types its options parameter as Record instead of Record. --- ...resentAssistantMessage-custom-tool.spec.ts | 18 +++++++- webview-ui/src/components/chat/ChatRow.tsx | 13 ++++-- .../IgnoredDisabledToolsNotice.spec.tsx | 44 ++++++++++++++++++- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts index e4b4839ccb..7c641f6d13 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts @@ -462,7 +462,23 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { expect(mockTask.consecutiveMistakeCount).toBe(0) const { attemptCompletionTool } = await import("../../tools/AttemptCompletionTool") - expect(attemptCompletionTool.handle).toHaveBeenCalled() + expect(attemptCompletionTool.handle).toHaveBeenCalledWith( + mockTask, + expect.objectContaining({ + type: "tool_use", + id: "tool_call_protocol_123", + name: "attempt_completion", + partial: false, + }), + expect.objectContaining({ + askApproval: expect.any(Function), + handleError: expect.any(Function), + pushToolResult: expect.any(Function), + askFinishSubTaskApproval: expect.any(Function), + toolDescription: expect.any(Function), + toolCallId: "tool_call_protocol_123", + }), + ) }) it("treats a model-excluded attempt_completion as blocked and answers it with an error tool_result", async () => { diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index e90a7f56c7..77ae4529c0 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -1591,13 +1591,20 @@ export const ChatRowContent = ({ ) } case "ignored_disabled_tools_warning": { - const ignoredData = safeJsonParse<{ ignoredTools: string[] }>(message.text || "{}") - if (!ignoredData?.ignoredTools) return null + const ignoredData = safeJsonParse<{ ignoredTools: unknown }>(message.text || "{}") + const ignoredTools = ignoredData?.ignoredTools + if ( + !Array.isArray(ignoredTools) || + ignoredTools.length === 0 || + !ignoredTools.every((tool) => typeof tool === "string") + ) { + return null + } return ( ) diff --git a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx index 3124dc71c4..f8265b7c45 100644 --- a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx @@ -7,7 +7,7 @@ import { ChatRowContent } from "../ChatRow" vi.mock("react-i18next", () => ({ useTranslation: () => ({ - t: (key: string, options?: Record) => { + t: (key: string, options?: Record) => { const map: Record = { "chat:ignoredDisabledTools.title": "Disabled tool ignored", "chat:ignoredDisabledTools.messageTemplate": @@ -84,4 +84,46 @@ describe("ChatRow - ignored disabled-tools notice", () => { expect(container.firstChild).toBeNull() expect(screen.queryByText("Disabled tool ignored")).not.toBeInTheDocument() }) + + it("renders nothing when ignoredTools is a non-array string", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + text: JSON.stringify({ ignoredTools: "attempt_completion" }), + } + + const { container } = renderChatRow(message) + + expect(container.firstChild).toBeNull() + expect(screen.queryByText("Disabled tool ignored")).not.toBeInTheDocument() + }) + + it("renders nothing when ignoredTools is an empty array", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + text: JSON.stringify({ ignoredTools: [] }), + } + + const { container } = renderChatRow(message) + + expect(container.firstChild).toBeNull() + expect(screen.queryByText("Disabled tool ignored")).not.toBeInTheDocument() + }) + + it("renders nothing when ignoredTools contains a non-string entry", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + text: JSON.stringify({ ignoredTools: ["attempt_completion", 42] }), + } + + const { container } = renderChatRow(message) + + expect(container.firstChild).toBeNull() + expect(screen.queryByText("Disabled tool ignored")).not.toBeInTheDocument() + }) }) From ed51a12ba495b60153b6e33f619bc2d5a8b6b716 Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Tue, 22 Sep 2026 19:38:43 +0200 Subject: [PATCH 4/5] Cover the resume path in the ignored disabled-tools notice tests Follow-up to PR review feedback on #1751. The notice that a disabled attempt_completion entry is ignored was only pinned on the new-task path: the spec drove startTask and asserted exactly one ignored_disabled_tools_warning message. The documented behavior that resuming a saved task does not re-emit the notice had no test. Task.ignored-disabled-tools-notice.spec.ts now also drives resumeTaskFromHistory for a history item whose profile lists attempt_completion in disabledTools. The new test asserts that no ignored_disabled_tools_warning message is said during the resume, next to liveness checks (the resume_task ask is issued and the task loop starts once) so a silent early return cannot pass as a zero-notice result. The provider fixture was extracted from the startTask helper so both paths share it; the existing new-task assertions are unchanged. --- ...Task.ignored-disabled-tools-notice.spec.ts | 76 ++++++++++++++++++- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts b/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts index 60eaae08c8..5782738518 100644 --- a/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts +++ b/src/core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts @@ -2,11 +2,12 @@ import * as vscode from "vscode" -import type { ModelInfo, ProviderSettings } from "@roo-code/types" +import type { ClineMessage, HistoryItem, ModelInfo, ProviderSettings } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" import { Task } from "../Task" import { ClineProvider } from "../../webview/ClineProvider" +import type { ApiMessage } from "../../task-persistence" vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { @@ -105,7 +106,10 @@ vi.mock("delay", () => ({ type StartTaskAccess = { startTask: (task?: string, images?: string[]) => Promise + resumeTaskFromHistory: () => Promise getEnabledMcpToolsCount: () => Promise<{ enabledToolCount: number; enabledServerCount: number }> + getSavedClineMessages: () => Promise + getSavedApiConversationHistory: () => Promise initiateTaskLoop: (userContent: unknown[]) => Promise } @@ -120,9 +124,9 @@ type ProviderStateShaped = Partial disabledTools?: string[] } -async function startTaskWithDisabledTools(disabledTools: string[] | undefined, profileExcludedTools?: string[]) { +function createMockProvider(disabledTools: string[] | undefined): ClineProvider { const providerState = { disabledTools } as ProviderStateShaped - const mockProvider = { + return { context: { globalStorageUri: { fsPath: "/test/storage" }, }, @@ -135,6 +139,10 @@ async function startTaskWithDisabledTools(disabledTools: string[] | undefined, p updateTaskHistory: vi.fn().mockResolvedValue(undefined), // ClineProvider's surface is too broad to type this fixture fully; double assertion is the last resort (AGENTS.md) — only task-scoped fields are read. } as unknown as ClineProvider +} + +async function startTaskWithDisabledTools(disabledTools: string[] | undefined, profileExcludedTools?: string[]) { + const mockProvider = createMockProvider(disabledTools) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.anthropic, @@ -174,6 +182,57 @@ async function startTaskWithDisabledTools(disabledTools: string[] | undefined, p return saySpy.mock.calls.filter(([type]) => type === "ignored_disabled_tools_warning") } +async function resumeTaskWithDisabledTools(disabledTools: string[] | undefined) { + const mockProvider = createMockProvider(disabledTools) + + const apiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + + const historyItem: HistoryItem = { + id: "resume-notice-task", + number: 7, + task: "historical task", + ts: Date.now() - 60_000, + tokensIn: 10, + tokensOut: 5, + totalCost: 0.01, + } + + const task = new Task({ + provider: mockProvider, + apiConfiguration, + historyItem, + startTask: false, + }) + + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + const askSpy = vi.spyOn(task, "ask").mockResolvedValue({ response: "yesButtonClicked" }) + // Persisted-history reads are stubbed so the resume path drives entirely in + // memory; the saved API history ends on an assistant turn, the ordinary + // interrupted-task shape that reaches the resume ask and loop start. + const taskAccess = getStartTaskAccess(task) + vi.spyOn(taskAccess, "getSavedClineMessages").mockResolvedValue([ + { ts: historyItem.ts, type: "say", say: "text", text: "historical task" }, + ]) + vi.spyOn(taskAccess, "getSavedApiConversationHistory").mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "historical task" }], ts: historyItem.ts }, + { role: "assistant", content: [{ type: "text", text: "Working on it." }], ts: historyItem.ts + 1 }, + ]) + vi.spyOn(task, "overwriteApiConversationHistory").mockResolvedValue(undefined) + const initiateLoopSpy = vi.spyOn(taskAccess, "initiateTaskLoop").mockResolvedValue(undefined) + + await taskAccess.resumeTaskFromHistory() + + return { + noticeCalls: saySpy.mock.calls.filter(([type]) => type === "ignored_disabled_tools_warning"), + askSpy, + initiateLoopSpy, + } +} + describe("Task - ignored disabled-tools notice", () => { it("notifies exactly once when disabledTools lists a tool that cannot be disabled", async () => { const noticeCalls = await startTaskWithDisabledTools(["execute_command", "attempt_completion"]) @@ -203,4 +262,15 @@ describe("Task - ignored disabled-tools notice", () => { expect(noticeCalls).toHaveLength(1) expect(JSON.parse(noticeCalls[0][1] as string)).toEqual({ ignoredTools: ["attempt_completion"] }) }) + + it("stays silent when a task carrying the same disabledTools entry resumes from history", async () => { + const { noticeCalls, askSpy, initiateLoopSpy } = await resumeTaskWithDisabledTools(["attempt_completion"]) + + // Liveness: without these, the zero below could pass on a resume path + // that returned early instead of reaching the point a notice could occur. + expect(askSpy).toHaveBeenCalledWith("resume_task") + expect(initiateLoopSpy).toHaveBeenCalledTimes(1) + + expect(noticeCalls).toHaveLength(0) + }) }) From 06b14023b7a5cf2a5fdd968b9bd3b7ffe2119c39 Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Sat, 3 Oct 2026 23:34:38 +0200 Subject: [PATCH 5/5] fix: drop the availability claim from the ignored-tools notice Follow-up to PR review feedback on #1751. The notice telling the user that a disabled attempt_completion entry was ignored claimed the tool "will remain available". That is false in one state: when the active model profile's excludedTools also names the tool, the effective policy still strips it, so the notice overpromised. The profile-level excludedTools veto is intentional behavior, so the copy changed rather than the policy. messageTemplate now only states why the entry was ignored: "The following tools cannot be disabled, so their disabledTools entries were ignored: {{tools}}." The string is updated in all 18 locale chat.json files; every locale currently replicated the false claim, so an en-only edit would leave 17 of them asserting it. Task.ts detection and emission are untouched, and {{tools}} is byte-preserved everywhere. Two test changes lock in the fix. In IgnoredDisabledToolsNotice.spec.tsx the two assertion strings move to the new copy and a regression test renders the notice and fails if any availability wording reappears (6 tests to 7). validateToolUse.spec.ts gains a builder/validator agreement table: real buildToolRequirements output driven through the real isToolAllowedForMode and validateToolUse across six rows, including the overlap case where both lists name attempt_completion and the profile veto wins. Additions only; every pre-existing test is byte-identical (27 tests to 33). --- .../tools/__tests__/validateToolUse.spec.ts | 67 +++++++++++++++++++ .../IgnoredDisabledToolsNotice.spec.tsx | 26 ++++++- webview-ui/src/i18n/locales/ca/chat.json | 2 +- webview-ui/src/i18n/locales/de/chat.json | 2 +- webview-ui/src/i18n/locales/en/chat.json | 2 +- webview-ui/src/i18n/locales/es/chat.json | 2 +- webview-ui/src/i18n/locales/fr/chat.json | 2 +- webview-ui/src/i18n/locales/hi/chat.json | 2 +- webview-ui/src/i18n/locales/id/chat.json | 2 +- webview-ui/src/i18n/locales/it/chat.json | 2 +- webview-ui/src/i18n/locales/ja/chat.json | 2 +- webview-ui/src/i18n/locales/ko/chat.json | 2 +- webview-ui/src/i18n/locales/nl/chat.json | 2 +- webview-ui/src/i18n/locales/pl/chat.json | 2 +- webview-ui/src/i18n/locales/pt-BR/chat.json | 2 +- webview-ui/src/i18n/locales/ru/chat.json | 2 +- webview-ui/src/i18n/locales/tr/chat.json | 2 +- webview-ui/src/i18n/locales/vi/chat.json | 2 +- webview-ui/src/i18n/locales/zh-CN/chat.json | 2 +- webview-ui/src/i18n/locales/zh-TW/chat.json | 2 +- 20 files changed, 109 insertions(+), 20 deletions(-) diff --git a/src/core/tools/__tests__/validateToolUse.spec.ts b/src/core/tools/__tests__/validateToolUse.spec.ts index d839cdc646..c4a02a9bcf 100644 --- a/src/core/tools/__tests__/validateToolUse.spec.ts +++ b/src/core/tools/__tests__/validateToolUse.spec.ts @@ -1,10 +1,12 @@ // npx vitest run src/core/tools/__tests__/validateToolUse.spec.ts import { toolNamesSchema, type ModeConfig } from "@roo-code/types" +import type { ModelInfo } from "@roo-code/types" import { modes } from "../../../shared/modes" import { TOOL_GROUPS } from "../../../shared/tools" +import { buildToolRequirements } from "../../prompts/tools/effective-tool-policy" import { validateToolUse, isToolAllowedForMode } from "../validateToolUse" const codeMode = modes.find((m) => m.slug === "code")?.slug || "code" @@ -282,3 +284,68 @@ describe("mode-validator", () => { }) }) }) + +describe("buildToolRequirements ↔ validator agreement", () => { + // The prompt advertises the tool set computed from the same disabledTools/modelInfo + // inputs that `buildToolRequirements` turns into the requirements map `validateToolUse` + // consumes. Driving real builder output through the real validator pins that agreement + // on the path execution actually takes, not just between two maps. + + /** A ModelInfo carrying only the schema-required fields, plus optional profile exclusions. */ + const model = (excludedTools?: string[]): ModelInfo => ({ + contextWindow: 100_000, + supportsPromptCache: true, + ...(excludedTools ? { excludedTools } : {}), + }) + + it("leaves attempt_completion callable when neither list names it", () => { + const requirements = buildToolRequirements(undefined, model()) + expect(isToolAllowedForMode("attempt_completion", codeMode, [], requirements)).toBe(true) + expect(() => validateToolUse("attempt_completion", codeMode, [], requirements)).not.toThrow() + }) + + it("keeps attempt_completion callable when only disabledTools names it", () => { + // The completion tool has no coherent disabled state, so the builder's + // protocol partition must strip this entry out of the requirements map. + const requirements = buildToolRequirements(["attempt_completion"], model()) + expect(isToolAllowedForMode("attempt_completion", codeMode, [], requirements)).toBe(true) + expect(() => validateToolUse("attempt_completion", codeMode, [], requirements)).not.toThrow() + }) + + it("rejects attempt_completion when only the model profile excludes it", () => { + const requirements = buildToolRequirements(undefined, model(["attempt_completion"])) + expect(isToolAllowedForMode("attempt_completion", codeMode, [], requirements)).toBe(false) + expect(() => validateToolUse("attempt_completion", codeMode, [], requirements)).toThrow( + 'Tool "attempt_completion" is not allowed in code mode.', + ) + }) + + it("rejects attempt_completion when both lists name it", () => { + // A model-profile exclusion declares provider capability, a mechanism the + // user-disable exemption deliberately does not cover, so its veto stands + // even though the identical disabledTools entry is ignored. + const requirements = buildToolRequirements(["attempt_completion"], model(["attempt_completion"])) + expect(isToolAllowedForMode("attempt_completion", codeMode, [], requirements)).toBe(false) + expect(() => validateToolUse("attempt_completion", codeMode, [], requirements)).toThrow( + 'Tool "attempt_completion" is not allowed in code mode.', + ) + }) + + it("still blocks an always-available sibling disabled through the real builder", () => { + // The exemption covers the protocol tool only; its always-available siblings + // must remain blockable when the disable flows through the real builder. + const requirements = buildToolRequirements(["switch_mode"], model()) + expect(isToolAllowedForMode("switch_mode", codeMode, [], requirements)).toBe(false) + expect(() => validateToolUse("switch_mode", codeMode, [], requirements)).toThrow( + 'Tool "switch_mode" is not allowed in code mode.', + ) + }) + + it("still blocks an ordinary tool disabled through the real builder", () => { + const requirements = buildToolRequirements(["execute_command"], model()) + expect(isToolAllowedForMode("execute_command", codeMode, [], requirements)).toBe(false) + expect(() => validateToolUse("execute_command", codeMode, [], requirements)).toThrow( + 'Tool "execute_command" is not allowed in code mode.', + ) + }) +}) diff --git a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx index f8265b7c45..0272ee00f5 100644 --- a/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/IgnoredDisabledToolsNotice.spec.tsx @@ -11,7 +11,7 @@ vi.mock("react-i18next", () => ({ const map: Record = { "chat:ignoredDisabledTools.title": "Disabled tool ignored", "chat:ignoredDisabledTools.messageTemplate": - "The following tools cannot be disabled and will remain available: {{tools}}.", + "The following tools cannot be disabled, so their disabledTools entries were ignored: {{tools}}.", } const template = map[key] ?? key if (!options) return template @@ -54,10 +54,32 @@ describe("ChatRow - ignored disabled-tools notice", () => { expect(screen.getByText("Disabled tool ignored")).toBeInTheDocument() expect( - screen.getByText("The following tools cannot be disabled and will remain available: attempt_completion."), + screen.getByText( + "The following tools cannot be disabled, so their disabledTools entries were ignored: attempt_completion.", + ), ).toBeInTheDocument() }) + it("does not claim the tool remains available", () => { + const message: ClineMessage = { + type: "say", + say: "ignored_disabled_tools_warning", + ts: Date.now(), + text: JSON.stringify({ ignoredTools: ["attempt_completion"] }), + } + + renderChatRow(message) + + expect( + screen.getByText( + "The following tools cannot be disabled, so their disabledTools entries were ignored: attempt_completion.", + ), + ).toBeInTheDocument() + // The notice names what was ignored but never promises the tool stays usable: + // a model profile's excludedTools can still strip the same tool. + expect(screen.queryByText(/available/i)).not.toBeInTheDocument() + }) + it("renders nothing when the notice message text is missing", () => { const message: ClineMessage = { type: "say", diff --git a/webview-ui/src/i18n/locales/ca/chat.json b/webview-ui/src/i18n/locales/ca/chat.json index 95ddc2fcd1..72c43c866f 100644 --- a/webview-ui/src/i18n/locales/ca/chat.json +++ b/webview-ui/src/i18n/locales/ca/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Eina desactivada ignorada", - "messageTemplate": "Les eines següents no es poden desactivar i continuaran disponibles: {{tools}}." + "messageTemplate": "Les eines següents no es poden desactivar, per això les entrades de disabledTools s'han ignorat: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/de/chat.json b/webview-ui/src/i18n/locales/de/chat.json index 397d3f3997..f964f156fe 100644 --- a/webview-ui/src/i18n/locales/de/chat.json +++ b/webview-ui/src/i18n/locales/de/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Deaktiviertes Tool ignoriert", - "messageTemplate": "Die folgenden Tools können nicht deaktiviert werden und bleiben verfügbar: {{tools}}." + "messageTemplate": "Die folgenden Tools können nicht deaktiviert werden; die entsprechenden disabledTools-Einträge wurden ignoriert: {{tools}}." }, "readCommandOutput": { "title": "Zoo las Befehlsausgabe" diff --git a/webview-ui/src/i18n/locales/en/chat.json b/webview-ui/src/i18n/locales/en/chat.json index 080ba71916..5343e29262 100644 --- a/webview-ui/src/i18n/locales/en/chat.json +++ b/webview-ui/src/i18n/locales/en/chat.json @@ -478,7 +478,7 @@ }, "ignoredDisabledTools": { "title": "Disabled tool ignored", - "messageTemplate": "The following tools cannot be disabled and will remain available: {{tools}}." + "messageTemplate": "The following tools cannot be disabled, so their disabledTools entries were ignored: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/es/chat.json b/webview-ui/src/i18n/locales/es/chat.json index ea0b368218..7f1335515f 100644 --- a/webview-ui/src/i18n/locales/es/chat.json +++ b/webview-ui/src/i18n/locales/es/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Herramienta deshabilitada ignorada", - "messageTemplate": "Las siguientes herramientas no se pueden deshabilitar y seguirán disponibles: {{tools}}." + "messageTemplate": "Las siguientes herramientas no se pueden desactivar, por lo que sus entradas de disabledTools se ignoraron: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/fr/chat.json b/webview-ui/src/i18n/locales/fr/chat.json index b07a2f5a7c..c45a4d7b91 100644 --- a/webview-ui/src/i18n/locales/fr/chat.json +++ b/webview-ui/src/i18n/locales/fr/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Outil désactivé ignoré", - "messageTemplate": "Les outils suivants ne peuvent pas être désactivés et resteront disponibles : {{tools}}." + "messageTemplate": "Les outils suivants ne peuvent pas être désactivés ; leurs entrées disabledTools ont été ignorées : {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/hi/chat.json b/webview-ui/src/i18n/locales/hi/chat.json index a5e4f56232..4d4d61f14b 100644 --- a/webview-ui/src/i18n/locales/hi/chat.json +++ b/webview-ui/src/i18n/locales/hi/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "निष्क्रिय टूल अनदेखा किया गया", - "messageTemplate": "ये टूल निष्क्रिय नहीं किए जा सकते और उपलब्ध रहेंगे: {{tools}}." + "messageTemplate": "निम्नलिखित टूल निष्क्रिय नहीं किए जा सकते, इसलिए उनकी disabledTools प्रविष्टियाँ अनदेखी की गईं: {{tools}}।" }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/id/chat.json b/webview-ui/src/i18n/locales/id/chat.json index 48871e9479..89dcfb0297 100644 --- a/webview-ui/src/i18n/locales/id/chat.json +++ b/webview-ui/src/i18n/locales/id/chat.json @@ -496,7 +496,7 @@ }, "ignoredDisabledTools": { "title": "Tool nonaktif diabaikan", - "messageTemplate": "Tool berikut tidak dapat dinonaktifkan dan akan tetap tersedia: {{tools}}." + "messageTemplate": "Tool berikut tidak dapat dinonaktifkan, sehingga entri disabledTools-nya diabaikan: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/it/chat.json b/webview-ui/src/i18n/locales/it/chat.json index c51641be7d..7d2dc2294a 100644 --- a/webview-ui/src/i18n/locales/it/chat.json +++ b/webview-ui/src/i18n/locales/it/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Strumento disabilitato ignorato", - "messageTemplate": "I seguenti strumenti non possono essere disabilitati e rimarranno disponibili: {{tools}}." + "messageTemplate": "I seguenti strumenti non possono essere disabilitati, quindi le relative voci di disabledTools sono state ignorate: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/ja/chat.json b/webview-ui/src/i18n/locales/ja/chat.json index 58a1d24ba2..020c5726be 100644 --- a/webview-ui/src/i18n/locales/ja/chat.json +++ b/webview-ui/src/i18n/locales/ja/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "無効化されたツールは無視されました", - "messageTemplate": "次のツールは無効化できず、引き続き利用可能です: {{tools}}。" + "messageTemplate": "次のツールは無効化できないため、対応する disabledTools エントリは無視されました: {{tools}}。" }, "readCommandOutput": { "title": "Zooがコマンド出力を読み込みました" diff --git a/webview-ui/src/i18n/locales/ko/chat.json b/webview-ui/src/i18n/locales/ko/chat.json index 6c2c460c2a..16fd3e6743 100644 --- a/webview-ui/src/i18n/locales/ko/chat.json +++ b/webview-ui/src/i18n/locales/ko/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "비활성화된 도구 무시됨", - "messageTemplate": "다음 도구는 비활성화할 수 없으며 계속 사용할 수 있습니다: {{tools}}." + "messageTemplate": "다음 도구는 비활성화할 수 없으므로 해당 disabledTools 항목이 무시되었습니다: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/nl/chat.json b/webview-ui/src/i18n/locales/nl/chat.json index 65dbdc2b75..b2f1342add 100644 --- a/webview-ui/src/i18n/locales/nl/chat.json +++ b/webview-ui/src/i18n/locales/nl/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Uitgeschakelde tool genegeerd", - "messageTemplate": "De volgende tools kunnen niet worden uitgeschakeld en blijven beschikbaar: {{tools}}." + "messageTemplate": "De volgende tools kunnen niet worden uitgeschakeld, zodat de bijbehorende disabledTools-vermeldingen zijn genegeerd: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/pl/chat.json b/webview-ui/src/i18n/locales/pl/chat.json index d3bf01ae30..eceb61bef5 100644 --- a/webview-ui/src/i18n/locales/pl/chat.json +++ b/webview-ui/src/i18n/locales/pl/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Zignorowano wyłączane narzędzie", - "messageTemplate": "Następujących narzędzi nie można wyłączyć i pozostaną dostępne: {{tools}}." + "messageTemplate": "Następujących narzędzi nie można wyłączyć, dlatego ich wpisy w disabledTools zostały zignorowane: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/pt-BR/chat.json b/webview-ui/src/i18n/locales/pt-BR/chat.json index 16e02ddd02..c7323b290b 100644 --- a/webview-ui/src/i18n/locales/pt-BR/chat.json +++ b/webview-ui/src/i18n/locales/pt-BR/chat.json @@ -490,7 +490,7 @@ }, "ignoredDisabledTools": { "title": "Ferramenta desativada ignorada", - "messageTemplate": "As seguintes ferramentas não podem ser desativadas e permanecerão disponíveis: {{tools}}." + "messageTemplate": "As ferramentas a seguir não podem ser desativadas, portanto suas entradas em disabledTools foram ignoradas: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/ru/chat.json b/webview-ui/src/i18n/locales/ru/chat.json index 7f7164cf43..94091d2d08 100644 --- a/webview-ui/src/i18n/locales/ru/chat.json +++ b/webview-ui/src/i18n/locales/ru/chat.json @@ -491,7 +491,7 @@ }, "ignoredDisabledTools": { "title": "Отключённый инструмент проигнорирован", - "messageTemplate": "Следующие инструменты нельзя отключить, и они останутся доступными: {{tools}}." + "messageTemplate": "Следующие инструменты нельзя отключить, поэтому их записи в disabledTools были проигнорированы: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/tr/chat.json b/webview-ui/src/i18n/locales/tr/chat.json index deb7ca2fc2..93157e34c5 100644 --- a/webview-ui/src/i18n/locales/tr/chat.json +++ b/webview-ui/src/i18n/locales/tr/chat.json @@ -491,7 +491,7 @@ }, "ignoredDisabledTools": { "title": "Devre dışı bırakılan araç yok sayıldı", - "messageTemplate": "Aşağıdaki araçlar devre dışı bırakılamaz ve kullanılabilir kalır: {{tools}}." + "messageTemplate": "Aşağıdaki araçlar devre dışı bırakılamaz; bu nedenle ilgili disabledTools girdileri yok sayıldı: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/vi/chat.json b/webview-ui/src/i18n/locales/vi/chat.json index fcb1c5df37..5b54a3026b 100644 --- a/webview-ui/src/i18n/locales/vi/chat.json +++ b/webview-ui/src/i18n/locales/vi/chat.json @@ -491,7 +491,7 @@ }, "ignoredDisabledTools": { "title": "Đã bỏ qua công cụ bị tắt", - "messageTemplate": "Các công cụ sau không thể bị tắt và sẽ vẫn khả dụng: {{tools}}." + "messageTemplate": "Các công cụ sau không thể bị tắt, nên các mục disabledTools tương ứng đã được bỏ qua: {{tools}}." }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/zh-CN/chat.json b/webview-ui/src/i18n/locales/zh-CN/chat.json index 6740b025e6..a7b903d4f8 100644 --- a/webview-ui/src/i18n/locales/zh-CN/chat.json +++ b/webview-ui/src/i18n/locales/zh-CN/chat.json @@ -491,7 +491,7 @@ }, "ignoredDisabledTools": { "title": "已忽略被禁用的工具", - "messageTemplate": "以下工具无法被禁用,将继续可用:{{tools}}。" + "messageTemplate": "以下工具无法被禁用,因此它们在 disabledTools 中的条目已被忽略:{{tools}}。" }, "readCommandOutput": { "title": "Zoo read command output" diff --git a/webview-ui/src/i18n/locales/zh-TW/chat.json b/webview-ui/src/i18n/locales/zh-TW/chat.json index a6299f97c8..42c0939d7b 100644 --- a/webview-ui/src/i18n/locales/zh-TW/chat.json +++ b/webview-ui/src/i18n/locales/zh-TW/chat.json @@ -481,7 +481,7 @@ }, "ignoredDisabledTools": { "title": "已忽略被停用的工具", - "messageTemplate": "以下工具無法被停用,將繼續可用:{{tools}}。" + "messageTemplate": "以下工具無法被停用,因此它在 disabledTools 中的條目已被忽略:{{tools}}。" }, "readCommandOutput": { "title": "Zoo read command output"