From 317e5446fb1b6fdcd1abd37104671766ba8c3dd7 Mon Sep 17 00:00:00 2001 From: KJ7LNW <93454819+KJ7LNW@users.noreply.github.com> Date: Fri, 28 Mar 2025 06:39:29 -0700 Subject: [PATCH 1/4] Fix list_code_definition_names to support files (#2046) * feat: enhance list_code_definition_names to support files Fix 'cwd option must be a path to a directory' error. Add support for analyzing individual source files. Update tool description to clarify file and directory usage. Signed-off-by: Eric Wheeler * test: updated system instruction snapshots Update system instruction snapshots for list_code_definition_names tool. The snapshots now reflect enhanced documentation that clarifies the tool can analyze both individual files and directories. Signed-off-by: Eric Wheeler --------- Signed-off-by: Eric Wheeler Co-authored-by: Eric Wheeler --- src/core/Cline.ts | 32 ++- .../__snapshots__/system.test.ts.snap | 225 +++++++++++++----- .../tools/list-code-definition-names.ts | 15 +- 3 files changed, 200 insertions(+), 72 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 97170eac9b..3a232902f5 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -2508,10 +2508,10 @@ export class Cline extends EventEmitter { } } case "list_code_definition_names": { - const relDirPath: string | undefined = block.params.path + const relPath: string | undefined = block.params.path const sharedMessageProps: ClineSayTool = { tool: "listCodeDefinitionNames", - path: getReadablePath(this.cwd, removeClosingTag("path", relDirPath)), + path: getReadablePath(this.cwd, removeClosingTag("path", relPath)), } try { if (block.partial) { @@ -2522,7 +2522,7 @@ export class Cline extends EventEmitter { await this.ask("tool", partialMessage, block.partial).catch(() => {}) break } else { - if (!relDirPath) { + if (!relPath) { this.consecutiveMistakeCount++ pushToolResult( await this.sayAndCreateMissingParamError("list_code_definition_names", "path"), @@ -2530,11 +2530,27 @@ export class Cline extends EventEmitter { break } this.consecutiveMistakeCount = 0 - const absolutePath = path.resolve(this.cwd, relDirPath) - const result = await parseSourceCodeForDefinitionsTopLevel( - absolutePath, - this.rooIgnoreController, - ) + const absolutePath = path.resolve(this.cwd, relPath) + let result: string + try { + const stats = await fs.stat(absolutePath) + if (stats.isFile()) { + const fileResult = await parseSourceCodeDefinitionsForFile( + absolutePath, + this.rooIgnoreController, + ) + result = fileResult ?? "No source code definitions found in this file." + } else if (stats.isDirectory()) { + result = await parseSourceCodeForDefinitionsTopLevel( + absolutePath, + this.rooIgnoreController, + ) + } else { + result = "The specified path is neither a file nor a directory." + } + } catch { + result = `${absolutePath}: does not exist or cannot be accessed.` + } const completeMessage = JSON.stringify({ ...sharedMessageProps, content: result, diff --git a/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap b/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap index 8d17a0e9b2..dcf7f3ab33 100644 --- a/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap +++ b/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap @@ -122,17 +122,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -512,17 +519,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -991,17 +1005,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -1434,17 +1455,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -1824,17 +1852,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -2214,17 +2249,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -2604,17 +2646,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -3043,17 +3092,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -3501,17 +3557,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -3940,17 +4003,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## apply_diff @@ -4392,17 +4462,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -4824,17 +4901,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -5376,17 +5460,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file @@ -5842,17 +5933,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## ask_followup_question @@ -6206,17 +6304,24 @@ Example: Requesting to list all files in the current directory ## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory /test/path) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory /test/path) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ## write_to_file diff --git a/src/core/prompts/tools/list-code-definition-names.ts b/src/core/prompts/tools/list-code-definition-names.ts index 753ac4c44d..259c59d189 100644 --- a/src/core/prompts/tools/list-code-definition-names.ts +++ b/src/core/prompts/tools/list-code-definition-names.ts @@ -2,16 +2,23 @@ import { ToolArgs } from "./types" export function getListCodeDefinitionNamesDescription(args: ToolArgs): string { return `## list_code_definition_names -Description: Request to list definition names (classes, functions, methods, etc.) used in source code files at the top level of the specified directory. This tool provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. +Description: Request to list definition names (classes, functions, methods, etc.) from source code. This tool can analyze either a single file or all files at the top level of a specified directory. It provides insights into the codebase structure and important constructs, encapsulating high-level concepts and relationships that are crucial for understanding the overall architecture. Parameters: -- path: (required) The path of the directory (relative to the current working directory ${args.cwd}) to list top level source code definitions for. +- path: (required) The path of the file or directory (relative to the current working directory ${args.cwd}) to analyze. When given a directory, it lists definitions from all top-level source files. Usage: Directory path here -Example: Requesting to list all top level source code definitions in the current directory +Examples: + +1. List definitions from a specific file: -. +src/main.ts + + +2. List definitions from all files in a directory: + +src/ ` } From 62a7bc7959220dedd6c04e60d2b884822f4f76c6 Mon Sep 17 00:00:00 2001 From: Diarmid Mackenzie Date: Fri, 28 Mar 2025 13:50:42 +0000 Subject: [PATCH 2/4] Refactor fetch instructions to new file (#2056) * Refactor fetch_instructions handling into separate module * Use correct case for module name --- src/core/Cline.ts | 65 +++-------------------- src/core/tools/fetchInstructionsTool.ts | 69 +++++++++++++++++++++++++ src/core/tools/types.ts | 12 +++++ 3 files changed, 89 insertions(+), 57 deletions(-) create mode 100644 src/core/tools/fetchInstructionsTool.ts create mode 100644 src/core/tools/types.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 3a232902f5..ade6c37244 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -29,7 +29,7 @@ import { everyLineHasLineNumbers, } from "../integrations/misc/extract-text" import { countFileLines } from "../integrations/misc/line-counter" -import { fetchInstructions } from "./prompts/instructions/instructions" +import { fetchInstructionsTool } from "./tools/fetchInstructionsTool" import { ExitCodeDetails } from "../integrations/terminal/TerminalProcess" import { Terminal } from "../integrations/terminal/Terminal" import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry" @@ -86,7 +86,7 @@ import { readLines } from "../integrations/misc/read-lines" import { getWorkspacePath } from "../utils/path" import { isBinaryFile } from "isbinaryfile" -type ToolResponse = string | Array +export type ToolResponse = string | Array type UserContent = Array export type ClineEvents = { @@ -148,9 +148,11 @@ export class Cline extends EventEmitter { private askResponseText?: string private askResponseImages?: string[] private lastMessageTs?: number - private consecutiveMistakeCount: number = 0 + // Not private since it needs to be accessible by tools + consecutiveMistakeCount: number = 0 private consecutiveMistakeCountForApplyDiff: Map = new Map() - private providerRef: WeakRef + // Not private since it needs to be accessible by tools + providerRef: WeakRef private abort: boolean = false didFinishAbortingStream = false abandoned = false @@ -2402,59 +2404,8 @@ export class Cline extends EventEmitter { } case "fetch_instructions": { - 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 this.ask("tool", partialMessage, block.partial).catch(() => {}) - break - } else { - if (!task) { - this.consecutiveMistakeCount++ - pushToolResult( - await this.sayAndCreateMissingParamError("fetch_instructions", "task"), - ) - break - } - - this.consecutiveMistakeCount = 0 - const completeMessage = JSON.stringify({ - ...sharedMessageProps, - content: task, - } satisfies ClineSayTool) - - const didApprove = await askApproval("tool", completeMessage) - if (!didApprove) { - break - } - - // now fetch the content and provide it to the agent. - const provider = this.providerRef.deref() - const mcpHub = provider?.getMcpHub() - if (!mcpHub) { - throw new Error("MCP hub not available") - } - const diffStrategy = this.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 - } + fetchInstructionsTool(this, block, askApproval, handleError, pushToolResult) + break } case "list_files": { diff --git a/src/core/tools/fetchInstructionsTool.ts b/src/core/tools/fetchInstructionsTool.ts new file mode 100644 index 0000000000..8304c76317 --- /dev/null +++ b/src/core/tools/fetchInstructionsTool.ts @@ -0,0 +1,69 @@ +import { Cline } from "../Cline" +import { fetchInstructions } from "../prompts/instructions/instructions" +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { ToolUse } from "../assistant-message" +import { formatResponse } from "../prompts/responses" +import { AskApproval, HandleError, PushToolResult } from "./types" + +export async function fetchInstructionsTool( + cline: Cline, + block: ToolUse, + askApproval: AskApproval, + handleError: HandleError, + pushToolResult: PushToolResult, +) { + switch (true) { + default: + 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(() => {}) + 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) + + const didApprove = await askApproval("tool", completeMessage) + if (!didApprove) { + 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}`)) + break + } + pushToolResult(content) + break + } + } catch (error) { + await handleError("fetch instructions", error) + break + } + } +} diff --git a/src/core/tools/types.ts b/src/core/tools/types.ts new file mode 100644 index 0000000000..d03ddd016e --- /dev/null +++ b/src/core/tools/types.ts @@ -0,0 +1,12 @@ +import { ClineAsk, ToolProgressStatus } from "../../schemas" +import { ToolResponse } from "../Cline" + +export type AskApproval = ( + type: ClineAsk, + partialMessage?: string, + progressStatus?: ToolProgressStatus, +) => Promise + +export type HandleError = (action: string, error: Error) => void + +export type PushToolResult = (content: ToolResponse) => void From b46f6adb78ded5d20529bb42000dc23beb79f4f8 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Fri, 28 Mar 2025 11:47:47 -0400 Subject: [PATCH 3/4] Extract code for read_file from Cline (#2059) --- src/core/Cline.ts | 150 +----- .../read-file-maxReadFileLine.test.ts | 444 ++++++++---------- src/core/tools/readFileTool.ts | 168 +++++++ src/core/tools/types.ts | 3 + 4 files changed, 374 insertions(+), 391 deletions(-) create mode 100644 src/core/tools/readFileTool.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index ade6c37244..7618a640eb 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -30,6 +30,7 @@ import { } from "../integrations/misc/extract-text" import { countFileLines } from "../integrations/misc/line-counter" import { fetchInstructionsTool } from "./tools/fetchInstructionsTool" +import { readFileTool } from "./tools/readFileTool" import { ExitCodeDetails } from "../integrations/terminal/TerminalProcess" import { Terminal } from "../integrations/terminal/Terminal" import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry" @@ -82,9 +83,7 @@ import { insertGroups } from "./diff/insert-groups" import { telemetryService } from "../services/telemetry/TelemetryService" import { validateToolUse, isToolAllowedForMode, ToolName } from "./mode-validator" import { parseXml } from "../utils/xml" -import { readLines } from "../integrations/misc/read-lines" import { getWorkspacePath } from "../utils/path" -import { isBinaryFile } from "isbinaryfile" export type ToolResponse = string | Array type UserContent = Array @@ -2256,151 +2255,8 @@ export class Cline extends EventEmitter { } case "read_file": { - 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(this.cwd, removeClosingTag("path", relPath)) : "" - const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) - - const sharedMessageProps: ClineSayTool = { - tool: "readFile", - path: getReadablePath(this.cwd, removeClosingTag("path", relPath)), - isOutsideWorkspace, - } - try { - if (block.partial) { - const partialMessage = JSON.stringify({ - ...sharedMessageProps, - content: undefined, - } satisfies ClineSayTool) - await this.ask("tool", partialMessage, block.partial).catch(() => {}) - break - } else { - if (!relPath) { - this.consecutiveMistakeCount++ - pushToolResult(await this.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 - this.consecutiveMistakeCount++ - await this.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 - this.consecutiveMistakeCount++ - await this.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 = this.rooIgnoreController?.validateAccess(relPath) - if (!accessAllowed) { - await this.say("rooignore_error", relPath) - pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath))) - - break - } - - this.consecutiveMistakeCount = 0 - const absolutePath = path.resolve(this.cwd, relPath) - const completeMessage = JSON.stringify({ - ...sharedMessageProps, - content: absolutePath, - } satisfies ClineSayTool) - - const didApprove = await askApproval("tool", completeMessage) - if (!didApprove) { - break - } - - // Get the maxReadFileLine setting - const { maxReadFileLine = 500 } = (await this.providerRef.deref()?.getState()) ?? {} - - // 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, this.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 - } - } catch (error) { - await handleError("reading file", error) - break - } + readFileTool(this, block, askApproval, handleError, pushToolResult, removeClosingTag) + break } case "fetch_instructions": { diff --git a/src/core/__tests__/read-file-maxReadFileLine.test.ts b/src/core/__tests__/read-file-maxReadFileLine.test.ts index dbcacf39a2..619ae3a605 100644 --- a/src/core/__tests__/read-file-maxReadFileLine.test.ts +++ b/src/core/__tests__/read-file-maxReadFileLine.test.ts @@ -1,5 +1,3 @@ -const DEBUG = false - import * as path from "path" import { countFileLines } from "../../integrations/misc/line-counter" import { readLines } from "../../integrations/misc/read-lines" @@ -8,7 +6,6 @@ import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" import { isBinaryFile } from "isbinaryfile" import { ReadFileToolUse } from "../assistant-message" import { Cline } from "../Cline" -import { ClineProvider } from "../webview/ClineProvider" // Mock dependencies jest.mock("../../integrations/misc/line-counter") @@ -21,7 +18,7 @@ jest.mock("../ignore/RooIgnoreController", () => ({ initialize() { return Promise.resolve() } - validateAccess(filePath: string) { + validateAccess() { return true } }, @@ -45,281 +42,240 @@ jest.mock("path", () => { }) describe("read_file tool with maxReadFileLine setting", () => { - // Mock original implementation first to use in tests - const originalCountFileLines = jest.requireActual("../../integrations/misc/line-counter").countFileLines - const originalReadLines = jest.requireActual("../../integrations/misc/read-lines").readLines - const originalExtractTextFromFile = jest.requireActual("../../integrations/misc/extract-text").extractTextFromFile - const originalAddLineNumbers = jest.requireActual("../../integrations/misc/extract-text").addLineNumbers - const originalParseSourceCodeDefinitionsForFile = - jest.requireActual("../../services/tree-sitter").parseSourceCodeDefinitionsForFile - const originalIsBinaryFile = jest.requireActual("isbinaryfile").isBinaryFile - - let cline: Cline - let mockProvider: any + // Test data const testFilePath = "test/file.txt" - const absoluteFilePath = "/home/ewheeler/src/roo/roo-main/test/file.txt" + const absoluteFilePath = "/test/file.txt" const fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5" const numberedFileContent = "1 | Line 1\n2 | Line 2\n3 | Line 3\n4 | Line 4\n5 | Line 5" const sourceCodeDef = "\n\n# file.txt\n1--5 | Content" + // Mocked functions with correct types + const mockedCountFileLines = countFileLines as jest.MockedFunction + const mockedReadLines = readLines as jest.MockedFunction + const mockedExtractTextFromFile = extractTextFromFile as jest.MockedFunction + const mockedAddLineNumbers = addLineNumbers as jest.MockedFunction + const mockedParseSourceCodeDefinitionsForFile = parseSourceCodeDefinitionsForFile as jest.MockedFunction< + typeof parseSourceCodeDefinitionsForFile + > + const mockedIsBinaryFile = isBinaryFile as jest.MockedFunction + const mockedPathResolve = path.resolve as jest.MockedFunction + + // Mock instances + const mockCline: any = {} + let mockProvider: any + let toolResult: string | undefined + beforeEach(() => { - jest.resetAllMocks() + jest.clearAllMocks() - // Reset mocks to simulate original behavior - ;(countFileLines as jest.Mock).mockImplementation(originalCountFileLines) - ;(readLines as jest.Mock).mockImplementation(originalReadLines) - ;(extractTextFromFile as jest.Mock).mockImplementation(originalExtractTextFromFile) - ;(parseSourceCodeDefinitionsForFile as jest.Mock).mockImplementation(originalParseSourceCodeDefinitionsForFile) - ;(isBinaryFile as jest.Mock).mockImplementation(originalIsBinaryFile) + // Setup path resolution + mockedPathResolve.mockReturnValue(absoluteFilePath) - // Default mock implementations - ;(countFileLines as jest.Mock).mockResolvedValue(5) - ;(readLines as jest.Mock).mockResolvedValue(fileContent) - ;(extractTextFromFile as jest.Mock).mockResolvedValue(numberedFileContent) - // Use the real addLineNumbers function - ;(addLineNumbers as jest.Mock).mockImplementation(originalAddLineNumbers) - ;(parseSourceCodeDefinitionsForFile as jest.Mock).mockResolvedValue(sourceCodeDef) - ;(isBinaryFile as jest.Mock).mockResolvedValue(false) + // Setup mocks for file operations + mockedIsBinaryFile.mockResolvedValue(false) + mockedAddLineNumbers.mockImplementation((content: string, startLine = 1) => { + return content + .split("\n") + .map((line, i) => `${i + startLine} | ${line}`) + .join("\n") + }) - // Add spy to debug the readLines calls - const readLinesSpy = jest.spyOn(require("../../integrations/misc/read-lines"), "readLines") - - // Mock path.resolve to return a predictable path - ;(path.resolve as jest.Mock).mockReturnValue(absoluteFilePath) - - // Create mock provider + // Setup mock provider mockProvider = { getState: jest.fn(), deref: jest.fn().mockReturnThis(), } - // Create a Cline instance with the necessary configuration - cline = new Cline({ - provider: mockProvider, - apiConfiguration: { apiProvider: "anthropic" } as any, - task: "Test read_file tool", // Required to satisfy constructor check - startTask: false, // Prevent actual task initialization - }) + // Setup Cline instance with mock methods + mockCline.cwd = "/" + mockCline.task = "Test" + mockCline.providerRef = mockProvider + mockCline.rooIgnoreController = { + validateAccess: jest.fn().mockReturnValue(true), + } + mockCline.say = jest.fn().mockResolvedValue(undefined) + mockCline.ask = jest.fn().mockResolvedValue(true) + mockCline.presentAssistantMessage = jest.fn() - // Set up the read_file tool use - const readFileToolUse: ReadFileToolUse = { + // Reset tool result + toolResult = undefined + }) + + /** + * Helper function to execute the read file tool with different maxReadFileLine settings + */ + async function executeReadFileTool(maxReadFileLine: number, totalLines = 5): Promise { + // Configure mocks based on test scenario + mockProvider.getState.mockResolvedValue({ maxReadFileLine }) + mockedCountFileLines.mockResolvedValue(totalLines) + + // Create a tool use object + const toolUse: ReadFileToolUse = { type: "tool_use", name: "read_file", - params: { - path: testFilePath, - }, + params: { path: testFilePath }, partial: false, } - // Set up the Cline instance for testing - const clineAny = cline as any + // Import the tool implementation dynamically to avoid hoisting issues + const { readFileTool } = require("../tools/readFileTool") - // Set up the required properties for the test - clineAny.assistantMessageContent = [readFileToolUse] - clineAny.currentStreamingContentIndex = 0 - clineAny.userMessageContent = [] - clineAny.presentAssistantMessageLocked = false - clineAny.didCompleteReadingStream = true - clineAny.didRejectTool = false - clineAny.didAlreadyUseTool = false + // Execute the tool + await readFileTool( + mockCline, + toolUse, + mockCline.ask, + jest.fn(), + (result: string) => { + toolResult = result + }, + (param: string, value: string) => value, + ) - // Mock methods that would be called during presentAssistantMessage - clineAny.say = jest.fn().mockResolvedValue(undefined) - clineAny.ask = jest.fn().mockImplementation((type, message) => { - return Promise.resolve({ response: "yesButtonClicked" }) + return toolResult + } + + describe("when maxReadFileLine is negative", () => { + it("should read the entire file using extractTextFromFile", async () => { + // Setup + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(-1) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedReadLines).not.toHaveBeenCalled() + expect(mockedParseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() + expect(result).toBe(numberedFileContent) }) }) - // Helper function to get user message content - const getUserMessageContent = (clineInstance: Cline) => { - const clineAny = clineInstance as any - return clineAny.userMessageContent - } + describe("when maxReadFileLine is 0", () => { + it("should return an empty content with source code definitions", async () => { + // Setup - for maxReadFileLine = 0, the implementation won't call readLines + mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(sourceCodeDef) - // Helper function to validate response lines - const validateResponseLines = ( - responseLines: string[], - options: { - expectedLineCount: number - shouldContainLines?: number[] - shouldNotContainLines?: number[] - }, - ) => { - if (options.shouldContainLines) { - const contentLines = responseLines.filter((line) => line.includes("Line ")) - expect(contentLines.length).toBe(options.expectedLineCount) - options.shouldContainLines.forEach((lineNum) => { - expect(contentLines[lineNum - 1]).toContain(`Line ${lineNum}`) - }) - } + // Execute + const result = await executeReadFileTool(0) - if (options.shouldNotContainLines) { - options.shouldNotContainLines.forEach((lineNum) => { - expect(responseLines.some((line) => line.includes(`Line ${lineNum}`))).toBe(false) - }) - } - } + // Verify + expect(mockedExtractTextFromFile).not.toHaveBeenCalled() + expect(mockedReadLines).not.toHaveBeenCalled() // Per implementation line 141 + expect(mockedParseSourceCodeDefinitionsForFile).toHaveBeenCalledWith( + absoluteFilePath, + mockCline.rooIgnoreController, + ) + expect(result).toContain("[Showing only 0 of 5 total lines") + expect(result).toContain(sourceCodeDef) + }) + }) - interface TestExpectations { - extractTextCalled: boolean - readLinesCalled: boolean - sourceCodeDefCalled: boolean - readLinesParams?: [string, number, number] - responseValidation: { - expectedLineCount: number - shouldContainLines?: number[] - shouldNotContainLines?: number[] - } - expectedContent?: string - truncationMessage?: string - includeSourceCodeDef?: boolean - } + describe("when maxReadFileLine is less than file length", () => { + it("should read only maxReadFileLine lines and add source code definitions", async () => { + // Setup + const content = "Line 1\nLine 2\nLine 3" + mockedReadLines.mockResolvedValue(content) + mockedParseSourceCodeDefinitionsForFile.mockResolvedValue(sourceCodeDef) - interface TestCase { - name: string - maxReadFileLine: number - setup?: () => void - expectations: TestExpectations - } + // Execute + const result = await executeReadFileTool(3) - // Test cases - const testCases: TestCase[] = [ - { - name: "read entire file when maxReadFileLine is -1", - maxReadFileLine: -1, - expectations: { - extractTextCalled: true, - readLinesCalled: false, - sourceCodeDefCalled: false, - responseValidation: { - expectedLineCount: 5, - shouldContainLines: [1, 2, 3, 4, 5], + // Verify - check behavior but not specific implementation details + expect(mockedExtractTextFromFile).not.toHaveBeenCalled() + expect(mockedReadLines).toHaveBeenCalled() + expect(mockedParseSourceCodeDefinitionsForFile).toHaveBeenCalledWith( + absoluteFilePath, + mockCline.rooIgnoreController, + ) + expect(result).toContain("1 | Line 1") + expect(result).toContain("2 | Line 2") + expect(result).toContain("3 | Line 3") + expect(result).toContain("[Showing only 3 of 5 total lines") + expect(result).toContain(sourceCodeDef) + }) + }) + + describe("when maxReadFileLine equals or exceeds file length", () => { + it("should use extractTextFromFile when maxReadFileLine > totalLines", async () => { + // Setup + mockedCountFileLines.mockResolvedValue(5) // File shorter than maxReadFileLine + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(10, 5) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(result).toBe(numberedFileContent) + }) + + it("should read with extractTextFromFile when file has few lines", async () => { + // Setup + mockedCountFileLines.mockResolvedValue(3) // File shorter than maxReadFileLine + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(5, 3) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedReadLines).not.toHaveBeenCalled() + expect(result).toBe(numberedFileContent) + }) + }) + + describe("when file is binary", () => { + it("should always use extractTextFromFile regardless of maxReadFileLine", async () => { + // Setup + mockedIsBinaryFile.mockResolvedValue(true) + mockedExtractTextFromFile.mockResolvedValue(numberedFileContent) + + // Execute + const result = await executeReadFileTool(3) + + // Verify + expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) + expect(mockedReadLines).not.toHaveBeenCalled() + expect(result).toBe(numberedFileContent) + }) + }) + + describe("with range parameters", () => { + it("should honor start_line and end_line when provided", async () => { + // Setup + const rangeToolUse: ReadFileToolUse = { + type: "tool_use", + name: "read_file", + params: { + path: testFilePath, + start_line: "2", + end_line: "4", }, - expectedContent: numberedFileContent, - }, - }, - { - name: "read entire file when maxReadFileLine >= file length", - maxReadFileLine: 10, - expectations: { - extractTextCalled: true, - readLinesCalled: false, - sourceCodeDefCalled: false, - responseValidation: { - expectedLineCount: 5, - shouldContainLines: [1, 2, 3, 4, 5], - }, - expectedContent: numberedFileContent, - }, - }, - { - name: "read zero lines and only provide line declaration definitions when maxReadFileLine is 0", - maxReadFileLine: 0, - expectations: { - extractTextCalled: false, - readLinesCalled: false, - sourceCodeDefCalled: true, - responseValidation: { - expectedLineCount: 0, - }, - truncationMessage: `[Showing only 0 of 5 total lines. Use start_line and end_line if you need to read more]`, - includeSourceCodeDef: true, - }, - }, - { - name: "read maxReadFileLine lines and provide line declaration definitions when maxReadFileLine < file length", - maxReadFileLine: 3, - setup: () => { - jest.clearAllMocks() - ;(countFileLines as jest.Mock).mockResolvedValue(5) - ;(readLines as jest.Mock).mockImplementation((path, endLine, startLine = 0) => { - const lines = fileContent.split("\n") - const actualEndLine = endLine !== undefined ? Math.min(endLine, lines.length - 1) : lines.length - 1 - const actualStartLine = startLine !== undefined ? Math.min(startLine, lines.length - 1) : 0 - const requestedLines = lines.slice(actualStartLine, actualEndLine + 1) - return Promise.resolve(requestedLines.join("\n")) - }) - }, - expectations: { - extractTextCalled: false, - readLinesCalled: true, - sourceCodeDefCalled: true, - readLinesParams: [absoluteFilePath, 2, 0], - responseValidation: { - expectedLineCount: 3, - shouldContainLines: [1, 2, 3], - shouldNotContainLines: [4, 5], - }, - truncationMessage: `[Showing only 3 of 5 total lines. Use start_line and end_line if you need to read more]`, - includeSourceCodeDef: true, - }, - }, - ] - - test.each(testCases)("should $name", async (testCase) => { - // Setup - if (testCase.setup) { - testCase.setup() - } - mockProvider.getState.mockResolvedValue({ maxReadFileLine: testCase.maxReadFileLine }) - - // Execute - await cline.presentAssistantMessage() - - // Verify mock calls - if (testCase.expectations.extractTextCalled) { - expect(extractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - } else { - expect(extractTextFromFile).not.toHaveBeenCalled() - } - - if (testCase.expectations.readLinesCalled) { - const params = testCase.expectations.readLinesParams - if (!params) { - throw new Error("readLinesParams must be defined when readLinesCalled is true") + partial: false, } - expect(readLines).toHaveBeenCalledWith(...params) - } else { - expect(readLines).not.toHaveBeenCalled() - } - if (testCase.expectations.sourceCodeDefCalled) { - expect(parseSourceCodeDefinitionsForFile).toHaveBeenCalled() - } else { - expect(parseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() - } + mockedReadLines.mockResolvedValue("Line 2\nLine 3\nLine 4") - // Verify response content - const userMessageContent = getUserMessageContent(cline) + // Import the tool implementation dynamically + const { readFileTool } = require("../tools/readFileTool") - if (DEBUG) { - console.log(`\n=== Test: ${testCase.name} ===`) - console.log(`maxReadFileLine: ${testCase.maxReadFileLine}`) - console.log("Response content:", JSON.stringify(userMessageContent, null, 2)) - } - const responseLines = userMessageContent[1].text.split("\n") + // Execute the tool + let rangeResult: string | undefined + await readFileTool( + mockCline, + rangeToolUse, + mockCline.ask, + jest.fn(), + (result: string) => { + rangeResult = result + }, + (param: string, value: string) => value, + ) - if (DEBUG) { - console.log(`Number of lines in response: ${responseLines.length}`) - } - - expect(userMessageContent.length).toBe(2) - expect(userMessageContent[0].text).toBe(`[read_file for '${testFilePath}'] Result:`) - - if (testCase.expectations.expectedContent) { - expect(userMessageContent[1].text).toBe(testCase.expectations.expectedContent) - } - - if (testCase.expectations.responseValidation) { - validateResponseLines(responseLines, testCase.expectations.responseValidation) - } - - if (testCase.expectations.truncationMessage) { - expect(userMessageContent[1].text).toContain(testCase.expectations.truncationMessage) - } - - if (testCase.expectations.includeSourceCodeDef) { - expect(userMessageContent[1].text).toContain(sourceCodeDef) - } + // Verify + expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 3, 1) // end_line - 1, start_line - 1 + expect(mockedAddLineNumbers).toHaveBeenCalledWith(expect.any(String), 2) // start with proper line numbers + }) }) }) diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts new file mode 100644 index 0000000000..6616a5fcd1 --- /dev/null +++ b/src/core/tools/readFileTool.ts @@ -0,0 +1,168 @@ +import path from "path" +import { Cline } from "../Cline" +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { ToolUse } from "../assistant-message" +import { formatResponse } from "../prompts/responses" +import { AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "./types" +import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getReadablePath } from "../../utils/path" +import { countFileLines } from "../../integrations/misc/line-counter" +import { readLines } from "../../integrations/misc/read-lines" +import { extractTextFromFile, addLineNumbers } from "../../integrations/misc/extract-text" +import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter" +import { isBinaryFile } from "isbinaryfile" + +export async function readFileTool( + cline: Cline, + block: ToolUse, + askApproval: AskApproval, + handleError: HandleError, + 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, + } + try { + if (block.partial) { + const partialMessage = JSON.stringify({ + ...sharedMessageProps, + content: undefined, + } satisfies ClineSayTool) + await cline.ask("tool", partialMessage, block.partial).catch(() => {}) + break + } 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 + } + + cline.consecutiveMistakeCount = 0 + const absolutePath = path.resolve(cline.cwd, relPath) + const completeMessage = JSON.stringify({ + ...sharedMessageProps, + content: absolutePath, + } satisfies ClineSayTool) + + const didApprove = await askApproval("tool", completeMessage) + if (!didApprove) { + break + } + + // Get the maxReadFileLine setting + const { maxReadFileLine = 500 } = (await cline.providerRef.deref()?.getState()) ?? {} + + // 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 + } + } catch (error) { + await handleError("reading file", error) + break + } + } +} diff --git a/src/core/tools/types.ts b/src/core/tools/types.ts index d03ddd016e..87d7d1ed36 100644 --- a/src/core/tools/types.ts +++ b/src/core/tools/types.ts @@ -1,4 +1,5 @@ import { ClineAsk, ToolProgressStatus } from "../../schemas" +import { ToolParamName } from "../assistant-message" import { ToolResponse } from "../Cline" export type AskApproval = ( @@ -10,3 +11,5 @@ export type AskApproval = ( export type HandleError = (action: string, error: Error) => void export type PushToolResult = (content: ToolResponse) => void + +export type RemoveClosingTag = (tag: ToolParamName, content?: string) => string From e7e5511b6525e2946d4420c657b6d04c177ba19f Mon Sep 17 00:00:00 2001 From: Sam Hoang Van Date: Fri, 28 Mar 2025 23:01:16 +0700 Subject: [PATCH 4/4] feat: prioritize "Add to Context" and add line number tracking (#2063) - Move "Add to Context" to the top of submenu and code actions for improved accessibility - Add line number tracking (startLine/endLine) to EditorContext and code actions - Update templates in support-prompt.ts to include line numbers in file references - Ensure backward compatibility with existing code This change improves the UX by making the frequently used "Add to Context" action more accessible and enhances context awareness by tracking and displaying line numbers for selected code. --- package.json | 8 ++--- src/activate/registerCodeActions.ts | 8 +++-- src/core/CodeActionProvider.ts | 36 +++++++++++++------ src/core/EditorUtils.ts | 6 ++++ src/core/__tests__/CodeActionProvider.test.ts | 14 ++++---- src/shared/support-prompt.ts | 8 ++--- 6 files changed, 53 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index 830b77d2ca..cf2d7a5e66 100644 --- a/package.json +++ b/package.json @@ -185,19 +185,19 @@ ], "roo-code.contextMenu": [ { - "command": "roo-cline.explainCode", + "command": "roo-cline.addToContext", "group": "1_actions@1" }, { - "command": "roo-cline.fixCode", + "command": "roo-cline.explainCode", "group": "1_actions@2" }, { - "command": "roo-cline.improveCode", + "command": "roo-cline.fixCode", "group": "1_actions@3" }, { - "command": "roo-cline.addToContext", + "command": "roo-cline.improveCode", "group": "1_actions@4" } ], diff --git a/src/activate/registerCodeActions.ts b/src/activate/registerCodeActions.ts index 35e0766628..31f474442d 100644 --- a/src/activate/registerCodeActions.ts +++ b/src/activate/registerCodeActions.ts @@ -53,20 +53,24 @@ const registerCodeAction = ( // Handle both code action and direct command cases. let filePath: string let selectedText: string + let startLine: number | undefined + let endLine: number | undefined let diagnostics: any[] | undefined if (args.length > 1) { // Called from code action. - ;[filePath, selectedText, diagnostics] = args + ;[filePath, selectedText, startLine, endLine, diagnostics] = args } else { // Called directly from command palette. const context = EditorUtils.getEditorContext() if (!context) return - ;({ filePath, selectedText, diagnostics } = context) + ;({ filePath, selectedText, startLine, endLine, diagnostics } = context) } const params = { ...{ filePath, selectedText }, + ...(startLine !== undefined ? { startLine: startLine.toString() } : {}), + ...(endLine !== undefined ? { endLine: endLine.toString() } : {}), ...(diagnostics ? { diagnostics } : {}), ...(userInput ? { userInput } : {}), } diff --git a/src/core/CodeActionProvider.ts b/src/core/CodeActionProvider.ts index 040021a51f..f9a90e854e 100644 --- a/src/core/CodeActionProvider.ts +++ b/src/core/CodeActionProvider.ts @@ -56,10 +56,26 @@ export class CodeActionProvider implements vscode.CodeActionProvider { const filePath = EditorUtils.getFilePath(document) const actions: vscode.CodeAction[] = [] + actions.push( + this.createAction( + ACTION_NAMES.ADD_TO_CONTEXT, + vscode.CodeActionKind.QuickFix, + COMMAND_IDS.ADD_TO_CONTEXT, + [ + filePath, + effectiveRange.text, + effectiveRange.range.start.line + 1, + effectiveRange.range.end.line + 1, + ], + ), + ) + actions.push( ...this.createActionPair(ACTION_NAMES.EXPLAIN, vscode.CodeActionKind.QuickFix, COMMAND_IDS.EXPLAIN, [ filePath, effectiveRange.text, + effectiveRange.range.start.line + 1, + effectiveRange.range.end.line + 1, ]), ) @@ -74,6 +90,8 @@ export class CodeActionProvider implements vscode.CodeActionProvider { ...this.createActionPair(ACTION_NAMES.FIX, vscode.CodeActionKind.QuickFix, COMMAND_IDS.FIX, [ filePath, effectiveRange.text, + effectiveRange.range.start.line + 1, + effectiveRange.range.end.line + 1, diagnosticMessages, ]), ) @@ -83,6 +101,8 @@ export class CodeActionProvider implements vscode.CodeActionProvider { ...this.createActionPair(ACTION_NAMES.FIX_LOGIC, vscode.CodeActionKind.QuickFix, COMMAND_IDS.FIX, [ filePath, effectiveRange.text, + effectiveRange.range.start.line + 1, + effectiveRange.range.end.line + 1, ]), ) } @@ -92,16 +112,12 @@ export class CodeActionProvider implements vscode.CodeActionProvider { ACTION_NAMES.IMPROVE, vscode.CodeActionKind.RefactorRewrite, COMMAND_IDS.IMPROVE, - [filePath, effectiveRange.text], - ), - ) - - actions.push( - this.createAction( - ACTION_NAMES.ADD_TO_CONTEXT, - vscode.CodeActionKind.QuickFix, - COMMAND_IDS.ADD_TO_CONTEXT, - [filePath, effectiveRange.text], + [ + filePath, + effectiveRange.text, + effectiveRange.range.start.line + 1, + effectiveRange.range.end.line + 1, + ], ), ) diff --git a/src/core/EditorUtils.ts b/src/core/EditorUtils.ts index ee81353b7b..eb7aa6c800 100644 --- a/src/core/EditorUtils.ts +++ b/src/core/EditorUtils.ts @@ -38,6 +38,10 @@ export interface EditorContext { filePath: string /** The effective text selected or derived from the document. */ selectedText: string + /** The starting line number of the selected text (1-based). */ + startLine: number + /** The ending line number of the selected text (1-based). */ + endLine: number /** Optional list of diagnostics associated with the effective range. */ diagnostics?: DiagnosticData[] } @@ -194,6 +198,8 @@ export class EditorUtils { return { filePath, selectedText: effectiveRange.text, + startLine: effectiveRange.range.start.line + 1, // Convert to 1-based line numbers + endLine: effectiveRange.range.end.line + 1, // Convert to 1-based line numbers ...(diagnostics.length > 0 ? { diagnostics } : {}), } } catch (error) { diff --git a/src/core/__tests__/CodeActionProvider.test.ts b/src/core/__tests__/CodeActionProvider.test.ts index 6042f41b2b..6ea2adf894 100644 --- a/src/core/__tests__/CodeActionProvider.test.ts +++ b/src/core/__tests__/CodeActionProvider.test.ts @@ -75,13 +75,13 @@ describe("CodeActionProvider", () => { const actions = provider.provideCodeActions(mockDocument, mockRange, mockContext) expect(actions).toHaveLength(7) // 2 explain + 2 fix logic + 2 improve + 1 add to context - expect((actions as any)[0].title).toBe(`${ACTION_NAMES.EXPLAIN} in New Task`) - expect((actions as any)[1].title).toBe(`${ACTION_NAMES.EXPLAIN} in Current Task`) - expect((actions as any)[2].title).toBe(`${ACTION_NAMES.FIX_LOGIC} in New Task`) - expect((actions as any)[3].title).toBe(`${ACTION_NAMES.FIX_LOGIC} in Current Task`) - expect((actions as any)[4].title).toBe(`${ACTION_NAMES.IMPROVE} in New Task`) - expect((actions as any)[5].title).toBe(`${ACTION_NAMES.IMPROVE} in Current Task`) - expect((actions as any)[6].title).toBe(ACTION_NAMES.ADD_TO_CONTEXT) + expect((actions as any)[0].title).toBe(ACTION_NAMES.ADD_TO_CONTEXT) + expect((actions as any)[1].title).toBe(`${ACTION_NAMES.EXPLAIN} in New Task`) + expect((actions as any)[2].title).toBe(`${ACTION_NAMES.EXPLAIN} in Current Task`) + expect((actions as any)[3].title).toBe(`${ACTION_NAMES.FIX_LOGIC} in New Task`) + expect((actions as any)[4].title).toBe(`${ACTION_NAMES.FIX_LOGIC} in Current Task`) + expect((actions as any)[5].title).toBe(`${ACTION_NAMES.IMPROVE} in New Task`) + expect((actions as any)[6].title).toBe(`${ACTION_NAMES.IMPROVE} in Current Task`) }) it("should provide fix action instead of fix logic when diagnostics exist", () => { diff --git a/src/shared/support-prompt.ts b/src/shared/support-prompt.ts index cc5e3e1d0d..d6391ff380 100644 --- a/src/shared/support-prompt.ts +++ b/src/shared/support-prompt.ts @@ -35,7 +35,7 @@ const supportPromptConfigs: Record = { \${userInput}`, }, EXPLAIN: { - template: `Explain the following code from file path @/\${filePath}: + template: `Explain the following code from file path @/\${filePath} \${startLine}:\${endLine} \${userInput} \`\`\` @@ -48,7 +48,7 @@ Please provide a clear and concise explanation of what this code does, including 3. Important patterns or techniques used`, }, FIX: { - template: `Fix any issues in the following code from file path @/\${filePath} + template: `Fix any issues in the following code from file path @/\${filePath} \${startLine}:\${endLine} \${diagnosticText} \${userInput} @@ -63,7 +63,7 @@ Please: 4. Explain what was fixed and why`, }, IMPROVE: { - template: `Improve the following code from file path @/\${filePath}: + template: `Improve the following code from file path @/\${filePath} \${startLine}:\${endLine} \${userInput} \`\`\` @@ -79,7 +79,7 @@ Please suggest improvements for: Provide the improved code along with explanations for each enhancement.`, }, ADD_TO_CONTEXT: { - template: `\${filePath}: + template: `\${filePath}:\${startLine}:\${endLine} \`\`\` \${selectedText} \`\`\``,