Skip to content

fix(hermes): preinstall WhatsApp bridge deps#5257

Merged
cv merged 2 commits into
mainfrom
codex/fix-hermes-whatsapp-bridge-deps
Jun 12, 2026
Merged

fix(hermes): preinstall WhatsApp bridge deps#5257
cv merged 2 commits into
mainfrom
codex/fix-hermes-whatsapp-bridge-deps

Conversation

@cv

@cv cv commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Preinstall Hermes' WhatsApp bridge Node dependencies in the sandbox base image so hermes whatsapp no longer tries to create node_modules under read-only /opt/hermes at runtime. The install remains deterministic by using the bridge lockfile and skipping future lockfile-less tarballs instead of resolving dependencies during image build.

Related Issue

Fixes #4764

Changes

  • Bake scripts/whatsapp-bridge dependencies with npm ci --prefix scripts/whatsapp-bridge during the Hermes base image build when the upstream lockfile is present.
  • Skip the optional bridge project when it is absent or lacks a lockfile, avoiding nondeterministic npm install fallback behavior.
  • Extend test/hermes-share-mount-deps.test.ts to cover bridge lockfile install, missing-lockfile skip, and absent-project skip cases.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Additional validation run:

  • npx biome check test/hermes-share-mount-deps.test.ts passes
  • npx vitest run test/hermes-share-mount-deps.test.ts passes
  • git diff --check passes
  • npx prek run hadolint --files agents/hermes/Dockerfile.base passes
  • SKIP=test-cli npx prek run --files agents/hermes/Dockerfile.base test/hermes-share-mount-deps.test.ts passes
  • Commit and pre-push hooks passed after adding a temporary python -> python3 shim for this host; without the shim, the unrelated e2e workflow-boundary test fails because this environment has python3 but no python.

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Chores

    • Improved Docker image build to ensure required adapter dependencies are installed during image creation, preventing runtime install failures during pairing flows.
  • Tests

    • Added tests covering scenarios for baked vs. skipped adapter dependency installation during image builds, including presence/absence of lockfiles and fixtures.

@cv cv self-assigned this Jun 12, 2026
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ea82bd92-7f8a-45f7-a9c6-61b6e978c6cd

📥 Commits

Reviewing files that changed from the base of the PR and between fad3e3b and 9254ac1.

📒 Files selected for processing (1)
  • test/hermes-share-mount-deps.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/hermes-share-mount-deps.test.ts

📝 Walkthrough

Walkthrough

Hermes Docker image build logic now pre-installs WhatsApp bridge Node dependencies to prevent runtime EACCES permission failures. A conditional npm ci executes when package-lock.json exists in scripts/whatsapp-bridge; otherwise the install is skipped with an informational message. Test helper and three new test cases validate this behavior.

Changes

Pre-bake WhatsApp bridge dependencies into Hermes image

Layer / File(s) Summary
Dockerfile WhatsApp bridge pre-install
agents/hermes/Dockerfile.base
Adds comment explaining why WhatsApp bridge dependencies must be baked into the image to avoid runtime permission failures, then extends the Hermes build step to conditionally run npm ci --prefix scripts/whatsapp-bridge when package-lock.json exists, otherwise logs and skips based on whether package.json is present.
Test helper and WhatsApp bridge install behavior
test/hermes-share-mount-deps.test.ts
Reorders imports, refactors runHermesInstallLayer helper to accept opts parameter for conditional WhatsApp bridge fixture creation including package.json and optional package-lock.json, updates the rm shell override to delegate to real rm, and adds three test cases asserting install behavior when lockfile is present, only package.json exists, and when the bridge directory is absent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A Docker bridge now stands so tall,
Pre-baking deps before the call,
No more EACCES fears at night,
WhatsApp pairing shines so bright,
Tests hop in to make it right. 📱✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix(hermes): preinstall WhatsApp bridge deps' clearly and concisely summarizes the main change: preinstalling WhatsApp bridge dependencies into the Hermes base image.
Linked Issues check ✅ Passed The PR successfully addresses issue #4764 by baking WhatsApp bridge dependencies into the Docker base image, preventing npm install failures at runtime due to permission errors on read-only /opt/hermes.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the WhatsApp bridge dependency installation issue: Dockerfile updates for conditional npm ci, test extensions validating the new behavior, and no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-hermes-whatsapp-bridge-deps

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: hermes-root-entrypoint-smoke-e2e, hermes-e2e-vitest
Optional E2E: whatsapp-qr-compact-e2e, hermes-secret-boundary-e2e, rebuild-hermes-e2e

