Skip to content

fix(media): default terminal QR to full-block mode (#77820)#77844

Merged
steipete merged 1 commit into
openclaw:mainfrom
neeravmakwana:fix/77820-terminal-qr-ansi
May 12, 2026
Merged

fix(media): default terminal QR to full-block mode (#77820)#77844
steipete merged 1 commit into
openclaw:mainfrom
neeravmakwana:fix/77820-terminal-qr-ansi

Conversation

@neeravmakwana

@neeravmakwana neeravmakwana commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Default renderQrTerminal and OpenClaw call sites to full-block (small: false) terminal QR output so the final row is not a pathological ANSI hotspot from the bundled qrcode renderer.

Root cause

With { type: "terminal", small: true }, node-qrcode merges QR modules using half-block glyphs; for odd-height matrices the last terminal row takes a code path that wraps each cell in SGR sequences instead of once per row. That yields a last line with far higher escape density than other rows, which breaks scanning in strict terminals, narrow buffers, or some SSH clients (see #77820).

Linked issue

Fixes #77820.

Why this change is safe

Rendering still uses the same library and the same payload; only the terminal glyph strategy changes. The QR is larger vertically but escape density stays consistent row-to-row. Callers that truly need compact mode can still pass { small: true }.

Security / runtime controls

No changes to authentication, channel credentials, gateway policy, or secret handling. This only affects how QR strings are formatted for stdout/CLI logs.

Real behavior proof

  • Behavior or issue addressed: Terminal QR rendering no longer defaults to compact half-block mode that can emit a dense ANSI final row and make WhatsApp/Feishu/pairing QR scans fail in some terminals.
  • Real environment tested: macOS Darwin local OpenClaw checkout, Node v26.0.0, qrcode@1.5.4, real renderQrTerminal runtime with the bundled qrcode package.
  • Exact steps or command run after this patch: pnpm exec tsx -e '<script imports ./src/media/qr-terminal.ts, renders https://wa.me/login/2@SAMPLE-TOKEN-1234567890ABCDEF, counts ANSI SGR sequences per non-empty terminal row>'
  • Evidence after fix: Copied terminal output from the real renderer probe:
{"behavior":"default full-block terminal QR","environment":"darwin node v26.0.0","rows":35,"medianSgrPerRow":70,"maxSgrPerRow":70,"ratio":1}
  • Observed result after fix: Default terminal QR output uses full-block rendering with stable row density; the sample produced 35 non-empty rows with median SGR per row 70 and max SGR per row 70, so there is no final-row ANSI hotspot. Earlier compact-mode output for the same sample measured median 3 and max 143.
  • What was not tested: Live openclaw channels login --channel whatsapp phone scan was not run here; proof covers the shared terminal renderer and all updated call sites.

Tests run

  • pnpm test src/media/qr-terminal.test.ts src/media/qr-terminal.render.test.ts extensions/whatsapp/src/login.coverage.test.ts -- --reporter=verbose
  • pnpm check:changed via Blacksmith Testbox tbx_01krdbswvw1amcfvsccjbzrzvy, run https://github.com/openclaw/openclaw/actions/runs/25716140762
  • git diff --check origin/main...HEAD

Out of scope

  • Upstream patch to node-qrcode for small: true.
  • Optional env/config toggle for compact vs full-block rendering.
  • Changing behavior for callers that explicitly pass { small: true }.

  • Mark as AI-assisted (tooling used for implementation and validation; proof command run locally on this branch)

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. channel: whatsapp-web Channel integration: whatsapp-web cli CLI command changes channel: feishu Channel integration: feishu size: XS and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 5, 2026
@neeravmakwana neeravmakwana marked this pull request as ready for review May 5, 2026 12:58
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 5, 2026
@clawsweeper

clawsweeper Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR changes the shared terminal QR helper to default to non-compact qrcode terminal output, removes compact-mode overrides from WhatsApp, Feishu, and openclaw qr call sites, and adds ANSI-density regression coverage plus a changelog entry.

Reproducibility: yes. at source level: current main defaults and call sites use compact qrcode terminal rendering, and qrcode@1.5.4 small-mode source emits reset/setup sequences through its per-cell palette path. The PR body also includes copied live renderer output showing the branch removes the SGR-count hotspot.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix copied terminal output from the real renderQrTerminal helper and an observed improved median/max SGR result for the changed behavior.

Next step before merge
No repair lane is needed because the PR already implements the narrow source fix and this review did not find a concrete defect for automation to repair.

Security
Cleared: The diff only changes QR stdout formatting, tests, and changelog text; it does not broaden credentials, dependency resolution, workflows, install scripts, or release surfaces.

Review details

Best possible solution:

Merge a narrow version of this PR after normal maintainer review and CI, keeping full-size terminal QR as the default while preserving explicit compact opt-in for callers that need it.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main defaults and call sites use compact qrcode terminal rendering, and qrcode@1.5.4 small-mode source emits reset/setup sequences through its per-cell palette path. The PR body also includes copied live renderer output showing the branch removes the SGR-count hotspot.

Is this the best way to solve the issue?

Yes. Defaulting to the upstream documented non-compact terminal mode is the narrowest maintainable fix for the broken output, and it avoids adding a new config knob while leaving { small: true } available for explicit compact-mode callers.

What I checked:

  • current-main compact default: Current main passes small: opts.small ?? true to qrcode.toString(..., { type: "terminal" }), so omitted QR options still use compact terminal rendering. (src/media/qr-terminal.ts:9, 0c977cd68793)
  • current-main compact call sites: Current main also explicitly passes { small: true } in WhatsApp login/session QR output, Feishu app-registration QR output, and the openclaw qr ASCII renderer. (0c977cd68793)
  • PR default change: The PR head changes the shared helper default to small: opts.small ?? false, while preserving explicit { small: true } opt-in behavior. (src/media/qr-terminal.ts:9, 37defed085aa)
  • PR regression test: The PR adds a real qrcode runtime test that renders a WhatsApp-style sample and asserts the maximum non-empty row SGR count stays within a bounded multiple of the median row count. (src/media/qr-terminal.render.test.ts:18, 37defed085aa)
  • upstream renderer contract: qrcode@1.5.4 dispatches options.small to terminal-small; the README documents small as terminal-only with default false, and the small renderer palette includes reset/foreground/setup sequences emitted per rendered cell.
  • real behavior proof in PR body: The PR body includes copied branch output from renderQrTerminal for a WhatsApp-style URL showing median SGR count 70 and max 70, compared with current main's reported median around 3 and max around 143 for the same sample. (37defed085aa)

Likely related people:

  • vincentkoc: Path history shows fix(qr): replace qrcode-terminal with qrcode-tui introduced the shared renderQrTerminal helper and compact default, and later WhatsApp login coverage maintenance touched the same flow. (role: introduced shared QR helper and recent WhatsApp maintainer; confidence: high; commits: ea25d7ed5bd6, 071db2ca6940, 841eb81baf37; files: src/media/qr-terminal.ts, src/media/qr-runtime.ts, extensions/whatsapp/src/login.coverage.test.ts)
  • steipete: Path history shows the dependency-trimming commit switched the helper to root-owned qrcode@1.5.4, and a recent WhatsApp commit routed login QR output through the runtime helper. (role: recent QR dependency/runtime and WhatsApp QR maintainer; confidence: high; commits: 7e5d6dba8001, 9efbae7acda4, db06fcd990ff; files: src/media/qr-terminal.ts, src/media/qr-runtime.ts, package.json)
  • mazhe-nerd: Path history shows the Feishu app-registration QR flow was introduced in the Feishu onboarding PR, which is one of the call sites this PR updates. (role: adjacent Feishu QR registration owner; confidence: medium; commits: 9e2ac8a1cb9a; files: extensions/feishu/src/app-registration.ts)

Remaining risk / open question:

  • The PR body does not include an end-to-end live WhatsApp phone scan; the proof covers the actual QR renderer output rather than full channel login.
  • Default terminal QR output becomes taller for every default renderQrTerminal surface, including Feishu and openclaw qr, though explicit compact opt-in remains available.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 0c977cd68793.

Re-review progress:

Avoid node-qrcode compact (small) terminal mode, which emits a dense
ANSI final row that breaks scanning on some terminals.

Covers WhatsApp/Feishu login flows and the pairing QR CLI path.

Co-authored-by: Cursor <cursoragent@cursor.com>
@steipete steipete force-pushed the fix/77820-terminal-qr-ansi branch from 37defed to eef693a Compare May 12, 2026 05:54
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 12, 2026
@steipete

Copy link
Copy Markdown
Contributor

Landing proof for rebased head eef693aae3c41f0d91ae05fee0f5bc8a49b9fe4d.

Commands run:

git diff --check origin/main...HEAD
pnpm test src/media/qr-terminal.test.ts src/media/qr-terminal.render.test.ts extensions/whatsapp/src/login.coverage.test.ts -- --reporter=verbose
pnpm exec tsx -e '<render ./src/media/qr-terminal.ts sample QR and count ANSI SGR rows>'
pnpm check:changed

Proof:

  • Targeted Vitest: 3 shards passed, 7 tests passed.
  • Renderer probe: {"behavior":"default full-block terminal QR","environment":"darwin node v26.0.0","rows":35,"medianSgrPerRow":70,"maxSgrPerRow":70,"ratio":1}.
  • pnpm check:changed: passed on Blacksmith Testbox tbx_01krdbswvw1amcfvsccjbzrzvy; run https://github.com/openclaw/openclaw/actions/runs/25716140762.
  • PR CI on eef693aae3c41f0d91ae05fee0f5bc8a49b9fe4d: green for CI, CodeQL/Critical Quality, OpenGrep, and current Real behavior proof. Stale cancelled/failed proof attempts were from pre-body-update runs and are superseded by the passing Real behavior proof checks.

Known proof gap: live WhatsApp phone scan was not run; proof covers the shared terminal QR renderer and all updated call sites.

Thanks @neeravmakwana and @KrasimirKralev.

@steipete steipete merged commit 0ac4837 into openclaw:main May 12, 2026
117 of 120 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Landed via rebase onto main.

Thanks @neeravmakwana and @KrasimirKralev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: feishu Channel integration: feishu channel: whatsapp-web Channel integration: whatsapp-web cli CLI command changes proof: supplied External PR includes structured after-fix real behavior proof. size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 'channels login --channel whatsapp' QR last row malforms due to excessive ANSI escapes (qrcode small=true mode)

2 participants