feat(sprout-dev-mcp): add read_file tool and replace_all to str_replace (#928)

Co-authored-by: Will Pfleger <wpfleger@block.xyz>
Signed-off-by: Will Pfleger <wpfleger@block.xyz>
Co-authored-by: npub16v54tttfqacx9ycvc3k0ut0npj564ahcuajzy6qjvh57ntmsf4uq4806j2 <d32955ad69077062930cc46cfe2df30ca9aaf6f8e76422681265e9e9af704d78@sprout-oss.stage.blox.sqprod.co>

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}:{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<read_file::ReadFileParams>,
+    ) -> Result<String, ErrorData> {
+        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,

crates/sprout-dev-mcp/src/paths.rs

@@ -1,16 +1,23 @@
-//! 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.
+//! `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_path` 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};
 
-/// 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<PathBuf, String> {
+pub(crate) const MAX_FILE_BYTES: u64 = 10 * 1024 * 1024;
+
+/// 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<PathBuf, String> {
     let raw = Path::new(path);
     let candidate: PathBuf = if raw.is_absolute() {
         raw.to_path_buf()
@@ -18,20 +25,93 @@ pub(crate) fn resolve_within(root: &Path, path: &str) -> Result<PathBuf, String>
         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)
+}
+
+/// 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_path(&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,
         ));
     }
-    Ok(resolved)
+    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)]
@@ -41,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");
@@ -53,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"));
     }
 }

crates/sprout-dev-mcp/src/read_file.rs

@@ -0,0 +1,227 @@
+use crate::shell::SharedState;
+use rmcp::ErrorData;
+use schemars::JsonSchema;
+use serde::Deserialize;
+
+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<usize>,
+    /// Maximum number of lines to return. Defaults to 2000.
+    #[serde(default)]
+    pub limit: Option<usize>,
+    /// Workspace root for relative path resolution. Defaults to server cwd.
+    #[serde(default)]
+    pub workdir: Option<String>,
+}
+
+pub fn run(state: &SharedState, p: ReadFileParams) -> Result<String, ErrorData> {
+    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();
+
+    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)];
+
+    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();
+
+    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}:{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)
+}
+
+#[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: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]
+    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}");
+        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]
+    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_allows_absolute_path() {
+        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 out = run(&state, p).expect("ok");
+        assert!(
+            out.contains("localhost"),
+            "expected /etc/hosts content, got: {out}"
+        );
+    }
+
+    #[test]
+    fn read_rejects_too_large() {
+        let dir = tempdir().expect("tempdir");
+        let f = dir.path().join("big.bin");
+        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 {
+            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}");
+    }
+
+    #[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:line3"), "out: {out}");
+    }
+}

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;
 
@@ -15,6 +14,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.
+    #[serde(default)]
+    pub replace_all: bool,
     #[serde(default)]
     pub workdir: Option<String>,
 }
@@ -33,128 +36,74 @@ pub fn run(state: &SharedState, p: StrReplaceParams) -> Result<String, ErrorData
         ));
     }
 
-    let workspace_root = 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 (target, content) = crate::paths::read_text_file(state, &p.path, p.workdir.as_deref())?;
+
+    let count = if p.replace_all {
+        content.matches(&p.old_str).count()
+    } else {
+        count_occurrences_capped(&content, &p.old_str)
     };
 
-    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 {
+    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!(
-                "file too large: {} is {} bytes (limit {} bytes)",
+                "old_str not found in {}.\nold_str (truncated): {:?}{hint}",
                 target.display(),
-                meta.len(),
-                MAX_FILE_BYTES
+                truncate(&p.old_str, 80)
             ),
             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 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(
+    if !p.replace_all && count > 1 {
+        return Err(ErrorData::invalid_params(
             format!(
                 "old_str matched multiple locations in {}; provide more surrounding context to make the match unique.",
                 target.display()
             ),
             None,
-        )),
+        ));
     }
-}
 
-pub(crate) use crate::paths::resolve_within;
+    // 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 new_content = if p.replace_all {
+        content.replace(&p.old_str, &p.new_str)
+    } else {
+        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) fn count_occurrences_capped(text: &str, pattern: &str) -> usize {
     if pattern.is_empty() {
@@ -294,6 +243,7 @@ mod tests {
             path: "a.txt".into(),
             old_str: "beta".into(),
             new_str: "BETA".into(),
+            replace_all: false,
             workdir: Some(dir.path().display().to_string()),
         };
         let out = run(&state, p).expect("ok");
@@ -305,20 +255,23 @@ 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()),
         };
         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}"
         );
     }
 
@@ -326,17 +279,73 @@ 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: false,
             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: 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: 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: false,
+            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}");
+    }
 }

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}"
         );
     }