Auto-dispatched E2E: hermes-root-entrypoint-smoke-e2e via nightly-e2e.yaml at 9254ac19bfe92ac09acc3fe66c85bb48ecffcc59nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E

  • hermes-root-entrypoint-smoke-e2e (high): Builds the real Hermes base/runtime image from agents/hermes/Dockerfile.base and exercises root-entrypoint startup. This is the highest-signal existing E2E for catching Dockerfile syntax, pinned dependency install, ownership, and startup regressions introduced by the new whatsapp-bridge npm ci layer.
  • hermes-e2e-vitest (high): Exercises the real Hermes install/onboard/runtime boundary with health and live inference checks. The Dockerfile.base change affects the Hermes sandbox runtime image used by real assistant flows, so the general Hermes onboarding/runtime E2E should pass before merge.

Optional E2E

  • whatsapp-qr-compact-e2e (medium): Adjacent WhatsApp pairing coverage. It validates the real WhatsApp QR renderer/preload path, but it does not currently build the Hermes image or verify whatsapp-bridge node_modules under /opt/hermes, so it is useful but not sufficient as the merge-blocking check for this PR.
  • hermes-secret-boundary-e2e (high): Builds Hermes images and checks the sandbox secret boundary. Optional confidence that adding a new preinstalled Node dependency tree under /opt/hermes does not disturb Hermes image startup/config boundary assumptions.
  • rebuild-hermes-e2e (high): Optional lifecycle confidence for Hermes base-image changes: verifies rebuild applies a current Hermes base image while preserving state and inference credentials. Useful because this PR changes the Hermes base layer, but the functional change is not rebuild-specific.

New E2E recommendations

  • Hermes WhatsApp bridge pairing (high): No existing E2E appears to build the changed Hermes base image and then run the actual Hermes WhatsApp bridge/pairing path far enough to prove the sandbox user no longer attempts to create /opt/hermes/scripts/whatsapp-bridge/node_modules at runtime. Add a hermetic smoke that builds the Hermes image, runs as the sandbox user, asserts scripts/whatsapp-bridge/node_modules exists when the lockfile ships, and verifies hermes whatsapp or the bridge startup reaches the QR/pairing boundary without EACCES, without requiring a phone scan.
    • Suggested test: Add a Hermes WhatsApp bridge dependency/pairing smoke E2E covering /opt/hermes/scripts/whatsapp-bridge/node_modules and the no-runtime-write contract.
  • Hermes PR-local base-image resolution (medium): The reusable resolve-hermes-base-image action falls back to a local agents/hermes/Dockerfile.base build only when GHCR candidates are unavailable or incompatible. For PRs changing agents/hermes/Dockerfile.base, an existing published latest image may be used instead, so some image E2Es may not exercise the PR's base Dockerfile unless they build it directly.
    • Suggested test: Add CI/E2E coverage ensuring Hermes base-image resolution detects agents/hermes/Dockerfile.base changes relative to origin/main and builds the PR-local Hermes base image.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None. Advisor reported no Vitest E2E scenario impact.

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • agents/hermes/Dockerfile.base

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 1 needs attention, 2 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 2 still apply, 0 new items found

Review findings

