diff --git a/.changeset/seed-replay-lookup-corruption.md b/.changeset/seed-replay-lookup-corruption.md new file mode 100644 index 0000000000..230f31fd6b --- /dev/null +++ b/.changeset/seed-replay-lookup-corruption.md @@ -0,0 +1,22 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/runtime": patch +--- + +fix(seed): replaying seeds no longer corrupts lookup natural keys on the upsert update path + +Every dev-server restart replayed package seeds in upsert mode, and any record whose +lookup/master_detail was authored as a natural key could have that reference overwritten +with NULL on the update path (`NOT NULL constraint failed` on required columns; silent +link loss on nullable ones). Four fixes: + +- An unresolved reference now leaves the column untouched (deferred to pass 2) or drops + the record loudly — it is never written as NULL over an existing row. +- DB-side reference resolution probes the target dataset's declared `externalId` (e.g. + `email`) before falling back to `name` and `id`, matching how in-memory resolution + already keyed records. +- A rejected update (e.g. a `state_machine` rule vetoing the replay) no longer severs + natural-key resolution for downstream child datasets. +- Replays are idempotent: an upsert/update whose declared fields already match the + existing row is skipped instead of rewritten (no more `updated_at` churn or lifecycle + re-validation on every boot). diff --git a/packages/metadata-protocol/src/seed-loader-replay.test.ts b/packages/metadata-protocol/src/seed-loader-replay.test.ts new file mode 100644 index 0000000000..d1b5598358 --- /dev/null +++ b/packages/metadata-protocol/src/seed-loader-replay.test.ts @@ -0,0 +1,318 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi } from 'vitest'; +import { SeedLoaderService } from './seed-loader'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; + +/** + * Replay regression: seeds with lookup natural keys must survive a dev-server + * restart (the upsert UPDATE path), not just first boot (the INSERT path). + * + * Third-party eval 2026-07-17 (15.1.x): every restart, the seed replay's + * update path wrote `candidate = NULL` for records whose lookup was authored + * as a natural key — only a NOT NULL column stopped silent data corruption. + * + * Unlike seed-loader.test.ts's engine mock (whose find() ignores `where` and + * returns the whole table — masking exactly this bug), this mock filters + * `where` faithfully like a real engine. + */ + +function createLogger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + +function createFaithfulEngine(): { engine: IDataEngine; store: Record } { + const store: Record = {}; + let idCounter = 0; + + const engine = { + find: vi.fn(async (objectName: string, query?: any) => { + let records = store[objectName] || []; + if (query?.where) { + records = records.filter((r) => + Object.entries(query.where).every(([k, v]) => r[k] === v), + ); + } + if (typeof query?.limit === 'number') { + records = records.slice(0, query.limit); + } + return records; + }), + findOne: vi.fn(async (objectName: string, query?: any) => { + const rows = await (engine.find as any)(objectName, { ...query, limit: 1 }); + return rows[0] ?? null; + }), + insert: vi.fn(async (objectName: string, data: any) => { + if (!store[objectName]) store[objectName] = []; + if (Array.isArray(data)) { + const records = data.map((d) => ({ id: `gen-${++idCounter}`, ...d })); + store[objectName].push(...records); + return records; + } + const record = { id: `gen-${++idCounter}`, ...data }; + store[objectName].push(record); + return record; + }), + update: vi.fn(async (objectName: string, data: any) => { + const records = store[objectName] || []; + const idx = records.findIndex((r) => r.id === data.id); + if (idx >= 0) { + records[idx] = { ...records[idx], ...data }; + return records[idx]; + } + return data; + }), + delete: vi.fn(async () => ({ deleted: 1 })), + count: vi.fn(async (objectName: string) => (store[objectName] || []).length), + aggregate: vi.fn(async () => []), + } as unknown as IDataEngine; + + return { engine, store }; +} + +function createMetadata(): IMetadataService { + const objects: Record = { + my_app_candidate: { + name: 'my_app_candidate', + fields: { + name: { type: 'text' }, + email: { type: 'text' }, + }, + }, + my_app_interview: { + name: 'my_app_interview', + fields: { + name: { type: 'text' }, + candidate: { type: 'lookup', reference: 'my_app_candidate', required: true }, + stage: { type: 'text' }, + }, + }, + }; + return { + getObject: vi.fn(async (name: string) => objects[name]), + listObjects: vi.fn(async () => Object.values(objects)), + register: vi.fn(async () => {}), + get: vi.fn(async (_t: string, name: string) => objects[name]), + list: vi.fn(async () => []), + unregister: vi.fn(async () => {}), + exists: vi.fn(async () => false), + listNames: vi.fn(async () => []), + } as unknown as IMetadataService; +} + +// Mirrors the AppPlugin inline-seed config (defaultMode upsert, multiPass on). +const CONFIG = { + dryRun: false, + haltOnError: false, + multiPass: true, + defaultMode: 'upsert', + batchSize: 1000, + transaction: false, +} as any; + +// Parent's natural key is a non-`name` externalId (email) — the exact shape +// from the eval repro (interview.candidate: 'alice@example.com'). +const SEEDS = [ + { + object: 'my_app_candidate', + externalId: 'email', + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [ + { name: 'Alice Smith', email: 'alice@example.com' }, + { name: 'Bob Jones', email: 'bob@example.com' }, + ], + }, + { + object: 'my_app_interview', + externalId: 'name', + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [ + { name: 'iv-alice-1', candidate: 'alice@example.com', stage: 'screening' }, + { name: 'iv-bob-1', candidate: 'bob@example.com', stage: 'onsite' }, + ], + }, +] as any[]; + +/** No interview update may ever carry a NULL or unresolved (email-string) candidate. */ +function corruptInterviewUpdates(engine: IDataEngine) { + return (engine.update as any).mock.calls.filter( + ([obj, data]: [string, any]) => + obj === 'my_app_interview' && + 'candidate' in data && + (data.candidate == null || String(data.candidate).includes('@')), + ); +} + +describe('seed replay (restart) — lookup natural keys on the update path', () => { + it('keeps lookups pointing at the right parent after a second load (no NULL overwrite)', async () => { + const { engine, store } = createFaithfulEngine(); + const metadata = createMetadata(); + + // Boot #1 — fresh DB, insert path. + const first = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + expect(first.success).toBe(true); + + const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id; + const bobId = store.my_app_candidate.find((r) => r.email === 'bob@example.com')!.id; + expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId); + expect(store.my_app_interview.find((r) => r.name === 'iv-bob-1')!.candidate).toBe(bobId); + + // Boot #2 — same DB, new loader instance (dev-server restart): upsert + // matches existing rows and takes the UPDATE path. + const second = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + + // The replay must not corrupt the association: still the right parent id, + // never NULL (a nullable column would silently lose the link; a NOT NULL + // column turns every replayed record into a constraint error). + expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId); + expect(store.my_app_interview.find((r) => r.name === 'iv-bob-1')!.candidate).toBe(bobId); + expect(corruptInterviewUpdates(engine)).toEqual([]); + + expect(second.success).toBe(true); + expect(second.summary.totalErrored).toBe(0); + }); + + it('replaying an unchanged seed is a no-op (skip, no update churn)', async () => { + const { engine, store } = createFaithfulEngine(); + const metadata = createMetadata(); + + await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + (engine.update as any).mockClear(); + const before = structuredClone(store); + + const replay = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + + // Nothing the seed declares changed → no update at all: updated_at stays + // put, lifecycle validation (e.g. a state_machine rule) never re-fires. + expect(engine.update).not.toHaveBeenCalled(); + expect(replay.summary.totalUpdated).toBe(0); + expect(replay.summary.totalSkipped).toBe(4); + expect(replay.summary.totalErrored).toBe(0); + expect(store).toEqual(before); + }); + + it('a rejected parent update (state-machine style) must not cascade into NULLed child lookups', async () => { + const { engine, store } = createFaithfulEngine(); + const metadata = createMetadata(); + + await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id; + const bobId = store.my_app_candidate.find((r) => r.email === 'bob@example.com')!.id; + + // A user edited rows at runtime (the eval scenario: kanban drags changed + // candidate stages, someone rescheduled an interview) — so the replay's + // updates are NOT no-ops — and the object now vetoes the transition back + // (my-app's `Invalid stage transition.`). + for (const r of store.my_app_candidate) r.name = r.name + ' (edited)'; + for (const r of store.my_app_interview) r.stage = 'edited'; + const realUpdate = (engine.update as any).getMockImplementation(); + (engine.update as any).mockImplementation(async (objectName: string, data: any) => { + if (objectName === 'my_app_candidate') throw new Error('Invalid stage transition.'); + return realUpdate(objectName, data); + }); + + const replay = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + + // The candidate updates legitimately failed (reported), but the interview + // records still resolved their parents and re-applied the seed values. + expect(replay.summary.totalErrored).toBe(2); + expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId); + expect(store.my_app_interview.find((r) => r.name === 'iv-bob-1')!.candidate).toBe(bobId); + expect(store.my_app_interview.every((r) => r.stage !== 'edited')).toBe(true); + expect(corruptInterviewUpdates(engine)).toEqual([]); + }); + + it('resolves a lookup from the DB by the target dataset externalId when the parent dataset is absent', async () => { + const { engine, store } = createFaithfulEngine(); + const metadata = createMetadata(); + + // Parents already in the DB (seeded earlier / another load). + await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id; + + // A later load carries ONLY the child dataset, so the in-memory + // insertedRecords map has no candidate entries — resolution must fall + // back to the DB and query the candidate dataset's externalId (email), + // not the hardcoded 'name' column. + const childOnly = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: [ + { ...SEEDS[0] }, // candidate dataset present but EMPTY → only its externalId declaration matters + { + object: 'my_app_interview', + externalId: 'name', + mode: 'upsert', + env: ['prod', 'dev', 'test'], + records: [{ name: 'iv-alice-2', candidate: 'alice@example.com', stage: 'onsite' }], + }, + ].map((s, idx) => (idx === 0 ? { ...s, records: [] } : s)) as any, + config: CONFIG, + }); + + expect(childOnly.summary.totalErrored).toBe(0); + expect(store.my_app_interview.find((r) => r.name === 'iv-alice-2')!.candidate).toBe(aliceId); + }); + + it('an unresolvable lookup never reaches the row: deferred pass leaves the column alone, single-pass drops the record', async () => { + const { engine, store } = createFaithfulEngine(); + const metadata = createMetadata(); + + await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: SEEDS, + config: CONFIG, + }); + const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id; + + // Poison the seed: alice's interview now references a candidate that does + // not exist anywhere. Change another field so the update is not a no-op. + const poisoned = structuredClone(SEEDS); + poisoned[1].records[0].candidate = 'ghost@example.com'; + poisoned[1].records[0].stage = 'changed'; + + // multiPass: the update must NOT touch the candidate column (pass 2 also + // fails → reported), preserving the existing association. + const deferred = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: poisoned, + config: CONFIG, + }); + expect(deferred.success).toBe(false); + expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId); + expect(corruptInterviewUpdates(engine)).toEqual([]); + + // single-pass: the whole record is dropped (reported + counted), rather + // than written with a corrupted reference. + (engine.update as any).mockClear(); + const singlePass = await new SeedLoaderService(engine, metadata, createLogger()).load({ + seeds: poisoned, + config: { ...CONFIG, multiPass: false }, + }); + expect(singlePass.success).toBe(false); + expect(singlePass.summary.totalErrored).toBeGreaterThanOrEqual(1); + expect( + (engine.update as any).mock.calls.filter(([obj, data]: [string, any]) => obj === 'my_app_interview' && data.name === 'iv-alice-1'), + ).toEqual([]); + expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId); + }); +}); diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index 6b30ab5fa0..0daf777292 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -32,11 +32,22 @@ const DEFAULT_EXTERNAL_ID_FIELD = 'name'; * * Provides metadata-driven seed data loading with: * - Automatic lookup/master_detail reference resolution via externalId + * (in-memory for records seeded this load, DB probe by the target + * dataset's declared externalId otherwise) * - Topological dependency ordering (parents before children) * - Multi-pass loading for circular references * - Dry-run validation mode * - Upsert support honoring SeedSchema mode + * - Idempotent replay: an upsert/update whose declared fields already match + * the existing row is skipped (no update_at churn, no re-validation) — + * seeds replay on every dev-server boot and package re-publish * - Actionable error reporting + * + * Replay safety invariant: a reference that cannot be resolved is NEVER + * written as NULL (or as its raw natural-key string) over an existing row — + * resolution failures either leave the column untouched (deferred to pass 2) + * or drop the record loudly. See the 15.1.x replay corruption incident: + * every restart used to sever one lookup per replayed child record. */ export class SeedLoaderService implements ISeedLoaderService { private engine: IDataEngine; @@ -96,8 +107,18 @@ export class SeedLoaderService implements ISeedLoaderService { // 3. Order datasets by topological insert order const orderedDatasets = this.orderDatasets(datasets, graph.insertOrder); - // 4. Build reference lookup map from metadata (field → target object) - const refMap = this.buildReferenceMap(graph); + // 4. Build reference lookup map from metadata (field → target object). + // Reference values are authored against the TARGET dataset's externalId + // (e.g. `interview.candidate: 'alice@example.com'` with the candidate + // dataset declaring `externalId: 'email'`), so DB-side resolution must + // query that same field — not a hardcoded 'name'. First boot masked this: + // the in-memory insertedRecords map (keyed by the dataset's externalId) + // resolved everything, but on replay any per-record miss fell through to + // the DB probe and silently failed. See the replay corruption fix below. + const externalIdByObject = new Map( + request.seeds.map(d => [d.object, d.externalId || DEFAULT_EXTERNAL_ID_FIELD]), + ); + const refMap = this.buildReferenceMap(graph, externalIdByObject); // 5. Pass 1: Insert/upsert records, resolving references const insertedRecords = new Map>(); // object → externalIdValue → internalId @@ -351,6 +372,7 @@ export class SeedLoaderService implements ISeedLoaderService { } // Resolve references + let unresolvedRefError = false; for (const ref of objectRefs) { const fieldValue = record[ref.field]; if (fieldValue === undefined || fieldValue === null) continue; @@ -382,7 +404,10 @@ export class SeedLoaderService implements ISeedLoaderService { allErrors.push(error); this.logger.warn(`[SeedLoader] ${error.message}`, { recordIndex: i }); // Drop the unresolvable value so it never reaches the driver. - record[ref.field] = null; + // Removing the key (not writing null) matters on the upsert UPDATE + // path: an explicit null would overwrite the existing row's valid + // reference, silently severing the link on every seed replay. + delete record[ref.field]; continue; } @@ -403,8 +428,14 @@ export class SeedLoaderService implements ISeedLoaderService { record[ref.field] = dbId; referencesResolved++; } else if (config.multiPass) { - // Defer to pass 2 - record[ref.field] = null; + // Defer to pass 2. REMOVE the field rather than writing null: + // on insert a missing column lands NULL anyway (placeholder until + // pass 2 back-fills it), but on the upsert UPDATE path an explicit + // null would OVERWRITE the existing row's already-correct + // reference — every dev-server restart severed one link per + // replayed record (NOT NULL columns turned this into a loud + // constraint error; nullable ones silently lost the association). + delete record[ref.field]; deferredUpdates.push({ objectName, recordExternalId: String(record[externalId] ?? ''), @@ -416,7 +447,10 @@ export class SeedLoaderService implements ISeedLoaderService { }); referencesDeferred++; } else { - // Cannot resolve - record error + // Cannot resolve and no pass 2 will run — skip the whole record + // (LOUD: counted + reported). Writing it anyway would either + // carry the raw natural-key string into the FK column or, on + // update, corrupt the existing row. const error: ReferenceResolutionError = { sourceObject: objectName, field: ref.field, @@ -428,6 +462,7 @@ export class SeedLoaderService implements ISeedLoaderService { }; errors.push(error); allErrors.push(error); + unresolvedRefError = true; } } else { // Dry-run: attempt resolution, report error if not found @@ -448,6 +483,14 @@ export class SeedLoaderService implements ISeedLoaderService { } } + // A definitively unresolvable reference (no pass 2 to fix it) drops the + // record — reported above, counted here. Better a missing seed row than + // a written one with a corrupted or unresolved reference. + if (unresolvedRefError && !config.dryRun) { + errored++; + continue; + } + // Insert/upsert the record if (!config.dryRun) { if (hasSelfRef) { @@ -470,6 +513,14 @@ export class SeedLoaderService implements ISeedLoaderService { } } catch (err: any) { errored++; + // Same cascade guard as the batched update path: the row may + // already exist (rejected update), so keep its natural-key + // mapping alive for downstream reference resolution. + const existingId = this.extractId(existingRecords?.get(String(record[externalId] ?? ''))); + const externalIdValue = String(record[externalId] ?? ''); + if (externalIdValue && existingId) { + insertedRecords.get(objectName)!.set(externalIdValue, existingId); + } const error = this.buildWriteError(objectName, record, externalId, i, err); errors.push(error); allErrors.push(error); @@ -485,12 +536,19 @@ export class SeedLoaderService implements ISeedLoaderService { insertedRecords.get(objectName)!.set(externalIdValue, decision.id); } } else if (decision.action === 'update') { + // Register the externalId → id mapping BEFORE attempting the + // write: the row exists and its id is known regardless of whether + // this update succeeds. A rejected update (e.g. a state_machine + // rule vetoing the transition back to the seed value) must not + // sever downstream natural-key resolution — that cascade is what + // turned one legitimate validation error into NULLed-out child + // references on every dev-server restart. + if (externalIdValue) { + insertedRecords.get(objectName)!.set(externalIdValue, decision.id); + } try { await withTransientRetry(() => this.engine.update(objectName, { ...record, id: decision.id }, opts)); updated++; - if (externalIdValue) { - insertedRecords.get(objectName)!.set(externalIdValue, decision.id); - } } catch (err: any) { errored++; const error = this.buildWriteError(objectName, record, externalId, i, err); @@ -570,43 +628,38 @@ export class SeedLoaderService implements ISeedLoaderService { value: unknown, organizationId?: string, ): Promise { - try { - const where: Record = { [targetField]: value }; - // Per-tenant replay: when scoping is requested, only consider - // rows that belong to the target tenant so cross-tenant rows - // never get borrowed as a "resolved" reference (would silently - // create a cross-org FK). - if (organizationId) where.organization_id = organizationId; - const records = await this.engine.find(targetObject, { - where, - fields: ['id'], - limit: 1, - context: { isSystem: true }, - } as any); - if (records && records.length > 0) { - return String(records[0].id || records[0]._id); - } - // Fallback: the value may already be the target's internal id rather than - // its natural key — a seed that wires a lookup to a real existing record - // (e.g. a people field → the current user, whose id is not a UUID/ObjectId - // so `looksLikeInternalId` did not short-circuit). Resolving by id lets a - // valid id resolve instead of dangling null, with no risk of a false - // natural-key match (an id either exists or it does not). - if (targetField !== 'id') { - const byId: Record = { id: value }; - if (organizationId) byId.organization_id = organizationId; - const idMatch = await this.engine.find(targetObject, { - where: byId, + // Probe order: the target dataset's declared externalId (threaded in as + // `targetField` via buildReferenceMap), then the historical 'name' + // default, then the internal id. Each is exact-match, so extra probes + // can only rescue a reference, never mis-resolve one. + const probeFields = [targetField]; + if (targetField !== DEFAULT_EXTERNAL_ID_FIELD) probeFields.push(DEFAULT_EXTERNAL_ID_FIELD); + if (targetField !== 'id') probeFields.push('id'); + for (const probeField of probeFields) { + try { + const where: Record = { [probeField]: value }; + // Per-tenant replay: when scoping is requested, only consider + // rows that belong to the target tenant so cross-tenant rows + // never get borrowed as a "resolved" reference (would silently + // create a cross-org FK). + if (organizationId) where.organization_id = organizationId; + const records = await this.engine.find(targetObject, { + where, fields: ['id'], limit: 1, context: { isSystem: true }, } as any); - if (idMatch && idMatch.length > 0) { - return String(idMatch[0].id || idMatch[0]._id); + // The 'id' probe covers a seed that wires a lookup to a real existing + // record (e.g. a people field → the current user, whose id is not a + // UUID/ObjectId so `looksLikeInternalId` did not short-circuit); an id + // either exists or it does not, so there is no false-match risk. + if (records && records.length > 0) { + return String(records[0].id || records[0]._id); } + } catch { + // Target object (or this probe's column) may not exist — try the next + // probe rather than aborting resolution outright. } - } catch { - // Target object may not exist yet } return null; } @@ -720,6 +773,9 @@ export class SeedLoaderService implements ISeedLoaderService { return { action: 'skipped' }; } const id = this.extractId(existing); + if (this.isNoOpReplay(record, existing)) { + return { action: 'skipped', id }; + } await this.engine.update(objectName, { ...record, id }, opts); return { action: 'updated', id }; } @@ -727,6 +783,9 @@ export class SeedLoaderService implements ISeedLoaderService { case 'upsert': { if (existing) { const id = this.extractId(existing); + if (this.isNoOpReplay(record, existing)) { + return { action: 'skipped', id }; + } await this.engine.update(objectName, { ...record, id }, opts); return { action: 'updated', id }; } else { @@ -776,9 +835,15 @@ export class SeedLoaderService implements ISeedLoaderService { switch (mode) { case 'update': - return existing ? { action: 'update', id: this.extractId(existing)! } : { action: 'skip' }; + if (!existing) return { action: 'skip' }; + return this.isNoOpReplay(record, existing) + ? { action: 'skip', id: this.extractId(existing) } + : { action: 'update', id: this.extractId(existing)! }; case 'upsert': - return existing ? { action: 'update', id: this.extractId(existing)! } : { action: 'insert' }; + if (!existing) return { action: 'insert' }; + return this.isNoOpReplay(record, existing) + ? { action: 'skip', id: this.extractId(existing) } + : { action: 'update', id: this.extractId(existing)! }; case 'ignore': return existing ? { action: 'skip', id: this.extractId(existing) } : { action: 'insert' }; case 'insert': @@ -788,6 +853,55 @@ export class SeedLoaderService implements ISeedLoaderService { } } + /** + * A seed replay (dev-server restart, package re-publish) re-loads the same + * records over existing rows. When nothing the seed declares actually + * differs, rewriting the row is pure churn: `updated_at` gets bumped every + * boot, lifecycle validation re-runs (a state_machine rule can even veto + * the no-op), and history tracking logs a phantom edit. Skip those. + * + * Only fields PRESENT in the seed record are compared (the row's extra + * columns — audit fields, values edited at runtime that the seed does not + * pin — never block the skip). Comparison is conservative: any doubt + * (unparseable dates, type mismatches) reads as "changed", falling back to + * the historical update behavior. + */ + private isNoOpReplay(record: Record, existing: Record): boolean { + for (const [key, value] of Object.entries(record)) { + if (key === 'id') continue; + if (!this.seedValueEquals(value, existing[key])) return false; + } + return true; + } + + /** Loose equality across driver round-trip representations (see isNoOpReplay). */ + private seedValueEquals(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a == null || b == null) return a == null && b == null; + // Booleans come back as 0/1 from SQLite. + if (typeof a === 'boolean' || typeof b === 'boolean') { + const toNum = (v: unknown) => (typeof v === 'boolean' ? Number(v) : Number(String(v))); + return toNum(a) === toNum(b); + } + // Dates come back as driver-formatted strings or epoch numbers. + if (a instanceof Date || b instanceof Date) { + const toTime = (v: unknown) => + v instanceof Date ? v.getTime() : typeof v === 'number' ? v : Date.parse(String(v)); + const ta = toTime(a); + const tb = toTime(b); + return Number.isFinite(ta) && Number.isFinite(tb) && ta === tb; + } + if (typeof a === 'object' || typeof b === 'object') { + try { + return JSON.stringify(a) === JSON.stringify(b); + } catch { + return false; + } + } + // number 5 vs '5' after a driver round-trip. + return String(a) === String(b); + } + /** Builds the same `ReferenceResolutionError` shape a failed write has always reported. */ private buildWriteError( objectName: string, @@ -940,11 +1054,26 @@ export class SeedLoaderService implements ISeedLoaderService { }); } - private buildReferenceMap(graph: ObjectDependencyGraph): Map { + private buildReferenceMap( + graph: ObjectDependencyGraph, + externalIdByObject?: Map, + ): Map { const map = new Map(); for (const node of graph.nodes) { if (node.references.length > 0) { - map.set(node.object, node.references); + // Resolve against the TARGET dataset's declared externalId when this + // load carries one (copy-on-write — graph.nodes is part of the public + // result and keeps the metadata-level 'name' default). Targets with + // no dataset in this load (e.g. a user field → os_user) keep 'name'. + const references = externalIdByObject + ? node.references.map(ref => { + const datasetExternalId = externalIdByObject.get(ref.targetObject); + return datasetExternalId && datasetExternalId !== ref.targetField + ? { ...ref, targetField: datasetExternalId } + : ref; + }) + : node.references; + map.set(node.object, references); } } return map; @@ -957,8 +1086,9 @@ export class SeedLoaderService implements ISeedLoaderService { ): Promise> { const map = new Map(); try { + // Full rows (not just id + externalId): the write decision compares the + // incoming seed record against the existing row to skip no-op replays. const findArgs: Record = { - fields: ['id', externalId], context: { isSystem: true }, }; // Per-tenant replay: restrict to the target tenant's own rows diff --git a/packages/runtime/src/seed-loader.test.ts b/packages/runtime/src/seed-loader.test.ts index cc81fc0d94..f21590549d 100644 --- a/packages/runtime/src/seed-loader.test.ts +++ b/packages/runtime/src/seed-loader.test.ts @@ -457,10 +457,12 @@ describe('SeedLoaderService', () => { expect(refError!.message).toContain("account_id: \"Acme Corp\""); // The object wrapper must NOT reach the driver (it would throw "can only - // bind"); the guard drops it to null instead. + // bind"); the guard REMOVES the field (an explicit null would overwrite + // the existing row's valid reference on the upsert update path). const contactInsertCall = (engine.insert as any).mock.calls.find((c: any[]) => c[0] === 'contact'); if (contactInsertCall) { - expect(contactInsertCall[1].account_id).toBeNull(); + const written = Array.isArray(contactInsertCall[1]) ? contactInsertCall[1][0] : contactInsertCall[1]; + expect('account_id' in written).toBe(false); } });