diff --git a/.changeset/t13324-carry-bare-accounts.md b/.changeset/t13324-carry-bare-accounts.md new file mode 100644 index 0000000000..c241445145 --- /dev/null +++ b/.changeset/t13324-carry-bare-accounts.md @@ -0,0 +1,8 @@ +--- +id: t13324-carry-bare-accounts +tasks: [T13324] +kind: fix +summary: a vault restore keeps the reconcile record that matches the restored bare tables, so a pulled reconciled store enables sync +--- + +The store's record of the bare legacy tables a reconcile carried (`_exodus_recovery_bare_accounts`, T13320) is local-only, so a vault pull replaced it with this machine's (usually none) while the twins arrived from the reconciled device: the pulled store was refused as a strand. A restore now settles it after the bare tables (local-only) are this machine's: per bare table it keeps the snapshot's record, else this machine's, whichever still matches the bare table by row count and key digest, and drops a record neither matches. Bundles already carry the table whole; it never syncs. diff --git a/packages/core/src/cloud/__tests__/nexus-vault.test.ts b/packages/core/src/cloud/__tests__/nexus-vault.test.ts index fef44c5c68..1a606d0d52 100644 --- a/packages/core/src/cloud/__tests__/nexus-vault.test.ts +++ b/packages/core/src/cloud/__tests__/nexus-vault.test.ts @@ -67,7 +67,15 @@ import { ROW_IDENTITY_RECIPE_KEY, } from '../../store/row-identity.js'; import { setCaptureEnabled } from '../../store/sync/capture.js'; -import { isSyncFlagOn, setSyncFlag } from '../../store/sync/flags.js'; +import { + BARE_ACCOUNTS_TABLE, + bareTableDigest, + isSyncFlagOn, + LegacyOnlyStoreError, + legacyStrands, + recordBareAccounts, + setSyncFlag, +} from '../../store/sync/flags.js'; import { cutGenesis, genesisCutOf, genesisPending } from '../../store/sync/genesis.js'; import { readStreamCursor } from '../../store/sync/pull.js'; import { ensureProjectReplica, storeHwm } from '../../store/sync/replica.js'; @@ -3287,6 +3295,56 @@ describe('cloud vault round 3 (#1773)', () => { ]); }); + it("device B pulls device A's reconciled store and enables sync without being refused (T13324)", async () => { + const { a, b } = await twoMachines(); + await on(a, () => pushNexusVault(vopts(a))); + await restoreOntoB(b); + // Both machines hold the project's bare legacy rows (local-only, never in a snapshot). + const legacy = `CREATE TABLE tasks (id TEXT PRIMARY KEY, title TEXT); + INSERT INTO tasks VALUES ('T0', 'task 0'), ('T8', 'legacy'), ('T9', 'legacy');`; + exec(b, legacy); + const enable = (m: Machine) => { + const db = new DatabaseSync(path.join(m.root, '.cleo', 'cleo.db')); + try { + // The fixture's two-column _sync_replica predates the journal schema. + db.exec('DROP TABLE IF EXISTS _sync_replica'); + return setSyncFlag(db, 'sync.capture', true); + } finally { + db.close(); + } + }; + expect(() => enable(b)).toThrow(LegacyOnlyStoreError); + // A reconciles: T8 and T9 are carried, T9 is deleted since, and the store + // records the run (T13320). + exec(a, `${legacy} INSERT INTO tasks_tasks (id, title) VALUES ('T8', 'legacy');`); + const da = new DatabaseSync(path.join(a.root, '.cleo', 'cleo.db')); + try { + recordBareAccounts(da, [bareTableDigest(da, 'main', 'tasks')], 'exodus-reconcile-a'); + expect(legacyStrands(da)).toEqual([]); + } finally { + da.close(); + } + await on(a, () => pushNexusVault(vopts(a))); + + const pulled = await on(b, () => restoreNexusVault(vopts(b, { mode: 'pull' }))); + expect(pulled.status).toBe('restored'); + // B keeps its own bare rows and takes A's record, which describes them. + expect(sql(b, 'SELECT id FROM tasks ORDER BY id')).toEqual([ + { id: 'T0' }, + { id: 'T8' }, + { id: 'T9' }, + ]); + expect(sql(b, "SELECT id FROM tasks_tasks WHERE id IN ('T8', 'T9')")).toEqual([{ id: 'T8' }]); + expect(sql(b, `SELECT bare_table, run FROM "${BARE_ACCOUNTS_TABLE}"`)).toEqual([ + { bare_table: 'tasks', run: 'exodus-reconcile-a' }, + ]); + expect((await on(b, () => verifyNexusVault(vopts(b)))).verdict).toBe('match'); + expect(enable(b)).toBe(true); + expect(sql(b, "SELECT value FROM _sync_meta WHERE key = 'sync.capture'")).toEqual([ + { value: '1' }, + ]); + }); + it('N6: a push whose state write was lost does not leave the device behind its own snapshot', async () => { const { a, b } = await twoMachines(); await on(a, () => pushNexusVault(vopts(a))); diff --git a/packages/core/src/store/__tests__/vault-manifest.test.ts b/packages/core/src/store/__tests__/vault-manifest.test.ts index 9b862d797e..099d71005a 100644 --- a/packages/core/src/store/__tests__/vault-manifest.test.ts +++ b/packages/core/src/store/__tests__/vault-manifest.test.ts @@ -16,8 +16,18 @@ import type { DatabaseSync as _DatabaseSyncType } from 'node:sqlite'; import { SYNC_SCHEMA_VERSION } from '@cleocode/contracts'; import { VAULT_REMOTE_PATH_PREFIX } from '@cleocode/paths'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { + BARE_ACCOUNTS_TABLE, + bareTableDigest, + legacyStrands, + recordBareAccounts, +} from '../sync/flags.js'; import { TRIGGER_SUSPEND_TABLE_DDL } from '../sync/trigger-classes.js'; -import { classifyTable, SCHEMA_STATE_TABLES } from '../table-classification.js'; +import { + classifyTable, + SCHEMA_STATE_TABLES, + SNAPSHOT_STATE_TABLES, +} from '../table-classification.js'; import { buildVaultManifest, carryMachineState, @@ -1013,3 +1023,98 @@ describe('carry keeps the staged file schema state (T13104)', () => { } }); }); + +describe('carry settles the reconcile record against the bare tables as restored (T13324)', () => { + /** + * A store whose twin holds T1 and whose bare `tasks` (local-only) holds + * `bare`; with `record`, the store recorded a reconcile carrying that bare + * table (T13320). A recorded T2 the twin lacks was carried and deleted since. + */ + const store = (name: string, bare: readonly string[], record: boolean) => { + const f = path.join(tmp, `${name}.db`); + const db = new DatabaseSync(f); + db.exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, title TEXT); + CREATE TABLE tasks_tasks (id TEXT PRIMARY KEY, title TEXT); + INSERT INTO tasks_tasks VALUES ('T1', 'legacy');`); + const ins = db.prepare("INSERT INTO tasks VALUES (?, 'legacy')"); + for (const id of bare) ins.run(id); + if (record) recordBareAccounts(db, [bareTableDigest(db, 'main', 'tasks')], `run-${name}`); + db.close(); + return f; + }; + const records = (f: string) => { + const db = new DatabaseSync(f, { readOnly: true }); + try { + const has = db + .prepare("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = ?") + .get(BARE_ACCOUNTS_TABLE); + return has === undefined + ? [] + : db.prepare(`SELECT bare_table, run FROM "${BARE_ACCOUNTS_TABLE}"`).all(); + } finally { + db.close(); + } + }; + const strands = (f: string) => { + const db = new DatabaseSync(f); + try { + return legacyStrands(db); + } finally { + db.close(); + } + }; + + it("a pull keeps the snapshot's record when this machine's bare table matches it", () => { + const staged = store('staged', ['T1', 'T2'], true); + const live = store('live', ['T1', 'T2'], false); + // Carrying this machine's record (none) would refuse the restored store. + expect(strands(live)).toEqual([ + { bareTable: 'tasks', table: 'tasks_tasks', missing: 1, shadowed: 0 }, + ]); + const out = carryMachineState(staged, live, 'project'); + expect(records(staged)).toEqual([{ bare_table: 'tasks', run: 'run-staged' }]); + expect(out.preserved).not.toContain(BARE_ACCOUNTS_TABLE); + expect(out.skipped).not.toContain(BARE_ACCOUNTS_TABLE); + expect(strands(staged)).toEqual([]); + }); + + it("a pull keeps this machine's record when the snapshot has none that matches", () => { + const staged = store('staged', ['T1', 'T2'], false); + const live = store('live', ['T1', 'T2'], true); + carryMachineState(staged, live, 'project'); + expect(records(staged)).toEqual([{ bare_table: 'tasks', run: 'run-live' }]); + expect(strands(staged)).toEqual([]); + }); + + it('a record whose key digest does not match the bare table as restored is dropped', () => { + const staged = store('staged', ['T1', 'T2'], true); + const live = store('live', ['T1', 'T2', 'T3'], false); + carryMachineState(staged, live, 'project'); + expect(records(staged)).toEqual([]); + expect(strands(staged)).toEqual([ + { bareTable: 'tasks', table: 'tasks_tasks', missing: 2, shadowed: 0 }, + ]); + }); + + it('a new machine has no bare rows, so the record goes and nothing is stranded', () => { + const staged = store('staged', ['T1', 'T2'], true); + carryMachineState(staged, null, 'project', { snapshotRoot: '/A/root' }); + expect(records(staged)).toEqual([]); + expect(strands(staged)).toEqual([]); + }); + + it('every snapshot-state table is local-only in its scope (never synced, never in the manifest)', () => { + expect(SNAPSHOT_STATE_TABLES.project).toContain(BARE_ACCOUNTS_TABLE); + for (const scope of ['project', 'global'] as const) { + for (const t of SNAPSHOT_STATE_TABLES[scope]) { + const c = classifyTable(scope, t); + expect({ scope, t, class: 'class' in c ? c.class : c.kind }).toEqual({ + scope, + t, + class: 'local-only', + }); + expect(isVaultManifestTable(scope, t)).toBe(false); + } + } + }); +}); diff --git a/packages/core/src/store/sync/flags.ts b/packages/core/src/store/sync/flags.ts index f487179255..aa69b1e19e 100644 --- a/packages/core/src/store/sync/flags.ts +++ b/packages/core/src/store/sync/flags.ts @@ -477,6 +477,79 @@ export function forgetBareAccounts(db: DatabaseSync, run: string): number { ); } +/** One record of {@link BARE_ACCOUNTS_TABLE}, as the store holds it. */ +export interface BareAccountRecord { + /** The bare legacy table. */ + readonly table: string; + /** Its row count when the run verified. */ + readonly rows: number; + /** Its key digest when the run verified ({@link bareTableDigest}). */ + readonly digest: string; + /** The reconcile's run directory name. */ + readonly run: string; + /** When the run verified (ISO 8601). */ + readonly recordedAt: string; +} + +/** + * Every record of {@link BARE_ACCOUNTS_TABLE} in `db` (none without the table). + * + * @param db - The store. + */ +export function readBareAccounts(db: DatabaseSync): BareAccountRecord[] { + if (!hasTable(db, BARE_ACCOUNTS_TABLE)) return []; + return ( + db + .prepare( + `SELECT bare_table AS "table", rows, digest, run, recorded_at AS recordedAt FROM main."${BARE_ACCOUNTS_TABLE}"`, + ) + .all() as Array<{ + table: string; + rows: number; + digest: string; + run: string; + recordedAt: string; + }> + ).map((r) => ({ ...r, rows: Number(r.rows) })); +} + +/** + * Settle the record of carried bare tables in a restored store (T13324). + * + * A vault restore places the snapshot's twins and record, but this machine's + * bare tables (local-only; none on a new machine). Per bare table, the record + * kept is the first of the store's own and `others` (this machine's, from + * before the restore) whose row count and key digest match the bare table as + * restored; a table neither matches loses its record, so its stranded rows are + * judged again ({@link legacyStrands}). + * + * @param db - The restored store, writable. + * @param others - This machine's records before the restore. + * @returns The bare tables whose record was dropped. + */ +export function settleRestoredBareAccounts( + db: DatabaseSync, + others: readonly BareAccountRecord[], +): string[] { + const own = readBareAccounts(db); + const tables = [...new Set([...own, ...others].map((r) => r.table))].sort(); + const dropped: string[] = []; + for (const table of tables) { + const mine = own.find((r) => r.table === table); + const theirs = others.find((r) => r.table === table); + const matches = (r: BareAccountRecord | undefined): r is BareAccountRecord => + r !== undefined && hasTable(db, table) && isAccounted(db, table, [r]); + if (matches(mine)) continue; + if (matches(theirs)) { + recordBareAccounts(db, [theirs], theirs.run, new Date(theirs.recordedAt)); + } else if (mine !== undefined) { + db.prepare(`DELETE FROM main."${BARE_ACCOUNTS_TABLE}" WHERE bare_table = ?`).run(table); + dropped.push(table); + } + } + return dropped; +} + /** Whether `bare` is unchanged since the store recorded a reconcile carrying it. */ function isAccounted(db: DatabaseSync, bare: string, records: readonly BareTableRecord[]): boolean { const mine = records.find((r) => r.table === bare); diff --git a/packages/core/src/store/table-classification.ts b/packages/core/src/store/table-classification.ts index fad07d475f..ebc0d929c3 100644 --- a/packages/core/src/store/table-classification.ts +++ b/packages/core/src/store/table-classification.ts @@ -2334,3 +2334,39 @@ export const SCHEMA_STATE_TABLES: Readonly export function isSchemaStateTable(scope: TableScope, name: string): boolean { return SCHEMA_STATE_TABLES[scope].includes(name); } + +/** + * The `local-only` tables whose rows record facts about OTHER rows of the same + * file, so they are consistent only with the snapshot that holds those rows: + * `_exodus_recovery_bare_accounts`, the key digests of the bare legacy tables + * a reconcile carried (T13320; `BARE_ACCOUNTS_TABLE` in `store/sync/flags.ts`). + * + * They never sync, but a snapshot or bundle carries them with the rows they + * describe, and a vault restore does not carry this machine's rows whole: it + * keeps, per bare table, the record (the snapshot's, else this machine's) that + * still matches the bare table as restored. Carrying only this machine's + * record would refuse as a strand a restored store whose twins the snapshot's + * reconcile settled (T13324). + * + * Every name here is classified `local-only` in its scope (asserted by the + * classification tests). + * + * @task T13324 + */ +export const SNAPSHOT_STATE_TABLES: Readonly> = { + project: ['_exodus_recovery_bare_accounts'], + global: [], +}; + +/** + * Whether a table records facts about the rows of its own file + * ({@link SNAPSHOT_STATE_TABLES}). + * + * @param scope - Which `cleo.db` the table lives in. + * @param name - The physical name as `sqlite_master` reports it. + * @returns `true` for a table a restore keeps as the snapshot has it. + * @task T13324 + */ +export function isSnapshotStateTable(scope: TableScope, name: string): boolean { + return SNAPSHOT_STATE_TABLES[scope].includes(name); +} diff --git a/packages/core/src/store/vault-manifest.ts b/packages/core/src/store/vault-manifest.ts index 9fdb597443..1e0f015ccf 100644 --- a/packages/core/src/store/vault-manifest.ts +++ b/packages/core/src/store/vault-manifest.ts @@ -44,6 +44,7 @@ import { } from './portable-bundle-relocate.js'; import { CREDENTIAL_COLUMNS, credentialRemedy, sha256File } from './portable-bundle-scan.js'; import { applyPerfPragmas } from './sqlite-pragmas.js'; +import { readBareAccounts, settleRestoredBareAccounts } from './sync/flags.js'; import { hasTriggerSuspendTable, TRIGGER_SUSPEND_TABLE, @@ -54,6 +55,7 @@ import { getTableRegistry, isPortableTableClass, isSchemaStateTable, + isSnapshotStateTable, } from './table-classification.js'; // node:sqlite interop (createRequire — Vitest strips `node:` prefix) @@ -715,6 +717,11 @@ function stableKey( * schema-version sentinels), which stays as the snapshot has it: it * describes the staged file, so the migrator that opens the restored store * reads the journal of that file, not this machine's old one or none (T13104). + * A table recording facts about the file's own rows (`SNAPSHOT_STATE_TABLES`: + * the bare-table key digests a reconcile recorded) is not carried either: + * once the bare tables (local-only) are this machine's, each keeps the + * record, the snapshot's or else this machine's, that still matches it, and + * loses one neither matches (T13324). * - `portable-secret` tables: live rows are upserted by a stable key (the * snapshot's copies arrive with their secrets cleared). Without a stable * key the live table is kept whole. @@ -823,6 +830,8 @@ export function carryMachineState( if (t === TRIGGER_SUSPEND_TABLE) continue; // The file's own schema state travels with the file (T13104). if (isSchemaStateTable(scope, t)) continue; + // So does what it records about its own rows, settled below (T13324). + if (isSnapshotStateTable(scope, t)) continue; const c = classifyTable(scope, t); if (c.kind !== 'entry' && c.kind !== 'pattern') continue; const credentialCols = CREDENTIAL_COLUMNS[t] ?? []; @@ -992,6 +1001,9 @@ export function carryMachineState( } if (scrubbed > 0) out.scrubbed.push({ table: t, rows: scrubbed }); } + // The bare tables are this machine's now: keep the record (the + // snapshot's, else this machine's) that still matches them (T13324). + if (scope === 'project') settleRestoredBareAccounts(staged, readBareAccounts(live)); }; // Carrying this machine's values is not a change to capture, and no // side-effect trigger may act on it (journal S2, T12819); guards stay on.