fix(messaging): let the operator choose the Hermes WhatsApp mode - #8423
fix(messaging): let the operator choose the Hermes WhatsApp mode#8423hunglp6d wants to merge 15 commits into
Conversation
The manifest rendered `WHATSAPP_MODE=bot` as a constant, so neither mode was
reachable. Bot mode rejects every sender when the allowlist is empty, and
onboarding never collects one, so a freshly onboarded Hermes sandbox with
WhatsApp paired could not deliver a message.
The value is also an override. The Hermes adapter reads
`os.getenv("WHATSAPP_MODE", "self-chat")`, and self-chat reads no allowlist at
all, so leaving the key unset is the configuration that works. NemoClaw replaced
that with the one mode that needs a value it never asks for.
Make the mode an input carrying that same default, with no prompt, matching the
shape `allowedIds` already has. Onboarding asks nothing extra and a paired
sandbox replies. Selecting bot stays a deliberate act:
WHATSAPP_MODE=bot WHATSAPP_ALLOWED_IDS=<number> nemoclaw <sandbox> channels add whatsapp
Verified live on a Hermes sandbox before the change: with self-chat and no
allowlist, a message to the paired account's own chat reached the agent and was
answered; with bot mode and `WHATSAPP_ALLOWED_IDS='*'`, a message from a second
number was answered as well.
`channels status` now lists the mode beside the allowlist, since the mode
decides whether an empty allowlist matters.
Signed-off-by: Hung Le <hple@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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:
📝 WalkthroughWalkthroughHermes WhatsApp mode is configurable through ChangesHermes WhatsApp mode configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Onboarding
participant WhatsAppManifest
participant HostState
participant HermesConfig
Onboarding->>WhatsAppManifest: seed mode self-chat
Onboarding->>HostState: store mode and allowlist
HostState->>HermesConfig: render WhatsApp settings
HermesConfig-->>HermesConfig: use self-chat or bot mode
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-8423.docs.buildwithfern.com/nemoclaw |
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: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
![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 `@docs/manage-sandboxes/set-up-whatsapp.mdx`:
- Line 90: In the WhatsApp adapter sentence, replace the American-English
spelling “afterwards” with “afterward,” preserving the rest of the wording and
meaning.
🪄 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: 57bf1b12-3439-465a-9313-127f69b703af
📒 Files selected for processing (12)
docs/manage-sandboxes/set-up-whatsapp.mdxsrc/lib/messaging-channel-config.test.tssrc/lib/messaging/applier/host-state-applier.test.tssrc/lib/messaging/channels/metadata.test.tssrc/lib/messaging/channels/whatsapp/manifest.tssrc/lib/messaging/channels/whatsapp/rendered-config-parser.tssrc/lib/messaging/channels/whatsapp/template-resolver.test.tssrc/lib/messaging/channels/whatsapp/template-resolver.tssrc/lib/onboard/messaging-channel-setup.test.tstest/e2e/live/channels-stop-start-helpers.tstest/generate-hermes-config.test.tstest/messaging-plan-test-helper.ts
Signed-off-by: Hung Le <hple@nvidia.com>
A rebuild renders from the persisted plan, and neither persistence path re-applies the input's validValues: normalizeFullInputs copies `value` verbatim, and inputReferenceFromManifest copies `persisted.value` onto the manifest spec. A stale or hand-edited registry entry could therefore render a WHATSAPP_MODE the bundled bridge cannot serve, leaving a channel that reports itself configured and answers nobody. Re-check the value where it is rendered and fall back to the mode the Hermes adapter already defaults to. The earlier fallback test exercised the env-to-compiler path, where validValues already drops an unusable value, so it could not have caught this. Both persistence paths now have a case. Signed-off-by: Hung Le <hple@nvidia.com>
"Refer to the section below" and "set both values together" both left the reader to infer the target. Name the section and the two environment variables instead. Signed-off-by: Hung Le <hple@nvidia.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer security and gate audit completed for revision
No actionable security defect was found. Before approval, a maintainer should explicitly accept both the legacy-mode behavior and the narrower issue scope. I am changing the PR description from auto-closing #8312 to describing this as a partial implementation, so the remaining work is not closed accidentally. GitHub Actions jobs are currently queued or cancelled during the reported service degradation. I have not rerun them. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Documentation follow-up completed in revision
|
sandl99
added
area: messaging
sandl99
added
integration: whatsapp
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Cover explicit Hermes bot mode through the complete channel lifecycle. Seed WHATSAPP_MODE=bot with a nonempty sender allowlist, then assert the persisted input and rendered WHATSAPP_MODE=bot before stop, after rebuild, and after start. This mode changes sender authorization, so complete the nine-category security review for the final head. Refresh onto current main and rerun the managed-runtime activation and required checks after adding the lifecycle evidence.
4 participants
Summary
The WhatsApp channel manifest rendered
WHATSAPP_MODE=botas a constant, so neither Hermes mode was reachable and a freshly onboarded sandbox could not deliver a message:botrejects every sender when the allowlist is empty, and onboarding never collects one. The mode becomes a manifest input carrying the Hermes adapter's ownself-chatdefault, so a paired sandbox replies with nothing else configured, and selectingbotstays a deliberate act. OpenClaw is unchanged.Related Issue
Addresses part of #8312
Changes
src/lib/messaging/channels/whatsapp/manifest.ts— add amodeinput (envKey: WHATSAPP_MODE,statePath: whatsappConfig.mode,validValues: ["self-chat", "bot"],defaultValue: "self-chat") and renderWHATSAPP_MODE={{whatsappConfig.mode}}instead of the constant.src/lib/messaging/channels/whatsapp/template-resolver.ts— resolvewhatsappConfig.mode, following thetelegramConfig.groupPolicyprecedent.src/lib/messaging/channels/whatsapp/rendered-config-parser.ts— list the mode beside the allowlist sochannels statusshows both. The mode decides whether an empty allowlist matters, and the existingWHATSAPP_ALLOWED_IDS: not setline carried no such context.docs/manage-sandboxes/set-up-whatsapp.mdx— add a Hermes-only "Reply to Other Senders" section, and qualify the existing allowlist sentence, which was written whenbotwas the only mode.The input declares no prompt. Onboarding asks nothing extra;
botis selected the same wayallowedIdsalready is, by exporting the variable beforechannels add. That path is not a new mechanism:manifest-compiler.ts:322already prefers an input's env value over its default, andworkflow-planner.ts:302already lets an incoming channel replace a stored one.host-state-applier.test.tsnow pins that replacement, since the documented reconfiguration flow depends on it.Two behavior changes worth calling out:
botrendersself-chatafter its next rebuild. Its persisted plan has nomodeinput, so the manifest default applies. Verified against production code (normalizePersistedInputsplus the resolver). A migration rule keyed on "allowlist present implies bot" was considered and rejected: the documented way to select WhatsApp non-interactively is to exportWHATSAPP_ALLOWED_IDS, so that rule would silently place new sandboxes into the mode this issue is about. Affected operators re-run the command in the new docs section.WHATSAPP_MODEnow selects the channel non-interactively for either agent, including OpenClaw, where the mode is not rendered.WHATSAPP_ALLOWED_IDSalready behaves this way; inputs cannot be scoped per agent today (agents:exists on hooks, not onChannelInputBaseSpec).Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/manage-sandboxes/set-up-whatsapp.mdx. Codex Desktop re-reviewed the completed revision after adding the legacy rebuild behavior, post-changechannels statusverification, and explicit wildcard warning.npm run docspassed with 0 errors and 2 repository warnings.DGX Station Hardware Evidence
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 unavailablenpx vitest run --project cli src/lib/messaging/channels/whatsapp/ src/lib/messaging/applier/host-state-applier.test.ts src/lib/messaging/channels/metadata.test.ts src/lib/messaging-channel-config.test.ts src/lib/onboard/messaging-channel-setup.test.ts— 170 passed;npx vitest run --project integration test/generate-hermes-config.test.ts— 54 passed.npx vitest run --changed origin/main --project cli --project plugin --project e2e-supportreports 9165 passed and 18 failed; the same 18 fail on a cleanorigin/maincheckout (state-file-key-merge-behavior,state-file-key-merge-file-safety,created-sandbox-finalization) and are unrelated to this change.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable; this change is scoped to one channel manifest and its tests.npm run checks:repositorypassed (layer import boundaries, source architecture budget, vitest project membership, test title style).npm run docsbuilds 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.Live verification
Built from this branch on a CPU sandbox (Hermes v0.19.0, OpenShell docker driver) and exercised through the normal commands.
Default onboard, after
channels add whatsappandrebuild:The documented reconfiguration, run verbatim on the same sandbox:
That also confirms the stored value is replaced rather than kept.
QR pairing completed on the same sandbox and wrote 892 files to
/sandbox/.hermes/platforms/whatsapp/session. An inbound message was not exchanged on this host: the gateway logged[Whatsapp] npm install failed:and left the bridge down, althoughnode_modulesis present in both bundled bridge trees. That failure is independent of this change, which only decides the rendered value.Signed-off-by: Hung Le hple@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes