fix(persistence): preserve metadata on implicit equal re-inserts

.changeset/preserve-implicit-reinsert-metadata.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/db-sqlite-persistence-core': patch
+---
+
+Preserve accepted row metadata when an equal source re-insert omits an explicit metadata write. Prevent peer reloads from restoring older cached metadata after that insert.

docs/contributing/oracle-coverage.md

@@ -4439,24 +4439,50 @@ must equal the model's, which a restart would read.
 `persisted-peers-full` repeats the lane under full row updates, where an
 update replaces its row in storage too; `persisted-peers-on-demand` repeats
 it on demand with a demand for every row. A reset over metadata of a missing
-row is too rare in generated histories to declare; a replay pins it. A
-random CI campaign (seed `654774679`) found a `persisted-peers-full` history
-that reproduces on `main`: an outer transaction inserts over a bare seed, an
-inner one re-inserts the row and commits, and a peer's full reload leaves the
-row with the cached seed's metadata, where the model expects none (law:
-the source reads its row metadata). The maintainer chose to leave it open,
-tracked with #2281; an `it.fails` replay through the same model, driver, and
-comparison pins its shrunk prefix, and the random campaign can still draw it.
-Owner: this oracle. A reset
+row is too rare in generated histories to declare; a replay pins it. Durable source equal re-inserts preserve accepted row metadata, including absence
+or metadata supplied by a later DbClient hydration seed, without making the keep
+a source metadata write (decisions 9, 14, and 18). The constructive
+`seedReinsertReloadHistoryArbitrary` and ordinary original replay use the shared
+independent model and per-action source metadata comparison. Controlled
+partial/full/on-demand lanes and native SQLite receiving lanes each pass fixed
+40 and random 40 generated histories, with actual named/full-reload notices in
+every native lane. Final fixed seed `527962096` independently reaches 40 inner
+commits, 21 named notices, 19 full reloads, 2 completed bare-key1 implicit keeps,
+and 2 completed non-bare implicit keeps per lane on both hosts. Every normal run
+enforces those completed fixed-branch guards; random branch counts are measured
+without a guarantee of reach. Controlled final random seed `-704889444` measures
+0 bare/4 non-bare; native `1784284659` measures 2 bare/1 non-bare. Older
+commit/notice guards still apply to both campaigns. Original RED and direct
+replay seed `2281001` remain preserved. Original-defect and
+always-delete-kept-metadata mutants both fail the shared product comparison
+through both drivers. An all-explicit-inner grammar ablation passes 40 product
+comparisons per host but fails the new completed-branch guards, with all hashes
+restored. Native on-demand RED can occur at inner commit before the explicit peer
+notice because actual coordination reads storage sooner; eager partial/full RED
+reaches explicit peer step 6.
+
+The completed-branch guards were added in independent review after the first
+production edit. M16 revealed zero bare-key1 reach at fixed seed `2281001`, then
+measured seed `527962096` supplied both premises. The bare conjunction has
+probability 1/24, so the reproducible fixed campaign supplies its durable
+assertion while random campaigns measure it. No prior guard, comparison,
+minimum, or run budget was removed or lowered. The model, grammar, and comparison
+are unchanged; only the new properties' fixed seeds were calibrated. Broader
+#2272 transport and host gaps remain open: this receiver covers two real Browser
+coordinators over Node BroadcastChannel/Web Locks and native SQLite in one
+process. It does not establish Chromium OPFS, Electron, transport gaps,
+leadership turnover, every partial-notice shape, or every shared session grammar.
+The constructive receiver uses core holds; broad controlled histories retain
+store/read holds. Owner: this sync metadata session oracle. A reset
 may rewrite its rows, which its reload then applies as source writes.
 `persisted-reads-seeds` crosses read holds with seeds and the cached row and
 aborts a buffered commit's signal, which abandons it at the read's end
 (decision 21); a seed that arrived after it stays judged against it
 (decision 22). Dependent suffixes of an abandoned transaction belong to the
