Remove switch from tools (#2091)

* HandleError returns a promise

* Remove unnecessary switch from tools

src/core/tools/fetchInstructionsTool.ts

@@ -12,58 +12,53 @@ export async function fetchInstructionsTool(
 	handleError: HandleError,
 	pushToolResult: PushToolResult,
 ) {
-	switch (true) {
-		default:
-			const task: string | undefined = block.params.task
-			const sharedMessageProps: ClineSayTool = {
-				tool: "fetchInstructions",
-				content: task,
+	const task: string | undefined = block.params.task
+	const sharedMessageProps: ClineSayTool = {
+		tool: "fetchInstructions",
+		content: task,
+	}
+	try {
+		if (block.partial) {
+			const partialMessage = JSON.stringify({
+				...sharedMessageProps,
+				content: undefined,
+			} satisfies ClineSayTool)
+			await cline.ask("tool", partialMessage, block.partial).catch(() => {})
+			return
+		} else {
+			if (!task) {
+				cline.consecutiveMistakeCount++
+				pushToolResult(await cline.sayAndCreateMissingParamError("fetch_instructions", "task"))
+				return
 			}
-			try {
-				if (block.partial) {
-					const partialMessage = JSON.stringify({
-						...sharedMessageProps,
-						content: undefined,
-					} satisfies ClineSayTool)
-					await cline.ask("tool", partialMessage, block.partial).catch(() => {})
-					break
-				} else {
-					if (!task) {
-						cline.consecutiveMistakeCount++
-						pushToolResult(await cline.sayAndCreateMissingParamError("fetch_instructions", "task"))
-						break
-					}
 
-					cline.consecutiveMistakeCount = 0
-					const completeMessage = JSON.stringify({
-						...sharedMessageProps,
-						content: task,
-					} satisfies ClineSayTool)
+			cline.consecutiveMistakeCount = 0
+			const completeMessage = JSON.stringify({
+				...sharedMessageProps,
+				content: task,
+			} satisfies ClineSayTool)
 
-					const didApprove = await askApproval("tool", completeMessage)
-					if (!didApprove) {
-						break
-					}
+			const didApprove = await askApproval("tool", completeMessage)
+			if (!didApprove) {
+				return
+			}
 
-					// now fetch the content and provide it to the agent.
-					const provider = cline.providerRef.deref()
-					const mcpHub = provider?.getMcpHub()
-					if (!mcpHub) {
-						throw new Error("MCP hub not available")
-					}
-					const diffStrategy = cline.diffStrategy
-					const context = provider?.context
-					const content = await fetchInstructions(task, { mcpHub, diffStrategy, context })
-					if (!content) {
-						pushToolResult(formatResponse.toolError(`Invalid instructions request: ${task}`))
-						break
-					}
-					pushToolResult(content)
-					break
-				}
-			} catch (error) {
-				await handleError("fetch instructions", error)
-				break
+			// now fetch the content and provide it to the agent.
+			const provider = cline.providerRef.deref()
+			const mcpHub = provider?.getMcpHub()
+			if (!mcpHub) {
+				throw new Error("MCP hub not available")
+			}
+			const diffStrategy = cline.diffStrategy
+			const context = provider?.context
+			const content = await fetchInstructions(task, { mcpHub, diffStrategy, context })
+			if (!content) {
+				pushToolResult(formatResponse.toolError(`Invalid instructions request: ${task}`))
+				return
 			}
+			pushToolResult(content)
+		}
+	} catch (error) {
+		await handleError("fetch instructions", error)
 	}
 }

src/core/tools/listFilesTool.ts

@@ -69,10 +69,8 @@ export async function listFilesTool(
 				return
 			}
 			pushToolResult(result)
-			return
 		}
 	} catch (error) {
 		await handleError("listing files", error)
-		return
 	}
 }

src/core/tools/readFileTool.ts

@@ -21,164 +21,159 @@ export async function readFileTool(
 	pushToolResult: PushToolResult,
 	removeClosingTag: RemoveClosingTag,
 ) {
-	switch (true) {
-		default:
-			const relPath: string | undefined = block.params.path
-			const startLineStr: string | undefined = block.params.start_line
-			const endLineStr: string | undefined = block.params.end_line
-
-			// Get the full path and determine if it's outside the workspace
-			const fullPath = relPath ? path.resolve(cline.cwd, removeClosingTag("path", relPath)) : ""
-			const isOutsideWorkspace = isPathOutsideWorkspace(fullPath)
-
-			const sharedMessageProps: ClineSayTool = {
-				tool: "readFile",
-				path: getReadablePath(cline.cwd, removeClosingTag("path", relPath)),
-				isOutsideWorkspace,
+	const relPath: string | undefined = block.params.path
+	const startLineStr: string | undefined = block.params.start_line
+	const endLineStr: string | undefined = block.params.end_line
+
+	// Get the full path and determine if it's outside the workspace
+	const fullPath = relPath ? path.resolve(cline.cwd, removeClosingTag("path", relPath)) : ""
+	const isOutsideWorkspace = isPathOutsideWorkspace(fullPath)
+
+	const sharedMessageProps: ClineSayTool = {
+		tool: "readFile",
+		path: getReadablePath(cline.cwd, removeClosingTag("path", relPath)),
+		isOutsideWorkspace,
+	}
+	try {
+		if (block.partial) {
+			const partialMessage = JSON.stringify({
+				...sharedMessageProps,
+				content: undefined,
+			} satisfies ClineSayTool)
+			await cline.ask("tool", partialMessage, block.partial).catch(() => {})
+			return
+		} else {
+			if (!relPath) {
+				cline.consecutiveMistakeCount++
+				pushToolResult(await cline.sayAndCreateMissingParamError("read_file", "path"))
+				return
+			}
+
+			// Check if we're doing a line range read
+			let isRangeRead = false
+			let startLine: number | undefined = undefined
+			let endLine: number | undefined = undefined
+
+			// Check if we have either range parameter
+			if (startLineStr || endLineStr) {
+				isRangeRead = true
+			}
+
+			// Parse start_line if provided
+			if (startLineStr) {
+				startLine = parseInt(startLineStr)
+				if (isNaN(startLine)) {
+					// Invalid start_line
+					cline.consecutiveMistakeCount++
+					await cline.say("error", `Failed to parse start_line: ${startLineStr}`)
+					pushToolResult(formatResponse.toolError("Invalid start_line value"))
+					return
+				}
+				startLine -= 1 // Convert to 0-based index
+			}
+
+			// Parse end_line if provided
+			if (endLineStr) {
+				endLine = parseInt(endLineStr)
+
+				if (isNaN(endLine)) {
+					// Invalid end_line
+					cline.consecutiveMistakeCount++
+					await cline.say("error", `Failed to parse end_line: ${endLineStr}`)
+					pushToolResult(formatResponse.toolError("Invalid end_line value"))
+					return
+				}
+
+				// Convert to 0-based index
+				endLine -= 1
+			}
+
+			const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath)
+			if (!accessAllowed) {
+				await cline.say("rooignore_error", relPath)
+				pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
+
+				return
+			}
+
+			const { maxReadFileLine = 500 } = (await cline.providerRef.deref()?.getState()) ?? {}
+
+			// Create line snippet description for approval message
+			let lineSnippet = ""
+			if (startLine !== undefined && endLine !== undefined) {
+				lineSnippet = t("tools:readFile.linesRange", { start: startLine + 1, end: endLine + 1 })
+			} else if (startLine !== undefined) {
+				lineSnippet = t("tools:readFile.linesFromToEnd", { start: startLine + 1 })
+			} else if (endLine !== undefined) {
+				lineSnippet = t("tools:readFile.linesFromStartTo", { end: endLine + 1 })
+			} else if (maxReadFileLine === 0) {
+				lineSnippet = t("tools:readFile.definitionsOnly")
+			} else if (maxReadFileLine > 0) {
+				lineSnippet = t("tools:readFile.maxLines", { max: maxReadFileLine })
+			}
+
+			cline.consecutiveMistakeCount = 0
+			const absolutePath = path.resolve(cline.cwd, relPath)
+
+			const completeMessage = JSON.stringify({
+				...sharedMessageProps,
+				content: absolutePath,
+				reason: lineSnippet,
+			} satisfies ClineSayTool)
+
+			const didApprove = await askApproval("tool", completeMessage)
+			if (!didApprove) {
+				return
 			}
+
+			// Count total lines in the file
+			let totalLines = 0
 			try {
-				if (block.partial) {
-					const partialMessage = JSON.stringify({
-						...sharedMessageProps,
-						content: undefined,
-					} satisfies ClineSayTool)
-					await cline.ask("tool", partialMessage, block.partial).catch(() => {})
-					break
+				totalLines = await countFileLines(absolutePath)
+			} catch (error) {
+				console.error(`Error counting lines in file ${absolutePath}:`, error)
+			}
+
+			// now execute the tool like normal
+			let content: string
+			let isFileTruncated = false
+			let sourceCodeDef = ""
+
+			const isBinary = await isBinaryFile(absolutePath).catch(() => false)
+
+			if (isRangeRead) {
+				if (startLine === undefined) {
+					content = addLineNumbers(await readLines(absolutePath, endLine, startLine))
 				} else {
-					if (!relPath) {
-						cline.consecutiveMistakeCount++
-						pushToolResult(await cline.sayAndCreateMissingParamError("read_file", "path"))
-						break
-					}
-
-					// Check if we're doing a line range read
-					let isRangeRead = false
-					let startLine: number | undefined = undefined
-					let endLine: number | undefined = undefined
-
-					// Check if we have either range parameter
-					if (startLineStr || endLineStr) {
-						isRangeRead = true
-					}
-
-					// Parse start_line if provided
-					if (startLineStr) {
-						startLine = parseInt(startLineStr)
-						if (isNaN(startLine)) {
-							// Invalid start_line
-							cline.consecutiveMistakeCount++
-							await cline.say("error", `Failed to parse start_line: ${startLineStr}`)
-							pushToolResult(formatResponse.toolError("Invalid start_line value"))
-							break
-						}
-						startLine -= 1 // Convert to 0-based index
-					}
-
-					// Parse end_line if provided
-					if (endLineStr) {
-						endLine = parseInt(endLineStr)
-
-						if (isNaN(endLine)) {
-							// Invalid end_line
-							cline.consecutiveMistakeCount++
-							await cline.say("error", `Failed to parse end_line: ${endLineStr}`)
-							pushToolResult(formatResponse.toolError("Invalid end_line value"))
-							break
-						}
-
-						// Convert to 0-based index
-						endLine -= 1
-					}
-
-					const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath)
-					if (!accessAllowed) {
-						await cline.say("rooignore_error", relPath)
-						pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
-
-						break
-					}
-
-					const { maxReadFileLine = 500 } = (await cline.providerRef.deref()?.getState()) ?? {}
-
-					// Create line snippet description for approval message
-					let lineSnippet = ""
-					if (startLine !== undefined && endLine !== undefined) {
-						lineSnippet = t("tools:readFile.linesRange", { start: startLine + 1, end: endLine + 1 })
-					} else if (startLine !== undefined) {
-						lineSnippet = t("tools:readFile.linesFromToEnd", { start: startLine + 1 })
-					} else if (endLine !== undefined) {
-						lineSnippet = t("tools:readFile.linesFromStartTo", { end: endLine + 1 })
-					} else if (maxReadFileLine === 0) {
-						lineSnippet = t("tools:readFile.definitionsOnly")
-					} else if (maxReadFileLine > 0) {
-						lineSnippet = t("tools:readFile.maxLines", { max: maxReadFileLine })
-					}
-
-					cline.consecutiveMistakeCount = 0
-					const absolutePath = path.resolve(cline.cwd, relPath)
-
-					const completeMessage = JSON.stringify({
-						...sharedMessageProps,
-						content: absolutePath,
-						reason: lineSnippet,
-					} satisfies ClineSayTool)
-
-					const didApprove = await askApproval("tool", completeMessage)
-					if (!didApprove) {
-						break
-					}
-
-					// Count total lines in the file
-					let totalLines = 0
-					try {
-						totalLines = await countFileLines(absolutePath)
-					} catch (error) {
-						console.error(`Error counting lines in file ${absolutePath}:`, error)
-					}
-
-					// now execute the tool like normal
-					let content: string
-					let isFileTruncated = false
-					let sourceCodeDef = ""
-
-					const isBinary = await isBinaryFile(absolutePath).catch(() => false)
-
-					if (isRangeRead) {
-						if (startLine === undefined) {
-							content = addLineNumbers(await readLines(absolutePath, endLine, startLine))
-						} else {
-							content = addLineNumbers(await readLines(absolutePath, endLine, startLine), startLine + 1)
-						}
-					} else if (!isBinary && maxReadFileLine >= 0 && totalLines > maxReadFileLine) {
-						// If file is too large, only read the first maxReadFileLine lines
-						isFileTruncated = true
-
-						const res = await Promise.all([
-							maxReadFileLine > 0 ? readLines(absolutePath, maxReadFileLine - 1, 0) : "",
-							parseSourceCodeDefinitionsForFile(absolutePath, cline.rooIgnoreController),
-						])
-
-						content = res[0].length > 0 ? addLineNumbers(res[0]) : ""
-						const result = res[1]
-						if (result) {
-							sourceCodeDef = `\n\n${result}`
-						}
-					} else {
-						// Read entire file
-						content = await extractTextFromFile(absolutePath)
-					}
-
-					// Add truncation notice if applicable
-					if (isFileTruncated) {
-						content += `\n\n[Showing only ${maxReadFileLine} of ${totalLines} total lines. Use start_line and end_line if you need to read more]${sourceCodeDef}`
-					}
-
-					pushToolResult(content)
-					break
+					content = addLineNumbers(await readLines(absolutePath, endLine, startLine), startLine + 1)
 				}
-			} catch (error) {
-				await handleError("reading file", error)
-				break
+			} else if (!isBinary && maxReadFileLine >= 0 && totalLines > maxReadFileLine) {
+				// If file is too large, only read the first maxReadFileLine lines
+				isFileTruncated = true
+
+				const res = await Promise.all([
+					maxReadFileLine > 0 ? readLines(absolutePath, maxReadFileLine - 1, 0) : "",
+					parseSourceCodeDefinitionsForFile(absolutePath, cline.rooIgnoreController),
+				])
+
+				content = res[0].length > 0 ? addLineNumbers(res[0]) : ""
+				const result = res[1]
+				if (result) {
+					sourceCodeDef = `\n\n${result}`
+				}
+			} else {
+				// Read entire file
+				content = await extractTextFromFile(absolutePath)
+			}
+
+			// Add truncation notice if applicable
+			if (isFileTruncated) {
+				content += `\n\n[Showing only ${maxReadFileLine} of ${totalLines} total lines. Use start_line and end_line if you need to read more]${sourceCodeDef}`
 			}
+
+			pushToolResult(content)
+		}
+	} catch (error) {
+		await handleError("reading file", error)
 	}
 }

src/core/tools/types.ts

@@ -8,7 +8,7 @@ export type AskApproval = (
 	progressStatus?: ToolProgressStatus,
 ) => Promise<boolean>
 
-export type HandleError = (action: string, error: Error) => void
+export type HandleError = (action: string, error: Error) => Promise<void>
 
 export type PushToolResult = (content: ToolResponse) => void