From bbd91bfa231735510ef8e497a77e6f1915f24faa Mon Sep 17 00:00:00 2001 From: dev-lead Date: Fri, 8 May 2026 13:44:51 -0700 Subject: [PATCH] test(dockerfile): align tui-resolution assertions with post-a49f4c6 design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two contract tests in test_dockerfile_pid1_reaping.py guarded properties of the @hermes/ink materialization dance introduced in 5f215b13 (PR #16690): ``--prefix node_modules/@hermes/ink`` install + ``omit=dev`` + nested-react cleanup + ``await import('@hermes/ink')`` smoke check. That mechanism was retired in a49f4c6 ("fix: prevent tui rebuilding assets") in favour of a simpler approach: 1. Copy the full ``ui-tui/packages/hermes-ink/`` tree (not just manifests) so npm can resolve the ``file:`` workspace dep against real content rather than a bare package.json. 2. Set ``ENV npm_config_install_links=false`` to force npm to install ``file:`` deps as symlinks on Debian's bundled npm 9.x (which defaults to ``install-links=true`` and installs as copies). The host-side lockfile is generated by npm 10+ using symlinks, so install-as-copy produces a hidden node_modules/.package-lock.json that permanently disagrees with the root lock on @hermes/ink — and that disagreement trips the TUI launcher's ``_tui_need_npm_install()`` check on every startup, triggering a runtime ``npm install`` that fails with EACCES. The tests were never updated for the new design; they remained pinned to the retired materialization step and the manifest-only COPY shape. This commit: - Updates ``test_dockerfile_installs_tui_dependencies`` to assert the full ``COPY ui-tui/packages/hermes-ink/ ui-tui/packages/hermes-ink/`` shape — catches a regression that reverts to manifest-only copies. - Replaces ``test_dockerfile_materializes_local_tui_ink_package`` with ``test_dockerfile_forces_npm_install_links_false_for_workspace_resolution``, which scans the parsed instruction list for an ENV directive (not a comment) setting ``npm_config_install_links=false``. Negative-tested: removing only the ENV line correctly fails the assertion even with the explanatory comment block above it left intact. PID-1 reaping tests (the file's primary purpose) are unmodified and continue to assert tini install + ENTRYPOINT routing. Partial close hermes-agent#9 — addresses 2 of the ~28 real failures surfaced after the disk-pressure fix; does not touch the other ~19+ unrelated test failures in that issue. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/tools/test_dockerfile_pid1_reaping.py | 50 ++++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/tests/tools/test_dockerfile_pid1_reaping.py b/tests/tools/test_dockerfile_pid1_reaping.py index 52532a78..a922171c 100644 --- a/tests/tools/test_dockerfile_pid1_reaping.py +++ b/tests/tools/test_dockerfile_pid1_reaping.py @@ -106,8 +106,18 @@ def test_dockerfile_entrypoint_routes_through_the_init(dockerfile_text): def test_dockerfile_installs_tui_dependencies(dockerfile_text): + """The Dockerfile must install ui-tui's npm dependencies during build, + and must copy the @hermes/ink workspace tree (not just its manifests) + so npm can resolve the ``file:`` workspace dep without falling back to + the bare manifest. See PR #16690 + a49f4c6 for the design. + """ assert "ui-tui/package.json" in dockerfile_text - assert "ui-tui/packages/hermes-ink/package-lock.json" in dockerfile_text + # ui-tui/packages/hermes-ink/ is referenced as a `file:` workspace dep + # from ui-tui/package.json. Copying the FULL tree (rather than just + # package.json + package-lock.json as in earlier revisions) is what lets + # npm resolve the workspace to real content. This assertion catches a + # regression that reverts to manifest-only copies. + assert "COPY ui-tui/packages/hermes-ink/ ui-tui/packages/hermes-ink/" in dockerfile_text assert any( "ui-tui" in step and "npm" in step and (" install" in step or " ci" in step) for step in _run_steps(dockerfile_text) @@ -121,17 +131,33 @@ def test_dockerfile_builds_tui_assets(dockerfile_text): ) -def test_dockerfile_materializes_local_tui_ink_package(dockerfile_text): - assert any( - "ui-tui" in step - and "node_modules/@hermes/ink" in step - and "packages/hermes-ink" in step - and "rm -rf packages/hermes-ink/node_modules" in step - and "npm install --omit=dev" in step - and "--prefix node_modules/@hermes/ink" in step - and "rm -rf node_modules/@hermes/ink/node_modules/react" in step - and "await import('@hermes/ink')" in step - for step in _run_steps(dockerfile_text) +def test_dockerfile_forces_npm_install_links_false_for_workspace_resolution(dockerfile_text): + """The Dockerfile must force npm to install ``file:`` deps as symlinks + rather than copies. + + Debian's bundled npm 9.x defaults to ``install-links=true`` (deps + installed as copies). The host-side ``ui-tui/package-lock.json`` is + generated by npm 10+ which uses symlinks, so an install-as-copy in the + image produces a hidden ``node_modules/.package-lock.json`` that + permanently disagrees with the root lockfile on the @hermes/ink entry. + That disagreement trips the TUI launcher's ``_tui_need_npm_install()`` + check on every startup and triggers a runtime ``npm install`` that + fails with EACCES (node_modules/ is root-owned from build time). + + This assertion replaces the older ``--prefix node_modules/@hermes/ink`` + materialization smoke test (PR #16690), which was retired in a49f4c6 + in favour of ``install-links=false`` because the materialization step + rebuilt TUI assets unnecessarily on every container start. + """ + instructions = _dockerfile_instructions(dockerfile_text) + has_env_directive = any( + instr.startswith("ENV ") and "npm_config_install_links=false" in instr + for instr in instructions + ) + assert has_env_directive, ( + "ENV npm_config_install_links=false missing — without it, Debian npm 9.x " + "installs `file:` deps as copies, breaking @hermes/ink workspace " + "resolution at runtime. See PR #16690 + a49f4c6." ) -- 2.45.2