COLLAB: concurrent delete and edit can lose an edit to the deleted block

kayg commented

2026-09-28 11:28:04 +00:00

Owner

tests/adversarial/editor.mjs reproduced a possible collaboration data-loss race on a real local server at branch job/single-pills, head 46497b6a7c03eb02a7cbbbfac3f36b563c906af7, seed 25608414.

The delete-edit probe created a note with first-target and second-target, synced two clients, then deleted the first block in one client while appending delete-first-edit-survivor to that block in the other. Both peers received updates, but the edited block became empty and the note saved without the marker. The probe timed out waiting for the edit to survive. It failed on the delete-first order, so the edit-first order did not run in this pass.

Evidence from the probe:

editedBlock was empty on the editor client.
Both client bodies contained only the race heading and second-target.
The saved Note body was "\\n# 186 delete-first delete edit race\\n\\nsecond-target\\n", with no concurrent edit marker.

Other builds and adversarial runners were active on the shared host. Please reproduce on a quieter host before attributing a cause. The existing #81 tracks a different rejected-update save failure; this probe exercises a concurrent delete/edit conflict.

`tests/adversarial/editor.mjs` reproduced a possible collaboration data-loss race on a real local server at branch `job/single-pills`, head `46497b6a7c03eb02a7cbbbfac3f36b563c906af7`, seed `25608414`. The `delete-edit` probe created a note with `first-target` and `second-target`, synced two clients, then deleted the first block in one client while appending `delete-first-edit-survivor` to that block in the other. Both peers received updates, but the edited block became empty and the note saved without the marker. The probe timed out waiting for the edit to survive. It failed on the delete-first order, so the edit-first order did not run in this pass. Evidence from the probe: - `editedBlock` was empty on the editor client. - Both client bodies contained only the race heading and `second-target`. - The saved Note body was `"\\n# 186 delete-first delete edit race\\n\\nsecond-target\\n"`, with no concurrent edit marker. Other builds and adversarial runners were active on the shared host. Please reproduce on a quieter host before attributing a cause. The existing #81 tracks a different rejected-update save failure; this probe exercises a concurrent delete/edit conflict.

kayg commented

2026-09-28 11:52:05 +00:00

Author

Owner

Additional observation from a full workspace test run at job/preview-attach head 45b6e0a4e2a62f9d2b0877d6aed6114cbad3db26: existing crates/calternal-collab/tests/two_clients.rs::concurrent_clients_and_external_writer_converge failed once. Both Yjs client edits (from A, from B) appeared in their converged client state, then the external writer appended external; after the 900 ms persistence wait, the stored Markdown contained first, second, and external only. The test failed its existing assertion that all three edits survive. Other builds and adversarial runs were active on the shared host, so this is evidence to reproduce, not a confirmed root cause. I left the test expectation unchanged and did not rerun the gate.

Additional observation from a full workspace test run at `job/preview-attach` head `45b6e0a4e2a62f9d2b0877d6aed6114cbad3db26`: existing `crates/calternal-collab/tests/two_clients.rs::concurrent_clients_and_external_writer_converge` failed once. Both Yjs client edits (`from A`, `from B`) appeared in their converged client state, then the external writer appended `external`; after the 900 ms persistence wait, the stored Markdown contained `first`, `second`, and `external` only. The test failed its existing assertion that all three edits survive. Other builds and adversarial runs were active on the shared host, so this is evidence to reproduce, not a confirmed root cause. I left the test expectation unchanged and did not rerun the gate.

kayg referenced this issue

2026-09-28 11:54:50 +00:00

