kayg/calternal

Watch

Fork 0

Collab: external writer can drop live client edits from persisted note #382

New issue

Closed

opened 2026-09-29 01:47:37 +00:00 by kayg · 3 comments

kayg commented

2026-09-29 01:47:37 +00:00

Owner

Evidence

During the single full workspace cargo test run on job/quota (HEAD 8f5af823), calternal-collab/tests/two_clients.rs::concurrent_clients_and_external_writer_converge failed at the persisted-note assertion on line 224.

The test's Bun client check passed with both live edits (from A, from B) and the external edit visible. After the test waited 900 ms and read the note from disk, the persisted Markdown was:

first

second

external

Both live client edits were missing while the external edit remained. The integration test reported 1 passed; 1 failed and cargo test exited 101 at -p calternal-collab --test two_clients.

This is one observed run on a shared host. Reproduce and inspect the persistence race on a quieter host. The existing test expectation was not changed, and this job did not rerun the workspace gate.

## Evidence During the single full workspace `cargo test` run on `job/quota` (HEAD `8f5af823`), `calternal-collab/tests/two_clients.rs::concurrent_clients_and_external_writer_converge` failed at the persisted-note assertion on line 224. The test's Bun client check passed with both live edits (`from A`, `from B`) and the external edit visible. After the test waited 900 ms and read the note from disk, the persisted Markdown was: ```markdown first second external ``` Both live client edits were missing while the external edit remained. The integration test reported `1 passed; 1 failed` and `cargo test` exited 101 at `-p calternal-collab --test two_clients`. This is one observed run on a shared host. Reproduce and inspect the persistence race on a quieter host. The existing test expectation was not changed, and this job did not rerun the workspace gate.

kayg referenced this issue

2026-09-29 02:02:25 +00:00

SECURITY: Prevent one User from exhausting Instance storage #333

kayg commented

2026-09-29 02:45:49 +00:00

Author

Owner

Starting #382 on job/editor-integrity at base 7d4842822d2d706a0758d0681fb1332fcec76b81 (the part 2 commit already in dev). I will merge the current dev tip before investigation/final gates, then reproduce the reconcile/flush race and add a deterministic regression.

Starting #382 on `job/editor-integrity` at base `7d4842822d2d706a0758d0681fb1332fcec76b81` (the part 2 commit already in `dev`). I will merge the current `dev` tip before investigation/final gates, then reproduce the reconcile/flush race and add a deterministic regression.

kayg commented

2026-09-29 03:01:44 +00:00

Author

Owner

Reproduced on focused integration run 12 of 200 while two CPU-bound workers ran. Bun confirmed both clients had from A, from B and external; the persisted-note assertion 900 ms later saw only first, second, external. The test panics at that assertion and aborts its server, so I am checking whether this is a delayed flush or a stale persisted snapshot before choosing the source fix.

Reproduced on focused integration run 12 of 200 while two CPU-bound workers ran. Bun confirmed both clients had `from A`, `from B` and `external`; the persisted-note assertion 900 ms later saw only `first`, `second`, `external`. The test panics at that assertion and aborts its server, so I am checking whether this is a delayed flush or a stale persisted snapshot before choosing the source fix.

kayg commented

2026-09-29 04:14:47 +00:00

Author

Owner

Completed

Branch: job/editor-integrity
Head: 3d46ca5575344bfd6773b2156454b7e297d6f80b (fix(collab): flush merged edits after external changes)
Push: git push origin job/editor-integrity reported Everything up-to-date.

external_change now flushes the live Yrs document after it applies the external Markdown diff, while it still holds the room write lock. A failed conditional write keeps the dirty mark and schedules the existing retry. This avoids returning from reconciliation with the external-only Markdown still on disk while the merged live edits wait for the debounce timer.

Added cfg(test) one-shot delay hooks before external reconcile applies its diff and before flush snapshots Yrs. The regression inserts two live edits after the external Markdown read, then another edit after reconcile but before the flush snapshot. It checks that the external change remains pending until the exact merged body is persisted.

Files: crates/calternal-collab/src/session.rs.

Reproduction and verification

Before the fix, the real two-client test reproduced at run 12 under two CPU-bound workers: both clients saw from A, from B and external, but the 900 ms disk read contained only first, second, external. The deterministic regression failed before the source change with external reconcile returned before its merged Note was persisted.

After the fix, the same two-client integration test passed 200/200 runs under two CPU-bound workers. The deterministic delayed-order regression passed. The focused real-server adversarial editor probe also passed:

PASS editor external Note body write during a live room seed=25608414 ms=314
PASS editor all areas seed=25608414 historySeeds=25608414

Gate output:

$ cargo fmt --check

Exit 0, no output.

$ cargo clippy --all-targets -- -D warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 8m 59s