🛠️ Needs attention

  • Runtime QR/EACCES behavior is still not validated (test/hermes-share-mount-deps.test.ts:141): The linked issue's user-visible acceptance is that `hermes whatsapp` in a rebuilt Hermes sandbox reaches the QR pairing flow instead of failing with `EACCES` while trying to create `/opt/hermes/scripts/whatsapp-bridge/node_modules` as the `sandbox` user. The added shell-fixture tests prove the Dockerfile branch runs `npm ci` and preserves bridge `node_modules`, but they do not execute the built image, confirm the actual pinned Hermes tarball contains the bridge lockfile, or run `hermes whatsapp` under runtime filesystem permissions.
    • Recommendation: Add or identify targeted runtime validation that builds or inspects the Hermes base image for the pinned Hermes release and runs `hermes whatsapp` as the `sandbox` user far enough to prove it does not attempt `npm install` or mkdir under `/opt/hermes/scripts/whatsapp-bridge/node_modules` before QR pairing.
    • Evidence: Issue [Brev][Agent&Skills] hermes whatsapp QR pairing fails with EACCES — npm install cannot write to /opt/hermes/scripts/whatsapp-bridge/node_modules #4764 expected result: "WhatsApp bridge dependencies install successfully and a QR code is displayed for phone pairing." The diff adds `npm ci --prefix scripts/whatsapp-bridge` in `agents/hermes/Dockerfile.base` and unit-style shell fixtures in `test/hermes-share-mount-deps.test.ts`, but no test runs the rebuilt sandbox image or the `hermes whatsapp` command path.

🔎 Worth checking

  • Source-of-truth review needed: Hermes WhatsApp bridge build-time preinstall and lockfile-less skip: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The source workaround is mostly explained and unit-tested, but the actual pinned tarball/image and runtime `hermes whatsapp` path remain unvalidated; this is covered by the runtime acceptance finding.
  • Review the newly baked WhatsApp bridge dependency tree (agents/hermes/Dockerfile.base:207): The base image now installs the upstream Hermes WhatsApp bridge Node project into production images when `scripts/whatsapp-bridge/package-lock.json` exists. The install remains deterministic because the Hermes tarball is checksum-pinned and `npm ci` uses the upstream lockfile, but the bridge dependency tree and any npm lifecycle scripts are inside the external Hermes release tarball rather than visible in this PR diff.
    • Recommendation: Before relying on this image, review or scan the pinned Hermes release's `scripts/whatsapp-bridge/package-lock.json` dependency tree for known vulnerabilities and license issues, and repeat that review whenever `HERMES_VERSION` or `HERMES_TARBALL_SHA256` changes.
    • Evidence: `agents/hermes/Dockerfile.base` adds `npm ci --prefix "${bridge_dir}" --prefer-offline --no-audit --no-fund` for `bridge_dir=scripts/whatsapp-bridge`; the tarball fetch remains protected by `${HERMES_TARBALL_SHA256}`.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — Build or inspect the Hermes base image for pinned `HERMES_VERSION=v2026.5.16` and assert `/opt/hermes/scripts/whatsapp-bridge/node_modules` exists when `scripts/whatsapp-bridge/package-lock.json` ships in the tarball.. The unit shell-fixture tests provide useful Dockerfile branch coverage, including the cleanup behavior that preserves bridge `node_modules`. The bug, however, occurs in a built Hermes sandbox as the `sandbox` user with `/opt/hermes` read-only, so runtime validation is needed for the linked acceptance path.
  • **Runtime validation** — Run `hermes whatsapp` as the `sandbox` user in a rebuilt Hermes sandbox and verify it reaches the QR pairing flow without attempting `npm install` or mkdir under `/opt/hermes/scripts/whatsapp-bridge/node_modules`.. The unit shell-fixture tests provide useful Dockerfile branch coverage, including the cleanup behavior that preserves bridge `node_modules`. The bug, however, occurs in a built Hermes sandbox as the `sandbox` user with `/opt/hermes` read-only, so runtime validation is needed for the linked acceptance path.
  • **Runtime validation** — Add an image/runtime check that `/opt/hermes` remains non-writable by `sandbox` while bridge dependencies are already present, proving the fix does not rely on weakening sandbox filesystem permissions.. The unit shell-fixture tests provide useful Dockerfile branch coverage, including the cleanup behavior that preserves bridge `node_modules`. The bug, however, occurs in a built Hermes sandbox as the `sandbox` user with `/opt/hermes` read-only, so runtime validation is needed for the linked acceptance path.
  • **Acceptance clause:** Running `hermes whatsapp` inside a Hermes Agent sandbox fails during the WhatsApp bridge dependency installation step. — add test evidence or identify existing coverage. The Dockerfile now preinstalls `scripts/whatsapp-bridge` dependencies at image build time when a lockfile exists, and the added fixture asserts the bridge `npm ci` branch is called. No test runs `hermes whatsapp` inside a built Hermes sandbox.
  • **Acceptance clause:** The command attempts `npm install` under `/opt/hermes/scripts/whatsapp-bridge/`, but that directory is owned by root and the sandbox user (`sandbox`) has no write permission. — add test evidence or identify existing coverage. The Dockerfile comment identifies this root-owned `/opt/hermes` write as the cause and moves dependency installation to image build time. The tests simulate build-time `node_modules` creation, but do not inspect runtime ownership or run as the `sandbox` user.
  • **Acceptance clause:** The QR code pairing screen is never reached. — add test evidence or identify existing coverage. The build-time dependency install should remove the EACCES blocker before QR pairing, but there is no runtime test that executes the QR pairing path.
  • **Acceptance clause:** 1. Install NemoClaw v0.0.58 with `NEMOCLAW_INSTALL_TAG=v0.0.58` — add test evidence or identify existing coverage. No installer-tag behavior is tested; this appears to be reproduction setup rather than code changed by this PR.
  • **Acceptance clause:** 2. Onboard with Hermes agent: `nemoclaw onboard --agent hermes --non-interactive --yes` — add test evidence or identify existing coverage. No onboarding path is exercised; onboarding code is unchanged and the fix depends on using a rebuilt Hermes base image.
