fix(irc): long non-ASCII messages are silently truncated at the 512-byte line limit#99138
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 5:03 PM ET / 21:03 UTC. Summary PR surface: Source +11, Tests +177. Total +188 across 2 files. Reproducibility: yes. from source and PR proof: current main can send 350 CJK characters in one PRIVMSG, which exceeds the IRC 512-character line limit once framing and CR-LF are included. I did not rerun the socket proof locally in this read-only review. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the focused Do we have a high-confidence way to reproduce the issue? Yes, from source and PR proof: current main can send 350 CJK characters in one PRIVMSG, which exceeds the IRC 512-character line limit once framing and CR-LF are included. I did not rerun the socket proof locally in this read-only review. Is this the best way to solve the issue? Yes. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 51771c3a1485. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +11, Tests +177. Total +188 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
bb1c0ca to
2eeba99
Compare
2eeba99 to
8851d3f
Compare
de61a67 to
8613446
Compare
8613446 to
ed241fb
Compare
…es are not truncated IRC caps a full protocol line at 512 bytes, but sendPrivmsg split outbound text by UTF-16 code units against the 350 char default. Multi-byte text such as CJK or emoji produced lines far beyond 512 bytes and servers silently dropped the tail of every oversized chunk. The chunker now also enforces a UTF-8 byte budget derived from the line framing overhead, splitting on code point boundaries and preferring spaces, while ASCII chunking is unchanged.
The colocated node:net import tripped the network-runtime-boundary PR diff scan for extensions/irc/src. The loopback server now lives in test/helpers, outside the scanned network runtime paths, and the CJK, emoji, and ASCII chunking cases keep driving the real client over a real TCP socket.
…elper to irc test-support The byte budget is now derived only from the 512-byte line limit and framing overhead, independent of messageChunkMaxChars, so low character caps with multibyte text keep advancing instead of dropping the message. The loopback IRC server helper moves from test/helpers to the extension-local package-root test-support surface so extension tests stay off repo helper bridges and raw socket use stays outside extensions/irc/src.
ed241fb to
0e6be40
Compare
|
Land-ready proof for exact head 0e6be40:
Before: a payload could satisfy the character cap yet exceed the IRC byte limit and be truncated or rejected. After: one selector bounds both limits and the loopback receiver proves lossless reconstruction. Known proof gap: no public IRC network was used; the production client path was tested through a real TCP socket with an inline protocol server, avoiding external-network noise while validating the exact wire bytes. |
|
Merged via squash.
|
…yte line limit (openclaw#99138) * fix(irc): chunk PRIVMSG by UTF-8 byte budget so long non-ASCII messages are not truncated IRC caps a full protocol line at 512 bytes, but sendPrivmsg split outbound text by UTF-16 code units against the 350 char default. Multi-byte text such as CJK or emoji produced lines far beyond 512 bytes and servers silently dropped the tail of every oversized chunk. The chunker now also enforces a UTF-8 byte budget derived from the line framing overhead, splitting on code point boundaries and preferring spaces, while ASCII chunking is unchanged. * test(irc): move loopback IRC server helper to shared test helpers The colocated node:net import tripped the network-runtime-boundary PR diff scan for extensions/irc/src. The loopback server now lives in test/helpers, outside the scanned network runtime paths, and the CJK, emoji, and ASCII chunking cases keep driving the real client over a real TCP socket. * fix(irc): decouple byte budget from character cap and move loopback helper to irc test-support The byte budget is now derived only from the 512-byte line limit and framing overhead, independent of messageChunkMaxChars, so low character caps with multibyte text keep advancing instead of dropping the message. The loopback IRC server helper moves from test/helpers to the extension-local package-root test-support surface so extension tests stay off repo helper bridges and raw socket use stays outside extensions/irc/src. * fix(irc): enforce UTF-8 wire limits * test(irc): satisfy loopback harness lint * test(irc): avoid implicit Promise return * test(irc): handle loopback close errors * fix(irc): preserve boundary word splitting --------- Co-authored-by: Peter Steinberger <[email protected]>
Related: #42121
What Problem This Solves
Fixes an issue where long IRC replies written mostly in multi-byte scripts (CJK, Cyrillic, emoji) arrive cut off, losing a large part of every chunk with no error. The outbound chunker in
sendPrivmsgsplits text into chunks of 350 UTF-16 code units, but IRC caps a protocol line at 512 bytes including framing and CRLF (RFC 2812 section 2.3). 350 CJK characters encode to 1050 UTF-8 bytes, so each wire line is over 1000 bytes and the server silently truncates everything past byte 512. Issue #42121 reported this exact symptom for Chinese text; it was closed for the block-streaming delivery behavior, while the protocol-level byte overflow remained on main (extensions/irc/src/client.ts:233).Why This Change Was Made
sendPrivmsgnow enforces a UTF-8 byte budget per chunk in addition to the existing character split. The budget is512 - byteLength("PRIVMSG <target> :\r\n"), derived only from the protocol line limit and the per-target framing overhead, and is deliberately independent ofmessageChunkMaxChars: the configured character cap keeps its meaning as a presentation preference applied by the first split, and the byte fit afterwards only enforces the wire limit. Because any single code point (at most 4 UTF-8 bytes) always fits the line budget, chunking always advances, including under very low character caps. The byte fit walks code points, so it never splits inside a surrogate pair, and it prefers breaking at the last space when one exists past half the fitted run, mirroring the existing character-split heuristic. ASCII behavior is unchanged (350 ASCII chars are 350 bytes, within budget).sendPrivmsgis the single protocol choke point: the persistent monitor client and transient sends (extensions/irc/src/send.ts,extensions/irc/src/monitor.ts) both route through it. The upstream markdown chunker (textChunkLimit: 350inextensions/irc/src/outbound-base.ts) splits by characters for presentation and cannot know per-target framing overhead, so the protocol boundary is the right owner and no other surface needs changes.The loopback IRC server used by the regression tests lives at
extensions/irc/test-support.ts, the extension-local package-root test-support surface (same pattern asextensions/browser/test-support.ts), keeping raw socket use out ofextensions/irc/srcruntime paths and off the repo test-helper bridges that extension tests must not import.Open PR #96572 touches the same loop but fixes a different defect (lone surrogates when the 350-code-unit split lands inside a surrogate pair) and adds no byte limit. The two changes are independent and compose; an adjacent-line rebase overlap is possible.
User Impact
Long IRC replies in CJK, Cyrillic, emoji, or other non-ASCII text now arrive complete instead of losing the tail of every oversized chunk. ASCII messages chunk exactly as before, and operator-configured
messageChunkMaxCharskeeps its meaning, including low caps with multibyte text.Evidence
Live before/after driving the real production
connectIrcClient(the same client used by the IRC channel's persistent and transient send paths) over a real loopback TCP socket. The only substituted component is the remote IRC server: a minimal loopback ircd that accepts registration and enforces the RFC 2812 512-byte line cap by truncating over-long lines exactly like a public network server. The client code, sanitizers, chunking, and socket I/O are all real, and both runs send the identical 900-character CJK message.Regression tests (
extensions/irc/src/client.test.ts) drive the same real client against a real loopback TCP server (extensions/irc/test-support.ts) with no mocks:messageChunkMaxChars: 100with 250 CJK chars: chunk shape [100, 100, 50], demonstrating the character cap is honored and not shrunk to the byte budget.messageChunkMaxChars: 2with CJK text: chunking still advances ([2, 2, 2, 2, 2]) even though the cap is below one code point's UTF-8 byte count.Validation on the changed files:
node scripts/run-vitest.mjs extensions/irc/src/client.test.ts: 14 passed with the patch; the byte-limit and low-cap cases fail without it.node scripts/run-vitest.mjs test/extension-test-boundary.test.ts: 11 passed (the suite that flags banned repo test-helper bridge imports).node --import tsx scripts/check-no-extension-test-core-imports.ts: OK.extensions/irc/src: no matches.node scripts/run-oxlint.mjs,oxfmt --check, andtsgoextensions production and test lanes: clean.What was not tested: no session against a public internet IRC network, and the full build was not run (the change is internal to the IRC plugin).