From 09b324c5b5f2559c9aa7b236851f0f4affb92717 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 15:08:38 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): seed replay compares temporal fields in their stored form isNoOpReplay compared a seed's time / datetime / date value with the driver's stored spelling literally, so '09:15' against stored '09:15:00' and '...T14:30:00Z' against '...T14:30:00.000Z' read as changed and the row was rewritten on every boot (updated_at bumped, lifecycle validation re-run, a phantom history edit). Both sides of a field the object declares temporal now go through temporalStorageForm, the one rule every driver applies on write; a value that rule cannot read still reads as changed. Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude --- .../src/seed-loader-replay.test.ts | 169 ++++++++++++++++++ packages/metadata-protocol/src/seed-loader.ts | 80 +++++++-- 2 files changed, 234 insertions(+), 15 deletions(-) diff --git a/packages/metadata-protocol/src/seed-loader-replay.test.ts b/packages/metadata-protocol/src/seed-loader-replay.test.ts index dcae11e8dc3..74908e16186 100644 --- a/packages/metadata-protocol/src/seed-loader-replay.test.ts +++ b/packages/metadata-protocol/src/seed-loader-replay.test.ts @@ -4,6 +4,7 @@ import { describe, it, expect, vi } from 'vitest'; import { SeedLoaderService } from './seed-loader.js'; import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { temporalComparandKind, temporalStorageForm } from '@objectstack/core'; /** * Replay regression: seeds with lookup natural keys must survive a dev-server @@ -322,3 +323,171 @@ describe('seed replay (restart) — lookup natural keys on the update path', () expect(store.my_app_interview.find((r) => r.name === 'iv-alice-1')!.candidate).toBe(aliceId); }); }); + +/** + * [#24562] A `time` / `datetime` / `date` value the seed spells short, replayed + * over the row a driver stored for it. + * + * The faithful engine above stores exactly what it is handed; a real driver + * does not. `SqlDriver` and `InMemoryDriver` both write a temporal field + * through `temporalStorageForm` (`@objectstack/core`, ADR-0053), so measured + * on a SQLite file DB `'09:15'` lands as `'09:15:00'`, + * `'2026-06-17T14:30:00Z'` as `'2026-06-17T14:30:00.000Z'`, and a `date` + * written as an instant as its `YYYY-MM-DD`. The engine below does the same, + * and stamps `updated_at` on every write the way the audit stamp does, so a + * replay that rewrites an unchanged row shows as a moved stamp. + * + * Both write paths compare through `isNoOpReplay`: the batched + * `decideWriteAction`, and the sequential `writeRecord` a self-referencing + * object takes (`hasSelfRef`) — each case below asserts which one ran. + */ +describe('seed replay — a temporal value spelled short is the value the driver stored', () => { + const ENTRY_FIELDS = { + name: { type: 'text' }, + starts_at: { type: 'time' }, + logged_at: { type: 'datetime' }, + worked_on: { type: 'date' }, + }; + const ENTRY = { name: 'tz_replay_entry', fields: ENTRY_FIELDS }; + const TREE_ENTRY = { + name: 'tz_replay_tree_entry', + fields: { ...ENTRY_FIELDS, parent: { type: 'lookup', reference: 'tz_replay_tree_entry' } }, + }; + const PATHS = [ + { path: 'batched decideWriteAction', object: ENTRY, sequential: false }, + { path: 'sequential writeRecord', object: TREE_ENTRY, sequential: true }, + ]; + + const SHORT_SPELLINGS: Array> = [ + { name: 'short-time', starts_at: '09:15' }, + { name: 'short-datetime', logged_at: '2026-06-17T14:30:00Z' }, + { name: 'zone-naive-datetime', logged_at: '2026-06-17 14:30' }, + { name: 'instant-date', worked_on: '2026-06-17T00:00:00Z' }, + // Control: already spelled the way the driver stores it. + { name: 'stored-spelling', starts_at: '09:05:30', logged_at: '2026-06-17T14:30:00.000Z', worked_on: '2026-06-17' }, + ]; + + function createStoringEngine(object: { name: string; fields: Record }) { + const { engine, store } = createFaithfulEngine(); + let clock = 0; + const stored = (row: Record) => { + const out: Record = { ...row, updated_at: `stamp-${++clock}` }; + for (const [key, value] of Object.entries(row)) { + const kind = temporalComparandKind(object.fields[key]?.type); + if (kind && value != null) out[key] = temporalStorageForm(value, kind); + } + return out; + }; + const insert = (engine.insert as any).getMockImplementation(); + (engine.insert as any).mockImplementation(async (o: string, data: any, opts?: any) => + insert(o, Array.isArray(data) ? data.map(stored) : stored(data), opts)); + const update = (engine.update as any).getMockImplementation(); + (engine.update as any).mockImplementation(async (o: string, data: any, opts?: any) => update(o, stored(data), opts)); + return { engine, store }; + } + + function metadataOf(object: { name: string }): IMetadataService { + return { + getObject: vi.fn(async (name: string) => (name === object.name ? object : undefined)), + listObjects: vi.fn(async () => [object]), + register: vi.fn(async () => {}), + get: vi.fn(async (_t: string, name: string) => (name === object.name ? object : undefined)), + list: vi.fn(async () => []), + unregister: vi.fn(async () => {}), + exists: vi.fn(async () => false), + listNames: vi.fn(async () => []), + } as unknown as IMetadataService; + } + + async function load( + engine: IDataEngine, + object: { name: string }, + mode: string, + records: Array>, + ) { + const writeRecord = vi.spyOn(SeedLoaderService.prototype as any, 'writeRecord'); + try { + const result = await new SeedLoaderService(engine, metadataOf(object), createLogger()).load({ + seeds: [{ object: object.name, externalId: 'name', mode, env: ['prod', 'dev', 'test'], records }] as any, + config: CONFIG, + }); + return { result, writeRecordCalls: writeRecord.mock.calls.length }; + } finally { + writeRecord.mockRestore(); + } + } + + const rowOf = (store: Record, object: { name: string }, name: string) => + store[object.name].find((r) => r.name === name)!; + const updatedNames = (engine: IDataEngine, store: Record, object: { name: string }) => + (engine.update as any).mock.calls + .map(([, data]: [string, any]) => store[object.name].find((r) => r.id === data.id)?.name) + .sort(); + + it.each(PATHS.flatMap((p) => ['upsert', 'update'].map((mode) => ({ ...p, mode }))))( + '$path, mode $mode: replaying the unchanged seed issues no update and moves no updated_at', + async ({ object, sequential, mode }) => { + const { engine, store } = createStoringEngine(object); + const first = await load(engine, object, 'upsert', SHORT_SPELLINGS); + expect(first.result.summary.totalInserted).toBe(5); + // The premise: what the column holds is the driver's spelling, not the seed's. + expect(rowOf(store, object, 'short-time').starts_at).toBe('09:15:00'); + expect(rowOf(store, object, 'short-datetime').logged_at).toBe('2026-06-17T14:30:00.000Z'); + expect(rowOf(store, object, 'zone-naive-datetime').logged_at).toBe('2026-06-17T14:30:00.000Z'); + expect(rowOf(store, object, 'instant-date').worked_on).toBe('2026-06-17'); + (engine.update as any).mockClear(); + const before = structuredClone(store); + + const replay = await load(engine, object, mode, SHORT_SPELLINGS); + + expect(replay.writeRecordCalls > 0).toBe(sequential); + expect(engine.update).not.toHaveBeenCalled(); + expect(replay.result.summary.totalUpdated).toBe(0); + expect(replay.result.summary.totalSkipped).toBe(5); + expect(replay.result.summary.totalErrored).toBe(0); + expect(store).toEqual(before); + }, + ); + + it.each(PATHS)('$path: a changed wall clock, instant or day is still rewritten, and only its row', async ({ object, sequential }) => { + const { engine, store } = createStoringEngine(object); + await load(engine, object, 'upsert', SHORT_SPELLINGS); + (engine.update as any).mockClear(); + const before = structuredClone(store); + const changed = SHORT_SPELLINGS.map((r) => + r.name === 'short-time' ? { ...r, starts_at: '09:16' } + : r.name === 'short-datetime' ? { ...r, logged_at: '2026-06-17T14:30:01Z' } + : r.name === 'instant-date' ? { ...r, worked_on: '2026-06-18T00:00:00Z' } + : r); + + const replay = await load(engine, object, 'upsert', changed); + + expect(replay.writeRecordCalls > 0).toBe(sequential); + expect(updatedNames(engine, store, object)).toEqual(['instant-date', 'short-datetime', 'short-time']); + expect(replay.result.summary.totalUpdated).toBe(3); + expect(replay.result.summary.totalSkipped).toBe(2); + expect(rowOf(store, object, 'short-time').starts_at).toBe('09:16:00'); + expect(rowOf(store, object, 'short-datetime').logged_at).toBe('2026-06-17T14:30:01.000Z'); + expect(rowOf(store, object, 'instant-date').worked_on).toBe('2026-06-18'); + for (const name of ['zone-naive-datetime', 'stored-spelling']) { + expect(rowOf(store, object, name)).toEqual(before[object.name].find((r) => r.name === name)); + } + }); + + it.each(PATHS)('$path: a value the storage rule cannot read still reads as changed', async ({ object, sequential }) => { + const { engine, store } = createStoringEngine(object); + await load(engine, object, 'upsert', SHORT_SPELLINGS); + (engine.update as any).mockClear(); + const unreadable = SHORT_SPELLINGS.map((r) => + r.name === 'short-time' ? { ...r, starts_at: 'quarter past nine' } + : r.name === 'short-datetime' ? { ...r, logged_at: 'not-a-date' } + : r); + + const replay = await load(engine, object, 'upsert', unreadable); + + expect(replay.writeRecordCalls > 0).toBe(sequential); + expect(updatedNames(engine, store, object)).toEqual(['short-datetime', 'short-time']); + expect(replay.result.summary.totalUpdated).toBe(2); + expect(replay.result.summary.totalSkipped).toBe(3); + }); +}); diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index 2751cbefed8..ee3eb38f87d 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -25,7 +25,7 @@ import { SEED_WRITE_EXECUTION_CONTEXT } from '@objectstack/spec/kernel'; import { resolveSeedRecord } from '@objectstack/formula'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult, runWithAdvisoryAggregation, type AdvisoryGroup, - temporalComparandKind, temporalStorageForm, isMissing, + temporalComparandKind, temporalStorageForm, type TemporalComparandKind, isMissing, } from '@objectstack/core'; // [#8442] The repo's ONE recogniser for "this throw is a record-validation // failure" — duck-typed on `code`/`name`, the same predicate `mapDataError` and @@ -476,6 +476,24 @@ function seedValuesInStoredForm(record: Record, definition: unk } } +/** + * [#24562] Field name → storage kind, for every field the definition declares + * `datetime`, `date` or `time` — the fields a replay compares in their STORED + * form (see {@link SeedLoaderService.isNoOpReplay}). Same field-type reading + * as {@link seedValuesInStoredForm}; a definition the loader cannot resolve + * declares none, and its fields keep the type-blind comparison. + */ +function temporalFieldKinds(definition: unknown): ReadonlyMap { + const kinds = new Map(); + const fields = (definition as { fields?: Record } | null | undefined)?.fields; + if (!fields || typeof fields !== 'object') return kinds; + for (const [name, field] of Object.entries(fields)) { + const kind = temporalComparandKind(field?.type); + if (kind) kinds.set(name, kind); + } + return kinds; +} + export class SeedLoaderService implements ISeedLoaderService { private engine: IDataEngine; private metadata: IMetadataService; @@ -1222,6 +1240,8 @@ export class SeedLoaderService implements ISeedLoaderService { // judge, and nothing here refuses a row for an owner it cannot prove. const objectDefinition = await this.resolveObjectDefinition(objectName); const rowsCarryOrganization = objectDefinition == null || seedRowNeedsOrganization(objectDefinition); + // [#24562] What a replay compares in the stored form (`isNoOpReplay`). + const temporalKinds = temporalFieldKinds(objectDefinition); // [ADR-0131 D9] With no organization pinned and none derivable on an install // that registers the organization object, a row of an organization-owned // object has no owner: it is REFUSED below, never written NULL. @@ -1697,7 +1717,7 @@ export class SeedLoaderService implements ISeedLoaderService { // it, and which organization an unowned row belongs to is the // organization-ownership ceremony's attribution, never the loader's. if (unownedHolders && unownedHolders.size > 0) { - const decision = this.decideWriteAction(record, mode, externalId, existingRecords, unownedHolders); + const decision = this.decideWriteAction(record, mode, externalId, existingRecords, temporalKinds, unownedHolders); if (decision.action === 'refuse') { errored++; const label = this.externalIdLabel(externalId); @@ -1751,7 +1771,7 @@ export class SeedLoaderService implements ISeedLoaderService { // verbatim — this line only moves WHEN the author learns, from after the // engine's rejection to before the write that provokes it. if (!config.dryRun && requiredDeferrals.length > 0) { - if (this.decideWriteAction(record, mode, externalId, existingRecords).action === 'insert') { + if (this.decideWriteAction(record, mode, externalId, existingRecords, temporalKinds).action === 'insert') { for (const deferral of requiredDeferrals) { if (earlySignalledFields.has(deferral.field)) continue; earlySignalledFields.add(deferral.field); @@ -1782,7 +1802,7 @@ export class SeedLoaderService implements ISeedLoaderService { // against an earlier one via `insertedRecords` — see `hasSelfRef`. try { const result = await this.writeRecord( - objectName, record, mode, externalId, existingRecords + objectName, record, mode, externalId, existingRecords, temporalKinds ); if (result.action === 'inserted') inserted++; @@ -1820,7 +1840,7 @@ export class SeedLoaderService implements ISeedLoaderService { ); } } else { - const decision = this.decideWriteAction(record, mode, externalId, existingRecords); + const decision = this.decideWriteAction(record, mode, externalId, existingRecords, temporalKinds); const externalIdValue = this.externalIdKey(record, externalId); if (decision.action === 'skip') { @@ -2620,7 +2640,8 @@ export class SeedLoaderService implements ISeedLoaderService { record: Record, mode: string, externalId: string | string[], - existingRecords?: Map, + existingRecords: Map | undefined, + temporalKinds: ReadonlyMap, ): Promise<{ action: 'inserted' | 'updated' | 'skipped'; id?: string }> { const existing = existingRecords?.get(this.externalIdKey(record, externalId)); const opts = SeedLoaderService.SEED_OPTIONS as any; @@ -2636,7 +2657,7 @@ export class SeedLoaderService implements ISeedLoaderService { return { action: 'skipped' }; } const id = this.extractId(existing); - if (this.isNoOpReplay(record, existing)) { + if (this.isNoOpReplay(record, existing, temporalKinds)) { return { action: 'skipped', id }; } await this.writeRecoveringSummary(objectName, () => withTransientRetry(() => this.engine.update(objectName, { ...record, id }, opts))); @@ -2646,7 +2667,7 @@ export class SeedLoaderService implements ISeedLoaderService { case 'upsert': { if (existing) { const id = this.extractId(existing); - if (this.isNoOpReplay(record, existing)) { + if (this.isNoOpReplay(record, existing, temporalKinds)) { return { action: 'skipped', id }; } await this.writeRecoveringSummary(objectName, () => withTransientRetry(() => this.engine.update(objectName, { ...record, id }, opts))); @@ -2697,7 +2718,8 @@ export class SeedLoaderService implements ISeedLoaderService { record: Record, mode: string, externalId: string | string[], - existingRecords?: Map, + existingRecords: Map | undefined, + temporalKinds: ReadonlyMap, unownedHolders?: Map, ): | { action: 'insert' } @@ -2712,12 +2734,12 @@ export class SeedLoaderService implements ISeedLoaderService { switch (mode) { case 'update': if (!existing) return { action: 'skip' }; - return this.isNoOpReplay(record, existing) + return this.isNoOpReplay(record, existing, temporalKinds) ? { action: 'skip', id: this.extractId(existing) } : { action: 'update', id: this.extractId(existing)! }; case 'upsert': if (!existing) return { action: 'insert' }; - return this.isNoOpReplay(record, existing) + return this.isNoOpReplay(record, existing, temporalKinds) ? { action: 'skip', id: this.extractId(existing) } : { action: 'update', id: this.extractId(existing)! }; case 'ignore': @@ -2741,19 +2763,47 @@ export class SeedLoaderService implements ISeedLoaderService { * pin — never block the skip). Comparison is conservative: any doubt * (unparseable dates, type mismatches) reads as "changed", falling back to * the historical update behavior. + * + * [#24562] A field the object declares `datetime`, `date` or `time` + * (`temporalKinds`, from {@link temporalFieldKinds}) is compared in its + * STORED form: both sides go through `temporalStorageForm`, the one rule + * every driver applies on write (ADR-0053). The question a replay asks is + * whether writing the seed value would change what the column holds, and a + * driver stores `'09:15'` as `'09:15:00'` and `'…T14:30:00Z'` as + * `'…T14:30:00.000Z'` — so two spellings of one instant, one calendar day or + * one wall clock are the same stored value and the row is skipped. Read + * literally, they rewrote the row on every boot. */ - private isNoOpReplay(record: Record, existing: Record): boolean { + private isNoOpReplay( + record: Record, + existing: Record, + temporalKinds: ReadonlyMap, + ): boolean { for (const [key, value] of Object.entries(record)) { if (key === 'id') continue; - if (!this.seedValueEquals(value, existing[key])) return false; + if (!this.seedValueEquals(value, existing[key], temporalKinds.get(key))) return false; } return true; } - /** Loose equality across driver round-trip representations (see isNoOpReplay). */ - private seedValueEquals(a: unknown, b: unknown): boolean { + /** + * Loose equality across driver round-trip representations (see isNoOpReplay). + * + * `kind` is the field's declared temporal kind, when it has one: both sides + * are then compared in the storage form of that kind, and equal only when + * that form is the same TEXT. `temporalStorageForm` hands a value it cannot + * read back unchanged (it never invents one), so an unparseable value keeps + * reading as changed against anything but its own identical spelling — the + * "any doubt reads as changed" rule — and a non-text result (an Invalid + * Date, a structure) is never equal. + */ + private seedValueEquals(a: unknown, b: unknown, kind?: TemporalComparandKind): boolean { if (a === b) return true; if (a == null || b == null) return a == null && b == null; + if (kind) { + const stored = temporalStorageForm(a, kind); + return typeof stored === 'string' && stored === temporalStorageForm(b, kind); + } // 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))); From f2d64ae4b317d8f9315c0a0f820ed4eb22f8de8e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 15:11:03 +0000 Subject: [PATCH 2/2] chore(changeset): metadata-protocol patch for the temporal seed replay comparison Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude --- .changeset/24562-seed-replay-time-equality.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/24562-seed-replay-time-equality.md diff --git a/.changeset/24562-seed-replay-time-equality.md b/.changeset/24562-seed-replay-time-equality.md new file mode 100644 index 00000000000..3e7aa501dd4 --- /dev/null +++ b/.changeset/24562-seed-replay-time-equality.md @@ -0,0 +1,11 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): a seed replay no longer rewrites an unchanged row whose `time`, `datetime` or `date` value the seed spells differently from the stored form (#24562) + +Clause-②: no + +The replay's no-op check (`isNoOpReplay`) compared a seed value with the stored one as text, so `'09:15'` against the stored `'09:15:00'`, `'2026-06-17T14:30:00Z'` against `'2026-06-17T14:30:00.000Z'`, and a `date` written as an instant against its stored `YYYY-MM-DD` all read as changed. Every boot rewrote those rows: `updated_at` moved, lifecycle validation ran again, history tracking recorded an edit nobody made, and a parent's summary recompute moved the parent's `updated_at` too. + +A field the object declares `time`, `datetime` or `date` is now compared in its stored form, by the rule the drivers apply on write (`temporalStorageForm`, `@objectstack/core`). Two spellings of the same wall clock, instant or calendar day skip the row. A different value is still rewritten, and a value that rule cannot read still counts as changed. What a seed may declare, and what any write stores, is unchanged.