Skip to content

fix(files): stop the collaborative editor rewriting and reflowing a document on open - #6652

Merged
icecrasher321 merged 3 commits into
stagingfrom
fix/files-editor-open-fidelity
Aug 13, 2026
Merged

fix(files): stop the collaborative editor rewriting and reflowing a document on open#6652
icecrasher321 merged 3 commits into
stagingfrom
fix/files-editor-open-fidelity

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Opening a file rewrote it, and the pane visibly re-laid-out a beat after it painted. Both came from the live document drifting out of the shape its own markdown can describe.

What was happening

ProseMirror appends an empty paragraph to any document that does not end in one. The server seeded the CRDT with the raw parse, so the first client to bind wrote that paragraph back into the shared document — which the relay saw as a real edit and persisted. Every open therefore:

  • uploaded the file under a fresh storage key and deleted the old one, 404ing the page's own in-flight content read (FileNotFoundError on a file you are looking at),
  • bumped contentUpdatedAt, so "Last Updated" moved just from viewing a file,
  • churned a blob write + delete + DB transaction per open.

And because a trailing blank line cannot serialize (postProcessSerializedMarkdown collapses it), the file never recorded that paragraph, so nothing ever reconciled the two — each client that seeded without seeing another's contribution stacked one more. Measured on a real document: 18 stacked empty paragraphs in the live doc against the placeholder's 1, i.e. the pane growing several hundred pixels the instant the live editor took over.

Changes

  • Seed and merge through the editor's own normal form (editorNormalForm in collab-doc/converter.ts), so binding is a no-op and canonicalizeYDoc — which is parse ∘ serialize — collapses an accumulated run back to one. Placed at the collab boundary rather than in parseMarkdownToDoc: only the CRDT has to agree with the editor. Every other consumer of the parse (paste, the round-trip probe, the read-only placeholder, note blocks, skills) renders through a real editor that normalizes itself — putting it in the parse broke 17 tests on exactly those surfaces.
  • Skip a persist whose projection already matches the durable bytes. Byte length is the free reject, so the compare read only happens when a no-op write is actually on the table. On a no-op it reports the file's current version, resyncing a stale If-Match instead of conflicting.
  • Revoke collaborative readiness on a fatal join. The sticky syncedOnce latch outlived the document: after a readiness timeout the provider drops synced so the gate closes, but the latch re-opened it on the offline fallback's seed flag — handing back an editable editor on a document the provider had abandoned, with client autosave gated off because collaboration is nominally on. Keystrokes went nowhere and vanished on reload, with no error shown.
  • Recover from a superseded storage key. A 404 from a content read means the pointer is stale, not that the file is gone: re-resolve the record so the read re-keys onto the current object. Invalidates the record, never the failed query, which is what keeps it loop-proof. And don't focus-refetch durable bytes while the relay owns durability.
  • Prefetch the workspace file list in the layout. The sidebar reads that query on every workspace route, so it registers before any page renders; HydrationBoundary hands an already-seen query to a useEffect that SSR never runs, so a page-level prefetch of that key could not reach the server render. The file route rendered a spinner and disagreed with the client about the header's markup — a hydration mismatch. Verified: the SSR probe went from filesLen:0, hasSelected:false to filesLen:19, hasSelected:true, and the served HTML now carries the real breadcrumb trail instead of the placeholder.
  • Load the document font with display: block. A swap repaints prose in metric-adjusted Arial first, so paragraphs re-wrap when the real face lands — visible on every hard refresh, since that bypasses the font cache.

