From e1ed7dc56ea07708a8913446f2b2b67bb4b9bafe Mon Sep 17 00:00:00 2001 From: Keaton Hoskins Date: Mon, 5 Oct 2026 11:11:25 -0700 Subject: [PATCH] fix(sync): apply finds natural-key rows whose uid was never filled (T13273) A natural-key row (dependencies, relations) has uid NULL until the open- time identity fill; the sealer journals it under the natural uid. Lookups by uid missed it: the own echo of every such insert voided on its origin with a UNIQUE guard conflict (never sequenced), and a rebase could not rewind it, so replicas diverged. The apply now adopts the row first: the row with no uid whose local key matches the op's `k` (refs resolved) gets its uid (the fill's derived value; capture never fires on the identity column). Done before planning each staged txn and before rewinding each local one. Co-Authored-By: Claude Opus 5.5 --- .changeset/t13273-natural-key-apply.md | 18 ++++++ .../store/sync/apply/__tests__/rebase.test.ts | 64 +++++++++++++++++++ packages/core/src/store/sync/apply/applier.ts | 39 +++++++++++ .../core/src/store/sync/apply/write-api.ts | 31 +++++++++ 4 files changed, 152 insertions(+) create mode 100644 .changeset/t13273-natural-key-apply.md diff --git a/.changeset/t13273-natural-key-apply.md b/.changeset/t13273-natural-key-apply.md new file mode 100644 index 0000000000..17242baef7 --- /dev/null +++ b/.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. diff --git a/packages/core/src/store/sync/apply/__tests__/rebase.test.ts b/packages/core/src/store/sync/apply/__tests__/rebase.test.ts index b1172b6507..c4bc57bea8 100644 --- a/packages/core/src/store/sync/apply/__tests__/rebase.test.ts +++ b/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); diff --git a/packages/core/src/store/sync/apply/applier.ts b/packages/core/src/store/sync/apply/applier.ts index 63bb119b9c..f0117817f4 100644 --- a/packages/core/src/store/sync/apply/applier.ts +++ b/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 = {}; + 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(); 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 }); diff --git a/packages/core/src/store/sync/apply/write-api.ts b/packages/core/src/store/sync/apply/write-api.ts index b9393cd2c0..ac3a689e16 100644 --- a/packages/core/src/store/sync/apply/write-api.ts +++ b/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>, + ): 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);