fix(code-health): TypeScript install honours the repo .npmrc; npm failures exit 2
Why (round-5 review, unsupportedpastels m5, D4): `npm install --prefix <cache>` makes the cache dir the npm project, so the repo's .npmrc (min-release-age=14) was ignored: `npm config get min-release-age` from the prefix was undefined. The repo .npmrc is now copied into the prefix before installing (verified: `npm config get min-release-age --prefix <cache>` -> 14). A failed or hung install raised CalledProcessError/TimeoutExpired, which cli.main does not catch, so the run exited 1 with a traceback that read like a finding; both now raise RuntimeError with npm's stderr, which cli.main maps to exit 2.
This commit is contained in:
@@ -30,6 +30,33 @@ def _cache_root() -> Path:
|
||||
return Path(base) / "hermes-code-health"
|
||||
|
||||
|
||||
def _install(repo: Path, prefix: Path, pin: str) -> None:
|
||||
npm = shutil.which("npm")
|
||||
if not npm:
|
||||
raise RuntimeError("code health needs node + npm to measure TypeScript files")
|
||||
prefix.mkdir(parents=True, exist_ok=True)
|
||||
# With --prefix, npm reads <prefix>/.npmrc as the project config and never the repo's, so
|
||||
# the repo's install policy (min-release-age, registry) is copied in to apply here too.
|
||||
npmrc = repo / ".npmrc"
|
||||
if npmrc.is_file():
|
||||
shutil.copyfile(npmrc, prefix / ".npmrc")
|
||||
else:
|
||||
(prefix / ".npmrc").unlink(missing_ok=True)
|
||||
what = f"npm install typescript@{pin} into {prefix}"
|
||||
try:
|
||||
subprocess.run(
|
||||
[npm, "install", "--no-save", "--no-package-lock", "--no-audit", "--no-fund",
|
||||
"--prefix", str(prefix), f"typescript@{pin}"],
|
||||
check=True, capture_output=True, text=True, encoding="utf-8", errors="replace",
|
||||
timeout=300, stdin=subprocess.DEVNULL,
|
||||
)
|
||||
# cli.main reports a RuntimeError as a tool failure (exit 2), never as a finding.
|
||||
except subprocess.CalledProcessError as exc:
|
||||
raise RuntimeError(f"{what} failed: {(exc.stderr or '').strip()[-2000:]}") from exc
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
raise RuntimeError(f"{what} timed out after {exc.timeout:.0f}s") from exc
|
||||
|
||||
|
||||
def resolve_typescript(repo: Path) -> Path:
|
||||
"""Directory of the pinned ``typescript`` package, installing it into a cache if absent."""
|
||||
pin = pinned_typescript(repo)
|
||||
@@ -41,14 +68,7 @@ def resolve_typescript(repo: Path) -> Path:
|
||||
prefix = _cache_root() / f"typescript-{pin}"
|
||||
cand = prefix / "node_modules" / "typescript"
|
||||
if _version_at(cand) != pin:
|
||||
npm = shutil.which("npm")
|
||||
if not npm:
|
||||
raise RuntimeError("code health needs node + npm to measure TypeScript files")
|
||||
subprocess.run(
|
||||
[npm, "install", "--no-save", "--no-package-lock", "--no-audit", "--no-fund",
|
||||
"--prefix", str(prefix), f"typescript@{pin}"],
|
||||
check=True, capture_output=True, timeout=300, stdin=subprocess.DEVNULL,
|
||||
)
|
||||
_install(repo, prefix, pin)
|
||||
return cand
|
||||
|
||||
|
||||
|
||||
@@ -8,12 +8,13 @@ from __future__ import annotations
|
||||
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from scripts.code_health.ts_measure import pinned_typescript
|
||||
from tests.scripts.test_code_health import REPO, _commit, _repo, _verdict
|
||||
from scripts.code_health.ts_measure import pinned_typescript, resolve_typescript
|
||||
from tests.scripts.test_code_health import REPO, _commit, _git, _repo, _verdict
|
||||
|
||||
|
||||
# --- TypeScript identity ----------------------------------------------------------------
|
||||
@@ -89,3 +90,28 @@ def test_ts_identity_ignores_comments(tmp_path, capsys, files, blocks):
|
||||
repo, base = _ts_base(tmp_path, {"web/l.ts": _TS_LEGACY})
|
||||
code, out = _verdict(repo, base, files, capsys)
|
||||
assert code == (1 if blocks else 0), out
|
||||
|
||||
|
||||
# --- m5: the TypeScript install honours the repo's .npmrc and fails as a RuntimeError -----
|
||||
|
||||
def test_typescript_install_uses_repo_npmrc_and_fails_cleanly(tmp_path, monkeypatch):
|
||||
npm = shutil.which("npm")
|
||||
if npm is None:
|
||||
pytest.skip("npm is not installed")
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
_git(repo, "init", "-q", "-b", "main")
|
||||
(repo / "package-lock.json").write_text(
|
||||
json.dumps({"packages": {"node_modules/typescript": {"version": pinned_typescript(REPO)}}}),
|
||||
encoding="utf-8")
|
||||
# An unreachable registry and an empty npm cache: only the repo's .npmrc can say so.
|
||||
(repo / ".npmrc").write_text(
|
||||
f"registry=http://127.0.0.1:9/\nfetch-retries=0\ncache={tmp_path / 'npm-cache'}\nmin-release-age=14\n",
|
||||
encoding="utf-8")
|
||||
monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache"))
|
||||
with pytest.raises(RuntimeError, match="typescript"):
|
||||
resolve_typescript(repo)
|
||||
prefix = next((tmp_path / "cache" / "hermes-code-health").iterdir())
|
||||
got = subprocess.run([npm, "config", "get", "min-release-age", "--prefix", str(prefix)],
|
||||
cwd=prefix, capture_output=True, text=True, timeout=60, check=True).stdout.strip()
|
||||
assert got == "14"
|
||||
|
||||
Reference in New Issue
Block a user