CALENDAR: Attach and Record voice in the item preview actions (reuse the composer's tray and recorder) #303

kayg commented

2026-09-28 12:01:13 +00:00

Author

Owner

Related data point (orchestrator, 2026-09-28): cargo test -p calternal-collab --test two_clients ('lost two client edits after an external write') failed once in the job/preview-attach gate run at very high host load, then passed 9/9 (6 on dev, 3 on the branch; one run at load average 63). The branch does not touch calternal-collab. Treat this as a possible real race exposed by latency (external day-file write vs in-flight client updates), not as noise: reproduce under artificial latency (tokio time-shifts or injected delays between the external write and client update delivery) and fix, or prove it is a test-harness timing assumption. Also needs an owner decision for the delete-vs-edit case above: when one user deletes a block while another edits it, should the edit resurrect the block (the edit wins) or be discarded (the delete wins, standard Yjs behaviour)?

Related data point (orchestrator, 2026-09-28): `cargo test -p calternal-collab --test two_clients` ('lost two client edits after an external write') **failed once** in the job/preview-attach gate run at very high host load, then **passed 9/9** (6 on dev, 3 on the branch; one run at load average 63). The branch does not touch calternal-collab. Treat this as a possible real race exposed by latency (external day-file write vs in-flight client updates), not as noise: reproduce under artificial latency (tokio time-shifts or injected delays between the external write and client update delivery) and fix, or prove it is a test-harness timing assumption. Also needs an owner decision for the delete-vs-edit case above: when one user deletes a block while another edits it, should the edit resurrect the block (the edit wins) or be discarded (the delete wins, standard Yjs behaviour)?

kayg referenced this issue

2026-09-28 13:04:18 +00:00

UI: every lone icon button is a pill-in-a-pill (shared single PillGroup) — sidebar toggle first, then hunt all instances #319

kayg commented

2026-09-28 18:46:45 +00:00

Author

Owner

Started editor-integrity on branch job/editor-integrity. Base dev SHA: fba83527f2. I am reading and reproducing the linked integrity reports before making changes.

Started editor-integrity on branch job/editor-integrity. Base dev SHA: fba83527f2cccf2334934bb1fd0932be7c0e209b. I am reading and reproducing the linked integrity reports before making changes.

kayg commented

2026-09-28 19:46:43 +00:00

Author

Owner

I reproduced both delivery orders in deterministic Yrs tests. Before the fix, the stale-edit-after-delete case and the edit-before-delete case each failed because merge_conflict_shadow returned early at conflict_update.is_some(). The server used a connection-wide history flag, so it skipped all updates from a connection after a root deletion and did not reconcile a shadow that already contained an edit when the deleting update arrived.

I changed the merge path to filter only updates that insert root blocks from local redo history, and to reconcile a pending shadow on a root deletion. All four focused conflict_shadow_tests now pass, including the captured paste undo/redo duplication guard. I also extended the local adversarial delete/edit probe to require the edited marker in both peers. The production-server replay is still pending.

I reproduced both delivery orders in deterministic Yrs tests. Before the fix, the stale-edit-after-delete case and the edit-before-delete case each failed because `merge_conflict_shadow` returned early at `conflict_update.is_some()`. The server used a connection-wide history flag, so it skipped all updates from a connection after a root deletion and did not reconcile a shadow that already contained an edit when the deleting update arrived. I changed the merge path to filter only updates that insert root blocks from local redo history, and to reconcile a pending shadow on a root deletion. All four focused `conflict_shadow_tests` now pass, including the captured paste undo/redo duplication guard. I also extended the local adversarial delete/edit probe to require the edited marker in both peers. The production-server replay is still pending.

kayg commented

2026-09-28 23:45:44 +00:00

Author

Owner

Editor-integrity job final report — branch job/editor-integrity, head b824878e (pushed).

#332 and #364: fixed the Yrs conflict-shadow merge that discarded the surviving edit when another client deleted that block. Deterministic tests cover both update orders and a same-connection redo update. The real-server delete/edit probe passed.
#346: the concurrent delete/edit loss is fixed by the same change. The history-text finding is not closed by this work; see #314 and #363 below.
#225 and #262: added structure-level editor history coverage, but did not rerun the 500-step production browser ID storm after the final branch state. Persistence of unique IDs in that exact storm remains unconfirmed.
#280 and #281: the focused history test keeps one pasted image over 50 undo/redo pairs. The 500-step production browser image storm was not rerun after the final branch state, so this exact case remains unconfirmed.
#314 and #363: focused Yjs/ProseMirror structure tests pass. The fixed-seed production browser history probe remained timing-sensitive: clean runs reported a stale or mismatched mounted ProseMirror view after Ctrl+Z, while one instrumented run passed 50 paste and IME round trips. I removed the unverified key scheduling change and kept the failure evidence for follow-up.

Verification:

bun run test src/collaborationUndo.test.ts: 4 passed.
cargo fmt --check: exit 0, no output.
cargo clippy --all-targets -- -D warnings: stopped at the four-hour job limit (exit 130) while compiling workspace dependencies.
cargo test, bun run check, and bun run test for the full web workspace were not run before the time limit.

Decision: conflict-shadow merge filters each incoming Yrs update against that update's own delete set. This keeps a later text edit that follows a root deletion while avoiding replay of a duplicate root insertion from the same redo update. The issue-specific tests pass; the full gates and listed browser storms remain for follow-up. Issues remain open.

Editor-integrity job final report — branch `job/editor-integrity`, head `b824878e` (pushed). - #332 and #364: fixed the Yrs conflict-shadow merge that discarded the surviving edit when another client deleted that block. Deterministic tests cover both update orders and a same-connection redo update. The real-server delete/edit probe passed. - #346: the concurrent delete/edit loss is fixed by the same change. The history-text finding is not closed by this work; see #314 and #363 below. - #225 and #262: added structure-level editor history coverage, but did not rerun the 500-step production browser ID storm after the final branch state. Persistence of unique IDs in that exact storm remains unconfirmed. - #280 and #281: the focused history test keeps one pasted image over 50 undo/redo pairs. The 500-step production browser image storm was not rerun after the final branch state, so this exact case remains unconfirmed. - #314 and #363: focused Yjs/ProseMirror structure tests pass. The fixed-seed production browser history probe remained timing-sensitive: clean runs reported a stale or mismatched mounted ProseMirror view after Ctrl+Z, while one instrumented run passed 50 paste and IME round trips. I removed the unverified key scheduling change and kept the failure evidence for follow-up. Verification: - `bun run test src/collaborationUndo.test.ts`: 4 passed. - `cargo fmt --check`: exit 0, no output. - `cargo clippy --all-targets -- -D warnings`: stopped at the four-hour job limit (exit 130) while compiling workspace dependencies. - `cargo test`, `bun run check`, and `bun run test` for the full web workspace were not run before the time limit. Decision: conflict-shadow merge filters each incoming Yrs update against that update's own delete set. This keeps a later text edit that follows a root deletion while avoiding replay of a duplicate root insertion from the same redo update. The issue-specific tests pass; the full gates and listed browser storms remain for follow-up. Issues remain open.

kayg referenced this issue

2026-09-28 23:45:45 +00:00

Investigate Editor undo and concurrent edit data loss #346

kayg referenced this issue

2026-09-28 23:45:48 +00:00

Editor delete and edit conflict did not reach both peers #364

kayg referenced this issue

2026-09-28 23:45:49 +00:00

Adversarial editor undo/redo round trip loses note text #314

kayg referenced this issue

2026-09-28 23:45:50 +00:00

Regression: undo/redo storm persists duplicate Note block IDs #225