From 80f4d26605ff0a907f2f9ff5a12cacf604d67301 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 9 Jun 2026 14:19:53 -0400 Subject: [PATCH 1/5] feat(sprout-dev-mcp): add read_file tool and replace_all to str_replace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents had no dedicated file-reading tool — reading required shelling out to cat -n, burning extra tokens on shell invocation overhead and command construction. Bulk renames (rename a variable across a file) required either N separate str_replace calls or falling back to sed, which has heredoc quoting pitfalls when called from JSON. read_file: offset/limit params enable windowed reads of large files without head/tail/sed gymnastics. Returns 1-based line-numbered output matching cat -n conventions. str_replace replace_all: backward-compatible optional bool; when true replaces all occurrences and reports the count, when false/absent the existing single-match uniqueness requirement is preserved. --- crates/sprout-dev-mcp/src/lib.rs | 14 +- crates/sprout-dev-mcp/src/read_file.rs | 234 +++++++++++++++++++++++ crates/sprout-dev-mcp/src/str_replace.rs | 161 +++++++++++++--- 3 files changed, 378 insertions(+), 31 deletions(-) create mode 100644 crates/sprout-dev-mcp/src/read_file.rs diff --git a/crates/sprout-dev-mcp/src/lib.rs b/crates/sprout-dev-mcp/src/lib.rs index 4d7c788c75..0809b933e0 100644 --- a/crates/sprout-dev-mcp/src/lib.rs +++ b/crates/sprout-dev-mcp/src/lib.rs @@ -10,6 +10,7 @@ use std::path::Path; use std::sync::Arc; mod paths; +mod read_file; mod rg; mod shell; mod shim; @@ -47,6 +48,17 @@ impl DevMcp { shell::run(&self.state, p, context.ct).await } + #[tool( + name = "read_file", + description = "Read a text file and return its contents with line numbers. Returns lines in `{number}\\t{content}` format. Use `offset` (0-based) and `limit` (default 2000) to window into large files. Path resolved relative to workdir (defaults to server cwd). Prefer over cat/head/tail." + )] + async fn read_file( + &self, + Parameters(p): Parameters, + ) -> Result { + read_file::run(&self.state, p) + } + #[tool( name = "view_image", description = "Load an image from a file path, http(s) URL, or data: URL and return it as an MCP image content block that multimodal LLMs (Anthropic, OpenAI-compatible, etc.) can see. Resizes to a longest-edge of 1568px by default (override with `max_dim`, range 64..=2048). Pass-through for already-small PNG/JPEG; transcodes oversize input to PNG (if alpha) or JPEG q85. Animated GIF/WebP rejected — provide a still frame. Hard cap 20 MiB source, ~4 MiB on the wire. Relative paths resolve under `workdir` (defaults to server cwd) and may not escape it." @@ -60,7 +72,7 @@ impl DevMcp { #[tool( name = "str_replace", - description = "Atomic find-and-replace in a file. old_str must occur exactly once. Returns a unified diff. Path resolved relative to workdir (defaults to server cwd). Prefer over sed/awk." + description = "Atomic find-and-replace in a file. old_str must occur exactly once unless replace_all is true, in which case all occurrences are replaced. Returns a unified diff. Path resolved relative to workdir (defaults to server cwd). Prefer over sed/awk." )] async fn str_replace( &self, diff --git a/crates/sprout-dev-mcp/src/read_file.rs b/crates/sprout-dev-mcp/src/read_file.rs new file mode 100644 index 0000000000..6d0281a948 --- /dev/null +++ b/crates/sprout-dev-mcp/src/read_file.rs @@ -0,0 +1,234 @@ +use crate::paths::resolve_within; +use crate::shell::SharedState; +use rmcp::ErrorData; +use schemars::JsonSchema; +use serde::Deserialize; +use std::path::PathBuf; + +const MAX_FILE_BYTES: u64 = 10 * 1024 * 1024; +const DEFAULT_LIMIT: usize = 2000; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct ReadFileParams { + /// File path (absolute or relative to workdir). + pub path: String, + /// 0-based line offset to start reading from. Defaults to 0. + #[serde(default)] + pub offset: Option, + /// Maximum number of lines to return. Defaults to 2000. + #[serde(default)] + pub limit: Option, + /// Workspace root for relative path resolution. Defaults to server cwd. + #[serde(default)] + pub workdir: Option, +} + +pub fn run(state: &SharedState, p: ReadFileParams) -> Result { + let workspace_root: PathBuf = match p.workdir.as_deref() { + Some(w) => PathBuf::from(w), + None => state.cwd.clone(), + }; + let target = match resolve_within(&workspace_root, &p.path) { + Ok(t) => t, + Err(e) => return Err(ErrorData::invalid_params(e, None)), + }; + + let meta = match std::fs::metadata(&target) { + Ok(m) => m, + Err(e) => { + return Err(ErrorData::internal_error( + format!("cannot stat {}: {e}", target.display()), + None, + )); + } + }; + if !meta.is_file() { + return Err(ErrorData::invalid_params( + format!("not a regular file: {}", target.display()), + None, + )); + } + if meta.len() > MAX_FILE_BYTES { + return Err(ErrorData::invalid_params( + format!( + "file too large: {} is {} bytes (limit {} bytes)", + target.display(), + meta.len(), + MAX_FILE_BYTES + ), + None, + )); + } + + let file = match std::fs::File::open(&target) { + Ok(f) => f, + Err(e) => { + return Err(ErrorData::internal_error( + format!("cannot open {}: {e}", target.display()), + None, + )); + } + }; + let mut buf = Vec::with_capacity(meta.len() as usize); + use std::io::Read; + match file.take(MAX_FILE_BYTES + 1).read_to_end(&mut buf) { + Ok(n) if n as u64 > MAX_FILE_BYTES => { + return Err(ErrorData::invalid_params( + format!("file grew past {} bytes during read", MAX_FILE_BYTES), + None, + )); + } + Ok(_) => {} + Err(e) => { + return Err(ErrorData::internal_error( + format!("cannot read {}: {e}", target.display()), + None, + )); + } + } + let content = match String::from_utf8(buf) { + Ok(s) => s, + Err(e) => { + return Err(ErrorData::internal_error( + format!("not valid UTF-8: {}: {e}", target.display()), + None, + )); + } + }; + + let all_lines: Vec<&str> = content.lines().collect(); + let total = all_lines.len(); + + if total == 0 { + return Ok(format!("{} is empty (0 lines)", p.path)); + } + + let offset = p.offset.unwrap_or(0); + let limit = p.limit.unwrap_or(DEFAULT_LIMIT); + + let slice = &all_lines[offset.min(total)..]; + let slice = &slice[..slice.len().min(limit)]; + + // 1-based line numbers in the output. + let start_line = offset + 1; + let end_line = offset + slice.len(); + + let mut out = format!( + "{} (lines {}-{} of {})\n", + p.path, start_line, end_line, total + ); + for (i, line) in slice.iter().enumerate() { + let line_number = offset + i + 1; + out.push_str(&format!("{line_number}\t{line}\n")); + } + // Remove the trailing newline added by the last iteration so the result + // has exactly one trailing newline (the format! above already ends with \n + // for the header, and each line pushes its own \n). + Ok(out) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::tempdir; + + fn make_state(cwd: &std::path::Path) -> SharedState { + let shim = crate::shim::Shim::install().expect("shim install"); + SharedState::new(cwd.to_path_buf(), shim).expect("state new") + } + + #[test] + fn read_basic() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("basic.txt"); + fs::write(&f, "line1\nline2\nline3\nline4\nline5\n").expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "basic.txt".into(), + offset: None, + limit: None, + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("lines 1-5 of 5"), "out: {out}"); + assert!(out.contains("1\tline1"), "out: {out}"); + assert!(out.contains("2\tline2"), "out: {out}"); + assert!(out.contains("3\tline3"), "out: {out}"); + assert!(out.contains("4\tline4"), "out: {out}"); + assert!(out.contains("5\tline5"), "out: {out}"); + } + + #[test] + fn read_offset_limit() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("ten.txt"); + let contents: String = (1..=10).map(|i| format!("line{i}\n")).collect(); + fs::write(&f, &contents).expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "ten.txt".into(), + offset: Some(3), + limit: Some(2), + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("lines 4-5 of 10"), "out: {out}"); + let line_count = out.lines().skip(1).count(); // skip header + assert_eq!(line_count, 2, "expected 2 data lines, got: {out}"); + assert!(out.contains("4\tline4"), "out: {out}"); + assert!(out.contains("5\tline5"), "out: {out}"); + } + + #[test] + fn read_empty_file() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("empty.txt"); + fs::write(&f, b"").expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "empty.txt".into(), + offset: None, + limit: None, + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("is empty (0 lines)"), "out: {out}"); + } + + #[test] + fn read_rejects_path_escape() { + let dir = tempdir().expect("tempdir"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "/etc/hosts".into(), + offset: None, + limit: None, + workdir: Some(dir.path().display().to_string()), + }; + let err = run(&state, p).unwrap_err(); + let msg = format!("{err:?}"); + assert!( + msg.contains("escapes workspace") || msg.contains("not accessible"), + "msg: {msg}" + ); + } + + #[test] + fn read_rejects_too_large() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("big.bin"); + let big = vec![b'a'; (MAX_FILE_BYTES as usize) + 1024]; + fs::write(&f, &big).expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "big.bin".into(), + offset: None, + limit: None, + workdir: Some(dir.path().display().to_string()), + }; + let err = run(&state, p).unwrap_err(); + let msg = format!("{err:?}"); + assert!(msg.contains("too large"), "msg: {msg}"); + } +} diff --git a/crates/sprout-dev-mcp/src/str_replace.rs b/crates/sprout-dev-mcp/src/str_replace.rs index 0251c97dc1..1525ba378a 100644 --- a/crates/sprout-dev-mcp/src/str_replace.rs +++ b/crates/sprout-dev-mcp/src/str_replace.rs @@ -15,6 +15,10 @@ pub struct StrReplaceParams { pub path: String, pub old_str: String, pub new_str: String, + /// When true, replace ALL occurrences of old_str instead of requiring + /// exactly one match. Defaults to false. + #[serde(default)] + pub replace_all: Option, #[serde(default)] pub workdir: Option, } @@ -105,52 +109,91 @@ pub fn run(state: &SharedState, p: StrReplaceParams) -> Result { + if p.replace_all.unwrap_or(false) { + let count = content.matches(&p.old_str as &str).count(); + if count == 0 { let hint = nearest_line_hint(&content, &p.old_str) .map(|h| format!("\n{h}")) .unwrap_or_default(); - Err(ErrorData::invalid_params( + return Err(ErrorData::invalid_params( format!( "old_str not found in {}.\nold_str (truncated): {:?}{hint}", target.display(), truncate(&p.old_str, 80) ), None, - )) + )); + } + let new_content = content.replace(&p.old_str as &str, &p.new_str); + if new_content.len() as u64 > MAX_FILE_BYTES { + return Err(ErrorData::invalid_params( + format!( + "result would exceed {} byte limit ({} bytes)", + MAX_FILE_BYTES, + new_content.len() + ), + None, + )); + } + if let Err(e) = atomic_write(&target, &new_content) { + return Err(ErrorData::internal_error( + format!("failed to write {}: {e}", target.display()), + None, + )); } - 1 => { - let new_content = content.replacen(&p.old_str, &p.new_str, 1); - if new_content.len() as u64 > MAX_FILE_BYTES { - return Err(ErrorData::invalid_params( + let diff = unified_diff(&content, &new_content, &target); + Ok(format!( + "Replaced {count} occurrence(s) in {}.\n\n{diff}", + target.display() + )) + } else { + let count = count_occurrences_capped(&content, &p.old_str); + match count { + 0 => { + let hint = nearest_line_hint(&content, &p.old_str) + .map(|h| format!("\n{h}")) + .unwrap_or_default(); + Err(ErrorData::invalid_params( format!( - "result would exceed {} byte limit ({} bytes)", - MAX_FILE_BYTES, - new_content.len() + "old_str not found in {}.\nold_str (truncated): {:?}{hint}", + target.display(), + truncate(&p.old_str, 80) ), None, - )); + )) } - if let Err(e) = atomic_write(&target, &new_content) { - return Err(ErrorData::internal_error( - format!("failed to write {}: {e}", target.display()), - None, - )); + 1 => { + let new_content = content.replacen(&p.old_str, &p.new_str, 1); + if new_content.len() as u64 > MAX_FILE_BYTES { + return Err(ErrorData::invalid_params( + format!( + "result would exceed {} byte limit ({} bytes)", + MAX_FILE_BYTES, + new_content.len() + ), + None, + )); + } + if let Err(e) = atomic_write(&target, &new_content) { + return Err(ErrorData::internal_error( + format!("failed to write {}: {e}", target.display()), + None, + )); + } + let diff = unified_diff(&content, &new_content, &target); + Ok(format!( + "Replaced 1 occurrence in {}.\n\n{diff}", + target.display() + )) } - let diff = unified_diff(&content, &new_content, &target); - Ok(format!( - "Replaced 1 occurrence in {}.\n\n{diff}", - target.display() - )) + _ => Err(ErrorData::invalid_params( + format!( + "old_str matched multiple locations in {}; provide more surrounding context to make the match unique.", + target.display() + ), + None, + )), } - _ => Err(ErrorData::invalid_params( - format!( - "old_str matched multiple locations in {}; provide more surrounding context to make the match unique.", - target.display() - ), - None, - )), } } @@ -294,6 +337,7 @@ mod tests { path: "a.txt".into(), old_str: "beta".into(), new_str: "BETA".into(), + replace_all: None, workdir: Some(dir.path().display().to_string()), }; let out = run(&state, p).expect("ok"); @@ -312,6 +356,7 @@ mod tests { path: "/etc/hosts".into(), old_str: "x".into(), new_str: "y".into(), + replace_all: None, workdir: Some(dir.path().display().to_string()), }; let err = run(&state, p).unwrap_err(); @@ -333,10 +378,66 @@ mod tests { path: "big.bin".into(), old_str: "a".into(), new_str: "b".into(), + replace_all: None, workdir: Some(dir.path().display().to_string()), }; let err = run(&state, p).unwrap_err(); let msg = format!("{err:?}"); assert!(msg.contains("too large"), "msg: {msg}"); } + + #[test] + fn run_replace_all_replaces_all_occurrences() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("multi.txt"); + fs::write(&f, "foo bar foo baz foo\n").expect("write"); + let state = make_state(dir.path()); + let p = StrReplaceParams { + path: "multi.txt".into(), + old_str: "foo".into(), + new_str: "qux".into(), + replace_all: Some(true), + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("Replaced 3 occurrence(s)"), "out: {out}"); + let contents = fs::read_to_string(&f).expect("read"); + assert_eq!(contents, "qux bar qux baz qux\n"); + } + + #[test] + fn run_replace_all_errors_on_zero_matches() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("nomatch.txt"); + fs::write(&f, "hello world\n").expect("write"); + let state = make_state(dir.path()); + let p = StrReplaceParams { + path: "nomatch.txt".into(), + old_str: "xyz".into(), + new_str: "abc".into(), + replace_all: Some(true), + workdir: Some(dir.path().display().to_string()), + }; + let err = run(&state, p).unwrap_err(); + let msg = format!("{err:?}"); + assert!(msg.contains("not found"), "msg: {msg}"); + } + + #[test] + fn run_without_replace_all_preserves_single_match_behavior() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("multi2.txt"); + fs::write(&f, "foo bar foo\n").expect("write"); + let state = make_state(dir.path()); + let p = StrReplaceParams { + path: "multi2.txt".into(), + old_str: "foo".into(), + new_str: "qux".into(), + replace_all: None, + workdir: Some(dir.path().display().to_string()), + }; + let err = run(&state, p).unwrap_err(); + let msg = format!("{err:?}"); + assert!(msg.contains("matched multiple locations"), "msg: {msg}"); + } } From 5552281508d6e960e25affda7b363f6403244a7a Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 9 Jun 2026 14:55:54 -0400 Subject: [PATCH 2/5] refactor(sprout-dev-mcp): address review feedback from 5-reviewer crossfire Codex flagged a potential OOM in replace_all: content.replace() allocates the full result before the size check runs. Added preflight size estimation via checked arithmetic before allocation. Three reviewers (Claude Functionality, Codex, Gemini) independently flagged the nonsensical header when offset >= total or limit == 0 (e.g. "lines 11-10 of 5"). Now returns "no lines in range" instead. Two reviewers (Claude Design, Gemini) flagged ~30 lines of duplicated size-check/write/diff/format between replace_all and single-match branches. Unified into a single flow that diverges only on counting and replacement strategy. Also extracted the ~40-line file-reading pipeline (resolve, stat, size check, read, UTF-8 decode) shared by read_file and str_replace into paths::read_text_file, changed replace_all from Option to bool, removed dead pub(crate) use re-export, removed stale comment and redundant as-casts, added edge-case tests for offset-past-end, limit=0, and files without trailing newline. --- crates/sprout-dev-mcp/src/paths.rs | 93 +++++++++- crates/sprout-dev-mcp/src/read_file.rs | 137 +++++++-------- crates/sprout-dev-mcp/src/str_replace.rs | 214 +++++++---------------- 3 files changed, 211 insertions(+), 233 deletions(-) diff --git a/crates/sprout-dev-mcp/src/paths.rs b/crates/sprout-dev-mcp/src/paths.rs index 68cb50379d..116e793df4 100644 --- a/crates/sprout-dev-mcp/src/paths.rs +++ b/crates/sprout-dev-mcp/src/paths.rs @@ -1,12 +1,20 @@ -//! Path resolution shared across dev-mcp tools. +//! Path resolution and file I/O shared across dev-mcp tools. //! //! `resolve_within` canonicalises a user-supplied path against a workspace //! root and rejects any result that escapes the root (e.g. via `..`, absolute //! paths, or symlinks). All tools that touch the filesystem must funnel //! through this helper so the escape policy stays consistent. +//! +//! `read_text_file` builds on `resolve_within` to provide the full +//! resolve → stat → size-check → read → UTF-8 decode pipeline shared by +//! `read_file` and `str_replace`. +use crate::shell::SharedState; +use rmcp::ErrorData; use std::path::{Path, PathBuf}; +pub(crate) const MAX_FILE_BYTES: u64 = 10 * 1024 * 1024; + /// Resolve `path` (absolute or relative) against `root` and require the /// canonicalised result to live under the canonicalised `root`. Returns an /// error string suitable for `ErrorData::invalid_params` on rejection. @@ -34,6 +42,89 @@ pub(crate) fn resolve_within(root: &Path, path: &str) -> Result Ok(resolved) } +/// Resolve a user-supplied path within the workspace, read the file, and +/// return `(resolved_path, utf8_content)`. Rejects files that are not +/// regular files, exceed `MAX_FILE_BYTES`, or are not valid UTF-8. +pub(crate) fn read_text_file( + state: &SharedState, + path: &str, + workdir: Option<&str>, +) -> Result<(PathBuf, String), ErrorData> { + let workspace_root: PathBuf = match workdir { + Some(w) => PathBuf::from(w), + None => state.cwd.clone(), + }; + let target = match resolve_within(&workspace_root, path) { + Ok(t) => t, + Err(e) => return Err(ErrorData::invalid_params(e, None)), + }; + + let meta = match std::fs::metadata(&target) { + Ok(m) => m, + Err(e) => { + return Err(ErrorData::internal_error( + format!("cannot stat {}: {e}", target.display()), + None, + )); + } + }; + if !meta.is_file() { + return Err(ErrorData::invalid_params( + format!("not a regular file: {}", target.display()), + None, + )); + } + if meta.len() > MAX_FILE_BYTES { + return Err(ErrorData::invalid_params( + format!( + "file too large: {} is {} bytes (limit {} bytes)", + target.display(), + meta.len(), + MAX_FILE_BYTES + ), + None, + )); + } + + let file = match std::fs::File::open(&target) { + Ok(f) => f, + Err(e) => { + return Err(ErrorData::internal_error( + format!("cannot open {}: {e}", target.display()), + None, + )); + } + }; + let mut buf = Vec::with_capacity(meta.len() as usize); + use std::io::Read; + match file.take(MAX_FILE_BYTES + 1).read_to_end(&mut buf) { + Ok(n) if n as u64 > MAX_FILE_BYTES => { + return Err(ErrorData::invalid_params( + format!("file grew past {} bytes during read", MAX_FILE_BYTES), + None, + )); + } + Ok(_) => {} + Err(e) => { + return Err(ErrorData::internal_error( + format!("cannot read {}: {e}", target.display()), + None, + )); + } + } + let content = match String::from_utf8(buf) { + Ok(s) => s, + Err(e) => { + return Err(ErrorData::internal_error( + format!("not valid UTF-8: {}: {e}", target.display()), + None, + )); + } + }; + + Ok((target, content)) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/sprout-dev-mcp/src/read_file.rs b/crates/sprout-dev-mcp/src/read_file.rs index 6d0281a948..e4da258b41 100644 --- a/crates/sprout-dev-mcp/src/read_file.rs +++ b/crates/sprout-dev-mcp/src/read_file.rs @@ -1,11 +1,8 @@ -use crate::paths::resolve_within; use crate::shell::SharedState; use rmcp::ErrorData; use schemars::JsonSchema; use serde::Deserialize; -use std::path::PathBuf; -const MAX_FILE_BYTES: u64 = 10 * 1024 * 1024; const DEFAULT_LIMIT: usize = 2000; #[derive(Debug, Deserialize, JsonSchema)] @@ -24,77 +21,7 @@ pub struct ReadFileParams { } pub fn run(state: &SharedState, p: ReadFileParams) -> Result { - let workspace_root: PathBuf = match p.workdir.as_deref() { - Some(w) => PathBuf::from(w), - None => state.cwd.clone(), - }; - let target = match resolve_within(&workspace_root, &p.path) { - Ok(t) => t, - Err(e) => return Err(ErrorData::invalid_params(e, None)), - }; - - let meta = match std::fs::metadata(&target) { - Ok(m) => m, - Err(e) => { - return Err(ErrorData::internal_error( - format!("cannot stat {}: {e}", target.display()), - None, - )); - } - }; - if !meta.is_file() { - return Err(ErrorData::invalid_params( - format!("not a regular file: {}", target.display()), - None, - )); - } - if meta.len() > MAX_FILE_BYTES { - return Err(ErrorData::invalid_params( - format!( - "file too large: {} is {} bytes (limit {} bytes)", - target.display(), - meta.len(), - MAX_FILE_BYTES - ), - None, - )); - } - - let file = match std::fs::File::open(&target) { - Ok(f) => f, - Err(e) => { - return Err(ErrorData::internal_error( - format!("cannot open {}: {e}", target.display()), - None, - )); - } - }; - let mut buf = Vec::with_capacity(meta.len() as usize); - use std::io::Read; - match file.take(MAX_FILE_BYTES + 1).read_to_end(&mut buf) { - Ok(n) if n as u64 > MAX_FILE_BYTES => { - return Err(ErrorData::invalid_params( - format!("file grew past {} bytes during read", MAX_FILE_BYTES), - None, - )); - } - Ok(_) => {} - Err(e) => { - return Err(ErrorData::internal_error( - format!("cannot read {}: {e}", target.display()), - None, - )); - } - } - let content = match String::from_utf8(buf) { - Ok(s) => s, - Err(e) => { - return Err(ErrorData::internal_error( - format!("not valid UTF-8: {}: {e}", target.display()), - None, - )); - } - }; + let (_target, content) = crate::paths::read_text_file(state, &p.path, p.workdir.as_deref())?; let all_lines: Vec<&str> = content.lines().collect(); let total = all_lines.len(); @@ -109,6 +36,13 @@ pub fn run(state: &SharedState, p: ReadFileParams) -> Result let slice = &all_lines[offset.min(total)..]; let slice = &slice[..slice.len().min(limit)]; + if slice.is_empty() { + return Ok(format!( + "{} (no lines in range, file has {} lines)", + p.path, total + )); + } + // 1-based line numbers in the output. let start_line = offset + 1; let end_line = offset + slice.len(); @@ -121,9 +55,6 @@ pub fn run(state: &SharedState, p: ReadFileParams) -> Result let line_number = offset + i + 1; out.push_str(&format!("{line_number}\t{line}\n")); } - // Remove the trailing newline added by the last iteration so the result - // has exactly one trailing newline (the format! above already ends with \n - // for the header, and each line pushes its own \n). Ok(out) } @@ -218,7 +149,7 @@ mod tests { fn read_rejects_too_large() { let dir = tempdir().expect("tempdir"); let f = dir.path().join("big.bin"); - let big = vec![b'a'; (MAX_FILE_BYTES as usize) + 1024]; + let big = vec![b'a'; (10 * 1024 * 1024_usize) + 1024]; fs::write(&f, &big).expect("write"); let state = make_state(dir.path()); let p = ReadFileParams { @@ -231,4 +162,54 @@ mod tests { let msg = format!("{err:?}"); assert!(msg.contains("too large"), "msg: {msg}"); } + + #[test] + fn read_offset_past_end() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("short.txt"); + fs::write(&f, "line1\nline2\n").expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "short.txt".into(), + offset: Some(100), + limit: None, + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("no lines in range"), "out: {out}"); + assert!(out.contains("file has 2 lines"), "out: {out}"); + } + + #[test] + fn read_limit_zero() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("some.txt"); + fs::write(&f, "line1\nline2\n").expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "some.txt".into(), + offset: None, + limit: Some(0), + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("no lines in range"), "out: {out}"); + } + + #[test] + fn read_file_without_trailing_newline() { + let dir = tempdir().expect("tempdir"); + let f = dir.path().join("notrail.txt"); + fs::write(&f, "line1\nline2\nline3").expect("write"); + let state = make_state(dir.path()); + let p = ReadFileParams { + path: "notrail.txt".into(), + offset: None, + limit: None, + workdir: Some(dir.path().display().to_string()), + }; + let out = run(&state, p).expect("ok"); + assert!(out.contains("lines 1-3 of 3"), "out: {out}"); + assert!(out.contains("3\tline3"), "out: {out}"); + } } diff --git a/crates/sprout-dev-mcp/src/str_replace.rs b/crates/sprout-dev-mcp/src/str_replace.rs index 1525ba378a..8b4627f44d 100644 --- a/crates/sprout-dev-mcp/src/str_replace.rs +++ b/crates/sprout-dev-mcp/src/str_replace.rs @@ -4,9 +4,8 @@ use schemars::JsonSchema; use serde::Deserialize; use similar::{DiffTag, TextDiff}; use std::io::Write; -use std::path::{Path, PathBuf}; +use std::path::Path; -const MAX_FILE_BYTES: u64 = 10 * 1024 * 1024; const MAX_INPUT_BYTES: usize = 1024 * 1024; const HINT_SCAN_LINE_LIMIT: usize = 200; @@ -16,9 +15,9 @@ pub struct StrReplaceParams { pub old_str: String, pub new_str: String, /// When true, replace ALL occurrences of old_str instead of requiring - /// exactly one match. Defaults to false. + /// exactly one match. #[serde(default)] - pub replace_all: Option, + pub replace_all: bool, #[serde(default)] pub workdir: Option, } @@ -37,168 +36,75 @@ pub fn run(state: &SharedState, p: StrReplaceParams) -> Result PathBuf::from(w), - None => state.cwd.clone(), - }; - let target = match resolve_within(&workspace_root, &p.path) { - Ok(t) => t, - Err(e) => return Err(ErrorData::invalid_params(e, None)), - }; + let (target, content) = crate::paths::read_text_file(state, &p.path, p.workdir.as_deref())?; - let meta = match std::fs::metadata(&target) { - Ok(m) => m, - Err(e) => { - return Err(ErrorData::internal_error( - format!("cannot stat {}: {e}", target.display()), - None, - )); - } + let count = if p.replace_all { + content.matches(&p.old_str).count() + } else { + count_occurrences_capped(&content, &p.old_str) }; - if !meta.is_file() { + + if count == 0 { + let hint = nearest_line_hint(&content, &p.old_str) + .map(|h| format!("\n{h}")) + .unwrap_or_default(); return Err(ErrorData::invalid_params( - format!("not a regular file: {}", target.display()), + format!( + "old_str not found in {}.\nold_str (truncated): {:?}{hint}", + target.display(), + truncate(&p.old_str, 80) + ), None, )); } - if meta.len() > MAX_FILE_BYTES { + if !p.replace_all && count > 1 { return Err(ErrorData::invalid_params( format!( - "file too large: {} is {} bytes (limit {} bytes)", - target.display(), - meta.len(), - MAX_FILE_BYTES + "old_str matched multiple locations in {}; provide more surrounding context to make the match unique.", + target.display() ), None, )); } - let file = match std::fs::File::open(&target) { - Ok(f) => f, - Err(e) => { - return Err(ErrorData::internal_error( - format!("cannot open {}: {e}", target.display()), - None, - )); - } - }; - let mut buf = Vec::with_capacity(meta.len() as usize); - use std::io::Read; - match file.take(MAX_FILE_BYTES + 1).read_to_end(&mut buf) { - Ok(n) if n as u64 > MAX_FILE_BYTES => { - return Err(ErrorData::invalid_params( - format!("file grew past {} bytes during read", MAX_FILE_BYTES), - None, - )); - } - Ok(_) => {} - Err(e) => { - return Err(ErrorData::internal_error( - format!("cannot read {}: {e}", target.display()), - None, - )); - } + // Preflight: reject before allocating if the result would exceed the limit. + let size_delta = (p.new_str.len() as i64) - (p.old_str.len() as i64); + let projected = (content.len() as i64).saturating_add(size_delta.saturating_mul(count as i64)); + if projected < 0 || projected as u64 > crate::paths::MAX_FILE_BYTES { + return Err(ErrorData::invalid_params( + format!( + "result would exceed {} byte limit ({} bytes projected)", + crate::paths::MAX_FILE_BYTES, + projected + ), + None, + )); } - let content = match String::from_utf8(buf) { - Ok(s) => s, - Err(e) => { - return Err(ErrorData::internal_error( - format!("not valid UTF-8: {}: {e}", target.display()), - None, - )); - } - }; - if p.replace_all.unwrap_or(false) { - let count = content.matches(&p.old_str as &str).count(); - if count == 0 { - let hint = nearest_line_hint(&content, &p.old_str) - .map(|h| format!("\n{h}")) - .unwrap_or_default(); - return Err(ErrorData::invalid_params( - format!( - "old_str not found in {}.\nold_str (truncated): {:?}{hint}", - target.display(), - truncate(&p.old_str, 80) - ), - None, - )); - } - let new_content = content.replace(&p.old_str as &str, &p.new_str); - if new_content.len() as u64 > MAX_FILE_BYTES { - return Err(ErrorData::invalid_params( - format!( - "result would exceed {} byte limit ({} bytes)", - MAX_FILE_BYTES, - new_content.len() - ), - None, - )); - } - if let Err(e) = atomic_write(&target, &new_content) { - return Err(ErrorData::internal_error( - format!("failed to write {}: {e}", target.display()), - None, - )); - } - let diff = unified_diff(&content, &new_content, &target); - Ok(format!( - "Replaced {count} occurrence(s) in {}.\n\n{diff}", - target.display() - )) + let new_content = if p.replace_all { + content.replace(&p.old_str, &p.new_str) } else { - let count = count_occurrences_capped(&content, &p.old_str); - match count { - 0 => { - let hint = nearest_line_hint(&content, &p.old_str) - .map(|h| format!("\n{h}")) - .unwrap_or_default(); - Err(ErrorData::invalid_params( - format!( - "old_str not found in {}.\nold_str (truncated): {:?}{hint}", - target.display(), - truncate(&p.old_str, 80) - ), - None, - )) - } - 1 => { - let new_content = content.replacen(&p.old_str, &p.new_str, 1); - if new_content.len() as u64 > MAX_FILE_BYTES { - return Err(ErrorData::invalid_params( - format!( - "result would exceed {} byte limit ({} bytes)", - MAX_FILE_BYTES, - new_content.len() - ), - None, - )); - } - if let Err(e) = atomic_write(&target, &new_content) { - return Err(ErrorData::internal_error( - format!("failed to write {}: {e}", target.display()), - None, - )); - } - let diff = unified_diff(&content, &new_content, &target); - Ok(format!( - "Replaced 1 occurrence in {}.\n\n{diff}", - target.display() - )) - } - _ => Err(ErrorData::invalid_params( - format!( - "old_str matched multiple locations in {}; provide more surrounding context to make the match unique.", - target.display() - ), - None, - )), - } + content.replacen(&p.old_str, &p.new_str, 1) + }; + + if let Err(e) = atomic_write(&target, &new_content) { + return Err(ErrorData::internal_error( + format!("failed to write {}: {e}", target.display()), + None, + )); } + let diff = unified_diff(&content, &new_content, &target); + let label = if count == 1 { + "1 occurrence".to_string() + } else { + format!("{count} occurrence(s)") + }; + Ok(format!( + "Replaced {label} in {}.\n\n{diff}", + target.display() + )) } -pub(crate) use crate::paths::resolve_within; - pub(crate) fn count_occurrences_capped(text: &str, pattern: &str) -> usize { if pattern.is_empty() { return 0; @@ -337,7 +243,7 @@ mod tests { path: "a.txt".into(), old_str: "beta".into(), new_str: "BETA".into(), - replace_all: None, + replace_all: false, workdir: Some(dir.path().display().to_string()), }; let out = run(&state, p).expect("ok"); @@ -356,7 +262,7 @@ mod tests { path: "/etc/hosts".into(), old_str: "x".into(), new_str: "y".into(), - replace_all: None, + replace_all: false, workdir: Some(dir.path().display().to_string()), }; let err = run(&state, p).unwrap_err(); @@ -371,14 +277,14 @@ mod tests { fn run_rejects_file_too_large() { let dir = tempdir().expect("tempdir"); let f = dir.path().join("big.bin"); - let big = vec![b'a'; (MAX_FILE_BYTES as usize) + 1024]; + let big = vec![b'a'; (crate::paths::MAX_FILE_BYTES as usize) + 1024]; fs::write(&f, &big).expect("write"); let state = make_state(dir.path()); let p = StrReplaceParams { path: "big.bin".into(), old_str: "a".into(), new_str: "b".into(), - replace_all: None, + replace_all: false, workdir: Some(dir.path().display().to_string()), }; let err = run(&state, p).unwrap_err(); @@ -396,7 +302,7 @@ mod tests { path: "multi.txt".into(), old_str: "foo".into(), new_str: "qux".into(), - replace_all: Some(true), + replace_all: true, workdir: Some(dir.path().display().to_string()), }; let out = run(&state, p).expect("ok"); @@ -415,7 +321,7 @@ mod tests { path: "nomatch.txt".into(), old_str: "xyz".into(), new_str: "abc".into(), - replace_all: Some(true), + replace_all: true, workdir: Some(dir.path().display().to_string()), }; let err = run(&state, p).unwrap_err(); @@ -433,7 +339,7 @@ mod tests { path: "multi2.txt".into(), old_str: "foo".into(), new_str: "qux".into(), - replace_all: None, + replace_all: false, workdir: Some(dir.path().display().to_string()), }; let err = run(&state, p).unwrap_err(); From 06a6f9beb8395e9e060333a66362492a2688c6b6 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Wed, 10 Jun 2026 12:56:36 -0400 Subject: [PATCH 3/5] refactor(sprout-dev-mcp): remove directory traversal limits from file tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shell tool already gives unrestricted filesystem access, so jailing read_file, str_replace, and view_image under the workspace root was inconsistent defense-in-depth that added friction (agents could not read files outside their workdir without shelling out) without adding real security. resolve_within is renamed to resolve_path since it no longer enforces containment — it still resolves relative paths against the root and canonicalizes (resolving symlinks and normalizing ..). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/sprout-dev-mcp/src/paths.rs | 40 +++++++++--------------- crates/sprout-dev-mcp/src/read_file.rs | 9 +++--- crates/sprout-dev-mcp/src/str_replace.rs | 10 +++--- crates/sprout-dev-mcp/src/view_image.rs | 10 +++--- 4 files changed, 31 insertions(+), 38 deletions(-) diff --git a/crates/sprout-dev-mcp/src/paths.rs b/crates/sprout-dev-mcp/src/paths.rs index 116e793df4..e23647a809 100644 --- a/crates/sprout-dev-mcp/src/paths.rs +++ b/crates/sprout-dev-mcp/src/paths.rs @@ -1,11 +1,10 @@ //! Path resolution and file I/O shared across dev-mcp tools. //! -//! `resolve_within` canonicalises a user-supplied path against a workspace -//! root and rejects any result that escapes the root (e.g. via `..`, absolute -//! paths, or symlinks). All tools that touch the filesystem must funnel -//! through this helper so the escape policy stays consistent. +//! `resolve_path` resolves and canonicalizes a user-supplied path against a +//! workspace root. No containment enforcement — the resolved path may land +//! anywhere on the filesystem (consistent with the `shell` tool's posture). //! -//! `read_text_file` builds on `resolve_within` to provide the full +//! `read_text_file` builds on `resolve_path` to provide the full //! resolve → stat → size-check → read → UTF-8 decode pipeline shared by //! `read_file` and `str_replace`. @@ -15,10 +14,10 @@ use std::path::{Path, PathBuf}; pub(crate) const MAX_FILE_BYTES: u64 = 10 * 1024 * 1024; -/// Resolve `path` (absolute or relative) against `root` and require the -/// canonicalised result to live under the canonicalised `root`. Returns an -/// error string suitable for `ErrorData::invalid_params` on rejection. -pub(crate) fn resolve_within(root: &Path, path: &str) -> Result { +/// Resolve `path` (absolute or relative) against `root` and canonicalize +/// the result. Returns an error string suitable for `ErrorData::invalid_params` +/// if the path cannot be resolved. +pub(crate) fn resolve_path(root: &Path, path: &str) -> Result { let raw = Path::new(path); let candidate: PathBuf = if raw.is_absolute() { raw.to_path_buf() @@ -26,19 +25,9 @@ pub(crate) fn resolve_within(root: &Path, path: &str) -> Result root.join(raw) }; - let root_canon = std::fs::canonicalize(root) - .map_err(|e| format!("workdir not accessible: {} ({e})", root.display()))?; - let resolved = std::fs::canonicalize(&candidate) .map_err(|e| format!("path not accessible: {} ({e})", candidate.display()))?; - if !resolved.starts_with(&root_canon) { - return Err(format!( - "path escapes workspace: {} not within {}", - resolved.display(), - root_canon.display() - )); - } Ok(resolved) } @@ -54,7 +43,7 @@ pub(crate) fn read_text_file( Some(w) => PathBuf::from(w), None => state.cwd.clone(), }; - let target = match resolve_within(&workspace_root, path) { + let target = match resolve_path(&workspace_root, path) { Ok(t) => t, Err(e) => return Err(ErrorData::invalid_params(e, None)), }; @@ -132,11 +121,11 @@ mod tests { use tempfile::tempdir; #[test] - fn resolve_within_rejects_escape() { + fn resolve_path_allows_outside_workspace() { let dir = tempdir().expect("tempdir"); let inside = dir.path().join("file.txt"); fs::write(&inside, b"x").expect("write"); - // Symlink targeting outside the dir should be rejected. + // Symlink targeting outside the dir should now resolve successfully. #[cfg(unix)] { let outside = std::env::temp_dir().join("dev-mcp-paths-escape-target"); @@ -144,12 +133,13 @@ mod tests { fs::write(&outside, b"y").expect("write outside"); let link = dir.path().join("link.txt"); std::os::unix::fs::symlink(&outside, &link).expect("symlink"); - let err = resolve_within(dir.path(), "link.txt").unwrap_err(); - assert!(err.contains("escapes workspace"), "got: {err}"); + let resolved = resolve_path(dir.path(), "link.txt").expect("resolve"); + let outside_canon = std::fs::canonicalize(&outside).expect("canonicalize"); + assert_eq!(resolved, outside_canon); let _ = fs::remove_file(&outside); } // Resolves a normal path inside. - let p = resolve_within(dir.path(), "file.txt").expect("resolve"); + let p = resolve_path(dir.path(), "file.txt").expect("resolve"); assert!(p.ends_with("file.txt")); } } diff --git a/crates/sprout-dev-mcp/src/read_file.rs b/crates/sprout-dev-mcp/src/read_file.rs index e4da258b41..c6cbbcf7bd 100644 --- a/crates/sprout-dev-mcp/src/read_file.rs +++ b/crates/sprout-dev-mcp/src/read_file.rs @@ -128,7 +128,7 @@ mod tests { } #[test] - fn read_rejects_path_escape() { + fn read_allows_absolute_path() { let dir = tempdir().expect("tempdir"); let state = make_state(dir.path()); let p = ReadFileParams { @@ -137,11 +137,10 @@ mod tests { limit: None, workdir: Some(dir.path().display().to_string()), }; - let err = run(&state, p).unwrap_err(); - let msg = format!("{err:?}"); + let out = run(&state, p).expect("ok"); assert!( - msg.contains("escapes workspace") || msg.contains("not accessible"), - "msg: {msg}" + out.contains("localhost"), + "expected /etc/hosts content, got: {out}" ); } diff --git a/crates/sprout-dev-mcp/src/str_replace.rs b/crates/sprout-dev-mcp/src/str_replace.rs index 8b4627f44d..7feb0aa10d 100644 --- a/crates/sprout-dev-mcp/src/str_replace.rs +++ b/crates/sprout-dev-mcp/src/str_replace.rs @@ -255,12 +255,14 @@ mod tests { } #[test] - fn run_rejects_path_outside_workspace() { + fn run_allows_path_outside_workspace() { let dir = tempdir().expect("tempdir"); let state = make_state(dir.path()); + // /etc/hosts is readable but won't contain our old_str — we expect + // a "not found" error, not a path-escape error. let p = StrReplaceParams { path: "/etc/hosts".into(), - old_str: "x".into(), + old_str: "UNIQUE_STRING_NOT_IN_HOSTS_FILE_abc123".into(), new_str: "y".into(), replace_all: false, workdir: Some(dir.path().display().to_string()), @@ -268,8 +270,8 @@ mod tests { let err = run(&state, p).unwrap_err(); let msg = format!("{err:?}"); assert!( - msg.contains("escapes workspace") || msg.contains("not accessible"), - "msg: {msg}" + msg.contains("not found"), + "expected 'not found' error (proving path resolved), got: {msg}" ); } diff --git a/crates/sprout-dev-mcp/src/view_image.rs b/crates/sprout-dev-mcp/src/view_image.rs index 5221ca942c..5a623b81e0 100644 --- a/crates/sprout-dev-mcp/src/view_image.rs +++ b/crates/sprout-dev-mcp/src/view_image.rs @@ -10,7 +10,7 @@ //! into the right provider-native shape on our behalf (see Goose's //! `providers::utils::convert_image` for a reference implementation). -use crate::paths::resolve_within; +use crate::paths::resolve_path; use crate::shell::SharedState; use base64::Engine; use image::{ @@ -140,7 +140,7 @@ async fn load_source( Some(w) => PathBuf::from(w), None => state.cwd.clone(), }; - let target = resolve_within(&workspace_root, src).map_err(invalid_params)?; + let target = resolve_path(&workspace_root, src).map_err(invalid_params)?; let meta = std::fs::metadata(&target).map_err(|e| { ErrorData::internal_error(format!("cannot stat {}: {e}", target.display()), None) })?; @@ -685,9 +685,11 @@ mod tests { } #[tokio::test] - async fn rejects_path_outside_workspace() { + async fn allows_path_outside_workspace() { let dir = tempdir().unwrap(); let state = make_state(dir.path()); + // /etc/hosts exists but is not an image — we expect a format error, + // not a path-escape error, proving the traversal limit is gone. let res = run( &state, ViewImageParams { @@ -700,7 +702,7 @@ mod tests { .unwrap_err(); let msg = format!("{res:?}"); assert!( - msg.contains("escapes workspace") || msg.contains("not accessible"), + msg.contains("unsupported image format") || msg.contains("empty image"), "{msg}" ); } From 05fb45b7108c672d80a72b5d0f06ac306cb9d785 Mon Sep 17 00:00:00 2001 From: npub16v54tttfqacx9ycvc3k0ut0npj564ahcuajzy6qjvh57ntmsf4uq4806j2 Date: Wed, 10 Jun 2026 13:15:09 -0400 Subject: [PATCH 4/5] fix(sprout-dev-mcp): use colon separator and add truncation metadata to read_file Switch line output from tab-separated (N\tline) to colon-separated (N:line) to match grep/rg conventions that models already understand. Add a truncation footer when the output window does not cover the full file so models know there is more content and how to fetch it. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/sprout-dev-mcp/src/read_file.rs | 35 ++++++++++++++++++-------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/crates/sprout-dev-mcp/src/read_file.rs b/crates/sprout-dev-mcp/src/read_file.rs index c6cbbcf7bd..b75867b61e 100644 --- a/crates/sprout-dev-mcp/src/read_file.rs +++ b/crates/sprout-dev-mcp/src/read_file.rs @@ -53,8 +53,15 @@ pub fn run(state: &SharedState, p: ReadFileParams) -> Result ); for (i, line) in slice.iter().enumerate() { let line_number = offset + i + 1; - out.push_str(&format!("{line_number}\t{line}\n")); + out.push_str(&format!("{line_number}:{line}\n")); } + + if end_line < total { + out.push_str(&format!( + "[showing lines {start_line}-{end_line} of {total}; use offset={end_line} to continue]\n" + )); + } + Ok(out) } @@ -83,11 +90,15 @@ mod tests { }; let out = run(&state, p).expect("ok"); assert!(out.contains("lines 1-5 of 5"), "out: {out}"); - assert!(out.contains("1\tline1"), "out: {out}"); - assert!(out.contains("2\tline2"), "out: {out}"); - assert!(out.contains("3\tline3"), "out: {out}"); - assert!(out.contains("4\tline4"), "out: {out}"); - assert!(out.contains("5\tline5"), "out: {out}"); + assert!(out.contains("1:line1"), "out: {out}"); + assert!(out.contains("2:line2"), "out: {out}"); + assert!(out.contains("3:line3"), "out: {out}"); + assert!(out.contains("4:line4"), "out: {out}"); + assert!(out.contains("5:line5"), "out: {out}"); + assert!( + !out.contains("[showing lines"), + "full file should have no truncation footer: {out}" + ); } #[test] @@ -105,10 +116,12 @@ mod tests { }; let out = run(&state, p).expect("ok"); assert!(out.contains("lines 4-5 of 10"), "out: {out}"); - let line_count = out.lines().skip(1).count(); // skip header - assert_eq!(line_count, 2, "expected 2 data lines, got: {out}"); - assert!(out.contains("4\tline4"), "out: {out}"); - assert!(out.contains("5\tline5"), "out: {out}"); + assert!(out.contains("4:line4"), "out: {out}"); + assert!(out.contains("5:line5"), "out: {out}"); + assert!( + out.contains("[showing lines 4-5 of 10; use offset=5 to continue]"), + "out: {out}" + ); } #[test] @@ -209,6 +222,6 @@ mod tests { }; let out = run(&state, p).expect("ok"); assert!(out.contains("lines 1-3 of 3"), "out: {out}"); - assert!(out.contains("3\tline3"), "out: {out}"); + assert!(out.contains("3:line3"), "out: {out}"); } } From 9de4d18bc1d3d6e645425a9aa5fabdc335468c31 Mon Sep 17 00:00:00 2001 From: npub16v54tttfqacx9ycvc3k0ut0npj564ahcuajzy6qjvh57ntmsf4uq4806j2 Date: Wed, 10 Jun 2026 13:17:53 -0400 Subject: [PATCH 5/5] fix(sprout-dev-mcp): update read_file tool description to reflect colon separator The tool description still referenced the old tab format after the separator change. Update it to match the actual output format. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/sprout-dev-mcp/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sprout-dev-mcp/src/lib.rs b/crates/sprout-dev-mcp/src/lib.rs index 0809b933e0..288193cee1 100644 --- a/crates/sprout-dev-mcp/src/lib.rs +++ b/crates/sprout-dev-mcp/src/lib.rs @@ -50,7 +50,7 @@ impl DevMcp { #[tool( name = "read_file", - description = "Read a text file and return its contents with line numbers. Returns lines in `{number}\\t{content}` format. Use `offset` (0-based) and `limit` (default 2000) to window into large files. Path resolved relative to workdir (defaults to server cwd). Prefer over cat/head/tail." + description = "Read a text file and return its contents with line numbers. Returns lines in `{number}:{content}` format. Use `offset` (0-based) and `limit` (default 2000) to window into large files. Path resolved relative to workdir (defaults to server cwd). Prefer over cat/head/tail." )] async fn read_file( &self,