refactor: redesign sync around per-device peers, pure rules, and a single state snapshot

[gmaclennan]

Note

Tentative / experimental. This PR proposes a redesign of src/sync/. It is a breaking change to the project.$sync API, so comapeo-mobile and comapeo-cloud need migrating before it can land. It exists to be evaluated and discussed. This description is the source of truth for the design (it replaces an in-repo design doc).

Why

Sync is the hardest part of this codebase to maintain: we have repeatedly shipped bugs around completion detection ("is sync done?"), progress reporting, and connected-device counts. #1303 is the complete catalogue of known findings in the current implementation, with failing reproductions for the deterministic ones — its CI is red by design. All bug IDs in this description (A1, C1, D1, G1, …) refer to that catalogue.

The recurring bugs are not one-off mistakes; they trace to four structural problems:

1. Peer identity was split across three hand-synced maps (per-connection, per-device, per-core). An overlapping reconnect corrupted them, making a connected device disappear from sync state (D1).
2. Replication decisions were a feedback loop through the observation layer: what to replicate with a peer was decided by listening to throttled events about how replication was going. That interleave is where the capability-clobber bug lived (C1).
3. Completion had four disagreeing definitions (isSynced, isInitiallySyncedWithPeer, getSyncStatus, arePresyncNamespacesSynced), each with different edge cases (B1, B3, F1).
4. No lifecycle owner: SyncApi had no close(), so project close/leave tore down cores underneath live sync listeners and timers (A1, A3/A4, P0.13).

The design

SyncApi                  facade: intent (start/stop/background), autostop,
 │                       waitForSync, public state, close()
 ├── PeerManager         THE single registry of connected devices
 │    └── SyncPeer (×N)  one per DEVICE (not per connection): its connections,
 │                       its role capability, which namespaces replicate with it
 └── SyncProgress        observation: one CoreSyncState per core, aggregated
      └── CoreSyncState  into one immutable snapshot (throttled)

Three principles:

• One source of truth for "who is connected". Peers are keyed by device ID and own a set of connections; a device disconnects only when its last connection closes. A stale connection's teardown can never remove a live connection's state (fixes D1).
• Decisions are pure functions of a snapshot (sync-rules.js): the sync-mode truth table, the per-peer namespace-enabling rules, and the completion predicates are plain functions over (intent, capability, snapshot), unit-tested exhaustively. Every consumer — public state, waitForSync, the invite flow's initial-sync wait, autostop — derives from the same snapshot with the same predicates, so completion definitions cannot disagree (fixes B3, F1 structurally).
• Capability has one writer, no pseudo-states. A peer's per-namespace capability is read from its role in exactly one sequenced code path, seeded from NO_ROLE until known. BLOCKED_ROLE.sync.auth is now 'allowed' (like LEFT), so a blocked device still receives the record telling it it was blocked — replacing the fragile preserve-auth workaround that C1 defeated. Capability changes explicitly invalidate the derived-state cache (the bug that 92ee66e patched and 66f1d82 reverted).

The auth gate (new behavior)

Namespaces now enable with a peer in stages:

1. auth — always (it carries roles and core ownership).
2. config, blobIndex — only once auth sync with that peer is complete and indexed and the capability re-read.
3. data, blob — additionally once the whole initial group completes with that peer.

This closes G1 (the stale-role leak): a device holding an out-of-date "member" record for a removed device previously started sending it config immediately on connect, before syncing the auth records that would reveal the removal. It also fixes the subtler variant where the capability check raced the indexing of just-downloaded role records. The gate explicitly awaits indexer.idle() and loops until the capability read is stable against newly-arriving records.

Completion semantics

"Is sync done?" reduces to: with every connected device, is there nothing left to send and nothing left to receive? The two subtleties are how "left to send/receive" is counted before transfers actually start, and who counts as a connected device. One rule for each:

