fix(hermes): align dashboard WhatsApp session path - #8229
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
sandl99
added
integration: whatsapp
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds compiled Hermes messaging preloads, manifest-driven runtime setup, WhatsApp session-path normalization, startup integration, permission checks, secret scans, and supporting tests. ChangesHermes messaging runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HermesStartup as Hermes startup
participant SandboxInit as sandbox-init.sh
participant MessagingManifest as messaging manifest
participant RuntimeEnv as trusted runtime environment
HermesStartup->>SandboxInit: Prepare messaging runtime
SandboxInit->>MessagingManifest: Load and validate runtime plan
SandboxInit->>RuntimeEnv: Apply aliases and install preloads
SandboxInit->>RuntimeEnv: Publish connect preload exports
SandboxInit-->>HermesStartup: Complete runtime preparation
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Signed-off-by: San Dang <sdang@nvidia.com>
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/hermes-whatsapp-session-path-patch.test.ts`:
- Line 85: Update the test title in the parametrize decorator to correct the
terminology from pairing source to gateway consumer, and add the issue-suffixed
reference (8184) at the end of the title following the coding guidelines for
local issue references in test titles. The test at line 85 should reflect that
the adapter is a gateway consumer while including the tracked defect identifier
in the final position of the test description.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 60047bb8-bcb9-4c4e-b411-7e7d494a3879
📒 Files selected for processing (4)
agents/hermes/Dockerfileagents/hermes/patch-whatsapp-session-path.mtstest/hermes-final-image-layout.test.tstest/hermes-whatsapp-session-path-patch.test.ts
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Exact-head CI failure is deterministic and branch-related on d7d2c17e4f9318deb33d388a1a9deb263f6dd8d7.
The Dockerfile adds seven build-only WhatsApp source/output integrity inputs, but MANAGED_STARTUP_PROFILE_EXCLUDED_DOCKER_INPUTS.hermes does not classify them. The repository contract in managed-startup-profile.test.ts therefore reports all seven as unowned Docker inputs. This is not a flaky shard failure and should not be rerun unchanged.
Please add these inputs to the Hermes build-only exclusion authority in src/lib/onboard/managed-startup/profile.ts with the appropriate integrity-pin reason, then rerun the focused startup-profile contract alongside the existing WhatsApp patch/image tests. Do not weaken the exhaustive Docker-input check.
The primary advisor lane also failed before producing a review, so fresh exact-head advisor evidence is still required after the fix.
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
test/hermes-messaging-runtime-setup.test.ts (2)
54-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe stub bypasses the behavior this test claims to prove.
The title states that manifest connect preloads reach the trusted runtime environment. Line 64 replaces
emit_messaging_connect_runtime_preload_exportswith a literal string, so the test proves only thatwrite_runtime_shell_envcalls the function. It does not prove that a connect preload recorded byinstall_messaging_runtime_preloadsbecomes a--requireentry inNODE_OPTIONS.Source the real
emit_messaging_connect_runtime_preload_exportsfromscripts/lib/sandbox-init.sh, point_MESSAGING_CONNECT_PRELOADS_FILEat a temporary list that contains a real file, then evaluate the emitted environment and assert the resultingNODE_OPTIONSvalue.Based on path instructions for
**/*.test.{ts,js,mts,mjs,cts,cjs}: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/hermes-messaging-runtime-setup.test.ts` around lines 54 - 78, Update the test around runtimeShellEnvFunction(HERMES_START) to source and invoke the real emit_messaging_connect_runtime_preload_exports implementation from scripts/lib/sandbox-init.sh instead of stubbing it. Configure _MESSAGING_CONNECT_PRELOADS_FILE with a temporary list containing a real preload file, evaluate the emitted environment, and assert NODE_OPTIONS contains the corresponding --require entry so the test verifies install-to-runtime propagation.Source: Path instructions
12-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the existing shell-function extraction helper.
test/hermes-start.test.tsalready providesextractShellFunctionFromSourcefor the same purpose. This localshellFunctionduplicates that logic and stops at the first\n}, so a future nested block or a column-0}inside a target function silently truncates the extracted body and the test still passes on a partial function.Move the existing helper into a shared test support module and import it here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/hermes-messaging-runtime-setup.test.ts` around lines 12 - 19, Replace the local shellFunction helper with the shared extractShellFunctionFromSource utility currently defined in hermes-start.test.ts. Move that helper into a shared test support module, update both tests to import it, and remove the duplicated implementation so extraction handles nested blocks consistently.agents/hermes/Dockerfile (1)
327-331: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard the preload flatten step against basename collisions and a silently empty copy.
The
cpflattens every*/runtime/*.jsinto one directory by basename. If two channels ship a runtime file with the same basename, the secondcpsilently overwrites the first, and a manifestsourcepath then resolves to another channel's code. Theif [ -d ... ]guard also lets the build succeed when the copy at Line 145 produces nothing; the failure then surfaces only at startup asMissing runtime preload source.Add a collision check and assert that at least one file was installed.
♻️ Proposed hardening for the flatten step
- && if [ -d /usr/local/lib/nemoclaw/preloads-compiled-channels ]; then \ - find /usr/local/lib/nemoclaw/preloads-compiled-channels -path '*/runtime/*.js' -type f \ - -exec sh -c 'for file do cp "$file" "/usr/local/lib/nemoclaw/preloads/$(basename "$file")"; done' sh {} +; \ - fi \ + && test -d /usr/local/lib/nemoclaw/preloads-compiled-channels \ + && find /usr/local/lib/nemoclaw/preloads-compiled-channels -path '*/runtime/*.js' -type f \ + -exec sh -c 'for file do \ + target="/usr/local/lib/nemoclaw/preloads/$(basename "$file")"; \ + if [ -e "$target" ]; then \ + echo "ERROR: duplicate messaging runtime preload basename: $target" >&2; \ + exit 1; \ + fi; \ + cp "$file" "$target"; \ + done' sh {} + \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/hermes/Dockerfile` around lines 327 - 331, Harden the preload flatten step in the Dockerfile before copying files: detect duplicate basenames among the runtime JavaScript files and fail the build instead of overwriting an existing preload. Also track the number of copied files and fail when no runtime preload was installed, while preserving cleanup of preloads-compiled-channels.scripts/lib/sandbox-init.sh (1)
1185-1206: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSensitive Data Exposure (CWE-1053)
Reachability: Internal
Reachability path
● Entry scripts/nemoclaw-start.sh:5010 write_runtime_shell_env │ ▼ ● Hop agents/hermes/start.sh:362 prepare_hermes_messaging_runtime │ ▼ ● Sink scripts/lib/sandbox-init.shAdd negative-path tests for
verify_messaging_runtime_secret_scans.Current tests stub this function. Cover a matching file, the configured exit code, a message that excludes the scanned content, and a missing file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/sandbox-init.sh` around lines 1185 - 1206, Add negative-path tests for verify_messaging_runtime_secret_scans that exercise a file matching the configured secretScans pattern, assert the function exits with the scan’s configured exitCode, verify stderr contains the path/message but not the scanned secret content, and confirm missing files are skipped without failure. Replace the existing stubbed-function coverage with these focused cases.Source: Path instructions
test/messaging-runtime-preload-packaging.test.ts (1)
38-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTest the packaged payload instead of Dockerfile text.
These assertions only match Dockerfile fragments. They do not prove that the final Hermes image contains usable compiled preloads.
Assert the final payload contains the expected runtime JavaScript file. Use the final-image test surface for this check.
As per path instructions, “Prefer observable outcomes through the public boundary over source-text ... assertions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/messaging-runtime-preload-packaging.test.ts` around lines 38 - 52, Update the Hermes packaging test around “packages the same manifest-owned runtime preloads for Hermes” to inspect the final image payload rather than matching Dockerfile source text. Use the existing final-image test surface to assert that the expected compiled runtime JavaScript preload is present and usable, removing or replacing the implementation-detail assertions.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lib/sandbox-init.sh`:
- Around line 1059-1064: Ensure apply_messaging_runtime_env_aliases returns
success when processing an alias with no message by preventing the conditional
printf command from determining the while-loop status. Keep exporting each alias
and printing non-empty messages unchanged, and make the function’s final status
successful for valid rows so the bare call in nemoclaw-start.sh does not abort
under set -e.
In `@src/lib/messaging/channels/whatsapp/runtime/whatsapp-hermes-session.ts`:
- Line 29: Preserve explicit session_path values in the WhatsApp Hermes session
argument handling by assigning HERMES_WHATSAPP_SESSION_PATH only when no path
was provided; update
src/lib/messaging/channels/whatsapp/runtime/whatsapp-hermes-session.ts lines
29-29 accordingly. Add a regression test in
src/lib/messaging/channels/whatsapp/runtime/whatsapp-hermes-session.test.ts
lines 13-61 that supplies a noncanonical explicit path and verifies it remains
unchanged.
---
Nitpick comments:
In `@agents/hermes/Dockerfile`:
- Around line 327-331: Harden the preload flatten step in the Dockerfile before
copying files: detect duplicate basenames among the runtime JavaScript files and
fail the build instead of overwriting an existing preload. Also track the number
of copied files and fail when no runtime preload was installed, while preserving
cleanup of preloads-compiled-channels.
In `@scripts/lib/sandbox-init.sh`:
- Around line 1185-1206: Add negative-path tests for
verify_messaging_runtime_secret_scans that exercise a file matching the
configured secretScans pattern, assert the function exits with the scan’s
configured exitCode, verify stderr contains the path/message but not the scanned
secret content, and confirm missing files are skipped without failure. Replace
the existing stubbed-function coverage with these focused cases.
In `@test/hermes-messaging-runtime-setup.test.ts`:
- Around line 54-78: Update the test around
runtimeShellEnvFunction(HERMES_START) to source and invoke the real
emit_messaging_connect_runtime_preload_exports implementation from
scripts/lib/sandbox-init.sh instead of stubbing it. Configure
_MESSAGING_CONNECT_PRELOADS_FILE with a temporary list containing a real preload
file, evaluate the emitted environment, and assert NODE_OPTIONS contains the
corresponding --require entry so the test verifies install-to-runtime
propagation.
- Around line 12-19: Replace the local shellFunction helper with the shared
extractShellFunctionFromSource utility currently defined in
hermes-start.test.ts. Move that helper into a shared test support module, update
both tests to import it, and remove the duplicated implementation so extraction
handles nested blocks consistently.
In `@test/messaging-runtime-preload-packaging.test.ts`:
- Around line 38-52: Update the Hermes packaging test around “packages the same
manifest-owned runtime preloads for Hermes” to inspect the final image payload
rather than matching Dockerfile source text. Use the existing final-image test
surface to assert that the expected compiled runtime JavaScript preload is
present and usable, removing or replacing the implementation-detail assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5bf7c77e-fb8b-49a1-8383-74aa2c993ae5
📒 Files selected for processing (21)
agents/hermes/Dockerfileagents/hermes/start.shscripts/lib/sandbox-init.shscripts/nemoclaw-start.shsrc/lib/messaging/channels/whatsapp/manifest.tssrc/lib/messaging/channels/whatsapp/runtime/whatsapp-hermes-session.test.tssrc/lib/messaging/channels/whatsapp/runtime/whatsapp-hermes-session.tssrc/lib/messaging/manifest/types.tstest/generate-hermes-config.test.tstest/hermes-final-image-layout.test.tstest/hermes-messaging-runtime-setup.test.tstest/hermes-start.test.tstest/http-proxy-fix-sync.test.tstest/messaging-runtime-preload-packaging.test.tstest/nemoclaw-start-guard-recovery.test.tstest/nemoclaw-start-runtime-env-alias.test.tstest/nemoclaw-start-slack-runtime.test.tstest/nemoclaw-start-telegram-runtime.test.tstest/nemoclaw-start.test.tstest/nemotron-inference-fix.test.tstest/support/entrypoint-script-fixture.ts
Signed-off-by: San Dang <sdang@nvidia.com>
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Re-reviewed exact head c3e80d3c3142a7ef2ec43d949650fbdffb684f6c using the manifest-first messaging contract. Moving the WhatsApp session shim into the channel manifest/runtime preload and keeping the shared initializer channel-neutral resolves the prior unowned integrity-input shape; that architecture is appropriate.
The replacement currently fails deterministically in the Hermes image build, however. tsconfig.runtime-preloads.json requires Node types, but the Hermes runtime-preload-builder compiles from /opt/nemoclaw-root while its node_modules lives at the sibling /opt/mcp-tool-discovery-runtime/node_modules. TypeScript therefore cannot resolve @types/node and reports missing process, require, Buffer, and NodeJS. The OpenClaw preload builder already establishes the required local node_modules symlink before invoking tsc; the Hermes stage does not.
Please give the Hermes compile root an explicit dependency-resolution path (the analogous builder-only symlink is the smallest current fix), and extend messaging-runtime-preload-packaging.test.ts to protect that Hermes resolution step. Then run the focused WhatsApp manifest/runtime/preload tests and obtain a green exact-head Hermes direct managed-startup build. Rerunning the unchanged head will reproduce the same compile failure.
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 2cc4096fcf26787beaa44d0426e79edc688f6dcd against the manifest-first messaging contract. The Hermes dependency-resolution fix exactly closes the deterministic build failure and is protected by the preload-packaging contract; the exact-head Hermes direct managed-startup and both image builds now pass. The broader follow-up keeps WhatsApp-specific behavior in the channel manifest/runtime asset while the shared sandbox initializer remains driven by serialized manifest data, so it does not reintroduce channel-specific core branching. The earlier integrity-input failure is removed with the obsolete standalone patcher rather than bypassed.
I found no remaining code blocker in this head. Remaining exact-head CI/E2E and the terminal advisor receipts remain visible and are not waived by this approval.
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/messaging-runtime-secret-scan.test.ts (1)
50-52: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClean up the temporary directory in a
finallyblock.
fs.rmSync(tmpDir, ...)runs only after setup andspawnSynccomplete normally. If any operation throws, the temporary directory remains. Wrap setup and execution intry/finallyto prevent test artifacts from accumulating.Suggested fix
- prepareConfig(configPath); - // existing plan and script writes - const result = spawnSync("bash", [scriptPath], { encoding: "utf-8", timeout: 5000 }); - fs.rmSync(tmpDir, { recursive: true, force: true }); - return { configPath, result }; + try { + prepareConfig(configPath); + // existing plan and script writes + const result = spawnSync("bash", [scriptPath], { encoding: "utf-8", timeout: 5000 }); + return { configPath, result }; + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/messaging-runtime-secret-scan.test.ts` around lines 50 - 52, Update the temporary-directory helper around spawnSync so all setup and script execution occur inside a try block, with fs.rmSync(tmpDir, { recursive: true, force: true }) in a finally block. Preserve returning configPath and result on successful execution while ensuring cleanup also occurs when setup or execution throws.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/messaging-runtime-secret-scan.test.ts`:
- Around line 50-52: Update the temporary-directory helper around spawnSync so
all setup and script execution occur inside a try block, with fs.rmSync(tmpDir,
{ recursive: true, force: true }) in a finally block. Preserve returning
configPath and result on successful execution while ensuring cleanup also occurs
when setup or execution throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a2c69368-84ea-4346-acc0-c813793fc45e
📒 Files selected for processing (1)
test/messaging-runtime-secret-scan.test.ts
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary - Keep the managed Hermes Dashboard inside NemoClaw's prepared, privilege-separated profile by passing upstream Hermes' `--isolated` flag. - Prevent Hermes from treating `profiles/dashboard-home` as a request to re-exec into its unified machine-level Dashboard. - Extend the existing Dashboard argument contract without changing ports, credentials, migration, permissions, or retry behavior. ## Affected main evidence - Run: https://github.com/NVIDIA/NemoClaw/actions/runs/31171807086 - Job: `sandbox-images-and-e2e / test-hermes-sandbox-image` (92846562212) - Candidate: `c31724f4e66e4d9abd0baab2ff68181830a4750a` - Scenario: `hermes root-entrypoint smoke preserves runtime layout and legacy state migration` - Signal: after the legacy profile is migrated and seeded, the Dashboard service owner exits before binding `127.0.0.1:19119`; the container exits 1. ## Root cause Hermes v2026.7.20 infers a named profile from a `HERMES_HOME` path under `profiles/<name>`. Without `--isolated`, `hermes dashboard` routes that launch to its unified machine-level Dashboard and leaves NemoClaw's prepared profile boundary. The legacy-state variant populates that profile before launch, exposing the route and causing the Dashboard owner to exit before the internal port binds. ## Ownership search - No open PR matches the exact bind-exit text, port `19119`, or `start_socat_forwarder` path. - No open PR edits `agents/hermes/start.sh` or `test/e2e/live/hermes-root-entrypoint-smoke.test.ts`. - #8229 concerns the Dashboard WhatsApp session path and does not touch this startup boundary. ## Validation - `npx vitest run --project integration test/hermes-start.test.ts test/hermes-dashboard-credential-launch.test.ts test/hermes-dashboard-profile-migration-security.test.ts` (47 passed) - `npm run build:cli` - `npm run validate:pr` - The three failing cases in `test/seed-hermes-dashboard-config.test.ts` reproduce unchanged on `main`; they are outside this patch and this root cause. ## Security review - `--isolated` keeps the Dashboard in the sandbox-owned profile that NemoClaw already prepares and permission-separates. - Loopback host/bind, gateway credential handoff, fail-closed migration, and filesystem permissions are unchanged. - No timeout, retry, test-coverage, or network-policy weakening; no E2E workflow was manually dispatched. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Hermes dashboard startup reliability by running the dashboard in isolated mode. * Prevented unintended dashboard re-execution in unified environments. * **Tests** * Added coverage confirming isolated dashboard behavior. * Preserved the opt-in behavior of the Hermes TUI flag. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
HERMES-22 live final-image evidenceRan the full live path on the final sandbox image built from this PR head. All four required items were captured. Details and the exact commands are below so this can be re-run. What was tested
The rebuild log line confirms the final image was layered on the pinned index: 1. Dashboard QR pairingPaired from the Dashboard UI, not the CLI. The Dashboard reported This is the contract this PR changes. Before the patch the Dashboard resolves through 2. Connected bridgeThe bridge process was absent before pairing and running after it. 3. Message pathAn inbound message from a second WhatsApp account — the number in 4. OpenShell proxy auditEnabled with No denials. The Baileys protocol-version fetch is admitted by the same preset, matching its documented scope. One thing to be aware of, outside this PRCompleting Dashboard pairing leaves the sandbox with a config hash that no longer matches its inputs: The Dashboard's apply step writes It did not block this run: the apply step uses Hermes' own restart path, so the bridge came up and stayed connected. But the sandbox is left in a state where the next guarded restart — This is not caused by this PR and I am not proposing to fix it here. Flagging it because it sits directly inside the flow HERMES-22 exercises, so anyone repeating this evidence run will hit it on the next restart. Reproduction# 1. build from this PR head, then onboard with WhatsApp enabled and no rebuild
WHATSAPP_ALLOWED_IDS=<sender-number> nemoclaw onboard --agent hermes --name <sandbox> \
--non-interactive --yes --yes-i-accept-third-party-software --no-gpu --fresh
# 2. enable the proxy audit before pairing
openshell settings set <sandbox> --key ocsf_json_enabled --value true
# 3. pair from the Dashboard, leaving the allowlist field blank so the rendered value is kept
# 4. capture
nemoclaw <sandbox> exec -- curl -s http://127.0.0.1:3000/health
nemoclaw <sandbox> exec -- sh -c 'ls /sandbox/.hermes/platforms/whatsapp/session | wc -l'
nemoclaw <sandbox> logs --tail 400 | grep -i whatsappEnabling WhatsApp during |
Maintainer evidence dispositionFor the published Hermes OCI index
Both published child configs report The current branch revision's HERMES-22 evidence is recorded here: Dashboard QR pairing, durable session placement, connected bridge, inbound message and reply, and OpenShell proxy audit all completed. No SBOM attestation is present. The index contains only the two platform manifests and their two SLSA provenance manifests; the trusted build enabled provenance but did not request an SBOM attestation. Whether that absence is acceptable for this migration or requires image republication is deferred to the security/release maintainer; I am not inferring that decision. The shard-3 failure was a load-sensitive timeout in unchanged |
Maintainer overlap dispositionPR #8229 changes the supported Hermes WhatsApp migration: it patches Dashboard pairing to write PR #8727 addresses #8718 by retaining These PRs have no declared supersession relationship, and #8229 does not implement #8718's expected status result for already-repaired v0.0.105 sandboxes. I am deferring both merge decisions until maintainers decide whether v0.0.106 must continue diagnosing the legacy configuration override or deliberately require migration to the canonical session path. I will not close either PR on the current evidence. |
Documentation writer review — blockedThe independent documentation review found three blocking issues:
Nonblocking: use lowercase “dashboard” when it is a technical noun rather than an exact UI label. The writer could not run |
The guide told an operator with credentials under the legacy dashboard profile to rebuild and pair again. That writes new credentials to the shared path but never removes the old ones: `profiles` and `dashboard-home` are both durable state that a rebuild restores, and startup treats the legacy copy as migration input, so the sandbox keeps two credential sets. Point the operator at `channels remove whatsapp`, which clears `platforms/whatsapp`, `profiles/dashboard-home/platforms/whatsapp/session`, and the legacy `dashboard-home/platforms/whatsapp/session` before the channel is added again, and qualify the durable-state claim for a sandbox that still holds both copies. Separate the completed manual HERMES-22 evidence from the check that is still outstanding: the live pairing, connection, reply, and proxy-audit results are recorded on the pull request, and no target in this repository pairs a live WhatsApp account, so the trusted manual E2E run remains the merge gate. Use lowercase `dashboard` for the technical noun, as the controlled word list requires, and extend the agent-variant documentation contract to cover the new cleanup command. Signed-off-by: Hung Le <hple@nvidia.com>
`channels status` told an operator with credentials only under the legacy dashboard profile to rebuild and pair again. A rebuild restores that profile rather than dropping it, so the sandbox ends up holding two credential sets and the split it reported is still there. Point the hint at `channels remove whatsapp`, which clears every WhatsApp session path, followed by `channels add whatsapp`. The command reference now describes the same procedure, so the report and the documentation give one answer. Signed-off-by: Hung Le <hple@nvidia.com>
|
@apurvvkumaria All three blocking findings are addressed in the two commits above. 1 — Legacy credentials survive rebuild and re-pair. Confirmed: The guide now points at
2 — HERMES-22 wording. The dependency review now separates the completed manual evidence — dashboard QR pairing, connected bridge, inbound reply, 3 — Receipt and validation text. The receipt is refreshed to the head, and the validation lines name the commit the runs happened at, Nonblocking: lowercase The hint edits |
<!-- markdownlint-disable MD041 --> ## Summary The WhatsApp channel manifest rendered `WHATSAPP_MODE=bot` as a constant, so neither Hermes mode was reachable and a freshly onboarded sandbox could not deliver a message. `channels add whatsapp` now asks the operator which senders the sandbox answers, and derives `WHATSAPP_DM_POLICY` from that answer and the allowlist. A blank answer keeps `self-chat`, so a paired sandbox works with nothing else configured. Answering `bot` without an allowlist renders `pairing`, so an unknown sender receives a pairing code the operator approves rather than being rejected outright. OpenClaw is unchanged. ## Related Issue Addresses part of #8312 ## Changes - `src/lib/messaging/channels/whatsapp/manifest.ts` — add a `mode` input (`envKey: WHATSAPP_MODE`, `statePath: whatsappConfig.mode`, `validValues: ["self-chat", "bot"]`, `defaultValue: "self-chat"`) with an operator prompt; wire the shared `common.configPrompt` enroll hook limited to `agents: ["hermes"]`; render `WHATSAPP_MODE={{whatsappConfig.mode}}` and `WHATSAPP_DM_POLICY={{whatsappConfig.dmPolicy}}` instead of the constant. - `src/lib/messaging/channels/whatsapp/template-resolver.ts` — resolve `whatsappConfig.mode`, following the `telegramConfig.groupPolicy` precedent, and add `whatsappConfig.dmPolicy`, which reads the mode as well as the allowlist. Remove the unreferenced `allowedIds.whatsapp.dmPolicy` case it replaces. - `src/lib/messaging/channels/whatsapp/rendered-config-parser.ts` — list the mode beside the allowlist so `channels status` shows both. The mode decides whether an empty allowlist matters, and the existing `WHATSAPP_ALLOWED_IDS: not set` line carried no such context. - `docs/manage-sandboxes/set-up-whatsapp.mdx` — rewrite the Hermes-only "Reply to Other Senders" section around the prompt, document approval and revocation, and qualify the existing allowlist sentence, which was written when `bot` was the only mode. - Tests: resolver cases for every mode and allowlist combination, prompt behavior against the real manifest hook, a compiler-level assertion that an OpenClaw plan never reaches the prompt, rendered `.env` assertions in `generate-hermes-config.test.ts`, and the live E2E probe now requires the derived policy. ### Why the DM policy is rendered rather than left unset The two halves of the Hermes WhatsApp adapter disagree on this key's default: the Node bridge reads `process.env.WHATSAPP_DM_POLICY || 'open'` while the Python adapter reads `os.getenv("WHATSAPP_DM_POLICY", "pairing")`. Leaving the key unset therefore left the bridge enforcing an empty allowlist and rejecting every sender, while `bot` mode also stops replies to the paired account's own self-chat — a sandbox nobody could reach. Rendering the value is what makes `bot` usable without an allowlist. This compensates for an upstream default divergence on the pinned runtime; a Hermes-side report is planned separately. The derivation is: | mode | allowlist | rendered `WHATSAPP_DM_POLICY` | |---|---|---| | `self-chat` | anything | line dropped — the bridge rejects non-self messages before it reads a policy | | `bot` | populated | `allowlist` | | `bot` | empty | `pairing` | | unusable stored value | anything | line dropped — the mode fallback decides, so a stale registry entry cannot open the gateway | ### Scope The prompt is limited to Hermes through the hook's `agents` list. `WHATSAPP_MODE` and `WHATSAPP_DM_POLICY` are rendered into the Hermes env, and the OpenClaw fragment carries no sender policy, so an OpenClaw operator would answer a question nothing consumes. WhatsApp and WeChat are the only two channels whose OpenClaw fragment omits `allowFrom` and `dmPolicy`, and `git log -S"allowFrom"` on this manifest is empty, so that omission is longstanding rather than lost here. Two behavior changes worth calling out: 1. **A sandbox already running `bot` renders `self-chat` after its next rebuild.** Its persisted plan has no `mode` input, so the manifest default applies. A migration rule keyed on "allowlist present implies bot" was considered and rejected: the documented way to select WhatsApp non-interactively is to export `WHATSAPP_ALLOWED_IDS`, so that rule would silently place new sandboxes into the mode this issue is about. Affected operators re-run `channels add whatsapp` and answer `bot`. 2. **Exporting `WHATSAPP_MODE` still selects the channel non-interactively for either agent**, including OpenClaw, where the mode is not rendered. `WHATSAPP_ALLOWED_IDS` already behaves this way; inputs cannot be scoped per agent today (`agents:` exists on hooks, not on `ChannelInputBaseSpec`). An operator who exports the variable also skips the prompt, which keeps the previously documented non-interactive path working. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: #8423 (comment) - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/manage-sandboxes/set-up-whatsapp.mdx` documents the Hermes modes, sender authorization, allowlist behavior, migration, recovery, and lifecycle verification. The strict documentation build passed with 0 errors and 2 existing warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: 62d3f37 --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/messaging/channels/whatsapp/template-resolver.test.ts src/lib/messaging/hooks/common/config-prompt.test.ts src/lib/messaging/compiler/manifest-compiler.test.ts` — 50 passed; `npx vitest run --project integration test/generate-hermes-config.test.ts` — 55 passed; `npx vitest run --project cli src/lib/onboard/messaging-channel-setup.test.ts src/lib/actions/sandbox/policy-channel-remove-flow.test.ts` — 47 passed. Adding an enroll hook to this manifest broke four assertions in those last two files, which were updated: one pinned the previous "onboarding asks nothing" contract, and three built a planner without a hook registry. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not applicable; `npm run validate:pr` passed after merging current main; this change is scoped to one channel manifest, the shared config-prompt hook's manifest wiring, and their tests. `npm run checks:repository` passed. `npx vitest run --project cli` after `npm run catalog:compile` reports 1158 of 1161 files passing; the three failures need Python 3.11 for `tomllib` and are unrelated. `npx vitest run --project integration` after `npm run build:cli` reports 720 of 734 files passing; the eight failures need Python 3.11 for `typing.NotRequired`, a locally absent `qrcode` package, or an external terminology tool, and none reference this change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without warnings (doc changes only) — `Found 0 errors and 2 warnings`. Fern does not enumerate the two warnings in the build output; they are not attributable to this page from the build alone. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ### Live verification Built from this branch on a CPU host (Hermes v0.19.0, OpenShell 0.0.99, docker driver) and exercised through the normal commands. Both modes were onboarded interactively so the prompt was answered rather than exported. The prompt, and the default answer: ``` self-chat replies only to messages the paired account sends to itself. bot replies to other senders and stops replying to that self-chat: an unknown sender receives a pairing code you approve with `hermes pairing approve whatsapp <code>`, unless you set WHATSAPP_ALLOWED_IDS to a fixed sender list before this command. WhatsApp reply mode [self-chat/bot; default: self-chat]: ✓ whatsapp WhatsApp reply mode saved ``` Pressing Enter, on sandbox `wa-self`: ``` $ nemoclaw wa-self exec -- grep -E '^WHATSAPP' /sandbox/.hermes/.env WHATSAPP_ENABLED=true WHATSAPP_MODE=self-chat ``` The policy line is absent, which is the intended result for `self-chat`. Hermes read the mode back as `Mode: personal number (self-chat)`, QR pairing completed, and after `gateway restart` the bridge ran with `--mode self-chat` and the agent replied to a message the paired account sent to itself. Answering `bot`, on sandbox `wa-bot`: ``` $ nemoclaw wa-bot exec -- grep -E '^WHATSAPP' /sandbox/.hermes/.env WHATSAPP_ENABLED=true WHATSAPP_MODE=bot WHATSAPP_DM_POLICY=pairing ``` Hermes read that mode back as `Mode: separate bot number`. On the same sandbox, the in-sandbox `hermes whatsapp` wizard was then used to write an allowlist, which appended `WHATSAPP_ALLOWED_USERS=*` outside the configuration NemoClaw seals; the next `gateway restart` refused to relaunch on the integrity guard. `rebuild --yes` restored the rendered configuration and dropped the hand-written line, leaving exactly the three lines above. That confirms the rendered value is authoritative over an out-of-band edit, and the documented warning about that wizard prompt comes from this run. An inbound message was not exchanged in `bot` mode on this host: the bridge did not spawn after that rebuild. The same code spawns the bridge in `self-chat` on the same branch, so the failure is downstream of the rendered value this change decides. Two known rebuild-time defects and the session-path behavior in #8229 are candidates; the diagnosis is separate from this PR. --- Signed-off-by: Hung Le <hple@nvidia.com> --------- Signed-off-by: Hung Le <hple@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: San Dang <sdang@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
7 participants
Summary
Hermes v2026.7.20 resolves dashboard WhatsApp pairing state relative to the dashboard profile while the gateway reads the main Hermes home. This change patches only the dashboard session helper so both use
/sandbox/.hermes/platforms/whatsapp/session; upstream Hermes CLI pairing and gateway code remain unchanged.Related Issue
Addresses the dashboard WhatsApp session-path mismatch in #8184. Upstream Baileys code 515, missing sync keys, connection reliability, and message-delivery defects remain outside this PR.
Changes
channels remove whatsapp, which clearsplatforms/whatsapp,profiles/dashboard-home/platforms/whatsapp/session, and the legacydashboard-home/platforms/whatsapp/sessionmigration source before the channel is added again.channels status. The Hermes legacy-session hint insrc/lib/messaging/channels/whatsapp/hooks/status-health-eval.tsnow nameschannels remove whatsappandchannels add whatsappinstead of a rebuild, anddocs/reference/commands.mdxdescribes that report.The direct source patch is required because the selected upstream Hermes release computes the dashboard path from its isolated home. The base-image patch guard and
test/hermes-whatsapp-dashboard-session-patch.test.tsprotect this compatibility contract.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/manage-sandboxes/set-up-whatsapp.mdxdocuments the shared session path, the supportedchannels remove whatsappcleanup that clears every WhatsApp session path, and the durable-state qualification for a sandbox that still holds legacy dashboard-profile credentials.docs/reference/commands.mdxdocuments the matchingchannels statusreport, whose hint names the same two commands.docs/security/hermes-0.19.0-dependency-review.mdrecords both child manifests, source and revision labels, per-platform SLSA provenance, the absent index and platform SBOM layers, and separates the completed manualHERMES-22live evidence from the trusted manual PR E2E run that remains the merge gate. Strict documentation validation completed with 0 errors and 2 pre-existing warnings.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailableef14268bc9ceb1bb9254d9b4752e31534f96af5e. The later documentation and status-hint work rerannpx vitest run test/agent-variant-docs.test.ts test/hermes-dependency-review.test.ts src/lib/messaging/channels/whatsapp/hooks src/lib/actions/sandbox/channel-status.test.ts— 5 files and 108 tests passed — andnpm run docscompleted with 0 errors and 2 pre-existing warnings.ef14268bc9ceb1bb9254d9b4752e31534f96af5eon a final image built from that revision: dashboard QR pairing, a connected bridge, an inbound message-path reply, and OpenShell proxy-audit admission. See fix(hermes): align dashboard WhatsApp session path #8229 (comment). Required CI and the trusted manual PR E2E run remain merge gates.npm run docsbuilds without warnings (doc changes only) — completed with 0 errors and 2 pre-existing warnings.Evidence currency: every commit after
ef14268bc9ceb1bb9254d9b4752e31534f96af5eis a merge ofmain. This branch's own sources are unchanged since that revision, and the reviewed subject is identical at the current head:agents/hermes/whatsapp-proxy.patchis byte-identical andagents/hermes/Dockerfilestill pins base indexsha256:3d54b928baef9df403227e846f73079d13ca8424a27cd5268ca97bac3f030b27. The later documentation commit changes prose only.Additional image evidence:
sha256:3d54b928baef9df403227e846f73079d13ca8424a27cd5268ca97bac3f030b27.sha256:63b5a1944a4613f90fa8f9f92f6a3ccb2ceac6df23bcefad0282ed4167e6c09cand arm64 childsha256:f30b5c2529bbb5c06de38d0c1a7018c27d6c08d5a917503fa354300f12e5568c.NVIDIA/NemoClawsource labels, revisiona7a7f3e470a75c404d316d2054445e16bb63b48c, and per-platform SLSA provenance from run31031662054, attempt 1. The index has no index-level attestation manifest, and neither platform attestation contains an SBOM layer.ef14268bc9ceb1bb9254d9b4752e31534f96af5ewith base indexsha256:3d54b928…3f030b27: dashboard pairing wrote credentials to/sandbox/.hermes/platforms/whatsapp/sessionwith the legacy directory empty, the bridge reported{"status":"connected"}, an inbound message from an allowlisted sender received an agent reply, and every WhatsApp flow was admitted underpolicy:whatsappin the OpenShell proxy audit. No target in this repository pairs a live WhatsApp account, so the trusted manual PR E2E run remains the merge gate for that evidence. Evidence and reproduction steps: fix(hermes): align dashboard WhatsApp session path #8229 (comment)Signed-off-by: San Dang sdang@nvidia.com