seb/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(node/nix): consolidate workspace lockfile + update all consumers

Browse Source 
Consolidate per-package package-lock.json files into a single root-level
workspace lockfile.  Update all consumers:

- Nix: shared src/npmDeps/npmDepsHash in lib.nix; devshell hook stamps
  package.json paths then runs npm ci from root; individual .nix files
  use mkNpmPassthru attrs instead of per-package fetchNpmDeps.
- Python CLI: new _workspace_root() helper so _tui_need_npm_install,
  _make_tui_argv, _build_web_ui resolve lockfile/node_modules from the
  workspace root.
- Desktop: replace --force-build/mtime heuristic with content-hash build
  stamp (_compute_desktop_content_hash via pathspec).  Remove --force-build
  flag.
- Dockerfile: single root npm install; no per-directory lockfile copies.
- CI: nix-lockfile-fix and osv-scanner reference root package-lock.json;
  apps/dashboard → apps/desktop.
- Tests: new test_tui_npm_install.py; desktop stamp tests in
  test_gui_command.py; updated assertions in test_cmd_update.py,
  test_web_ui_build.py, test_dockerfile_pid1_reaping.py.
- Docs: remove --force-build from desktop flag table.

Deleted: apps/desktop/package-lock.json, ui-tui/package-lock.json,
ui-tui/packages/hermes-ink/package-lock.json, web/package-lock.json.
This commit is contained in:
ethernet
2026-06-02 20:28:18 -04:00
parent 01eaba7061
commit a51a7b9b92
21 changed files with 2765 additions and 37028 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/hermes_cli/test_cmd_update.py
+42 -26
View File
@@ -198,36 +198,50 @@ class TestCmdUpdateBranchFallback:
if call.args and call.args[0][0] == "/usr/bin/npm"
]
# cmd_update runs npm commands in four locations:
# 1. repo root — slash-command / TUI bridge deps (subprocess.run)
# 2. ui-tui/ — Ink TUI deps (subprocess.run)
# 3. web/ — npm install (subprocess.run)
# 4. web/ — npm run build (_run_with_idle_timeout)
# cmd_update runs npm commands in these locations:
# 1. repo root — root-only install (--workspaces=false)
# 2. repo root — workspace install (--workspace ui-tui --workspace web)
# 3. web/ — npm ci --silent (if lockfile not at root)
# via _build_web_ui (subprocess.run)
# 4. web/ — npm run build (_run_with_idle_timeout)
#
# Repo-root and ui-tui installs intentionally omit `--silent` and run
# without `capture_output` so optional postinstall scripts (e.g.
# With a single workspace lockfile at the repo root, the root
# install covers all workspaces. The web/ ci call runs from the
# workspace root too (parent of web_dir) when the root lockfile
# exists.
#
# The root install omits `--silent` and runs without
# `capture_output` so optional postinstall scripts (e.g.
# `@askjo/camofox-browser`'s browser-binary fetch) print progress —
# otherwise long downloads look like a hang (#18840). The web/ install
# keeps `--silent` because its build step is short and noisy.
update_flags = [
# otherwise long downloads look like a hang (#18840).
root_flags = [
"/usr/bin/npm",
"ci",
"--no-fund",
"--no-audit",
"--progress=false",
"--workspaces=false",
]
ws_flags = [
"/usr/bin/npm",
"ci",
"--no-fund",
"--no-audit",
"--progress=false",
"--workspace",
"ui-tui",
"--workspace",
"web",
]
# Repo root additionally passes --workspaces=false so npm does not
# recursively install every apps/* workspace (desktop, shared).
repo_flags = [*update_flags, "--workspaces=false"]
assert npm_calls[:2] == [
(repo_flags, PROJECT_ROOT),
(update_flags, PROJECT_ROOT / "ui-tui"),
(root_flags, PROJECT_ROOT),
(ws_flags, PROJECT_ROOT),
]
if len(npm_calls) > 2:
# Only the web/ install is left in subprocess.run; the build moved
# to _run_with_idle_timeout to make Vite progress visible (#33788).
# The web/ install runs from the workspace root when the root
# lockfile exists (npm workspaces hoist node_modules upward).
assert npm_calls[2:] == [
(["/usr/bin/npm", "ci", "--silent"], PROJECT_ROOT / "web"),
(["/usr/bin/npm", "ci", "--silent"], PROJECT_ROOT),
]
# The web UI build itself went through the streaming helper.
@@ -236,21 +250,23 @@ class TestCmdUpdateBranchFallback:
assert idle_args[0] == ["/usr/bin/npm", "run", "build"]
assert idle_kwargs["cwd"] == PROJECT_ROOT / "web"
# Regression for #18840: repo root + ui-tui installs must stream
# output (capture_output=False) so postinstall progress is visible
# to the user.
repo_and_tui_calls = [
# Regression for #18840: root npm installs must stream output
# (capture_output=False) so postinstall progress is visible
# to the user. The _build_web_ui install uses --silent and
# capture_output=True, so exclude it.
root_install_calls = [
call
for call in mock_run.call_args_list
if call.args
and call.args[0][0] == "/usr/bin/npm"
and call.args[0][1] == "ci"
and call.kwargs.get("cwd") in {PROJECT_ROOT, PROJECT_ROOT / "ui-tui"}
and call.kwargs.get("cwd") == PROJECT_ROOT
and "--silent" not in call.args[0]
]
assert len(repo_and_tui_calls) == 2
for call in repo_and_tui_calls:
assert len(root_install_calls) == 2 # root-only + workspace install
for call in root_install_calls:
assert call.kwargs.get("capture_output") is False, (
"repo-root / ui-tui npm install must stream output "
"repo-root npm install must stream output "
"(no capture_output) so postinstall progress is visible"
)
tests/hermes_cli/test_tui_npm_install.py
+106
View File
@@ -193,3 +193,109 @@ def test_make_tui_argv_keeps_desktop_always_build_behaviour(
assert calls
assert calls[0][0][0] == ["/bin/npm", "run", "build"]
# ── _workspace_root helper ──────────────────────────────────────────
def test_workspace_root_returns_parent_when_subpackage(tmp_path: Path, main_mod) -> None:
"""Sub-package has package.json, no lockfile; parent has lockfile → parent."""
sub = tmp_path / "ui-tui"
sub.mkdir()
(sub / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
assert main_mod._workspace_root(sub) == tmp_path
def test_workspace_root_returns_dir_when_standalone(tmp_path: Path, main_mod) -> None:
"""No package.json → not a sub-package, return dir itself."""
assert main_mod._workspace_root(tmp_path) == tmp_path
def test_workspace_root_returns_dir_when_own_lockfile(tmp_path: Path, main_mod) -> None:
"""Has package.json AND its own lockfile → standalone, return dir."""
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
(tmp_path.parent / "package-lock.json").write_text("{}")
assert main_mod._workspace_root(tmp_path) == tmp_path
def test_workspace_root_returns_dir_when_no_parent_lockfile(
tmp_path: Path, main_mod
) -> None:
"""Has package.json, no own lockfile, but parent also has no lockfile → standalone."""
sub = tmp_path / "ui-tui"
sub.mkdir()
(sub / "package.json").write_text("{}")
# tmp_path has no package-lock.json either
assert main_mod._workspace_root(sub) == sub
def test_workspace_root_consistent_with_need_npm_install(
tmp_path: Path, main_mod
) -> None:
"""Divergence regression: if someone creates ui-tui/package-lock.json
by accident, _workspace_root (used by both _tui_need_npm_install AND
the npm install cwd) returns ui-tui/ for both, so they never disagree.
Before the shared helper, _tui_need_npm_install used a 3-condition
check (falling back to ui-tui/ when its own lockfile exists) while
the npm install cwd used a simpler check (still going to the parent
because the parent lockfile still exists). The shared helper
eliminates the split.
"""
sub = tmp_path / "ui-tui"
sub.mkdir()
(sub / "package.json").write_text("{}")
# Both sub and parent have lockfiles — accidental state
(sub / "package-lock.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
ws = main_mod._workspace_root(sub)
# _workspace_root sees sub has its own lockfile → treats it as standalone
assert ws == sub
# _tui_need_npm_install also uses _workspace_root, so both agree
assert main_mod._tui_need_npm_install.__code__.co_names
# (Smoke test: just confirm _tui_need_npm_install doesn't crash)
# It won't need install because the lockfile exists and there's no
# hidden lockfile to compare against, and ink is missing → True.
# But the key invariant is: ws_root for the need-check == ws_root
# for the install cwd — both use _workspace_root(sub).
def test_no_stray_lockfiles_in_workspace_subdirs(main_mod) -> None:
"""Workspace sub-directories must not contain their own package-lock.json.
With a single workspace root lockfile, per-directory lockfiles are
always accidental (typically from running ``npm install`` inside the
wrong directory). They cause ``_workspace_root`` to treat the
sub-package as standalone, which breaks hoisted ``node_modules``
resolution and can silently diverge the install cwd from the
lockfile-check root.
This is an invariant, not a change-detector: the workspace structure
is not expected to gain per-dir lockfiles.
"""
root = main_mod.PROJECT_ROOT
# Workspace members that live one level below the root and should
# NOT have their own lockfile. (ui-tui/packages/* members are
# two levels deep and even less likely to get accidental lockfiles,
# but we check them too for completeness.)
subdirs = [
root / "ui-tui",
root / "web",
root / "apps" / "desktop",
root / "apps" / "shared",
]
# Also sweep ui-tui/packages/* (hermes-ink etc.)
tui_pkgs = root / "ui-tui" / "packages"
if tui_pkgs.is_dir():
subdirs.extend(d for d in tui_pkgs.iterdir() if d.is_dir())
stray = [d for d in subdirs if (d / "package-lock.json").is_file()]
assert not stray, (
"stray package-lock.json found in workspace sub-directory(es); "
"delete them and run `npm install` from the repo root instead: "
+ ", ".join(str(d / "package-lock.json") for d in stray)
)
tests/hermes_cli/test_web_ui_build.py
+3 -1
View File
@@ -69,7 +69,9 @@ class TestWebUIBuildNeeded:
def test_returns_true_when_package_lock_newer_than_dist(self, tmp_path):
web_dir, dist_dir = _make_web_dir(tmp_path)
_touch(dist_dir / ".vite" / "manifest.json", offset=-10)
_touch(web_dir / "package-lock.json")
# With a single workspace root lockfile, the lockfile lives at the
# project root (tmp_path), not inside web_dir.
_touch(tmp_path / "package-lock.json")
assert _web_ui_build_needed(web_dir) is True
def test_returns_true_when_vite_config_newer_than_dist(self, tmp_path):