fix(desktop): preflight npm prefix writability in doctor installs#1732
Merged
Conversation
The doctor's adapter step ran npm install -g blind; on machines where npm's global prefix is root-owned (nodejs.org pkg installs, or version managers initialized only in .zshrc which login shells never source), users got a raw truncated EACCES dump with no way forward. The install now resolves npm's effective prefix in the same stripped-env login shell, fails fast when the target isn't writable, and classifies EACCES stderr as a fallback. A new optional hint field on InstallStepResult carries the remediation steps, rendered above the raw output in both the Doctor panel and onboarding setup.
Three follow-ups from code review: 1. Add a 30-second timeout to resolve_npm_prefix() using the same spawn+channel+SIGTERM pattern already in run_install_command. A wedged login shell (e.g. a blocking version-manager init) would have hung the preflight indefinitely; now it falls through to the stderr classifier path on timeout. 2. Extract install_shell_command() as the single source of truth for shell selection (/bin/zsh vs /bin/bash), hermit env-var strip (NPM_CONFIG_PREFIX / NPM_CONFIG_CACHE / COREPACK_HOME), PATH injection, and the setsid pre_exec. Both run_install_command and resolve_npm_prefix now call it, so the hermit-strip list cannot drift between the two paths. 3. Add a comment at the Phase-1 CLI-install loop noting that the npm EACCES preflight and classifier run only in Phase 2 because every cli_install_commands entry today is a curl-pipe; a future npm- global CLI install would need to add the guard there explicitly. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The previous commit returned None from resolve_npm_prefix() on timeout,
but npm_preflight_check maps None to a hard abort with NPM_MISSING_HINT
('Node.js / npm was not found'). A wedged-shell timeout would therefore
surface a misleading error and never reach the stderr classifier.
Introduce a NpmPrefix enum with three variants:
- Found(PathBuf): npm responded with a parseable prefix
- Unavailable: spawn failed, non-zero exit, or unparseable output
- TimedOut: 30s deadline exceeded
npm_preflight_check now maps:
- Unavailable → hard abort with NPM_MISSING_HINT (unchanged)
- Found + unwritable → EACCES guidance abort (unchanged)
- Found + writable or TimedOut → None (proceed to install)
On timeout the preflight logs a single eprintln so the fall-through is
not silent, then returns TimedOut, allowing run_install_command to run
and the stderr classifier to serve as the backstop.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Four scenarios covering the NpmPrefix enum paths added in the preceding
commits, all #[ignore]d so normal cargo test / CI skips them:
(a) writable prefix → npm_preflight_check returns None (proceed)
(b) read-only prefix → aborts with EACCES guidance hint
(c) npm not on PATH → aborts with NPM_MISSING_HINT (regression guard
for the byte-for-byte-unchanged branch Will was most concerned about)
(d) wedged login shell (npm shim sleeps 60s) → 30s timeout fires,
NpmPrefix::TimedOut → npm_preflight_check returns None (proceed)
Each test runs in a fresh subprocess (separate docker run) so the
login_shell_path() OnceLock is initialised from the correct HOME for
that scenario. The container entrypoint creates four isolated HOME dirs
under /tmp with crafted shell init files and optional ~/.npmrc before
invoking the pre-compiled test binary.
Harness files:
desktop/src-tauri/e2e/Dockerfile.preflight-e2e — builds on
rust:1.95-slim-bookworm with Node.js + zsh; non-root testuser;
no host volume mounts (host npm/PATH untouched)
desktop/src-tauri/e2e/run-e2e-scenarios.sh — entrypoint
desktop/src-tauri/e2e/Dockerfile.preflight-e2e.dockerignore
— overrides root .dockerignore so desktop/src-tauri/ is included
just target: desktop-preflight-e2e (docker build + docker run --rm)
Normal cargo test surface unchanged: 16 passed, 4 ignored.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This was referenced Jul 10, 2026
tlongwell-block
pushed a commit
that referenced
this pull request
Jul 11, 2026
Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz> * origin/main: fix(desktop): preserve archived observer history (#1752) fix(dev): install Lefthook hooks into shared .git/hooks dir (#1751) fix(desktop): surface codex config-parse failures and -32603 internal errors clearly (#1745) fix(desktop): fix workspace rail badge disappearance and persist mark-as-unread (#1747) chore(desktop): remove container-only npm-preflight E2E harness (#1749) fix(desktop): preflight npm prefix writability in doctor installs (#1732)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The doctor's adapter step was running
npm install -gblind — on machines where npm's global prefix is root-owned (nodejs.org pkg installer leaves/usr/localowned by root; version managers initialized only in~/.zshrcare invisible to the login shells Buzz uses), users hit a raw truncated EACCES dump with no path forward.Before running any
npm install -gadapter command, the doctor now resolvesnpm prefix -gin the same hermit-stripped login shell the install would use. If the prefix isn't writable it fails immediately with actionable remediation steps; if npm is missing it says that with a pointer to install instructions. An EACCES stderr classifier catches failures that slip past the preflight. A new optionalhintonInstallStepResultcarries the guidance to the frontend, rendered above the raw output in both the Doctor settings panel and onboarding setup.resolve_npm_prefix()spawns the same zsh/bash login shell asrun_install_command, stripsNPM_CONFIG_PREFIX/NPM_CONFIG_CACHE/COREPACK_HOME, and takes the last non-empty stdout line to skip version manager bannersnpm_install_target_is_writable()probesprefix/lib/node_modules→prefix/lib→prefixvia POSIXaccess(2); non-unix always returnstrue(the EACCES classifier still applies)npm_eacces_hint()matches both old (npm ERR! EACCES: permission denied) and new (npm error EACCES) npm stderr formatsinstallError.tsshared helper consolidates the error-message logic from bothDoctorSettingsPanelandSetupStep; error elements gainwhitespace-pre-lineso the multi-line guidance renders correctly