pcbjam/docs/features/ysync-review/06-bug-concurrent-seed-duplicates-layout.md
Gergő Törcsvári ae5d41a3d7
fix(ysync): review bugs 01–07 — listener registration, pad nets, child removals, dirty-set emit, targeted rebaseline, seed arbitration, stale hook (doc 17)
All seven ysync-review bugs fixed and verified; every repro's expected-fail
marker removed (they now run as regression tests). Also fixes two bugs found
while verifying (doc 17 F5/F6): file-seeded Y bodies are re-upserted in the
editor's serialization (doc-16 F4 was an artifact of bug 01), and the
0008-era "asyncify-fragile envelope parse" was really wrapInBoardEnvelope
emitting display layer names — canonical LSET::Name() fixes track/via/zone
v2 applies; makeFromBlob now logs parse errors instead of swallowing them.

Verified: shared 98, standalone collab 37, ysync e2e 20/20 (chromium),
collab regression set 21 passed / 3 pre-existing skips (firefox).

Bumps: kicad (board_commit child-removal listener notification),
web/pcbjam-shared (slot prune + arbitrated seed).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JgThWXtdvrYLK47EDFoGdq
2026-07-06 08:57:57 +02:00

4.6 KiB

Bug 06 — Concurrent first-seed duplicates kdoc_layout; corrupt materialization with no self-heal

Severity: medium (race window is small but the damage is durable file corruption) Status: FIXED 2026-07-03 — see 17 (batch 4: arbitrated seedDocToY, fix direction 2)

Where

  • web/pcbjam-shared/src/kicad-y.ts:79-94docToY: layout.delete(0, length) then layout.insert(0, doc.layout) inside one transaction
  • Seed decision points: web/standalone/src/wasm/collab/kicad-binding.ts:127-175 (seed() checks ydocHasState then seeds), provider.ts:56-68 (BroadcastChannel settleMs, default 300 ms), index.ts:85-98 (whenSynced()seed())

The race

Seed-vs-adopt is decided by a check-then-act on the client: after whenSynced(), if ydocHasState(doc) is false, the client seeds. Two clients opening the same fresh room concurrently can both observe "empty" before receiving each other's seed transaction:

  • network providers (partykit / hocuspocus): window ≈ one server round-trip between sync-complete and the peer's update arriving;
  • BroadcastChannel: window ≈ the whole settleMs (300 ms) if both tabs open together.

Two simultaneous docToY calls then merge as follows:

  • kdoc_meta.root — Y.Map LWW → converges fine.
  • kdoc_items — same uuid keys, (typically) identical values → LWW per key → converges fine.
  • kdoc_layout — breaks. Each client's delete sees only its own (empty) view; each insert is an independent CRDT op, and Y.Array keeps both sequences. The merged layout holds two (version …), two (paper …), two (lib_symbols …), and two {item: uuid} slots per root item.

Consequences

  • docToFile renders every root item twice (each {item} slot resolves; the per-render seen set only guards cycles within one path, not repeated slots) and duplicates the preamble forms → the materialized file is invalid or at best semantically doubled.
  • ydoc-mode opens materialize this corruption directly.
  • Nothing heals it: ydocHasState is now true, so no client ever re-seeds; applyDeltaToY only appends/removes individual root slots, it never rewrites the layout. The duplication is permanent for the room's lifetime.
  • If the two seeders had different file versions (one stale), items also interleave arbitrarily per-key — messier still, but the layout duplication alone is enough to corrupt.

Fix directions (pick one)

  1. Server-side seeding (cleanest): the sync server creates/initializes the room document from the stored file exactly once (it already owns the .ydoc blob); clients never file-seed. Kills the race by construction, and also removes the client's seed-authority special cases.
  2. Client-side seed arbitration: write a random seedNonce into kdoc_meta inside the same docToY transaction. After the transaction has round-tripped (next sync / short settle), re-read the nonce: LWW means exactly one seeder "won". A loser that sees a foreign nonce re-runs docToY? No — re-running re-inserts. The loser must instead retract its own layout inserts (delete slots whose insertion client-id is its own) or simply rewrite layout in a fresh transaction after observing the winner's state (delete-all + insert is safe once only one client does it, so gate the rewrite on "my nonce lost").
  3. Read-side dedup (mitigation, not a fix): yToDoc / docToFile could drop duplicate {item} slots and duplicate preamble heads. This masks the corruption for materialization but leaves the doc itself dirty; only worth doing as a safety net on top of 1 or 2.

Option 1 is recommended — it also solves stale-file double-seeding and removes the settleMs heuristic from the BroadcastChannel path.

Verification

Unit: two Y.Docs, docToY the same KicadDoc into both, sync updates both ways, assert docToFile output equals the single-seed output (currently it doesn't — layout doubles). Integration: open the same fresh project in two tabs simultaneously (Promise.all in the harness) and assert the room materializes cleanly.

Repro (2026-07-03): unit in web/pcbjam-shared/test/ysync-repros.test.ts (doubled {item} slot + materialization ≠ single-seed, plus a green CRDT-determinism baseline — both docs corrupt IDENTICALLY, which is why nothing downstream notices); e2e race in tests/kicad/ysync-two-tab.spec.ts (Promise.all start, equal settleMs, skip-guarded when the race happens not to fire — it fired on every observed run). See 16.