Skip to content

fix(irc): long non-ASCII messages are silently truncated at the 512-byte line limit#99138

Merged
steipete merged 8 commits into
openclaw:mainfrom
yetval:fix/irc-privmsg-byte-limit
Jul 5, 2026
Merged

fix(irc): long non-ASCII messages are silently truncated at the 512-byte line limit#99138
steipete merged 8 commits into
openclaw:mainfrom
yetval:fix/irc-privmsg-byte-limit

Conversation

@yetval

@yetval yetval commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

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 sendPrivmsg splits 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

sendPrivmsg now enforces a UTF-8 byte budget per chunk in addition to the existing character split. The budget is 512 - byteLength("PRIVMSG <target> :\r\n"), derived only from the protocol line limit and the per-target framing overhead, and is deliberately independent of messageChunkMaxChars: 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).

sendPrivmsg is 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: 350 in extensions/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 as extensions/browser/test-support.ts), keeping raw socket use out of extensions/irc/src runtime 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 messageChunkMaxChars keeps 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.

# BEFORE (pristine main a39b07bb15)
[BEFORE pristine main a39b07bb15] sent 1 message: 900 CJK chars (2700 UTF-8 bytes)
[BEFORE pristine main a39b07bb15] server received 3 PRIVMSG line(s):
  wire line 1070 bytes -> EXCEEDS 512, server truncated to 512
  wire line 1070 bytes -> EXCEEDS 512, server truncated to 512
  wire line 620 bytes -> EXCEEDS 512, server truncated to 512
[BEFORE pristine main a39b07bb15] text delivered to #general: 492/900 chars, 408 chars lost, intact=false

# AFTER (this patch, identical inputs)
[AFTER this patch, identical inputs] sent 1 message: 900 CJK chars (2700 UTF-8 bytes)
[AFTER this patch, identical inputs] server received 6 PRIVMSG line(s):
  wire line 512 bytes -> within 512, accepted whole
  wire line 512 bytes -> within 512, accepted whole
  wire line 512 bytes -> within 512, accepted whole
  wire line 512 bytes -> within 512, accepted whole
  wire line 512 bytes -> within 512, accepted whole
  wire line 260 bytes -> within 512, accepted whole
[AFTER this patch, identical inputs] text delivered to #general: 900/900 chars, 0 chars lost, intact=true

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:

  • CJK: every line fits 512 bytes and the full text is delivered.
  • Emoji: byte limit honored with no lone surrogates per line.
  • ASCII: chunk shape stays exactly [350, 350, 200], demonstrating preserved behavior.
  • messageChunkMaxChars: 100 with 250 CJK chars: chunk shape [100, 100, 50], demonstrating the character cap is honored and not shrunk to the byte budget.
  • messageChunkMaxChars: 2 with 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.
  • The Critical Quality network boundary diff grep run locally against this branch's added lines under extensions/irc/src: no matches.
  • node scripts/run-oxlint.mjs, oxfmt --check, and tsgo extensions 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).

@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 5:03 PM ET / 21:03 UTC.

Summary
This PR modifies connectIrcClient.sendPrivmsg to enforce a per-target 512-byte UTF-8 IRC line budget while preserving character-cap chunking, with loopback IRC regression tests.

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.

  • IRC wire budget: 1 hard 512-byte PRIVMSG line cap added. This changes outbound chunk boundaries, so maintainers should notice the preserved ASCII and character-cap coverage before merge.

Root-cause cluster
Relationship: canonical
Canonical: #99138
Summary: This PR is the canonical open patch for the IRC outbound PRIVMSG byte-overflow defect; the closed Chinese cut-off issue is overlapping evidence, and the merged surrogate PR is adjacent Unicode chunking work.

Members:

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Risk before merge

  • [P1] This PR changes the IRC outbound chunking choke point, so a bad merge could alter delivered message boundaries or content even with green CI; the CJK, emoji, ASCII, and low-cap proof should remain part of the merge decision.

Maintainer options:

  1. Land with current proof (recommended)
    Accept the IRC message-delivery risk if current CI finishes cleanly because the patch exercises the real client path and preserves the key existing chunk shapes.
  2. Ask for wider transport proof
    Require a public-network IRC smoke only if maintainers want confidence beyond the loopback server's protocol-limit proof.

Next step before merge

  • No ClawSweeper repair is needed; the remaining action is ordinary maintainer review and exact-head CI on this open PR.

Security
Cleared: Cleared: the diff changes IRC chunking and tests only, with no dependency, workflow, secret, permission, package-resolution, or new production execution surface.

Review details

Best possible solution:

Merge the focused sendPrivmsg byte-budget fix after maintainer review and current CI, keeping the regression tests that prove CJK, emoji, ASCII, and low character-cap behavior.

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. sendPrivmsg is the maintainable protocol boundary because transient sends and monitored replies both route through it, while upstream markdown chunking cannot know the target-specific IRC framing budget.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 message-delivery: The patch changes how every IRC PRIVMSG is chunked before send, so a regression could change delivered message boundaries or content.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after terminal output from the real connectIrcClient over loopback TCP showing 900 CJK characters truncated on main and delivered fully after the patch.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes before/after terminal output from the real connectIrcClient over loopback TCP showing 900 CJK characters truncated on main and delivered fully after the patch.

