fix(editor): a save the file has already moved past is dropped, not parked - #258
Merged
Merged
Conversation
…arked
The first version of this fix deployed and four more conflict copies appeared
in the same project within minutes — each one a strict SUBSET of the file it
sat beside:
live: status: active aaaasdfasdfjoais djfoi
copy: status: active aaa
Containment has two directions and only one was handled.
ours contains theirs -> our write is the superset. Rebase and retry.
This is what shipped, and it is right.
theirs contains ours -> the FILE IS AHEAD. A peer saved a newer state of
the document we share while our save was in
flight, so what we were writing is simply out of
date. There is nothing to write and nothing to
preserve.
The second case was falling through to preserve(), which parked an older
version of the file next to the newer one and called it a conflict. It is not
a conflict; it is a save that lost a race to a save that contained it. The
right answer is to record where the file is, go clean, and let our own
document catch up over the websocket like everyone else's.
Both gates are verified in both directions — with the branch removed each one
fails, naming the conflict copy in the message. That mattered here: the first
two attempts to verify this ran against a STALE BUNDLE, because disabling the
branch broke the TypeScript build and `npm run build` failed while the test
went on to pass against the previous build. A test run whose build did not
succeed proves nothing, and it looks exactly like a pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ssowonny
added a commit
that referenced
this pull request
Sep 23, 2026
Every co-editor used to write the file themselves, 700ms after their own last keystroke, each against whatever If-Match they last saw. With one document shared by N people that is N writers racing for one file, and every lost race parked somebody's text beside the file as a "conflict" that was the file against itself. Two rounds of client-side retry logic (#257, #258) did not stop it, because the race is between reading the base and the write arriving, and no amount of care about timing closes that. So nobody in the room writes. The hub holds the document, subscribes to every update through crdt.Doc.OnUpdate, and writes ONCE per pause: two seconds after the last change, or ten seconds after the first if the typing never pauses — the numbers Hocuspocus settled on for onStoreDocument, for the same reasons. The client, once its room is live, never arms its save timer again; a client that never reaches "live" is solo and saves exactly as before. One writer changes what an If-Match refusal MEANS. It can no longer be a co-editor — everyone in the room shares this document — so it can only be an agent, the CLI, or a device writing the file underneath the room. That case gets the conflict copy it deserves, made by the hub and attributed to the human who was editing, and the room rebases so it is not refused for the same stale base forever. Nothing is silently overwritten on either side. Folding the outside write INTO the live document is the better answer and is the hub's to give — it is the one place the splice could be made exactly once — and is filed, not done. Seeding subscribes AFTER the bytes are in, so the document a room was built from is never mistaken for an edit that needs writing back. Dropping a room disarms its timer and unsubscribes. The debounce constants are vars so a test can shrink them; production never sets them. Five server tests, each seen to FAIL with its mechanism removed: no watcher (two tests), no If-Match (one), and the two guards. Race-clean. The e2e gate — no client PUTs in a live room, file still updates, status line goes clean when the hub's write comes back — is WRITTEN AND UNRUN: Chromium cannot launch on this machine right now. It is not believed until it has been watched to fail against the previous bundle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ssowonny
added a commit
that referenced
this pull request
Sep 23, 2026
The two forced-409 tests (#257, #258) exercise the client's own save-and- retry. That path now exists only when the hub does NOT hold the document — in a live room the hub is the sole writer and the client never PUTs, so there is no save to refuse, and both tests failed on their own guard: "the 409 was never delivered, so nothing was tested". That guard did its job. They now refuse the websocket upgrade (routeWebSocket) so the editor gives up on the room after UNREACHABLE_MS and mounts solo — the older-hub / no- websocket-proxy case the retry was kept for. Verified both ways: with the retry removed, each still fails on the conflict copy it guards. Also closes #258's test body, which the rebase's union resolution had left open: #258's final expect was immediately followed by the next test's header comment, so one function swallowed the other and the file did not parse. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ssowonny
added a commit
that referenced
this pull request
Sep 23, 2026
* feat(collab): the hub is the only writer while a room is live Every co-editor used to write the file themselves, 700ms after their own last keystroke, each against whatever If-Match they last saw. With one document shared by N people that is N writers racing for one file, and every lost race parked somebody's text beside the file as a "conflict" that was the file against itself. Two rounds of client-side retry logic (#257, #258) did not stop it, because the race is between reading the base and the write arriving, and no amount of care about timing closes that. So nobody in the room writes. The hub holds the document, subscribes to every update through crdt.Doc.OnUpdate, and writes ONCE per pause: two seconds after the last change, or ten seconds after the first if the typing never pauses — the numbers Hocuspocus settled on for onStoreDocument, for the same reasons. The client, once its room is live, never arms its save timer again; a client that never reaches "live" is solo and saves exactly as before. One writer changes what an If-Match refusal MEANS. It can no longer be a co-editor — everyone in the room shares this document — so it can only be an agent, the CLI, or a device writing the file underneath the room. That case gets the conflict copy it deserves, made by the hub and attributed to the human who was editing, and the room rebases so it is not refused for the same stale base forever. Nothing is silently overwritten on either side. Folding the outside write INTO the live document is the better answer and is the hub's to give — it is the one place the splice could be made exactly once — and is filed, not done. Seeding subscribes AFTER the bytes are in, so the document a room was built from is never mistaken for an edit that needs writing back. Dropping a room disarms its timer and unsubscribes. The debounce constants are vars so a test can shrink them; production never sets them. Five server tests, each seen to FAIL with its mechanism removed: no watcher (two tests), no If-Match (one), and the two guards. Race-clean. The e2e gate — no client PUTs in a live room, file still updates, status line goes clean when the hub's write comes back — is WRITTEN AND UNRUN: Chromium cannot launch on this machine right now. It is not believed until it has been watched to fail against the previous bundle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(e2e): the client-side 409 tests now run in solo mode The two forced-409 tests (#257, #258) exercise the client's own save-and- retry. That path now exists only when the hub does NOT hold the document — in a live room the hub is the sole writer and the client never PUTs, so there is no save to refuse, and both tests failed on their own guard: "the 409 was never delivered, so nothing was tested". That guard did its job. They now refuse the websocket upgrade (routeWebSocket) so the editor gives up on the room after UNREACHABLE_MS and mounts solo — the older-hub / no- websocket-proxy case the retry was kept for. Verified both ways: with the retry removed, each still fails on the conflict copy it guards. Also closes #258's test body, which the rebase's union resolution had left open: #258's final expect was immediately followed by the next test's header comment, so one function swallowed the other and the file did not parse. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
ssowonny
added a commit
that referenced
this pull request
Sep 23, 2026
…ch box true (#261) The stage's list was marked done in #246 on the strength of its first item. "The client defers the write to the hub" and "a periodic snapshot while a room is live" were ticked and not built — a script replaced every "- [ ]" in the stage, and nothing re-read the list against the code. That gap is the direct cause of every conflict copy produced between #246 and #260: the 0-byte report, the 8x-duplicated document, forty-four copies. Two PRs (#257, #258) patched the symptom because the PRD said the cause was already fixed. #260 built the two missing items. Each box now names the PR that made it true. A tick is a claim. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
TL;DR
theirs ⊇ oursmeans the file is ahead — nothing to write, nothing to preserve. It was falling through topreserve().What was still wrong
The second case fell through to
preserve(), which parked an older version of the file next to the newer one and called it a conflict. It isn't a conflict — it's a save that lost a race to a save that contained it. The right answer is to record where the file is, go clean, and let our document catch up over the websocket like everyone else's.A verification trap worth naming
My first two attempts to verify this ran against a stale bundle. Disabling the branch broke the TypeScript build,
npm run buildfailed — and the test then ran against the previous build and passed.A test run whose build didn't succeed proves nothing, and it looks exactly like a pass. I only caught it by reading the build output rather than the test output. Both gates are now confirmed to fail with their branch removed, naming the conflict copy in the message.
Why this keeps taking two tries
Each round I fixed the case the evidence in front of me showed, and the next case only became visible once the first stopped firing. The honest summary is that "a 409 during co-editing" has three outcomes — superset, subset, and genuine divergence — and I shipped them one at a time instead of enumerating them up front.
Only the third deserves a conflict copy, and that's what's left.
🤖 Generated with Claude Code