refactor: address inline review comments

- Extract edit operation parameters into EDIT_OPERATION_PARAMS constant for better maintainability
- Improve XML parsing robustness with better validation and error handling
- Add tool name to FileRestrictionError for better debugging
- Add comprehensive test coverage for new error message formats
- Ensure FileRestrictionError is properly re-thrown during XML validation

Addresses feedback from @daniel-lxs in PR review comments

Author: roomote

src/shared/__tests__/modes.spec.ts

@@ -375,6 +375,14 @@ describe("FileRestrictionError", () => {
 		expect(error.name).toBe("FileRestrictionError")
 	})
 
+	it("formats error message with tool name when provided", () => {
+		const error = new FileRestrictionError("Markdown Editor", "\\.md$", undefined, "test.js", "write_to_file")
+		expect(error.message).toBe(
+			"Tool 'write_to_file' in mode 'Markdown Editor' can only edit files matching pattern: \\.md$. Got: test.js",
+		)
+		expect(error.name).toBe("FileRestrictionError")
+	})
+
 	describe("debug mode", () => {
 		it("is configured correctly", () => {
 			const debugMode = modes.find((mode) => mode.slug === "debug")
@@ -474,6 +482,20 @@ describe("FileRestrictionError", () => {
 		)
 		expect(error.name).toBe("FileRestrictionError")
 	})
+
+	it("formats error message with both tool name and description when provided", () => {
+		const error = new FileRestrictionError(
+			"Markdown Editor",
+			"\\.md$",
+			"Markdown files only",
+			"test.js",
+			"apply_diff",
+		)
+		expect(error.message).toBe(
+			"Tool 'apply_diff' in mode 'Markdown Editor' can only edit files matching pattern: \\.md$ (Markdown files only). Got: test.js",
+		)
+		expect(error.name).toBe("FileRestrictionError")
+	})
 })
 
 describe("getModeSelection", () => {

src/shared/modes.ts

@@ -203,11 +203,15 @@ export function getModeSelection(mode: string, promptComponent?: PromptComponent
 	}
 }
 
+// Edit operation parameters that indicate an actual edit operation
+const EDIT_OPERATION_PARAMS = ["diff", "content", "operations", "search", "replace", "args", "line"] as const
+
 // Custom error class for file restrictions
 export class FileRestrictionError extends Error {
-	constructor(mode: string, pattern: string, description: string | undefined, filePath: string) {
+	constructor(mode: string, pattern: string, description: string | undefined, filePath: string, tool?: string) {
+		const toolInfo = tool ? `Tool '${tool}' in mode '${mode}'` : `This mode (${mode})`
 		super(
-			`This mode (${mode}) can only edit files matching pattern: ${pattern}${description ? ` (${description})` : ""}. Got: ${filePath}`,
+			`${toolInfo} can only edit files matching pattern: ${pattern}${description ? ` (${description})` : ""}. Got: ${filePath}`,
 		)
 		this.name = "FileRestrictionError"
 	}
@@ -267,36 +271,46 @@ export function isToolAllowedForMode(
 		if (groupName === "edit" && options.fileRegex) {
 			const filePath = toolParams?.path
 			// Check if this is an actual edit operation (not just path-only for streaming)
-			const isEditOperation = !!(
-				toolParams?.diff ||
-				toolParams?.content ||
-				toolParams?.operations ||
-				toolParams?.search ||
-				toolParams?.replace ||
-				toolParams?.args
-			)
+			const isEditOperation = EDIT_OPERATION_PARAMS.some((param) => toolParams?.[param])
 
 			// Handle single file path validation
 			if (filePath && isEditOperation && !doesFileMatchRegex(filePath, options.fileRegex)) {
-				throw new FileRestrictionError(mode.name, options.fileRegex, options.description, filePath)
+				throw new FileRestrictionError(mode.name, options.fileRegex, options.description, filePath, tool)
 			}
 
 			// Handle XML args parameter (used by MULTI_FILE_APPLY_DIFF experiment)
 			if (toolParams?.args && typeof toolParams.args === "string") {
-				// Extract file paths from XML args
-				const filePathMatches = toolParams.args.match(/<path>([^<]+)<\/path>/g)
-				if (filePathMatches) {
-					for (const match of filePathMatches) {
-						const extractedPath = match.replace(/<\/?path>/g, "")
-						if (!doesFileMatchRegex(extractedPath, options.fileRegex)) {
-							throw new FileRestrictionError(
-								mode.name,
-								options.fileRegex,
-								options.description,
-								extractedPath,
-							)
+				// Extract file paths from XML args with improved validation
+				try {
+					const filePathMatches = toolParams.args.match(/<path>([^<]+)<\/path>/g)
+					if (filePathMatches) {
+						for (const match of filePathMatches) {
+							// More robust path extraction with validation
+							const pathMatch = match.match(/<path>([^<]+)<\/path>/)
+							if (pathMatch && pathMatch[1]) {
+								const extractedPath = pathMatch[1].trim()
+								// Validate that the path is not empty and doesn't contain invalid characters
+								if (extractedPath && !extractedPath.includes("<") && !extractedPath.includes(">")) {
+									if (!doesFileMatchRegex(extractedPath, options.fileRegex)) {
+										throw new FileRestrictionError(
+											mode.name,
+											options.fileRegex,
+											options.description,
+											extractedPath,
+											tool,
+										)
+									}
+								}
+							}
 						}
 					}
+				} catch (error) {
+					// Re-throw FileRestrictionError as it's an expected validation error
+					if (error instanceof FileRestrictionError) {
+						throw error
+					}
+					// If XML parsing fails, log the error but don't block the operation
+					console.warn(`Failed to parse XML args for file restriction validation: ${error}`)
 				}
 			}
 		}