cargo test -p calternal-collab ran 23 unit tests and earlier integration targets successfully, then failed in the existing shared Note revoke test. Exact failure summary:

test owner_editor_viewer_and_live_revoke ... FAILED
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.04s
error: test failed, to rerun pass `-p calternal-collab --test shared_notes`

The failure timed out waiting for the editor WebSocket to close after Share revocation. A single targeted rerun passed:

test owner_editor_viewer_and_live_revoke ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 11.84s

The remaining collaboration integration targets also passed when run directly: two_clients (2), untouched_bytes (5), vector_bridge (15), and wiki_embeds (5).

$ bun run check
svelte-check found 0 errors and 0 warnings

$ bun run test
 Test Files  119 passed (119)
      Tests  779 passed (779)
   Duration  83.93s (transform 50%, environment 18%, import 17%, tests 10%, setup 4%)

Vitest printed the existing jsdom messages Could not parse CSS stylesheet and Not implemented: Window's scrollTo() method; the command exited 0.

Cleanup output:

Removed 15334 files, 9.1GiB total

Gap and decision

The full crate test command had one shared Note revoke timeout under suite load. It passed on its targeted rerun; I did not change its expectation.

docs/DESIGN.md says external Markdown changes merge into the live Yrs document but does not specify when the merged document must be flushed. I chose to attempt that flush inline under the room write lock and retain the existing retry path for failures.

## Completed Branch: `job/editor-integrity` Head: `3d46ca5575344bfd6773b2156454b7e297d6f80b` (`fix(collab): flush merged edits after external changes`) Push: `git push origin job/editor-integrity` reported `Everything up-to-date`. `external_change` now flushes the live Yrs document after it applies the external Markdown diff, while it still holds the room write lock. A failed conditional write keeps the dirty mark and schedules the existing retry. This avoids returning from reconciliation with the external-only Markdown still on disk while the merged live edits wait for the debounce timer. Added `cfg(test)` one-shot delay hooks before external reconcile applies its diff and before flush snapshots Yrs. The regression inserts two live edits after the external Markdown read, then another edit after reconcile but before the flush snapshot. It checks that the external change remains pending until the exact merged body is persisted. Files: `crates/calternal-collab/src/session.rs`. ## Reproduction and verification Before the fix, the real two-client test reproduced at run 12 under two CPU-bound workers: both clients saw `from A`, `from B` and `external`, but the 900 ms disk read contained only `first`, `second`, `external`. The deterministic regression failed before the source change with `external reconcile returned before its merged Note was persisted`. After the fix, the same two-client integration test passed 200/200 runs under two CPU-bound workers. The deterministic delayed-order regression passed. The focused real-server adversarial editor probe also passed: ```text PASS editor external Note body write during a live room seed=25608414 ms=314 PASS editor all areas seed=25608414 historySeeds=25608414 ``` Gate output: ```text $ cargo fmt --check ``` Exit 0, no output. ```text $ cargo clippy --all-targets -- -D warnings Finished `dev` profile [unoptimized + debuginfo] target(s) in 8m 59s ``` `cargo test -p calternal-collab` ran 23 unit tests and earlier integration targets successfully, then failed in the existing shared Note revoke test. Exact failure summary: ```text test owner_editor_viewer_and_live_revoke ... FAILED test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.04s error: test failed, to rerun pass `-p calternal-collab --test shared_notes` ``` The failure timed out waiting for the editor WebSocket to close after Share revocation. A single targeted rerun passed: ```text test owner_editor_viewer_and_live_revoke ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 11.84s ``` The remaining collaboration integration targets also passed when run directly: `two_clients` (2), `untouched_bytes` (5), `vector_bridge` (15), and `wiki_embeds` (5). ```text $ bun run check svelte-check found 0 errors and 0 warnings ``` ```text $ bun run test Test Files 119 passed (119) Tests 779 passed (779) Duration 83.93s (transform 50%, environment 18%, import 17%, tests 10%, setup 4%) ``` Vitest printed the existing jsdom messages `Could not parse CSS stylesheet` and `Not implemented: Window's scrollTo() method`; the command exited 0. Cleanup output: ```text Removed 15334 files, 9.1GiB total ``` ## Gap and decision The full crate test command had one shared Note revoke timeout under suite load. It passed on its targeted rerun; I did not change its expectation. `docs/DESIGN.md` says external Markdown changes merge into the live Yrs document but does not specify when the merged document must be flushed. I chose to attempt that flush inline under the room write lock and retain the existing retry path for failures.

kayg referenced this issue from a commit

2026-09-29 04:20:33 +00:00

Merge job/editor-integrity part 3 (#382: external reconcile flushes merged live edits under the room write lock; 200/200 under load)

kayg closed this issue

2026-09-29 04:20:46 +00:00

kayg referenced this issue

2026-10-02 13:10:12 +00:00

PERF: target external Note changes to one collaboration room (#663) #769