Since last review details

Current findings:

  • Source-of-truth review needed: Hermes WhatsApp bridge build-time preinstall and lockfile-less skip: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The source workaround is mostly explained and unit-tested, but the actual pinned tarball/image and runtime `hermes whatsapp` path remain unvalidated; this is covered by the runtime acceptance finding.
  • Runtime QR/EACCES behavior is still not validated (test/hermes-share-mount-deps.test.ts:141): The linked issue's user-visible acceptance is that `hermes whatsapp` in a rebuilt Hermes sandbox reaches the QR pairing flow instead of failing with `EACCES` while trying to create `/opt/hermes/scripts/whatsapp-bridge/node_modules` as the `sandbox` user. The added shell-fixture tests prove the Dockerfile branch runs `npm ci` and preserves bridge `node_modules`, but they do not execute the built image, confirm the actual pinned Hermes tarball contains the bridge lockfile, or run `hermes whatsapp` under runtime filesystem permissions.
    • Recommendation: Add or identify targeted runtime validation that builds or inspects the Hermes base image for the pinned Hermes release and runs `hermes whatsapp` as the `sandbox` user far enough to prove it does not attempt `npm install` or mkdir under `/opt/hermes/scripts/whatsapp-bridge/node_modules` before QR pairing.
    • Evidence: Issue [Brev][Agent&Skills] hermes whatsapp QR pairing fails with EACCES — npm install cannot write to /opt/hermes/scripts/whatsapp-bridge/node_modules #4764 expected result: "WhatsApp bridge dependencies install successfully and a QR code is displayed for phone pairing." The diff adds `npm ci --prefix scripts/whatsapp-bridge` in `agents/hermes/Dockerfile.base` and unit-style shell fixtures in `test/hermes-share-mount-deps.test.ts`, but no test runs the rebuilt sandbox image or the `hermes whatsapp` command path.
  • Review the newly baked WhatsApp bridge dependency tree (agents/hermes/Dockerfile.base:207): The base image now installs the upstream Hermes WhatsApp bridge Node project into production images when `scripts/whatsapp-bridge/package-lock.json` exists. The install remains deterministic because the Hermes tarball is checksum-pinned and `npm ci` uses the upstream lockfile, but the bridge dependency tree and any npm lifecycle scripts are inside the external Hermes release tarball rather than visible in this PR diff.
    • Recommendation: Before relying on this image, review or scan the pinned Hermes release's `scripts/whatsapp-bridge/package-lock.json` dependency tree for known vulnerabilities and license issues, and repeat that review whenever `HERMES_VERSION` or `HERMES_TARBALL_SHA256` changes.
    • Evidence: `agents/hermes/Dockerfile.base` adds `npm ci --prefix "${bridge_dir}" --prefer-offline --no-audit --no-fund` for `bridge_dir=scripts/whatsapp-bridge`; the tarball fetch remains protected by `${HERMES_TARBALL_SHA256}`.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27386507980
