[Bug] `LWWRegister.merge` returns `this` when timestamp and replica id both tie, so two writes from one replica in the same millisecond make merge order-dependent and the coordinator-state snapshot diverges between nodes

[pathosDev]

Problem

LWWRegister.merge breaks commutativity — the first CRDT law — when two registers carry the same timestamp and the same replica id. The tiebreak is a strict >, so an equal replica id falls through to return this, which means a.merge(b) === a and b.merge(a) === b. Two nodes that receive the same pair of writes in different gossip orders keep different values, forever, with no signal.

This is not a theoretical tie. DistributedDataCoordinatorStateStore is the one production writer of an LWWRegister in the framework, and it hits both halves of the condition by construction:

• The replica id is fixed per node (this.replicaId, the node's own address), so every snapshot that node writes carries the identical id.
• The timestamp is state.takenAt, i.e. Date.now() captured in snapshotCoordinatorState().
• scheduleCoordinatorStateSave explicitly coalesces a burst of mutations into a follow-up save fired from the in-flight save's .finally. During a rebalance the mutations are back-to-back, so the follow-up snapshot routinely lands in the same millisecond as the one before it — same takenAt, same replica, different shardHome.

Two peers gossiping those two registers in opposite orders converge on different shard maps. Whichever one a new leader loads on LeaderChanged decides where every entity of that type lives.

The property test cannot catch it. Its generator advances a module-level nextTs by at least 1 on every draw, so no two generated registers ever share a timestamp — the tie branch is never exercised — and even if it were, the value is derived from the timestamp (v-${ts}), so a tie would produce two equal values and commute trivially. The comment directly above claims the opposite.

Evidence

src/crdt/LWWRegister.ts:54-64
  merge(other: LWWRegister<V>): LWWRegister<V> {
    // Empty register loses to any non-empty one.
    if (this._timestamp === 0) return other;
    if (other._timestamp === 0) return this;

    if (other._timestamp > this._timestamp) return other;
    if (other._timestamp < this._timestamp) return this;
    // Tie on timestamp — break by replica id so every node converges
    // to the same winner regardless of arrival order.
    return other._replica > this._replica ? other : this;
  }

The writer that supplies a constant replica id and a Date.now() timestamp:

src/cluster/sharding/CoordinatorState.ts:96-106
  async save(typeName: string, state: CoordinatorStateData): Promise<void> {
    this.dd.update<LWWRegister<CoordinatorStateData>>(
      this.keyFor(typeName),
      () => LWWRegister.empty<CoordinatorStateData>(),
      (reg) => reg.assign(this.replicaId, state, state.takenAt),
    );

src/cluster/sharding/ShardCoordinator.ts:929-934
    return {
      leader: this.options.cluster.selfAddress.toString(),
      takenAt: Date.now(),
      regions,
      shardHome,
    };

The coalescing that puts two of them in the same millisecond:

src/cluster/sharding/ShardCoordinator.ts:856-875
    if (this.coordinatorStateInFlight) {
      this.coordinatorStateDirty = true;
      return;
    }
    this.coordinatorStateInFlight = true;
    const snapshot = this.snapshotCoordinatorState();
    void store.save(this.options.typeName, snapshot)
      .catch((err) => {
        this.system.log.warn(
          `[sharding] coordinator-state save failed for '${this.options.typeName}'`,
          err,
        );
      })
      .finally(() => {
        this.coordinatorStateInFlight = false;
        if (this.coordinatorStateDirty) {
          this.coordinatorStateDirty = false;
          this.scheduleCoordinatorStateSave();
        }
      });

The test that is supposed to cover this, and the comment that misdescribes it:

tests/unit/crdt/CrdtProperties.test.ts:190-199
  test('idempotent / commutative / associative', () => {
    let nextTs = 1;
    const gen = (): LWWRegister<string> => {
      // Use deterministic-ish increasing timestamps so we cover both
      // "same ts → replica tiebreaker" and "different ts → newest wins".
      const ts = (nextTs += 1 + Math.floor(Math.random() * 3));
      return LWWRegister.empty<string>().assign(pickReplica(), `v-${ts}`, ts);
    };
    checkLaws(gen, (first, second) => first.equals(second));
  });

nextTs increases by ≥ 1 every call, so the "same ts" half of that comment describes a case the generator cannot produce.

Proposal

merge must be a total order on (timestamp, replica, value) with no fallthrough:

• Keep the timestamp and replica comparisons, then break a full tie on a deterministic function of the value — e.g. compare JSON.stringify(this._value) against the peer's, or a hash of it — so both orders pick the same side. Any total order works; what matters is that it never depends on which argument is this.
• Give CoordinatorStateData a monotonic sequence number and use it as the register timestamp (or as the tiebreak below the timestamp). The coordinator already knows the ordering of its own snapshots; encoding it removes the reliance on millisecond resolution entirely and also fixes the ordering of two saves separated by an NTP correction.
• Fix the property-test generator to draw timestamps from a small set so ties actually occur, and decouple the value from the timestamp so a tie can be observed.

Acceptance sketch

• a.merge(b).equals(b.merge(a)) holds for two registers with identical timestamp and replica and different values.
• The property test generator produces timestamp collisions (assert on the collision count) and values independent of the timestamp.
• Two coordinator-state snapshots written in the same millisecond by the same node converge to the same value on every replica regardless of arrival order.

Reference issues: #724 concerns a hostile timestamp accepted by LWWRegister.fromJSON with no plausibility bound — a different failure of the same class, and its fix (bounding the value) does not touch the tie path. #253 proposes a real deep-equality helper for CRDT values and is the natural home for the value comparison this fix needs.

Verification status

Found in the ten-lens production-readiness review of 2026-08-05 (v0.13.0) and re-verified before filing: reproduced by execution against the current tree (Bun, in-process, no cluster).

merge(a,b).value() = "A"
merge(b,a).value() = "B"
commutative        = false
coordinator-state, arrival order 1->2: [[0,"regionA"]]
coordinator-state, arrival order 2->1: [[0,"regionB"]]
generator produced 100000 distinct timestamps out of 100000 draws

The first pair is two assign('node-1', …, 1000) calls differing only in value. The third and fourth lines are two CoordinatorStateData snapshots with the same leader, the same takenAt and different shardHome — the shape scheduleCoordinatorStateSave produces during a rebalance burst. The last line is the property-test generator run 100 000 times: it never once produced the tie its own comment says it covers.

Part of the production-readiness review batch — tracked in #913.