fix(realtime): a body cut mid-emoji must not roll back the message - #185
Merged
Conversation
The quote path truncates by UTF-16 code unit — `qr[0].body.slice(0, 240)` — so quoting a message whose 240th code unit is half of a non-BMP character yields a lone surrogate. JSON.stringify emits that as the literal ASCII escape \ud83d, which survives transport byte-for-byte and is then refused by the outbox's cast: ERROR: invalid input syntax for type json DETAIL: Unicode low surrogate must follow a high surrogate. enqueueBroadcast runs INSIDE the caller's transaction, so the failure takes the reply with it: ROLLBACK discards the message row, the sequence bump and the updated_at touch, and the request 500s. Every retry fails identically, so one emoji-bearing message becomes permanently unquotable for every human and every agent in the workspace. This is new in 0.14. The same payload used to go to redis.publish AFTER COMMIT, where a lone surrogate was a cosmetic glyph. Moving it behind a ::jsonb cast inside the transaction turned that into a lost write. messages.body is TEXT and accepts the same bytes, which is why only the outbox half dies. Reproduced against Postgres 16 through enqueueBroadcast itself: SQLSTATE 22P02 on exactly the string router.ts:4133 produces. Scrubbed at the cast rather than at the 24 call sites, so fields added to future events are covered too. The repo already owns the helper — stripLoneSurrogates, whose own comment names body.slice(0, N) as the cause — it had simply never been applied to this boundary. A JSON.stringify replacer visits every string in the tree, so nesting and arrays need no traversal. The regression tests fail with the production error when the scrub is removed. One of them pins the other direction: a well-formed emoji has to survive intact.
Merged
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.
Quoting a message that contains an emoji can destroy the reply and 500.
The quote path truncates by UTF-16 code unit:
If the 240th code unit is half of a non-BMP character, the result ends in a lone surrogate.
JSON.stringifyrenders that as the literal ASCII escape\ud83d, which survives node-postgres' parameter encoding byte-for-byte — and Postgres refuses it:enqueueBroadcastruns inside the caller's transaction, so this doesn't just drop a notification. TheROLLBACKatrouter.ts:4189discards the sender'smessagesrow, theconversation_countersbump and theconversations.updated_attouch, and the request 500s. Every retry fails the same way, so one emoji-bearing message becomes permanently un-quotable — for humans through the API and for agents throughcumora send --quote(server/src/agents/cli.ts:2176, sameslice(0, 240)).Reproduced
Postgres 16, through
enqueueBroadcastitself, with exactly the stringrouter.ts:4133produces:And the asymmetry that makes it a lost write rather than a cosmetic one:
messages.bodyis TEXT and takes the same bytes happily. Only the outbox half dies, and it takes the transaction with it.Why this is new in 0.14
git diff v0.13.2..main -- server/src/api/router.tsshows the same payload used to go out asredis.publishhappened after COMMIT and took any string, so a lone surrogate was a glyph that rendered as�somewhere. Behind a::jsonbcast inside the transaction it became a rollback. This is precisely the ambiguitydocs/decisions/0001-transactional-realtime-outbox.mdset out to remove — the commit is meant to be the only success boundary — so it seemed worth fixing at that boundary rather than at the caller.The fix
Scrub at the cast, not at the 24
enqueueBroadcastcall sites across 8 files, so fields added to future events are covered without anyone remembering to.The helper already exists and its own doc comment names this exact cause:
It was written for the model boundary (
cli.ts,computer/engine.ts) and had simply never been pointed at this one. AJSON.stringifyreplacer visits every string in the tree, so nesting and arrays need no traversal of our own — worth noting because scrubbing the serialized text would not work: by then the surrogate is already the ASCII escape.Verification
realtime-outbox.test.ts. Two fail without the fix, with the production error:main.tsc --noEmit,biome lint ., all three source guards clean.Not changed
The quote card is still cut at 240 code units, so a truncated body can now end one character short of a glyph instead of showing a broken one. Fixing the truncation to be grapheme-aware is a display question, separate from the lost write, and I left it alone.