Target ref: fad3e3be02a573cb81f790995222b6881fe16718
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27388571508
Target ref: 9254ac19bfe92ac09acc3fe66c85bb48ecffcc59
Workflow ref: main
Requested jobs: hermes-root-entrypoint-smoke-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-root-entrypoint-smoke-e2e ✅ success

@cv cv merged commit 7f8af81 into main Jun 12, 2026
42 checks passed
@cv cv deleted the codex/fix-hermes-whatsapp-bridge-deps branch June 12, 2026 01:34
@cv cv added the v0.0.64 Release target label Jun 12, 2026
cv pushed a commit that referenced this pull request Jun 12, 2026
## Summary
- Add v0.0.64 release notes from the release announcement and link them
to the relevant deeper docs.
- Document that custom policy presets recorded through `policy-add
--from-file` and `--from-dir` survive snapshot restore and sandbox
recreation.
- Refresh generated NemoClaw user skills from the current source docs.

## Source summary
- #5104 -> `docs/manage-sandboxes/backup-restore.mdx`,
`docs/network-policy/customize-network-policy.mdx`: Documents custom
policy presets preserved through snapshot restore.
- #4955 -> `docs/about/release-notes.mdx`: Adds release-note coverage
for Brave web-search pinning and `BRAVE_API_KEY` placeholder
preservation.
- #5116, #5269 -> `docs/about/release-notes.mdx`: Adds release-note
coverage for Docker-driver gateway health and rootfs guard stability.
- #5241, #5085 -> `docs/about/release-notes.mdx`: Adds release-note
coverage for chat-completions provider selection and Nemotron Ultra 550B
tool-less request compatibility.
- #5268, #5210, #5257 -> `docs/about/release-notes.mdx`: Adds
release-note coverage for messaging render plan refresh, OpenClaw
scope-upgrade approval recovery, and Hermes WhatsApp bridge dependency
setup.
- Current source docs -> `.agents/skills/`: Regenerates user-skill
references so agent-facing guidance matches the source documentation.

## Verification
- `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix
nemoclaw-user --doc-platform fern-mdx`
- `npm run docs`
- `npm run build:cli`
- `npm run typecheck:cli`
- Commit/pre-push hooks: markdownlint, gitleaks, docs-to-skills
verification, TypeScript CLI, and skills YAML checks passed.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Clarified sandbox snapshot restore preserves custom policy presets and
restores them without original files.
* Switched sandbox setup and remote deployment guidance to Docker-based
workflows and emphasized remote onboarding flow.
* Expanded troubleshooting for gateway recovery, Docker GPU/WSL issues,
and onboarding resume.
* Added/updated CLI docs: advanced maintenance, session export,
upload/download wrappers, and status recovery guidance.
* Added v0.0.64 release notes and links to NemoClaw Community; fixed
command reference formatting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.64 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Brev][Agent&Skills] hermes whatsapp QR pairing fails with EACCES — npm install cannot write to /opt/hermes/scripts/whatsapp-bridge/node_modules

3 participants