fix(sync): apply finds natural-key rows whose uid was never filled (T13273)

.changeset/t13273-natural-key-apply.md

@@ -0,0 +1,18 @@
+---
+id: t13273-natural-key-apply
+tasks: [T13273]
+kind: fix
+summary: Sync apply finds natural-key rows whose uid was never filled - own echoes no longer void on their origin
+---
+
+A natural-key row, such as a task dependency or a task relation, keeps a NULL uid until the open-time identity fill writes it. The
+sealer still journals the row under its natural uid, which it derives from the key. The apply looked rows up by uid only, so it missed
+such a row:
+
+- **The own echo of every dependency or relation insert was voided on its origin.** The row looked unseen, the insert hit the UNIQUE key,
+  and the apply recorded a guard conflict. The echo was then never sequenced and kept its undo.
+- **A rebase could not rewind such a row,** so a foreign edge was judged against the un-rewound local edge and the replicas diverged.
+
+Before planning a transaction, and before rewinding a local one, the apply now gives each unfilled natural-key row its uid. It finds the
+row with no uid whose local key matches the op's key, with references resolved to local keys, and writes the uid. That is the same
+derived value the identity fill writes, and capture never fires on the identity column.

packages/core/src/store/sync/apply/__tests__/rebase.test.ts

@@ -758,6 +758,70 @@ describe('footprints widened by what guards read (R7-6)', () => {
   });
 });
 