-abort-graph oracle. The peer is the driver, not a second
-Collection, so the claim stops at the controlled coordinator; a second
-production driver with two real coordinators is the open receiving
-witness (issue #2272). Missed notices and notices over the targeted key limit are not
+abort-graph oracle. The broad peer grammar uses a controlled coordinator. The new constructive
+seed/re-insert/reload receiver uses two real Collections and coordinators for
+the bounded metadata law above; it does not close the broader host and transport
+receiving scope of issue #2272. Missed notices and notices over the targeted key limit are not
 generated; the persisted oracle's grids cover them. Two held-cleanup
 Query retention witnesses race on CI; the maintainer deferred the race until
 a failure reports the timer trace these witnesses now print, and the review

packages/db-sqlite-persistence-core/src/persisted.ts

@@ -4698,6 +4698,18 @@ class PersistedCollectionRuntime<
             value: operation.value as Record<string, unknown>,
           }
     })
+    // An equal re-insert keeps accepted metadata without writing it in this
+    // sync run. Its durable replacement must keep that same value, including
+    // a DbClient seed that never reached storage (decisions 9 and 14).
+    const rowMetadataWrites = new Map(transaction.rowMetadataWrites)
+    for (const { key } of transaction.insertOperations) {
+      if (rowMetadataWrites.has(key)) continue
+      const value = this.syncControls.metadata?.row.get(key)
+      rowMetadataWrites.set(
+        key,
+        value === undefined ? { type: `delete` } : { type: `set`, value },
+      )
+    }
     return {
       txId: safeRandomUUID(),
       term: streamPosition.term,
@@ -4706,7 +4718,7 @@ class PersistedCollectionRuntime<
       cacheGenerationClaimId: this.cacheGenerationClaim?.claimId,
       truncate: transaction.truncate,
       mutations,
-      rowMetadataMutations: toMetadataMutations(transaction.rowMetadataWrites),
+      rowMetadataMutations: toMetadataMutations(rowMetadataWrites),
       collectionMetadataMutations: toMetadataMutations(
         transaction.collectionMetadataWrites,
       ),

packages/db-sqlite-persistence-core/tests/sync-metadata-session-persisted.test.ts

@@ -5,6 +5,7 @@ import {
   absentMarker,
   cachedMetadata,
   cachedRow,
+  completedSeedReinsertReloadRegime,
   describeMismatch,
   historyRegimes,
   persistedCachedLane,
@@ -19,13 +20,17 @@ import {
   persistedReadSeedLane,
   persistedSeedLane,
   runSessionHistory,
+  seedReinsertReloadHistoryArbitrary,
+  seedReinsertReloadReplay,
   sessionCollectionKeys,
   sessionHistoryArbitrary,
 } from '../../db/tests/sync-metadata-session-oracle'
 import {
   oraclePropertyOptions,
+  oracleReplayTest,
   oracleRuns,
   readOracleRunConfig,
+  recordOracleReplay,
 } from '../../db/tests/oracle-config'
 import { createOracleReach } from '../../db/tests/oracle-reach'
 import { SingleProcessCoordinator, persistedCollectionOptions } from '../src'
@@ -1209,29 +1214,13 @@ describe(`sync metadata session oracle through the persisted wrapper: peer repla
   it.each(peerFullReplays)(`replays under full rows: %s`, (_name, history) =>
     assertPersistedHistory(history, persistedPeerFullLane),
   )
-  // Known gap, pinned from the persisted-peers-full random campaign (seed
-  // 654774679, shrunk to this prefix): an outer transaction inserts over a
-  // bare seed, an inner one re-inserts the row and commits, and a peer's full
-  // reload then leaves row 1 with the cached seed's metadata, where the model
-  // expects none. This reproduces on main and is tracked with #2281. It
-  // replays through the same model, driver, and comparison. Remove `fails`
-  // when the reload stops restoring that metadata.
-  it.fails(
-    `replays under full rows: a reload after an inner re-insert over a seed clears the seed's metadata`,
-    () =>
-      assertPersistedHistory(
-        [
-          { type: `begin` },
-          { type: `seed`, key: 1, narrow: false, bare: true },
-          { type: `insertOverSeed`, key: 1, metadata: false },
-          { type: `begin` },
-          { type: `reinsert`, key: 1, metadata: false },
-          { type: `commit` },
-          { type: `peer`, key: 2, notice: `reload` },
-        ],
-        persistedPeerFullLane,
-      ),
-  )
+  // Pinned from the persisted-peers-full random campaign (seed 654774679):
+  // an outer transaction inserts over a bare seed and an inner equal re-insert
+  // commits first. Its durable replacement must keep the accepted metadata
+  // (absent here), so a peer's full reload cannot restore the older cache
+  // metadata. This uses the campaigns' model, driver, and comparison.
+  it(`replays under full rows: a reload after an inner re-insert over a seed clears the seed's metadata`, () =>
+    assertPersistedHistory(seedReinsertReloadReplay, persistedPeerFullLane))
   it.each(peerOnDemandReplays)(`replays on demand: %s`, (_name, history) =>
     assertPersistedHistory(history, persistedPeerOnDemandLane),
   )
@@ -1241,48 +1230,140 @@ describe(`sync metadata session oracle through the persisted wrapper: peer repla
   )
 })
 
-describe(`sync metadata session oracle through the persisted wrapper: nesting and seeds`, () => {
-  for (const {
-    lane,
-    property: lanePropertyName,
-    seed,
-    runs,
-    reach: states,
-  } of extraLanes) {
-    const runLane = (history: Array<SessionAction>) =>
-      assertPersistedHistory(history, lane)
-    if (requestedReplayProperty === undefined) {
-      const reach = createOracleReach(`${lanePropertyName} (fixed)`, states, {
-        minimum: 5,
-      })
-      fcTest.prop([sessionHistoryArbitrary(lane)], {
-        numRuns: oracleRuns(runs),
-        seed,
-      })(
-        `keeps the session law on ${lane.name} histories (fixed)`,
-        (history) => {
+// The broad peer campaigns can already find this defect. This constructive
+// grammar measures its scarce overlap and adjacent legal histories directly;
+// it uses the existing independent model, driver, and per-action comparison.
+const seedReloadProperty = `sync-metadata-session.persisted-seed-reinsert-reload`
+for (const campaign of [`fixed`, `random`] as const) {
+  if (
+    requestedReplayProperty !== undefined &&
+    (requestedReplayProperty !== seedReloadProperty || campaign === `fixed`)
+  )
+    continue
+  it(`keeps inserted metadata across peer reloads (${campaign})`, async () => {
+    const lanes = [
+      persistedPeerLane,
+      persistedPeerFullLane,
+      persistedPeerOnDemandLane,
+    ]
+    const states = lanes.flatMap((lane) => [
+      `${lane.name}: inner commit`,
+      `${lane.name}: row`,
+      `${lane.name}: reload`,
+    ])
+    const completedStates = lanes.flatMap((lane) => [
+      `${lane.name}: completed kept bare seed reload`,
+      `${lane.name}: completed kept non-bare seed reload`,
+    ])
+    const reach = createOracleReach(seedReloadProperty, states)
+    const completedReach = createOracleReach(
+      seedReloadProperty,
+      completedStates,
+    )
+    const result = await fc.check(
+      fc.asyncProperty(seedReinsertReloadHistoryArbitrary, async (history) => {
+        const failures: Array<string> = []
+        for (const lane of lanes) {
           reach.beginHistory()
-          for (const regime of historyRegimes(lane, history))
-            if (states.includes(regime)) reach.reach(regime)
-          return runLane(history)
-        },
-        240_000,
-      )
-      it(`reaches the declared states in the ${lane.name} fixed campaign`, () =>
-        reach.assertReached())
-      fcTest.prop(
-        [sessionHistoryArbitrary(lane)],
-        oraclePropertyOptions(50, lanePropertyName),
-      )(
-        `keeps the session law on ${lane.name} histories (random)`,
-        runLane,
-        120_000,
+          completedReach.beginHistory()
+          const result = await runSessionHistory(lane, history, () =>
+            createPersistedDriver(
+              true,
+              true,
+              false,
+              true,
+              lane.fullRows === true,
+              lane.onDemand === true,
+            ),
+          )
+          const last = history.at(-1)!
+          const innerCommit = history.findIndex(
+            (action) => action.type === `commit`,
+          )
+          if (result.steps > innerCommit)
+            reach.reach(`${lane.name}: inner commit`)
+          if (last.type === `peer` && result.steps === history.length)
+            reach.reach(`${lane.name}: ${last.notice}`)
+          const completed = completedSeedReinsertReloadRegime(history, result)
+          if (completed !== undefined)
+            completedReach.reach(`${lane.name}: ${completed}`)
+          if (result.mismatch)
+            failures.push(
+              `${lane.name}\n${describeMismatch(history, result.mismatch)}`,
+            )
+        }
+        if (failures.length) throw new Error(failures.join(`\n\n`))
+      }),
+      campaign === `fixed`
+        ? { numRuns: oracleRuns(40), seed: 527962096 }
+        : oraclePropertyOptions(40, seedReloadProperty),
+    )
+    recordOracleReplay(seedReloadProperty, result)
+    console.info(
+      `ORACLE_REACH ${JSON.stringify({ property: seedReloadProperty, campaign, seed: result.seed, runs: result.numRuns, shrinks: result.numShrinks, completedBranchGuards: campaign === `fixed` ? `enforced` : `measured`, counts: Object.fromEntries([...reach.counts(), ...completedReach.counts()]) })}`,
+    )
+    if (result.failed)
+      throw new Error(
+        `${result.error}\nseed=${result.seed} path=${result.counterexamplePath}`,
       )
-    } else if (requestedReplayProperty === lanePropertyName) {
-      fcTest.prop(
-        [sessionHistoryArbitrary(lane)],
-        oraclePropertyOptions(50, lanePropertyName),
-      )(`replays ${lane.name} session histories`, runLane, 120_000)
+    reach.assertReached()
+    // The bare-key-1 conjunction has probability 1/24: random40 would miss it
+    // about 18% of the time. Every normal run enforces the fixed40 branch
+    // guards; the identical random40 campaign measures them without a promise
+    // of reach. Existing inner/row/reload guards apply to both campaigns.
+    if (campaign === `fixed`) completedReach.assertReached()
+  }, 120_000)
+}
+
+const describeNested = oracleReplayTest(
+  describe,
+  ...extraLanes.map(({ property }) => property),
+)
+describeNested(
+  `sync metadata session oracle through the persisted wrapper: nesting and seeds`,
+  () => {
+    for (const {
+      lane,
+      property: lanePropertyName,
+      seed,
+      runs,
+      reach: states,
+    } of extraLanes) {
+      const runLane = (history: Array<SessionAction>) =>
+        assertPersistedHistory(history, lane)
+      if (requestedReplayProperty === undefined) {
+        const reach = createOracleReach(`${lanePropertyName} (fixed)`, states, {
+          minimum: 5,
+        })
+        fcTest.prop([sessionHistoryArbitrary(lane)], {
+          numRuns: oracleRuns(runs),
+          seed,
+        })(
+          `keeps the session law on ${lane.name} histories (fixed)`,
+          (history) => {
+            reach.beginHistory()
+            for (const regime of historyRegimes(lane, history))
+              if (states.includes(regime)) reach.reach(regime)
+            return runLane(history)
+          },
+          240_000,
+        )
+        it(`reaches the declared states in the ${lane.name} fixed campaign`, () =>
+          reach.assertReached())
+        fcTest.prop(
+          [sessionHistoryArbitrary(lane)],
+          oraclePropertyOptions(50, lanePropertyName),
+        )(
+          `keeps the session law on ${lane.name} histories (random)`,
+          runLane,
+          120_000,
+        )
+      } else if (requestedReplayProperty === lanePropertyName) {
+        fcTest.prop(
+          [sessionHistoryArbitrary(lane)],
+          oraclePropertyOptions(50, lanePropertyName),
+        )(`replays ${lane.name} session histories`, runLane, 120_000)
+      }
     }
-  }
-})
+  },
+)