• What counts as pending: evidence-based wants. Before a peer starts pulling data, its bitfields tell us nothing — yet progress reporting must already show "this peer needs 200 blocks". So a connected peer is assumed to want every block it doesn't have. That assumption invents a transfer, and an invented transfer is a completion liability: if the peer has no way to even learn that a core exists (e.g. the cores of a device that was never a member, which we know about only through pre-haves), the invented transfer can never happen, and completion would wait for it forever. The rule: only apply the assumption when the peer can know about the core — it existed when the peer connected, or we have seen proof it knows (an open replication channel for it, pre-haves from it, an explicit want-range). Err on the "assume" side and completion hangs on phantom transfers; err on the "don't assume" side and completion reports done while real data is still pending — bug B1 was the old code doing the latter for cores discovered after a peer connected.
• Who counts: the connected-peer registry, after the auth gate. Completion is evaluated only over currently-connected devices, so a peer that disconnects mid-sync can never wedge the remaining peers: whatever it still had for us, or still needed from us, is deliberately forgotten on disconnect (there is currently no way to persist "a device out there has data we lack"). In the other direction, a connected peer always blocks completion until the channel handshake and its auth gate have finished — before that we don't know what the peer has, or even whether its role permits syncing, so declaring "complete" would recreate the old "initial sync reported complete before config synced" class of bug.

A channel still opening (length handshake pending) blocks completion; a closed channel with nothing outstanding does not — so a peer that has data sync turned off doesn't block everyone else's completion or autostop.

Public API (breaking)

$sync.getState() / 'sync-state' → {
  syncMode: 'stopped' | 'initial' | 'all',
  initial: { have, toReceive, toSend },   // totals: each unique block once
  data:    { have, toReceive, toSend },
  devices: {
    [deviceId]: {
      initial: { isSyncEnabled, isComplete, toReceive, toSend },
      data:    { isSyncEnabled, isComplete, toReceive, toSend },
    }
  }
}

Old	New
initial.isSyncEnabled / data.isSyncEnabled (top level)	syncMode !== 'stopped' / syncMode === 'all'
remoteDeviceSyncState	devices — now a reliable connected-device list across reconnects
…[d].<group>.want (they want from us)	devices[d].<group>.toSend
…[d].<group>.wanted (we want from them)	devices[d].<group>.toReceive
(none)	devices[d].<group>.isComplete
waitForSync('full')	waitForSync('all')

Count names are now always from the local device's point of view (have / toReceive / toSend) — the old want/wanted flipped meaning between the local entry and remote entries. Per-device isSyncEnabled means "replication channels are open with this device", i.e. both sides enabled it. SyncApi gains an idempotent close(), called first in project._close().

The top-level initial/data totals count each unique block once, however many devices hold or need it — they are not the sum of the per-device counts (which count a block once per device, and would over-report by up to the peer count). The union numbers come straight from the same bitfield walk that produces everything else; they shrink as blocks move and reach 0 exactly at completion, making them the right "progress toward done" denominators. For a one-call summary, aggregateSyncState() on the new dependency-free @comapeo/core/sync-state.js subpath export combines these totals with completion and device counts, without pulling backend code into a frontend bundle.

The API surface is documented for downstream implementers in docs/api/sync.md, with the old→new mapping in docs/guides/migrating.md.

Internal vocabulary: the three confusable tri-states are now visibly distinct — capability (allowed/blocked: may we sync?), channel (closed/opening/open: is the session up?), and completion (booleans from predicates, never stored as an enum). "presync"/"full" are retired for initial/all.

