Skip to content

fix: install playwright-cli into a private prefix when npm's is root-owned - #8637

Open
RadiumGu wants to merge 1 commit into
kirodotdev:mainfrom
RadiumGu:fix/browser-cli-unwritable-npm-prefix
Open

fix: install playwright-cli into a private prefix when npm's is root-owned#8637
RadiumGu wants to merge 1 commit into
kirodotdev:mainfrom
RadiumGu:fix/browser-cli-unwritable-npm-prefix

Conversation

@RadiumGu

@RadiumGu RadiumGu commented Sep 5, 2026

Copy link
Copy Markdown

Problem

On a host whose npm global prefix is not writable by the running user, the Browser settings panel's install button could never succeed. install() ran a bare npm install -g @playwright/cli@latest and returned on the first failure:

npm error code EACCES
npm error syscall mkdir
npm error path /usr/local/node-v22.12.0-linux-arm64/lib/node_modules/@playwright
npm error Error: EACCES: permission denied, mkdir '.../node_modules/@playwright'

Measured on Amazon Linux 2023 (arm64) where that prefix's lib/node_modules is owned by nobody:nobody mode 0755. A root-owned npm prefix is an ordinary host property — a distribution tarball unpacked into /usr/local, a system package, a container image built as root — not an operator mistake. The only exit was the standalone installer, which the operator had to notice, copy and run in their own terminal.

The module already had every piece needed to avoid this: KIROCREW_PLAYWRIGHT_CLI_HOME, _standalone_node_modules() probing an unprivileged prefix, and playwright-cli.sh performing exactly this install. install() just never used them.

Change

On an EACCES/EPERM/EROFS failure only, retry into a prefix the user owns (<config dir>/playwright-cli), reusing npm's --global --prefix layout so _standalone_node_modules() and _launcher_node_modules() need no new special case.

Matched on npm's error code, not its prose. npm's own advice — "try running the command again as root/Administrator" — is the one remedy that must not be taken: it would leave a root-owned tree the gateway then cannot upgrade. A registry rejection or 403 is deliberately not retried: a different prefix cannot fix it, and retrying would repeat the failure while hiding the real error behind a second one.

Three supporting changes:

  • cli_path() and cli_env() also search the private prefix bin dir, appended so a system-wide npm install -g still wins whenever one exists. Without this the retry would SUCCEED while the panel kept reporting the CLI absent — the failure shape cli_path()'s own docstring calls the worst available here.
  • A best-effort ~/.local/bin symlink covers the agent, which is documented to run playwright-cli as a plain shell command and so resolves the login $PATH, which can never hold a KiroCrew-chosen prefix. Same convention playwright-cli.sh follows. It never replaces a non-symlink already there, and failure is logged rather than failing the step.
  • The refused global attempt stays in the steps list so the operator sees what happened, but no longer vetoes an install the retry completed — the same pattern _download_browser() already uses for a refused --with-deps.

Verification

End to end on the failing host:

step before after
npm-install-global EACCES rc=243, install aborts EACCES rc=243, classified, retried
npm-install-private-prefix ok
install-browser never reached ok
install-skills never reached ok

detect() afterwards: installed: True, cli_version: 0.1.19, browser_ok: True. bash -lc 'playwright-cli --version' resolves through the symlink, and the browser-revision gate reads chromium-1243 through the package tree, confirming attribution survives the symlink chain.

10 new tests; 277 passed / 3 skipped across test_browser_cli_install.py, test_browser_cli_launch.py, test_browser_cli_os_deps.py, test_playwright_cli_installer.py, test_spawn_audit.py. Two existing cli_env PATH-composition assertions were updated for the appended layer.

Not fixed here

One adjacent defect found while verifying, in os_deps.py rather than install.py, and now fixed separately in #8639: install-browser reports ok on a host missing browser libraries. os_deps.host_deps_unsatisfied keys on Playwright's "Host system is missing dependencies" warning, which Playwright never emits here — its registry scans chrome-linux while the arm64 build unpacks into chrome-linux-arm64, so validation finds nothing, calls the host good and writes a 30-day DEPENDENCIES_VALIDATED marker. A false negative, not a missing warning.

Correction to an earlier revision of this description: it also claimed manual_deps_command() omits mesa-libgbm. That was wrong — the package has always been in _RPM_CHROMIUM_PACKAGES (35 packages, mesa-libgbm included). The claim came from reading a truncated repr(...)[:400] of the generated command, which cuts off mid-token at libxs, hiding the last four packages. No such defect exists.

…owned

On a host whose npm global prefix is not writable by the running user,
`npm install -g @playwright/cli` fails EACCES and the Browser panel's
install button could never succeed. The only exit was the standalone
installer, which the operator had to notice, copy and run themselves.

Measured on Amazon Linux where the prefix
/usr/local/node-v22.12.0-linux-arm64/lib/node_modules is owned by
`nobody`. A root-owned prefix is an ordinary host property -- a
distribution tarball unpacked into /usr/local, a system package, a
container image built as root -- not an operator mistake.

On EACCES/EPERM/EROFS only, retry into a prefix this user owns, reusing
npm's `--global --prefix` layout so the existing package-tree and
browser-revision probes need no new special case. Matched on npm's error
code rather than its prose, because its own advice ("run again as root")
is the one remedy that must not be taken: it would leave a root-owned
tree the gateway then cannot upgrade. A registry rejection is not
retried -- a different prefix cannot fix it, and retrying would hide the
real error behind a second one.

cli_path() and cli_env() now also search the private prefix bin dir,
appended so a system-wide install still wins when one exists. Without
that, the retry would SUCCEED while the panel kept reporting the CLI
absent. A best-effort ~/.local/bin symlink covers the agent, which runs
playwright-cli as a plain shell command and so resolves the login PATH;
it never replaces a non-symlink already there.

Verified end to end on the failing host: the global attempt reproduces
EACCES rc=243 and is classified, the retry succeeds, and install() now
completes all four steps with browser_ok true.
@RadiumGu
RadiumGu requested a review from a team as a code owner September 5, 2026 03:13
@RadiumGu
RadiumGu requested a review from cixuuz September 5, 2026 03:13
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention labels Sep 5, 2026
@dwu96

dwu96 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

1 similar comment
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants