From 90dff2154b5055e9c6dcb03e898ef7687f8d3307 Mon Sep 17 00:00:00 2001 From: Leo <5faf251baee50ee6bcde338aef6acdd70bb3e60115664c2cd490d94a55dfc488@buzz.block.builderlab.xyz> Date: Tue, 18 Aug 2026 18:53:07 -0400 Subject: [PATCH 1/2] fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_path only handled absolute paths and workspace-relative joins, so a user-named tilde path like `~/.claude/skills/x` fell through the relative branch and resolved under the workspace root (`/app/~/.claude/...`), which never exists. The shell tool expands `~` via bash, so the file tools diverged from it — agents in the PR #6261 named-path benchmark all hit this and had to recover. Expand a leading `~` (bare `~` or `~/...`) to the user home directory at the single chokepoint resolve_path, matching shell semantics. Home is read from $HOME (%USERPROFILE% on Windows). `~user` is intentionally left untouched — it needs a passwd lookup and is out of scope, consistent with the conservative posture for un-mappable MSYS forms. expand_tilde takes home as a parameter so it stays pure and testable without mutating the process environment. Closes #6270 Co-authored-by: Salman Mohammed Signed-off-by: Salman Mohammed --- crates/buzz-dev-mcp/src/paths.rs | 116 ++++++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 2 deletions(-) diff --git a/crates/buzz-dev-mcp/src/paths.rs b/crates/buzz-dev-mcp/src/paths.rs index 1770d562fa2..b84d4dec23d 100644 --- a/crates/buzz-dev-mcp/src/paths.rs +++ b/crates/buzz-dev-mcp/src/paths.rs @@ -1,8 +1,10 @@ //! Path resolution and file I/O shared across dev-mcp tools. //! //! `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). +//! workspace root. A leading `~` expands to the user's home directory (bare +//! `~` or `~/...`), matching the shell tool. 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 @@ -28,6 +30,17 @@ pub(crate) fn resolve_path(root: &Path, path: &str) -> Result { #[cfg(windows)] let path = &msys_to_windows(path); + // Expand a leading `~` (bare or `~/...`) to the user's home directory, + // matching the shell tool's tilde semantics. Without this, a user-named + // path like `~/.claude/skills/x` takes the relative branch and resolves + // under the workspace root (`/~/.claude/...`), which never exists. + // We deliberately do NOT handle `~user` (another user's home): that needs + // a passwd lookup and is out of scope, mirroring the conservative posture + // for un-mappable MSYS forms above. `~user...` falls through untouched and + // fails with the clear `path not accessible` error rather than mis-mapping. + let expanded = expand_tilde(path, home_dir().as_deref()); + let path: &str = expanded.as_deref().unwrap_or(path); + let raw = Path::new(path); let candidate: PathBuf = if raw.is_absolute() { raw.to_path_buf() @@ -41,6 +54,53 @@ pub(crate) fn resolve_path(root: &Path, path: &str) -> Result { Ok(resolved) } +/// Expand a leading `~` to the user's home directory, returning `Some(expanded)` +/// when a rewrite happened and `None` when the input should be used unchanged. +/// +/// Handles the two shell forms that map deterministically to a home directory: +/// - bare `~` -> `home` +/// - `~/rest` (or `~\rest` on Windows) -> `/rest` +/// +/// A leading `~` followed by anything else (`~user`, `~+`, `~foo`) is a form we +/// cannot resolve without extra state, so it is left untouched — consistent with +/// how `msys_to_windows` leaves un-mappable inputs alone. Returns `None` when +/// `home` is `None` (unset) so the caller falls back to the raw path. Kept pure +/// (home passed in) so it is testable without mutating process environment. +fn expand_tilde(path: &str, home: Option<&str>) -> Option { + let rest = path.strip_prefix('~')?; + // Only a bare `~` or a `~` immediately followed by a path separator is a + // home-relative reference. Anything else (`~user`) is left to the caller. + let is_sep = |c: char| c == '/' || (cfg!(windows) && c == '\\'); + if !rest.is_empty() && !rest.starts_with(is_sep) { + return None; + } + + let home = home?; + if home.is_empty() { + return None; + } + + if rest.is_empty() { + // Bare `~` -> home directory. + return Some(home.to_string()); + } + // `~/rest` -> `/rest`. `rest` begins with a separator, so strip it to + // avoid an absolute-looking join and let `Path` re-add the separator. + let tail = rest.trim_start_matches(is_sep); + let joined = Path::new(home).join(tail); + Some(joined.to_string_lossy().into_owned()) +} + +/// The user's home directory from the environment: `%USERPROFILE%` on Windows, +/// `$HOME` elsewhere. Returns `None` if the variable is unset or not UTF-8. +fn home_dir() -> Option { + #[cfg(windows)] + let var = std::env::var_os("USERPROFILE"); + #[cfg(not(windows))] + let var = std::env::var_os("HOME"); + var?.to_str().map(str::to_string) +} + /// Translate the MSYS/Cygwin absolute path forms bash would accept into a /// native Windows path, matching `cygpath -w` semantics so the file tools /// resolve the same inputs the `shell` tool does. Anything that is not a @@ -208,6 +268,58 @@ mod tests { assert!(p.ends_with("file.txt")); } + // `expand_tilde` is pure (home is passed in), so these cases need no env + // mutation and cannot race parallel tests. + #[test] + fn expand_tilde_forms() { + let home = "/home/agent"; + + // Non-tilde inputs are never rewritten. + assert_eq!(expand_tilde("file.txt", Some(home)), None); + assert_eq!(expand_tilde("/abs/path", Some(home)), None); + assert_eq!(expand_tilde("sub/~notleading", Some(home)), None); + + // `~user` and other non-separator suffixes are left for the caller. + assert_eq!(expand_tilde("~user/x", Some(home)), None); + assert_eq!(expand_tilde("~foo", Some(home)), None); + + // Bare `~` and `~/rest` expand against the supplied home. + assert_eq!(expand_tilde("~", Some(home)), Some(home.to_string())); + let expanded = expand_tilde("~/.claude/skills/x", Some(home)).expect("expands"); + assert_eq!( + expanded, + Path::new(home).join(".claude/skills/x").to_string_lossy() + ); + + // Unset or empty home -> no rewrite, caller falls back to the raw path. + assert_eq!(expand_tilde("~/rest", None), None); + assert_eq!(expand_tilde("~", None), None); + assert_eq!(expand_tilde("~/rest", Some("")), None); + } + + // End-to-end through `resolve_path`, exercising the real `home_dir()` env + // read: a `~/...` path resolves against the actual home directory, not the + // workspace root. Uses a temp file created under the real home so it does + // not mutate the environment. + #[test] + fn resolve_path_expands_tilde_against_home() { + let home = match home_dir() { + Some(h) if !h.is_empty() => h, + _ => return, // No home in this environment (e.g. minimal CI) — skip. + }; + let marker = format!(".dev-mcp-tilde-test-{}", std::process::id()); + let target = Path::new(&home).join(&marker); + fs::write(&target, b"z").expect("write under home"); + + let workspace = tempdir().expect("tempdir"); + let resolved = resolve_path(workspace.path(), &format!("~/{marker}")) + .expect("tilde path resolves against home, not workspace"); + let want = std::fs::canonicalize(&target).expect("canon"); + assert_eq!(resolved, want); + + let _ = fs::remove_file(&target); + } + // Windows MSYS-absolute path translation. These test `msys_to_windows` // directly (the pure rewrite) rather than `resolve_path`, because the latter // canonicalizes against the real filesystem and we want deterministic From 29e0082ff425b7129662891041210f9f042f527a Mon Sep 17 00:00:00 2001 From: Leo <5faf251baee50ee6bcde338aef6acdd70bb3e60115664c2cd490d94a55dfc488@buzz.block.builderlab.xyz> Date: Wed, 19 Aug 2026 12:49:52 -0400 Subject: [PATCH 2/2] fix(buzz-dev-mcp): prefer $HOME and MSYS-translate it for ~ expansion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit home_dir() read %USERPROFILE% first on Windows, but the shell tool expands ~ via bash against $HOME — and $HOME is passed through to the MCP child on every platform (buzz-agent PASSTHROUGH_ENV) and inherited by the shell tool (no env_clear in shell.rs). So the file tools diverged from the shell tool whenever HOME != USERPROFILE (git-bash HOME, or an mcpServers[].env override), undercutting the PR's stated "match the shell tool" goal. Prefer non-empty $HOME on every platform, fall back to %USERPROFILE% only on Windows when HOME is unset. On Windows the chosen home is run through msys_to_windows so a git-bash HOME=/c/Users/x becomes C:\\Users\\x — ~ expansion happens after msys_to_windows in resolve_path, so an untranslated MSYS home would otherwise reach canonicalize and be rejected. An MSYS home with no Windows equivalent (/home/x) is left untranslated and fails with the clear path-not-accessible error, the intended conservative outcome. Selection is factored into a pure select_home(home, userprofile) so it is unit-testable without mutating process env. New tests cover HOME-first preference + empty/unset fallback, and (Windows) MSYS translation of the home value with divergent HOME/USERPROFILE — which also fixes the prior test's silent no-op when HOME is unset on CI. Co-authored-by: Salman Mohammed Signed-off-by: Salman Mohammed --- crates/buzz-dev-mcp/src/paths.rs | 100 +++++++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 5 deletions(-) diff --git a/crates/buzz-dev-mcp/src/paths.rs b/crates/buzz-dev-mcp/src/paths.rs index b84d4dec23d..2c75a112f78 100644 --- a/crates/buzz-dev-mcp/src/paths.rs +++ b/crates/buzz-dev-mcp/src/paths.rs @@ -91,14 +91,49 @@ fn expand_tilde(path: &str, home: Option<&str>) -> Option { Some(joined.to_string_lossy().into_owned()) } -/// The user's home directory from the environment: `%USERPROFILE%` on Windows, -/// `$HOME` elsewhere. Returns `None` if the variable is unset or not UTF-8. +/// The user's home directory from the environment. Reads `$HOME` first, falling +/// back to `%USERPROFILE%` on Windows, then hands the raw values to `select_home` +/// (pure, so it is testable without mutating process env). Returns `None` if no +/// usable value is set or the value is not UTF-8. fn home_dir() -> Option { + let home = std::env::var_os("HOME").and_then(|v| v.into_string().ok()); #[cfg(windows)] - let var = std::env::var_os("USERPROFILE"); + let userprofile = std::env::var_os("USERPROFILE").and_then(|v| v.into_string().ok()); #[cfg(not(windows))] - let var = std::env::var_os("HOME"); - var?.to_str().map(str::to_string) + let userprofile: Option = None; + select_home(home.as_deref(), userprofile.as_deref()) +} + +/// Choose the home directory from the two env candidates, preferring `$HOME`. +/// +/// `$HOME` is preferred because that is exactly what bash — and therefore the +/// `shell` tool — expands `~` against, and `HOME` is passed through to the MCP +/// child on every platform (see `buzz-agent`'s `PASSTHROUGH_ENV`). Picking +/// `USERPROFILE` first on Windows would diverge from the shell tool whenever the +/// two differ (a git-bash `HOME=/c/Users/x`, or an `mcpServers[].env` override), +/// which is precisely the "match the shell tool" contract this fix exists for. +/// `USERPROFILE` is only a Windows fallback for when `HOME` is unset. +/// +/// On Windows the chosen value is passed through `msys_to_windows` so an MSYS +/// `HOME` (`/c/Users/x`) becomes a native path (`C:\Users\x`) — `~` expansion +/// happens after `msys_to_windows` in `resolve_path`, so the spliced-in home +/// would otherwise never be translated and `canonicalize` would reject it. An +/// MSYS form with no Windows equivalent (`/home/x`) falls through untranslated +/// and fails with the clear `path not accessible` error, the correct outcome. +/// Empty strings are treated as unset. +fn select_home(home: Option<&str>, userprofile: Option<&str>) -> Option { + fn non_empty(v: Option<&str>) -> Option<&str> { + v.filter(|s| !s.is_empty()) + } + let chosen = non_empty(home).or_else(|| non_empty(userprofile))?; + #[cfg(windows)] + { + Some(msys_to_windows(chosen)) + } + #[cfg(not(windows))] + { + Some(chosen.to_string()) + } } /// Translate the MSYS/Cygwin absolute path forms bash would accept into a @@ -297,6 +332,28 @@ mod tests { assert_eq!(expand_tilde("~/rest", Some("")), None); } + // `select_home` is pure (both env candidates passed in), so it exercises the + // HOME-first preference and empty/unset handling without mutating process + // env or racing parallel tests. `select_home` itself does not gate the + // fallback by platform — `home_dir` is what only supplies `userprofile` on + // Windows — so these assertions hold identically on every platform. + #[test] + fn select_home_prefers_home() { + // $HOME wins when both are set. + assert_eq!( + select_home(Some("/home/agent"), Some("/other")), + Some("/home/agent".to_string()) + ); + // Empty $HOME is treated as unset -> fall back to the second candidate. + assert_eq!( + select_home(Some(""), Some("/other")), + Some("/other".to_string()) + ); + // No usable candidate -> None. + assert_eq!(select_home(None, None), None); + assert_eq!(select_home(Some(""), Some("")), None); + } + // End-to-end through `resolve_path`, exercising the real `home_dir()` env // read: a `~/...` path resolves against the actual home directory, not the // workspace root. Uses a temp file created under the real home so it does @@ -351,6 +408,39 @@ mod tests { assert_eq!(msys_to_windows(r"C:\Users\x"), r"C:\Users\x"); } + // Windows `select_home` behavior: HOME still wins over USERPROFILE, and + // an MSYS-form HOME is translated to a native path so the value spliced + // in during `~` expansion (which runs after `msys_to_windows`) resolves. + // Both candidates are passed in, so this needs no process-env mutation + // and does not silently no-op the way a real-env read would when HOME is + // unset on CI. + #[test] + fn select_home_translates_msys_home_and_prefers_it() { + // Divergent HOME/USERPROFILE: HOME wins, and its MSYS cygdrive form + // is translated to the native path so canonicalize can use it. + assert_eq!( + select_home(Some("/c/Users/agent"), Some(r"C:\Users\other")), + Some(r"C:\Users\agent".to_string()) + ); + // A native-form HOME is preferred and passes through unchanged. + assert_eq!( + select_home(Some(r"C:\Users\agent"), Some(r"C:\Users\other")), + Some(r"C:\Users\agent".to_string()) + ); + // HOME unset -> fall back to USERPROFILE (already native). + assert_eq!( + select_home(None, Some(r"C:\Users\other")), + Some(r"C:\Users\other".to_string()) + ); + // An MSYS HOME with no Windows equivalent (`/home/x`) is left + // untranslated; it fails downstream with a clear error rather than + // being mis-mapped — the intended conservative outcome. + assert_eq!( + select_home(Some("/home/agent"), None), + Some("/home/agent".to_string()) + ); + } + #[test] fn relative_path_passes_through_unchanged() { // No leading slash — left for the caller's `root.join`.