[claude] Sync head truncates HybridDateTime to milliseconds, permanently stranding commits

[myieye]

[Claude, autonomous]

Tip

A starter branch is attached to this issue to jump-start the implementation: claude/harmony-issue-89-oga6ra  •  browse the diff

It takes the boringly-simple route instead of the full-precision one below: make the sync-head comparison inclusive on the millisecond (> → >=) in both GetMissingCommits paths, so the head's whole millisecond is re-offered rather than stranded. Per-client the HLC is monotonic, so re-offering that millisecond can never leapfrog a commit. Re-applying a commit the remote already holds is idempotent (FilterExistingCommits drops it), so the only cost is re-sending the head-millisecond commits each sync. No SyncState wire-format change and no back-compat path needed.

Tests cover every flavour of the bug and each fails before the one-line-per-path change: counter-sibling on the head millisecond, a stranded commit after the head advances (both on a whole-millisecond boundary, which the DB pre-filter drops, and with sub-millisecond ticks, which the in-memory guard drops), a full clamp burst, and an end-to-end sync across both syncable backends.

Bug. The per-client sync head is Max(DateTime).ToUnixTimeMilliseconds() (QueryHelpers.GetSyncState), and GetMissingCommits selects the push set with a strict > on that millisecond — both the DB pre-filter and the in-memory guard (QueryHelpers.cs:53,56). But commits carry microsecond DateTime and a Counter. Any commit sharing the head's millisecond fails ms > ms and is treated as already-synced: never offered, and the sync reports success. Once a later commit advances the head, the DB pre-filter excludes it too. The result is permanent, silent commit loss — usually just missing data, or a NotSupportedException in SnapshotWorker if the stranded commit was the create for an entity whose later edit did sync.

How several commits end up sharing one millisecond — HLC clamping. When the wall clock is at or behind lastDateTime, GetDateTime stamps successive commits with the same DateTime and an incrementing Counter. Two ways in: a peer's future-dated commit pulls the HLC ahead of local real time, or the local clock steps back. Either way, every commit made during the catch-up window shares one millisecond; per client, only the first that the server already holds survives, and the rest strand.

Fix. Make the head precision-complete: carry the full HybridDateTime (DateTime + Counter) per client and compare with (DateTime, Counter, Id) ordering, as WhereAfter already does. Per-client commits strictly increase in (DateTime, Counter), so a full-precision watermark is exact. SyncState is a client↔server wire type, so keep a back-compat path for clients still sending millisecond-only heads.

Related: #79 (same watermark weakness, ClientId-duplication variant; its per-client digest would also detect this loss); #7 (future-dated commits, one clamp trigger).