Label justifications:

  • P2: The PR fixes a real IRC message-delivery bug for long non-ASCII replies, but the blast radius is limited to the IRC plugin.
  • merge-risk: 🚨 message-delivery: The patch changes how every IRC PRIVMSG is chunked before send, so a regression could change delivered message boundaries or content.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes before/after terminal output from the real connectIrcClient over loopback TCP showing 900 CJK characters truncated on main and delivered fully after the patch.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after terminal output from the real connectIrcClient over loopback TCP showing 900 CJK characters truncated on main and delivered fully after the patch.
Evidence reviewed

PR surface:

Source +11, Tests +177. Total +188 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 34 23 +11
Tests 1 178 1 +177
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 212 24 +188

What I checked:

  • Current main behavior: Current main normalizes the target/text and chunks only when chunk.length > messageChunkMaxChars before writing PRIVMSG, so byte length is not enforced on the existing send path. (extensions/irc/src/client.ts:220, 51771c3a1485)
  • PR head implementation: The PR head adds IRC_MAX_LINE_BYTES, walks code points in takeIrcPrivmsgChunk, computes maxChunkBytes from PRIVMSG <target> :\r\n, and sends only fitted chunks. (extensions/irc/src/client.ts:15, 0e6be40305ae)
  • Single send choke point: Both transient sends and monitored inbound replies call client.sendPrivmsg, so fixing the byte budget inside connectIrcClient covers the IRC protocol write boundary. (extensions/irc/src/send.ts:90, 51771c3a1485)
  • Regression coverage: The PR head tests CJK, emoji, ASCII, low character caps, and one-unit astral-character caps through the real client against a loopback TCP IRC server. (extensions/irc/src/client.test.ts:180, 0e6be40305ae)
  • IRC protocol contract: RFC 2812 says IRC messages are CR-LF terminated lines that must not exceed 512 characters including the trailing CR-LF, leaving 510 for command and parameters. (rfc-editor.org)
  • Related closed issue: The related Chinese IRC cut-off report was closed for the block-streaming path, but its visible symptom overlaps the byte-overflow behavior this PR now targets.

Likely related people:

  • steipete: Git history shows commit 35272d0 introduced the current IRC client, send, monitor, and outbound-base files, and this PR is currently assigned to steipete with maintainer follow-up commits on the branch. (role: introduced behavior and recent area contributor; confidence: high; commits: 35272d01b5dd, a50373d72380, 0e6be40305ae; files: extensions/irc/src/client.ts, extensions/irc/src/send.ts, extensions/irc/src/monitor.ts)
  • llagy009: The merged surrogate-boundary PR changed the same sendPrivmsg loop and added adjacent regression coverage for Unicode chunking. (role: recent adjacent contributor; confidence: medium; commits: 685c22dfb7e1; files: extensions/irc/src/client.ts, extensions/irc/src/client.surrogate.test.ts)
  • vignesh07: CONTRIBUTING.md lists Vignesh for IRC ownership, which makes this a likely review-routing candidate even though the central blame trail is steipete/llagy009. (role: listed IRC area owner; confidence: medium; files: CONTRIBUTING.md)
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-02T16:27:36.568Z sha bb1c0ca :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 2, 2026
@yetval
yetval force-pushed the fix/irc-privmsg-byte-limit branch from bb1c0ca to 2eeba99 Compare July 2, 2026 17:00
@steipete steipete self-assigned this Jul 5, 2026
@steipete
steipete force-pushed the fix/irc-privmsg-byte-limit branch from 2eeba99 to 8851d3f Compare July 5, 2026 20:37
@steipete
steipete force-pushed the fix/irc-privmsg-byte-limit branch 2 times, most recently from de61a67 to 8613446 Compare July 5, 2026 20:47
@clawsweeper clawsweeper Bot removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@steipete
steipete force-pushed the fix/irc-privmsg-byte-limit branch from 8613446 to ed241fb Compare July 5, 2026 20:55
…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.
yetval and others added 7 commits July 5, 2026 21:57
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.
@steipete
steipete force-pushed the fix/irc-privmsg-byte-limit branch from ed241fb to 0e6be40 Compare July 5, 2026 20:57
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 5, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Land-ready proof for exact head 0e6be40:

  • Rewrote the original two-pass character/byte slicing into one code-point-aware selector that enforces the configured character cap and remaining UTF-8 wire budget together.
  • Preserved the existing word-boundary behavior when the delimiter is exactly at the cap, plus forward progress for a one-unit cap followed by an astral code point.
  • Exercised the production IRC client against a real TCP loopback server. CJK, emoji, ASCII, and low-cap payloads reconstruct exactly; every emitted PRIVMSG line is at most 512 bytes including CRLF, matching the Modern IRC wire limit: https://modern.ircdocs.horse/
  • Sanitized AWS Crabbox run run_6a15fdf8be57 passed 20 focused tests and the changed-surface extension/test typechecks, lint, dependency guards, import-cycle check, and database-first guard: https://crabbox.openclaw.ai/portal/runs/run_6a15fdf8be57
  • Exact remote commands: pnpm test extensions/irc/src/client.test.ts extensions/irc/src/client.surrogate.test.ts; pnpm check:changed -- extensions/irc/src/client.ts extensions/irc/src/client.test.ts extensions/irc/src/client.surrogate.test.ts
  • Fresh exact-head autoreview: clean, no accepted/actionable findings (0.86).
  • GitHub CI: no relevant failures or pending checks at this head. Native review artifacts validate READY FOR /prepare-pr.

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.

@steipete
steipete merged commit 89881b6 into openclaw:main Jul 5, 2026
119 of 124 checks passed
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 6, 2026
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: irc merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants