OR-Map tombstone GC: epoch machinery (dark prune) + re-admission gate + CI fixes [SPEC-342b/342c]#109
Merged
Merged
Conversation
…rait surface R1 confirm/relocate decision: RELOCATE tombstone_frontier.rs core-rust -> server-rust. Dependency-direction check (verified live): nothing in core-rust consumes the contract; the sole implementer is server-side (epoch counter, per-client frontier, reconnect gate). server-rust is the correct home; the wire tombstone shape stays Vec<String> in core-rust. Finalized trait/type surface (types/traits ONLY, no bodies): - Add pub(crate) sub-trait CausalFrontierRanking: CausalFrontier carrying tracked_cursors() -> Vec<(ClientId, Epoch)>, kept OFF the general CausalFrontier so a bare &CausalFrontier holder cannot compute a subset-MIN prune path; the point-lookup client_cursor() is not on the surface. pub(crate) encodes 'only the RAM-pressure forget-driver ranks' in the visibility system. Return shape is Vec (object-safe) so the downstream forget-driver may use &dyn dispatch. - Declare PruneSafety: CausalFrontier (supertrait EXPOSES low_water_mark; fleet-wide-MIN stays doc-enforced/reviewer-checkable, not type-enforced). - gate_decision_holds_at_commit consumes token by value; GateToken drops Clone (not Copy) for a genuine single-use/linear token. Re-admission safety rests on the commit-time re-check, not token unforgeability (impl-note for the gate child). - GateToken fields kept pub with an accepted-forgeability doc note (forgeability is not load-bearing); avoids a constructor body that would violate types-only. - confirm_apply/is_tracked doc-contracts state tracking is implicit via first confirm_apply, 'unknown == forgotten' collapses first-connect and forgotten-reconnect into the same gated path, and no track_client/register method exists or is needed. - MaxRetention/GateToken remain serde-free (server-internal metadata). cargo build/fmt/clippy (-D warnings) green for both crates.
- New dedicated module service/domain/key_writer.rs (NOT tombstone_frontier.rs, which stays traits/types-only per SPEC-342a) - DashMap<(map_name, key), Arc<tokio::sync::Mutex<()>>> registry; acquire() returns an owned RAII guard (OwnedMutexGuard) that can be held across .await points and is not tied to the registry's own lifetime - No eviction (default lifecycle per R5 option (a)): entries only grow via or_insert_with, documented as an accepted operational memory concern; naive eviction would race with a waiter holding a live Arc clone - Co-located tests prove (a) concurrent acquisitions on the same key serialize a read-then-write RMW with no lost update, (b) different keys do not block each other, (c) the registry never evicts (AC14)
…d-vs-add race) CrdtService::apply_single_op's OR_ADD branch performed an unlocked store.get -> merge -> store.put read-modify-write. Two concurrent OR_ADDs on the same key could each read the pre-mutation OrMap state and race to put, with the second put silently clobbering the first's merge and dropping an add. - CrdtService gains an internal key_writer: Arc<KeyWriterRegistry> field, constructed in new() so existing call sites are unaffected - Acquire the per-key guard right before the store.get -> merge -> store.put region (crdt.rs OR_ADD branch) and hold it through the single store.put merge-commit - Scope is OR_ADD only: the identical OR_REMOVE RMW is explicitly left unlocked here (342b's responsibility) -- a partial deployment closes add-vs-add only, not add-vs-remove interleaving - New concurrent_or_adds_on_same_key_lose_no_update test drives N concurrent OR_ADDs through the real apply path and asserts all survive; verified to fail (6/20 survivors) with the lock temporarily removed, confirming the test catches the real race
…mit put Drop the OR_ADD per-key writer guard the instant store.put returns rather than at end-of-arm scope, so the critical region is exactly store.get -> store.put. The payload construction that follows only clones owned locals and touches no shared store state; holding the lock across it would needlessly serialize unrelated writers to the same key. Hardens the documented get..put invariant at the code level and removes a latent footgun if post-put .await work is ever added to the arm. Surfaced by the cross-vendor /xreview (glm-5.2) as a HIGH; refuted as a live bug (no post-put await existed) but applied as defensive tightening.
…invariant Fold the xreview-surfaced latent footgun (a future second CrdtService over the same store would mint distinct per-key mutexes and reopen the SPEC-333b race) into a WHY-comment on the key_writer field. Comment-only; no logic change.
…en, AuthAck.deviceId/deviceToken) + Zod parity - AuthMessage gains optional deviceToken (additive; old clients unaffected; Default intentionally not derived — required token field) - AuthAckData gains optional deviceId + deviceToken (returned only on mint/rotate) - TS Zod parity in base-schemas.ts + client-message-schemas.ts
…R/fencing - device_identity.rs: present-or-mint credential store over shared MapDataStore under reserved namespace; injective length-prefixed (principal,deviceId) key; SHA-256 hash-at-rest; constant-time verify (subtle); fail-open-to-new-identity mint; revoke; frontier_client_id helper - connection.rs: ConnectionMetadata.device_id (one-shot); ConnectionRegistry device-ownership map with atomic TAKEOVER (DashMap shard-locked swap), is_current_owner fencing primitive, release on disconnect only-if-still-owner; module registered via #[path] to hold the <=5-file cap - fix collateral AuthMessage/ConnectionMetadata struct literals for the new fields (tests/benches)
…O_AUTH) - shared bind_device_identity helper: present-or-mint, one-shot device_id bind, TAKEOVER + close displaced; fail-open (never blocks auth) - JWT mode: Ok(principal) arm runs present-or-mint, returns deviceId/deviceToken in AUTH_ACK (one-shot is structural — single Phase-1 bind then break 'auth) - NO_AUTH mode: opportunistic Phase-2 Auth intercepted at WS layer under sentinel namespace, replies AUTH_ACK, never enters data-plane dispatch; explicit one-shot guard drops repeat AUTH; JWT Phase-2 re-auth stays unsupported (dropped)
…dential
- Load persisted deviceToken/deviceId from meta storage (memoized read) and
present deviceToken on the AUTH message so the server rebinds device identity
- NO_AUTH connect now sends an opportunistic AUTH{token:''} for present-or-mint,
with the existing grace timer as legacy fallback (never anonymous-auth deadlock)
- Thread AuthAckMessage into handleAuthAck; persist minted/rotated credential,
keep existing token when the ack omits deviceToken (plain re-bind)
- Legacy-server inference: proceed auth-optional on a non-AUTH_ACK first message
or grace timeout, degraded-to-legacy without a deviceId
…ence
- New SyncEngineDeviceCredential suite: mint-on-AUTH_ACK persisted and
re-presented on reconnect, re-bind keeps existing token, legacy paths
(no AUTH_ACK / non-AUTH_ACK first) proceed without a device identity
- Update auth-optional + SyncEngine connection tests for the new NO_AUTH
opportunistic AUTH{token:''} present-or-mint behavior
…s, # Errors, Debug fields) - backtick NO_AUTH in doc comments (clippy::doc_markdown) - checked casts in now_millis/mint (u128->u64, u64->i64 via try_from) - # Errors sections on present_or_mint/revoke (clippy::missing_errors_doc) - ConnectionRegistry Debug impl includes new device_ownership/conn_identity fields
…l-collision (xreview) - cross-vendor review: a JWT sub literally equal to the content sentinel collided with the NO_AUTH namespace (length prefix can't disambiguate identical content) - replace content sentinel with a structural leading tag (a=authenticated principal, n=no-auth) that principal content can never forge; unify credential-store + frontier/ownership keying on one identity_key - present_or_mint/mint/revoke now take principal_id: Option<&str>; drop NO_AUTH_SENTINEL const - add noauth_namespace_cannot_be_forged_by_any_principal_content test
… fix - New additive DeviceHelloMessage (base.rs) + DeviceAckData (client_events.rs) structs and DEVICE_HELLO/DEVICE_ACK Message enum variants; orthogonal to AUTH so a JWT server silently drops a token-less client's device presentation instead of AUTH_FAIL+teardown. - Zod parity: DeviceHelloMessageSchema, DeviceAckMessageSchema + discriminated-union members. - Fix review C1: add device_token:None to the 3 AuthMessage test literals in base.rs (workspace clippy --all-targets was red; -p topgun-server alone never compiled that target).
… claim liveness
- websocket.rs: replace the opportunistic empty-token AUTH interception with a
Phase-2 DEVICE_HELLO handler (both modes; present-or-mint -> DEVICE_ACK; one-shot
guard; never enters data-plane dispatch). Phase 1 untouched, so a JWT server keeps
silently dropping a token-less client's DEVICE_HELLO instead of AUTH_FAIL+teardown.
- device_identity.rs: hand-written redacting Debug on DeviceBinding (raw secret can no
longer leak via {:?}); tracing-capture test asserts the secret/token never hit logs
on mint or verify (AC3); plus Debug-redaction unit test.
- connection.rs: M6 post-insert liveness re-check in claim_device_ownership so a
self-disconnect interleaving cannot orphan ownership to a dead connection; regression test.
- classify.rs: route DEVICE_HELLO (client->server) / DEVICE_ACK (server->client) as
non-dispatchable handshake frames.
…not empty-token AUTH - Token-less connect now sends a dedicated DEVICE_HELLO frame (sendDeviceHello) instead of an empty-token AUTH — the latter is AUTH_FAIL'd + disconnected by a real JWT server, breaking the 'connect first, supply token later' flow (review C2). A JWT server silently drops DEVICE_HELLO (non-AUTH) and sends AUTH_REQUIRED, so the client parks and Case 3 is preserved. - handleDeviceAck persists deviceId/deviceToken from DEVICE_ACK and completes the connection auth-optional; credentialed clients keep bundling deviceToken on the real AUTH. - sendAuth no longer emits an empty-token AUTH; legacy inference keys off DEVICE_ACK. - Route DEVICE_ACK through ClientMessageHandlers; tests updated to the DeviceHello flow.
…egression) - Drives a real TopGunClient + real JWT server (mocks don't validate JWTs, which is what hid the original regression): a token-less client survives (parks in AUTHENTICATING, not torn down) and authenticates after setAuthToken (Case 3). - Raw-ws guarantee: DEVICE_HELLO to a JWT server is not AUTH_FAILed, the socket stays OPEN, and a subsequent valid AUTH authenticates on the same connection.
…diff - Backtick NO_AUTH in DeviceHello/DeviceAck doc comments. - claim_device_ownership consumes its String param (move into forward insert; keep a clone for the liveness re-check) — fixes needless_pass_by_value. - M3 tracing-capture visitor uses write! instead of push_str(&format!(..)).
…it (xreview HIGH) Cross-vendor review found the liveness re-check broke TAKEOVER fencing: when a DEAD connection's claim displaced a LIVE owner A, the re-check removed the key entirely (orphaning it to nobody) AND returned Some(A), so the caller tore down the live owner for a takeover that never happened. Now: if the dead claim displaced a prior owner, atomically restore it (get_mut holds the shard write-lock across the check-and-swap, guarding a racing newer takeover); report None so the caller does not cancel the prior owner. Regression test claim_by_disconnected_connection_restores_displaced_live_owner.
…cant entry The M6 restore branch used get_mut, which no-ops when a concurrent remove() already deleted device_ownership[K] (its remove_if(owner==id) matched), so a displaced LIVE prior owner was never restored -- orphaned to nobody, failing is_current_owner forever (its cursor ACKs would be rejected). Extract the reconciliation into reconcile_dead_claim() and make the forward-ownership update atomic via DashMap Entry: the Vacant arm re-inserts the displaced live owner; Occupied-by-us restores; a newer takeover is left untouched. Deterministic regression tests for all three arms (the Vacant arm is the closed bug).
If ensureDeviceCredentialLoaded is slow, the grace timer can complete the connection auth-optional before sendDeviceHello's DEVICE_ACK returns. handleDeviceAck now persists the identity unconditionally but only re-drives handleAuthAck while deviceAckPending is still set, so a late DEVICE_ACK cannot bounce CONNECTED -> SYNCING.
…view HIGH) The Vacant-restore fix over-corrected: prev (the displaced prior owner) can itself disconnect between the claim and reconcile, so its own remove() already ran and will never run again. Unconditionally re-inserting it pinned a permanently-stuck dead owner that is_current_owner affirms forever. Liveness-check prev before restoring in both arms; if no live prior owner exists, leave the identity ownerless. Regression test reconcile_dead_claim_does_not_restore_a_dead_prior_owner.
- ClientApplyAckMessage { cursor: u64 } via serde_number integer helper; NO
identity field on the wire (server derives (principal, deviceId) from the
authenticated connection)
- ClientApplyAck enum variant on Message (#[serde(tag="type")] owns the
discriminant, rename CLIENT_APPLY_ACK)
- mechanical exhaustive-match arm in classify.rs (non-dispatchable control frame,
handled at the websocket layer; zero-cascade additive-variant consequence)
- round-trip + integer-wire-format + single-type-discriminant tests
…on (G2) - TombstoneFrontier (tombstone_frontier_impl.rs): durable per-device confirmed- apply cursor store; advance rule new = max(stored, min(claimed, delivered_conn, current_max_epoch)); loss-conservative, delivered-clamped, globally-bounded, monotone; inclusive fencepost; LWM = MIN across tracked; settable/injectable current_max_epoch + per-connection delivered (default at Wave 2); regression- claim gate (never rolls stored back); best-effort redb persistence in a NEW additive keyspace + rehydration - websocket.rs: route CLIENT_APPLY_ACK -> confirm_apply, identity derived from ConnectionMetadata.principal + .device_id (never the wire), rejected unless is_current_owner (342i fencing); rehydrate persisted cursor into the frontier on connection establishment BEFORE any ACK; drop per-connection delivered state on disconnect - AppState.frontier wired over the shared data store (module.rs, topgun_server.rs) - 14 frontier unit/persistence tests: dropped-ACK, replay/reorder, global-bound, delivered-clamp, cross-principal, two-devices-LWM-min, fencepost-inclusive, regression-gate, cursor-survives-restart, reconnect-rehydrates-before-ACK
…mit (G3) - ClientApplyAckMessageSchema (CLIENT_APPLY_ACK, cursor: number) in packages/core + added to the discriminated MessageSchema union (cross-language parity) - SyncEngine emits CLIENT_APPLY_ACK ONLY after the applied server batch is durably committed to IndexedDB (apply-not-receive): emitConfirmedApply runs after the applyServerEvent await chain resolves; cumulative-monotonic (non-advancing epoch not re-sent); forward-compatible epochOfServerEvent (0 until prune-wiring stamps epochs on the delta wire -> inert, not incorrect) - tests: emit-after-durable-commit ordering, cursor monotonicity, epoch-less inert
… x2) - forget_client now async + deletes the durable redb cursor, so a forget is durable (forgotten -> unknown -> full resync). Left as-was, rehydrate on reconnect silently re-tracked a forgotten client at its stale cursor and dropped the low-water-mark below an already-pruned watermark -> resurrection. - low_water_mark vacuous case returns 0 (prune nothing) instead of current_max_epoch (default u64::MAX). Rehydration is lazy, so an empty post-restart frontier means 'no client reconnected yet', not 'no client to protect' -> the old value licensed pruning tombstones a not-yet-reconnected laggard still needs. Conservative direction, matches the spec philosophy. - Regression tests: forget_client_deletes_durable_cursor_so_rehydrate_is_noop, empty_frontier_lwm_is_zero_prunes_nothing.
…view LOW x2) - emitConfirmedApply advances lastAckedEpoch ONLY on a successful sendMessage (returns bool, never throws). A failed send previously advanced the cursor anyway, permanently skipping that epoch's ACK (monotone: a lower epoch is never re-sent) -> server cursor stuck. - setAuthToken resets lastAckedEpoch to 0: a new token may name a different principal whose server-side per-device cursor is independent; without the reset the new identity's ACKs are suppressed by the prior identity's epoch. - Tests: failed-ACK-send-does-not-advance-and-retries, setAuthToken-resets-cursor.
- add covering_epoch: Option<u64> to ORMapSyncRespRoot/Leaf/DiffResponse payloads (camelCase, skip_serializing_if/default, serde_number decode) — wire tombstones: Vec<String> byte layout unchanged - Zod parity (coveringEpoch, optional non-negative int) in sync-schemas - roundtrip + omitted-when-none + integer-not-float tests
…-safety - server-authoritative monotonic epoch counter stamped from the op sequence (epoch<->seq lockstep, EPOCH_WIDTH ops/epoch), never from client tag millis - RAM-only epoch->tags / epoch->max_seq index (pure cache); TombstoneRef - PruneSafety::is_epoch_prune_eligible folds over low_water_mark ONLY (342a contract); reserved epoch-0 sentinel rejected at trait level - drain_prunable applies the call-site conjunction (LWM && durable watermark), iterates ACTUAL index keys (never 0..=max); durable_epoch_watermark returns constant 0 in this child (dark by construction) with test-only injection - feeds current_max_epoch ACK bound; current_epoch = covering epoch - unit tests: AC2 server-authoritative stamping, epoch<->seq lockstep, AC3(i) one-behind-client pins fleet-wide, AC3(ii) watermark conjunct load-bearing, AC3a dark-by-construction (first epoch>=1, sentinel-0 safe)
- CrdtService gains .with_frontier() / .with_key_writer() builders (shared frontier + per-key writer, new() call sites unaffected) - OR_REMOVE apply: take the per-key writer around the RMW, stamp each genuinely-new tombstone with the current server epoch, run the wholesale prune sweep after apply (dark: watermark 0 -> drains nothing) - delete the 'grows unbounded' stopgap comment - prune_epoch_tombstones helper (shared with sync.rs): drops drained tags from storage RAM+redb under the per-key writer - AC4 test: prune wired into the OR write path (injected watermark drops the epoch's tombstone; a higher pinned epoch survives)
- SyncService gains .with_frontier(frontier, key_writer) builder - OR-Map root/leaf/diff responses convey the covering epoch (current max stamped epoch) and mark it delivered on the connection for the 342e ACK clamp — including the empty-diff root, for empty-diff liveness - run the wholesale prune sweep at OR-Map leaf-handler entry (dark: no-op in production; drops from storage under the shared per-key writer when active) - AC4 test: prune wired into the SYNC leaf; AC3b server half: covering epoch conveyed + marked delivered on ORMapSyncInit
- construct the causal frontier + shared KeyWriterRegistry once in build_services and thread the SAME instances into CrdtService (.with_frontier / .with_key_writer), SyncService (.with_frontier) and AppState — no second frontier (dual-router STOP-rule satisfied: sole production CrdtService::new) - TOPGUN_EPOCH_WIDTH env (default 1000), surfaced in a startup config log line
- ORMapSyncHandler learns coveringEpoch from OR-Map sync responses and confirms it after durable apply via onCoveringEpochApplied -> emitConfirmedApply - empty diff (root matches) ACKs the covering epoch so an up-to-date client still advances its cursor; root mismatch defers the ACK until leaves/diff are applied - ORMapCoveringEpoch tests: empty-diff ACK, mismatch defers, leaf/diff ACK, missing/zero epoch never ACKed
…d-drop (xreview HIGH x2 / MED x2) Cross-vendor xreview (glm-5.2) findings on the sf-342b diff, all latent (bite when SPEC-342j activates the watermark), fixed now: - HIGH (TOCTOU): all five covering-epoch conveyance sites in sync.rs read the epoch AFTER computing the root/entries — a concurrent OR_REMOVE stamped in between made the client ACK an epoch whose tombstones it never received. Epoch now read BEFORE the data at every site; WHY-comment on the helper makes epoch-before-data a stated load-bearing ordering. - HIGH (orphaned index entries): drain_prunable destroyed epoch_tags entries before the storage drop succeeded; a get/put error silently orphaned the tag un-prunable forever. drain now returns (Epoch, TombstoneRef); prune_epoch_tombstones re-inserts via restore_tombstone_ref on ANY failure; failure logs escalated debug -> warn. Regression test: restore round-trips through a second drain. - MED (342a contract deviation): is_epoch_prune_eligible used inclusive >=; the LOCKED contract says 'advanced PAST epoch' (strict). Inclusive also admitted a still-accumulating-epoch prune trace (covering epoch = current_epoch, still open at width > 1). Now strict >; tests updated, new pin test: LWM == epoch NOT eligible, LWM == epoch+1 eligible. - MED (perf): drain evaluated the LWM fold per epoch key on every OR_REMOVE and SYNC-leaf request even in dark mode. Early-return on watermark == 0 (production dark fast-path) + cheap watermark conjunct short-circuits the fold. - MED (test-only injector): set_durable_epoch_watermark now #[cfg(test)] — production code structurally cannot activate the prune early.
…llback (xreview LOW x1) An invalid or zero TOPGUN_EPOCH_WIDTH silently fell back to the default, leaving the operator believing their setting took effect. The rejected value and the default now surface in a boot-visible tracing::warn (project precedent: config surprises must be visible at startup).
… + mocks
The client-side cross-map covering-epoch min-barrier fix needs to enumerate
persisted-but-not-yet-opened OR-Map stores (the reserved
`__sys__:{mapName}:tombstones` meta-key convention) to build a consistent
held-map snapshot. Add getAllMetaKeys() to IStorageAdapter, mirroring
getAllKeys() for the meta namespace, and implement it in IDBAdapter,
EncryptedStorageAdapter (plaintext passthrough — key names are never
encrypted), and mcp-server's InMemoryStorageAdapter.
Update every test mock/fake IStorageAdapter implementation across
client, adapters, and adapter-better-auth so the interface addition
does not silently degrade the barrier in test environments (same
pattern SPEC-321 used for commitWrite/deleteOp).
The covering-epoch/confirmed-apply cursor is GLOBAL across all OR-Maps, but the client was confirming it off a SINGLE map's sync completion. Device B holding maps `tags` and `other_map` could ACK a covering epoch conveyed by `tags`'s sync response alone, even though `other_map` (in the same epoch window) had not synced on that connection yet — dark today (durable_epoch_watermark constant 0) but a live resurrection vector the moment SPEC-342j activates the real watermark. Fix (per the cross-vendor-locked design): - SyncEngine now tracks per-OR-Map coverage (`orMapCoverage`) and ACKs only `min(coverage[m] for m in heldOrMaps)`, monotone — a held map with no completed sync this connection contributes 0, stalling the ACK rather than over-claiming. - `heldOrMaps` is a consistent snapshot (`computeHeldOrMapNames`) taken ONCE per connection, before the first sync-init message goes out: the union of open ORMap instances and persisted-but-not-yet-opened stores (enumerated via the new getAllMetaKeys storage API). - `startMerkleSync` now instantiates + restores every persisted OR-Map in the held set (`instantiateAndRestoreOrMap`) so an unopened store actually gets synced instead of permanently stalling the barrier. - `registerMap` late-joins a map opened after the snapshot with coverage 0, so it blocks further advance without retroactively narrowing or widening an already-computed barrier. - `ORMapSyncHandler.onCoveringEpochApplied` now reports the mapName alongside the epoch so SyncEngine can fold coverage per map. `handleServerEvent`/`handleServerBatchEvent` (live-push epoch, still dead wire-wise) intentionally keep calling emitConfirmedApply directly rather than routing through the new barrier, documented at epochOfServerEvent to avoid silently reintroducing the same gap once that path is wired. Tests: multi-map interleaving, persisted-but-not-instantiated map enumeration, empty held-set, late-opened map, and per-connection snapshot/coverage reset (SyncEngine.test.ts); handler-level mapName propagation (ORMapCoveringEpoch.test.ts).
…cursors Companion to the client-side cross-map ACK min-barrier fix. Before that fix a client's device-wide confirmed-apply cursor could be inflated by a single OR-Map's sync completion while other held maps lagged behind. Any such inflated cursor is already durably persisted server-side (redb, CURSOR_MAP) and would otherwise survive a clean restart straight into SPEC-342j's prune activation. Version-bump the reserved cursor keyspace (_topgun_tombstone_cursors -> _topgun_tombstone_cursors_v2) so a pre-bump row is never read back — orphaned, not migrated; reclaimed later by 342f's TTL sweep. Safe today because pruning is dark (durable_epoch_watermark constant 0): a "loss" here degrades to the already-accepted unknown -> forgotten -> full-resync path. No client-side persisted-cursor version bump is needed: the client's lastAckedEpoch was verified to be purely in-memory (reset every connection), so there is no durable client-side row to poison. Test: a cursor written under the retired pre-bump keyspace name is never rehydrated into a fresh frontier over the same store.
…iew HIGH x2 / LOW x1) - Held-set enumeration failure now fail-closes ALL covering-epoch ACKs for the connection (heldSetIncomplete) instead of silently running the min-barrier over a partial in-memory-only universe - Server push events (single + batch) route their epoch through applyMapCoverage's cross-map min-barrier instead of calling emitConfirmedApply directly; batch-wide highest-epoch shortcut removed - IDBAdapter.getAllMetaKeys throws (never a silent []) when the database is unavailable, feeding the fail-closed path - Late-join registerMap WHY-comment: emitted ACK stands only because a genuinely-new map is empty-at-open; the missed-persisted-store sibling is closed by fail-closed enumeration - Apply-not-receive tests rewritten to the barrier-era contract (OR-Map + driven held-set snapshot); 2 new regression tests (enumeration failure, push-path barrier routing)
…ly enumeration gap)
Review v2 Major: the covering-epoch held-set snapshot enumerated persisted
OR-Maps only via the :tombstones meta-key, which add-only maps never write, so a
lazily-opened add-only store was invisible to the barrier and the device ACK
could advance past its un-received tombstones (cross-map resurrection vector,
latent until SPEC-342j activates the prune).
- Eager `__sys__:{name}:ormap` existence marker at BOTH durable-write seams:
transactionally in recordOperation's commitWrite (local writes — verified NOT
routed through the persist helpers) and marker-first in persistORMapKey/
persistORMapTombstones (server-origin applies). Once per session.
- One-time legacy-store backfill inside computeHeldOrMapNames (gated by a durable
done-flag): scans ALL kv keys, attributes OR-Maps by records-array shape,
stamps markers; done-flag set only on success so a throw fail-closes the
connection (heldSetIncomplete) and retries next connection. Correctness, not
optional: the _v2 cursor reset gives a fresh server cursor no conservative
fallback, so an unmarked legacy add-only map would build inflated ACKs.
- Enumeration matches :ormap OR :tombstones; registerMap WHY-comment corrected
(the 'every persisted store is in the snapshot' premise is now actually true).
Tests: THE REGRESSION (legacy add-only store discovered + blocks device ACK) +
local/server marker seams + marker-first ordering + backfill discriminator/
done-flag + backfill-failure fail-closed. Client 709/709, adapters 47/47,
mcp 109/109.
xask (glm-5.2) drove the seam correction: local write path bypasses the persist
helpers (Critical) -> marker at both seams; marker-first ordering; scan-all-keys;
inherit TODO-577 :-prefix ambiguity; scale-check confirmed backfill is correctness.
…ackfill CAN hide a real map (Review v3 xreview minor) The backfill's :-prefix note conflated two distinct collision cases. The colon-in-VALUE case (LWW array value under a first-colon-split name) is over-conservative (adds a phantom, coverage 0). The colon-in-NAME case (map name contains ':') splits to the wrong prefix, never stamps the real map's marker, and CAN hide it from the held-set snapshot -> ACK inflation once the durability watermark activates. Documented as the real resurrection sub-case tracked in TODO-577.
…wire fields - TombstoneFrontier: is_forgotten (unknown==forgotten OR cursor lag > K), is_protection_active (gate goes live with 342j watermark — gate-before-activation), set_forget_lag_epochs + DEFAULT_FORGET_LAG_EPOCHS, gate_decision_holds_at_commit inherent wrapper; extend the commit-time re-check to be lag-aware (not is_tracked-only) - ORMapSyncInit.claimed_epoch (client cursor for regressed-replica detection) + ORMapSyncRespRootPayload.full_resync (server->client REPLACE signal), both additive - update construction sites (classify + soak bench)
…g + REPLACE routing (sync.rs) - handle_ormap_push_diff: connection-keyed pre-apply gate fires BEFORE merge, gating inbound tombstone union AND records; per-key single-writer held from the gate re-check through store.put (gate->commit TOCTOU close, R5); commit-time re-check via lag-aware gate_decision_holds_at_commit (R13 mid-batch discipline); fail-closed on absent frontier/key_writer (R12); verbatim-timestamp accept-as- untrusted (R11); third prune site wired, dark by construction (R2) - resolve_client_id off server-authenticated (principal, deviceId) metadata (G9) - covering_epoch gated: no eager set_delivered for a forgotten/regressed client (R9) across all three OR-Map sync handlers; deferred to resync completion (R8) - sync_gated: forgotten/unknown/regressed detection + is_regressed on sync-init; handle_ormap_sync_init routes a gated client to full-snapshot REPLACE (full_resync) without advancing delivered_conn / rolling back the stored cursor (R6/R7)
- op_path_gated: drop a forgotten client's OR-bearing op on the direct-connection write path (handle_client_op + handle_op_batch), keyed by the server-authenticated (principal, deviceId) identity. Non-load-bearing / dark until 342j: the OR_ADD apply regenerates the tag, so a same-tag op-path resurrection is vacuous; the load-bearing surface is ORMapPushDiff. Inert for HTTP/anonymous/system (no connection)
…h closures - ac5: forgotten/unknown client push blocked before merge (protection active) - ac6: tracked client push merges under the per-key gate->commit span - ac17: unknown identity (no device) rejected fail-closed - ac4: third prune site in push-diff is dark by construction (drops zero epochs) - ac7: forgotten client sync-init routed to full-snapshot REPLACE (full_resync) - ac8: regressed replica routed, stored cursor not rolled back, delivered not advanced - ac10: forgotten sync-init does not advance delivered_conn (covering_epoch gating)
…C discard - ORMapSyncHandler: full_resync root routing -> discard local + pull snapshot, covering-epoch NOT confirmed at REPLACE-root time (deferred to post-snapshot); sendSyncInit reports claimedEpoch (confirmed-apply cursor) for regressed detection - SyncEngine.replaceOrMapFromSnapshot: clear materialized OR-Map (memory + durable keys + tombstones), discard pending OR ops whose HLC precedes the snapshot boundary (subsumed), keep at-or-after ops for re-drive through the gated push; wire onFullResync + getClaimedEpoch into the ORMapSyncHandler config - tests: fullResync REPLACE routing + claimedEpoch (handler), AC15 HLC-boundary oplog discard (SyncEngine); update offline-durability handler config
- cargo fmt on sync.rs/crdt.rs; clippy: doc backticks, let-else, too_many_lines allow - prettier/eslint on client REPLACE files + tests
replaceOrMapFromSnapshot swallowed deleteOp failures and could splice a subsumed pending op from memory while it survived on disk (saveOpLog does NOT persist the opLog — deleteOp is the only durable removal), so on restart the op reloaded and re-drove through the gated write path, resurrecting the removed value once its tombstone was pruned. - Delete durably FIRST, then splice from memory (never the reverse); do not swallow deleteOp failure — an unresolved rejection aborts REPLACE so the op stays in BOTH memory and disk, and the resync is retried. - Treat a non-numeric subsumed op id as a HARD failure (abort), not a silent skip; op ids are always numeric (IndexedDB autoincrement). - Drop the misleading idsToDelete>0 => saveOpLog() (a no-op for ops). - Tests: a failed durable delete aborts (op not dropped-from-memory-kept- on-disk); a non-numeric id aborts.
epoch_at_gate was constructed at the push-diff gate but never read: the commit-time re-check (gate_decision_holds_at_commit) does a LIVE is_forgotten re-read of the client against the CURRENT epoch, never comparing a snapshotted gate-time epoch. Drop the dead field + its construction, and correct the GateToken docs to state the freshness signal is read live at commit. - tombstone_frontier.rs: remove epoch_at_gate; doc now says the token carries only client identity, freshness read live at commit (TOCTOU semantics unchanged). - sync.rs: drop the epoch_at_gate initializer at the gate construction site.
…init None claim Two push-diff/sync re-admission gaps in the SPEC-342c gate: FIX 1 (push-diff gate): the active gate admitted a push iff !is_forgotten, which does NOT satisfy R10 point-1 — a regressed/not-yet-admitted connection that flushes a queued push BEFORE completing its resync has a high STORED cursor (is_forgotten=false) and was wrongly admitted, resurrecting adds whose tombstones were pruned. Now also require delivered_conn > 0 (the post-resync admission signal, advanced only on snapshot-resync completion) inside the is_protection_active branch; a None connection_id fails closed. FIX 2 (sync-INIT only): a client omitting claimed_epoch (None) evaded the regressed check and was eagerly re-admitted. Force gated=true when claimed_epoch.is_none() AND protection is active, at the sync-init call site ONLY — the merkle-bucket / diff continuations still pass None as post-init reads and keep plain sync_gated semantics. - Tests: push held while delivered_conn==0 then merges after admission; a None claim under active protection forces full-resync even for a tracked client. ac10 tracked arm now reports its real claimed_epoch.
… on REPLACE The persistedKeys removal loop swallowed storageAdapter.remove failures (.catch + log + continue), unlike the pending-oplog discard which aborts. A transient remove failure left an orphan record on disk after its tombstone was cleared -> on restart the orphan re-materializes with no suppressing tombstone -> merkle re-push -> resurrection once the server prunes the tombstone. Remove durably FIRST (fail-closed, no swallow) and BEFORE map.clear(), so an abort leaves both memory and disk intact for a clean retry. Regression test: a materialized-key remove failure aborts the REPLACE with in-memory state preserved.
The merkle-bucket and diff-request handlers passed claimed_epoch=None to sync_gated, so is_regressed could not re-run there. A regressed replica (stale-high stored cursor -> is_forgotten=false) mid-resync had its delivered_conn eagerly advanced by covering_epoch, re-enabling admission before the snapshot landed (R8/R9 violation). Add sync_gated_continuation which also gates on the not-yet-admitted delivered==0 signal (the same signal the push-diff gate uses); dark by construction. Regression test: a not-yet-admitted regressed connection's merkle-bucket does not advance delivered_conn under active protection.
- deps: bump crossbeam-epoch 0.9.18 -> 0.9.20 (RUSTSEC-2026-0204, published 2026-07-06 — the sole run-level failure of every nightly since Jul 7) - ci: anchor all load-harness --json-output paths to $GITHUB_WORKSPACE — cargo runs bench binaries with CWD = the package dir, so results landed in packages/server-rust/ while the validate steps (bash -e + jq) and upload-artifact read from the workspace root; under bash -e the first jq on the missing file exits 2, so the vector gate has never passed on schedule - ci: perf-gate timeout 15 -> 30 min (cold-cache harness build alone can eat 15; every nightly Jun 28..Jul 6 died as a timeout cancellation)
Deploying topgun with
|
| Latest commit: |
0882159
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://56384800.topgun-f45.pages.dev |
| Branch Preview URL: | https://fix-sf-342b-ormap-marker-enu.topgun-f45.pages.dev |
…ters + prettier - tests/integration-rust/helpers/memory-storage.ts, tests/doc-tests/helpers/ memory-adapter.ts, examples/sync-lab/src/lib/memory-storage.ts implement IStorageAdapter but were missed by the getAllMetaKeys interface addition — broke Build/doc-tests/integration/vitest CI jobs at compile time - prettier normalization on 2 files that predate this branch (format:check gate was red on them)
…ites) Four integration test files define their own inline IStorageAdapter implementations missed by both previous cascade passes — the suites failed at ts-jest compile time (mcp-server, live-query-limit-clamp, reconnect, cluster-routing).
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.
Summary
55 commits closing two SPEC-342 children plus CI repairs. Everything here went through the full SpecFlow lifecycle locally (per-spec: fresh-context audits, cross-vendor /xask design checkpoints, implementation, fresh-context reviews with /xreview gates — 3 review cycles for 342b, 2 for 342c).
SPEC-342b — server-authoritative epoch machinery, prune DARK by construction
epoch→tagsindex stamped at OR_REMOVE apply;PruneSafety::is_epoch_prune_eligible(fleet-wide-MIN LWM fold) + call-siteprune_boundconjunction.durable_epoch_watermark ≡ 0until SPEC-342j wires the real byte-durability watermark (gate-before-activation: 342j also depends on 342c, so no prune-without-gate window ever exists).__sys__:{name}:ormapmarkers + legacy backfill for add-only stores; cursor keyspace bumped to_v2to orphan pre-barrier inflated cursors).SPEC-342c — forgotten-client RE-ADMISSION gate
handle_ormap_push_diffkeyed by server-authenticated(principal, deviceId)(342i), gating records AND the inbound tombstone union, held under the 342d per-key single-writer gate→commit (TOCTOU close).covering_epoch/delivered_conngated for forgotten/regressed connections (a forgotten client can no longer re-admit itself via sync-init); regressed replicas get an authoritative REPLACE resync + client pending-oplog HLC discard; op-path defence-in-depth gate.CI (nightly reds the repo has been emailing about)
crossbeam-epoch0.9.18 → 0.9.20 (advisory published Jul 6 — the sole run-level failure of every nightly since Jul 7).--json-output results-vec-*.jsonlanded inpackages/server-rust/while the validate step (bash -e + jq) read the workspace root → exit 2. All harness output paths now anchored to$GITHUB_WORKSPACE(perf gate had the same latent bug).Test evidence (local, full gates at each review)
-D warnings/ fmt / prettier clean;pnpm test:simgreen at spec gatesNotes for reviewers
Optionfields; old clients degrade gracefully — documented per spec)..specflow/is local-only and not part of this PR.