Skip to content

fix(qa-matrix): keep shared reply previews UTF-16 safe#102656

Merged
steipete merged 2 commits into
openclaw:mainfrom
Pandah97:fix/qamatrix-shared-truncate-utf16
Jul 9, 2026
Merged

fix(qa-matrix): keep shared reply previews UTF-16 safe#102656
steipete merged 2 commits into
openclaw:mainfrom
Pandah97:fix/qamatrix-shared-truncate-utf16

Conversation

@Pandah97

@Pandah97 Pandah97 commented Jul 9, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

The shared Matrix QA reply artifact bounded its trimmed body preview with raw UTF-16 code-unit slicing. A preview boundary through an emoji or another supplementary character could store malformed evidence.

Why This Change Was Made

buildMatrixReplyArtifact, used across room, media, restart, and E2EE scenarios, now calls the public plugin SDK truncateUtf16Safe helper. The reviewed branch is rebased onto current main and retains the existing distinction between a missing body (undefined) and a present whitespace-only body ("").

User Impact

Matrix QA evidence remains valid Unicode at the 200-unit preview cap. Scenario matching, full observed events, token checks, and empty-body artifact semantics are unchanged.

Evidence

  • Focused shared scenario-runtime suite passed.
  • Exact regression covers an emoji crossing the 200-unit boundary and the prior whitespace-only artifact behavior.
  • Focused oxfmt and oxlint passed; git diff --check passed.
  • Fresh branch autoreview found no accepted or actionable findings (0.99 confidence).

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 8:21 AM ET / 12:21 UTC.

Summary
The PR replaces Matrix QA reply artifact preview slicing with truncateUtf16Safe and adds a regression test for surrogate-boundary and whitespace-only reply bodies.

PR surface: Source +1, Tests +17. Total +18 across 2 files.

Reproducibility: yes. Source inspection shows current main can split a surrogate pair when replyBody?.slice(0, 200) cuts after the first code unit of an emoji, for example 199 ASCII code units followed by a supplementary character.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #102656
Summary: This PR is the viable open implementation path for the shared Matrix reply preview UTF-16 fix; the related duplicate is closed unmerged.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post a redacted Matrix QA run artifact, runtime log, terminal output, or transcript that shows the boundary-emoji reply preview after this head; redact private details such as IPs, tokens, phone numbers, and non-public endpoints.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports focused tests and CI-style checks only; before merge, add a redacted Matrix QA runtime artifact, log, terminal output, or transcript showing the after-fix preview behavior and update the PR body for re-review.

Risk before merge

  • [P1] Contributor proof is still unit/CI-only; maintainers do not yet have a real Matrix QA artifact, runtime transcript, or redacted log showing the after-fix boundary-emoji preview behavior.

Maintainer options:

  1. Decide the mitigation before merge
    Land the narrow SDK-helper substitution after the contributor adds redacted Matrix QA runtime proof showing the boundary-emoji reply preview remains valid Unicode.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The prior code issue is fixed in the updated head, but ClawSweeper should not queue a repair because the remaining gate is contributor-owned real behavior proof.

Security
Cleared: The diff only imports an existing SDK text helper and adds focused tests; it does not touch dependencies, workflows, secrets, or executable supply-chain surfaces.

Review details

Best possible solution:

Land the narrow SDK-helper substitution after the contributor adds redacted Matrix QA runtime proof showing the boundary-emoji reply preview remains valid Unicode.

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

Yes. Source inspection shows current main can split a surrogate pair when replyBody?.slice(0, 200) cuts after the first code unit of an emoji, for example 199 ASCII code units followed by a supplementary character.

Is this the best way to solve the issue?

Yes. The existing plugin SDK truncateUtf16Safe helper is the right owner-boundary fix for bundled plugin code, and the updated ternary preserves the existing undefined versus empty-string artifact semantics.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f786efddcd88.

Label changes

Label justifications:

  • P3: This is a narrow QA harness artifact edge-case fix with limited user-facing blast radius.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports focused tests and CI-style checks only; before merge, add a redacted Matrix QA runtime artifact, log, terminal output, or transcript showing the after-fix preview behavior and update the PR body for re-review.
