fix(whatsapp): force compact pairing QR at the real qrcode renderer (#4522) - #5016
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 (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a Node preload that forces compact terminal QR rendering for WhatsApp pairing, wires it into connect-session NODE_OPTIONS (defensive re-injection), replaces brittle tests with deterministic probes, adds a standalone E2E script and messaging-provider probe, updates migration inventory and test fixtures, and exposes a conditional CI regression job. ChangesWhatsApp Compact QR Rendering
Sequence Diagram(s)sequenceDiagram
participant CLI as nemoClaw CLI
participant Node as Node Runtime
participant ModuleHook as Module._load Hook
participant OpenClaw as openclaw renderer
participant Terminal as Terminal Renderer
CLI->>Node: spawn openclaw (with or without NODE_OPTIONS preload)
Node->>ModuleHook: load qrcode / qrcode-terminal (request contains "qrcode")
ModuleHook->>ModuleHook: detect shape, patch module to inject { small: true } for terminal
Node->>OpenClaw: call renderQrTerminal(payload)
OpenClaw->>Terminal: qrcode.toString(..., { type: "terminal", small: true })
Terminal-->>CLI: compact QR output (fewer rows)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
yimoj
force-pushed
the
fix/4522-whatsapp-compact-qr
branch
from
June 9, 2026 04:19
af4d523 to
d0bb9b4
Compare
![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: 2
🧹 Nitpick comments (1)
.github/workflows/regression-e2e.yaml (1)
440-460: ⚡ Quick winConsider adding artifact upload on failure for debugging consistency.
Other E2E jobs in this workflow upload logs when tests fail (e.g., lines 186-195, 224-233, 291-299, 338-346). Adding artifact upload for this job would improve debuggability when CI failures occur.
📦 Proposed enhancement: add artifact upload
- name: Run WhatsApp compact-QR reporter-workflow E2E test run: bash test/e2e/test-whatsapp-qr-compact-e2e.sh + + - name: Upload WhatsApp compact-QR logs on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: whatsapp-qr-compact-e2e-logs + path: | + /tmp/nemoclaw-wa-qr-e2e.*.log + if-no-files-found: ignoreNote: Adjust the
pathpattern based on actual log file paths written by the test script.🤖 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 @.github/workflows/regression-e2e.yaml around lines 440 - 460, The whatsapp-qr-compact-e2e job lacks an artifact upload step for failing runs; add a post-test step after the "Run WhatsApp compact-QR reporter-workflow E2E test" step that uses actions/upload-artifact to upload the test logs when the job fails (use if: failure() or if: always() to capture logs on failure), name the artifact clearly (e.g., whatsapp-qr-compact-e2e-logs) and point the path to the files produced by test/e2e/test-whatsapp-qr-compact-e2e.sh (adjust the path pattern to match that script's log output).
🤖 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 @.github/workflows/regression-e2e.yaml:
- Around line 456-457: The Checkout step currently uses "uses:
actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10" (named "Checkout");
update that step to add persist-credentials: false so the GITHUB_TOKEN is not
persisted to the workspace and cannot be used by later steps—modify the
"Checkout" step's step definition to include the persist-credentials: false
option under the uses entry.
- Around line 440-460: The whatsapp-qr-compact-e2e job currently runs
test/e2e/test-whatsapp-qr-compact-e2e.sh without ensuring Node.js is installed;
add an explicit setup-node step (e.g. actions/setup-node@v4) before the "Run
WhatsApp compact-QR reporter-workflow E2E test" step and set a stable
node-version (for example '18') so the node and npm commands used by the test
script are guaranteed to be available.
---
Nitpick comments:
In @.github/workflows/regression-e2e.yaml:
- Around line 440-460: The whatsapp-qr-compact-e2e job lacks an artifact upload
step for failing runs; add a post-test step after the "Run WhatsApp compact-QR
reporter-workflow E2E test" step that uses actions/upload-artifact to upload the
test logs when the job fails (use if: failure() or if: always() to capture logs
on failure), name the artifact clearly (e.g., whatsapp-qr-compact-e2e-logs) and
point the path to the files produced by test/e2e/test-whatsapp-qr-compact-e2e.sh
(adjust the path pattern to match that script's log output).
🪄 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: 33abd692-35fa-481c-88da-bf2af9c05c66
📒 Files selected for processing (6)
.github/workflows/regression-e2e.yamlnemoclaw-blueprint/scripts/whatsapp-qr-compact.jsscripts/nemoclaw-start.shtest/e2e/test-messaging-providers.shtest/e2e/test-whatsapp-qr-compact-e2e.shtest/whatsapp-qr-compact.test.ts
yimoj
force-pushed
the
fix/4522-whatsapp-compact-qr
branch
from
June 9, 2026 04:30
d0bb9b4 to
8efc8eb
Compare
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 `@test/nemoclaw-start.test.ts`:
- Around line 5241-5242: Two separate array entries '_SLACK_GUARD_SCRIPT=""' and
'_WHATSAPP_QR_COMPACT_SCRIPT=""' push the test file over the 5300-line
guardrail; to reclaim one line, collapse them into a single line (e.g., combine
the two entries into one comma-separated entry or otherwise merge their text) so
both settings remain but occupy one physical line; update the array element
containing '_SLACK_GUARD_SCRIPT' and '_WHATSAPP_QR_COMPACT_SCRIPT' accordingly.
🪄 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: 3479696b-0be1-4266-b358-23c77d44c19d
📒 Files selected for processing (8)
.github/workflows/regression-e2e.yamlnemoclaw-blueprint/scripts/whatsapp-qr-compact.jsscripts/nemoclaw-start.shtest/e2e-scenario/migration/legacy-inventory.jsontest/e2e/test-messaging-providers.shtest/e2e/test-whatsapp-qr-compact-e2e.shtest/nemoclaw-start.test.tstest/whatsapp-qr-compact.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/nemoclaw-start.sh
- test/e2e/test-messaging-providers.sh
- nemoclaw-blueprint/scripts/whatsapp-qr-compact.js
- test/whatsapp-qr-compact.test.ts
yimoj
force-pushed
the
fix/4522-whatsapp-compact-qr
branch
2 times, most recently
from
June 9, 2026 04:40
c90ab6e to
cbbb222
Compare
Collaborator
Author
|
Thanks @coderabbitai — addressed both:
Also removed the source-text assertions (the |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
( ˘ᵕ˘)🐇 |
yimoj
force-pushed
the
fix/4522-whatsapp-compact-qr
branch
2 times, most recently
from
June 9, 2026 04:50
373a8d9 to
754e41f
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/regression-e2e.yaml (2)
456-460:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd Node.js setup step before running the test.
The test script explicitly requires
nodeandnpmcommands and fails if they are not available (test/e2e/test-whatsapp-qr-compact-e2e.sh lines 63-74). Whileubuntu-latestrunners currently include Node.js by default, relying on implicit availability is fragile and can break if GitHub updates runner images.🔧 Proposed fix: add setup-node step
steps: - name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version: "22" + - name: Run WhatsApp compact-QR reporter-workflow E2E test run: bash test/e2e/test-whatsapp-qr-compact-e2e.shBased on learnings from other jobs in this workflow that require Node.js (lines 174-177, 213-216, 279-282, 328-331, 363-366), all use explicit
setup-nodesteps.🤖 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 @.github/workflows/regression-e2e.yaml around lines 456 - 460, Add an explicit Node.js setup step (using actions/setup-node) immediately before the "Run WhatsApp compact-QR reporter-workflow E2E test" step so the test script test/e2e/test-whatsapp-qr-compact-e2e.sh can rely on node and npm; update the workflow to mirror the other jobs that call setup-node (see the earlier setup-node usages) and ensure the setup-node action specifies the Node.js version required by the test.
456-457:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet
persist-credentials: falseon checkout to reduce token exposure.To minimize security risk, disable credential persistence in the checkout step. This prevents the
GITHUB_TOKENfrom being available to subsequent steps that don't require it.🔒 Proposed fix
- name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false🤖 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 @.github/workflows/regression-e2e.yaml around lines 456 - 457, The Checkout step (the step with "name: Checkout" and "uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10") should add the option persist-credentials: false to its step configuration; update that step so the checkout action is invoked with persist-credentials: false to prevent GITHUB_TOKEN from being persisted to subsequent steps.
🤖 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.
Duplicate comments:
In @.github/workflows/regression-e2e.yaml:
- Around line 456-460: Add an explicit Node.js setup step (using
actions/setup-node) immediately before the "Run WhatsApp compact-QR
reporter-workflow E2E test" step so the test script
test/e2e/test-whatsapp-qr-compact-e2e.sh can rely on node and npm; update the
workflow to mirror the other jobs that call setup-node (see the earlier
setup-node usages) and ensure the setup-node action specifies the Node.js
version required by the test.
- Around line 456-457: The Checkout step (the step with "name: Checkout" and
"uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10") should add
the option persist-credentials: false to its step configuration; update that
step so the checkout action is invoked with persist-credentials: false to
prevent GITHUB_TOKEN from being persisted to subsequent steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 11dd91e9-f605-4114-bd33-0d89b64c5258
📒 Files selected for processing (8)
.github/workflows/regression-e2e.yamlnemoclaw-blueprint/scripts/whatsapp-qr-compact.jsscripts/nemoclaw-start.shtest/e2e-scenario/migration/legacy-inventory.jsontest/e2e/test-messaging-providers.shtest/e2e/test-whatsapp-qr-compact-e2e.shtest/nemoclaw-start.test.tstest/whatsapp-qr-compact.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- scripts/nemoclaw-start.sh
- test/e2e/test-messaging-providers.sh
- test/nemoclaw-start.test.ts
- nemoclaw-blueprint/scripts/whatsapp-qr-compact.js
- test/whatsapp-qr-compact.test.ts
- test/e2e-scenario/migration/legacy-inventory.json
…VIDIA#4522) The in-sandbox `openclaw channels login --channel whatsapp` pairing QR still rendered full size (~56 rows) and overflowed the terminal after PR NVIDIA#4607. Root cause: the upstream @openclaw/whatsapp plugin renders the QR via `renderQrTerminal()` → the `qrcode` package's `toString(text, { type: "terminal", small })`, and the bundled plugin (version-matched to OpenClaw, e.g. 2026.5.27) passes NO `small` flag, so it defaults to full size. The previous fix patched the unrelated `qrcode-terminal` package, which the WhatsApp plugin never loads — so the compact rendering was never applied at the actual pairing entrypoint. Fix: - Rewrite the NODE_OPTIONS preload to patch the `qrcode` package's `toString` (the real renderer) and force `small: true` for terminal renders, independent of what the plugin version passes. Detection requires an OWN `toString` + `create` so internal qrcode submodules are not mutated. The qrcode-terminal `generate` path is still patched as a fallback. Non-terminal renders (svg/png/utf8) are untouched. - Wire the preload into the connect-session NODE_OPTIONS (deferred `[ -f ]` guard) so ANY openclaw invocation in the session renders compact, not just the bypassable openclaw() shell-function path; the guard injection remains as defense-in-depth. Coverage (proves rendered QR size, not just preload presence): - New hermetic E2E `test/e2e/test-whatsapp-qr-compact-e2e.sh` installs the exact @openclaw/whatsapp + openclaw versions pinned in Dockerfile.base and drives the real `renderQrTerminal` symbol the channel-login onQr callback uses: 56 rows without the preload, 29 rows with it. Wired into regression-e2e.yaml. - Sandbox parity (test-messaging-providers.sh M-WA6d) renders the QR in-sandbox through the baked renderer with the connect-session NODE_OPTIONS active and asserts compact dimensions. - Unit test rewritten to patch/verify the `qrcode` package shape and the connect-session wiring. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
yimoj
force-pushed
the
fix/4522-whatsapp-compact-qr
branch
from
June 9, 2026 05:04
754e41f to
5b72c3d
Compare
Collaborator
Author
|
@coderabbitai review Addressed the two workflow findings in 5b72c3d: added an explicit |
Contributor
|
✅ Action performedReview finished.
|
cv
approved these changes
Jun 9, 2026
jyaunches
pushed a commit
that referenced
this pull request
Jun 10, 2026
## Summary - Add v0.0.62 release notes from Discussion #5100 and link release highlights to the relevant docs pages. - Document the release's GPU sandbox recreation, sandbox-side local inference verification, and Hermes dashboard port guard in the command and inference references. - Refresh generated NemoClaw user skills for the release-prep docs set. ## Source Summary - #4956 -> `docs/reference/commands.mdx`: Document CDI-first Docker GPU recreation behavior for Linux Docker-driver sandboxes. - #5024 -> `docs/inference/use-local-inference.mdx`: Document sandbox-runtime verification of the `inference.local` local inference route. - #5018 -> `docs/reference/commands.mdx`: Document Jetson/Tegra device-node group propagation for sandbox CUDA initialization. - #5012, #4763, #4706, #5030, #5015 -> `docs/about/release-notes.mdx`: Summarize onboarding and recovery reliability fixes, including the reserved Hermes API port guard. - #5017 and #5043 -> `docs/about/release-notes.mdx`, `docs/reference/commands.mdx`: Summarize mutable OpenClaw config recovery and host-side `agents list` coverage. - #5010 and #5016 -> `docs/about/release-notes.mdx`: Summarize Hermes upstream metadata visibility and WhatsApp QR rendering reliability. - #5045 and prior source docs in the v0.0.62 range -> `.agents/skills/`: Refresh generated user-skill references from the current docs source. ## Skipped - #5019 -> skipped for new prose because it touched `openclaw-sandbox-permissive.yaml`, which matches `docs/.docs-skip`. Existing source docs remain the source for generated skill synchronization. ## Verification - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `npm run docs` (passes; Fern reports 0 errors and 1 hidden warning) - Pre-commit hooks passed during commit, including docs-to-skills verification, markdown lint, gitleaks, and skills YAML tests. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `nemoclaw <name> agents list` command. * v0.0.62 release notes added summarizing onboarding and recovery improvements. * **Bug Fixes** * Improved GPU sandbox onboarding reliability (NVIDIA CDI path, Jetson/Tegra device handling). * Better local inference verification and recovery for Linux Docker-driver GPU sandboxes. * Quieter/earlier handling of onboarding drift and port collisions. * **Documentation** * Expanded GPU passthrough, inference verification, writable paths (`/dev/pts`), port 8642 restriction, and command examples. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
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
4 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
The in-sandbox
openclaw channels login --channel whatsapppairing QR still rendered full size (~56 rows) and overflowed the terminal after #4607, making it impossible to scan. This forces compact rendering at the actual renderer the pairing flow uses, with an E2E that proves the rendered QR dimensions.Related Issue
Fixes #4522
Changes
renderQrTerminal()→ theqrcodepackage'stoString(text, { type: "terminal", small }), and the bundled@openclaw/whatsapp(version-matched to OpenClaw, e.g.2026.5.27) passes nosmallflag, so it defaults to full size. The previous fix patched the unrelatedqrcode-terminalpackage, which the WhatsApp plugin never loads — so compact rendering was never applied at the pairing entrypoint.nemoclaw-blueprint/scripts/whatsapp-qr-compact.js: rewrite the NODE_OPTIONS preload to patch theqrcodepackage'stoString(the real renderer) and forcesmall: truefor terminal renders, independent of what the plugin version passes. Detection requires an owntoString+create, so internal qrcode submodules are not mutated; theqrcode-terminalgeneratepath is still patched as a fallback. Non-terminal renders (svg/png/utf8 data URIs) are untouched.scripts/nemoclaw-start.sh: wire the preload into the connect-sessionNODE_OPTIONS(deferred[ -f ]guard) so any openclaw invocation in the session renders compact — not only the bypassableopenclaw()shell-function path. The guard injection remains as defense-in-depth.test/e2e/test-whatsapp-qr-compact-e2e.sh(new): hermetic reporter-workflow E2E that installs the exact@openclaw/whatsapp+openclawversions pinned inDockerfile.baseand drives the realrenderQrTerminalsymbol the channel-loginonQrcallback uses, asserting the QR is oversized without the preload and compact with it. Wired intoregression-e2e.yaml.test/e2e/test-messaging-providers.sh(M-WA6d): render the QR in-sandbox through the baked renderer with the connect-sessionNODE_OPTIONSactive and assert compact dimensions.test/whatsapp-qr-compact.test.ts: rewritten to patch/verify theqrcodepackage shape (terminal forced small; non-terminal untouched; idempotent).Type of Change
Verification
Reporter-workflow E2E (proves rendered QR size at the real WhatsApp renderer). The exact reporter workflow is
nemoclaw onboard(WhatsApp enabled) →nemoclaw <sandbox> connect→openclaw channels login --channel whatsapp. A full live pairing needs a phone + WhatsApp account, but the bug is purely in QR rendering, which happens in the plugin'sonQrcallback before any phone interaction. The E2E installs the exact@openclaw/whatsapp@2026.5.27+openclaw@2026.5.27the sandbox bundles and drives the samerenderQrTerminalsymbol (openclaw/plugin-sdk/media-runtime) thatopenclaw channels login --channel whatsappcalls.Reporter-workflow E2E ran locally — command:
bash test/e2e/test-whatsapp-qr-compact-e2e.sh. Log output (on PR head):Pipeline E2E covers the reporter workflow — job
whatsapp-qr-compact-e2ein.github/workflows/regression-e2e.yamlruns the sametest/e2e/test-whatsapp-qr-compact-e2e.shon the PR head (dispatch:gh workflow run regression-e2e.yaml -f jobs=whatsapp-qr-compact-e2e -f pr_number=5016). The nightlytest/e2e/test-messaging-providers.sh(checkM-WA6d) additionally renders the QR in-sandbox through the baked plugin with the connect-sessionNODE_OPTIONSactive and asserts the same compact dimensions.Targeted suites pass —
test/whatsapp-qr-compact.test.ts(19),test/nemoclaw-start.test.ts(138),test/e2e-scenario/framework-tests/e2e-migration-inventory.test.ts, all green vianpx vitest run.npm test— full suite green; the only reds were CI flakes in unrelated slow subprocess tests (model-router-python,status-gateway-lifecycle,tunnel-command) that pass in isolation and are not touched by this change.Tests added or updated for new or changed behavior
No secrets, API keys, or credentials committed
Reviewed with the code-review skill and CodeRabbit; actionable findings fixed (over-broad qrcode module match;
setup-node+persist-credentials: falseon the new CI job; failure-artifact upload; test-file size budget; zero source-shape assertions)Signed-off-by: Yimo Jiang yimoj@nvidia.com
Summary by CodeRabbit
New Features
Tests
Documentation
Chores
Maintenance