fix(mirror): stop a split multi-byte character corrupting mirrored output - #99
Merged
ralyodio merged 1 commit intoJul 31, 2026
Conversation
…tput
followFile decoded each poll slice with toString("utf8"), but a slice
boundary lands wherever the poll caught the file — regularly in the middle
of a multi-byte character. Both the 65536-byte read slice and the gap
between two ticks split characters, and each half decoded alone becomes
U+FFFD.
Engines draw their full-screen UI out of box-drawing characters, three
UTF-8 bytes each, so any mirrored session that ran past the read slice
showed replacement characters instead of a frame.
Decode through a StringDecoder so an incomplete tail is held back until
the rest of it arrives, reset it when a replaced transcript forces a
resync, and flush it on stop so a genuinely truncated tail is still
reported rather than swallowed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
followFilehands the mirror whatever bytes a poll tick happened to catch, then decodes each slice on its own withtoString("utf8"). That boundary lands wherever it lands, and regularly that is in the middle of a multi-byte character. Each half decodes to U+FFFD, so the character is destroyed in both directions.Two boundaries do it:
Buffer.allocUnsafe(Math.min(65536, size - offset)))script -fflushes part of a character and the rest lands after the next pollThis matters because of what engines actually print. A framed TUI is built out of box-drawing characters (
│ ┌ ─ ┤ └), and every one of those is three UTF-8 bytes. Any mirrored session whose output ran past the read slice showed a wall of replacement characters where the frame should be.Reproduced on unmodified main
x* 65535 followed by─(U+2500, bytese2 94 80), so the character's first byte is the last byte of read one:And split across two ticks,
héllowritten ash+ first byte ofé, then the rest:Both round-trip exactly after the fix.
The fix
Decode through
StringDecoderinstead of per-slicetoString. It holds an incomplete tail back until the rest of it arrives. Three small things go with it:stop(), so a genuinely truncated tail (killed child) is still reported rather than swallowedonChunkis skipped when a slice decodes to nothing, which also means thefirstflag inengines.mjsstill marks the real opening slice21 insertions, 3 deletions, and 9 of the added lines are comment.
Tests
New
test/pty-utf8-split.test.mjs, 9 tests.4 are the bug (fail before, pass after): a character straddling the 64KB boundary; a character split across two ticks; a box-drawing UI larger than the read slice; and astral characters (4-byte emoji, 2 code units) with the pad length swept so the boundary walks through every byte of the character.
5 are controls that pass both ways, so the fix cannot buy clean decoding by dropping or reordering output: ASCII still streams in order and drains on stop; a truncated tail is still emitted and not swallowed; a replaced transcript still resyncs; a file that does not exist yet is still picked up;
stop()is still idempotent.Fail-before verified with
git checkout -- src/pty.mjs: 4 fail / 5 pass unpatched, 9/9 patched. Full suite 385 pass, 0 fail.Deliberately not in this PR
stripScriptBannerhas the same class of split. The header regex is anchored to the start of a slice, so ifscript(1)'s banner is ever cut in half by a tick, neither half matches and the bookkeeping line reaches the mirror. Real, but it needs a small buffering decision about how long to hold the opening slice back, which is a design call rather than a fix.statSyncsizes the new file, butreadSyncstill uses the fd opened on the old inode. Reopening on resync would fix it. Separate defect, and a transcript in a per-session temp dir is unlikely to rotate.Happy to do either if you want them.