Summary
In the collab relay's op handler, a batch is applied to the GraphStore, broadcast to peers, and acked to the sender before/independently of the durable oplog.append, which is wrapped in a try/except that logs and continues on failure. When the append fails, the op is live everywhere except the durable log and the dedup index.
backend/topix/api/router/collab.py (_handle_message, ~L385–L403):
seq = await oplog.next_seq(board_id)
room.seq = seq
await apply_batch(graph_store=..., ops=ops) # applied to factory
try:
await oplog.append(board_id, seq, batch) # <-- may fail; caught + continue
except Exception:
logger.exception(...)
room.remember_batch_unlocked(seq, batch)
# ... broadcast peer-op + op-applied
Impact
- Catch-up gap (permanent). A peer that was offline reconnects with
since_seq < seq: batches_since skips the missing seq, so it never receives that op via catch-up. The code comment says it "self-heals on the next full snapshot," but a catch-up-mode client never receives a snapshot, so it does not self-heal.
- Reconnect double-apply. The original sender replays its outbox;
seq_for_batch returns None (never durably recorded), so the batch is applied again at a new seq → double apply + serverSeq divergence between live and reloaded state.
Suggested direction
Make durability ordering explicit: either append to the oplog before broadcasting/acking (fail the op if the append fails, so the client keeps it pending and retries), or treat a failed append as a hard error for that op rather than a soft log-and-continue. At minimum, don't ack op-applied for a batch that isn't durably recorded.
Context
Found in the second code-review pass of the offline-first PR (#154). Relay is flag-gated (v2), so this is sync-hardening, not a release blocker.
Summary
In the collab relay's
ophandler, a batch is applied to the GraphStore, broadcast to peers, and acked to the sender before/independently of the durableoplog.append, which is wrapped in atry/exceptthat logs and continues on failure. When the append fails, the op is live everywhere except the durable log and the dedup index.backend/topix/api/router/collab.py(_handle_message, ~L385–L403):Impact
since_seq < seq:batches_sinceskips the missing seq, so it never receives that op via catch-up. The code comment says it "self-heals on the next full snapshot," but a catch-up-mode client never receives a snapshot, so it does not self-heal.seq_for_batchreturnsNone(never durably recorded), so the batch is applied again at a new seq → double apply +serverSeqdivergence between live and reloaded state.Suggested direction
Make durability ordering explicit: either append to the oplog before broadcasting/acking (fail the op if the append fails, so the client keeps it pending and retries), or treat a failed append as a hard error for that op rather than a soft log-and-continue. At minimum, don't ack
op-appliedfor a batch that isn't durably recorded.Context
Found in the second code-review pass of the offline-first PR (#154). Relay is flag-gated (v2), so this is sync-hardening, not a release blocker.