+describe('natural-key rows (T13273)', () => {
+  it("a dependency insert's own echo applies on its origin and is sequenced", async () => {
+    const [a, b, c] = await threeReplicas();
+    publish(a, write(a, `${addTask('TA', 'ta')}; ${addTask('TB', 'tb')}`));
+    for (const r of [a, b, c]) sync(r);
+    const la = write(
+      a,
+      "INSERT INTO tasks_task_dependencies (task_id, depends_on) VALUES ('TA', 'TB')",
+    );
+    publish(a, la);
+    expect(sync(a), 'the echo of an unfilled natural row voided on its origin').toMatchObject({
+      applied: 1,
+      void: 0,
+      conflicts: 0,
+    });
+    expect(outcome(a, la)).toBe('applied');
+    const sealed = n(a.db, 'SELECT count(*) AS n FROM _sync_txn');
+    seal(a);
+    expect(n(a.db, 'SELECT count(*) AS n FROM _sync_txn'), 'filling the uid was journaled').toBe(
+      sealed,
+    );
+    sync(b);
+    sync(c);
+    const deps = (r: Replica) =>
+      r.db
+        .prepare('SELECT task_id, depends_on, uid FROM tasks_task_dependencies ORDER BY 1, 2')
+        .all();
+    expect(deps(b)).toEqual(deps(a));
+    expect(deps(c)).toEqual(deps(a));
+  });
+
+  it('a dependency cycle closed through edges neither txn names converges (closure)', async () => {
+    const [a, b, c] = await threeReplicas();
+    publish(
+      a,
+      write(
+        a,
+        `${addTask('TA', 'ta')}; ${addTask('TB', 'tb')}; ${addTask('TC', 'tc')}; ${addTask('TD', 'td')};
+         INSERT INTO tasks_task_dependencies (task_id, depends_on) VALUES ('TB', 'TC'), ('TD', 'TA')`,
+      ),
+    );
+    for (const r of [a, b, c]) sync(r);
+    const la = write(
+      a,
+      "INSERT INTO tasks_task_dependencies (task_id, depends_on) VALUES ('TA', 'TB')",
+    );
+    publish(
+      b,
+      write(b, "INSERT INTO tasks_task_dependencies (task_id, depends_on) VALUES ('TC', 'TD')"),
+    );
+    expect(sync(a), 'the foreign edge met the local one only through the closure').toMatchObject({
+      rebased: 1,
+    });
+    publish(a, la);
+    for (const r of [a, b, c]) sync(r);
+    expect(outcome(a, la)).toBe('void');
+    const deps = (r: Replica) =>
+      r.db.prepare('SELECT task_id, depends_on FROM tasks_task_dependencies ORDER BY 1, 2').all();
+    expect(deps(b)).toEqual(deps(a));
+    expect(deps(c)).toEqual(deps(a));
+    converged([a, b, c]);
+  });
+});
+
 describe('foreign-touch index bounds (#1912 follow-ups)', () => {
   it('a restarted capture counter marks the index incomplete, so the fast path declines', async () => {
     const a = await replica(RA);

packages/core/src/store/sync/apply/applier.ts

@@ -769,6 +769,43 @@ function touchedRows(ops: readonly LedgerOp[]): TouchedRow[] {
   );
 }
 
+/**
+ * Give the unfilled natural-key rows `ops` name their uid (T13273): the
+ * op's key (`k`, references as uids) resolved to local keys. A key whose
+ * reference does not resolve is left for planning to judge.
+ */
+function adoptNaturalRows(
+  db: DatabaseSync,
+  api: ApplyApi,
+  defs: (table: string) => CaptureTableDef | null,
+  ops: readonly LedgerOp[],
+): void {
+  for (const op of ops) {
+    const def = defs(op.t);
+    if (!def || !op.k) continue;
+    const local: Record<string, LedgerWireValue> = {};
+    let resolved = true;
+    for (const [col, v] of Object.entries(op.k)) {
+      if (typeof v === 'object' && v !== null && '$inc' in v) {
+        resolved = false;
+        break;
+      }
+      const target = def.refs.get(col);
+      if (target && typeof v === 'string') {
+        const ref = resolveRef(db, target, v);
+        if (ref.kind !== 'row') {
+          resolved = false;
+          break;
+        }
+        local[col] = ref.key;
+      } else {
+        local[col] = v;
+      }
+    }
+    if (resolved) api.adoptNaturalRow(op.t, op.u, local);
+  }
+}
+
 /** Whether `op` writes an append-only table (insert-only rows, never rewound). */
 function isAppendOnly(defs: (table: string) => CaptureTableDef | null, op: LedgerOp): boolean {
   return defs(op.t)?.appendOnly === true;
@@ -943,6 +980,7 @@ function rewindTxns(c: OpContext, plan: RebasePlan): Rewound {
   const after = new Map<string, RowMetaFull | null>();
   withTriggersSuspended(c.db, ['capture', 'guard', 'side-effect'], 'rewind', () => {
     for (const l of [...plan.rewind].reverse()) {
+      adoptNaturalRows(c.db, c.api, c.defs, l.ops);
       for (let i = l.ops.length - 1; i >= 0; i--) {
         const op = l.ops[i] as LedgerOp;
         const def = c.defs(op.t);
@@ -1141,6 +1179,7 @@ export function applyStagedTxns(db: DatabaseSync, opts: ApplyStagedOptions): App
       }
       const actor = st.txn.actor ? JSON.stringify(st.txn.actor) : null;
       const result = withApplyFrame(db, opts.scope, actor, (api) => {
+        adoptNaturalRows(db, api, defs, st.txn.ops);
         const plan = planTxn(db, api, st, defs, opts.replica);
         if (plan.kind !== 'apply') {
           markTxns(db, st.parts, plan.kind, { reason: plan.reason, nowIso });

packages/core/src/store/sync/apply/write-api.ts

@@ -152,6 +152,21 @@ export interface ApplyWriteApi {
    * @returns Whether a row was deleted.
    */
   deleteRow(table: string, uid: string): boolean;
+  /**
+   * Give a natural-key row whose uid was never filled its uid (T13273). A
+   * natural row's uid is a function of its key, so the sealer journals it
+   * before the open-time identity fill writes it to the row; until then a
+   * lookup by uid misses the row. Finds the row with no uid whose local key
+   * equals `localKey` and writes `uid` into it, uncaptured (a derived value,
+   * as the fill writes it).
+   *
+   * @returns Whether a row was given the uid.
+   */
+  adoptNaturalRow(
+    table: string,
+    uid: string,
+    localKey: Readonly<Record<string, LedgerWireValue>>,
+  ): boolean;
   /**
    * Re-key a row (a K op): its uid, and its birth fingerprint when `newBfp`
    * is given. Records `*K` (enc = the new uid) and moves the row's meta and
@@ -480,6 +495,22 @@ export function createApplyWriteApi(
       return true;
     },
 
+    adoptNaturalRow(table, uid, localKey) {
+      assertActive();
+      const def = defOf(table);
+      if (rowExists(table, uid) || def.key.some((k) => !(k in localKey))) return false;
+      // The uid is an identity column: capture never fires on it.
+      return (
+        Number(
+          db
+            .prepare(
+              `UPDATE main.${ident(table)} SET ${ident(UID_COLUMN)} = ? WHERE ${ident(UID_COLUMN)} IS NULL AND ${def.key.map((k) => `${ident(k)} IS ?`).join(' AND ')}`,
+            )
+            .run(uid, ...def.key.map((k) => wireToSql(localKey[k] as LedgerWireValue))).changes,
+        ) === 1
+      );
+    },
+
     rekeyRow(table, uid, newUid, newBfp) {
       assertActive();
       const def = defOf(table);