Refactor modes

Author: mrubens

src/core/Cline.ts

@@ -45,7 +45,8 @@ import { arePathsEqual, getReadablePath } from "../utils/path"
 import { parseMentions } from "./mentions"
 import { AssistantMessageContent, parseAssistantMessage, ToolParamName, ToolUseName } from "./assistant-message"
 import { formatResponse } from "./prompts/responses"
-import { addCustomInstructions, codeMode, SYSTEM_PROMPT } from "./prompts/system"
+import { addCustomInstructions, SYSTEM_PROMPT } from "./prompts/system"
+import { modes, defaultModeSlug } from "../shared/modes"
 import { truncateHalfConversation } from "./sliding-window"
 import { ClineProvider, GlobalFileNames } from "./webview/ClineProvider"
 import { detectCodeOmission } from "../integrations/editor/detect-omission"
@@ -1126,7 +1127,7 @@ export class Cline {
 				// Validate tool use based on current mode
 				const { mode } = await this.providerRef.deref()?.getState() ?? {}
 				try {
-					validateToolUse(block.name, mode ?? codeMode)
+					validateToolUse(block.name, mode ?? defaultModeSlug)
 				} catch (error) {
 					this.consecutiveMistakeCount++
 					pushToolResult(formatResponse.toolError(error.message))
@@ -2585,12 +2586,12 @@ export class Cline {
 
 		// Add current mode and any mode-specific warnings
 		const { mode } = await this.providerRef.deref()?.getState() ?? {}
-		const currentMode = mode ?? codeMode
+		const currentMode = mode ?? defaultModeSlug
 		details += `\n\n# Current Mode\n${currentMode}`
-		
+
 		// Add warning if not in code mode
 		if (!isToolAllowedForMode('write_to_file', currentMode) || !isToolAllowedForMode('execute_command', currentMode)) {
-			details += `\n\nNOTE: You are currently in '${currentMode}' mode which only allows read-only operations. To write files or execute commands, the user will need to switch to 'code' mode. Note that only the user can switch modes.`
+			details += `\n\nNOTE: You are currently in '${currentMode}' mode which only allows read-only operations. To write files or execute commands, the user will need to switch to '${defaultModeSlug}' mode. Note that only the user can switch modes.`
 		}
 
 		if (includeFileDetails) {

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

@@ -1,20 +1,15 @@
-import { isToolAllowedForMode, validateToolUse } from '../mode-validator'
-import { codeMode, architectMode, askMode } from '../prompts/system'
-import { CODE_ALLOWED_TOOLS, READONLY_ALLOWED_TOOLS, ToolName } from '../tool-lists'
+import { Mode, isToolAllowedForMode, TestToolName, getModeConfig, modes } from '../../shared/modes';
+import { validateToolUse } from '../mode-validator';
 
-// For testing purposes, we need to handle the 'unknown_tool' case
-type TestToolName = ToolName | 'unknown_tool';
-
-// Helper function to safely cast string to TestToolName for testing
-function asTestTool(str: string): TestToolName {
-    return str as TestToolName;
-}
+const asTestTool = (tool: string): TestToolName => tool as TestToolName;
+const [codeMode, architectMode, askMode] = modes.map(mode => mode.slug);
 
 describe('mode-validator', () => {
     describe('isToolAllowedForMode', () => {
         describe('code mode', () => {
             it('allows all code mode tools', () => {
-                CODE_ALLOWED_TOOLS.forEach(tool => {
+                const mode = getModeConfig(codeMode);
+                mode.tools.forEach(([tool]) => {
                     expect(isToolAllowedForMode(tool, codeMode)).toBe(true)
                 })
             })
@@ -25,64 +20,33 @@ describe('mode-validator', () => {
         })
 
         describe('architect mode', () => {
-            it('allows only read-only and MCP tools', () => {
-                // Test allowed tools
-                READONLY_ALLOWED_TOOLS.forEach(tool => {
+            it('allows configured tools', () => {
+                const mode = getModeConfig(architectMode);
+                mode.tools.forEach(([tool]) => {
                     expect(isToolAllowedForMode(tool, architectMode)).toBe(true)
                 })
-
-                // Test specific disallowed tools that we know are in CODE_ALLOWED_TOOLS but not in READONLY_ALLOWED_TOOLS
-                const disallowedTools = ['execute_command', 'write_to_file', 'apply_diff'] as const;
-                disallowedTools.forEach(tool => {
-                    expect(isToolAllowedForMode(tool as ToolName, architectMode)).toBe(false)
-                })
             })
         })
 
         describe('ask mode', () => {
-            it('allows only read-only and MCP tools', () => {
-                // Test allowed tools
-                READONLY_ALLOWED_TOOLS.forEach(tool => {
+            it('allows configured tools', () => {
+                const mode = getModeConfig(askMode);
+                mode.tools.forEach(([tool]) => {
                     expect(isToolAllowedForMode(tool, askMode)).toBe(true)
                 })
-
-                // Test specific disallowed tools that we know are in CODE_ALLOWED_TOOLS but not in READONLY_ALLOWED_TOOLS
-                const disallowedTools = ['execute_command', 'write_to_file', 'apply_diff'] as const;
-                disallowedTools.forEach(tool => {
-                    expect(isToolAllowedForMode(tool as ToolName, askMode)).toBe(false)
-                })
             })
         })
     })
 
     describe('validateToolUse', () => {
         it('throws error for disallowed tools in architect mode', () => {
-            expect(() => validateToolUse('write_to_file' as ToolName, architectMode)).toThrow(
-                'Tool "write_to_file" is not allowed in architect mode.'
+            expect(() => validateToolUse('unknown_tool', 'architect')).toThrow(
+                'Tool "unknown_tool" is not allowed in architect mode.'
             )
         })
 
-        it('throws error for disallowed tools in ask mode', () => {
-            expect(() => validateToolUse('execute_command' as ToolName, askMode)).toThrow(
-                'Tool "execute_command" is not allowed in ask mode.'
-            )
-        })
-
-        it('throws error for unknown tools in code mode', () => {
-            expect(() => validateToolUse(asTestTool('unknown_tool'), codeMode)).toThrow(
-                'Tool "unknown_tool" is not allowed in code mode.'
-            )
-        })
-
-        it('does not throw for allowed tools', () => {
-            // Code mode
-            expect(() => validateToolUse('write_to_file' as ToolName, codeMode)).not.toThrow()
-
-            // Architect mode
-            expect(() => validateToolUse('read_file' as ToolName, architectMode)).not.toThrow()
-
-            // Ask mode
-            expect(() => validateToolUse('browser_action' as ToolName, askMode)).not.toThrow()
+        it('does not throw for allowed tools in architect mode', () => {
+            expect(() => validateToolUse('read_file', 'architect')).not.toThrow()
         })
     })
 })

src/core/diff/strategies/__tests__/new-unified.test.ts

@@ -28,7 +28,7 @@ describe('main', () => {
   describe('getToolDescription', () => {
       it('should return tool description with correct cwd', () => {
           const cwd = '/test/path'
-          const description = strategy.getToolDescription(cwd)
+          const description = strategy.getToolDescription({ cwd })
 
           expect(description).toContain('apply_diff')
           expect(description).toContain(cwd)

src/core/diff/strategies/__tests__/search-replace.test.ts

@@ -1521,12 +1521,12 @@ function two() {
 
         it('should include the current working directory', async () => {
             const cwd = '/test/dir'
-            const description = await strategy.getToolDescription(cwd)
+            const description = await strategy.getToolDescription({ cwd })
             expect(description).toContain(`relative to the current working directory ${cwd}`)
         })
 
         it('should include required format elements', async () => {
-            const description = await strategy.getToolDescription('/test')
+            const description = await strategy.getToolDescription({ cwd: '/test' })
             expect(description).toContain('<<<<<<< SEARCH')
             expect(description).toContain('=======')
             expect(description).toContain('>>>>>>> REPLACE')
@@ -1535,7 +1535,7 @@ function two() {
         })
 
         it('should document start_line and end_line parameters', async () => {
-            const description = await strategy.getToolDescription('/test')
+            const description = await strategy.getToolDescription({ cwd: '/test' })
             expect(description).toContain('start_line: (required) The line number where the search block starts.')
             expect(description).toContain('end_line: (required) The line number where the search block ends.')
         })

src/core/diff/strategies/__tests__/unified.test.ts

@@ -10,7 +10,7 @@ describe('UnifiedDiffStrategy', () => {
     describe('getToolDescription', () => {
         it('should return tool description with correct cwd', () => {
             const cwd = '/test/path'
-            const description = strategy.getToolDescription(cwd)
+            const description = strategy.getToolDescription({ cwd })
 
             expect(description).toContain('apply_diff')
             expect(description).toContain(cwd)

src/core/diff/strategies/new-unified/index.ts

@@ -107,7 +107,7 @@ export class NewUnifiedDiffStrategy implements DiffStrategy {
 		return { hunks }
 	}
 
-	getToolDescription(cwd: string): string {
+	getToolDescription(args: { cwd: string; toolOptions?: { [key: string]: string } }): string {
 		return `# apply_diff Tool Rules:
 
 Generate a unified diff similar to what "diff -U0" would produce. 
@@ -173,7 +173,7 @@ Format Requirements:
 Be precise, consistent, and follow these rules carefully to generate correct diffs!
 
 Parameters:
-- path: (required) The path of the file to apply the diff to (relative to the current working directory ${cwd})
+- path: (required) The path of the file to apply the diff to (relative to the current working directory ${args.cwd})
 - diff: (required) The diff content in unified format to apply to the file.
 
 Usage:

src/core/diff/strategies/search-replace.ts

@@ -65,7 +65,7 @@ export class SearchReplaceDiffStrategy implements DiffStrategy {
         this.bufferLines = bufferLines ?? BUFFER_LINES;
     }
 
-    getToolDescription(cwd: string): string {
+    getToolDescription(args: { cwd: string; toolOptions?: { [key: string]: string } }): string {
         return `## apply_diff
 Description: Request to replace existing code using a search and replace block.
 This tool allows for precise, surgical replaces to files by specifying exactly what content to search for and what to replace it with.
@@ -76,7 +76,7 @@ If you're not confident in the exact content to search for, use the read_file to
 When applying the diffs, be extra careful to remember to change any closing brackets or other syntax that may be affected by the diff farther down in the file.
 
 Parameters:
-- path: (required) The path of the file to modify (relative to the current working directory ${cwd})
+- path: (required) The path of the file to modify (relative to the current working directory ${args.cwd})
 - diff: (required) The search/replace block defining the changes.
 - start_line: (required) The line number where the search block starts.
 - end_line: (required) The line number where the search block ends.

src/core/diff/strategies/unified.ts

@@ -2,12 +2,12 @@ import { applyPatch } from "diff"
 import { DiffStrategy, DiffResult } from "../types"
 
 export class UnifiedDiffStrategy implements DiffStrategy {
-    getToolDescription(cwd: string): string {
+    getToolDescription(args: { cwd: string; toolOptions?: { [key: string]: string } }): string {
         return `## apply_diff
 Description: Apply a unified diff to a file at the specified path. This tool is useful when you need to make specific modifications to a file based on a set of changes provided in unified diff format (diff -U3).
 
 Parameters:
-- path: (required) The path of the file to apply the diff to (relative to the current working directory ${cwd})
+- path: (required) The path of the file to apply the diff to (relative to the current working directory ${args.cwd})
 - diff: (required) The diff content in unified format to apply to the file.
 
 Format Requirements:

src/core/diff/types.ts

@@ -15,10 +15,10 @@ export type DiffResult =
 export interface DiffStrategy {
     /**
      * Get the tool description for this diff strategy
-     * @param cwd The current working directory
+     * @param args The tool arguments including cwd and toolOptions
      * @returns The complete tool description including format requirements and examples
      */
-    getToolDescription(cwd: string): string
+    getToolDescription(args: { cwd: string; toolOptions?: { [key: string]: string } }): string
 
     /**
      * Apply a diff to the original content

src/core/mode-validator.ts

@@ -1,27 +1,7 @@
-import { Mode } from './prompts/types'
-import { codeMode } from './prompts/system'
-import { CODE_ALLOWED_TOOLS, READONLY_ALLOWED_TOOLS, ToolName, ReadOnlyToolName } from './tool-lists'
+import { Mode, isToolAllowedForMode, TestToolName, getModeConfig } from '../shared/modes';
 
-// Extended tool type that includes 'unknown_tool' for testing
-export type TestToolName = ToolName | 'unknown_tool';
-
-// Type guard to check if a tool is a valid tool
-function isValidTool(tool: TestToolName): tool is ToolName {
-    return CODE_ALLOWED_TOOLS.includes(tool as ToolName);
-}
-
-// Type guard to check if a tool is a read-only tool
-function isReadOnlyTool(tool: TestToolName): tool is ReadOnlyToolName {
-    return READONLY_ALLOWED_TOOLS.includes(tool as ReadOnlyToolName);
-}
-
-export function isToolAllowedForMode(toolName: TestToolName, mode: Mode): boolean {
-    if (mode === codeMode) {
-        return isValidTool(toolName);
-    }
-    // Both architect and ask modes use the same read-only tools
-    return isReadOnlyTool(toolName);
-}
+export { isToolAllowedForMode };
+export type { TestToolName };
 
 export function validateToolUse(toolName: TestToolName, mode: Mode): void {
     if (!isToolAllowedForMode(toolName, mode)) {