fix(T13868): concurrent first docs updates no longer wedge the second device's stream

.changeset/t13868-concurrent-first-doc-update.md

@@ -0,0 +1,39 @@
+---
+id: t13868-concurrent-first-doc-update
+tasks: [T13868]
+kind: fix
+summary: concurrent first docs updates on two devices no longer wedge the second device's stream with "FOREIGN KEY constraint failed"; doc version links travel as uids
+---
+
+When two devices each made their first `docs update` of the same synced doc,
+the second device's project stream failed with "FOREIGN KEY constraint
+failed" on every later `cloud sync`. The peer's segment stayed staged and the
+doc diverged.
+
+- **The wedge.** To apply the peer's earlier segment, the second device
+  rewinds its own local transaction. The rewind undid its INSERT of the doc
+  row (`conduit_attachments`), but skips append-only children, so its own
+  `conduit_attachment_versions` rows still pointed at the doc and the
+  immediate foreign-key check refused the delete. Rewind, apply and replay
+  share one transaction, so the rewind now defers foreign keys to COMMIT,
+  as the applier already does for a key move. The doc row is back (from the
+  stream or the replay) before the check runs.
+- **Version links arrived as NULL.** `conduit_attachment_versions` and
+  `conduit_attachment_approvals` declared their uid columns
+  (`attachment_uid`, `parent_version_uid`, `version_uid`) as references.
+  References are translated through the target's LOCAL key (`slug`, `id`),
+  so the sealer found no row and sent NULL, and every peer stored versions
+  with no doc and no parent. Those columns already hold uids, so they now
+  travel verbatim, as `conduit_doc_heads` and the slug aliases already do.
+- **A refused apply names the row.** When a deferred foreign key still fails
+  at COMMIT, the refusal names the child row's uid and its parent table
+  (`E_SYNC_APPLY_FK`) instead of a bare "FOREIGN KEY constraint failed".
+
+A store already wedged retries the staged segment on its next sync and
+applies it. Version and approval rows that peers already stored with a NULL
+link are not rewritten by this change. The device that wrote them now hashes
+its own rows with the link included, so the next migration pass on that
+device marks the two tables for repair. The repair then journals each row once
+with its real link, and that fills in the NULLs on peers. This is a one-time
+update burst on those rows. Their append-only rules are declared but not
+enforced, so peers apply it.

packages/core/src/store/row-identity-registry.ts

