fix(setup): install harness CLIs into a user-owned npm prefix
A system Node install points npm's global prefix at a root-owned directory (/usr/local), so `npm install -g <pkg>` fails with EACCES and setup reports a generic "install failed". The reflex fix, sudo, is what the vendor docs warn against. Probe npm's global prefix for writability and, when it is not writable, target `--prefix ~/.local` instead. That dir is already on resolve_cli_binary's fallback ladder, so the freshly-installed CLI still resolves. The change lives in harness_install_command, the single source for both the argv we exec and every "run it manually" hint, so the fallback message now prints a command that works rather than the one that just failed. Signed-off-by: Dhruv Gupta <dhruv0811@gmail.com>
This commit is contained in:
@@ -40,6 +40,7 @@ import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from functools import cache
|
||||
from pathlib import Path
|
||||
from typing import NamedTuple
|
||||
|
||||
@@ -556,9 +557,36 @@ def harness_cli_installed(key: str) -> bool:
|
||||
return resolve_cli_binary(spec.binary) is not None
|
||||
|
||||
|
||||
@cache
|
||||
def _npm_global_prefix_writable() -> bool:
|
||||
"""Whether ``npm install -g`` can write to npm's configured global prefix.
|
||||
|
||||
A system Node install points the global prefix at a root-owned dir
|
||||
(``/usr/local``), where a bare ``npm install -g`` dies with ``EACCES``.
|
||||
Cached because the readiness screens render install hints per family.
|
||||
"""
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
["npm", "prefix", "-g"], check=False, capture_output=True, text=True, timeout=30
|
||||
)
|
||||
except (OSError, subprocess.TimeoutExpired):
|
||||
return True # can't tell — keep the plain global install
|
||||
prefix = Path(proc.stdout.strip() or "/")
|
||||
# npm writes bin/ and lib/node_modules under the prefix; permission is
|
||||
# decided by the deepest ancestor that already exists.
|
||||
while not prefix.exists() and prefix != prefix.parent:
|
||||
prefix = prefix.parent
|
||||
return os.access(prefix, os.W_OK)
|
||||
|
||||
|
||||
def harness_install_command(key: str) -> list[str]:
|
||||
"""Return the argv that installs the harness CLI.
|
||||
|
||||
When npm's global prefix is root-owned, the argv targets a user-owned
|
||||
prefix (``~/.local``, already probed by
|
||||
:func:`omnigent._platform.resolve_cli_binary`) instead of failing with
|
||||
``EACCES`` — ``sudo npm install -g`` is what the vendor docs warn against.
|
||||
|
||||
:param key: A harness family or :data:`PI_KEY`.
|
||||
:returns: The install command, e.g. ``["npm", "install", "-g",
|
||||
"@anthropic-ai/claude-code"]`` or an explicitly configured vendor
|
||||
@@ -576,6 +604,8 @@ def harness_install_command(key: str) -> list[str]:
|
||||
package = spec.package
|
||||
if package is None:
|
||||
raise ValueError(f"{key!r} has no npm package; show its install_hint instead")
|
||||
if not _npm_global_prefix_writable():
|
||||
return ["npm", "install", "-g", "--prefix", str(Path.home() / ".local"), package]
|
||||
return ["npm", "install", "-g", package]
|
||||
|
||||
|
||||
|
||||
@@ -13,6 +13,10 @@ import omnigent._platform as _platform
|
||||
from omnigent.onboarding import harness_install as hi
|
||||
from omnigent.onboarding.provider_config import ANTHROPIC_FAMILY, GEMINI_FAMILY, OPENAI_FAMILY
|
||||
|
||||
# Bound before the autouse fixture can stub it, so the probe's own test runs
|
||||
# the real implementation.
|
||||
_REAL_PREFIX_PROBE = hi._npm_global_prefix_writable
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _stub_cli_fallback_dirs(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
@@ -28,6 +32,56 @@ def _stub_cli_fallback_dirs(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(_platform, "_cli_fallback_dirs", lambda: ())
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _writable_npm_prefix(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Pin the npm-global-prefix probe writable, and drop its cache.
|
||||
|
||||
``harness_install_command`` appends ``--prefix ~/.local`` when npm's global
|
||||
prefix is root-owned, so the plain-``-g`` assertions here would flip on a
|
||||
machine with a system Node. The cache is cleared so the tests that *do*
|
||||
exercise the root-owned branch can't leak a verdict into the others.
|
||||
"""
|
||||
hi._npm_global_prefix_writable.cache_clear()
|
||||
monkeypatch.setattr(hi, "_npm_global_prefix_writable", lambda: True)
|
||||
|
||||
|
||||
def test_install_command_uses_user_prefix_when_global_is_root_owned(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A root-owned npm prefix installs into ``~/.local`` rather than EACCES-ing.
|
||||
|
||||
``sudo npm install -g`` is what the vendor docs warn against, and
|
||||
``~/.local/bin`` is already on ``resolve_cli_binary``'s ladder, so the
|
||||
freshly-installed CLI still resolves.
|
||||
"""
|
||||
monkeypatch.setattr(hi, "_npm_global_prefix_writable", lambda: False)
|
||||
assert hi.harness_install_command(ANTHROPIC_FAMILY) == [
|
||||
"npm",
|
||||
"install",
|
||||
"-g",
|
||||
"--prefix",
|
||||
str(Path.home() / ".local"),
|
||||
"@anthropic-ai/claude-code",
|
||||
]
|
||||
|
||||
|
||||
def test_npm_global_prefix_writable_detects_root_owned(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
"""The probe reads ``npm prefix -g`` and reports the dir's writability."""
|
||||
prefix = tmp_path / "usr" / "local"
|
||||
prefix.mkdir(parents=True)
|
||||
|
||||
def _run(argv: list[str], **_: object) -> subprocess.CompletedProcess[str]:
|
||||
assert argv == ["npm", "prefix", "-g"]
|
||||
return subprocess.CompletedProcess(argv, 0, stdout=f"{prefix}\n", stderr="")
|
||||
|
||||
monkeypatch.setattr(hi.subprocess, "run", _run)
|
||||
monkeypatch.setattr(hi.os, "access", lambda p, _mode: Path(p) != prefix)
|
||||
assert _REAL_PREFIX_PROBE() is False
|
||||
_REAL_PREFIX_PROBE.cache_clear()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"key,binary,package",
|
||||
[
|
||||
|
||||
Reference in New Issue
Block a user