fix(db): pendingOptimisticDirectUpserts leaks after writeInsert in onInsert handler, causing duplication and stale $synced #1440

Bug Report

Description

When writeInsert is called inside an onInsert handler (the documented pattern for pessimistic updates since 0.6), pendingOptimisticDirectUpserts leaks the optimistic mutation key. This causes:

1. Different-key case (client temp ID → server sequential ID): Both the client key and server key appear in the collection — the optimistic item is never removed, resulting in duplication.
2. Same-key case: $synced stays false permanently and the optimistic data shadows the server data written by writeInsert.

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

Reproduction

Setup — matches the documented pattern:

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

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

      collection.utils.writeInsert(serverItems)
      return { refetch: false }
    },
  })
)

// Insert with client-generated temp ID
collection.insert({ id: -Date.now(), resource_id: 42, name: 'New' })

Expected: After handler completes, only the server item (with server ID) is visible, $synced: true.

Actual: 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.

For the same-key variant (client and server use the same ID), the item shows $synced: false and the optimistic data shadows the server data from writeInsert.

Failing Tests

Two reproduction tests added to 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

Both 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. writeInsert(serverItems) → commitPendingTransactions writes to syncedData, 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)

Why Updates Are Unaffected

In the user's code, onUpdate doesn't return { refetch: false }, so the default refetch runs and clears pendingOptimisticDirectUpserts as a side effect. If { refetch: false } were added to onUpdate with writeUpdate, the same bug would occur.

Potential Fix Approaches

Option A: syncedData.has(key) guard in recomputeOptimisticState

Skip re-adding to pendingOptimisticDirectUpserts when syncedData already has the key. Simple, but only fixes the same-key case. The different-key case needs an additional cleanup mechanism.

Option B: Option A + cleanup in scheduleTransactionCleanup

When a transaction is removed from the map, also clear its mutation keys from pendingOptimisticDirectUpserts. Fixes both cases, but is too aggressive — it would break the valid { refetch: false } without writeInsert flow where optimistic state should persist until a future sync confirms it.

Option C: Track which direct transactions had immediate 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 is the most precise fix — it only changes behavior when we know a sync write was committed during the handler. All four flows are handled correctly:

Environment

• @tanstack/db@0.6.1
• @tanstack/query-db-collection@1.0.32
• @tanstack/react-db@0.1.79
• syncMode: 'on-demand'