fix(messaging): add WhatsApp channel diagnostics (#4386)#4406
Merged
Conversation
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a sandbox channel status action and oclif command that probes messaging channels (WhatsApp-specialized), parses heartbeat/log/process evidence into signals, selects a verdict (healthy/idle/unpaired/config_gap/policy_gap/probe_failed), and renders JSON or human text; includes tests, doctor/onboard UX changes, and docs. ChangesWhatsApp Channel Status Diagnostics
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as sandbox:channels:status
participant Action as showSandboxChannelStatus
participant Exec as ExecRunner
participant Eval as evaluateWhatsappDiagnostics
participant Render as renderReport
User->>CLI: nemoclaw <sandbox> channels status --channel whatsapp
CLI->>Action: invoke with {sandbox, channel, asJson, quietJson}
Action->>Action: resolve WhatsApp state directories
Action->>Exec: execute probe shell script (openshell sandbox exec)
Exec-->>Action: stdout (heartbeat + logs + process state)
Action->>Action: parseProbeOutput -> ProbeInput
Action->>Eval: evaluateWhatsappDiagnostics(ProbeInput)
Eval-->>Action: WhatsappDiagnosticReport {verdict, signals, hints}
Action->>Render: renderReport(report, asJson)
Render-->>Action: JSON or formatted text
Action->>Action: exitCodeFor(verdict) → 0 or 1
Action-->>CLI: report
CLI-->>User: stdout + process.exitCode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/reference/commands.mdx`:
- Around line 766-768: Replace the fenced code block that currently uses
"console" and includes a "$" prompt for the command `nemoclaw my-assistant
channels status --channel whatsapp` with a copyable shell block: change the
fence language from "console" to "bash" (or "sh") and remove the leading "$" so
the block contains just the raw command `nemoclaw my-assistant channels status
--channel whatsapp`; ensure no prompt markers remain and the block is
runnable/copyable.
🪄 Autofix (Beta)
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: ad075eea-5800-4ac5-af15-33ecbb2d7264
📒 Files selected for processing (12)
docs/reference/commands.mdxsrc/commands/sandbox/channels/status.tssrc/lib/actions/sandbox/channel-status.test.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/cli/command-registry.test.tssrc/lib/cli/public-display-defaults.tssrc/lib/onboard/messaging-channel-setup.tssrc/lib/sandbox/channels.tssrc/lib/sandbox/whatsapp-diagnostics.test.tssrc/lib/sandbox/whatsapp-diagnostics.ts
yimoj
added a commit
to yimoj/NemoClaw
that referenced
this pull request
May 28, 2026
The added `bash`-style invocation in docs/reference/commands.mdx now follows the project's documented convention — copyable command blocks use a `bash` fence with no leading `$`. The rest of the file's legacy `console` blocks are left untouched to keep this change scoped to the new section CodeRabbit flagged on PR NVIDIA#4406. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Adds `nemoclaw <sandbox> channels status [--channel <name>] [--json]`, a bounded host-side diagnostic that probes a sandbox's WhatsApp bridge and reports QR/session state, Noise WebSocket state, inbound event delivery, policy coverage, and channel registration as separate signals. A paired channel with no observed inbound delivery now returns verdict `idle` (exit 1) so the silent "paired but no inbound events" failure mode from the issue cannot pass as healthy. The doctor's Messaging check no longer reports `ok` when WhatsApp is enabled — conflict-signature heuristics do not cover WhatsApp Web — and instead points operators at the new diagnostic. The diagnostic sanitizes timestamps, redacts heartbeat error/note fields to a fixed category vocabulary, and whitelists connection-state strings so the host-side JSON output cannot leak phone numbers, message bodies, or tokens that bridges sometimes embed in their heartbeat files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
The added `bash`-style invocation in docs/reference/commands.mdx now follows the project's documented convention — copyable command blocks use a `bash` fence with no leading `$`. The rest of the file's legacy `console` blocks are left untouched to keep this change scoped to the new section CodeRabbit flagged on PR NVIDIA#4406. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…cs (NVIDIA#4386) Two follow-up hardenings on top of the rebased WhatsApp diagnostic: - Strict ISO-8601 only for heartbeat timestamps. `Date.parse` accepts loose forms (bare integer, `Date.toString()` output with parenthesized text) that the diagnostic would otherwise echo back through `channels status --json` or use as inbound evidence. Reject anything that does not match the ISO 8601 shape and re-emit accepted values via `toISOString()` so the rendered detail is deterministic regardless of how the bridge serialized the timestamp. - Explicit `PROC_DONE` marker after the in-sandbox pgrep so the orchestrator distinguishes "pgrep completed with no matches" (the bridge has crashed or runs under an unmatched name → fail) from "the probe aborted before reaching pgrep" (timeout/exec failure → info, do not gate verdict on it). Broadens the pgrep pattern to also catch processes whose argv carries the WhatsApp state-dir path so an embedded bridge inside the agent process still matches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
yimoj
force-pushed
the
fix/4386-whatsapp-inbound-diagnostics
branch
from
021096c to
fc54994
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/actions/sandbox/channel-status.test.ts (1)
368-407: ⚡ Quick winAlways restore the
process.exitspy viafinally.In this test,
exitSpy.mockRestore()only runs on the happy path. If anyexpector awaited call throws earlier, the mockedprocess.exitcan leak into subsequent tests.Proposed fix
- const exitSpy = vi - .spyOn(process, "exit") - .mockImplementation(((code?: number) => { - throw new Error(`process.exit(${code})`); - }) as never); - const { deps: depsNoMatch, out_lines: linesNoMatch } = makeDeps({ - exec: () => ({ status: 0, stdout: stdoutNoMatch, stderr: "" }), - }); - try { - await showSandboxChannelStatus("alpha", { deps: depsNoMatch }); - } catch { - /* expected exit(1) for stale-heartbeat + no bridge */ - } - const dumpNoMatch = linesNoMatch.join("\n"); - expect(dumpNoMatch).toMatch(/Bridge process: no WhatsApp bridge process observed/); - expect(dumpNoMatch).toMatch(/Verdict:.*idle/); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + try { + const { deps: depsNoMatch, out_lines: linesNoMatch } = makeDeps({ + exec: () => ({ status: 0, stdout: stdoutNoMatch, stderr: "" }), + }); + try { + await showSandboxChannelStatus("alpha", { deps: depsNoMatch }); + } catch { + /* expected exit(1) for stale-heartbeat + no bridge */ + } + const dumpNoMatch = linesNoMatch.join("\n"); + expect(dumpNoMatch).toMatch(/Bridge process: no WhatsApp bridge process observed/); + expect(dumpNoMatch).toMatch(/Verdict:.*idle/); - const stdoutTimeout = [ - "NEMOCLAW_WA_DIAG_OK", - "DIR /sandbox/.openclaw/whatsapp POPULATED", - "NEMOCLAW_WA_HEARTBEAT_BEGIN", - JSON.stringify({ - lastInboundAt: "2026-05-28T03:59:30.000Z", - messagesHandled: 1, - connectionState: "open", - }), - "NEMOCLAW_WA_HEARTBEAT_END", - "NEMOCLAW_WA_LOG_BEGIN", - "NEMOCLAW_WA_LOG_END", - // No PROC_DONE — simulating a probe that aborted before reaching - // the pgrep stage. - ].join("\n"); - const { deps: depsTimeout, out_lines: linesTimeout } = makeDeps({ - exec: () => ({ status: 0, stdout: stdoutTimeout, stderr: "" }), - }); - await showSandboxChannelStatus("alpha", { deps: depsTimeout }); - const dumpTimeout = linesTimeout.join("\n"); - expect(dumpTimeout).toMatch(/Bridge process: could not enumerate sandbox processes/); - expect(dumpTimeout).toMatch(/Verdict:.*healthy/); - exitSpy.mockRestore(); + const stdoutTimeout = [ + "NEMOCLAW_WA_DIAG_OK", + "DIR /sandbox/.openclaw/whatsapp POPULATED", + "NEMOCLAW_WA_HEARTBEAT_BEGIN", + JSON.stringify({ + lastInboundAt: "2026-05-28T03:59:30.000Z", + messagesHandled: 1, + connectionState: "open", + }), + "NEMOCLAW_WA_HEARTBEAT_END", + "NEMOCLAW_WA_LOG_BEGIN", + "NEMOCLAW_WA_LOG_END", + // No PROC_DONE — simulating a probe that aborted before reaching + // the pgrep stage. + ].join("\n"); + const { deps: depsTimeout, out_lines: linesTimeout } = makeDeps({ + exec: () => ({ status: 0, stdout: stdoutTimeout, stderr: "" }), + }); + await showSandboxChannelStatus("alpha", { deps: depsTimeout }); + const dumpTimeout = linesTimeout.join("\n"); + expect(dumpTimeout).toMatch(/Bridge process: could not enumerate sandbox processes/); + expect(dumpTimeout).toMatch(/Verdict:.*healthy/); + } finally { + exitSpy.mockRestore(); + }🤖 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 `@src/lib/actions/sandbox/channel-status.test.ts` around lines 368 - 407, The test currently mocks process.exit with exitSpy but only calls exitSpy.mockRestore() on the happy path, which can leak the mock if an assertion or awaited call throws; wrap the exercised calls (the first try/catch, the subsequent await showSandboxChannelStatus and all expects) in a try/finally and call exitSpy.mockRestore() in the finally block so process.exit is always restored; reference the existing exitSpy (spyOn(process, "exit")), the showSandboxChannelStatus calls, and the makeDeps usage to locate where to add the try/finally.
🤖 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.
Nitpick comments:
In `@src/lib/actions/sandbox/channel-status.test.ts`:
- Around line 368-407: The test currently mocks process.exit with exitSpy but
only calls exitSpy.mockRestore() on the happy path, which can leak the mock if
an assertion or awaited call throws; wrap the exercised calls (the first
try/catch, the subsequent await showSandboxChannelStatus and all expects) in a
try/finally and call exitSpy.mockRestore() in the finally block so process.exit
is always restored; reference the existing exitSpy (spyOn(process, "exit")), the
showSandboxChannelStatus calls, and the makeDeps usage to locate where to add
the try/finally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7939be4d-26de-4e0d-8468-c55a5d31fdde
📒 Files selected for processing (12)
docs/reference/commands.mdxsrc/commands/sandbox/channels/status.tssrc/lib/actions/sandbox/channel-status.test.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/cli/command-registry.test.tssrc/lib/cli/public-display-defaults.tssrc/lib/onboard/messaging-channel-setup.tssrc/lib/sandbox/channels.tssrc/lib/sandbox/whatsapp-diagnostics.test.tssrc/lib/sandbox/whatsapp-diagnostics.ts
✅ Files skipped from review due to trivial changes (1)
- docs/reference/commands.mdx
🚧 Files skipped from review as they are similar to previous changes (10)
- src/lib/cli/command-registry.test.ts
- src/lib/onboard/messaging-channel-setup.ts
- src/lib/cli/public-display-defaults.ts
- src/lib/actions/sandbox/policy-channel.ts
- src/lib/sandbox/channels.ts
- src/commands/sandbox/channels/status.ts
- src/lib/actions/sandbox/doctor.ts
- src/lib/sandbox/whatsapp-diagnostics.ts
- src/lib/actions/sandbox/channel-status.ts
- src/lib/sandbox/whatsapp-diagnostics.test.ts
CodeRabbit nit on PR NVIDIA#4406: the new "distinguishes pgrep no-match from probe abort" test mocked `process.exit` but only called `exitSpy.mockRestore()` on the happy path. If any assertion threw, the mock would leak into subsequent tests. Wrap the body in try/finally so the spy is always restored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Contributor
|
Actionable comments posted: 0 |
wscurran
added
fix
integration: whatsapp
Contributor
cv
approved these changes
May 29, 2026
wscurran
added
area: cli
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
nemoclaw <sandbox> channels status [--channel <name>] [--json], a bounded host-side diagnostic that surfaces QR/session state, Noise WebSocket state, inbound event delivery, policy coverage, and channel registration as separate signals so a paired-but-idle WhatsApp channel cannot be silently treated as healthy.Related Issue
Fixes #4386
Changes
nemoclaw <sandbox> channels statuscommand (src/commands/sandbox/channels/status.ts).src/lib/sandbox/whatsapp-diagnostics.ts) — pure parsing and verdict logic with sanitized timestamp, connection-state, and error-category outputs.src/lib/actions/sandbox/channel-status.ts) — boundedopenshell sandbox execprobe (8s) with shell that filters the probe shell out of its own pgrep, supports both OpenClaw (/sandbox/.openclaw/whatsapp) and Hermes (/sandbox/.hermes/platforms/whatsapp/session) state layouts, and routes paused channels through the basic per-channel report instead of probing a torn-down bridge.idlewhen paired + WebSocket up + no inbound observed (the [Brev][Agent&UX] WhatsApp inbound messages not received despite active Noise protocol WebSocket and successful pairing #4386 shape), and refuses to declarehealthywhen any fail signal is present.lastInboundAtvalues, collapse non-whitelisted connection states toother, redact heartbeat note/lastError to a category vocabulary (unauthorized,connection-closed,rate-limited,logged-out,qr-expired,other), and replace Node'sJSON.parseerror message with a fixed string so the host-side JSON output never echoes raw sandbox-owned heartbeat contents.sandbox doctor's Messaging check no longer reportsokwhen WhatsApp is enabled (it points to the new diagnostic instead).channels addand onboarding QR branches now surfacesetupNotes, including a hint to runchannels statusafter pairing.sh -nregression guard for the probe script.Type of Change
Verification
npx vitest run src/lib/sandbox/whatsapp-diagnostics.test.ts src/lib/actions/sandbox/channel-status.test.ts ...(352/352 across related suites).npm run build:cliandnpm run typecheck:cliclean.bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-clipasses (new command documented indocs/reference/commands.mdx).codex review -c sandbox_mode="danger-full-access" --uncommitteduntil no actionable findings.wa-diag-4386-*): pre-fixchannels statusdid not exist anddoctorreported WhatsApp[ok]; post-fixchannels statusreports separate signals with a non-zero exit anddoctorsurfaces a pointer to the new command. Live WhatsApp QR pairing was not possible in the triage environment (reported in the issue analysis), so inbound-delivery healthy/idle paths are covered by hermetic fixtures.Signed-off-by: Yimo Jiang yimoj@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes