From de5101f216a38483d59f5cfc2d897005da6000c4 Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Wed, 20 May 2026 19:00:26 +1000 Subject: [PATCH] BRAINROT P6: wire developer.read with default 2000-line cap Restores the 'read' tool on DeveloperClient. The handler was removed in f2ad0a852 because ACP clients delegate filesystem I/O and intercept 'read' at the AcpTools layer. Non-ACP sessions were left without a structured read tool, forcing the model into shell({command:'cat ...'}) which has no line/limit knobs. Changes: - 'read' tool registered on DeveloperClient with FileReadParams schema. - DEFAULT_FILE_READ_LIMIT = 2000 lines applied when caller passes no 'limit'. When truncation occurs, the result body is followed by: [goose: showing lines X-Y of Z (N more lines not shown). Pass 'line' and/or 'limit' to read a different range.] - developer_instructions text on both POSIX and Windows updated to tell the model to prefer 'read' over 'cat'/'sed' / 'type'/'Get-Content'. - ACP layer is unchanged: when fs_read is enabled, AcpTools still intercepts 'read' and routes to the ACP client; otherwise it falls through to the developer impl with the new cap. Tests updated: previous read tests asserted exact untruncated bodies; they're replaced with three tests covering (a) small file no marker, (b) explicit range emits marker, (c) default cap truncates a 5000-line file at line 2000 with marker. Signed-off-by: Michael Neale --- .../platform_extensions/developer/edit.rs | 67 +++++++++++++++++-- .../platform_extensions/developer/mod.rs | 33 +++++++-- 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/crates/goose/src/agents/platform_extensions/developer/edit.rs b/crates/goose/src/agents/platform_extensions/developer/edit.rs index 2ab34dc789..3fd1381c93 100644 --- a/crates/goose/src/agents/platform_extensions/developer/edit.rs +++ b/crates/goose/src/agents/platform_extensions/developer/edit.rs @@ -7,6 +7,10 @@ use serde::Deserialize; const NO_MATCH_PREVIEW_LINES: usize = 20; +/// Default line cap for `file_read` when the caller passes no `limit`. Mirrors +/// pi's `read` default. The model can pass an explicit `limit` to override. +pub const DEFAULT_FILE_READ_LIMIT: u32 = 2000; + #[derive(Debug, Deserialize, JsonSchema)] pub struct FileReadParams { /// Absolute path to the file to read. @@ -47,8 +51,32 @@ impl EditTools { match fs::read_to_string(&path) { Ok(content) => { - let content = apply_line_limit(&content, params.line, params.limit); - CallToolResult::success(vec![Content::text(content).with_priority(0.0)]) + // If the model didn't pass a limit, apply the default cap so + // huge files don't dump megabytes into context. The marker + // tells the model how to fetch more if it needs it. + let effective_limit = params.limit.or(Some(DEFAULT_FILE_READ_LIMIT)); + let body = apply_line_limit(&content, params.line, effective_limit); + let total_lines = content.lines().count() as u32; + let start_line = params.line.unwrap_or(1); + let end_line = start_line + .saturating_add(effective_limit.unwrap_or(0).saturating_sub(1)) + .min(total_lines); + let was_truncated = end_line < total_lines || start_line > 1; + let text = if was_truncated { + let omitted = + total_lines.saturating_sub(end_line) + start_line.saturating_sub(1); + format!( + "{body}\n[goose: showing lines {start}-{end} of {total} ({omitted} more lines not shown). \ +Pass `line` and/or `limit` to read a different range.]", + start = start_line, + end = end_line, + total = total_lines, + omitted = omitted, + ) + } else { + body + }; + CallToolResult::success(vec![Content::text(text).with_priority(0.0)]) } Err(error) => CallToolResult::error(vec![Content::text(format!( "Failed to read {}: {}", @@ -310,7 +338,7 @@ mod tests { } #[test] - fn test_file_read() { + fn test_file_read_small_file_no_truncation_marker() { let dir = setup(); let path = dir.path().join("read.txt"); fs::write(&path, "line1\nline2\nline3").unwrap(); @@ -326,11 +354,12 @@ mod tests { ); assert!(!result.is_error.unwrap_or(false)); + // File fits within the default 2000-line cap, so no marker. assert_eq!(extract_text(&result), "line1\nline2\nline3"); } #[test] - fn test_file_read_partial() { + fn test_file_read_partial_emits_marker() { let dir = setup(); let path = dir.path().join("read.txt"); fs::write(&path, "line1\nline2\nline3").unwrap(); @@ -346,7 +375,35 @@ mod tests { ); assert!(!result.is_error.unwrap_or(false)); - assert_eq!(extract_text(&result), "line2\n"); + let text = extract_text(&result); + assert!(text.starts_with("line2\n"), "body: {text}"); + assert!(text.contains("[goose: showing lines 2-2 of 3")); + assert!(text.contains("2 more lines not shown")); + } + + #[test] + fn test_file_read_default_cap_truncates_huge_file() { + let dir = setup(); + let path = dir.path().join("big.txt"); + let big: String = (1..=5000).map(|i| format!("line{i}\n")).collect(); + fs::write(&path, &big).unwrap(); + let tools = EditTools::new(); + + let result = tools.file_read_with_cwd( + FileReadParams { + path: path.to_string_lossy().to_string(), + line: None, + limit: None, + }, + None, + ); + assert!(!result.is_error.unwrap_or(false)); + let text = extract_text(&result); + // Should have stopped around line 2000 and emitted a marker. + assert!(text.contains("line2000\n")); + assert!(!text.contains("line2001\n"), "should not include line 2001"); + assert!(text.contains("[goose: showing lines 1-2000 of 5000")); + assert!(text.contains("3000 more lines not shown")); } #[test] diff --git a/crates/goose/src/agents/platform_extensions/developer/mod.rs b/crates/goose/src/agents/platform_extensions/developer/mod.rs index 2a171fca7c..40ddf3a04e 100644 --- a/crates/goose/src/agents/platform_extensions/developer/mod.rs +++ b/crates/goose/src/agents/platform_extensions/developer/mod.rs @@ -7,7 +7,7 @@ use crate::agents::mcp_client::{Error, McpClientTrait}; use crate::agents::ToolCallContext; use anyhow::Result; use async_trait::async_trait; -use edit::{EditTools, FileEditParams, FileWriteParams}; +use edit::{EditTools, FileEditParams, FileReadParams, FileWriteParams}; use indoc::indoc; use rmcp::model::{ CallToolResult, Content, Implementation, InitializeResult, JsonObject, ListToolsResult, @@ -41,8 +41,9 @@ fn developer_instructions() -> &'static str { For editing software, prefer the flow of using tree to understand the codebase structure and file sizes. When you need to search, prefer findstr or Select-String (via shell). - Then use type or Get-Content to gather the context you need, always reading before - editing. Use write and edit to efficiently make changes. Test and verify as appropriate. + Use the `read` tool (with `line`/`limit` for large files) to fetch file contents — it's + cheaper than spawning a shell. Always read before editing. Use write and edit to + efficiently make changes. Test and verify as appropriate. "} } else { indoc! {" @@ -55,8 +56,9 @@ fn developer_instructions() -> &'static str { For editing software, prefer the flow of using tree to understand the codebase structure and file sizes. When you need to search, prefer rg which correctly respects gitignored - content. Then use cat or sed to gather the context you need, always reading before editing. - Use write and edit to efficiently make changes. Test and verify as appropriate. + content. Use the `read` tool (with `line`/`limit` for large files) to fetch file contents + — it's cheaper than spawning `cat`/`sed`. Always read before editing. Use write and + edit to efficiently make changes. Test and verify as appropriate. When running Python scripts or commands, always use `python3` instead of `python`. "} @@ -96,6 +98,18 @@ impl DeveloperClient { pub(crate) fn get_tools() -> Vec { vec![ + Tool::new( + "read".to_string(), + "Read a text file from disk. Returns the file body. By default returns at most 2000 lines starting from line 1; pass `line` and/or `limit` to fetch a different window. Prefer narrow ranges — large reads waste context.".to_string(), + Self::schema::(), + ) + .annotate(ToolAnnotations::from_raw( + Some("Read".to_string()), + Some(true), + Some(false), + Some(true), + Some(false), + )), Tool::new( "write".to_string(), "Create a new file or overwrite an existing file. Creates parent directories if needed.".to_string(), @@ -184,6 +198,13 @@ impl McpClientTrait for DeveloperClient { Ok(params) => Ok(self.shell_tool.shell_with_cwd(params, working_dir).await), Err(error) => Ok(ShellTool::error_result(&format!("Error: {error}"), None)), }, + "read" => match Self::parse_args::(arguments) { + Ok(params) => Ok(self.edit_tools.file_read_with_cwd(params, working_dir)), + Err(error) => Ok(CallToolResult::error(vec![Content::text(format!( + "Error: {error}" + )) + .with_priority(0.0)])), + }, "write" => match Self::parse_args::(arguments) { Ok(params) => Ok(self.edit_tools.file_write_with_cwd(params, working_dir)), Err(error) => Ok(CallToolResult::error(vec![Content::text(format!( @@ -232,7 +253,7 @@ mod tests { .map(|t| t.name.to_string()) .collect(); - assert_eq!(names, vec!["write", "edit", "shell", "tree"]); + assert_eq!(names, vec!["read", "write", "edit", "shell", "tree"]); } fn test_context(data_dir: std::path::PathBuf) -> PlatformExtensionContext {