Keep large bracket-heavy pastes from stalling the server #389
- MA
## Why
A reviewer pasted a large block of text into a document. The server rejected it, then lost its storage writer lease and shut down.
I reproduced this. The cause is not the rejection path or lease timing. It is two quadratic loops that run synchronously on the event loop whenever the server projects a document:
- **micromark (parse):** at every unescaped `]`, the parser looks back through the whole paragraph for an opener. Our serializer escapes `[` but leaves `]` alone, so `room.apply` re-parses prose full of brackets in quadratic time. 32 KB of `]` takes 5.2 s to parse. 128 KB takes minutes. - **mdast-util-to-markdown (serialize):** `safe()` checks each escape with `positions.includes(position)`, so serializing gets quadratic in the number of escaped characters. 128 KB of `]` took 2.3 s per projection, and `apply` projects several times.
During either loop the lease renewal timer can't run. After 25 s the safety watchdog fires and the server stops. The watchdog is doing its job: a process that can't prove it still holds the lease should stop.
The `a|` paste from the report is fast on its own (about 30 ms). In the reviewer's document, the stall came from bracket-heavy content (their earlier tests pasted many `[site](…)` / `` strings as literal text). A single 128 KB paste of `[a]` reproduces it on `main`.
## What changed
- `packages/dialect/src/serialize.ts`: escape every `]` in prose, mirroring the default rule for `[`. The parser no longer runs its lookback on server-produced source. Links, references and footnotes are unchanged. - `patches/mdast-util-to-markdown@2.1.2.patch`: one line. `positions.includes(position)` becomes `Object.hasOwn(infos, position)`. Same behaviour, now linear. - Canonical output changes only for prose that contains `]` (now `\]`). Restore already accepts `serialize(parse(stored))`, so stored documents load unchanged and are rewritten on their next edit.
## Evidence
Two browser contexts on a fake-GitHub server (port 8894), with a preload that logs event-loop stalls. One context pastes 128 KB of `[a]` into a document that has open questions and a comment.
**Before (main):** ``` [dbg] apply 56725ms ok=true n=1 [lag] event loop blocked ~341070ms chopin: storage writer lease was not renewed before its safety deadline chopin: plan persistence failed - StorageError: storage lease chopin:writer is no longer held ``` The second context never received the paste, and the process exited. The reviewer's original log shows the same sequence after `rejected batch: source-too-large`.
**After:** the paste is accepted and reaches the second context within 5 s. A second paste over the limit gets `rejected batch: source-too-large` and the room rebuilds. The longest event-loop stall is 824 ms, and the server keeps running.
`room.apply` on a 128 KB paste, server only:
| Paste | Before | After | |---|---|---| | `[a]` × 64 KB | 12.7 s | 0.65 s | | `]` × 128 KB (refused, too large) | minutes | 0.27 s | | `**a` × 128 KB | 2.1 s | 0.2 s |
## Deploy notes
- **Drop the bun patch when upgrading to mdast-util-to-markdown 2.1.3 or later.** Upstream has the same fix, but 2.1.3 also changes canonical output, so treat that upgrade as its own migration. - **`sourceHash` changes for documents that contain `]`.** Their first projection after deploy writes `\]`, so the hash changes once. Expect a one-time regeneration of the summary and description for those documents, and a spurious `changedSinceReference` on Chat references made before the deploy. - **Anchors stored before deploy.** A block digest computed before deploy won't match a block that contains `]`. This only matters when the anchor's Yjs relative position was already unresolvable (for example after an epoch replacement). Those anchors orphan rather than recover by digest. That case is narrow.
## Testing
- `bun test packages/dialect apps/server/src/plan/room.test.ts`: 148 pass. - New dialect tests check that `]` is escaped and round-trips, that a limit-sized `[a]` paragraph re-parses in under 5 s, and that links and footnotes are unchanged. - New room test runs a heartbeat timer during an accepted and a refused limit-sized bracket paste and asserts no stall over 5 s. Without the fix it fails: longest stall 134 s, test run 358 s. With the fix it passes in 2.4 s. - `bun test` (full): 4065 pass, 0 fail. One earlier full run had a single unrelated failure under heavy machine load (load average about 80) that did not recur. - `bun run ci`, `bun run types`: pass. - Hosted E2E passed on the final rebased head (`eb995b73`), alongside validation and container.
## Not in this PR
- **`
## Paste and projection flow

- MA
**Hold — please don't merge yet.** Changes canonical serialization (escapes `]`) and adds a dependency patch to fix a server event-loop stall that can drop the writer lease. A review is in progress; I'll post the verdict here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
MaggieAppletonhead-ref-force-pushed eb995b7329fblast month on GitHub
- MA
**Review:** Looks good
I checked the patch against upstream, the digest and restore risks, round-trip stability, and a wider set of pathological pastes.
**1. Stored documents and anchors: safe.** I seeded a document with `]` in a heading, prose, task items and a long paragraph on the merge base. It had a Decision card related to two `]` blocks and two open comment passages over `]` text. I then loaded the same Postgres database under this branch. - Restore passes. Nothing was rejected, and the log has no `stored plan source does not match its Yjs checkpoint` or `session:error`. - `Service.open` re-expresses every anchor through its Yjs relative position, which still resolves, so the digests are re-minted in the new format. The anchors are not orphaned under `rebase` or `carry`. The passages still `locate`. - In two browser contexts, comment highlights render. An edit containing `]` reaches the other context and survives a reload. Afterwards the anchors still resolve, including recovery by digest alone onto a fresh epoch. - Rolling back is also safe: `main` accepts `\]` source through `serialize(parse(source))`.
Residual, not blocking: a raw stored digest from before this change can no longer recover by digest *alone*. I confirmed this onto a fresh epoch: main recovers it, this branch orphans it. This only matters for an anchor whose position was already unresolvable when the deploy happens, which in practice means `recoverOnNextEdit` anchors waiting for a deleted `]` block to reappear. Those lose their one-edit recovery window. Narrow enough to accept, but worth a line in the PR description.
**2. Round-trip: stable.** `serialize(parse(x))` is idempotent and leaves the tree unchanged for all of these: inline, full, collapsed and shortcut references; unresolved references; footnote refs and definitions containing `]`; `- [x]` / `- [ ]` task items; code spans (including `` x]`]y ``) and fenced code; inline and block math; tables; headings; emphasis and strikethrough; link text with `\]`; link titles and `<…>` destinations containing `]`; `![alt]]`; `\\]`; `]`. Code, math, destinations and task markers keep a raw `]`.
**3. Patch: semantically identical.** `positions` and `infos` always get the same keys at the same time, so `Object.hasOwn(infos, position)` matches `positions.includes(position)` exactly. Upstream fixed this the same way in **2.1.3** (2026-09-27): `own.call(infos, position)`. Patching is the right call for now, because 2.1.3 also changes serialization: it adds CR/LF escapes, autolink escapes, HTML-kind block joining, and a rewritten `container-phrasing` emphasis encoding. Upgrading would mean a canonical-output migration. Suggestion: add a note that this patch should be dropped once we move to ≥2.1.3.
**4. Other pathological pastes: no remaining stalls.** I timed a single-paragraph paste through `room.apply` at 32, 64 and 128 KB for `]`, `[`, `[a]`, `*`, `_`, `` ` ``, `<`, `\`, `(`, `**a`, `_a`, `*a `, `. With the `UNSAFE` rule removed, both new tests fail, and the room test stalls for 145 s. 148/148 pass on the branch.
Minor: the change also alters `sourceHash` for any document containing `]`. As a one-off after deploy, in-flight summary/description commits will no-op and regenerate, and Chat references created earlier will report `changedSinceReference` for those documents. That's benign, but it could go in the "Canonical output changes" note too.
- MA
Hold lifted.
MaggieAppletonmerged 9800ccf6e424last month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub