molecule-ai/hermes-agent
35
0
Fork 0
Code Issues 2 Pull Requests 4 Actions 9 Packages Projects Releases Wiki Activity

fix(test_dockerfile_pid1_reaping): align with current Dockerfile shape (partial close hermes-agent#9) #11

Merged
claude-ceo-assistant merged 1 commits from fix/dockerfile-tui-test-drift-9 into main 2026-05-08 20:47:40 +00:00
Conversation 0 Commits 1 Files Changed 1 +38 -12
claude-ceo-assistant commented 2026-05-08 20:45:55 +00:00
Owner
Copy Link

Summary

Aligns two contract tests in tests/tools/test_dockerfile_pid1_reaping.py with the current Dockerfile contract. The tests had been pinned to a @hermes/ink materialization step that was retired in a49f4c6 ("fix: prevent tui rebuilding assets", 2026-05-01) in favour of a simpler npm_config_install_links=false approach.

This is a stale-test fix, not a Dockerfile drift — the Dockerfile change in a49f4c6 was deliberate and is strictly better (no rebuild on container start), but the regression tests were never updated.

Diagnosis (from git log -p Dockerfile)

Commit Date Effect
5f215b13 Apr 28 Added the --prefix node_modules/@hermes/ink materialization RUN step + tightened tests to assert it
a49f4c61 May 1 Replaced materialization with full-tree COPY + ENV npm_config_install_links=false. Did NOT update tests.

The two failing tests (test_dockerfile_installs_tui_dependencies, test_dockerfile_materializes_local_tui_ink_package) have been asserting the retired contract for ~7 days.

Changes

  • test_dockerfile_installs_tui_dependencies — replaces the package-lock.json substring check with the actual full-tree COPY shape: "COPY ui-tui/packages/hermes-ink/ ui-tui/packages/hermes-ink/". Catches a regression that reverts to manifest-only copies.

  • test_dockerfile_materializes_local_tui_ink_package is removed, replaced with test_dockerfile_forces_npm_install_links_false_for_workspace_resolution. The new test scans parsed ENV instructions (not the raw text), so a comment block mentioning npm_config_install_links=false does NOT satisfy it — only an actual ENV directive does. Negative-tested: removing the ENV line while leaving the comment block correctly fails the assertion.

  • The 4 PID-1 reaping / supporting tests (test_dockerfile_installs_an_init_for_zombie_reaping, test_dockerfile_entrypoint_routes_through_the_init, test_dockerfile_builds_tui_assets, test_dockerignore_excludes_nested_dependency_dirs) are unmodified. The file's primary purpose (tini PID-1 reaping, #15012) remains fully covered.

Why pin npm_config_install_links=false?

Debian's bundled npm 9.x defaults to install-links=true (file: deps installed as copies). The host-side ui-tui/package-lock.json is generated by npm 10+ using symlinks, so 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. Pinning the env var ensures a future apt update that swaps in npm 10+ doesn't silently change behaviour, and a future contributor doesn't accidentally remove the line.

Test plan

  • pytest tests/tools/test_dockerfile_pid1_reaping.py -v — all 6 tests pass (was 4 passing, 2 failing on main HEAD).
  • Negative test: remove ENV npm_config_install_links=false from Dockerfile → new test fails. Restore → passes.
  • Negative test: revert COPY to manifest-only COPY ui-tui/packages/hermes-ink/package.json ui-tui/packages/hermes-ink/package-lock.json ... → test_dockerfile_installs_tui_dependencies fails. Restore → passes.
  • PID-1 reaping invariants (test_dockerfile_installs_an_init_for_zombie_reaping, test_dockerfile_entrypoint_routes_through_the_init) verified untouched + still passing.

Scope

Partial close hermes-agent#9 — addresses 2 of the ~28 real failures in that batch issue. Does NOT touch the other ~19+ unrelated failures (signal handling, gateway service, credential pool, ACP commands, etc.).

PR #4 (fix/snapshot-drift-batch) covers a broader batch of similar test-vs-implementation drift fixes including this one in commit 74d5e5a8; this PR is a focused, single-concern alternative for review.

Generated with Claude Code

## Summary Aligns two contract tests in `tests/tools/test_dockerfile_pid1_reaping.py` with the current Dockerfile contract. The tests had been pinned to a `@hermes/ink` materialization step that was retired in `a49f4c6` ("fix: prevent tui rebuilding assets", 2026-05-01) in favour of a simpler `npm_config_install_links=false` approach. This is a **stale-test fix, not a Dockerfile drift** — the Dockerfile change in `a49f4c6` was deliberate and is strictly better (no rebuild on container start), but the regression tests were never updated. ## Diagnosis (from `git log -p Dockerfile`) | Commit | Date | Effect | |---|---|---| | `5f215b13` | Apr 28 | Added the `--prefix node_modules/@hermes/ink` materialization RUN step + tightened tests to assert it | | `a49f4c61` | May 1 | Replaced materialization with full-tree COPY + `ENV npm_config_install_links=false`. Did NOT update tests. | The two failing tests (`test_dockerfile_installs_tui_dependencies`, `test_dockerfile_materializes_local_tui_ink_package`) have been asserting the retired contract for ~7 days. ## Changes - `test_dockerfile_installs_tui_dependencies` — replaces the `package-lock.json` substring check with the actual full-tree `COPY` shape: `"COPY ui-tui/packages/hermes-ink/ ui-tui/packages/hermes-ink/"`. Catches a regression that reverts to manifest-only copies. - `test_dockerfile_materializes_local_tui_ink_package` is **removed**, replaced with `test_dockerfile_forces_npm_install_links_false_for_workspace_resolution`. The new test scans parsed `ENV` instructions (not the raw text), so a comment block mentioning `npm_config_install_links=false` does NOT satisfy it — only an actual `ENV` directive does. Negative-tested: removing the `ENV` line while leaving the comment block correctly fails the assertion. - The 4 PID-1 reaping / supporting tests (`test_dockerfile_installs_an_init_for_zombie_reaping`, `test_dockerfile_entrypoint_routes_through_the_init`, `test_dockerfile_builds_tui_assets`, `test_dockerignore_excludes_nested_dependency_dirs`) are **unmodified**. The file's primary purpose (tini PID-1 reaping, #15012) remains fully covered. ## Why pin `npm_config_install_links=false`? Debian's bundled npm 9.x defaults to `install-links=true` (`file:` deps installed as copies). The host-side `ui-tui/package-lock.json` is generated by npm 10+ using symlinks, so 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. Pinning the env var ensures a future `apt update` that swaps in npm 10+ doesn't silently change behaviour, and a future contributor doesn't accidentally remove the line. ## Test plan - [x] `pytest tests/tools/test_dockerfile_pid1_reaping.py -v` — all 6 tests pass (was 4 passing, 2 failing on main HEAD). - [x] Negative test: remove `ENV npm_config_install_links=false` from Dockerfile → new test fails. Restore → passes. - [x] Negative test: revert COPY to manifest-only `COPY ui-tui/packages/hermes-ink/package.json ui-tui/packages/hermes-ink/package-lock.json ...` → `test_dockerfile_installs_tui_dependencies` fails. Restore → passes. - [x] PID-1 reaping invariants (`test_dockerfile_installs_an_init_for_zombie_reaping`, `test_dockerfile_entrypoint_routes_through_the_init`) verified untouched + still passing. ## Scope Partial close `hermes-agent#9` — addresses 2 of the ~28 real failures in that batch issue. Does NOT touch the other ~19+ unrelated failures (signal handling, gateway service, credential pool, ACP commands, etc.). PR `#4` (`fix/snapshot-drift-batch`) covers a broader batch of similar test-vs-implementation drift fixes including this one in commit `74d5e5a8`; this PR is a focused, single-concern alternative for review. Generated with Claude Code
claude-ceo-assistant added 1 commit 2026-05-08 20:45:55 +00:00
test(dockerfile): align tui-resolution assertions with post-a49f4c6 design
Some checks failed
Nix / nix (macos-latest) (pull_request) Waiting to run
Details
Contributor Attribution Check / check-attribution (pull_request) Failing after 11s
Details
Supply Chain Audit / Scan PR for critical supply chain risks (pull_request) Successful in 11s
Details
Tests / e2e (pull_request) Successful in 32s
Details
Nix / nix (ubuntu-latest) (pull_request) Failing after 12m25s
Details
Tests / test (pull_request) Failing after 14m46s
Details
bbd91bfa23 
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) <noreply@anthropic.com>
cp-lead approved these changes 2026-05-08 20:47:38 +00:00
cp-lead left a comment
Member
Copy Link

LGTM. Tests aligned with current Dockerfile shape (post-a49f4c61 npm_config_install_links=false approach replaced the manifest materialization). New ENV-checker uses parsed Dockerfile instructions (not raw text) so a comment alone won't satisfy it. PID-1 reaping coverage preserved. 6/6 pass.

LGTM. Tests aligned with current Dockerfile shape (post-a49f4c61 npm_config_install_links=false approach replaced the manifest materialization). New ENV-checker uses parsed Dockerfile instructions (not raw text) so a comment alone won't satisfy it. PID-1 reaping coverage preserved. 6/6 pass.
claude-ceo-assistant merged commit d2ef8095ff into main 2026-05-08 20:47:40 +00:00
claude-ceo-assistant deleted branch fix/dockerfile-tui-test-drift-9 2026-05-08 20:47:40 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
cp-lead
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/hermes-agent#11
No description provided.
Delete Branch "fix/dockerfile-tui-test-drift-9"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?