Merge pull request #464 from RooVetGit/only_allow_diffs_if_enabled

Only allow usage of diff tool if enabled in settings

Author: mrubens

.changeset/nervous-radios-sneeze.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Only allow use of the diff tool if it's enabled in settings

src/core/Cline.ts

@@ -1,7 +1,7 @@
 import { Anthropic } from "@anthropic-ai/sdk"
 import cloneDeep from "clone-deep"
 import { DiffStrategy, getDiffStrategy, UnifiedDiffStrategy } from "./diff/DiffStrategy"
-import { validateToolUse, isToolAllowedForMode } from "./mode-validator"
+import { validateToolUse, isToolAllowedForMode, ToolName } from "./mode-validator"
 import delay from "delay"
 import fs from "fs/promises"
 import os from "os"
@@ -827,7 +827,9 @@ export class Cline {
 				mode,
 				customPrompts,
 				customModes,
+				this.customInstructions,
 				preferredLanguage,
+				this.diffEnabled,
 			)
 		})()
 
@@ -1140,11 +1142,13 @@ export class Cline {
 					await this.browserSession.closeBrowser()
 				}
 
-				// Validate tool use based on current mode
+				// Validate tool use before execution
 				const { mode } = (await this.providerRef.deref()?.getState()) ?? {}
 				const { customModes } = (await this.providerRef.deref()?.getState()) ?? {}
 				try {
-					validateToolUse(block.name, mode ?? defaultModeSlug, customModes)
+					validateToolUse(block.name as ToolName, mode ?? defaultModeSlug, customModes ?? [], {
+						apply_diff: this.diffEnabled,
+					})
 				} catch (error) {
 					this.consecutiveMistakeCount++
 					pushToolResult(formatResponse.toolError(error.message))
@@ -2637,8 +2641,10 @@ export class Cline {
 
 		// Add warning if not in code mode
 		if (
-			!isToolAllowedForMode("write_to_file", currentMode, customModes ?? []) &&
-			!isToolAllowedForMode("apply_diff", currentMode, customModes ?? [])
+			!isToolAllowedForMode("write_to_file", currentMode, customModes ?? [], {
+				apply_diff: this.diffEnabled,
+			}) &&
+			!isToolAllowedForMode("apply_diff", currentMode, customModes ?? [], { apply_diff: this.diffEnabled })
 		) {
 			const currentModeName = getModeBySlug(currentMode, customModes)?.name ?? currentMode
 			const defaultModeName = getModeBySlug(defaultModeSlug, customModes)?.name ?? defaultModeSlug

src/core/__tests__/mode-validator.test.ts

@@ -74,6 +74,50 @@ describe("mode-validator", () => {
 				// Should not allow tools from other groups
 				expect(isToolAllowedForMode("write_to_file", codeMode, customModes)).toBe(false)
 			})
+
+			it("respects tool requirements in custom modes", () => {
+				const customModes = [
+					{
+						slug: "custom-mode",
+						name: "Custom Mode",
+						roleDefinition: "Custom role",
+						groups: ["edit"] as const,
+					},
+				]
+				const requirements = { apply_diff: false }
+
+				// Should respect disabled requirement even if tool group is allowed
+				expect(isToolAllowedForMode("apply_diff", "custom-mode", customModes, requirements)).toBe(false)
+
+				// Should allow other edit tools
+				expect(isToolAllowedForMode("write_to_file", "custom-mode", customModes, requirements)).toBe(true)
+			})
+		})
+
+		describe("tool requirements", () => {
+			it("respects tool requirements when provided", () => {
+				const requirements = { apply_diff: false }
+				expect(isToolAllowedForMode("apply_diff", codeMode, [], requirements)).toBe(false)
+
+				const enabledRequirements = { apply_diff: true }
+				expect(isToolAllowedForMode("apply_diff", codeMode, [], enabledRequirements)).toBe(true)
+			})
+
+			it("allows tools when their requirements are not specified", () => {
+				const requirements = { some_other_tool: true }
+				expect(isToolAllowedForMode("apply_diff", codeMode, [], requirements)).toBe(true)
+			})
+
+			it("handles undefined and empty requirements", () => {
+				expect(isToolAllowedForMode("apply_diff", codeMode, [], undefined)).toBe(true)
+				expect(isToolAllowedForMode("apply_diff", codeMode, [], {})).toBe(true)
+			})
+
+			it("prioritizes requirements over mode configuration", () => {
+				const requirements = { apply_diff: false }
+				// Even in code mode which allows all tools, disabled requirement should take precedence
+				expect(isToolAllowedForMode("apply_diff", codeMode, [], requirements)).toBe(false)
+			})
 		})
 	})
 
@@ -87,5 +131,21 @@ describe("mode-validator", () => {
 		it("does not throw for allowed tools in architect mode", () => {
 			expect(() => validateToolUse("read_file", "architect", [])).not.toThrow()
 		})
+
+		it("throws error when tool requirement is not met", () => {
+			const requirements = { apply_diff: false }
+			expect(() => validateToolUse("apply_diff", codeMode, [], requirements)).toThrow(
+				'Tool "apply_diff" is not allowed in code mode.',
+			)
+		})
+
+		it("does not throw when tool requirement is met", () => {
+			const requirements = { apply_diff: true }
+			expect(() => validateToolUse("apply_diff", codeMode, [], requirements)).not.toThrow()
+		})
+
+		it("handles undefined requirements gracefully", () => {
+			expect(() => validateToolUse("apply_diff", codeMode, [], undefined)).not.toThrow()
+		})
 	})
 })

src/core/mode-validator.ts

@@ -4,8 +4,13 @@ import { ToolName } from "../shared/tool-groups"
 export { isToolAllowedForMode }
 export type { ToolName }
 
-export function validateToolUse(toolName: ToolName, mode: Mode, customModes?: ModeConfig[]): void {
-	if (!isToolAllowedForMode(toolName, mode, customModes ?? [])) {
+export function validateToolUse(
+	toolName: ToolName,
+	mode: Mode,
+	customModes?: ModeConfig[],
+	toolRequirements?: Record<string, boolean>,
+): void {
+	if (!isToolAllowedForMode(toolName, mode, customModes ?? [], toolRequirements)) {
 		throw new Error(`Tool "${toolName}" is not allowed in ${mode} mode.`)
 	}
 }