Skip to content

Commit 515f11a

Browse files
os-zhuangclaude
andauthored
fix(seed): seed replay must not corrupt lookup natural keys on the upsert update path (#3097)
Third-party eval (15.1.x, 2026-07-17): every dev-server restart, the seed replay severed lookups authored as natural keys — `update my_app_interview set candidate = NULL … NOT NULL constraint failed`. Only the NOT NULL column stood between the replay and silent data loss; a nullable lookup would have been cleared without a trace. Root-cause chain (three stacked defects): 1. A parent update legitimately rejected by a state_machine rule (user had edited rows at runtime) did not register the row's externalId → id mapping, severing in-memory natural-key resolution for every child dataset. 2. The DB fallback probe hardcoded `name` as the match column instead of the target dataset's declared externalId (`email`), so it could never rescue the miss — first boot masked this because in-memory resolution handled everything. 3. The multi-pass defer path wrote `record[field] = null` straight through the update, overwriting the existing row's correct reference on every replay. Fixes: - Resolution failures never write NULL (or the raw natural-key string) over an existing row: deferred references leave the column untouched for pass 2; a definitively unresolvable reference drops the record loudly (counted + reported). - resolveFromDatabase probes the target dataset's externalId first, then `name`, then `id` — exact matches only, so extra probes can rescue but never mis-resolve. - update registers externalId → id before attempting the write (the row exists regardless of the write's outcome), on both the batched and the self-referencing paths. - Idempotent replay: an upsert/update whose declared fields already match the existing row is skipped — no updated_at churn, no lifecycle re-validation, no state_machine veto on every boot. Regression tests replay seeds twice through a faithful engine mock (where- filtering, unlike the legacy mock that returned the whole table and masked the DB-probe bug), covering the rejected-parent cascade, the externalId DB probe, defer-must-not-touch, single-pass record drop, and no-op skips. Verified end-to-end on the real stack (AppPlugin → ObjectQL → SqlDriver, three boots incl. user-edited rows + FSM veto): links intact, zero constraint errors, idempotent third boot. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 8923843 commit 515f11a

4 files changed

Lines changed: 519 additions & 47 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/runtime": patch
4+
---
5+
6+
fix(seed): replaying seeds no longer corrupts lookup natural keys on the upsert update path
7+
8+
Every dev-server restart replayed package seeds in upsert mode, and any record whose
9+
lookup/master_detail was authored as a natural key could have that reference overwritten
10+
with NULL on the update path (`NOT NULL constraint failed` on required columns; silent
11+
link loss on nullable ones). Four fixes:
12+
13+
- An unresolved reference now leaves the column untouched (deferred to pass 2) or drops
14+
the record loudly — it is never written as NULL over an existing row.
15+
- DB-side reference resolution probes the target dataset's declared `externalId` (e.g.
16+
`email`) before falling back to `name` and `id`, matching how in-memory resolution
17+
already keyed records.
18+
- A rejected update (e.g. a `state_machine` rule vetoing the replay) no longer severs
19+
natural-key resolution for downstream child datasets.
20+
- Replays are idempotent: an upsert/update whose declared fields already match the
21+
existing row is skipped instead of rewritten (no more `updated_at` churn or lifecycle
22+
re-validation on every boot).
Lines changed: 318 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,318 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi } from 'vitest';
4+
import { SeedLoaderService } from './seed-loader';
5+
import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts';
6+
7+
/**
8+
* Replay regression: seeds with lookup natural keys must survive a dev-server
9+
* restart (the upsert UPDATE path), not just first boot (the INSERT path).
10+
*
11+
* Third-party eval 2026-07-17 (15.1.x): every restart, the seed replay's
12+
* update path wrote `candidate = NULL` for records whose lookup was authored
13+
* as a natural key — only a NOT NULL column stopped silent data corruption.
14+
*
15+
* Unlike seed-loader.test.ts's engine mock (whose find() ignores `where` and
16+
* returns the whole table — masking exactly this bug), this mock filters
17+
* `where` faithfully like a real engine.
18+
*/
19+
20+
function createLogger() {
21+
return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() };
22+
}
23+
24+
function createFaithfulEngine(): { engine: IDataEngine; store: Record<string, any[]> } {
25+
const store: Record<string, any[]> = {};
26+
let idCounter = 0;
27+
28+
const engine = {
29+
find: vi.fn(async (objectName: string, query?: any) => {
30+
let records = store[objectName] || [];
31+
if (query?.where) {
32+
records = records.filter((r) =>
33+
Object.entries(query.where).every(([k, v]) => r[k] === v),
34+
);
35+
}
36+
if (typeof query?.limit === 'number') {
37+
records = records.slice(0, query.limit);
38+
}
39+
return records;
40+
}),
41+
findOne: vi.fn(async (objectName: string, query?: any) => {
42+
const rows = await (engine.find as any)(objectName, { ...query, limit: 1 });
43+
return rows[0] ?? null;
44+
}),
45+
insert: vi.fn(async (objectName: string, data: any) => {
46+
if (!store[objectName]) store[objectName] = [];
47+
if (Array.isArray(data)) {
48+
const records = data.map((d) => ({ id: `gen-${++idCounter}`, ...d }));
49+
store[objectName].push(...records);
50+
return records;
51+
}
52+
const record = { id: `gen-${++idCounter}`, ...data };
53+
store[objectName].push(record);
54+
return record;
55+
}),
56+
update: vi.fn(async (objectName: string, data: any) => {
57+
const records = store[objectName] || [];
58+
const idx = records.findIndex((r) => r.id === data.id);
59+
if (idx >= 0) {
60+
records[idx] = { ...records[idx], ...data };
61+
return records[idx];
62+
}
63+
return data;
64+
}),
65+
delete: vi.fn(async () => ({ deleted: 1 })),
66+
count: vi.fn(async (objectName: string) => (store[objectName] || []).length),
67+
aggregate: vi.fn(async () => []),
68+
} as unknown as IDataEngine;
69+
70+
return { engine, store };
71+
}
72+
73+
function createMetadata(): IMetadataService {
74+
const objects: Record<string, any> = {
75+
my_app_candidate: {
76+
name: 'my_app_candidate',
77+
fields: {
78+
name: { type: 'text' },
79+
email: { type: 'text' },
80+
},
81+
},
82+
my_app_interview: {
83+
name: 'my_app_interview',
84+
fields: {
85+
name: { type: 'text' },
86+
candidate: { type: 'lookup', reference: 'my_app_candidate', required: true },
87+
stage: { type: 'text' },
88+
},
89+
},
90+
};
91+
return {
92+
getObject: vi.fn(async (name: string) => objects[name]),
93+
listObjects: vi.fn(async () => Object.values(objects)),
94+
register: vi.fn(async () => {}),
95+
get: vi.fn(async (_t: string, name: string) => objects[name]),
96+
list: vi.fn(async () => []),
97+
unregister: vi.fn(async () => {}),
98+
exists: vi.fn(async () => false),
99+
listNames: vi.fn(async () => []),
100+
} as unknown as IMetadataService;
101+
}
102+
103+
// Mirrors the AppPlugin inline-seed config (defaultMode upsert, multiPass on).
104+
const CONFIG = {
105+
dryRun: false,
106+
haltOnError: false,
107+
multiPass: true,
108+
defaultMode: 'upsert',
109+
batchSize: 1000,
110+
transaction: false,
111+
} as any;
112+
113+
// Parent's natural key is a non-`name` externalId (email) — the exact shape
114+
// from the eval repro (interview.candidate: 'alice@example.com').
115+
const SEEDS = [
116+
{
117+
object: 'my_app_candidate',
118+
externalId: 'email',
119+
mode: 'upsert',
120+
env: ['prod', 'dev', 'test'],
121+
records: [
122+
{ name: 'Alice Smith', email: 'alice@example.com' },
123+
{ name: 'Bob Jones', email: 'bob@example.com' },
124+
],
125+
},
126+
{
127+
object: 'my_app_interview',
128+
externalId: 'name',
129+
mode: 'upsert',
130+
env: ['prod', 'dev', 'test'],
131+
records: [
132+
{ name: 'iv-alice-1', candidate: 'alice@example.com', stage: 'screening' },
133+
{ name: 'iv-bob-1', candidate: 'bob@example.com', stage: 'onsite' },
134+
],
135+
},
136+
] as any[];
137+
138+
/** No interview update may ever carry a NULL or unresolved (email-string) candidate. */
139+
function corruptInterviewUpdates(engine: IDataEngine) {
140+
return (engine.update as any).mock.calls.filter(
141+
([obj, data]: [string, any]) =>
142+
obj === 'my_app_interview' &&
143+
'candidate' in data &&
144+
(data.candidate == null || String(data.candidate).includes('@')),
145+
);
146+
}
147+
148+
describe('seed replay (restart) — lookup natural keys on the update path', () => {
149+
it('keeps lookups pointing at the right parent after a second load (no NULL overwrite)', async () => {
150+
const { engine, store } = createFaithfulEngine();
151+
const metadata = createMetadata();
152+
153+
// Boot #1 — fresh DB, insert path.
154+
const first = await new SeedLoaderService(engine, metadata, createLogger()).load({
155+
seeds: SEEDS,
156+
config: CONFIG,
157+
});
158+
expect(first.success).toBe(true);
159+
160+
const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id;
161+
const bobId = store.my_app_candidate.find((r) => r.email === 'bob@example.com')!.id;
162+
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId);
163+
expect(store.my_app_interview.find((r) => r.name === 'iv-bob-1')!.candidate).toBe(bobId);
164+
165+
// Boot #2 — same DB, new loader instance (dev-server restart): upsert
166+
// matches existing rows and takes the UPDATE path.
167+
const second = await new SeedLoaderService(engine, metadata, createLogger()).load({
168+
seeds: SEEDS,
169+
config: CONFIG,
170+
});
171+
172+
// The replay must not corrupt the association: still the right parent id,
173+
// never NULL (a nullable column would silently lose the link; a NOT NULL
174+
// column turns every replayed record into a constraint error).
175+
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId);
176+
expect(store.my_app_interview.find((r) => r.name === 'iv-bob-1')!.candidate).toBe(bobId);
177+
expect(corruptInterviewUpdates(engine)).toEqual([]);
178+
179+
expect(second.success).toBe(true);
180+
expect(second.summary.totalErrored).toBe(0);
181+
});
182+
183+
it('replaying an unchanged seed is a no-op (skip, no update churn)', async () => {
184+
const { engine, store } = createFaithfulEngine();
185+
const metadata = createMetadata();
186+
187+
await new SeedLoaderService(engine, metadata, createLogger()).load({
188+
seeds: SEEDS,
189+
config: CONFIG,
190+
});
191+
(engine.update as any).mockClear();
192+
const before = structuredClone(store);
193+
194+
const replay = await new SeedLoaderService(engine, metadata, createLogger()).load({
195+
seeds: SEEDS,
196+
config: CONFIG,
197+
});
198+
199+
// Nothing the seed declares changed → no update at all: updated_at stays
200+
// put, lifecycle validation (e.g. a state_machine rule) never re-fires.
201+
expect(engine.update).not.toHaveBeenCalled();
202+
expect(replay.summary.totalUpdated).toBe(0);
203+
expect(replay.summary.totalSkipped).toBe(4);
204+
expect(replay.summary.totalErrored).toBe(0);
205+
expect(store).toEqual(before);
206+
});
207+
208+
it('a rejected parent update (state-machine style) must not cascade into NULLed child lookups', async () => {
209+
const { engine, store } = createFaithfulEngine();
210+
const metadata = createMetadata();
211+
212+
await new SeedLoaderService(engine, metadata, createLogger()).load({
213+
seeds: SEEDS,
214+
config: CONFIG,
215+
});
216+
const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id;
217+
const bobId = store.my_app_candidate.find((r) => r.email === 'bob@example.com')!.id;
218+
219+
// A user edited rows at runtime (the eval scenario: kanban drags changed
220+
// candidate stages, someone rescheduled an interview) — so the replay's
221+
// updates are NOT no-ops — and the object now vetoes the transition back
222+
// (my-app's `Invalid stage transition.`).
223+
for (const r of store.my_app_candidate) r.name = r.name + ' (edited)';
224+
for (const r of store.my_app_interview) r.stage = 'edited';
225+
const realUpdate = (engine.update as any).getMockImplementation();
226+
(engine.update as any).mockImplementation(async (objectName: string, data: any) => {
227+
if (objectName === 'my_app_candidate') throw new Error('Invalid stage transition.');
228+
return realUpdate(objectName, data);
229+
});
230+
231+
const replay = await new SeedLoaderService(engine, metadata, createLogger()).load({
232+
seeds: SEEDS,
233+
config: CONFIG,
234+
});
235+
236+
// The candidate updates legitimately failed (reported), but the interview
237+
// records still resolved their parents and re-applied the seed values.
238+
expect(replay.summary.totalErrored).toBe(2);
239+
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId);
240+
expect(store.my_app_interview.find((r) => r.name === 'iv-bob-1')!.candidate).toBe(bobId);
241+
expect(store.my_app_interview.every((r) => r.stage !== 'edited')).toBe(true);
242+
expect(corruptInterviewUpdates(engine)).toEqual([]);
243+
});
244+
245+
it('resolves a lookup from the DB by the target dataset externalId when the parent dataset is absent', async () => {
246+
const { engine, store } = createFaithfulEngine();
247+
const metadata = createMetadata();
248+
249+
// Parents already in the DB (seeded earlier / another load).
250+
await new SeedLoaderService(engine, metadata, createLogger()).load({
251+
seeds: SEEDS,
252+
config: CONFIG,
253+
});
254+
const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id;
255+
256+
// A later load carries ONLY the child dataset, so the in-memory
257+
// insertedRecords map has no candidate entries — resolution must fall
258+
// back to the DB and query the candidate dataset's externalId (email),
259+
// not the hardcoded 'name' column.
260+
const childOnly = await new SeedLoaderService(engine, metadata, createLogger()).load({
261+
seeds: [
262+
{ ...SEEDS[0] }, // candidate dataset present but EMPTY → only its externalId declaration matters
263+
{
264+
object: 'my_app_interview',
265+
externalId: 'name',
266+
mode: 'upsert',
267+
env: ['prod', 'dev', 'test'],
268+
records: [{ name: 'iv-alice-2', candidate: 'alice@example.com', stage: 'onsite' }],
269+
},
270+
].map((s, idx) => (idx === 0 ? { ...s, records: [] } : s)) as any,
271+
config: CONFIG,
272+
});
273+
274+
expect(childOnly.summary.totalErrored).toBe(0);
275+
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-2')!.candidate).toBe(aliceId);
276+
});
277+
278+
it('an unresolvable lookup never reaches the row: deferred pass leaves the column alone, single-pass drops the record', async () => {
279+
const { engine, store } = createFaithfulEngine();
280+
const metadata = createMetadata();
281+
282+
await new SeedLoaderService(engine, metadata, createLogger()).load({
283+
seeds: SEEDS,
284+
config: CONFIG,
285+
});
286+
const aliceId = store.my_app_candidate.find((r) => r.email === 'alice@example.com')!.id;
287+
288+
// Poison the seed: alice's interview now references a candidate that does
289+
// not exist anywhere. Change another field so the update is not a no-op.
290+
const poisoned = structuredClone(SEEDS);
291+
poisoned[1].records[0].candidate = 'ghost@example.com';
292+
poisoned[1].records[0].stage = 'changed';
293+
294+
// multiPass: the update must NOT touch the candidate column (pass 2 also
295+
// fails → reported), preserving the existing association.
296+
const deferred = await new SeedLoaderService(engine, metadata, createLogger()).load({
297+
seeds: poisoned,
298+
config: CONFIG,
299+
});
300+
expect(deferred.success).toBe(false);
301+
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId);
302+
expect(corruptInterviewUpdates(engine)).toEqual([]);
303+
304+
// single-pass: the whole record is dropped (reported + counted), rather
305+
// than written with a corrupted reference.
306+
(engine.update as any).mockClear();
307+
const singlePass = await new SeedLoaderService(engine, metadata, createLogger()).load({
308+
seeds: poisoned,
309+
config: { ...CONFIG, multiPass: false },
310+
});
311+
expect(singlePass.success).toBe(false);
312+
expect(singlePass.summary.totalErrored).toBeGreaterThanOrEqual(1);
313+
expect(
314+
(engine.update as any).mock.calls.filter(([obj, data]: [string, any]) => obj === 'my_app_interview' && data.name === 'iv-alice-1'),
315+
).toEqual([]);
316+
expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId);
317+
});
318+
});

0 commit comments

Comments
 (0)