fix: exempt attempt_completion from disabledTools

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(),
 })

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
 

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,40 @@ 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).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 () => {

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)
 

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,29 +414,26 @@ 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", () => {
 		const reqs = buildToolRequirements(["write_file"])
 		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")

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", () => {