fix(desktop): preflight npm prefix writability in doctor installs (#1732)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
2026-08-18 06:50:31 +02:00 · 2026-07-10 19:25:58 -04:00
parent d80ed3180b
commit 58aee8f913
13 changed files with 919 additions and 25 deletions
@@ -186,6 +186,23 @@ desktop-tauri-check: _ensure-sidecar-stubs
 desktop-tauri-test: _ensure-sidecar-stubs
    cd desktop/src-tauri && cargo test

+# Run the containerised npm-preflight E2E scenarios (4 tests, ~30s for the timeout test).
+# Builds a Docker image with Node.js + zsh and runs the four #[ignore]d E2E tests
+# against a crafted environment. Does NOT touch the host's npm, PATH, or installs.
+desktop-preflight-e2e:
+    #!/usr/bin/env bash
+    set -euo pipefail
+    if ! command -v docker &>/dev/null; then
+        echo "docker is required for desktop-preflight-e2e"
+        echo "Install it from https://docs.docker.com/get-docker/"
+        exit 1
+    fi
+    docker build \
+        -f desktop/src-tauri/e2e/Dockerfile.preflight-e2e \
+        -t buzz-preflight-e2e \
+        .
+    docker run --rm buzz-preflight-e2e
+
 # Build the full desktop Tauri app locally (unsigned, for testing)
 # Sidecar binary list must stay in sync with _ensure-sidecar-stubs above.
 # pnpm install is unconditional here: release builds must start from a clean dep tree.
@@ -168,7 +168,11 @@ const overrides = new Map([
  // getProfile/updateProfile/getUserProfile/getUsersBatch/searchUsers) moved to
  // tauriProfiles.ts; limit ratcheted down 1360 → 1241 to bank the headroom.
  // baked-env fold-in: getBakedBuildEnv + BakedEnvEntry type adds ~28 lines.
-  ["src/shared/api/tauri.ts", 1271],
+  // doctor-npm-eacces-preflight: hint field on RawInstallStepResult + mapper
+  // passthrough (+2 lines).
+  ["src/shared/api/tauri.ts", 1273],
+  // doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line).
+  ["src/shared/api/types.ts", 1001],
  // readiness-gate: PersonaDialog.tsx threads computeLocalModeGate +
  // requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog
  // shows required markers and credential amber rows (parity with
@@ -0,0 +1,75 @@
+#
+# Containerised E2E harness for the npm-preflight scenarios in
+# desktop/src-tauri/src/commands/agent_discovery.rs.
+#
+# This image:
+#   1. Compiles the buzz_lib test binary (via cargo test --no-run).
+#   2. Runs four #[ignore]d E2E tests in sequence, each in a crafted
+#      environment that controls npm presence and prefix writability.
+#
+# Build context: repository root (so COPY . . captures everything needed).
+# Invoked via:   just desktop-preflight-e2e
+#
+# Host isolation: npm is installed INSIDE the image; no host volume is
+# mounted that could write to the host's npm prefix or PATH.
+
+ARG RUST_VERSION=1.95
+FROM rust:${RUST_VERSION}-slim-bookworm
+
+# Install Node.js LTS (provides npm) and zsh plus the system libs required by
+# Tauri / buzz-desktop build dependencies (webkitgtk etc. are test-binary deps).
+RUN apt-get update -y && \
+    apt-get install -y --no-install-recommends \
+        curl \
+        ca-certificates \
+        pkg-config \
+        cmake \
+        zsh \
+        libasound2-dev \
+        libwebkit2gtk-4.1-dev \
+        libssl-dev \
+        libgtk-3-dev \
+        libayatana-appindicator3-dev \
+        librsvg2-dev \
+    && curl -fsSL https://deb.nodesource.com/setup_lts.x | bash - \
+    && apt-get install -y --no-install-recommends nodejs \
+    && rm -rf /var/lib/apt/lists/*
+
+# Create a non-root user for running the tests. Writability / EACCES
+# scenarios require a genuinely unprivileged user.
+RUN useradd --create-home --shell /bin/bash testuser
+
+# Copy the repo source. The per-Dockerfile .dockerignore excludes target/,
+# node_modules/, and web/ to keep the context lean while including
+# desktop/src-tauri/ (which the root .dockerignore would exclude).
+WORKDIR /build
+COPY . .
+
+# Create sidecar stubs, compile the buzz_lib test binary, and install it.
+# All done as root so the resulting binary is available system-wide.
+RUN arch=$(rustc -vV | sed -n 's|host: ||p') && \
+    mkdir -p /build/desktop/src-tauri/binaries && \
+    for bin in buzz-acp buzz-agent buzz-dev-mcp git-credential-nostr buzz; do \
+        touch "/build/desktop/src-tauri/binaries/${bin}-${arch}"; \
+    done && \
+    cd /build/desktop/src-tauri && \
+    cargo test --lib --no-run --message-format=json 2>/dev/null \
+        | grep '"executable"' \
+        | grep 'buzz_lib' \
+        | sed 's/.*"executable":"\([^"]*\)".*/\1/' \
+        | head -1 > /tmp/test-bin-path.txt && \
+    TEST_BIN=$(cat /tmp/test-bin-path.txt) && \
+    echo "Test binary: $TEST_BIN" && \
+    test -f "$TEST_BIN" && \
+    cp "$TEST_BIN" /usr/local/bin/buzz-lib-tests && \
+    chmod 755 /usr/local/bin/buzz-lib-tests
+
+# Install the scenario entrypoint.
+COPY desktop/src-tauri/e2e/run-e2e-scenarios.sh /usr/local/bin/run-e2e-scenarios.sh
+RUN chmod 755 /usr/local/bin/run-e2e-scenarios.sh
+
+# Run as the non-root user so the EACCES writability test behaves correctly.
+USER testuser
+WORKDIR /home/testuser
+
+ENTRYPOINT ["/usr/local/bin/run-e2e-scenarios.sh"]
@@ -0,0 +1,33 @@
+# Per-Dockerfile .dockerignore for the npm-preflight E2E harness.
+# Overrides the root .dockerignore for this build target.
+# The root .dockerignore excludes desktop/ (correct for the relay image)
+# but this harness needs desktop/src-tauri/ for the Rust source.
+
+# Large build artifacts — exclude to keep context lean.
+**/target/
+desktop/src-tauri/target/
+node_modules/
+**/node_modules/
+web/dist/
+
+# VCS, IDE
+.git/
+.github/
+.vscode/
+.idea/
+.scratch/
+
+# Secrets
+.env
+.env.*
+!.env.example
+*.pem
+*.key
+secrets/
+
+# Non-Rust things the E2E build doesn't need.
+web/
+mobile/
+docs/
+desktop/dist/
+desktop/node_modules/
@@ -0,0 +1,110 @@
+#!/usr/bin/env bash
+# run-e2e-scenarios.sh — containerised E2E runner for the npm-preflight tests.
+#
+# Runs four scenarios in sequence. Each scenario:
+#   1. Creates a dedicated $HOME directory under /tmp with crafted shell init
+#      files and optional ~/.npmrc that control npm visibility and prefix.
+#   2. Exports HOME to that directory so login_shell_path() (OnceLock) initialises
+#      from the correct init files for that test.
+#   3. Runs the specific #[ignore]d test via the pre-compiled test binary.
+#
+# Each test runs in a SEPARATE PROCESS so the OnceLock for login_shell_path()
+# is fresh. All paths are inside the container; nothing touches the host.
+#
+# Exit code: 0 if all four pass, 1 if any fail.
+
+set -euo pipefail
+
+NPM_BIN=$(command -v npm || true)
+if [ -z "$NPM_BIN" ]; then
+    echo "FATAL: npm not found in container PATH — check the Dockerfile installs nodejs" >&2
+    exit 1
+fi
+
+TEST_BIN=/usr/local/bin/buzz-lib-tests
+PASS=0
+FAIL=0
+
+run_scenario() {
+    local name="$1"
+    local home_dir="$2"
+    local test_filter="$3"
+    echo ""
+    echo "════════════════════════════════════════════════════════════"
+    echo "  Scenario: $name"
+    echo "  HOME: $home_dir"
+    echo "════════════════════════════════════════════════════════════"
+    if HOME="$home_dir" "$TEST_BIN" "$test_filter" --ignored --nocapture 2>&1; then
+        echo "  ✅  PASSED"
+        PASS=$((PASS + 1))
+    else
+        echo "  ❌  FAILED"
+        FAIL=$((FAIL + 1))
+    fi
+}
+
+# ── Scenario (a): writable prefix → proceed ──────────────────────────────────
+HOME_A=$(mktemp -d /tmp/e2e-home-writable-XXXXXX)
+mkdir -p "$HOME_A/.npm-global/lib/node_modules"
+# .npmrc: point npm prefix at a user-owned directory.
+echo "prefix=$HOME_A/.npm-global" > "$HOME_A/.npmrc"
+# Login shell init: put npm on PATH.
+# install_shell_command selects /bin/zsh if present, else /bin/bash.
+SHELL_INIT_A="$HOME_A/.bash_profile"
+[ -x /bin/zsh ] && SHELL_INIT_A="$HOME_A/.zprofile"
+echo "export PATH=\"$(dirname "$NPM_BIN"):\$PATH\"" > "$SHELL_INIT_A"
+
+run_scenario "writable prefix → proceed" "$HOME_A" "test_e2e_writable_prefix_proceeds"
+
+# ── Scenario (b): read-only prefix → EACCES abort ────────────────────────────
+HOME_B=$(mktemp -d /tmp/e2e-home-readonly-XXXXXX)
+# No ~/.npmrc → npm uses its compiled-in default (/usr/local), which is
+# root-owned and not writable by testuser.
+# Login shell init: put npm on PATH.
+SHELL_INIT_B="$HOME_B/.bash_profile"
+[ -x /bin/zsh ] && SHELL_INIT_B="$HOME_B/.zprofile"
+echo "export PATH=\"$(dirname "$NPM_BIN"):\$PATH\"" > "$SHELL_INIT_B"
+
+run_scenario "read-only prefix → EACCES abort" "$HOME_B" "test_e2e_readonly_prefix_aborts_with_eacces_guidance"
+
+# ── Scenario (c): npm missing → NPM_MISSING_HINT abort ───────────────────────
+HOME_C=$(mktemp -d /tmp/e2e-home-no-npm-XXXXXX)
+# Create a temp dir that has no npm binary, then set PATH to only that dir.
+NO_NPM_DIR=$(mktemp -d /tmp/e2e-no-npm-bin-XXXXXX)
+# Login shell init: restrict PATH to a directory confirmed to have no npm.
+SHELL_INIT_C="$HOME_C/.bash_profile"
+[ -x /bin/zsh ] && SHELL_INIT_C="$HOME_C/.zprofile"
+echo "export PATH=\"$NO_NPM_DIR\"" > "$SHELL_INIT_C"
+
+run_scenario "npm missing → NPM_MISSING_HINT abort" "$HOME_C" "test_e2e_npm_missing_aborts_with_missing_hint"
+
+# ── Scenario (d): wedged shell → 30s timeout → proceed ───────────────────────
+HOME_D=$(mktemp -d /tmp/e2e-home-wedged-XXXXXX)
+# Create an npm shim that blocks for longer than the 30s deadline.
+# The login shell init puts the shim dir FIRST on PATH so it shadows real npm.
+SHIM_DIR="$HOME_D/.npm-shim"
+mkdir -p "$SHIM_DIR"
+cat > "$SHIM_DIR/npm" << 'SHIM'
+#!/bin/sh
+# Simulate a wedged npm (e.g. a slow version-manager hook).
+sleep 60
+SHIM
+chmod 755 "$SHIM_DIR/npm"
+
+SHELL_INIT_D="$HOME_D/.bash_profile"
+[ -x /bin/zsh ] && SHELL_INIT_D="$HOME_D/.zprofile"
+# The init file adds the shim dir first, so 'npm' resolves to the shim.
+# Real npm is also on PATH so login_shell_path() (echo $PATH) works fine —
+# the block only triggers when 'npm prefix -g' is actually invoked.
+echo "export PATH=\"$SHIM_DIR:$(dirname "$NPM_BIN"):\$PATH\"" > "$SHELL_INIT_D"
+
+echo ""
+echo "  NOTE: scenario (d) intentionally waits ~30s for the timeout to fire."
+run_scenario "wedged shell → 30s timeout → proceed" "$HOME_D" "test_e2e_wedged_shell_timeout_proceeds"
+
+# ── Summary ───────────────────────────────────────────────────────────────────
+echo ""
+echo "════════════════════════════════════════════════════════════"
+echo "  Results: $PASS passed, $FAIL failed"
+echo "════════════════════════════════════════════════════════════"
+[ "$FAIL" -eq 0 ]
@@ -67,6 +67,11 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result<InstallRuntimeResult
    let mut steps = Vec::new();

    // Phase 1: Install CLI if missing and commands are available.
+    // NOTE: the npm EACCES preflight and `npm_eacces_hint` classifier only run
+    // in Phase 2 below. Today every entry in `cli_install_commands` is a
+    // curl-pipe; all `npm install -g` commands live in `adapter_install_commands`.
+    // If a future runtime adds an npm-global CLI install it must also add the
+    // preflight and classifier to this loop.
    if let Some(cli) = runtime.underlying_cli {
        if crate::managed_agents::resolve_command(cli).is_none() {
            for cmd in runtime.cli_install_commands {
@@ -90,7 +95,19 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result<InstallRuntimeResult
        .any(|cmd| crate::managed_agents::resolve_command(cmd).is_some());
    if !adapter_found {
        for cmd in runtime.adapter_install_commands {
-            let result = run_install_command("adapter", cmd);
+            if is_npm_global_install(cmd) {
+                if let Some(step) = npm_preflight_check("adapter", cmd) {
+                    steps.push(step);
+                    return Ok(InstallRuntimeResult {
+                        success: false,
+                        steps,
+                    });
+                }
+            }
+            let mut result = run_install_command("adapter", cmd);
+            if !result.success && result.hint.is_none() && is_npm_global_install(cmd) {
+                result.hint = npm_eacces_hint(&result.stderr, cmd);
+            }
            let success = result.success;
            steps.push(result);
            if !success {
@@ -111,8 +128,12 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result<InstallRuntimeResult
    })
 }

-fn run_install_command(step: &str, command: &str) -> InstallStepResult {
-    let shell_path = crate::managed_agents::login_shell_path();
+/// Build a login-shell `Command` for `command` with the hermit env vars
+/// stripped and the user's PATH set. This is the single source of truth for
+/// the shell selection and environment cleanup shared by `run_install_command`
+/// and `resolve_npm_prefix` — keeping them in sync so the hermit-strip list
+/// can't drift between the two paths.
+fn install_shell_command(command: &str) -> std::process::Command {
    let shell = if std::path::Path::new("/bin/zsh").exists() {
        "/bin/zsh"
    } else {
@@ -128,7 +149,7 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
    cmd.env_remove("NPM_CONFIG_CACHE");
    cmd.env_remove("COREPACK_HOME");

-    if let Some(ref path) = shell_path {
+    if let Some(ref path) = crate::managed_agents::login_shell_path() {
        cmd.env("PATH", path);
    }

@@ -146,6 +167,12 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
        }
    }

+    cmd
+}
+
+fn run_install_command(step: &str, command: &str) -> InstallStepResult {
+    let mut cmd = install_shell_command(command);
+
    let mut child = match cmd
        .stdin(std::process::Stdio::null())
        .stdout(std::process::Stdio::piped())
@@ -161,6 +188,7 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
                stdout: String::new(),
                stderr: format!("failed to spawn shell: {e}"),
                exit_code: None,
+                hint: None,
            };
        }
    };
@@ -216,6 +244,7 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
                stdout: String::new(),
                stderr: "install command timed out after 5 minutes".to_string(),
                exit_code: None,
+                hint: None,
            };
        }

@@ -231,6 +260,7 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
                    stdout: truncate_output(stdout),
                    stderr: truncate_output(stderr_raw),
                    exit_code: status.code(),
+                    hint: None,
                };
            }
            Ok(Err(e)) => {
@@ -244,6 +274,7 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
                    stdout: String::new(),
                    stderr: format!("failed to check process status: {e}"),
                    exit_code: None,
+                    hint: None,
                };
            }
            Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