Bugs addressed (IDs from the #1303 catalogue)

ID	Fix
A1, A3/A4, P0.13	SyncApi.close() tears down listeners, timers, throttles, websockets; guarded async continuations
A2, P0.12	partially: sync now tears down first in project._close() and its continuations survive cores closing under it; the pre-existing datatype/indexer leave‖close race remains (tracked as a todo test)
B1	registry-gated completion + evidence-based wants + late-core back-fill
B2	the data-enable gate is per-peer, via the same shared predicate as every other completion question
B3	completion predicates skip genuinely-blocked namespaces, so invite-as-blocked resolves
C1	single sequenced capability writer; BLOCKED_ROLE.sync.auth = 'allowed' removes the workaround C1 defeated
C2	withdrawn as a bug (see #1303 §4): a blocked device's pre-block data deliberately keeps propagating — blocking revokes what the device may receive, not the project's claim on data it contributed. Only NO_ROLE (never-a-member) devices' cores are skipped
C3	capability reads are sequenced with a monotonic read id — a stale read can never overwrite a newer one
D1	per-device SyncPeer with a connection set; per-connection channel accounting in CoreSyncState
D2	disconnect is keyed on the single peer registry and idempotent — no early-return path skips cleanup
D3	gone (the class it lived in no longer exists)
E1–E4	public state is derived in one pass from one snapshot (E1); per-device group isSyncEnabled derives from live channels, never a half-seeded merge (E2); devices semantics are documented and per-group flags distinguish auth-only peers (E3); role transitions flow through the single capability writer + cache invalidation (E4)
F1	status enums are never merged; group completion is a predicate
G1	the auth gate (above)

(A0 and F2 were already fixed on main before this branch.)

Unchanged (wire compatibility)

The protocol is untouched: extension messages, pre-have broadcasting, blob download intents, and the bitfield-walk derivation math all stay as they were. The staged gate only changes when we replicate namespaces — old versions just see channels open slightly later. cross-version-sync.js (syncing with published @comapeo/core 2.0.1) passes.

Testing

The sync tests were rebuilt around a declarative scenario helper (test-e2e/sync-scenario.js): named devices with roles, explicit dial-based topology, seeding at realistic volumes (hundreds of observations per device — several code paths only activate past one 32-bit bitfield word — and real multi-MB photo fixtures), and assertions that state intent (assertDocsConverged compares full document content, assertNeverReceived proves non-delivery, recordStateInvariant proves "X never happened"). One concern per file: data / state / lifecycle / capability / connection / blobs. The bug repros from #1303 are ported as passing regression tests. The sync module exposes no test seams — every e2e assertion goes through the public API. See docs/development/sync-testing.md for the structure and coverage decisions.

Two scenarios cover paths beyond local discovery: rapid websocket reconnect against a real @comapeo/cloud server (the path where overlapping connections from one device actually occur in production — the D1 territory), and blob-original non-relay through a non-archive intermediate (working as designed: originals require direct contact with the authoring device).

Full suite: ~250 unit + ~260 e2e tests pass (one todo documenting the pre-existing leave‖close datatype race).

Review

The branch had a two-agent adversarial correctness review (core internals; integration + test-coverage regressions). Both HIGH findings it produced — pre-have-only cores blocking completion forever, and completion reading as vacuously true before a peer's role was known — are fixed in-branch with regression tests. Known accepted limitations (documented in code comments): the channel-open transition uses core.update() resolution, which isn't strictly per-peer (covered in practice by connect-time pre-haves); blocks a peer acquires mid-session on a closed channel aren't visible until the channel opens.

Resolved questions

• Public state shape: per-device state plus top-level per-group totals with union semantics (each unique block once — not derivable client-side, so no duplicated definition); project-wide aggregation lives in this package as aggregateSyncState() on the @comapeo/core/sync-state.js subpath (see above), so frontends don't each define their own meaning for the totals.
• BLOCKED devices' pre-block data: keeps propagating, same as LEFT. The risk blocking addresses is the device reading project data, not the data it wrote — which may be exactly the valuable data the project needs (and any bad edits are recoverable from the immutable log). Only NO_ROLE devices' cores are skipped.
• The leave‖close datatype race (outside sync): tracked with fix suggestions on project.close() race condition #407; stays a todo test here.