mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
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 <michael.neale@gmail.com>
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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<Tool> {
|
||||
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::<FileReadParams>(),
|
||||
)
|
||||
.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::<FileReadParams>(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::<FileWriteParams>(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 {
|
||||
|
||||
Reference in New Issue
Block a user