fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths

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<PathBuf, String> {
     #[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 (`<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,88 @@ pub(crate) fn resolve_path(root: &Path, path: &str) -> Result<PathBuf, String> {
     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) -> `<home>/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<String> {
+    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` -> `<home>/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. 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<String> {
+    let home = std::env::var_os("HOME").and_then(|v| v.into_string().ok());
+    #[cfg(windows)]
+    let userprofile = std::env::var_os("USERPROFILE").and_then(|v| v.into_string().ok());
+    #[cfg(not(windows))]
+    let userprofile: Option<String> = 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<String> {
+    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
 /// 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 +303,80 @@ 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);
+    }
+
+    // `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
+    // 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
@@ -239,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`.