@@ -262,6 +293,7 @@ fn run_install_command(step: &str, command: &str) -> InstallStepResult {
                    stdout: String::new(),
                    stderr: "internal error: wait thread disconnected".to_string(),
                    exit_code: None,
+                    hint: None,
                };
            }
        }
@@ -295,6 +327,236 @@ fn floor_char_boundary(s: &str, mut index: usize) -> usize {
    index
 }

+// ── npm EACCES preflight ──────────────────────────────────────────────────────
+
+/// Returns true when `command` is an npm global install invocation.
+fn is_npm_global_install(command: &str) -> bool {
+    let t = command.trim_start();
+    t.starts_with("npm install -g ") || t.starts_with("npm i -g ")
+}
+
+/// Guidance text for the EACCES / unwritable-prefix case.
+fn npm_eacces_guidance(command: &str) -> String {
+    format!(
+        "npm's global install directory isn't writable by your user.\n\
+\n\
+Fix (no sudo):\n\
+  1. Run:  npm config set prefix ~/.npm-global\n\
+  2. Add to ~/.zprofile:  export PATH=\"$HOME/.npm-global/bin:$PATH\"\n\
+  3. Restart Buzz, then click Install again.\n\
+\n\
+Or install manually, then click Refresh:\n\
+  sudo {command}"
+    )
+}
+
+/// Guidance text shown when npm / Node.js is not found in the login-shell PATH.
+const NPM_MISSING_HINT: &str = "Node.js / npm was not found. Install Node.js \
+(https://nodejs.org or your version manager), restart Buzz, then click Install again.\n\
+If npm works in your terminal, make sure your Node version manager is initialized in \
+~/.zprofile (not only ~/.zshrc) — Buzz resolves tools via non-interactive login shells.";
+
+/// Result of probing `npm prefix -g` in the hermit-stripped login shell.
+#[cfg(unix)]
+enum NpmPrefix {
+    /// npm responded with a parseable prefix path.
+    Found(std::path::PathBuf),
+    /// npm was not found, the spawn failed, the command returned a non-zero
+    /// exit, or the output could not be parsed.
+    Unavailable,
+    /// The probe exceeded the 30-second deadline (e.g. a version-manager init
+    /// that blocks on `/dev/tty`). The install should proceed so the stderr
+    /// classifier remains the backstop.
+    TimedOut,
+}
+
+/// Spawn the same login shell used by `run_install_command` and run
+/// `npm prefix -g` to discover where npm would install global packages.
+#[cfg(unix)]
+fn resolve_npm_prefix() -> NpmPrefix {
+    let mut cmd = install_shell_command("npm prefix -g");
+    let mut child = match cmd
+        .stdin(std::process::Stdio::null())
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped())
+        .spawn()
+    {
+        Ok(c) => c,
+        Err(_) => return NpmPrefix::Unavailable,
+    };
+
+    // Drain stdout/stderr on background threads to prevent pipe-buffer deadlock.
+    let stdout_pipe = child.stdout.take();
+    let stderr_pipe = child.stderr.take();
+    let stdout_thread = std::thread::spawn(move || {
+        let mut buf = Vec::new();
+        if let Some(mut pipe) = stdout_pipe {
+            let _ = pipe.read_to_end(&mut buf);
+        }
+        buf
+    });
+    let stderr_thread = std::thread::spawn(move || {
+        // Drain stderr so the child doesn't block on a full pipe.
+        if let Some(mut pipe) = stderr_pipe {
+            let _ = std::io::copy(&mut pipe, &mut std::io::sink());
+        }
+    });
+
+    let child_pid = child.id();
+    let (tx, rx) = std::sync::mpsc::channel();
+    let wait_thread = std::thread::spawn(move || {
+        let status = child.wait();
+        let _ = tx.send(status);
+    });
+
+    // 30-second timeout — plenty for `npm prefix -g`; intentionally shorter
+    // than the 5-minute install budget in `run_install_command`.
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
+    let raw_bytes: Option<Vec<u8>> = loop {
+        let remaining = deadline.saturating_duration_since(std::time::Instant::now());
+        if remaining.is_zero() {
+            // Timed out: send SIGTERM, clean up threads, signal the caller to
+            // fall through to the install path rather than abort.
+            unsafe { libc::kill(child_pid as i32, libc::SIGTERM) };
+            drop(rx);
+            let _ = wait_thread.join();
+            let _ = stdout_thread.join();
+            let _ = stderr_thread.join();
+            eprintln!(
+                "buzz: npm prefix probe timed out after 30s; \
+                 proceeding to install (stderr classifier is the backstop)"
+            );
+            return NpmPrefix::TimedOut;
+        }
+        match rx.recv_timeout(std::time::Duration::from_millis(200).min(remaining)) {
+            Ok(Ok(status)) => {
+                let _ = wait_thread.join();
+                let stdout = stdout_thread.join().unwrap_or_default();
+                let _ = stderr_thread.join();
+                break if status.success() { Some(stdout) } else { None };
+            }
+            Ok(Err(_)) | Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
+                let _ = wait_thread.join();
+                let _ = stdout_thread.join();
+                let _ = stderr_thread.join();
+                break None;
+            }
+            Err(std::sync::mpsc::RecvTimeoutError::Timeout) => continue,
+        }
+    };
+
+    let bytes = match raw_bytes {
+        Some(b) => b,
+        None => return NpmPrefix::Unavailable,
+    };
+    let raw = String::from_utf8_lossy(&bytes).into_owned();
+    // Version managers can print banner lines before the real prefix — take the
+    // last non-empty line to skip any preamble.
+    let prefix = match raw.lines().rfind(|l| !l.trim().is_empty()) {
+        Some(l) => l.trim().to_string(),
+        None => return NpmPrefix::Unavailable,
+    };
+    if prefix.is_empty() {
+        return NpmPrefix::Unavailable;
+    }
+    NpmPrefix::Found(std::path::PathBuf::from(prefix))
+}
+
+/// Check write access to a file-system path using the POSIX `access(2)` syscall.
+#[cfg(unix)]
+fn unix_is_writable(path: &std::path::Path) -> bool {
+    use std::os::unix::ffi::OsStrExt;
+    let bytes = path.as_os_str().as_bytes();
+    let Ok(c_path) = std::ffi::CString::new(bytes) else {
+        return false;
+    };
+    // SAFETY: `access` is a pure read-only syscall; we pass a valid NUL-terminated
+    // path and a standard flag constant.  This mirrors the existing `setsid`/`kill`
+    // usage in this file.
+    unsafe { libc::access(c_path.as_ptr(), libc::W_OK) == 0 }
+}
+
+/// Returns true when the directory where npm would write global packages is
+/// writable by the current process user.
+///
+/// On non-unix platforms always returns `true` — the EACCES preflight is a
+/// no-op there; the stderr classifier still applies.
+fn npm_install_target_is_writable(prefix: &std::path::Path) -> bool {
+    #[cfg(unix)]
+    {
+        // Probe the most specific candidate that exists; fall back up the tree.
+        for candidate in &[
+            prefix.join("lib/node_modules"),
+            prefix.join("lib"),
+            prefix.to_path_buf(),
+        ] {
+            if candidate.exists() {
+                return unix_is_writable(candidate);
+            }
+        }
+        // Nothing exists — npm couldn't create it either.
+        unix_is_writable(prefix)
+    }
+    #[cfg(not(unix))]
+    {
+        let _ = prefix;
+        true
+    }
+}
+
+/// Inspect `stderr` for known npm EACCES patterns and return actionable
+/// guidance if matched, or `None` when the error is unrelated.
+fn npm_eacces_hint(stderr: &str, command: &str) -> Option<String> {
+    if stderr.contains("EACCES: permission denied") || stderr.contains("npm error EACCES") {
+        Some(npm_eacces_guidance(command))
+    } else {
+        None
+    }
+}
+
+/// Run the npm preflight before executing an npm global install command.
+/// Returns `Some(failed InstallStepResult)` to abort, or `None` to proceed.
+fn npm_preflight_check(step: &str, command: &str) -> Option<InstallStepResult> {
+    #[cfg(unix)]
+    {
+        match resolve_npm_prefix() {
+            NpmPrefix::Unavailable => Some(InstallStepResult {
+                step: step.to_string(),
+                command: command.to_string(),
+                success: false,
+                stdout: String::new(),
+                stderr: String::new(),
+                exit_code: None,
+                hint: Some(NPM_MISSING_HINT.to_string()),
+            }),
+            NpmPrefix::Found(prefix) if !npm_install_target_is_writable(&prefix) => {
+                Some(InstallStepResult {
+                    step: step.to_string(),
+                    command: command.to_string(),
+                    success: false,
+                    stdout: String::new(),
+                    stderr: format!(
+                        "npm global prefix '{}' is not writable by the current user.",
+                        prefix.display()
+                    ),
+                    exit_code: None,
+                    hint: Some(npm_eacces_guidance(command)),
+                })
+            }
+            // `Found` + writable, or `TimedOut` — proceed; let the install run and
+            // the stderr classifier serve as the backstop.
+            _ => None,
+        }
+    }
+    #[cfg(not(unix))]
+    {
+        let _ = (step, command);
+        None
+    }
+}
+
+// ── end npm preflight ─────────────────────────────────────────────────────────
+
 #[tauri::command]
 pub async fn discover_managed_agent_prereqs(
    input: DiscoverManagedAgentPrereqsRequest,
@@ -342,3 +604,318 @@ pub async fn list_relay_agents(state: State<'_, AppState>) -> Result<Vec<RelayAg
        .unwrap_or_else(|| serde_json::json!([]));
    serde_json::from_value(agents).map_err(|e| format!("agent parse failed: {e}"))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // ── is_npm_global_install ─────────────────────────────────────────────────
+
+    #[test]
+    fn test_is_npm_global_install_accepts_catalog_claude_command() {
+        assert!(is_npm_global_install(
+            "npm install -g @agentclientprotocol/claude-agent-acp"
+        ));
+    }
+
+    #[test]
+    fn test_is_npm_global_install_accepts_catalog_codex_command() {
+        assert!(is_npm_global_install(
+            "npm install -g @zed-industries/codex-acp"
+        ));
+    }
+
+    #[test]
+    fn test_is_npm_global_install_accepts_short_flag() {
+        assert!(is_npm_global_install("npm i -g some-package"));
+    }
+
+    #[test]
+    fn test_is_npm_global_install_accepts_leading_whitespace() {
+        assert!(is_npm_global_install("  npm install -g foo"));
+    }
+
+    #[test]
+    fn test_is_npm_global_install_rejects_curl_pipe() {
+        assert!(!is_npm_global_install(
+            "curl -fsSL https://example.com/install.sh | bash"
+        ));
+    }
+
+    #[test]
+    fn test_is_npm_global_install_rejects_non_global_install() {
+        assert!(!is_npm_global_install("npm install foo"));
+    }
+
+    #[test]
+    fn test_is_npm_global_install_rejects_unrelated_command() {
+        assert!(!is_npm_global_install("cargo install some-tool"));
+    }
+
+    // ── npm_eacces_hint ───────────────────────────────────────────────────────
+
+    #[test]
+    fn test_npm_eacces_hint_detects_old_format() {
+        let stderr = "npm ERR! code EACCES\nnpm ERR! syscall mkdir\nnpm ERR! path /usr/local/lib/node_modules\nnpm ERR! errno -13\nnpm ERR! Error: EACCES: permission denied, mkdir '/usr/local/lib/node_modules'";
+        assert!(npm_eacces_hint(stderr, "npm install -g foo").is_some());
+    }
+
+    #[test]
+    fn test_npm_eacces_hint_detects_new_format() {
+        let stderr = "npm error EACCES: permission denied, mkdir '/usr/local/lib/node_modules'";
+        assert!(npm_eacces_hint(stderr, "npm install -g foo").is_some());
+    }
+
+    #[test]
+    fn test_npm_eacces_hint_returns_none_for_404_stderr() {
+        let stderr = "npm error 404 Not Found - GET https://registry.npmjs.org/no-such-pkg";
+        assert!(npm_eacces_hint(stderr, "npm install -g no-such-pkg").is_none());
+    }
+
+    #[test]
+    fn test_npm_eacces_hint_guidance_contains_npm_global_path() {
+        let hint = npm_eacces_hint("EACCES: permission denied", "npm install -g foo").unwrap();
+        assert!(hint.contains("~/.npm-global"), "hint: {hint}");
+    }
+
+    #[test]
+    fn test_npm_eacces_hint_guidance_contains_zprofile() {
+        let hint = npm_eacces_hint("EACCES: permission denied", "npm install -g foo").unwrap();
+        assert!(hint.contains("~/.zprofile"), "hint: {hint}");
+    }
+
+    #[test]
+    fn test_npm_eacces_hint_guidance_contains_sudo_command() {
+        let hint = npm_eacces_hint("EACCES: permission denied", "npm install -g foo").unwrap();
+        assert!(hint.contains("sudo npm install -g foo"), "hint: {hint}");
+    }
+
+    // ── npm_install_target_is_writable ────────────────────────────────────────
+
+    #[cfg(unix)]
+    #[test]
+    fn test_npm_install_target_is_writable_true_on_writable_dir() {
+        let dir = tempfile::tempdir().unwrap();
+        assert!(npm_install_target_is_writable(dir.path()));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_npm_install_target_is_writable_false_when_lib_node_modules_unwritable() {
+        use std::os::unix::fs::PermissionsExt;
+        let dir = tempfile::tempdir().unwrap();
+        let lib = dir.path().join("lib");
+        let node_modules = lib.join("node_modules");
+        std::fs::create_dir_all(&node_modules).unwrap();
+        // Make node_modules read-only.
+        std::fs::set_permissions(&node_modules, std::fs::Permissions::from_mode(0o555)).unwrap();
+        let result = npm_install_target_is_writable(dir.path());
+        // Restore before the dir is dropped so cleanup can delete it.
+        std::fs::set_permissions(&node_modules, std::fs::Permissions::from_mode(0o755)).unwrap();
+        // Skip this assertion when running as root (root can write to 0o555).
+        if unsafe { libc::getuid() } != 0 {
+            assert!(!result);
+        }
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_npm_install_target_is_writable_walks_up_to_lib() {
+        let dir = tempfile::tempdir().unwrap();
+        // Create only `lib/` — no `lib/node_modules`.
+        std::fs::create_dir(dir.path().join("lib")).unwrap();
+        assert!(npm_install_target_is_writable(dir.path()));
+    }
+
+    // ── npm-preflight E2E (container-only) ───────────────────────────────────
+    //
+    // These tests drive the real `resolve_npm_prefix` / `npm_preflight_check`
+    // path against a crafted environment. They are `#[ignore]`d so normal
+    // `cargo test` / CI skips them; the dedicated Docker harness at
+    // `desktop/src-tauri/e2e/Dockerfile.preflight-e2e` runs them with
+    // `cargo test --lib -- --ignored <test_name>` in a pre-configured container
+    // where npm presence and global-prefix writability are controlled.
+    //
+    // Run locally via:  just desktop-preflight-e2e
+
+    /// (a) npm present, global prefix writable →  preflight returns None (proceed).
+    ///
+    /// Preconditions (set by the container entrypoint for this scenario):
+    ///   - npm is on PATH in the login shell.
+    ///   - ~/.npmrc sets `prefix` to a writable user dir, e.g. ~/.npm-global.
+    ///   - HOME is a temp dir with the npmrc and a minimal .zprofile / .bash_profile
+    ///     that preserves a PATH containing npm.
+    #[cfg(unix)]
+    #[ignore]
+    #[test]
+    fn test_e2e_writable_prefix_proceeds() {
+        // resolve_npm_prefix() must find npm and return a writable prefix.
+        let prefix = match resolve_npm_prefix() {
+            NpmPrefix::Found(p) => p,
+            NpmPrefix::Unavailable => {
+                panic!("resolve_npm_prefix() returned Unavailable — npm must be on PATH in the login shell for this scenario. Check the container HOME setup.");
+            }
+            NpmPrefix::TimedOut => {
+                panic!("resolve_npm_prefix() timed out — check that the login shell init files do not block.");
+            }
+        };
+        assert!(
+            npm_install_target_is_writable(&prefix),
+            "npm global prefix '{}' must be writable for this scenario; \
+             check that ~/.npmrc sets prefix to a user-writable directory",
+            prefix.display()
+        );
+        // The full preflight must pass (return None = proceed).
+        let result = npm_preflight_check("adapter", "npm install -g some-pkg");
+        assert!(
+            result.is_none(),
+            "npm_preflight_check should return None (proceed) when prefix is writable, \
+             got: {result:?}"
+        );
+    }
+
+    /// (b) npm present, global prefix NOT writable → preflight aborts with EACCES guidance.
+    ///
+    /// Preconditions (set by the container entrypoint for this scenario):
+    ///   - npm is on PATH in the login shell.
+    ///   - npm's global prefix resolves to a root-owned path (e.g. /usr/local).
+    ///     No ~/.npmrc prefix override — npm uses its compiled-in default.
+    ///   - Container user is non-root.
+    #[cfg(unix)]
+    #[ignore]
+    #[test]
+    fn test_e2e_readonly_prefix_aborts_with_eacces_guidance() {
+        // resolve_npm_prefix() must find npm.
+        let prefix = match resolve_npm_prefix() {
+            NpmPrefix::Found(p) => p,
+            NpmPrefix::Unavailable => {
+                panic!("resolve_npm_prefix() returned Unavailable — npm must be on PATH in the login shell for this scenario. Check the container HOME setup.");
+            }
+            NpmPrefix::TimedOut => {
+                panic!("resolve_npm_prefix() timed out — check that the login shell init files do not block.");
+            }
+        };
+        assert!(
+            !npm_install_target_is_writable(&prefix),
+            "npm global prefix '{}' must be NOT writable for this scenario; \
+             the container user must not own the prefix (e.g. /usr/local is root-owned)",
+            prefix.display()
+        );
+        // The preflight must abort with success:false and the EACCES guidance hint.
+        let step_result = npm_preflight_check("adapter", "npm install -g some-pkg")
+            .expect("npm_preflight_check should return Some(abort) when prefix is not writable");
+        assert!(
+            !step_result.success,
+            "InstallStepResult.success must be false for an EACCES abort"
+        );
+        let hint = step_result
+            .hint
+            .expect("InstallStepResult.hint must be Some for an EACCES abort");
+        assert!(
+            hint.contains("npm-global"),
+            "EACCES hint should contain npm-global remediation; got: {hint}"
+        );
+        assert!(
+            hint.contains(".zprofile"),
+            "EACCES hint should mention .zprofile; got: {hint}"
+        );
+    }
+
+    /// (c) npm not on PATH → preflight aborts with NPM_MISSING_HINT (regression guard).
+    ///
+    /// This verifies the genuine npm-missing branch is byte-for-byte unchanged
+    /// after the NpmPrefix enum refactor.
+    ///
+    /// Preconditions (set by the container entrypoint for this scenario):
+    ///   - HOME is a temp dir whose .zprofile / .bash_profile explicitly sets
+    ///     PATH to a directory that does NOT contain npm (e.g. PATH=/usr/bin:/bin
+    ///     on a system where npm lives only in /usr/local/bin).
+    ///   - No npm shim on any PATH component.
+    #[cfg(unix)]
+    #[ignore]
+    #[test]
+    fn test_e2e_npm_missing_aborts_with_missing_hint() {
+        // resolve_npm_prefix() must report Unavailable (not TimedOut).
+        match resolve_npm_prefix() {
+            NpmPrefix::Unavailable => {} // expected
+            NpmPrefix::Found(p) => {
+                panic!(
+                    "resolve_npm_prefix() returned Found({}) — npm must NOT be on PATH \
+                     for this scenario; check that HOME's shell init strips npm from PATH",
+                    p.display()
+                );
+            }
+            NpmPrefix::TimedOut => {
+                panic!(
+                    "resolve_npm_prefix() timed out — the shell init must fail quickly, not block"
+                );
+            }
+        }
+        // The preflight must hard-abort with NPM_MISSING_HINT.
+        let step_result = npm_preflight_check("adapter", "npm install -g some-pkg")
+            .expect("npm_preflight_check should return Some(abort) when npm is missing");
+        assert!(
+            !step_result.success,
+            "InstallStepResult.success must be false for a missing-npm abort"
+        );
+        let hint = step_result
+            .hint
+            .expect("InstallStepResult.hint must be Some for a missing-npm abort");
+        assert!(
+            hint.contains("nodejs.org"),
+            "missing-npm hint should contain nodejs.org; got: {hint}"
+        );
+        assert!(
+            hint.contains(".zprofile"),
+            "missing-npm hint should mention .zprofile; got: {hint}"
+        );
+        // Regression guard: the hint text must exactly match NPM_MISSING_HINT.
+        assert_eq!(
+            hint, NPM_MISSING_HINT,
+            "missing-npm hint text must be byte-for-byte NPM_MISSING_HINT"
+        );
+    }
+
+    /// (d) Login shell blocks (wedged version-manager init) → 30s timeout fires,
+    /// preflight returns None so Phase 2 install runs (stderr classifier is backstop).
+    ///
+    /// This is the new behavior added by the NpmPrefix enum: TimedOut → proceed,
+    /// not abort with a misleading "npm not found" message.
+    ///
+    /// Preconditions (set by the container entrypoint for this scenario):
+    ///   - HOME is a temp dir with a .zprofile / .bash_profile that is NORMAL for
+    ///     the `echo $PATH` call made by login_shell_path(), but where PATH contains
+    ///     an npm shim script that runs `sleep 60` — so the npm invocation itself
+    ///     blocks, not the PATH discovery.
+    ///
+    /// Wall-clock: ~30s (the timeout deadline).
+    #[cfg(unix)]
+    #[ignore]
+    #[test]
+    fn test_e2e_wedged_shell_timeout_proceeds() {
+        // resolve_npm_prefix() must return TimedOut, not Unavailable or Found.
+        match resolve_npm_prefix() {
+            NpmPrefix::TimedOut => {} // expected
+            NpmPrefix::Unavailable => {
+                panic!(
+                    "resolve_npm_prefix() returned Unavailable — expected TimedOut. \
+                     Check that the npm shim on PATH blocks (sleeps) rather than failing immediately."
+                );
+            }
+            NpmPrefix::Found(p) => {
+                panic!(
+                    "resolve_npm_prefix() returned Found({}) — npm shim must block, \
+                     not return a valid prefix",
+                    p.display()
+                );
+            }
+        }
+        // The preflight must return None (proceed), not abort.
+        let result = npm_preflight_check("adapter", "npm install -g some-pkg");
+        assert!(
+            result.is_none(),
+            "npm_preflight_check should return None (proceed to install) on timeout; \
+             a wedged shell must not abort with NPM_MISSING_HINT. Got: {result:?}"
+        );
+    }
+}
@@ -560,6 +560,11 @@ pub struct InstallStepResult {
    pub stdout: String,
    pub stderr: String,
    pub exit_code: Option<i32>,
+    /// Actionable guidance shown in the UI when this step failed due to a
+    /// recognized condition (e.g. EACCES on a root-owned npm global prefix).
+    /// `None` when the step succeeded or no pattern matched.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub hint: Option<String>,
 }

 /// Aggregate result of installing a runtime (may include CLI + adapter steps).
@@ -14,6 +14,7 @@ import {
 } from "@/features/agents/hooks";
 import { describeResolvedCommand } from "@/features/agents/ui/agentUi";
 import type { AcpRuntimeCatalogEntry } from "@/shared/api/types";
+import { getInstallErrorMessage } from "@/shared/lib/installError";
 import { cn } from "@/shared/lib/cn";
 import { useTheme } from "@/shared/theme/ThemeProvider";
 import { Badge } from "@/shared/ui/badge";
@@ -264,7 +265,7 @@ function RuntimeCard({
        <RuntimeDetails runtime={runtime} />

        {installError ? (
-          <p className="mt-3 rounded-md border border-destructive/25 bg-destructive/10 px-3 py-2 text-xs leading-5 text-destructive">
+          <p className="mt-3 whitespace-pre-line rounded-md border border-destructive/25 bg-destructive/10 px-3 py-2 text-xs leading-5 text-destructive">
            {installError}
          </p>
        ) : null}
@@ -287,19 +288,6 @@ function RuntimeCard({
  );
 }

-function getInstallErrorMessage(result: {
-  steps: { stderr: string; stdout: string; step: string }[];
-}) {
-  const lastStep = result.steps[result.steps.length - 1];
-  if (!lastStep) {
-    return "Install failed with no output.";
-  }
-
-  return `Step "${lastStep.step}" failed: ${
-    lastStep.stderr || lastStep.stdout || "unknown error"
-  }`;
-}
-
 function RuntimeProvidersSection({
  runtimeProviders,
 }: {
@@ -323,7 +311,7 @@ function RuntimeProvidersSection({
          ...current,
          [runtimeId]: result.success
            ? { error: null, success: true }
-            : { error: getInstallErrorMessage(result), success: false },
+            : { error: getInstallErrorMessage(result.steps), success: false },
        }));
      },
      onError: (error) => {
@@ -15,6 +15,7 @@ import {
 } from "@/features/agents/hooks";
 import { describeResolvedCommand } from "@/features/agents/ui/agentUi";
 import type { AcpRuntimeCatalogEntry } from "@/shared/api/types";
+import { getInstallErrorMessage } from "@/shared/lib/installError";
 import { cn } from "@/shared/lib/cn";
 import { Button } from "@/shared/ui/button";
 import { SettingsOptionGroup } from "./SettingsOptionGroup";
@@ -213,7 +214,7 @@ function RuntimeRow({
          </p>
        ) : null}
        {installError ? (
-          <p className="mt-2 rounded-lg border border-destructive/30 bg-destructive/10 px-3 py-1.5 text-sm text-destructive">
+          <p className="mt-2 whitespace-pre-line rounded-lg border border-destructive/30 bg-destructive/10 px-3 py-1.5 text-sm text-destructive">
            {installError}
          </p>
        ) : null}
@@ -244,14 +245,11 @@ export function DoctorSettingsPanel() {
            [runtimeId]: { success: true, error: null },
          }));
        } else {
-          const lastStep = result.steps[result.steps.length - 1];
          setInstallResults((prev) => ({
            ...prev,
            [runtimeId]: {
              success: false,
-              error: lastStep
-                ? `Step "${lastStep.step}" failed: ${lastStep.stderr || lastStep.stdout || "unknown error"}`
-                : "Install failed with no output.",
+              error: getInstallErrorMessage(result.steps),
            },
          }));
        }
@@ -237,6 +237,7 @@ export type RawInstallStepResult = {
  stdout: string;
  stderr: string;
  exit_code: number | null;
+  hint?: string;
 };

 export type RawInstallRuntimeResult = {
@@ -910,6 +911,7 @@ function fromRawInstallRuntimeResult(
      stdout: step.stdout,
      stderr: step.stderr,
      exitCode: step.exit_code,
+      hint: step.hint,
    })),
  };
 }
