fix(whatsapp): report the paired in-process bridge in channels status - #7015
Conversation
… and refactor Signed-off-by: Hung Le <hple@nvidia.com>
…dge liveness 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:
📝 WalkthroughWalkthroughAdds an OpenClaw WhatsApp status-health hook using authoritative sandbox status JSON and shared channel-health reports. Registers the hook through the manifest and built-in registry, then routes sandbox channel status through the generic health-hook runner with fallback reporting. ChangesWhatsApp channel health
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChannelStatus
participant HookRunner
participant WhatsappStatusHealth
participant SandboxExec
ChannelStatus->>HookRunner: run channel health hook
HookRunner->>WhatsappStatusHealth: invoke whatsapp.statusHealth
WhatsappStatusHealth->>SandboxExec: execute OpenClaw status JSON command
SandboxExec-->>WhatsappStatusHealth: return probe result
WhatsappStatusHealth-->>HookRunner: return channelHealth report
HookRunner-->>ChannelStatus: render channel status
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit b6d0ae1 in the TypeScript / code-coverage/cliThe overall coverage in commit b6d0ae1 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
1 warning · 0 suggestionsWarningsWarnings do not block.
|
E2E Target Results — ❌ Some tests failedRun: 29494504124
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/lib/actions/sandbox/channel-status.ts`:
- Around line 24-27: Update showSandboxChannelStatus() to route status-hook
execution through the channel-health manifest rather than gating
runChannelHealthHook() on diagnostic.deepProbe or legacy diagnostic values.
Remove the WhatsappDiagnosticReport import and keep the status path typed as
ChannelHealthReport, ensuring real phase: "status" hooks are executed.
In `@src/lib/messaging/channels/whatsapp/hooks/status-health.ts`:
- Around line 249-252: The WhatsApp health probe must not mark the bridge alive
from any historical startup or listening log. Update the shell logic around
__wa_scoped and GW_ALIVE to derive liveness from the latest successful readiness
event, invalidate it when a later stop or error event appears, and ensure failed
or stale starts cannot override pgrep or produce connectionState "open".
- Around line 217-223: Update the directory-status logic around the quotedDirs
shell loop in src/lib/messaging/channels/whatsapp/hooks/status-health.ts:217-223
to report a populated credentials layout only when the authoritative nested
WhatsApp session artifact exists, not merely when credentials/whatsapp contains
entries. In
src/lib/messaging/channels/whatsapp/hooks/status-health.test.ts:118-144, replace
injected POPULATED results with behavioral coverage through the public boundary
for missing, incomplete, and valid account-session layouts.
🪄 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: 9670ec56-cac1-4e67-8766-e540708b945b
📒 Files selected for processing (11)
src/lib/actions/sandbox/channel-status-config.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/messaging/channels/manifests.test.tssrc/lib/messaging/channels/whatsapp/hooks/index.tssrc/lib/messaging/channels/whatsapp/hooks/status-health-eval.test.tssrc/lib/messaging/channels/whatsapp/hooks/status-health-eval.tssrc/lib/messaging/channels/whatsapp/hooks/status-health.test.tssrc/lib/messaging/channels/whatsapp/hooks/status-health.tssrc/lib/messaging/channels/whatsapp/manifest.tssrc/lib/messaging/hooks/builtins.tssrc/lib/messaging/hooks/hook-runner.test.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/lib/messaging/channels/whatsapp/hooks/status-health.test.ts (1)
43-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFail when the sequential mock receives an unexpected extra call.
Repeating the final result can hide additional probe executions, allowing the test at Lines 230–249 to pass despite violating its one-exec-per-hook assumption.
Proposed fix
return vi.fn((_sandbox: string, _command: string, _timeout: number): ExecResult => { - const value = results[call] ?? results[results.length - 1] ?? null; - call += 1; - return value; + if (call >= results.length) { + throw new Error(`Unexpected exec call ${call + 1}`); + } + return results[call++]; });As per path instructions, flag conditionals that let tests pass without exercising their 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 `@src/lib/messaging/channels/whatsapp/hooks/status-health.test.ts` around lines 43 - 48, Update makeSequentialExec so calls beyond the supplied results fail immediately instead of reusing the final result or returning null. Preserve sequential result consumption for expected calls, ensuring tests such as the one-exec-per-hook case detect unexpected extra executions.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 `@src/lib/messaging/channels/whatsapp/hooks/status-health.ts`:
- Around line 203-215: Update the !wa branch in the WhatsApp health evaluation
to derive recentLogSignals from json.gatewayReachable: report the gateway as
unreachable when it is false, and report the channel as unavailable or
unconfigured when it is true. Preserve probeReachable and the null runtime
fields so the evaluator still resolves to “unknown.”
- Around line 261-276: Update summarizeOpenclawLive to whitelist healthState
against the documented enum before adding it to parts. Preserve recognized
non-healthy states, map any other non-null value to the fixed "unknown" token,
and avoid emitting arbitrary external text verbatim.
- Around line 347-351: Update epochMsToIso to validate that the numeric epoch
value is within JavaScript Date’s supported range before calling toISOString().
Return null for out-of-range values, while preserving the existing null behavior
for non-numeric, non-finite, and non-positive inputs.
- Around line 90-93: Validate the agent value before the probe selection in the
health-check flow: only "hermes" and the supported OpenClaw agent should
proceed. Deny or skip unsupported values safely instead of routing every
non-Hermes value to runOpenclawStatusProbe, while preserving the existing probe
behavior for valid agents.
---
Nitpick comments:
In `@src/lib/messaging/channels/whatsapp/hooks/status-health.test.ts`:
- Around line 43-48: Update makeSequentialExec so calls beyond the supplied
results fail immediately instead of reusing the final result or returning null.
Preserve sequential result consumption for expected calls, ensuring tests such
as the one-exec-per-hook case detect unexpected extra executions.
🪄 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: eae2d0c1-cbd0-4336-a427-f969c0b7b83c
📒 Files selected for processing (4)
src/lib/actions/sandbox/channel-status.test.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/messaging/channels/whatsapp/hooks/status-health.test.tssrc/lib/messaging/channels/whatsapp/hooks/status-health.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/channel-status.ts
…msg) Signed-off-by: Hung Le <hple@nvidia.com>
…A/NemoClaw into fix/whatsapp-status-probe-unpaired
Signed-off-by: Hung Le <hple@nvidia.com>
E2E Target Results — ✅ All requested tests passedRun: 29562437470
|
hunglp6d
added
VRDC
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Maintainer sweep update: the current-base CI run passed on its first attempt, and 8 of 10 selected live E2E jobs passed. The two Hermes outcomes do not point to this PR's WhatsApp status-probe change:
The existing approval stands. I am deferring the remaining merge decision on live infrastructure evidence; no source changes are requested from the contributor. |
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Exact-head review at e0100d4 found a production contract mismatch. The probe reads json.channels.whatsapp, but the pinned OpenClaw 2026.6.10 status command returns per-channel account arrays under channelAccounts and healthy payloads need not contain channels or gatewayReachable. A real healthy response is therefore classified as probe_failed. Consume channelAccounts.whatsapp, select the default account deterministically, and add regression coverage using the exact upstream-shaped payload.
Co-authored-by: Hung Le <hple@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the exact-head OpenClaw response-contract blocker in verified signed commit 046e987, with original author Hung Le credited as co-author. The probe now consumes channelAccounts.whatsapp, selects channelDefaultAccountId.whatsapp deterministically, fails closed on malformed or ambiguous account payloads, and retains the bounded legacy summary fallback only when channelAccounts is absent. Validation before push: 76 focused tests passed, Biome and diff checks passed, and CLI build/typecheck passed. Fresh exact-head CI is running. |
Co-authored-by: Hung Le <hple@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the remaining exact-head advisor warning in verified signed commit d68b395. Official OpenClaw source confirms both the pinned 2026.6.10 producer and NemoClaw blueprint compatibility floor 2026.3.11 emit channelAccounts plus channelDefaultAccountId, so the unproven summary-only fallback was removed and now fails closed. Invalid-field tests were kept meaningful by moving them onto the authoritative account payload. Validation: 77 focused tests, CLI typecheck, Biome, and diff check passed. Hung Le remains credited as co-author. |
Superseded by exact-head fixes 046e987 and d68b395: the probe now consumes channelAccounts/default account state and fails closed on summary-only payloads. Independent audit passed with 77 focused tests, zero unresolved threads, and green exact-head CI/advisors. Protected E2E remains a separate required gate.
|
Exact-head E2E follow-up: the single permitted failed-job rerun completed successfully.
The required |
|
The prior exact-revision E2E verdict is terminal for head The intervening mainline change does not overlap this PR's WhatsApp status-probe files. Maintainer edits are disabled for the branch, so please merge current |
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `## v0.0.90` entry to `docs/changelog/2026-07-20.mdx` before the release tag is planned. The update also corrects the documented custom-image migration window so the compatibility fallback that first ships in v0.0.90 remains available until v0.0.91. ## Changes - Add the v0.0.90 summary and detailed release bullets for managed-image routing, provider-reset recovery, WhatsApp health reporting, and DGX Station guidance. - Keep the newest release first in the shared dated changelog and use root-absolute links to the canonical OpenClaw routes. - Correct `docs/reference/commands.mdx` to state that the legacy image route selector remains supported through v0.0.90 and is removed in v0.0.91. - Release source summary: - [#7264](#7264) -> `docs/resources/prompt-assets/dgx-station.md`, `docs/changelog/2026-07-20.mdx`: Record the versioned Station installer path, Nemotron 3 Ultra 550B default, and explicit DeepSeek override. - [#7261](#7261) -> `docs/get-started/dgx-station-preparation.mdx`, `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`, `docs/changelog/2026-07-20.mdx`: Include the OpenIB, legacy recovery, and Additional Setup documentation follow-ups. - [#7232](#7232) -> `docs/changelog/2026-07-20.mdx`: Document provider-reset recovery for wrapped OpenShell attachment diagnostics. - [#7189](#7189) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-20.mdx`: Document the managed-image route-selector rename and correct its one-release migration window. - [#7015](#7015) -> `docs/changelog/2026-07-20.mdx`: Document corrected OpenClaw WhatsApp health reporting. - No additional user-facing page update is needed for [#7193](#7193), [#7110](#7110), [#6783](#6783), or [#7263](#7263) because they change contributor governance, internal CI or release automation, or editorial style without changing supported user behavior. - [#7242](#7242) and [#7225](#7225) are already ancestors of and documented in v0.0.89, so this entry does not duplicate them despite their stale v0.0.90 labels. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog heading, SPDX form, version order, and published links. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR does not change `scripts/prepare-dgx-station-host.sh` or runtime behavior. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## 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 check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run; this is a focused documentation-only change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with 0 errors and two unrelated baseline warnings for unauthenticated redirect checks and the existing light-mode contrast ratio. - [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) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.90 covering inference routing, credential reset behavior, WhatsApp status detection, and DGX Station coding-agent guidance. - Updated custom Dockerfile guidance to document continued support for the legacy provider argument through v0.0.90. - Clarified that legacy declarations must be renamed to `NEMOCLAW_INFERENCE_PROVIDER_ID` before v0.0.91. - Added and refreshed related documentation links. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
8 participants
Summary
nemoclaw <sandbox> channels status --channel whatsappreported a working, paired WhatsApp bot asunpairedwith a dead bridge: the probe checked the pre-2026.6.10 session path and pgrep'd for a separate bridge process that no longer exists (the bridge now runs in-process inside the gateway). This teaches the probe the current OpenClaw session location and derives in-process liveness from the canonical gateway log, and moves the whole probe into a manifest-firstphase:"status"hook sochannel-status.tscarries zero WhatsApp-specific code (mirroring the Telegram hook from #6887).Related Issue
Fixes #7016
Changes
credentials/whatsapp/<account>in addition to the legacywhatsapp/dir (OpenClaw 2026.6.10+ stores the paired Baileys session there), so a paired sandbox is no longer misread asunpaired./tmp/gateway.log(where NemoClaw redirects gateway stdout, peragent/gateway-script-shared.ts; the same log the Telegram hook reads), scoped to[whatsapp]lines and emitting only markers + a redacted ISO timestamp — never a raw log line that could carry a phone number.exec.status === 0) before trusting the result, so a timed-out probe classifies asprobe_failedinstead of reading a verdict off partial stdout (matches the Telegram hook).messaging/channels/whatsapp/hooks/{status-health,status-health-eval}.ts, register aphase:"status"hook in the whatsapp manifest, and run it through the generic status-hook runner.channel-status.tsloses ~312 lines of WhatsApp-specific code; its dispatch now routes bothin-sandbox-qrandlog-taildeep-probe channels through the same genericrunChannelHealthHook. Deletessandbox/whatsapp-diagnostics.ts.messaging/channels/whatsapp/hooks/status-health.test.ts+status-health-eval.test.ts: credentials-path evidence, gateway-log heartbeat synthesis,/tmp/gateway.logpath assertion (guards the path regression), non-zero-exit →probe_failed, and ash -nsyntax check of the generated probe script.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Hung Le hple@nvidia.com
Summary by CodeRabbit
New Features
whatsapp.statusHealth) that reports bridge liveness and heartbeat when available (OpenClaw-based; Hermes bypasses the probe).Bug Fixes
Tests