fix(db): drop stale direct optimistic rows after txid sync

.changeset/fix-stale-optimistic-server-key.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/db': patch
+---
+
+Fix stale optimistic rows persisting when sync confirms a different server-generated key. Previously, direct transactions (from `collection.insert()` etc.) had their optimistic rows exempted from stale-row cleanup, which prevented temp-key rows from being removed when the server returned a different primary key.

.github/workflows/e2e-tests.yml

@@ -28,7 +28,7 @@ jobs:
       - name: Setup Node.js
         uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
         with:
-          node-version: '20'
+          node-version: '22.13'
           cache: 'pnpm'
 
       - name: Install dependencies

AGENTS.md

@@ -14,6 +14,7 @@ This guide provides principles and patterns for AI agents contributing to the Ta
 8. [Function Design](#function-design)
 9. [Modern JavaScript Patterns](#modern-javascript-patterns)
 10. [Edge Cases and Corner Cases](#edge-cases-and-corner-cases)
+11. [Git and PR Hygiene](#git-and-pr-hygiene)
 
 ## Type Safety
 
@@ -563,6 +564,37 @@ const filtered = items.filter((item) => item.value > 0)
    // Should ignore the stale snapshot
    ```
 
+## Git and PR Hygiene
+
+### Never Rewrite Published Branch History
+
+**Do not amend, rebase, squash, or force-push a branch after it has been pushed or has an open PR.** This includes `git push --force` and `git push --force-with-lease`.
+
+Once a branch is visible to others, treat its history as shared. If CI fails or follow-up changes are needed, add a normal follow-up commit and push normally.
+
+**❌ Bad:**
+
+```bash
+git commit --amend --no-edit
+git push --force-with-lease
+```
+
+**✅ Good:**
+
+```bash
+git add <files>
+git commit -m "fix: address CI failure"
+git push
+```
+
+**Key Principles:**
+
+- Never force-push unless the user explicitly asks for it in that moment.
+- Do not assume `--force-with-lease` is acceptable; it still rewrites shared history.
+- Prefer small follow-up commits over rewritten history on PR branches.
+- If a clean history is desired, let the human maintainer squash or rebase during merge.
+- If you think history rewriting is necessary, stop and ask for explicit confirmation before running any command.
+
 ## Package Versioning
 
 ### Understand Semantic Versioning

packages/db/src/collection/mutations.ts

@@ -231,9 +231,7 @@ export class CollectionMutationsManager<
     } else {
       // Create a new transaction with a mutation function that calls the onInsert handler
       const directOpTransaction = createTransaction<TOutput>({
-        metadata: {
-          [DIRECT_TRANSACTION_METADATA_KEY]: true,
-        },
+        metadata: { [DIRECT_TRANSACTION_METADATA_KEY]: true },
         mutationFn: async (params) => {
           // Call the onInsert handler with the transaction and collection
           return await this.config.onInsert!({
@@ -430,9 +428,7 @@ export class CollectionMutationsManager<
 
     // Create a new transaction with a mutation function that calls the onUpdate handler
     const directOpTransaction = createTransaction<TOutput>({
-      metadata: {
-        [DIRECT_TRANSACTION_METADATA_KEY]: true,
-      },
+      metadata: { [DIRECT_TRANSACTION_METADATA_KEY]: true },
       mutationFn: async (params) => {
         // Call the onUpdate handler with the transaction and collection
         return this.config.onUpdate!({
@@ -536,9 +532,7 @@ export class CollectionMutationsManager<
     // Create a new transaction with a mutation function that calls the onDelete handler
     const directOpTransaction = createTransaction<TOutput>({
       autoCommit: true,
-      metadata: {
-        [DIRECT_TRANSACTION_METADATA_KEY]: true,
-      },
+      metadata: { [DIRECT_TRANSACTION_METADATA_KEY]: true },
       mutationFn: async (params) => {
         // Call the onDelete handler with the transaction and collection
         return this.config.onDelete!({

packages/db/src/collection/state.ts

@@ -1160,6 +1160,33 @@ export class CollectionStateManager<
         }
       }
 
+      // A completed optimistic insert may have used a temporary client key while
+      // the sync confirmation used a different server-generated key. Once a
+      // sync commit has been applied, stop retaining completed optimistic keys
+      // that were not confirmed by this commit so the temporary row is removed.
+      for (const key of this.pendingOptimisticDirectUpserts) {
+        if (!changedKeys.has(key)) {
+          changedKeys.add(key)
+          if (!currentVisibleState.has(key)) {
+            const previousValue = previousOptimisticUpserts.get(key)
+            if (previousValue !== undefined) {
+              currentVisibleState.set(key, previousValue)
+            }
+          }
+          this.pendingOptimisticUpserts.delete(key)
+          this.pendingLocalOrigins.delete(key)
+        }
+      }
+      for (const key of this.pendingOptimisticDirectDeletes) {
+        if (!changedKeys.has(key)) {
+          changedKeys.add(key)
+        }
+        this.pendingOptimisticDeletes.delete(key)
+        this.pendingLocalOrigins.delete(key)
+      }
+      this.pendingOptimisticDirectUpserts.clear()
+      this.pendingOptimisticDirectDeletes.clear()
+
       // Now check what actually changed in the final visible state
       for (const key of changedKeys) {
         const previousVisibleValue = currentVisibleState.get(key)

packages/db/tests/collection.test.ts

@@ -11,6 +11,7 @@ import {
   MissingUpdateHandlerError,
 } from '../src/errors'
 import { createTransaction } from '../src/transactions'
+import { createLiveQueryCollection, eq } from '../src/query/index.js'
 import {
   flushPromises,
   mockSyncCollectionOptionsNoInitialState,
@@ -41,6 +42,95 @@ describe(`Collection`, () => {
     expect(() => createCollection()).toThrow(CollectionRequiresConfigError)
   })
 
+  it(`removes optimistic insert when sync confirms with a different server-generated key`, async () => {
+    const options = mockSyncCollectionOptionsNoInitialState<{
+      id: number
+      text: string
+    }>({
+      id: `server-generated-key-test`,
+      getKey: (item) => item.id,
+      startSync: true,
+    })
+    const collection = createCollection(options)
+
+    options.utils.markReady()
+    await collection.stateWhenReady()
+
+    const tx = collection.insert({ id: 4733, text: `two` })
+
+    expect(getStateEntries(collection)).toEqual([
+      [4733, { id: 4733, text: `two` }],
+    ])
+
+    options.utils.begin()
+    options.utils.write({ type: `insert`, value: { id: 24, text: `two` } })
+    options.utils.commit()
+
+    // The sync commit is held while the local insert transaction is persisting.
+    expect(getStateEntries(collection)).toEqual([
+      [4733, { id: 4733, text: `two` }],
+    ])
+
+    options.utils.resolveSync()
+    await tx.isPersisted.promise
+    await flushPromises()
+
+    expect(getStateEntries(collection)).toEqual([[24, { id: 24, text: `two` }]])
+  })
+
+  it(`updates live queries when an optimistic insert is replaced by a different server key`, async () => {
+    const options = mockSyncCollectionOptionsNoInitialState<{
+      id: number
+      text: string
+      project_id: number
+    }>({
+      id: `server-generated-key-live-query-test`,
+      getKey: (item) => item.id,
+      startSync: true,
+    })
+    const collection = createCollection(options)
+    const liveCollection = createLiveQueryCollection((q) =>
+      q
+        .from({ collection })
+        .where(({ collection }) => eq(collection.project_id, 1))
+        .select(({ collection }) => ({
+          id: collection.id,
+          text: collection.text,
+          project_id: collection.project_id,
+          $synced: collection.$synced,
+          $origin: collection.$origin,
+          $key: collection.$key,
+        })),
+    )
+
+    options.utils.markReady()
+    await liveCollection.preload()
+
+    const tx = collection.insert({ id: 4733, text: `two`, project_id: 1 })
+
+    expect(liveCollection.toArray.map((todo) => todo.id)).toEqual([4733])
+
+    options.utils.begin()
+    options.utils.write({
+      type: `insert`,
+      value: { id: 24, text: `two`, project_id: 1 },
+    })
+    options.utils.commit()
+
+    options.utils.resolveSync()
+    await tx.isPersisted.promise
+    await flushPromises()
+
+    expect(
+      liveCollection.toArray.map((todo) => ({
+        id: todo.id,
+        synced: todo.$synced,
+        origin: todo.$origin,
+        key: todo.$key,
+      })),
+    ).toEqual([{ id: 24, synced: true, origin: `remote`, key: 24 }])
+  })
+
   it(`should throw an error when trying to use mutation operations outside of a transaction`, async () => {
     // Create a collection with sync but no mutationFn
     const collection = createCollection<{ value: string }>({

packages/db/tests/utils.ts

@@ -311,6 +311,7 @@ type MockSyncCollectionConfigNoInitialState<T> = {
   id: string
   getKey: (item: T) => string | number
   autoIndex?: `off` | `eager`
+  startSync?: boolean
   defaultIndexType?: IndexConstructor
 }
 

packages/electric-db-collection/tests/electric.test.ts

@@ -739,6 +739,49 @@ describe(`Electric Integration`, () => {
       expect(testCollection._state.syncedData.size).toEqual(1)
     })
 
+    it(`should remove optimistic insert when txid sync confirms a different server-generated key`, async () => {
+      const txid = 1234
+      const onInsert = vi.fn().mockResolvedValue({ txid })
+
+      const testCollection = createCollection(
+        electricCollectionOptions({
+          id: `test-server-generated-key-txid`,
+          shapeOptions: {
+            url: `http://test-url`,
+            params: { table: `test_table` },
+          },
+          startSync: true,
+          getKey: (item: Row) => item.id as number,
+          onInsert,
+        }),
+      )
+
+      const tx = testCollection.insert({ id: 4733, text: `two` })
+
+      expect(stripVirtualProps(testCollection.get(4733))).toEqual({
+        id: 4733,
+        text: `two`,
+      })
+
+      subscriber([
+        {
+          key: `24`,
+          value: { id: 24, text: `two` },
+          headers: { operation: `insert`, txids: [txid] },
+        },
+        { headers: { control: `up-to-date` } },
+      ])
+
+      await tx.isPersisted.promise
+
+      expect(testCollection.has(4733)).toBe(false)
+      expect(stripVirtualProps(testCollection.get(24))).toEqual({
+        id: 24,
+        text: `two`,
+      })
+      expect(Array.from(testCollection.state.keys())).toEqual([24])
+    })
+
     it(`should support void strategy when handler returns nothing`, async () => {
       const onInsert = vi.fn().mockResolvedValue(undefined)