Evidence reviewed

PR surface:

Source +1, Tests +17. Total +18 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 18 1 +17
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 20 2 +18

What I checked:

Likely related people:

  • vincentkoc: Blame on the current shared Matrix reply artifact implementation points to 0ac8933721f7e47ad679b06c894f4cbdb55b3d14, authored and committed by this GitHub login. (role: recent area contributor; confidence: high; commits: 0ac8933721f7; files: extensions/qa-matrix/src/runners/contract/scenario-runtime-shared.ts, extensions/qa-matrix/src/runners/contract/scenario-types.ts)
  • gumadeiras: History for the Matrix contract runtime path includes 5042b8b8e37749208f764c8d59cbda8805b20310, which split the Matrix contract runtime that now owns this shared helper. (role: feature origin contributor; confidence: medium; commits: 5042b8b8e377; files: extensions/qa-matrix/src/runners/contract/scenario-runtime-shared.ts, extensions/qa-matrix/src/runners/contract/scenario-types.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-09T10:30:21.123Z sha 05c363b :: needs real behavior proof before merge. :: [P3] Preserve empty body previews

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 9, 2026
Pandah97 and others added 2 commits July 9, 2026 12:58
@Pandah97 @steipete
fix(qa-matrix): keep reply body preview truncation UTF-16 safe
24b4c98 
Replace `.slice(0, 200)` with `truncateUtf16Safe(replyBody, 200)` in
buildMatrixReplyArtifact to prevent surrogate pair corruption.

Ref. lsr911 pattern — mechanical substitution, no behavior change.
@steipete
test(qa-matrix): preserve reply preview semantics
38655d5
@steipete
steipete force-pushed the fix/qamatrix-shared-truncate-utf16 branch from 05c363b to 38655d5 Compare July 9, 2026 12:00
@steipete steipete changed the title fix(qa-matrix): keep reply body preview truncation UTF-16 safe fix(qa-matrix): keep shared reply previews UTF-16 safe Jul 9, 2026
@steipete steipete mentioned this pull request Jul 9, 2026
Closed
7 tasks
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Maintainer proof on exact head 38655d5462df1ec66ebf2dbf51cc8189aebf36e6 closes the requested Matrix runtime gap.

  • Sanitized direct-AWS Crabbox, public networking, no Tailscale, no hydration, and no instance profile.
  • Real bundled Matrix plugin against a disposable local Tuwunel homeserver; content capture disabled and no Matrix/model credentials used.
  • The partial- and quiet-streaming scenarios passed 6/6 checks. Both produced 200-UTF-16-unit reply previews that were well-formed and contained no replacement character: https://crabbox.openclaw.ai/portal/runs/run_cb348f231e08
  • A same-head exact boundary probe placed an emoji across unit 200. It returned the 199-unit safe prefix with no lone surrogate or replacement character, while preserving full-body token matching and the empty-string versus missing-body distinction.
  • The focused boundary regression passed 2/2.
  • Fresh exact-head autoreview is clean at 0.99.

Only redacted derived results were retained; homeserver state, registration data, account/event IDs, and captured content were excluded. Native exact-head prepare remains the merge gate.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@steipete
steipete merged commit 0e62669 into openclaw:main Jul 9, 2026
95 checks passed
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 9, 2026
Open
@steipete steipete mentioned this pull request Jul 9, 2026
Merged
This was referenced Jul 10, 2026
Closed
Merged
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
@Pandah97 @steipete
fix(qa-matrix): keep shared reply previews UTF-16 safe (openclaw#102656)
902c096 
* fix(qa-matrix): keep reply body preview truncation UTF-16 safe

Replace `.slice(0, 200)` with `truncateUtf16Safe(replyBody, 200)` in
buildMatrixReplyArtifact to prevent surrogate pair corruption.

Ref. lsr911 pattern — mechanical substitution, no behavior change.

* test(qa-matrix): preserve reply preview semantics

---------

Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Pandah97 @steipete