@@ -578,6 +578,7 @@ export type InstallStepResult = {
  stdout: string;
  stderr: string;
  exitCode: number | null;
+  hint?: string;
 };

 export type InstallRuntimeResult = {
@@ -0,0 +1,69 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { getInstallErrorMessage } from "./installError.ts";
+
+test("getInstallErrorMessage: empty steps array returns fallback", () => {
+  assert.equal(getInstallErrorMessage([]), "Install failed with no output.");
+});
+
+test("getInstallErrorMessage: failed step without hint contains step name and stderr", () => {
+  const message = getInstallErrorMessage([
+    {
+      step: "adapter",
+      command: "npm install -g @block/buzz-acp",
+      success: false,
+      stdout: "",
+      stderr: "EACCES: permission denied",
+      exitCode: 1,
+    },
+  ]);
+  assert.match(message, /Step "adapter" failed:/);
+  assert.match(message, /EACCES: permission denied/);
+});
+
+test("getInstallErrorMessage: failed step without hint does not contain hint-ish text", () => {
+  const message = getInstallErrorMessage([
+    {
+      step: "adapter",
+      command: "npm install -g @block/buzz-acp",
+      success: false,
+      stdout: "",
+      stderr: "EACCES: permission denied",
+      exitCode: 1,
+    },
+  ]);
+  assert.doesNotMatch(message, /npm config set prefix/);
+});
+
+test("getInstallErrorMessage: failed step with hint starts with hint and still contains stderr", () => {
+  const hint =
+    "Fix the npm prefix ownership:\n  sudo chown -R $USER $(npm config get prefix)";
+  const message = getInstallErrorMessage([
+    {
+      step: "adapter",
+      command: "npm install -g @block/buzz-acp",
+      success: false,
+      stdout: "",
+      stderr: "EACCES: permission denied, mkdir '/usr/local/lib'",
+      exitCode: 1,
+      hint,
+    },
+  ]);
+  assert.ok(message.startsWith(hint), "message should start with hint");
+  assert.match(message, /EACCES: permission denied/);
+});
+
+test("getInstallErrorMessage: failed step with empty stderr falls back to stdout", () => {
+  const message = getInstallErrorMessage([
+    {
+      step: "node",
+      command: "node --version",
+      success: false,
+      stdout: "some stdout output",
+      stderr: "",
+      exitCode: 1,
+    },
+  ]);
+  assert.match(message, /some stdout output/);
+});
@@ -0,0 +1,15 @@
+import type { InstallStepResult } from "@/shared/api/types";
+
+/**
+ * Build the user-visible error message for a failed install.
+ * When the last step carries an actionable hint, it is shown first,
+ * followed by the raw step failure detail.
+ */
+export function getInstallErrorMessage(steps: InstallStepResult[]): string {
+  const lastStep = steps[steps.length - 1];
+  if (!lastStep) {
+    return "Install failed with no output.";
+  }
+  const base = `Step "${lastStep.step}" failed: ${lastStep.stderr || lastStep.stdout || "unknown error"}`;
+  return lastStep.hint ? `${lastStep.hint}\n\n${base}` : base;
+}