Unify effective tool policy across prompts and tool filtering

src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts

@@ -23,6 +23,15 @@ vi.mock("@roo-code/core", () => ({
 	},
 }))
 
+// Mock the tool handlers so the tests only exercise validation (toolRequirements)
+// and never the real tool execution logic.
+vi.mock("../../tools/AttemptCompletionTool", () => ({
+	attemptCompletionTool: { handle: vi.fn().mockResolvedValue(undefined) },
+}))
+vi.mock("../../tools/AskFollowupQuestionTool", () => ({
+	askFollowupQuestionTool: { handle: vi.fn().mockResolvedValue(undefined) },
+}))
+
 // presentAssistantMessage records tool usage through TelemetryService.instance.
 vi.mock("@roo-code/telemetry", () => ({
 	TelemetryService: {
@@ -333,6 +342,169 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
 				edit: false,
 			})
 		})
+
+		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.
+			mockTask.assistantMessageContent = [
+				{
+					type: "tool_use",
+					id: "tool_call_protocol_123",
+					name: "attempt_completion",
+					params: {},
+					nativeArgs: {},
+					partial: false,
+				},
+			]
+
+			mockTask.providerRef = {
+				deref: () => ({
+					getState: vi.fn().mockResolvedValue({
+						mode: "code",
+						customModes: [],
+						experiments: {
+							customTools: false,
+						},
+						disabledTools: ["attempt_completion"],
+					}),
+				}),
+			}
+
+			// 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 })
+
+			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)
+
+			// The completion handler must not run for the rejected call.
+			const { attemptCompletionTool } = await import("../../tools/AttemptCompletionTool")
+			expect(attemptCompletionTool.handle).not.toHaveBeenCalled()
+		})
+
+		it("treats a model-excluded attempt_completion as blocked and answers it with an error tool_result", async () => {
+			// A model excludedTools entry suppresses the protocol tool in the
+			// effective policy, so the execution gate must see the same
+			// restriction with disabledTools unset.
+			mockTask.assistantMessageContent = [
+				{
+					type: "tool_use",
+					id: "tool_call_protocol_excluded_123",
+					name: "attempt_completion",
+					params: {},
+					nativeArgs: {},
+					partial: false,
+				},
+			]
+
+			mockTask.api.getModel = () => ({ id: "test-model", info: { excludedTools: ["attempt_completion"] } })
+
+			mockTask.providerRef = {
+				deref: () => ({
+					getState: vi.fn().mockResolvedValue({
+						mode: "code",
+						customModes: [],
+						experiments: {
+							customTools: false,
+						},
+					}),
+				}),
+			}
+
+			// 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 })
+
+			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)
+
+			// The completion handler must not run for the rejected call.
+			const { attemptCompletionTool } = await import("../../tools/AttemptCompletionTool")
+			expect(attemptCompletionTool.handle).not.toHaveBeenCalled()
+
+			// Absent model metadata must not derail the requirements build: the
+			// protocol-tool leg simply sees no exclusions, and the call validates
+			// normally instead of erroring out.
+			mockTask.api.getModel = () => undefined
+			mockTask.currentStreamingContentIndex = 0
+			mockTask.userMessageContent = []
+			mockTask.consecutiveMistakeCount = 0
+			mockTask.didAlreadyUseTool = false
+			mockTask.didCompleteReadingStream = false
+
+			await presentAssistantMessage(mockTask)
+
+			expect(validateToolUseMock).toHaveBeenCalledTimes(2)
+			expect(validateToolUseMock.mock.calls[1][3]).toEqual({})
+			expect(mockTask.consecutiveMistakeCount).toBe(0)
+			const phase2Errors = mockTask.userMessageContent.filter((block: { type?: string; is_error?: boolean }) => {
+				return block.type === "tool_result" && block.is_error
+			})
+			expect(phase2Errors).toHaveLength(0)
+		})
+
+		it("still marks ordinary tools (ask_followup_question) as blocked", async () => {
+			mockTask.assistantMessageContent = [
+				{
+					type: "tool_use",
+					id: "tool_call_ordinary_123",
+					name: "ask_followup_question",
+					params: { question: "Which option?" },
+					nativeArgs: { question: "Which option?" },
+					partial: false,
+				},
+			]
+
+			mockTask.providerRef = {
+				deref: () => ({
+					getState: vi.fn().mockResolvedValue({
+						mode: "code",
+						customModes: [],
+						experiments: {
+							customTools: false,
+						},
+						disabledTools: ["ask_followup_question"],
+					}),
+				}),
+			}
+
+			await presentAssistantMessage(mockTask)
+
+			const validateToolUseMock = vi.mocked(validateToolUse)
+			expect(validateToolUseMock).toHaveBeenCalled()
+			const toolRequirements = validateToolUseMock.mock.calls[0][3]
+			expect(toolRequirements).toMatchObject({
+				ask_followup_question: false,
+			})
+		})
 	})
 
 	describe("Partial blocks", () => {

src/core/assistant-message/presentAssistantMessage.ts

@@ -36,6 +36,7 @@ import { skillTool } from "../tools/SkillTool"
 import { generateImageTool } from "../tools/GenerateImageTool"
 import { applyDiffTool as applyDiffToolClass } from "../tools/ApplyDiffTool"
 import { isValidToolName, validateToolUse } from "../tools/validateToolUse"
+import { buildToolRequirements } from "../prompts/tools/effective-tool-policy"
 import { codebaseSearchTool } from "../tools/CodebaseSearchTool"
 
 import { formatResponse } from "../prompts/responses"
@@ -604,16 +605,11 @@ export async function presentAssistantMessage(cline: Task) {
 				const isCustomTool = Boolean(stateExperiments?.customTools && customToolRegistry.has(block.name))
 
 				try {
-					const toolRequirements =
-						disabledTools?.reduce(
-							(acc: Record<string, boolean>, tool: string) => {
-								acc[tool] = false
-								const resolvedToolName = resolveToolAlias(tool)
-								acc[resolvedToolName] = false
-								return acc
-							},
-							{} as Record<string, boolean>,
-						) ?? {}
+					// 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
+					// `buildToolRequirements` in effective-tool-policy.ts.
+					const toolRequirements = buildToolRequirements(disabledTools, modelInfo?.info)
 
 					validateToolUse(
 						block.name as ToolName,