Fix: large text edits crashed live collab (stack overflow in the text RGA) - #47
Merged
Conversation
… RGA)
A text element of roughly 200KB or more broke live collaboration entirely —
not just for that element. Found while verifying a background agent's report;
reproduced and bisected against the real engine.
CAUSE. tokenize() is per-character, so ~200KB of text is ~200,000 tokens.
applyTxtToState inserted them with
st.toks.splice(idx, 0, ...toks)
and spreading 200k+ values as call ARGUMENTS overflows V8's stack. Measured:
100KB fine, 200KB and up throw RangeError.
WHY IT WAS SO BAD. The throw happens inside diff(), which session.flush()
calls on every debounced edit — so once a document contained a large text
element, flush() threw and NOTHING synced for the rest of the session. Every
subsequent edit, on any slide, was silently lost to collaborators. The failure
is invisible: no error surfaces, peers just stop receiving.
FIX. Splice without a spread — slice/concat instead. Costs one array copy on
an operation that already allocates the token array.
VERIFIED. Repro now passes at 200/300/500KB. Convergence rig SEEDS=300 ALL
PASS (46,402 checks). Plus a targeted large-text convergence test the rig
doesn't cover (it uses short strings): two replicas concurrently editing a
300KB element converge to identical html with BOTH contributions preserved.
Independent of the relay work — this would still break collab with the relay
limits fixed.
nyblnet
added a commit
that referenced
this pull request
Jul 25, 2026
Current: the maintainer reviews every PR before it reaches main, agents included. No auto-merge. Visibility into what the agents produce matters more than throughput while the multi-agent workflow is still being shaken out. Records the supporting config already in place (one required approval, CI as a required status check so a red build cannot merge, admin bypass retained). Also records the FUTURE ACTION discussed but deliberately not taken: when review becomes the bottleneck, consider auto-merging app-zone PRs on green CI — with a permanent human-review exclusion list for the paths where a bad merge is silent or catastrophic (kernel/src/, slides/src/sync/ especially crdt.ts, server/, the release and signing scripts, and anything touching the splice contract or update-manifest shape). Notes that the exclusion list must be enforceable rather than merely documented before auto-merge is enabled, and that the convergence rig is necessary but not sufficient for crdt.ts — it only generates short strings, which is how the large-text overflow in #47 slipped through.
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
1 participant
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.
A text element of roughly 200 KB or more broke live collaboration entirely
— and not just for that element. Surfaced by a background agent while it was
doing unrelated work; I reproduced and bisected it against the real engine
before fixing.
Cause
tokenize()is per-character, so ~200 KB of text is ~200,000 tokens.applyTxtToStateinserted them with:Spreading 200k+ values as call arguments overflows V8's stack. Measured
against the real engine: 100 KB fine, 200 KB and above throw
RangeError.Why it was so damaging
The throw happens inside
diff(), whichsession.flush()calls on everydebounced edit. So once a document contained one large text element,
flush()threw and nothing synced for the rest of the session — everylater edit, on any slide, silently lost to collaborators.
And it's invisible: no error surfaces, peers simply stop receiving. A user
would experience it as "collaboration randomly stopped working."
Fix
Splice without a spread —
slice/concatinstead. One extra array copy on anoperation that already allocates the token array.
Verified
SEEDS=300ALL PASS (46,402 checks) — the mandatory gatefor any
crdt.tschange.short strings): two replicas concurrently editing a 300 KB element converge
to identical html with both contributions preserved.
tsc -bclean.Scope
One line of behaviour change in
crdt.ts, deliberately kept minimal givenit's the highest-risk file in the repo. Independent of the relay work in #43
and the client work in #45 — this would still break collab with both of those
merged.
Worth noting the rig didn't catch this because it only generates short
strings. A follow-up worth considering: add a large-text case to
scripts/test-sync.tsso the gate covers this class.