fix(messaging): let the operator choose the Hermes WhatsApp mode - #8423
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 |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 82c3fe5 in the TypeScript / code-coverage/cliThe overall coverage in commit 82c3fe5 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence 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 for the commit under review. Recommended E2E: None Manual-only E2E: 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 `@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
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Security review completed for revision 62d3f37 after merging current main. Result: no new security finding.
Validation passed: eight CLI test files with 119 tests, the Hermes config suite with 55 tests, JS and CLI typechecks, the complete PR validation, and the documentation build with 0 errors. Fresh repository checks are running. The prior live note that the bot bridge did not spawn remains a non-security acceptance condition until the updated lifecycle evidence or a maintainer decision covers it. |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
The requested Hermes bot lifecycle and nine-category security review are present on the refreshed branch, and I pushed b4ca758 to close the sole required-suite failure on the final head. The failing shard was unrelated to messaging: the Linux-only static trampoline compile exceeded the generic 15-second Vitest limit under CI load. Its explicit timeout is now 30 seconds; behavior and assertions are unchanged. Current lifecycle evidence:
Validation:
Requesting re-review on the final head. |
The latest PR commit adds Hermes bot lifecycle evidence, completes the security review, includes the current base, and has passing managed-runtime activation evidence. Dismissing this resolved review before a full current-commit review.
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed the current head, code diff, required CI, and advisor feedback. No blocking code findings; required checks are green. The invalid-WHATSAPP_MODE coverage and auxiliary OpenClaw activation failure remain nonblocking follow-up and maintainer-disposition items.
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
5 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.channels add whatsappnow asks the operator which senders the sandbox answers, and derivesWHATSAPP_DM_POLICYfrom that answer and the allowlist. A blank answer keepsself-chat, so a paired sandbox works with nothing else configured. Answeringbotwithout an allowlist renderspairing, 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 amodeinput (envKey: WHATSAPP_MODE,statePath: whatsappConfig.mode,validValues: ["self-chat", "bot"],defaultValue: "self-chat") with an operator prompt; wire the sharedcommon.configPromptenroll hook limited toagents: ["hermes"]; renderWHATSAPP_MODE={{whatsappConfig.mode}}andWHATSAPP_DM_POLICY={{whatsappConfig.dmPolicy}}instead of the constant.src/lib/messaging/channels/whatsapp/template-resolver.ts— resolvewhatsappConfig.mode, following thetelegramConfig.groupPolicyprecedent, and addwhatsappConfig.dmPolicy, which reads the mode as well as the allowlist. Remove the unreferencedallowedIds.whatsapp.dmPolicycase it replaces.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— 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 whenbotwas the only mode..envassertions ingenerate-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 readsos.getenv("WHATSAPP_DM_POLICY", "pairing"). Leaving the key unset therefore left the bridge enforcing an empty allowlist and rejecting every sender, whilebotmode also stops replies to the paired account's own self-chat — a sandbox nobody could reach. Rendering the value is what makesbotusable without an allowlist. This compensates for an upstream default divergence on the pinned runtime; a Hermes-side report is planned separately.The derivation is:
WHATSAPP_DM_POLICYself-chatbotallowlistbotpairingScope
The prompt is limited to Hermes through the hook's
agentslist.WHATSAPP_MODEandWHATSAPP_DM_POLICYare 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 omitsallowFromanddmPolicy, andgit log -S"allowFrom"on this manifest is empty, so that omission is longstanding rather than lost here.Two behavior changes worth calling out:
botrendersself-chatafter its next rebuild. Its persisted plan has nomodeinput, 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 exportWHATSAPP_ALLOWED_IDS, so that rule would silently place new sandboxes into the mode this issue is about. Affected operators re-runchannels add whatsappand answerbot.WHATSAPP_MODEstill 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). An operator who exports the variable also skips the prompt, which keeps the previously documented non-interactive path working.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/manage-sandboxes/set-up-whatsapp.mdxdocuments the Hermes modes, sender authorization, allowlist behavior, migration, recovery, and lifecycle verification. The strict documentation build passed with 0 errors and 2 existing 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/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.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable;npm run validate:prpassed 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:repositorypassed.npx vitest run --project cliafternpm run catalog:compilereports 1158 of 1161 files passing; the three failures need Python 3.11 fortomlliband are unrelated.npx vitest run --project integrationafternpm run build:clireports 720 of 734 files passing; the eight failures need Python 3.11 fortyping.NotRequired, a locally absentqrcodepackage, or an external terminology tool, and none reference this change.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 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:
Pressing Enter, on sandbox
wa-self:The policy line is absent, which is the intended result for
self-chat. Hermes read the mode back asMode: personal number (self-chat), QR pairing completed, and aftergateway restartthe bridge ran with--mode self-chatand the agent replied to a message the paired account sent to itself.Answering
bot, on sandboxwa-bot:Hermes read that mode back as
Mode: separate bot number. On the same sandbox, the in-sandboxhermes whatsappwizard was then used to write an allowlist, which appendedWHATSAPP_ALLOWED_USERS=*outside the configuration NemoClaw seals; the nextgateway restartrefused to relaunch on the integrity guard.rebuild --yesrestored 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
botmode on this host: the bridge did not spawn after that rebuild. The same code spawns the bridge inself-chaton 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