fix(update): anchor the web toolchain check on npm's actual search path

Reshapes the salvaged fix so it recovers the `tsc: not found` build without
mis-diagnosing healthy trees.

The npm config forcing is dropped. It rested on the premise that an inherited
`npm_config_omit=dev` can beat the `--include=dev` CLI flag; npm resolves
command-line flags above environment config and filters `omit` by `include`,
so the flag already wins. Verified on npm 10 and 11.5.1: with
`npm_config_omit=dev` set and `--include=dev` passed, devDependencies install.
`npm_config_production` is worse than redundant — npm 9 removed it, so setting
it prints `npm warn config production Use --omit=dev instead.` on every
install.

The readiness probe now reads every root `npm run build` searches, not just
the workspace root. npm links a package's bin shims under the package itself
when it owns its lockfile (#42973), so a root-only check called a working tree
broken: it forced a redundant install, skipped the build entirely, and made
`hermes web` exit 1 on a layout that builds fine today.

The pre-build probe is gone with it. A build that works is never second-guessed
and no filesystem introspection gates it; recovery is driven by the failure
instead. When the build cannot resolve tsc or vite, we reinstall (visibly) and
retry before the generic delayed retry, which otherwise just reruns the same
command and leaves the stale dist in place forever. The lockfile-hash skip
still invalidates on an incomplete toolchain so the next update repairs itself.

Also drops the branches that changed behavior based on whether a test mock was
installed, and replaces the mock-call-count tests with real temp trees covering
both hoisting layouts, the Windows shim extensions, and each shell's wording of
an unresolvable binary.

Co-authored-by: Gerardo Camorlinga Jr. <gercamjr.dev@gmail.com>
This commit is contained in:
Brooklyn Nicholson 2026-07-27 12:52:42 -05:00
parent 363d1aefa4
commit 690ecfa1c7
3 changed files with 198 additions and 179 deletions

View file

@ -5195,15 +5195,6 @@ def _run_npm_install_deterministic(
# --silent/capture_output). It no-ops when CI is set — same as the TUI
# install path and nix/lib.nix npm ci hooks.
run_env = {**os.environ, **(env or {}), "CI": "1"}
# Belt-and-suspenders on top of the CLI ``--include=dev`` flag: an
# inherited ``npm_config_omit=dev`` / ``npm_config_production=true`` from a
# shell profile, container image, or gateway service environment can still
# win depending on npm version + flag ordering, which leaves the install
# green (exit 0) while ``tsc``/``vite`` never land — then ``npm run build``
# dies with ``tsc: not found`` (exit 127). Force the config-level knobs.
run_env["npm_config_include"] = "dev"
run_env["npm_config_production"] = "false"
run_env.pop("npm_config_omit", None)
lockfile = cwd / "package-lock.json"
if lockfile.exists():
@ -5235,11 +5226,6 @@ def _run_npm_install_deterministic(
)
def _node_modules_bin_dir(root: Path) -> Path:
"""Return ``root/node_modules/.bin`` (may not exist yet)."""
return root / "node_modules" / ".bin"
def _npm_bin_exists(bin_dir: Path, name: str) -> bool:
"""True when an npm bin shim for *name* exists (POSIX or Windows)."""
return any(
@ -5248,34 +5234,52 @@ def _npm_bin_exists(bin_dir: Path, name: str) -> bool:
)
def _web_build_toolchain_ready(project_root: Path) -> bool:
"""Return True when the web UI build toolchain shims are present.
def _web_build_toolchain_ready(*roots: Path) -> bool:
"""True when ``tsc`` and ``vite`` shims are reachable from any of *roots*.
``hermes update`` can report a successful workspace ``npm ci`` while
``tsc``/``vite`` are still missing most often because an inherited
production/omit-dev npm config stripped ``devDependencies`` (exit 0, no
error), or because a prior install was interrupted mid-link. The web
build then fails with ``tsc: not found``. Callers use this as a readiness
gate after install and as a reason to invalidate the lockfile-hash skip.
Callers must pass every root the build would search; checking only one
reports a healthy tree as broken.
"""
if not (project_root / "web" / "package.json").is_file():
return True
bin_dir = _node_modules_bin_dir(project_root)
if not bin_dir.is_dir():
return False
return _npm_bin_exists(bin_dir, "tsc") and _npm_bin_exists(bin_dir, "vite")
bin_dirs = [
bin_dir
for bin_dir in (root / "node_modules" / ".bin" for root in roots)
if bin_dir.is_dir()
]
return bool(bin_dirs) and all(
any(_npm_bin_exists(bin_dir, tool) for bin_dir in bin_dirs)
for tool in ("tsc", "vite")
)
def _prepend_node_bin_dirs(env: dict[str, str], *roots: Path) -> dict[str, str]:
"""Prepend each root's ``node_modules/.bin`` onto PATH (existing dirs only)."""
merged = dict(env)
parts = [p for p in merged.get("PATH", "").split(os.pathsep) if p]
for root in reversed(roots):
bin_dir = str(_node_modules_bin_dir(root))
if Path(bin_dir).is_dir() and bin_dir not in parts:
parts.insert(0, bin_dir)
merged["PATH"] = os.pathsep.join(parts)
return merged
def _web_toolchain_roots(web_dir: Path) -> tuple[Path, ...]:
"""Roots whose ``node_modules/.bin`` can satisfy the web build.
``npm run build`` prepends ``node_modules/.bin`` for the package and each
of its ancestors, so shims hoisted to the workspace root and shims nested
under a package that owns its lockfile (#42973) are equally valid.
"""
return (web_dir, web_dir.parent)
def _missing_web_build_tool(output: str) -> str | None:
"""Return the build tool a failed ``npm run build`` could not resolve.
Each shell words this differently: ``sh: 1: tsc: not found`` (dash),
``vite: command not found`` (bash/zsh), and ``'tsc' is not recognized as
an internal or external command`` (cmd.exe).
"""
lowered = output.lower()
for tool in ("tsc", "vite"):
if any(
phrase in lowered
for phrase in (
f"{tool}: not found",
f"{tool}: command not found",
f"'{tool}' is not recognized",
)
):
return tool
return None
def _build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool:
@ -5387,23 +5391,13 @@ def _do_build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool:
npm_cwd, npm_workspace_args = _termux_workspace_install_context(web_dir)
def _install_web_deps(*, silent: bool) -> "subprocess.CompletedProcess":
extra = (*npm_workspace_args,)
if silent:
extra = (*extra, "--silent")
return _run_npm_install_deterministic(
npm,
npm_cwd,
extra_args=extra,
extra_args=(*npm_workspace_args, "--silent") if silent else npm_workspace_args,
env=build_env,
)
def _refresh_build_path() -> dict[str, str]:
# npm lifecycle scripts normally inject node_modules/.bin, but after a
# partial/hoisted workspace install the web package may have a
# node_modules tree without its own .bin while tsc/vite live at the
# workspace root. Prepending both keeps `tsc -b && vite build` resolvable.
return _prepend_node_bin_dirs(build_env, web_dir, npm_cwd)
r1 = _install_web_deps(silent=True)
if r1.returncode != 0:
_say(
@ -5414,49 +5408,6 @@ def _do_build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool:
if fatal:
_say(" Run manually: npm install --workspace web && npm run build -w web")
return False
# Install can exit 0 while still omitting devDependencies (production/omit
# config races). Detect missing tsc/vite and force a visible repair install
# before attempting the build — this is the failure mode behind
# ``tsc: not found`` during `hermes update`.
project_root_for_tools = (
web_dir.parent.parent if web_dir.parent.name == "apps" else web_dir.parent
)
# Only repair when an install produced a node_modules tree but left the
# toolchain incomplete. Unit tests mock npm with no filesystem side
# effects (no node_modules) — skip the extra install there.
if (
(project_root_for_tools / "node_modules").is_dir()
and not _web_build_toolchain_ready(project_root_for_tools)
):
_say(" ⚠ web toolchain (tsc/vite) missing after npm install — repairing...")
r_repair = _install_web_deps(silent=False)
if r_repair.returncode != 0:
_say(
f" {'' if fatal else ''} Web UI toolchain repair failed"
+ ("" if fatal else " (hermes web will not be available)")
)
_relay(r_repair)
if fatal:
_say(
" Run manually: npm install --workspace web --include=dev "
"&& npm run build -w web"
)
return False
if not _web_build_toolchain_ready(project_root_for_tools):
_say(
f" {'' if fatal else ''} tsc/vite still missing after repair"
+ ("" if fatal else " (hermes web will not be available)")
)
if fatal:
_say(
" Run manually: npm install --workspace web --include=dev "
"&& npm run build -w web"
)
return not fatal
build_env = _refresh_build_path()
# First attempt — stream output via idle-timeout helper (issue #33788).
# capture_output=True on a long Vite build looks identical to a hang;
# users react by rebooting, which leaves the editable install in a
@ -5464,13 +5415,15 @@ def _do_build_web_ui(web_dir: Path, *, fatal: bool = False) -> bool:
# recoverable (the stale-dist fallback below handles the kill path).
r2 = _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir, env=build_env)
if r2.returncode != 0:
build_out = (r2.stdout or "") + (r2.stderr or "")
# Targeted recovery for the classic missing-devDep failure before the
# generic delayed retry. Re-install (non-silent) + refresh PATH.
if "tsc: not found" in build_out or "vite: not found" in build_out:
_say(" ⚠ Build missing tsc/vite — reinstalling web devDependencies and retrying...")
# The install above can exit 0 while leaving the tree without a build
# toolchain — a lockfile-hash skip over a half-installed tree, or an
# interrupted link step. The generic retry below just reruns the same
# command, so `tsc: not found` survives it and the stale dist is
# served forever. Reinstall (non-silent, so the user sees it) first.
missing_tool = _missing_web_build_tool((r2.stdout or "") + (r2.stderr or ""))
if missing_tool:
_say(f" ⚠ Build could not resolve {missing_tool} — reinstalling web dependencies...")
_install_web_deps(silent=False)
build_env = _refresh_build_path()
r2 = _run_with_idle_timeout([npm, "run", "build"], cwd=web_dir, env=build_env)
if r2.returncode != 0:
# Retry once after a short delay — covers boot-time races on Windows
@ -10258,11 +10211,14 @@ def _npm_lockfile_changed(hermes_root: Path) -> bool:
# node_modules means the cache was recorded by another checkout.
if not (PROJECT_ROOT / "node_modules").is_dir():
return True
# A matching lockfile hash with a half-installed tree (e.g. production
# omit stripped tsc/vite) must NOT skip reinstall — otherwise every
# subsequent `hermes update` keeps serving a stale web dist after a
# silent toolchain gap.
if not _web_build_toolchain_ready(PROJECT_ROOT):
# A matching lockfile hash over a tree whose web build toolchain never
# landed must NOT skip the reinstall — otherwise every later `hermes
# update` keeps rebuilding against a half-installed tree and serving a
# stale dist.
web_dir = PROJECT_ROOT / "web"
if (web_dir / "package.json").is_file() and not _web_build_toolchain_ready(
*_web_toolchain_roots(web_dir)
):
return True
try:
# Key the cache by PROJECT_ROOT so parallel worktrees don't collide.
@ -10440,18 +10396,6 @@ def _update_node_dependencies() -> list[str]:
env=nixos_env,
)
if ws_result.returncode == 0:
if _web_build_toolchain_ready(PROJECT_ROOT):
_record_npm_lockfile_hash(shared_hermes_root)
print(" ✓ repo root + ui-tui, web workspaces (desktop skipped)")
return []
# Mocked unit tests call this with a successful npm exit code but no
# real node_modules tree — trust the exit code in that case. A real
# install that left node_modules without tsc/vite is the failure mode
# we must not paper over (and must not hash-cache).
if (PROJECT_ROOT / "node_modules").is_dir():
print(" ⚠ npm workspace install finished without tsc/vite shims")
print(" (devDependencies likely omitted — check NODE_ENV / npm omit)")
return _partial_update_failure("ui-tui, web workspaces")
_record_npm_lockfile_hash(shared_hermes_root)
print(" ✓ repo root + ui-tui, web workspaces (desktop skipped)")
return []

View file

@ -149,6 +149,41 @@ class TestCmdUpdateNpmLockfileCache:
)
assert hm._npm_lockfile_changed(tmp_path) is True
def test_missing_web_build_toolchain_defeats_skip(self, tmp_path, monkeypatch):
"""A hash recorded over a tree that never got tsc/vite must not skip.
Otherwise the half-installed tree is permanent: every later update
trusts the hash, the build keeps failing, and the stale dist is served
forever.
"""
from hermes_cli import main as hm
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
(tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}')
(tmp_path / "package.json").write_text('{"workspaces": ["web"]}')
(tmp_path / "web").mkdir()
(tmp_path / "web" / "package.json").write_text("{}")
bin_dir = tmp_path / "node_modules" / ".bin"
bin_dir.mkdir(parents=True)
hm._record_npm_lockfile_hash(tmp_path)
assert hm._npm_lockfile_changed(tmp_path) is True
(bin_dir / "tsc").touch()
(bin_dir / "vite").touch()
assert hm._npm_lockfile_changed(tmp_path) is False
def test_toolchain_check_skipped_without_a_web_package(self, tmp_path, monkeypatch):
"""Prebuilt bundles ship no web/ source — they must still skip."""
from hermes_cli import main as hm
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
(tmp_path / "package-lock.json").write_text('{"lockfileVersion": 3}')
(tmp_path / "node_modules").mkdir()
hm._record_npm_lockfile_hash(tmp_path)
assert hm._npm_lockfile_changed(tmp_path) is False
def test_workspace_package_json_edit_defeats_skip(self, tmp_path, monkeypatch):
"""The manifest list comes from the root package.json `workspaces`
globs (npm's source of truth), so ANY workspace (desktop included)

View file

@ -21,8 +21,11 @@ import pytest
from hermes_cli.main import (
_web_ui_build_needed,
_build_web_ui,
_run_npm_install_deterministic,
_compute_web_ui_content_hash,
_missing_web_build_tool,
_run_npm_install_deterministic,
_web_build_toolchain_ready,
_web_toolchain_roots,
_web_ui_stamp_path,
_write_web_ui_build_stamp,
)
@ -565,80 +568,79 @@ class TestBuildWebUIFlock:
assert ".web_ui_build.lock" in gitignore.read_text(encoding="utf-8")
class TestNpmInstallDevDepEnvForcing:
"""Config-level guards so production/omit-dev env can't strip tsc/vite."""
def _link_shims(bin_dir: Path, *names: str) -> None:
bin_dir.mkdir(parents=True, exist_ok=True)
for name in names:
(bin_dir / name).touch()
def test_forces_npm_config_include_dev_and_clears_omit(self, tmp_path):
class TestWebBuildToolchainReady:
"""A tree is ready when the build can resolve tsc AND vite from any root.
``npm run build`` searches ``node_modules/.bin`` from the script's own
package up through every ancestor, so a shim in either place counts.
"""
def test_missing_toolchain_is_not_ready(self, tmp_path):
web_dir, _ = _make_web_dir(tmp_path)
(web_dir / "package-lock.json").write_text("{}", encoding="utf-8")
mock_cp = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
with patch("hermes_cli.main.subprocess.run", return_value=mock_cp) as mock_run:
_run_npm_install_deterministic(
"/usr/bin/npm",
web_dir,
env={
"NODE_ENV": "production",
"npm_config_omit": "dev",
"npm_config_production": "true",
},
)
_, kwargs = mock_run.call_args
env = kwargs["env"]
assert env["npm_config_include"] == "dev"
assert env["npm_config_production"] == "false"
assert "npm_config_omit" not in env
assert env["CI"] == "1"
class TestWebToolchainReadyAndRepair:
def test_toolchain_ready_requires_tsc_and_vite_shims(self, tmp_path):
from hermes_cli.main import _web_build_toolchain_ready
assert _web_build_toolchain_ready(web_dir, tmp_path) is False
def test_partial_toolchain_is_not_ready(self, tmp_path):
web_dir, _ = _make_web_dir(tmp_path)
root = web_dir.parent
assert _web_build_toolchain_ready(root) is False
bin_dir = root / "node_modules" / ".bin"
bin_dir.mkdir(parents=True)
(bin_dir / "tsc").touch()
assert _web_build_toolchain_ready(root) is False
(bin_dir / "vite").touch()
assert _web_build_toolchain_ready(root) is True
def test_build_repairs_when_node_modules_exists_without_tsc(self, tmp_path):
"""Partial install (node_modules present, no tsc) triggers a repair install."""
from hermes_cli.main import _resolve_node_runtime_npm
_link_shims(tmp_path / "node_modules" / ".bin", "tsc")
assert _web_build_toolchain_ready(web_dir, tmp_path) is False
def test_hoisted_shims_at_workspace_root_are_ready(self, tmp_path):
web_dir, _ = _make_web_dir(tmp_path)
_link_shims(tmp_path / "node_modules" / ".bin", "tsc", "vite")
assert _web_build_toolchain_ready(web_dir, tmp_path) is True
def test_shims_nested_under_the_package_are_ready(self, tmp_path):
"""#42973 layout: web/ owns its lockfile, so npm links shims there."""
web_dir, _ = _make_web_dir(tmp_path)
(tmp_path / "package-lock.json").write_text("{}", encoding="utf-8")
# Simulate a production-omit install: tree exists, toolchain missing.
(tmp_path / "node_modules").mkdir()
(tmp_path / "node_modules" / ".bin").mkdir()
_link_shims(web_dir / "node_modules" / ".bin", "tsc", "vite")
assert _web_build_toolchain_ready(web_dir, tmp_path) is True
install_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
build_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
install_calls = {"n": 0}
def test_shims_split_across_roots_are_ready(self, tmp_path):
web_dir, _ = _make_web_dir(tmp_path)
_link_shims(tmp_path / "node_modules" / ".bin", "tsc")
_link_shims(web_dir / "node_modules" / ".bin", "vite")
assert _web_build_toolchain_ready(web_dir, tmp_path) is True
def fake_install(*_a, **_k):
install_calls["n"] += 1
# After the repair install, pretend tsc/vite appeared.
if install_calls["n"] >= 2:
bin_dir = tmp_path / "node_modules" / ".bin"
(bin_dir / "tsc").touch()
(bin_dir / "vite").touch()
return install_ok
@pytest.mark.parametrize("shim", ["tsc.cmd", "tsc.ps1", "tsc.exe"])
def test_windows_shim_extensions_count(self, tmp_path, shim):
web_dir, _ = _make_web_dir(tmp_path)
_link_shims(tmp_path / "node_modules" / ".bin", shim, "vite.cmd")
assert _web_build_toolchain_ready(web_dir, tmp_path) is True
with patch("hermes_cli.main._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \
patch("hermes_cli.main._run_npm_install_deterministic", side_effect=fake_install) as mock_install, \
patch("hermes_cli.main._run_with_idle_timeout", return_value=build_ok), \
patch("hermes_cli.main._web_ui_build_needed", return_value=True), \
patch("hermes_cli.main._write_web_ui_build_stamp"):
result = _build_web_ui(web_dir)
assert result is True
# First silent install + one repair install.
assert mock_install.call_count == 2
class TestWebToolchainRoots:
def test_searches_the_package_and_its_workspace_root(self, tmp_path):
web_dir, _ = _make_web_dir(tmp_path)
assert _web_toolchain_roots(web_dir) == (web_dir, tmp_path)
def test_build_retries_install_on_tsc_not_found(self, tmp_path):
class TestMissingWebBuildTool:
"""Every shell words an unresolvable binary differently."""
@pytest.mark.parametrize(
"output,expected",
[
("sh: 1: tsc: not found\nnpm error code 127", "tsc"),
("bash: line 1: vite: command not found", "vite"),
("'tsc' is not recognized as an internal or external command", "tsc"),
("error TS2307: Cannot find module './x'", None),
("", None),
],
)
def test_detects_the_unresolvable_tool(self, output, expected):
assert _missing_web_build_tool(output) == expected
class TestBuildRecoversFromMissingToolchain:
def test_reinstalls_and_retries_when_the_build_cannot_resolve_tsc(self, tmp_path):
"""The generic retry reruns the same command, so it can't fix this alone."""
web_dir, _ = _make_web_dir(tmp_path)
(tmp_path / "package-lock.json").write_text("{}", encoding="utf-8")
install_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
@ -656,6 +658,44 @@ class TestWebToolchainReadyAndRepair:
result = _build_web_ui(web_dir)
assert result is True
# Initial install + repair install after tsc: not found.
assert mock_install.call_count == 2
assert mock_build.call_count == 2
def test_healthy_tree_builds_without_an_extra_install(self, tmp_path):
"""No pre-build probing: a build that works is never second-guessed."""
web_dir, _ = _make_web_dir(tmp_path)
(tmp_path / "package-lock.json").write_text("{}", encoding="utf-8")
_link_shims(web_dir / "node_modules" / ".bin", "tsc", "vite")
install_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
build_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
with patch("hermes_cli.main._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \
patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok) as mock_install, \
patch("hermes_cli.main._run_with_idle_timeout", return_value=build_ok) as mock_build, \
patch("hermes_cli.main._web_ui_build_needed", return_value=True), \
patch("hermes_cli.main._write_web_ui_build_stamp"):
result = _build_web_ui(web_dir)
assert result is True
assert mock_install.call_count == 1
assert mock_build.call_count == 1
def test_unrelated_build_failure_takes_the_generic_retry_only(self, tmp_path):
web_dir, _ = _make_web_dir(tmp_path)
(tmp_path / "package-lock.json").write_text("{}", encoding="utf-8")
install_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
type_error = __import__("subprocess").CompletedProcess(
[], 2, stdout="src/app.tsx(3,1): error TS2307: Cannot find module\n", stderr=""
)
build_ok = __import__("subprocess").CompletedProcess([], 0, stdout="", stderr="")
with patch("hermes_cli.main._resolve_node_runtime_npm", return_value="/usr/bin/npm"), \
patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok) as mock_install, \
patch("hermes_cli.main._run_with_idle_timeout", side_effect=[type_error, build_ok]), \
patch("hermes_cli.main._web_ui_build_needed", return_value=True), \
patch("hermes_cli.main._write_web_ui_build_stamp"), \
patch("hermes_cli.main._time.sleep"):
result = _build_web_ui(web_dir)
assert result is True
assert mock_install.call_count == 1