Optimistic insert not removed when server returns a different key

[goatrenterguy]

Bug Report

Description

When an onInsert handler syncs server data back to the collection (via writeInsert, refetch, or any sync path), the optimistic insert under the original client key is never removed if the server returns a different key. This causes:

1. Duplication: Both the client-key item ($synced: false, $origin: local) and the server-key item ($synced: true, $origin: remote) appear in the collection. The client-key item persists forever.
2. Stale $synced (same-key variant): When client and server use the same key, $synced stays false permanently and the optimistic data shadows the server data.

Both violate the 0.6 contract: "when your mutation handler promise resolves, the optimistic state is removed."

Reproduction

Different key — via writeInsert:

const collection = createCollection(
  queryCollectionOptions({
    id: 'assignments',
    getKey: (item) => item.id,
    queryKey: ['assignments'],
    queryFn: async () => api.getAssignments(),
    queryClient,

    onInsert: async ({ transaction, collection }) => {
      const newItems = transaction.mutations.map((m) => m.modified)
      // Client uses id: -Date.now(), server returns sequential DB id
      const serverItems = await api.createAssignments(newItems)
      collection.utils.writeInsert(serverItems)
      return { refetch: false }
    },
  }),
)

collection.insert({ id: -Date.now(), resource_id: 42, name: 'New' })
// Result: two items visible — one under client key, one under server key

Different key — via refetch:

onInsert: async ({ transaction, collection }) => {
  const newItems = transaction.mutations.map((m) => m.modified)
  const serverItems = await api.createAssignments(newItems)
  // No writeInsert — just refetch. Server returns the item under a different key.
  await collection.utils.refetch()
},

Same result: the optimistic item under the client key persists alongside the refetched server item.

Same key — via writeInsert:

onInsert: async ({ transaction, collection }) => {
  const newItems = transaction.mutations.map((m) => m.modified)
  // Server returns same key but adds server-computed fields
  const serverItems = await api.createAssignments(newItems)
  collection.utils.writeInsert(serverItems)
  return { refetch: false }
},

Result: single item, but $synced stays false and the optimistic data (without server-computed fields) shadows the server data from writeInsert.

Failing Tests

Three reproduction tests in packages/query-db-collection/tests/query.test.ts:

• should not duplicate items when writeInsert uses a different key than the optimistic insert
• should mark item as synced when writeInsert uses the same key as the optimistic insert
• should not duplicate items when refetch returns a different key than the optimistic insert

All fail on current main with the @tanstack/db dist rebuilt.

Root Cause

Introduced in 9952921e ("Virtual props implementation #1213"). The pendingOptimisticDirectUpserts set was added to keep optimistic state visible between transaction completion and sync confirmation (for correct $synced tracking).

The sequence:

1. collection.insert() → creates direct transaction, mutation key = clientKey
2. onInsert runs (tx state = persisting)
3. Sync data is committed (via writeInsert or refetch) — commitPendingTransactions clears pendingOptimisticDirectUpserts for the sync key (which may differ from clientKey)
4. Handler returns → commit() sets state to completed → touchCollection() → recomputeOptimisticState(false)
5. In recomputeOptimisticState: the completed-transaction loop unconditionally re-adds clientKey to pendingOptimisticDirectUpserts (line 503)
6. isPersisted.resolve() → microtask: scheduleTransactionCleanup removes the transaction from the map
7. Any subsequent recomputeOptimisticState (from observer refetch, etc.) finds clientKey in pendingOptimisticDirectUpserts but no transaction to process → key persists forever, resurrected into optimisticUpserts via the seeding step (line 555)

Proposed Fix: Track which direct transactions had sync writes

Add a Set<string> (directTransactionsWithSyncWrites) to CollectionStateManager. When commitPendingTransactions processes an immediate sync transaction (which only comes from writeInsert/writeUpdate/writeDelete called inside a handler), record the ID of any persisting direct transaction. Then in recomputeOptimisticState, skip re-adding to pendingOptimisticDirectUpserts for transactions in this set.

This only changes behavior when a sync write was committed during the handler — the non-writeInsert flows are unaffected.

Trade-off: This approach uses an indirect signal — it infers "writeInsert was called" by observing that an immediate sync transaction was processed while a direct transaction was persisting. If a future change introduces another source of immediate sync transactions, it could incorrectly suppress the pending optimistic state. It also adds a Set that must stay in sync across three locations (commitPendingTransactions, recomputeOptimisticState, scheduleTransactionCleanup).

A potentially cleaner alternative would be a first-class flag on the transaction itself (e.g., transaction.hasSyncWrites = true) set explicitly by writeInsert/writeUpdate/writeDelete. That's a direct signal rather than an inferred one, but requires touching transactions.ts as well.

Flow	Result
writeInsert + different key	fixed
writeInsert + same key	fixed
refetch + different key	fixed
No writeInsert + default refetch	safe
No writeInsert + refetch: false	safe

Environment

• @tanstack/db@0.6.1
• @tanstack/query-db-collection@1.0.32
• @tanstack/react-db@0.1.79

[am1006]

Been trapped in this same problem for a little while. Thanks for raising this issue!

Here is a scenario we found:

  Timeline:

  1. collection.insert() → optimisticUpserts["019d..."] = {id: 0}, tx state = persisting
  2. writeUpsert({id: 142}) inside onInsert → triggers commitPendingTransactions:
    - syncedData["019d..."] = {id: 142} ✓
    - pendingOptimisticDirectUpserts.delete("019d...") ✓ (line 990)
    - But then the active transaction overlay (lines 1137-1161) re-applies the persisting tx's mutations → optimisticUpserts["019d..."] = {id: 0} back again
  3. onInsert returns → tx becomes completed → recomputeOptimisticState:
    - Completed transaction loop: pendingOptimisticDirectUpserts.add("019d...") ← re-added (line 503)
    - Seeding: pendingOptimisticDirectUpserts.has("019d...") → true → optimisticUpserts["019d..."] = {id: 0}
  4. get("019d...") → optimistic wins → {id: 0} forever

  The bug: Step 2 correctly clears pendingOptimisticDirectUpserts, but Step 3 unconditionally re-adds it from the completed transaction. The cleanup is undone.

  $synced and all data fields (id, created_at) are stuck on the optimistic values permanently.

In practice, to bypass this issue, we manually patched the library with:

- if (isDirectTransaction) {
+ if (isDirectTransaction && !this.syncedData.has(mutation.key)) {

If syncedData already has the key (from writeUpsert), don't re-add to pendingOptimisticDirectUpserts. The optimistic overlay drops, synced data takes over.

[goatrenterguy]

@am1006 that looks right, pendingOptimisticDirectUpserts doesn't get cleared out. One thing that I did find is that even if you direct write with the same id the item still has $synced as false