Skip to content

fix(gateway): keep close reasons valid UTF-8#100045

Closed
MonkeyLeeT wants to merge 1 commit into
openclaw:mainfrom
MonkeyLeeT:codex/fix-close-reason-utf8
Closed

fix(gateway): keep close reasons valid UTF-8#100045
MonkeyLeeT wants to merge 1 commit into
openclaw:mainfrom
MonkeyLeeT:codex/fix-close-reason-utf8

Conversation

@MonkeyLeeT

@MonkeyLeeT MonkeyLeeT commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #99976

What Problem This Solves

Fixes a gateway close-reason truncation bug where a byte slice could split a multibyte UTF-8 character and decode into a replacement character. That replacement could garble the close reason and re-encode over the WebSocket close-frame byte budget.

Why This Change Was Made

Gateway close-reason truncation now chooses a valid UTF-8 code point boundary at or before the byte limit before decoding the sliced buffer. The change keeps the existing fallback behavior for empty close reasons and preserves ASCII close reasons that already fit.

User Impact

Non-ASCII gateway close reasons stay readable and remain within the RFC-safe byte limit.

Evidence

  • Before/after byte repro with node --import tsx --input-type=module, comparing the exact origin/main truncation expression with this branch's truncateCloseReason:
    • Split UTF-8 input: 136 bytes.
    • origin/main behavior: tail xxxxxxx�, bytes: 121, containsReplacement: true.
    • This branch, split-boundary case: tail xxxxxxxx, bytes: 118, containsReplacement: false.
    • This branch, exact-boundary case: tail xxxxxx😀, bytes: 120, containsReplacement: false.
    • Empty fallback remains invalid handshake.
  • Runtime path checked:
    • src/gateway/server/ws-connection/message-handler.ts:750, :886, :1003, :1546, and :1886 all pass gateway handshake/authorization close reasons through truncateCloseReason.
  • node scripts/run-vitest.mjs src/gateway/server/close-reason.test.ts
    • Passed: Test Files 4 passed (4), Tests 20 passed (20).
  • node scripts/run-vitest.mjs src/gateway/server.auth.default-token.test.ts
    • Passed full default auth-token gateway suite: Test Files 2 passed (2), Tests 38 passed (38).
  • node scripts/run-vitest.mjs src/gateway/client.test.ts -t "gateway rejects"
    • Passed selected client rejection handling: Test Files 2 passed (2), Tests 4 passed | 118 skipped (122).
  • git diff --check origin/main...HEAD
    • Passed with no whitespace errors.
  • .agents/skills/autoreview/scripts/autoreview --mode local
    • Single draft-PR autoreview round run before commit; passed with autoreview clean: no accepted/actionable findings reported and overall: patch is correct (0.93).
  • Remote proof note:
    • GitHub Real behavior proof run 28715260183, job 85155380858, succeeded, but only verified PR problem/evidence text (External PR includes problem context and evidence.); it did not execute this PR's code, so it is not counted as behavioral proof.
    • Blacksmith Testbox-through-Crabbox changed gate was attempted but blocked locally because the blacksmith executable is not installed.
    • AWS Crabbox changed gate was attempted but blocked because this machine is not logged into the OpenClaw Crabbox broker.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: S labels Jul 4, 2026
@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: this PR is a valid draft candidate for the linked gateway UTF-8 close-reason bug, but #100047 now targets the same helper, is open and non-draft, is mergeable/maintainer-editable, and has positive terminal screenshot proof, making it the safer canonical landing path.

Root-cause cluster
Relationship: superseded
Canonical: #100047
Summary: This PR and #100047 are duplicate candidate fixes for the same gateway close-reason UTF-8 bug; the latter is the better canonical landing path because it is open, non-draft, mergeable, maintainer-editable, and proof-positive.

Members:

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

Canonical path: Consolidate review and landing on the single proof-positive candidate at #100047, then close the linked issue after one UTF-8-safe helper fix merges.

So I’m closing this here and keeping the remaining discussion on #100047.

Review details

Best possible solution:

Consolidate review and landing on the single proof-positive candidate at #100047, then close the linked issue after one UTF-8-safe helper fix merges.

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

Yes. Current main still uses raw buf.subarray(0, maxBytes).toString(), and a focused Node command using the same expression with 118 ASCII bytes plus emoji produced 121 bytes ending in U+FFFD.

Is this the best way to solve the issue?

Yes for the helper-level fix, but this PR is no longer the best landing path. The same issue is covered by a non-draft, proof-positive duplicate PR that can carry the maintainer review instead.

Security review:

Security review cleared: The diff only changes a gateway string truncation helper and colocated tests; it does not touch secrets, auth policy, dependencies, workflows, package resolution, or other code-execution surfaces.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • steipete: GitHub commit metadata shows fix: surface handshake reasons and the later module split that extracted src/gateway/server/close-reason.ts were authored/committed by steipete. (role: introduced behavior and helper path; confidence: high; commits: 146f7ab43325, bcbfb357bec7; files: src/gateway/server.ts, src/gateway/server/close-reason.ts, src/gateway/server/ws-connection/message-handler.ts)
  • vincentkoc: Current shallow blame on the helper points at a recent gateway-adjacent refresh commit, and recent message-handler history includes gateway auth/origin-adjacent changes in this area. (role: recent adjacent gateway contributor; confidence: medium; commits: 46ad3f9ae94c, 66d8117d4483, cf04b2463255; files: src/gateway/server/close-reason.ts, src/gateway/server/ws-connection/message-handler.ts)

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

@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. P2 Normal backlog priority with limited blast radius. labels Jul 4, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 4, 2026
Merged
@MonkeyLeeT MonkeyLeeT closed this Jul 4, 2026
@MonkeyLeeT
MonkeyLeeT deleted the codex/fix-close-reason-utf8 branch July 4, 2026 23:50
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

gateway Gateway runtime P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S 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.

[Bug]: truncateCloseReason can exceed its byte cap and emit U+FFFD mojibake when cutting mid-UTF-8 sequence

1 participant

@MonkeyLeeT