test: add comprehensive tests for shell PATH resolution

Unit tests for shell_env module:
- merge_paths: deduplication, ordering, empty inputs, identical paths
- get_shell_path: caching (OnceLock), no trailing newline, no empty entries,
  valid UTF-8, returns standard directories
- try_resolve_path_from_shell: login shell, nonexistent shell, invalid flags

Unit tests for stdio transport helpers:
- resolve_command: with/without shell path, nonexistent, restricted path
- inject_shell_path: adds when missing, respects existing, no-op when None
- command_hint: Docker vs non-Docker hints
- classify_stderr_line: error/warn/debug/info classification

Integration tests:
- Shell PATH finds system commands (sh, ls, env)
- Shell PATH has more entries than minimal default
- Child process receives injected shell PATH
- User-set PATH override not clobbered by injection
- StdioTransport resolves commands via shell PATH

Signed-off-by: Claude <noreply@anthropic.com>

https://claude.ai/code/session_01EYZVAqzgNBAAtJptw9KRwB
Signed-off-by: Claude <noreply@anthropic.com>

Author: claude

crates/mcpmux-gateway/src/pool/transport/shell_env.rs

@@ -153,6 +153,8 @@ fn merge_paths(primary: &str, secondary: &str) -> String {
 mod tests {
     use super::*;
 
+    // ── merge_paths tests ──────────────────────────────────────────
+
     #[cfg(unix)]
     #[test]
     fn test_merge_paths_deduplicates() {
@@ -181,6 +183,36 @@ mod tests {
         assert_eq!(result, "/a:/b");
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn test_merge_paths_empty_primary() {
+        let result = merge_paths("", "/a:/b");
+        assert_eq!(result, "/a:/b");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_merge_paths_both_empty() {
+        let result = merge_paths("", "");
+        assert_eq!(result, "");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_merge_paths_identical() {
+        let result = merge_paths("/a:/b:/c", "/a:/b:/c");
+        assert_eq!(result, "/a:/b:/c");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_merge_paths_many_duplicates() {
+        let result = merge_paths("/a:/b:/c:/d", "/d:/c:/b:/a:/e");
+        assert_eq!(result, "/a:/b:/c:/d:/e");
+    }
+
+    // ── get_shell_path tests ───────────────────────────────────────
+
     #[cfg(unix)]
     #[test]
     fn test_get_shell_path_returns_something() {
@@ -194,4 +226,87 @@ mod tests {
             path_str
         );
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_get_shell_path_is_cached() {
+        // Calling twice should return the exact same reference (OnceLock)
+        let first = get_shell_path();
+        let second = get_shell_path();
+        assert!(first.is_some());
+        assert!(second.is_some());
+        // Same pointer — verifies caching via OnceLock
+        assert!(std::ptr::eq(first.unwrap(), second.unwrap()));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_get_shell_path_has_no_trailing_newline() {
+        let path = get_shell_path();
+        if let Some(p) = path {
+            let s = p.to_string_lossy();
+            assert!(
+                !s.ends_with('\n') && !s.ends_with('\r'),
+                "PATH should not have trailing newlines: {:?}",
+                s
+            );
+        }
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_get_shell_path_entries_are_not_empty() {
+        let path = get_shell_path();
+        if let Some(p) = path {
+            let s = p.to_string_lossy();
+            for entry in s.split(':') {
+                assert!(
+                    !entry.is_empty(),
+                    "PATH should not contain empty entries: {:?}",
+                    s
+                );
+            }
+        }
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_get_shell_path_is_valid_utf8() {
+        let path = get_shell_path();
+        if let Some(p) = path {
+            assert!(p.to_str().is_some(), "PATH should be valid UTF-8: {:?}", p);
+        }
+    }
+
+    // ── try_resolve_path_from_shell tests ──────────────────────────
+
+    #[cfg(unix)]
+    #[test]
+    fn test_try_resolve_shell_with_login_flag() {
+        // /bin/sh should work with -l -c
+        let result = try_resolve_path_from_shell("/bin/sh", &["-l", "-c"]);
+        assert!(result.is_some(), "Should resolve PATH from /bin/sh -l -c");
+        let path = result.unwrap();
+        assert!(!path.is_empty(), "PATH should not be empty");
+        assert!(
+            path.contains("/bin") || path.contains("/usr"),
+            "PATH should contain standard dirs: {}",
+            path
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_try_resolve_shell_nonexistent_shell() {
+        let result = try_resolve_path_from_shell("/nonexistent/shell_binary_xyz", &["-l", "-c"]);
+        assert!(result.is_none(), "Should fail for nonexistent shell");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_try_resolve_shell_invalid_flags() {
+        // --bogus-flag should cause the shell to error
+        let result = try_resolve_path_from_shell("/bin/sh", &["--bogus-flag-xyz", "-c"]);
+        assert!(result.is_none(), "Should fail with invalid shell flags");
+    }
 }

crates/mcpmux-gateway/src/pool/transport/stdio.rs

@@ -343,3 +343,186 @@ fn inject_shell_path(env: &mut HashMap<String, String>, shell_path: Option<&std:
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::ffi::OsString;
+
+    // ── resolve_command tests ──────────────────────────────────────
+
+    #[test]
+    fn test_resolve_command_finds_sh_with_shell_path() {
+        // /bin/sh exists on every Unix system
+        #[cfg(unix)]
+        {
+            let path = OsString::from("/bin:/usr/bin");
+            let result = resolve_command("sh", Some(&path));
+            assert!(result.is_ok(), "Should find 'sh' in /bin:/usr/bin");
+        }
+    }
+
+    #[test]
+    fn test_resolve_command_finds_command_without_shell_path() {
+        // Without shell_path, falls back to which::which (uses process PATH)
+        #[cfg(unix)]
+        {
+            let result = resolve_command("sh", None);
+            assert!(result.is_ok(), "Should find 'sh' via process PATH");
+        }
+    }
+
+    #[test]
+    fn test_resolve_command_returns_error_for_nonexistent() {
+        let fake_path = OsString::from("/nonexistent/path");
+        let result = resolve_command("this_command_surely_does_not_exist_xyz", Some(&fake_path));
+        assert!(result.is_err(), "Should fail for nonexistent command");
+    }
+
+    #[test]
+    fn test_resolve_command_not_found_in_restricted_path() {
+        // Even if 'sh' exists, it shouldn't be found if PATH points elsewhere
+        let path = OsString::from("/tmp/empty_dir_that_does_not_exist");
+        let result = resolve_command("sh", Some(&path));
+        assert!(
+            result.is_err(),
+            "Should not find 'sh' in a path that doesn't contain it"
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_resolve_command_with_full_shell_path() {
+        // Use the actual shell-resolved PATH to find a real command
+        if let Some(shell_path) = shell_env::get_shell_path() {
+            let result = resolve_command("sh", Some(shell_path));
+            assert!(result.is_ok(), "Should find 'sh' using resolved shell PATH");
+        }
+    }
+
+    // ── inject_shell_path tests ────────────────────────────────────
+
+    #[test]
+    fn test_inject_shell_path_adds_when_missing() {
+        let mut env = HashMap::new();
+        env.insert("FOO".to_string(), "bar".to_string());
+
+        let path = OsString::from("/usr/bin:/usr/local/bin");
+        inject_shell_path(&mut env, Some(&path));
+
+        assert_eq!(
+            env.get("PATH"),
+            Some(&"/usr/bin:/usr/local/bin".to_string()),
+            "PATH should be injected"
+        );
+        assert_eq!(
+            env.get("FOO"),
+            Some(&"bar".to_string()),
+            "Existing vars should be preserved"
+        );
+    }
+
+    #[test]
+    fn test_inject_shell_path_respects_existing_path() {
+        let mut env = HashMap::new();
+        env.insert("PATH".to_string(), "/custom/path".to_string());
+
+        let path = OsString::from("/usr/bin:/usr/local/bin");
+        inject_shell_path(&mut env, Some(&path));
+
+        assert_eq!(
+            env.get("PATH"),
+            Some(&"/custom/path".to_string()),
+            "User-set PATH should not be overridden"
+        );
+    }
+
+    #[test]
+    fn test_inject_shell_path_noop_when_none() {
+        let mut env = HashMap::new();
+        env.insert("FOO".to_string(), "bar".to_string());
+
+        inject_shell_path(&mut env, None);
+
+        assert!(
+            !env.contains_key("PATH"),
+            "Should not inject PATH when shell_path is None"
+        );
+    }
+
+    #[test]
+    fn test_inject_shell_path_empty_env() {
+        let mut env = HashMap::new();
+
+        let path = OsString::from("/a:/b:/c");
+        inject_shell_path(&mut env, Some(&path));
+
+        assert_eq!(env.get("PATH"), Some(&"/a:/b:/c".to_string()));
+        assert_eq!(env.len(), 1, "Should only have PATH");
+    }
+
+    // ── command_hint tests ─────────────────────────────────────────
+
+    #[test]
+    fn test_command_hint_docker() {
+        assert!(command_hint("docker").contains("Docker Desktop"));
+        assert!(command_hint("/usr/local/bin/docker").contains("Docker Desktop"));
+    }
+
+    #[test]
+    fn test_command_hint_non_docker() {
+        assert_eq!(command_hint("npx"), "");
+        assert_eq!(command_hint("node"), "");
+        assert_eq!(command_hint("python"), "");
+    }
+
+    // ── classify_stderr_line tests ─────────────────────────────────
+
+    #[test]
+    fn test_classify_stderr_error() {
+        assert_eq!(
+            classify_stderr_line("ERROR: something failed"),
+            LogLevel::Error
+        );
+        assert_eq!(
+            classify_stderr_line("fatal: not a git repository"),
+            LogLevel::Error
+        );
+        assert_eq!(
+            classify_stderr_line("thread 'main' panicked"),
+            LogLevel::Error
+        );
+    }
+
+    #[test]
+    fn test_classify_stderr_warn() {
+        assert_eq!(
+            classify_stderr_line("WARN: deprecated feature"),
+            LogLevel::Warn
+        );
+        assert_eq!(
+            classify_stderr_line("Warning: something is off"),
+            LogLevel::Warn
+        );
+    }
+
+    #[test]
+    fn test_classify_stderr_debug() {
+        assert_eq!(
+            classify_stderr_line("DEBUG: internal state"),
+            LogLevel::Debug
+        );
+        assert_eq!(
+            classify_stderr_line("trace: verbose output"),
+            LogLevel::Debug
+        );
+    }
+
+    #[test]
+    fn test_classify_stderr_info_default() {
+        assert_eq!(
+            classify_stderr_line("Server listening on port 3000"),
+            LogLevel::Info
+        );
+    }
+}