@@ -693,28 +693,24 @@ export const ROW_IDENTITY: Readonly<Record<TableScope, readonly RowIdentitySpec[
     {
       // Append-only: version identity is the uid + parent_version_uid DAG;
       // the local UUIDv4 id never travels and cannot collide across devices.
+      // attachment_uid / parent_version_uid already hold uids, so they travel
+      // verbatim like conduit_doc_heads: `refs` map LOCAL keys (slug, id) and
+      // sealed them as NULL on every peer (T13868).
       table: 'conduit_attachment_versions',
       kind: 'minted',
       key: ['id'],
       birth: 'created_at',
       birthFacts: ['author_agent_id', 'content_hash'],
-      refs: [
-        { column: 'attachment_uid', table: 'conduit_attachments' },
-        { column: 'parent_version_uid', table: 'conduit_attachment_versions' },
-      ],
       task: 'T13354',
     },
     {
       // Append-only: a revoke is a new row (tombstone), never an UPDATE/DELETE.
+      // attachment_uid / version_uid hold uids and travel verbatim (T13868).
       table: 'conduit_attachment_approvals',
       kind: 'minted',
       key: ['id'],
       birth: 'created_at',
       birthFacts: ['reviewer_agent_id'],
-      refs: [
-        { column: 'attachment_uid', table: 'conduit_attachments' },
-        { column: 'version_uid', table: 'conduit_attachment_versions' },
-      ],
       task: 'T13354',
     },
     {

packages/core/src/store/sync/__tests__/two-device-e2e.test.ts

@@ -41,6 +41,7 @@ import {
   secretValuesKey,
 } from '../../../cloud/secret-seal.js';
 import { DocsReadModel } from '../../../docs/docs-read-model.js';
+import { updateDocBySlug } from '../../../docs/docs-update.js';
 import {
   _resetDualScopeDbCache,
   getDualScopeNativeDb,
@@ -900,6 +901,70 @@ describe('docs reach a second device (T12919)', () => {
     expect(refs(b.db)).toEqual(refs(a.db));
   });
 
+  for (const firstPull of ['b', 'a'] as const) {
+    it(`concurrent first docs updates on both devices converge with no FK failure, ${firstPull} pulling first (T13868)`, async () => {
+      const storageKey = `${SHA.slice(0, 2)}/${SHA.slice(2)}.md`;
+      const blobJson = JSON.stringify({
+        kind: 'blob',
+        sha256: SHA,
+        storageKey,
+        mime: 'text/markdown',
+        size: BODY.length,
+      });
+      const { a, b } = await twoDevices();
+      const stream = fakeStream();
+      for (const r of [a, b]) uniqueDocIndexes(r.db);
+      const at = new Date(++clockMs).toISOString();
+      write(
+        a.db,
+        `INSERT INTO docs_attachments (id, sha256, attachment_json, created_at, ref_count, slug, type)
+           VALUES ('doc-1', '${SHA}', '${blobJson}', '${at}', 0, 'gate-crdt', 'note');`,
+      );
+      await push(a, stream);
+      await pull(b, stream);
+      await pull(a, stream);
+      // `docs add` left the bytes on A; a vault restore or bytes pull puts them on B.
+      for (const name of ['a', 'b']) {
+        const p = join(dir, name, '.cleo', 'attachments', 'sha256', storageKey);
+        mkdirSync(join(p, '..'), { recursive: true });
+        writeFileSync(p, BODY);
+      }
+      // Each device makes its FIRST text update of the doc before seeing the other's.
+      for (const [r, name] of [
+        [a, 'a'],
+        [b, 'b'],
+      ] as const) {
+        vi.stubEnv('CLEO_DIR', join(dir, name, '.cleo')); // the update's native handle is the ambient project
+        const out = await updateDocBySlug(join(dir, name), {
+          slug: 'gate-crdt',
+          content: `${BODY}edit from ${name}\n`,
+          attachedBy: `agent-${name}`,
+        });
+        expect(out.ok, `${name}: ${JSON.stringify(out)}`).toBe(true);
+        await push(r, stream);
+      }
+      vi.stubEnv('CLEO_DIR', undefined);
+      const order = firstPull === 'b' ? [b, a] : [a, b];
+      for (const r of order) await pull(r, stream);
+      for (const r of order) await pull(r, stream);
+      for (const r of [a, b]) {
+        expect(inboxCounts(r.db, STREAM), r.device).not.toHaveProperty('staged');
+        expect(r.db.prepare('PRAGMA foreign_key_check').all(), r.device).toEqual([]);
+      }
+      const docState = (r: Replica) => ({
+        doc: r.db.prepare('SELECT uid, slug FROM conduit_attachments ORDER BY uid').all(),
+        versions: r.db
+          .prepare('SELECT uid, attachment_uid FROM conduit_attachment_versions ORDER BY uid')
+          .all(),
+        head: r.db
+          .prepare('SELECT attachment_uid, head_version_uid FROM conduit_doc_heads ORDER BY uid')
+          .all(),
+      });
+      expect(docState(a).doc).toHaveLength(1);
+      expect(docState(b)).toEqual(docState(a));
+    }, 120_000);
+  }
+
   /**
    * A fake Nexus for doc bytes (T13721): presign/complete and the stream
    * attachment routes (E32/E33), blobs in memory. `routes: false` is a server

packages/core/src/store/sync/apply/__tests__/apply-frame.test.ts

@@ -146,6 +146,29 @@ describe('withApplyFrame (§3.2)', () => {
     );
   });
 
+  it('a deferred FK the COMMIT refuses names the row and its parent, and rolls back (T13868)', async () => {
+    const db = await store();
+    let thrown: unknown;
+    try {
+      withApplyFrame(db, 'project', null, () => {
+        db.exec('PRAGMA defer_foreign_keys = ON');
+        db.exec(
+          "INSERT INTO conduit_doc_slug_aliases (slug, doc_uid, uid) VALUES ('old-slug', 'no-such-doc', 'alias-uid-1')",
+        );
+      });
+    } catch (err) {
+      thrown = err;
+    }
+    expect(thrown).toMatchObject({
+      code: 'E_SYNC_APPLY_FK',
+      message: expect.stringContaining(
+        'conduit_doc_slug_aliases alias-uid-1 → conduit_attachments',
+      ),
+    });
+    expect(n(db, 'SELECT count(*) AS n FROM conduit_doc_slug_aliases')).toBe(0);
+    expect(db.isTransaction).toBe(false);
+  });
+
   it('runApplyFrame awaits the preparation, then runs the synchronous body on the handle queue', async () => {
     const db = await store();
     const out = await runApplyFrame(

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

@@ -1469,6 +1469,10 @@ function describeConflicts(cs: readonly MergeConflict[]): string {
 function rewindTxns(c: OpContext, plan: RebasePlan): Rewound {
   const kept = new Map<string, KeptRow>();
   const after = new Map<string, RowMetaFull | null>();
+  // T13868: rewind skips append-only children, so undoing a parent's INSERT
+  // leaves them pointing at it until the stream or the replay puts it back.
+  // Rewind, apply and replay share this frame's transaction: check FKs at COMMIT.
+  c.db.exec('PRAGMA defer_foreign_keys = ON');
   withTriggersSuspended(c.db, ['capture', 'guard', 'side-effect'], 'rewind', () => {
     for (const l of [...plan.rewind].reverse()) {
       adoptNaturalRows(c.db, c.api, c.defs, l.ops);

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

@@ -34,17 +34,62 @@ import { finishCaptureFrame, openCaptureFrame } from '../capture.js';
 import { receiveClock } from '../clock-store.js';
 import { type ApplyWriteApi, createApplyWriteApi } from './write-api.js';
 
-/** A frame misuse: an async body, a nested call, or a call after the frame ended. */
+/**
+ * A frame misuse (an async body, a nested call, a call after the frame
+ * ended), or a deferred foreign key the frame's COMMIT refused, with the rows
+ * named (`E_SYNC_APPLY_FK`, T13868).
+ */
 export class ApplyFrameError extends Error {
   constructor(
-    readonly code: 'E_SYNC_APPLY_ASYNC' | 'E_SYNC_APPLY_NESTED' | 'E_SYNC_APPLY_ENDED',
+    readonly code:
+      | 'E_SYNC_APPLY_ASYNC'
+      | 'E_SYNC_APPLY_NESTED'
+      | 'E_SYNC_APPLY_ENDED'
+      | 'E_SYNC_APPLY_FK',
     message: string,
   ) {
     super(message);
     this.name = 'ApplyFrameError';
   }
 }
 
+/**
+ * The rows a failed COMMIT's deferred foreign keys point away from, named for
+ * the refusal (T13868): a bare "FOREIGN KEY constraint failed" on every sync
+ * says nothing about which row to repair. Null when `err` is no FK failure or
+ * nothing is pending (an immediate check already undid its statement).
+ */
+function namedFkFailure(db: DatabaseSync, err: unknown): ApplyFrameError | null {
+  if (!(err instanceof Error) || !err.message.includes('FOREIGN KEY constraint failed')) {
+    return null;
+  }
+  const rows = db.prepare('PRAGMA main.foreign_key_check').all() as Array<{
+    table: string;
+    rowid: number | null;
+    parent: string;
+  }>;
+  if (rows.length === 0) return null;
+  const uidOf = (table: string, rowid: number | null): string | null => {
+    try {
+      const row = db
+        .prepare(`SELECT uid FROM main."${table.replaceAll('"', '""')}" WHERE rowid = ?`)
+        .get(rowid) as { uid: string | null } | undefined;
+      return row?.uid ?? null;
+    } catch {
+      return null; // a table without a uid column: the rowid names the row
+    }
+  };
+  const named = rows
+    .slice(0, 5)
+    .map((r) => `${r.table} ${uidOf(r.table, r.rowid) ?? `rowid ${r.rowid}`} → ${r.parent}`);
+  const more = rows.length > 5 ? ` (+${rows.length - 5} more)` : '';
+  // @sync-invariant none:local-only the frame rolls back whole; this only names the rows the deferred FK check refused
+  return new ApplyFrameError(
+    'E_SYNC_APPLY_FK',
+    `FOREIGN KEY constraint failed: ${named.join('; ')}${more}`,
+  );
+}
+
 /** A synchronous result: never a thenable. */
 export type SyncResult<T> = T extends PromiseLike<unknown> ? never : T;
 
@@ -132,7 +177,11 @@ export function withApplyFrame<T>(
     db.exec('COMMIT');
     return out;
   } catch (err) {
-    if (db.isTransaction) db.exec('ROLLBACK');
+    if (db.isTransaction) {
+      const named = namedFkFailure(db, err);
+      db.exec('ROLLBACK');
+      if (named) throw named;
+    }
     throw err;
   } finally {
     active = false;