Also: a detail-route loading.tsx (the segment was inheriting the list chrome — an options bar and table header a document page doesn't have), normalize.ts renamed to field.ts now that it holds only the field constant, and the duplicate COLLAB_DOC_FIELD in the streaming path folded into it.

Verification

Measured in a headless browser against real documents, not just unit tests:

  • placeholder vs live editor on a cold room: 18 blocks vs 18, zero differing positions, no geometry change at the swap (previously 52 vs 69).
  • a warm, edited room joined by a second client: identical again — the accumulation path is closed, not just healed.
  • POST /api/internal/file-doc/seed unaffected at ~25ms.

New regression tests: 6 "binding an editor to the seed changes nothing" cases (list/heading/table/rule/paragraph/blank-line endings), 6 no-op-persist cases including the fail-open paths, 4 stale-storage-key cases, and 2 readiness cases that both return ready: true under the old formula. The parity helper now compares against the shared normal form — it had been asserting a shape neither side renders.

markdown-parse.test.ts's two 400-seed property tests were re-budgeted from 30s to 60s: this branch adds the second one, and at 30s both timed out under whole-suite parallelism while passing standalone.

Full apps/sim suite: 1825 files / 23948 tests, 0 failures. tsc, api-validation, react-query, client-boundary, utils, import-specifiers all clean.

Reveal the live document only once it has settled

collabReady flipped the moment the provider reported synced, but a room's remaining updates can still be in flight — reconnecting to a room edited moments ago, the handshake lands on the base state and the edit arrives milliseconds later. The live editor was therefore un-hidden onto an intermediate CRDT state and corrected itself in view.

Measured on a reload right after a Mod-Shift-ArrowDown block move:

t=1484ms editor#1 (placeholder)   T|BBB|AAA|CCC   ← correct the whole time
t=1570ms editor#2 (live) REVEALED T|AAA|BBB|CCC   ← pre-move state
t=1577ms editor#2 (live)          T|BBB|AAA|CCC   ← corrects 7ms later

The placeholder was right; the swap was the only reason anything moved. Readiness now waits for one animation frame with no update to the shared document, and any update restarts the wait. Going NOT-ready stays immediate, so nothing keeps a fatal or unsynced document editable a moment longer than before. After the fix the same repro absorbs the stale state while still hidden:

t=2154ms editor#2 hidden=true   (empty)
t=2246ms editor#2 hidden=true   T|AAA|BBB|CCC   ← stale, but not on screen
t=2256ms editor#2 hidden=false  T|BBB|AAA|CCC   ← revealed already correct

Cost is at most one frame of a placeholder that is already showing the correct content.

Ruled out along the way, by measurement rather than reasoning: the move does reach the durable markdown (within 6s, and the typing control too), so this was never a lost write.

Unrelated flake seen while validating

lib/workflows/diff/diff-engine.test.ts times out at 10s under whole-suite parallelism (2.4s standalone) in roughly two of three full runs. Untouched by this branch — flagging because CI may hit it.


Second round: serve a whole document on join, and stop persistence deadlocking

The reflow survived the fixes above, and driving a real browser against the running app found two more causes — one in the relay, one in persistence — plus a client recovery that was never running.

Opening a file replayed it

A room rebuilds itself from the file's Redis stream one entry at a time, into the same Y.Doc that fans every update out to its room — and the join attached the socket before that finished, and before the server seed. A client was therefore never sent the document; it was sent the document's history, and it watched that replay. Reload right after moving a block and the block moved again in front of you.

file-doc.join-readiness.test.ts reproduces it against the real store: with the fix reverted it reports ['AAA', 'BBBAAA'] where one state was due.

The join now assembles the room before attaching a client to it — it awaits the stream catch-up and the seed, so the client's first sync is the finished document, in one message. The seed is memoized on the room so concurrent joins wait for the same one rather than the second being served an empty doc, and a task that loses the seed lock pulls the winner's seed out of the stream instead of waiting for the tailer to push it. That last part is also the "a freshly uploaded file is never editable" bug: it was waiting on a delivery that never came, until the client's readiness deadline lapsed into read-only.

The client-side quiet-frame gate that had been standing in for this is deleted. It was unsound in both directions — it delayed a document that was already correct, and it opened mid-flight anyway whenever updates arrived more than a frame apart, which is exactly what a cross-region Redis and a long room history produce.

Persistence had deadlocked, so the placeholder was permanently stale

The If-Match token is a remembered timestamp: held in the room, which dies with it, and in a cluster key written fire-and-forget. A relay that exits in the moments after a successful write comes back holding a version older than the file's; every persist then fails the CAS; and because a conflict neither writes nor advances the token, that document can never be persisted again. Measured on a live file:

value
cluster If-Match token 1786594348911
file's content_updated_at 1786594418409
unpersisted entries in the stream 103

The durable markdown froze there — and the editor's placeholder is built from it, so every reload painted the pre-edit document and the live one corrected it on screen. That is what still looked like a "replay" after the relay fix.

  • The cluster version write after a successful persist is now awaited. It is the only record that survives a teardown.
  • On a conflict, ask the content, not the clock: if the file still holds the bytes this document last projected (the tag written with every persist), nothing out-of-band exists to protect, so re-sync the token and write. Any other bytes and the conflict stands exactly as before.

The stale-key recovery was never running

useStaleKeyRecovery was requested from inside the failing read's own queryFn — where react-query drops it. Instrumented: the recovery fired, and no fetch happened. So nothing re-resolved the record and the reader sat on a dead key showing "Failed to load file content" until something unrelated refetched. It now runs off that cycle and cancels a record read already in flight (which could only hand back the key we know is dead), and while the record is re-resolving the surface reports loading, not failure.

One document per file, for its whole life

A document rebuilt from markdown is a different document to Yjs — its items carry new client ids — so a client still holding the old one merges the file into itself, twice, on both sides, and the relay persists that. Confirmed through the real converter. The rebuild happened whenever the stored binary was absent (a file opened but never edited had no stored document, so every cold open minted a new one) or stale (any external write).

The seed now stores what it builds and resumes it with a CRDT diff when the markdown moved on out-of-band, so there is only ever one. As a backstop the document carries an identity the join ack names, and a client offered a different one refuses to sync instead of merging — read-only until a reload, never corruption.

Embedded images

The inline route sent no-cache with no validator, so every open re-downloaded the whole image — ~1 MB per open of a real document, with the image area blank until it landed. A ?key= names one storage object and a content write never rewrites one (updateWorkspaceFileContent always mints a fresh key), so those bytes are immutable; ?fileId= names the file, whose bytes move under it, so that form still revalidates. Verified in a browser: inline FROM CACHE on repeat opens, previously 1012 KB every time.

Verification

  • Relay: 277 tests, including a new store-enabled join suite that fails without the fix.
  • App: 1034 tests across collab-doc, workspace-files, copilot/tools/server/files, the file-viewer, the serve/inline routes and the query hooks.
  • In a browser, against the running app: edit-then-reload with zero delay now shows placeholder and live editor identical (T|BBB|AAA|CCC both) where it previously revealed the pre-move order; a plain open no longer rewrites the file (byte-identical, key unchanged).
  • tsc, check:api-validation, check:react-query, check:client-boundary, check:import-specifiers, biome — all clean.

Known residual

The first view of an image the browser has never fetched still reflows once: its box is reserved from stored dimensions, and those are written the first time the browser measures it. Fixing that means carrying the intrinsic size on the image node so the very first paint reserves the box — a schema change, deliberately not folded in here.

@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 13, 2026 5:53am

Request Review

@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Touches realtime join/hydration, collaborative persistence/version tokens, and CRDT/markdown parity — areas where subtle bugs cause silent data loss or document corruption; mitigated by broad new tests but rollout still warrants careful monitoring.

Overview
Stops the collaborative Files editor from rewriting on open, replaying stream history on screen, and reflowing after the placeholder paints — by keeping the live Yjs document, durable markdown, and what the user sees in agreement.

Relay (file-doc / file-doc-store) — A join now awaits shared-stream catch-up and server seed (ensureRoomReady, pendingJoins, memoized seeding) before socket.join and the first sync, so the client gets the finished doc in one message instead of watching Redis entries replay. Losers of the seed lock pull the winner’s seed via catchUp; cluster version after persist is awaited so If-Match cannot lag and deadlock writes. Join success can include docId so clients refuse to merge a rebuilt document.

Collab pipeline — CRDT seeds and agent writes use editorNormalForm; canonicalizeYDoc converges snapshots onto what markdown can round-trip. Markdown parse preserves authored blank lines (with bounds) instead of stripping them; stripEmptyTopLevelParagraphs is removed in favor of that model.

Client — Readiness closes on fatal (timeout / access revoked) so an abandoned provider cannot reopen an editable surface with autosave off. FileDocProvider rejects DOCUMENT_REPLACED when docId mismatches. Collaborative surfaces disable focus refetch of durable bytes; 404 on a storage key triggers record re-resolution (deferred off the failing queryFn) and shows loading during recovery.

App polish — Workspace layout prefetches the file list for SSR/header parity; file detail route gets its own loading chrome; inline images keyed by storage object get long-lived private cache; Season Sans uses display: block to avoid swap reflow on hard refresh.

Reviewed by Cursor Bugbot for commit cd413c5. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR normalizes collaborative documents before seeding, strengthens relay hydration and persistence recovery, and improves file-route rendering and content delivery.

  • Builds and seeds one stable Yjs document per file before clients join.
  • Recovers persistence tokens, superseded storage keys, and collaborative readiness failures.
  • Prefetches workspace file metadata for SSR and adds cache policy for immutable inline objects.
  • Adds regression coverage across the realtime relay, collaboration provider, persistence, queries, and inline-file route.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/realtime/src/handlers/file-doc.ts Makes joins await complete stream hydration and memoized seeding while preserving authorization and room-lifecycle checks.
apps/realtime/src/handlers/file-doc-store.ts Adds repeatable stream catch-up with numeric Redis stream-ID ordering and duplicate-entry bookkeeping protection.
apps/sim/lib/collab-doc/seed.ts Persists and resumes stable collaborative document state rather than rebuilding a new CRDT identity.
apps/sim/lib/collab-doc/persist.ts Avoids redundant durable writes and reconciles stale persistence versions when durable content still matches the relay projection.
apps/sim/app/api/workspaces/[id]/files/inline/route.ts Applies long-lived private caching only to storage-key-addressed immutable content while file-ID requests continue revalidating.
apps/sim/hooks/queries/workspace-files.ts Moves stale-key record re-resolution outside the failing query cycle and presents recovery as loading until re-resolution completes.

Sequence Diagram

sequenceDiagram
  participant Browser
  participant Sim as Sim App
  participant Relay as Realtime Relay
  participant Redis
  participant Storage
  Browser->>Relay: Join file document
  Relay->>Redis: Catch up complete stream
  alt Stream has no seed
    Relay->>Sim: Request normalized seed
    Sim->>Storage: Read durable markdown
    Sim-->>Relay: Seed update and version
    Relay->>Redis: Store seed document
  end
  Relay-->>Browser: Join success and complete Yjs state
  Browser->>Relay: Collaborative updates
  Relay->>Redis: Append CRDT updates
  Relay->>Sim: Persist projected markdown with version
  Sim->>Storage: Write new content object
  Sim-->>Relay: Current persisted version
  Relay->>Redis: Store synchronized version
Loading

Reviews (5): Last reviewed commit: "fix(files): name every collaborative doc..." | Re-trigger Greptile

Comment thread apps/sim/lib/collab-doc/persist.ts
Comment thread apps/sim/lib/collab-doc/converter.ts
@icecrasher321
icecrasher321 force-pushed the fix/files-editor-open-fidelity branch from 361f0f7 to 9674e20 Compare August 13, 2026 02:15
@icecrasher321
icecrasher321 force-pushed the fix/files-editor-open-fidelity branch from 9674e20 to 105051b Compare August 13, 2026 02:29
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

bugbot run

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321
icecrasher321 force-pushed the fix/files-editor-open-fidelity branch from 105051b to 9a6c25d Compare August 13, 2026 02:51
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

bugbot run

Comment thread apps/sim/lib/collab-doc/converter.ts
@icecrasher321
icecrasher321 force-pushed the fix/files-editor-open-fidelity branch from 9a6c25d to 6cb2a7b Compare August 13, 2026 03:16
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6cb2a7b. Configure here.

icecrasher321 and others added 2 commits August 12, 2026 22:37
…ocument on open

Opening a file rewrote it. Binding an editor to a seeded document emits a Yjs
update of its own — ProseMirror appends an empty paragraph to any doc that does
not end in one — which the relay saw as a real edit and persisted. Every open
therefore uploaded the file under a FRESH storage key and deleted the old one,
404ing the page's own in-flight content read, bumping "Last Updated" just from
viewing, and churning a blob per open. Worse, a trailing blank line cannot
serialize, so the file never recorded that paragraph and nothing reconciled the
two: each client that seeded without seeing another's contribution stacked one
more. A real document reached 18 against the placeholder's 1 — measured as the
pane growing several hundred pixels the instant the live editor took over.

- Seed and merge through the editor's own normal form (`editorNormalForm`), so
  binding is a no-op and `canonicalizeYDoc` collapses an accumulated run back to
  one. Placed at the collab boundary, not in `parseMarkdownToDoc`: only the CRDT
  has to agree with the editor — every other consumer of the parse renders
  through a real editor that normalizes itself.
- Skip a persist whose projection already matches the durable bytes. Byte length
  is the free reject, so the compare read only happens when a no-op write is
  actually on the table.
- Revoke collaborative readiness on a fatal join. The sticky `syncedOnce` latch
  outlived the document: after a readiness timeout the provider drops `synced`
  so the gate closes, but the latch re-opened it on the offline fallback's seed
  flag — handing back an EDITABLE editor on a document the provider had
  abandoned, with client autosave gated off because collaboration is nominally
  on. Keystrokes went nowhere and vanished on reload, with no error shown.
- Recover from a superseded storage key instead of stranding the reader: a 404
  re-resolves the file record, so the read re-keys onto the current object. And
  do not focus-refetch durable bytes while the relay owns durability.
- Prefetch the workspace file list in the layout, where the sidebar already
  reads it. `HydrationBoundary` defers an already-seen query to an effect that
  SSR never runs, so a page-level prefetch of that key could not reach the
  server render — the file route rendered a spinner and disagreed with the
  client about the header's markup (a hydration mismatch).
- Load the document font with `display: block`. A swap repaints prose in
  metric-adjusted Arial first, so paragraphs re-wrap when the real face lands.

Also: a detail-route `loading.tsx` (the segment was inheriting the list chrome),
`normalize.ts` renamed to `field.ts` now that it holds only the field constant,
and the duplicate `COLLAB_DOC_FIELD` in the streaming path folded into it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…locking

Opening a file right after an edit replayed it — the block you had just moved
moved again, in front of you. A room rebuilds itself from the file's Redis
stream one entry at a time, into the same Y.Doc that fans every update out to
its room, and the join attached the socket before that finished and before the
server seed. So a client was never sent the document; it was sent the
document's history, and it watched that replay. The new join-readiness test
reproduces it exactly — ['AAA', 'BBBAAA'] where one state was due — and fails
without the fix.

Underneath it, persistence had deadlocked. The If-Match token is a REMEMBERED
timestamp: held in the room, which dies with it, and in a cluster key written
fire-and-forget. A relay that exits in the moments after a successful write
comes back holding a version older than the file's, every persist then fails
the CAS, and because a conflict neither writes nor advances the token, that
document can never be persisted again. Measured on a live file: token
1786594348911 against a content version of 1786594418409, with 103 unpersisted
entries still in the stream. The durable markdown froze there — and the
editor's placeholder is built from it, so every reload painted the pre-edit
document and the live one corrected it on screen.

- Assemble a room before attaching a client to it. The join awaits the stream
  catch-up and the seed, so the first sync IS the finished document, in one
  message. The seed is memoized on the room, so concurrent joins wait for the
  same one instead of the second being served an empty doc; a task that loses
  the seed lock PULLS the winner's seed from the stream rather than waiting for
  the tailer to push it, which is what left a freshly uploaded file read-only
  until its readiness deadline lapsed.
- Drop the client-side quiet-frame gate that was standing in for this. It was
  unsound in both directions: it delayed a document that was already correct,
  and it opened mid-flight anyway whenever updates arrived more than a frame
  apart, which is what a cross-region Redis and a long room history produce.
- Await the cluster version write after a successful persist. It is the only
  record that survives a teardown, and one round trip after a blob write is not
  a cost worth a wedged document.
- On a version conflict, ask the CONTENT, not the clock. If the file still
  holds the bytes this document last projected — the tag written with every
  persist — then nothing out-of-band exists to protect, so re-sync the token and
  write. Any other bytes and the conflict stands exactly as before.
- Actually run the stale-storage-key recovery. It was requested from inside the
  failing read's own queryFn, where react-query drops it, so nothing re-resolved
  the record and the reader sat on a dead key showing "Failed to load file
  content" until something unrelated refetched. It now runs off that cycle and
  cancels a read already in flight, which could only hand back the dead key.
  While the record is re-resolving the surface reports loading, not failure.
- One document per file, for its whole life. A document rebuilt from markdown is
  a DIFFERENT document to Yjs — its items carry new client ids — so a client
  still holding the old one merges the file into itself, twice, on both sides.
  The seed now stores what it builds (until that row existed, a file opened but
  never edited was rebuilt on every open) and resumes it with a CRDT diff when
  the markdown moved on out-of-band. A document also carries an identity the
  join ack names, so a tab that outlived its room is refused rather than merged.
- Let a browser keep an embedded image. The inline route sent no-cache with no
  validator, so every open re-downloaded the whole image — measured at ~1 MB per
  open of a real document, with the image area blank until it landed. A `key`
  names one storage object and a content write never rewrites one, so those
  bytes are immutable; a `fileId` names the file, whose bytes move, so that form
  still revalidates.
@icecrasher321
icecrasher321 force-pushed the fix/files-editor-open-fidelity branch from 6cb2a7b to 35f1597 Compare August 13, 2026 05:39
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

bugbot run

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/collab-doc/seed.ts
…e URL names

Two findings from review, both real.

A document stored before identities existed is returned by the seed's fast path
on every open, and that path never named one — so those files could never
acquire an identity, and the join-ack guard could never fire for them. That is
the population most likely to have a tab that outlived its room, which is the
case the guard exists for. The fast path now names an unnamed document and
stores it, once: minting without storing would name it differently on every
open and the guard would start refusing clients that hold the very same
document.

The inline route marked a response immutable whenever the caller passed a key,
but a key is resolved to a FILE and the file's current key is what gets
streamed. A content write landing between those two reads would serve the new
bytes under a URL naming the old object — and cached for a year, that is wrong
forever. The flag is now what it always meant: the URL names the exact object
that was streamed.
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit cd413c5. Configure here.

@icecrasher321
icecrasher321 merged commit e8d278b into staging Aug 13, 2026
30 checks passed
@icecrasher321
icecrasher321 deleted the fix/files-editor-open-fidelity branch August 13, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant