From ccafecddc667dc4b479cca5023e293c7f295f4e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 06:52:41 +0000 Subject: [PATCH] fix(objectql,platform-objects): attest a fresh datastore on this boot's own data, not on the emptiness it remembers (#4769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A store created from empty recorded both ADR-0104 migrations as verified at `kernel:ready`, while the same boot was still seeding rows whose values contradict them. The certificate was true when written and false a second later: the first boot ran warn-first and kept the data, every later boot read the certificate, enforced it, and rejected the rows its predecessor had written. Same data, same code, one restart. Three changes, one invariant — a boot may not prove a contract it violates in that same boot: - The attestation waits for `app:seeded` (the inline seed's settle point, background continuation included), with `kernel:ready` as the backstop for kernels that never seed. Both enter the same idempotent call. - `attestFreshDatastore` consults the engine's tally of ADMITTED value-shape violations first and declines any migration id this boot has already contradicted, naming the object.field and the command that closes the gate. The two ids are judged independently — a bad `cover` does not sink the `location` gate. The engine records those admissions from the warn-first path with the exact predicate strict mode uses, so certifying still needs a scan while refuting needs the one counterexample the write already computed. - A certificate contradicted AFTER it was issued is revoked from the write path that contradicted it, scoped to creation attestations on a store this boot created; evidence produced by a real migration run is never rewritten. Also fixes the memoized flag read, the other half of why the first boot looked green: "the ledger says no" is now distinguished from "the ledger could not be asked" (sys_migration not registered yet, or the query threw). Both keep the gate closed, only the former is remembered, so one unlucky early write no longer freezes a whole boot's posture. The `kernel:bootstrapped` advisory reads the ledger directly for the same reason. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny --- .../adr0104-attest-after-boot-writes.md | 58 +++ .../src/adr0104-attestation-evidence.test.ts | 358 ++++++++++++++++++ packages/objectql/src/engine.ts | 304 +++++++++++++-- packages/objectql/src/index.ts | 9 + .../src/validation/record-validator.ts | 66 +++- packages/platform-objects/src/plugin.test.ts | 79 ++++ packages/platform-objects/src/plugin.ts | 32 +- .../src/system/migration-flag.test.ts | 68 ++++ .../src/system/migration-flag.ts | 68 +++- 9 files changed, 1005 insertions(+), 37 deletions(-) create mode 100644 .changeset/adr0104-attest-after-boot-writes.md create mode 100644 packages/objectql/src/adr0104-attestation-evidence.test.ts diff --git a/.changeset/adr0104-attest-after-boot-writes.md b/.changeset/adr0104-attest-after-boot-writes.md new file mode 100644 index 0000000000..21f9981ffa --- /dev/null +++ b/.changeset/adr0104-attest-after-boot-writes.md @@ -0,0 +1,58 @@ +--- +"@objectstack/objectql": patch +"@objectstack/platform-objects": patch +--- + +fix(objectql,platform-objects): 一次启动不能证明它自己随即违反的契约 —— ADR-0104 空库自证改为在本次启动写完数据后下结论 (#4769) + +一个全新部署第一次 `pnpm dev` 全绿(130 rows,0 ERROR),**第二次启动开始永久 10 条 +ERROR**、10 条种子记录写不进去。数据没变、代码没变,只是重启了一次;被拒的正是首启 +自己写进去的数据。 + +根因不是哪个值算错了,是**顺序反了**。`sys_migration` 里那两行 +(`adr-0104-file-references` / `adr-0104-value-shapes`)带着 +`{"attested":"datastore-created-empty"}` 写在 `kernel:ready`,而同一次启动的 seed +还在往里写行。「空库 ⇒ 没有历史值」这个推理成立的前提是**没有数据可写**,而它恰恰 +写在即将写入 130 行之前 —— 证明落笔那一刻是真的,一秒之后就不是了。于是首启在 +warn-first 下把数据留下,之后每一次启动读到这张证书、进入 strict、拒掉前任写下的 +那批行。 + +## 改了什么 + +**证书必须覆盖它所声称的那批数据。** + +- **写入时机**:新库自证改为在**本次启动自己的数据落定之后**进行 —— + `app:seeded`(inline seed 结算点,含超出 `OS_INLINE_SEED_BUDGET_MS` 后台跑完的 + 那一半),不 seed 的 kernel 仍由 `kernel:ready` 兜底。两条路径进的是同一个幂等 + 调用。 +- **写入前提**:`attestFreshDatastore` 先问引擎「这次启动放行过违反该契约的值吗」。 + 引擎在 warn-first 放行每一个不合形状的值时,用**与 strict 模式完全相同的判定**把 + 它记下来 —— 证明干净需要扫全库,证伪只需要一个反例,而这个反例写路径已经算出来 + 了。任一条被本次启动证伪的迁移 id **不再自证**,部署维持 warn-first(真实且可 + 恢复),并在日志里指名是哪个 `对象.字段` 让这道闸没关上、该跑哪条 `os migrate`。 + 两行一起改:`adr-0104-file-references` 与 `adr-0104-value-shapes` 各自独立判定, + 一个 `cover` 不合形状不牵连 `location`,反之亦然。 +- **写入之后**:证书若在签发之后被本次启动推翻(操作员显式开了 + `OS_ALLOW_LAX_MEDIA_VALUES` / `OS_ALLOW_LAX_VALUE_SHAPES`,或后台 seed 收尾晚于 + 签发),引擎**撤销**它 —— `verified_at` 清空、`blocking` 记上、`details` 保留原 + `attested` 并补一条 `revoked`。只针对**本次启动亲手创建的库**上的自证行:扫过全 + 库的真实迁移证据不会被一次写入的观察推翻。 + +**记忆化的第二张脸也一并修了。** 首启之所以「看起来是绿的」,一半靠的是进程内正好 +缓存了 `false`。`sys_migration` 在 kernel init 期间才注册,而第一条写可能赶在它之 +前 —— 那次读根本没读到账本,却被当成结论冻结了一整个进程的姿态。现在区分两种否定: +**问过了、账本说不**(结论,照旧缓存)与**根本问不到**(未注册 / 查询抛错 —— 依旧 +答 `false`,闸依旧关着,但不记住,下一次写再问一次)。代价是账本存在之前每次写多一 +次 registry 查表(在任何查询之前就短路),账本可读之后即止。 + +启动横幅那条 ADR-0104 建议行(`kernel:bootstrapped`)也改为直接读账本而非读记忆化 +结果 —— 否则一个刚刚自证成功的新部署会被告知去跑一条已经不需要跑的迁移。 + +## 对既有部署的影响 + +- 数据本来就合规的新部署:行为不变,照旧 born-migrated,启动即 strict。 +- 种子数据不合规的新部署:**不再**发出那张假证书。首启与之后每一次启动一致地停在 + warn-first,并且每次都告诉你是哪一个值、跑哪条命令。数据本身该怎么修还是怎么修 + (showcase 的 `cover` 种子值在 #4774 单独跟踪)。 +- 已经跑过 `os migrate … --apply` 的部署:完全不受影响 —— 扫描得来的证据不经由本 + 次改动的任何路径改写。 diff --git a/packages/objectql/src/adr0104-attestation-evidence.test.ts b/packages/objectql/src/adr0104-attestation-evidence.test.ts new file mode 100644 index 0000000000..4e0832f90c --- /dev/null +++ b/packages/objectql/src/adr0104-attestation-evidence.test.ts @@ -0,0 +1,358 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0104 / #4769 — a boot may not prove a contract it violates in that same + * boot. + * + * The defect these pin was not a wrong value anywhere; it was a wrong ORDER. + * A store created from empty was recorded as `verified` for both ADR-0104 + * migrations, because emptiness settles both facts — and then the same boot + * seeded rows whose values contradict them. Every assertion in the ledger was + * true at the instant it was written and false a second later. The visible + * result: the FIRST boot ran warn-first and kept the data, every LATER boot + * read the certificate, enforced it, and rejected the very rows its + * predecessor had written. Same data, same code, one restart. + * + * So the timing is the subject, and the tests are written as BOOTS — two + * engines over one store, the second one finding the tables the first created. + * A single-boot test cannot see this defect at all: boot one is green in the + * broken build too. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectQL } from './engine'; +import { + FILE_REFERENCES_MIGRATION_ID, + VALUE_SHAPES_MIGRATION_ID, +} from '@objectstack/spec/system'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +/** One process's view of a datastore that outlives it. */ +type Store = Map>>; + +const newStore = (): Store => new Map(); + +function rowsOf(store: Store, object: string): Array> { + let rows = store.get(object); + if (!rows) { + rows = []; + store.set(object, rows); + } + return rows; +} + +/** + * A driver over a store that survives "restarts", reporting the schema-sync + * stats of the boot that owns it: `created` on the boot that made the tables, + * `existing` on every boot after — which is exactly what + * `wasDatastoreCreatedFromEmpty()` reads. + */ +function makeDriver(store: Store, stats: { created: number; existing: number }): IDataDriver { + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => row[k] === v); + }; + return { + name: 'default', + version: '1.0.0', + async connect() {}, + async disconnect() {}, + getSchemaSyncStats: () => stats, + async find(object: string, ast: any) { + return rowsOf(store, object).filter((r) => matches(r, ast?.where)); + }, + async findOne(object: string, ast: any) { + return rowsOf(store, object).find((r) => matches(r, ast?.where)) ?? null; + }, + async count(object: string) { return rowsOf(store, object).length; }, + async create(object: string, data: any) { + const row = { ...data }; + rowsOf(store, object).push(row); + return row; + }, + async update(object: string, id: string, data: any) { + const rows = rowsOf(store, object); + const idx = rows.findIndex((r) => r.id === id); + if (idx < 0) return null; + rows[idx] = { ...rows[idx], ...data }; + return rows[idx]; + }, + async delete() { return true; }, + async bulkCreate(object: string, docs: any[]) { + for (const d of docs) rowsOf(store, object).push({ ...d }); + return docs; + }, + async syncSchema() {}, + async dropTable() {}, + } as unknown as IDataDriver; +} + +/** The showcase shape that triggered #4769: a media field and a covered one. */ +const TASK = { + name: 'showcase_task', + fields: { + id: { type: 'text' }, + title: { type: 'text' }, + cover: { type: 'image' }, + place: { type: 'location' }, + }, +}; + +/** Enough of `sys_migration` for the flag reader and the revocation write. */ +const FLAG_OBJECT = { + name: 'sys_migration', + fields: { + id: { type: 'text' }, + last_run_at: { type: 'datetime' }, + verified_at: { type: 'datetime' }, + applied_at: { type: 'datetime' }, + blocking: { type: 'number' }, + advisory: { type: 'number' }, + details: { type: 'textarea' }, + }, +}; + +/** Boot a process against `store`. `created` distinguishes boot 1 from boot 2. */ +function boot(store: Store, opts: { created: boolean }): ObjectQL { + const engine = new ObjectQL(); + engine.registerDriver( + makeDriver(store, opts.created ? { created: 2, existing: 0 } : { created: 0, existing: 2 }), + true, + ); + engine.registerApp({ + id: 'showcase_pkg', + name: 'Showcase', + objects: [TASK, FLAG_OBJECT], + } as any); + return engine; +} + +/** What the fresh-datastore attestation writes when it certifies a store. */ +function creationAttestation(id: string): Record { + const now = new Date().toISOString(); + return { + id, + last_run_at: now, + verified_at: now, + applied_at: null, + blocking: 0, + advisory: 0, + details: JSON.stringify({ attested: 'datastore-created-empty' }), + }; +} + +/** The showcase's own seed value: a URL where an opaque `sys_file` id belongs. */ +const OFF_SHAPE_COVER = 'https://cdn.example.com/placeholder-cover.png'; + +describe('ADR-0104 fresh-datastore attestation vs. the boot that seeds (#4769)', () => { + it('records the counterexample a warn-first admission creates, keyed by the migration it disproves', async () => { + const store = newStore(); + const first = boot(store, { created: true }); + + // Warn-first: no flag row, so the value is ADMITTED and now stored. + await expect( + first.insert('showcase_task', { id: 't1', title: 'Ship it', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + + const admitted = first.valueShapeViolationsAdmitted(); + expect(Object.keys(admitted)).toEqual([FILE_REFERENCES_MIGRATION_ID]); + expect(admitted[FILE_REFERENCES_MIGRATION_ID].count).toBe(1); + expect(admitted[FILE_REFERENCES_MIGRATION_ID].first).toMatchObject({ + object: 'showcase_task', + field: 'cover', + type: 'image', + }); + // The media counterexample says nothing about the OTHER gate — two + // migrations, two facts (the file flag never vouched for `location`). + expect(admitted[VALUE_SHAPES_MIGRATION_ID]).toBeUndefined(); + }); + + it('a covered-class admission disproves the value-shape migration, not the file one', async () => { + const store = newStore(); + const first = boot(store, { created: true }); + + // A `location` is `{lat, lng}`; a display string is the classic legacy shape. + await expect( + first.insert('showcase_task', { id: 't2', title: 'Map it', place: '40.7128,-74.0060' }), + ).resolves.toBeDefined(); + + const admitted = first.valueShapeViolationsAdmitted(); + expect(admitted[VALUE_SHAPES_MIGRATION_ID]?.count).toBe(1); + expect(admitted[VALUE_SHAPES_MIGRATION_ID]?.first).toMatchObject({ + object: 'showcase_task', + field: 'place', + }); + expect(admitted[FILE_REFERENCES_MIGRATION_ID]).toBeUndefined(); + }); + + it('a clean boot leaves no counterexample, so nothing stops it certifying', async () => { + const store = newStore(); + const first = boot(store, { created: true }); + + await expect( + first.insert('showcase_task', { id: 't3', title: 'Fine', cover: 'file_01H0000000000000000000' }), + ).resolves.toBeDefined(); + + expect(first.valueShapeViolationsAdmitted()).toEqual({}); + }); + + /** + * THE REGRESSION. The certificate is already in the ledger when the seed + * runs — the order #4769 reported — so declining to write it cannot help. + * The boot that contradicts its own certificate revokes it, and the next + * boot therefore never enforces a contract against data it inherited. + */ + it('two boots, one dataset: the second must not reject what the first wrote', async () => { + const store = newStore(); + + // ── boot 1 ──────────────────────────────────────────────────────── + const first = boot(store, { created: true }); + // The seed starts before the attestation, so this boot's reading of the + // ledger is "nothing recorded" — the state #4769 describes. + await expect( + first.insert('showcase_task', { id: 't0', title: 'First', cover: 'file_01H0000000000000000000' }), + ).resolves.toBeDefined(); + // The attestation lands next (a store created empty is clean, and it is + // — for another few milliseconds). + rowsOf(store, 'sys_migration').push(creationAttestation(FILE_REFERENCES_MIGRATION_ID)); + // Then the seed writes the row that disproves it. Still admitted, because + // this boot read the ledger before the certificate existed. + await expect( + first.insert('showcase_task', { id: 't1', title: 'Ship it', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + + // The ledger stops claiming what the store contradicts. + await vi.waitFor(() => { + const row = rowsOf(store, 'sys_migration').find((r) => r.id === FILE_REFERENCES_MIGRATION_ID); + expect(row?.verified_at).toBeNull(); + expect(row?.blocking).toBeGreaterThan(0); + // The revocation says why, without discarding how the row got there. + const details = JSON.parse(String(row?.details ?? '{}')); + expect(details.attested).toBe('datastore-created-empty'); + expect(details.revoked).toBe('boot-admitted-violating-value'); + }); + + // ── boot 2: same store, tables already there ────────────────────── + const second = boot(store, { created: false }); + expect(second.wasDatastoreCreatedFromEmpty()).toBe(false); + + // The seeder's upsert replays the same dataset. Before the fix this threw + // `Cover Image has an invalid image value: Expected an opaque sys_file id` + // — a deployment rejecting its own data because it had restarted once. + await expect( + second.update('showcase_task', { id: 't1', title: 'Ship it', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + await expect( + second.insert('showcase_task', { id: 't9', title: 'New row', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + }); + + /** + * The control for the test above: enforcement is not disabled, it is + * un-certified. A ledger row that stands — evidence by scan, from a real + * `os migrate … --apply` — still makes boot 2 reject an off-shape value. + */ + it('a certificate the boot never contradicted still enforces on the next boot', async () => { + const store = newStore(); + + const first = boot(store, { created: true }); + await expect( + first.insert('showcase_task', { id: 't0', title: 'First', cover: 'file_01H0000000000000000000' }), + ).resolves.toBeDefined(); + rowsOf(store, 'sys_migration').push(creationAttestation(FILE_REFERENCES_MIGRATION_ID)); + await expect( + first.insert('showcase_task', { id: 't1', title: 'Clean', cover: 'file_01H0000000000000000001' }), + ).resolves.toBeDefined(); + + const row = rowsOf(store, 'sys_migration').find((r) => r.id === FILE_REFERENCES_MIGRATION_ID); + expect(row?.verified_at).not.toBeNull(); + + const second = boot(store, { created: false }); + await expect( + second.insert('showcase_task', { id: 't2', title: 'Bad', cover: OFF_SHAPE_COVER }), + ).rejects.toThrow(/invalid image value/i); + }); + + /** + * A store this boot did NOT create carries history that is not ours to + * vouch for either way: a verified row there is evidence by scan, and a + * single write's observation must not overturn a walk of the whole store. + */ + it('never revokes a flag on a store this boot did not create', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(creationAttestation(FILE_REFERENCES_MIGRATION_ID)); + // Someone else's deployment, still lenient by operator choice. + const engine = boot(store, { created: false }); + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + try { + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Lax', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + // Counted (the fact is true), but the ledger is left alone. + expect(engine.valueShapeViolationsAdmitted()[FILE_REFERENCES_MIGRATION_ID]?.count).toBe(1); + const row = rowsOf(store, 'sys_migration').find((r) => r.id === FILE_REFERENCES_MIGRATION_ID); + expect(row?.verified_at).not.toBeNull(); + } finally { + vi.unstubAllEnvs(); + } + }); +}); + +/** + * #4769 second face — the memoized flag read. + * + * The first boot looked green partly by luck: a write that happened before + * `sys_migration` existed cached "not verified" for the whole process. That is + * not a read of the ledger, it is the absence of one, and freezing it makes a + * boot's posture depend on which write happened first. + */ +describe('migration-flag memoization: a read that never happened is not an answer (#4769)', () => { + /** Registers the business object only — `sys_migration` arrives later. */ + function bootWithoutLedger(store: Store): ObjectQL { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver(store, { created: 0, existing: 2 }), true); + engine.registerApp({ id: 'p', name: 'P', objects: [TASK] } as any); + return engine; + } + + it('re-asks once the ledger exists, instead of freezing the pre-registration answer', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(creationAttestation(FILE_REFERENCES_MIGRATION_ID)); + const engine = bootWithoutLedger(store); + + // The write that beat the platform objects to the boot: no ledger to ask, + // so it is admitted — correct, and not something to remember. + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Early', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + + // `sys_migration` registers (platform-objects' init) — no invalidation + // call, because nothing here knows a stale answer is cached. + engine.registerApp({ id: 'sys', name: 'Sys', objects: [FLAG_OBJECT] } as any); + + await expect( + engine.insert('showcase_task', { id: 't2', title: 'Later', cover: OFF_SHAPE_COVER }), + ).rejects.toThrow(/invalid image value/i); + }); + + it('still reads the ledger once per process per gate when it could actually be read', async () => { + const store = newStore(); + const engine = boot(store, { created: false }); + const driver: any = (engine as any).drivers.get('default'); + const find = vi.spyOn(driver, 'find'); + + await engine.insert('showcase_task', { id: 't1', cover: 'file_01H0000000000000000000' }); + await engine.insert('showcase_task', { id: 't2', cover: 'file_01H0000000000000000001' }); + await engine.insert('showcase_task', { id: 't3', cover: 'file_01H0000000000000000002' }); + + // `showcase_task` declares both a media and a covered field, so both gates + // are live — one read each, never one per write. The conclusive negative + // (the ledger is readable and empty) IS memoized; only "could not ask" is + // re-asked. + const flagReads = find.mock.calls.filter((c: unknown[]) => c[0] === 'sys_migration'); + expect(flagReads).toHaveLength(2); + expect(flagReads.map((c: any) => c[1]?.where?.id).sort()).toEqual( + [FILE_REFERENCES_MIGRATION_ID, VALUE_SHAPES_MIGRATION_ID].sort(), + ); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 3250ba6d0f..380d799b3c 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -83,6 +83,7 @@ import type { Expression } from '@objectstack/spec'; import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec'; import { bindHooksToEngine } from './hook-binder.js'; import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, buildFieldError, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField } from './validation/record-validator.js'; +import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js'; import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; import { applyHaving } from './having-filter.js'; @@ -93,6 +94,28 @@ import { type DanglingReferenceReport, } from './integrity/dangling-reference-audit.js'; +/** + * One read of a `sys_migration` flag row: the verdict, plus whether the ledger + * could actually be consulted to reach it (#4769). Both negatives keep the + * gate closed; only the conclusive one is worth remembering. + */ +interface MigrationFlagRead { + verified: boolean; + conclusive: boolean; +} + +/** + * What this boot has ADMITTED against one ADR-0104 migration's contract — + * the counterexample that forbids attesting it (#4769). `count` is a lower + * bound (every admitted value is counted, but a revocation records the total + * known at the moment it wrote); `first` is the one an operator is shown, + * because a prescription needs a place to start, not a census. + */ +export interface AdmittedValueShapeViolationTally { + count: number; + first: { object: string; field: string; type: string; detail: string }; +} + /** * The lifecycle events the engine actually dispatches via `triggerHooks`. This * is the single source of truth for what a hook can subscribe to — kept in @@ -3060,15 +3083,13 @@ export class ObjectQL implements IObjectQLEngine { * short-circuits before any query. */ async isFileReferencesMigrationVerified(): Promise { - if (!this.fileReferencesMigrationVerified) { - this.fileReferencesMigrationVerified = this.readMigrationFlagVerified( - FILE_REFERENCES_MIGRATION_ID, - '[value-shape] this deployment has verified the file-as-reference migration — ' + - 'media value shapes are enforced and released field files may be collected ' + - '(ADR-0104 / #3617)', - ); - } - return this.fileReferencesMigrationVerified; + return this.readMigrationFlagMemoized( + 'fileReferencesMigrationVerified', + FILE_REFERENCES_MIGRATION_ID, + '[value-shape] this deployment has verified the file-as-reference migration — ' + + 'media value shapes are enforced and released field files may be collected ' + + '(ADR-0104 / #3617)', + ); } /** @@ -3081,14 +3102,53 @@ export class ObjectQL implements IObjectQLEngine { * classes. */ async isValueShapesMigrationVerified(): Promise { - if (!this.valueShapesMigrationVerified) { - this.valueShapesMigrationVerified = this.readMigrationFlagVerified( - VALUE_SHAPES_MIGRATION_ID, - '[value-shape] this deployment has verified the value-shape scan — reference and ' + - 'structured-JSON value shapes are enforced (ADR-0104 / #3438)', - ); - } - return this.valueShapesMigrationVerified; + return this.readMigrationFlagMemoized( + 'valueShapesMigrationVerified', + VALUE_SHAPES_MIGRATION_ID, + '[value-shape] this deployment has verified the value-shape scan — reference and ' + + 'structured-JSON value shapes are enforced (ADR-0104 / #3438)', + ); + } + + /** + * The memoized seam both public flag readers share — and the place where + * "read once per process" is kept from meaning "answer from a read that + * never happened" (#4769). + * + * The two negatives this read can produce are NOT the same fact: + * + * - **Conclusive.** The ledger was reachable and it does not authorise the + * gate (no row, `verified_at` null, blocking findings). Evidence was + * consulted; memoize it. A later in-process migration run announces + * itself through {@link invalidateDataMigrationFlags}. + * - **Inconclusive.** The ledger could not be ASKED — `sys_migration` is + * not registered yet, or the query threw. Both still answer `false` (an + * unaskable gate stays closed), but freezing that for the life of the + * process turns one unlucky early write into a whole boot's posture. It + * is exactly how one boot ends up lax over data the next boot rejects: + * the platform objects register during kernel init while the very first + * write can land before them, so the answer cached is about a moment + * when nothing could have answered at all. + * + * So an inconclusive read is answered but not kept. The retry costs one + * registry lookup per write until the ledger exists (it short-circuits + * before any query), and stops the moment a real read succeeds. + */ + private async readMigrationFlagMemoized( + slot: 'fileReferencesMigrationVerified' | 'valueShapesMigrationVerified', + migrationId: string, + verifiedLog: string, + ): Promise { + const cached = this[slot]; + if (cached) return (await cached).verified; + const pending = this.readMigrationFlagVerified(migrationId, verifiedLog); + this[slot] = pending; + const result = await pending; + // Only a read that actually consulted the ledger may be remembered. Clear + // by identity so a concurrent `invalidateDataMigrationFlags()` (or a + // re-read that already replaced this slot) is not undone here. + if (!result.conclusive && this[slot] === pending) this[slot] = null; + return result.verified; } /** @@ -3098,11 +3158,21 @@ export class ObjectQL implements IObjectQLEngine { * an unreadable table, a malformed row — all `false`. Enforcement derives * from evidence, and absent evidence is not permission. * + * `conclusive` says whether the ledger was actually consulted, so the caller + * can tell "asked, and the answer is no" from "could not ask" — see + * {@link readMigrationFlagMemoized}. It never changes the verdict, only + * whether that verdict is worth remembering. + * * Costs nothing on a kernel without the platform objects: the registry * lookup short-circuits before any query. */ - private async readMigrationFlagVerified(migrationId: string, verifiedLog: string): Promise { - if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return false; + private async readMigrationFlagVerified( + migrationId: string, + verifiedLog?: string, + ): Promise<{ verified: boolean; conclusive: boolean }> { + if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) { + return { verified: false, conclusive: false }; + } try { const rows = await this.find(DATA_MIGRATION_FLAG_OBJECT, { where: { id: migrationId }, @@ -3110,7 +3180,7 @@ export class ObjectQL implements IObjectQLEngine { context: { isSystem: true } as ExecutionContextInput, }); const row: any = rows?.[0]; - if (!row || row.id !== migrationId) return false; + if (!row || row.id !== migrationId) return { verified: false, conclusive: true }; const verified = isDataMigrationFlagVerified({ id: migrationId, last_run_at: String(row.last_run_at ?? ''), @@ -3119,13 +3189,175 @@ export class ObjectQL implements IObjectQLEngine { // coercion lands on NaN, which fails the === 0 test. blocking: typeof row.blocking === 'number' ? row.blocking : Number(row.blocking ?? Number.NaN), }); - if (verified) this.logger.info(verifiedLog); - return verified; + if (verified && verifiedLog) this.logger.info(verifiedLog); + return { verified, conclusive: true }; } catch { - return false; // unreadable evidence → stay lenient + return { verified: false, conclusive: false }; // unreadable evidence → stay lenient, keep asking } } + // ──────────────────────────────────────────────────────────────────── + // The counterexamples this boot has written (ADR-0104 / #4769) + // ──────────────────────────────────────────────────────────────────── + + /** + * Every ADR-0104 value-shape violation this process has ADMITTED, tallied + * per migration id — the evidence that stops a boot certifying a contract it + * has itself broken. + * + * ## The invariant this exists to keep + * + * A boot may not prove a contract it violates in that same boot. The + * fresh-datastore attestation used to do exactly that: a store created from + * empty was recorded as `verified` because emptiness settles both ADR-0104 + * facts — and then the very same boot seeded rows whose values contradict + * them. The certificate was true at the instant it was written and false a + * second later, so the FIRST boot ran warn-first over data it stored and + * every LATER boot read the certificate, enforced, and rejected the data its + * own predecessor had written. Nothing changed but a restart. + * + * ## Why the write path is the right witness + * + * Certifying a deployment clean needs a complete scan; showing it is NOT + * clean needs one counterexample. The write path already computes that + * counterexample with the exact predicate strict mode would use, so this + * tally is free, exact, and impossible to drift from enforcement — the three + * properties a second scan implementation would have had to earn. + * + * Keyed by migration id (not by field class) so the one place that maps a + * value class to the flag that gates it stays here, beside the gates + * themselves. A consumer asks about `adr-0104-file-references` and gets an + * answer about `adr-0104-file-references`. + */ + valueShapeViolationsAdmitted(): Record { + const out: Record = {}; + for (const [migrationId, tally] of this.admittedValueShapeViolations) { + out[migrationId] = { count: tally.count, first: { ...tally.first } }; + } + return out; + } + + /** Per-migration tally of admitted violations; see the accessor above. */ + private readonly admittedValueShapeViolations = new Map(); + /** Ids whose creation attestation this process has already torn up. */ + private readonly retractedCreationAttestations = new Set(); + /** Serializes retraction writes so concurrent violations issue one update. */ + private creationAttestationRetraction: Promise = Promise.resolve(); + + /** + * The sink `validateRecord` reports admitted violations to. Built per write + * (one closure per call, not per row) so the tally can name the object. + */ + private admittedViolationSink(object: string): AdmittedValueShapeViolationSink { + return (violation) => this.noteAdmittedValueShapeViolation(object, violation); + } + + private noteAdmittedValueShapeViolation(object: string, violation: AdmittedValueShapeViolation): void { + const migrationId = + violation.gate === 'media' ? FILE_REFERENCES_MIGRATION_ID : VALUE_SHAPES_MIGRATION_ID; + const existing = this.admittedValueShapeViolations.get(migrationId); + if (existing) { + existing.count += 1; + } else { + this.admittedValueShapeViolations.set(migrationId, { + count: 1, + first: { object, field: violation.field, type: violation.type, detail: violation.detail }, + }); + } + this.retractCreationAttestation(migrationId); + } + + /** + * Tear up a creation attestation this boot has just contradicted (#4769). + * + * The attestation normally never gets written in the first place: it asks + * {@link valueShapeViolationsAdmitted} before recording anything, so a boot + * that seeded a violating value declines to certify. This closes the other + * order — the certificate is already in the ledger and the contradicting + * value lands afterwards, which is reachable whenever the deployment is + * still lenient at that moment (`OS_ALLOW_LAX_MEDIA_VALUES` / + * `OS_ALLOW_LAX_VALUE_SHAPES`, or a seed that finishes in the background + * after its budget). Without this the ledger would keep asserting a fact the + * store contradicts, and the NEXT boot would enforce it against exactly the + * data this one wrote. + * + * Deliberately narrow: + * + * - only on a store THIS boot created from empty, so the only verified row + * that can exist is the attestation this boot issued. Evidence produced by + * a real `os migrate … --apply` run is never rewritten from here — a scan + * that walked the whole store outranks a single write's observation, and + * an operator who opted into leniency did not ask us to revoke their + * migration; + * - never inserts. No row means nothing was certified, and the attestation + * declining on the tally is what keeps it that way; + * - once per migration id per process, and never awaited by the write. The + * recorded `blocking` count is therefore a LOWER bound — which is all the + * gate reads, since any non-zero count closes it. + */ + private retractCreationAttestation(migrationId: string): void { + if (this.retractedCreationAttestations.has(migrationId)) return; + if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return; + if (!this.wasDatastoreCreatedFromEmpty()) return; + this.retractedCreationAttestations.add(migrationId); + this.creationAttestationRetraction = this.creationAttestationRetraction + .then(async () => { + const rows = await this.find(DATA_MIGRATION_FLAG_OBJECT, { + where: { id: migrationId }, + limit: 1, + context: { isSystem: true } as ExecutionContextInput, + }); + const row: any = rows?.[0]; + if (!row || row.id !== migrationId) return; // nothing certified — nothing to revoke + if (row.verified_at == null) return; // gate already closed + const tally = this.admittedValueShapeViolations.get(migrationId); + const now = new Date().toISOString(); + let details: Record = {}; + if (typeof row.details === 'string' && row.details.length > 0) { + try { + const parsed = JSON.parse(row.details); + if (parsed && typeof parsed === 'object') details = parsed as Record; + } catch { + details = { previous_details: row.details }; + } + } + await this.update( + DATA_MIGRATION_FLAG_OBJECT, + { + id: migrationId, + verified_at: null, + blocking: tally?.count ?? 1, + details: JSON.stringify({ + ...details, + revoked: 'boot-admitted-violating-value', + revoked_at: now, + revoked_by: tally?.first, + }), + updated_at: now, + }, + { context: { isSystem: true } as ExecutionContextInput }, + ); + this.invalidateDataMigrationFlags(); + this.logger.warn( + `[value-shape] revoked '${migrationId}': this deployment was recorded as verified at ` + + 'creation, then this boot wrote a value that contradicts it ' + + `(${tally?.first.object}.${tally?.first.field}: ${tally?.first.detail}). ` + + 'The gate is closed again — fix the data, then run `os migrate ' + + (migrationId === FILE_REFERENCES_MIGRATION_ID ? 'files-to-references' : 'value-shapes') + + ' --apply` to re-earn it (ADR-0104 / #4769).', + ); + }) + .catch((err: any) => { + // Bookkeeping must never surface as a write failure. Staying verified + // is the bad direction, so say so loudly rather than silently. + this.logger.warn( + `[value-shape] could not revoke the creation attestation for '${migrationId}' ` + + `(${err?.message ?? err}) — the ledger still claims this deployment is verified ` + + 'while its data contradicts that; run the migration to re-derive it (#4769)', + ); + }); + } + /** * Drop the memoized deployment migration flags so the next write re-reads * them. For a host that runs a data migration in-process and wants its @@ -3178,7 +3410,13 @@ export class ObjectQL implements IObjectQLEngine { if ((media || mediaByEnv) && (covered || coveredByEnv)) break; } - if (media && !(await this.isFileReferencesMigrationVerified())) { + // [#4769] Read the ledger, not the memo. This advisory runs once, at + // `kernel:bootstrapped`, and the fresh-datastore attestation may have + // written its rows moments earlier — after the boot's first write had + // already memoized "not verified". Reporting from that memo tells a + // brand-new deployment to run a migration whose gate is already closed. + // One query per applicable gate, once per boot. + if (media && !(await this.readMigrationFlagVerified(FILE_REFERENCES_MIGRATION_ID)).verified) { this.logger.info( '[value-shape] media values are checked but NOT enforced here, and released files are ' + 'never collected — this deployment has not verified its file migration. Run ' + @@ -3186,7 +3424,7 @@ export class ObjectQL implements IObjectQLEngine { 'to close the gate (ADR-0104 / #3617).', ); } - if (covered && !(await this.isValueShapesMigrationVerified())) { + if (covered && !(await this.readMigrationFlagVerified(VALUE_SHAPES_MIGRATION_ID)).verified) { this.logger.info( '[value-shape] reference and structured-JSON values are checked but NOT enforced here — ' + 'this deployment has not verified its value-shape scan. Run `os migrate value-shapes` ' + @@ -3267,8 +3505,8 @@ export class ObjectQL implements IObjectQLEngine { * lenient — the safe direction. A host that migrates in-process can call * {@link invalidateDataMigrationFlags} instead of waiting for a restart. */ - private fileReferencesMigrationVerified: Promise | null = null; - private valueShapesMigrationVerified: Promise | null = null; + private fileReferencesMigrationVerified: Promise | null = null; + private valueShapesMigrationVerified: Promise | null = null; /** Lazily-built index: child object name → roll-up summary descriptors on * parent objects that aggregate it. Invalidated when packages register. */ @@ -4203,6 +4441,9 @@ export class ObjectQL implements IObjectQLEngine { // a media field, and memoized after the first object that does. const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(schemaForValidation); const valueShapeStrict = await this.valueShapeStrictFor(schemaForValidation); + // [#4769] Where a warn-first admission is recorded, so this boot cannot + // go on to certify a contract it has just written data against. + const onAdmittedValueShapeViolation = this.admittedViolationSink(object); // Locale + translation hooks for the rejection messages (#3957) — // resolved once for the batch, identical for every row. const msgCtx = this.validationMessageContext(object, opCtx.context); @@ -4220,7 +4461,7 @@ export class ObjectQL implements IObjectQLEngine { if (rowErrors[i] !== undefined) continue; try { normalizeMultiValueFields(schemaForValidation, rows[i]); - validateRecord(schemaForValidation, rows[i], 'insert', { mediaValueShapeStrict, valueShapeStrict, messages: msgCtx }); + validateRecord(schemaForValidation, rows[i], 'insert', { mediaValueShapeStrict, valueShapeStrict, messages: msgCtx, onAdmittedValueShapeViolation }); evaluateValidationRules(schemaForValidation as any, rows[i], 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context), messages: msgCtx }); await this.assertReferencesResolve( schemaForValidation, rows[i], suppliedPerRow[i], opCtx.context, msgCtx, @@ -4500,10 +4741,13 @@ export class ObjectQL implements IObjectQLEngine { const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(updateSchema); const valueShapeStrict = await this.valueShapeStrictFor(updateSchema); const updateMsgCtx = this.validationMessageContext(object, opCtx.context); + // [#4769] See the insert path — an update admits values on the same + // terms, so it owes the same counterexample. + const onAdmittedValueShapeViolation = this.admittedViolationSink(object); if (hookContext.input.id) { await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); - validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx }); + validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation }); if (needsPriorRecord(updateSchema as any) || (this.hooks.get('afterUpdate')?.length ?? 0) > 0) { const priorAst: QueryAST = { object, where: { id: hookContext.input.id }, limit: 1 }; priorRecord = await driver.findOne(object, priorAst, hookContext.input.options as any); @@ -4534,7 +4778,7 @@ export class ObjectQL implements IObjectQLEngine { } else if (options?.multi && driver.updateMany) { await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); - validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx }); + validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation }); // [#2982] Consume the middleware-composed AST seeded above, so // the injected row-scoping (RLS write filter, sharing's // editable-rows filter) actually binds the driver operation. Fail diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index ba0862ebea..87e7e42eb9 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -48,6 +48,7 @@ export type { CompanionFieldMeta, CompanionObjectMeta } from './search-companion // Export Engine export { ObjectQL, ObjectRepository, ScopedContext } from './engine.js'; export type { HookHandler, HookEntry, OperationContext, EngineMiddleware } from './engine.js'; +export type { AdmittedValueShapeViolationTally } from './engine.js'; export { SummaryRecomputeError } from './summary-errors.js'; export type { SummaryRecomputeFailure } from './summary-errors.js'; // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() @@ -75,6 +76,14 @@ export type { WrapDeclarativeOptions } from './hook-wrappers.js'; // Export Validation export { ValidationError, validateRecord } from './validation/record-validator.js'; export type { FieldValidationError } from './validation/record-validator.js'; +// [ADR-0104 / #4769] The counterexample a boot produces by ADMITTING an +// off-shape value. Exported because the fresh-datastore attestation +// (`@objectstack/platform-objects`) reads it before certifying anything: a +// boot may not prove a contract it has itself just broken. +export type { + AdmittedValueShapeViolation, + AdmittedValueShapeViolationSink, +} from './validation/record-validator.js'; // [ADR-0104 D1 / #3438] The value-shape scan behind `os migrate value-shapes`. // Read-only by design: it produces the evidence and the CALLER records the // flag. The engine deliberately does not depend on `@objectstack/platform-objects` diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 9374c772b4..6f736b308d 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -353,6 +353,7 @@ function validateOne( mediaStrict = false, ctx?: ValidationMessageContext, valueStrict = false, + onAdmitted?: AdmittedValueShapeViolationSink, ): FieldValidationError | null { const fail = ( code: FieldErrorCode, @@ -519,6 +520,19 @@ function validateOne( if (isMedia ? mediaStrictEffective(mediaStrict) : valueShapeStrictEffective(valueStrict)) { return fail('invalid_type', { type: t, detail }, 'invalid_value_shape'); } + // [#4769] The write is about to be ADMITTED: this deployment will hold a + // value that the same contract, once enforced, rejects. Report it before + // the log line, and unconditionally — `warnOnce` dedupes by field, and a + // deployment's evidence must count every value, not every distinct field. + // Whoever may attest this deployment's ADR-0104 posture reads this: a + // boot cannot certify a contract it has itself just broken. + if (onAdmitted) { + try { + onAdmitted({ gate: isMedia ? 'media' : 'value-shape', field: name, type: t, detail }); + } catch { + // An evidence sink must never be the reason a write fails. + } + } // The warn-first path is a DEVELOPER log line, not an end-user message — // it names the API field and stays English so it greps the same in every // deployment's logs. @@ -691,6 +705,41 @@ function shapeSchemaFor(def: FieldDef): ReturnType { return schema; } +/** + * One ADR-0104 value-shape violation this deployment ADMITTED (#4769). + * + * "Admitted", not "found": the warn-first path let the value through, so the + * store now holds it. That is a different fact from a rejection — a rejected + * value never becomes part of this deployment's data and settles nothing about + * it, while an admitted one is a standing counterexample to the very contract + * the gate would later enforce. + * + * It exists because a NEGATIVE needs only one witness. Certifying a deployment + * clean takes a complete scan (`os migrate …`); showing it is not clean takes + * a single admitted value, which the write path has already computed with the + * exact predicate strict mode uses. So this costs nothing and is exactly as + * authoritative as the enforcement it anticipates. + */ +export interface AdmittedValueShapeViolation { + /** + * Which ADR-0104 gate this value would fail once enforced — the two classes + * are attested by two different migrations, so a counterexample to one says + * nothing about the other (see `FILE_REFERENCES_MIGRATION_ID` vs + * `VALUE_SHAPES_MIGRATION_ID`). + */ + gate: 'media' | 'value-shape'; + /** API field name the value was written to. */ + field: string; + /** The declared field type, so a report can group by what went wrong. */ + type: string; + /** The first parse issue — the prescription an author acts on. */ + detail: string; +} + +/** Where {@link AdmittedValueShapeViolation}s are reported. Never throws into + * the write path: the caller wraps it. */ +export type AdmittedValueShapeViolationSink = (violation: AdmittedValueShapeViolation) => void; + export interface ValidateRecordOptions { /** * Has THIS DEPLOYMENT completed and self-check-verified the ADR-0104 @@ -726,6 +775,18 @@ export interface ValidateRecordOptions { * behavior for any caller that has no principal to read a locale from. */ messages?: ValidationMessageContext; + + /** + * Called for every value-shape violation this call ADMITS (#4769) — i.e. the + * warn-first path, never the rejecting one. The engine passes a sink so the + * deployment's own contradiction of an ADR-0104 contract is recorded where + * the fresh-datastore attestation can see it, instead of being a log line + * that only a human ever reads. + * + * Omitted by every caller that does not attest anything; the check costs one + * truthiness test on a path that is already building an error message. + */ + onAdmittedValueShapeViolation?: AdmittedValueShapeViolationSink; } /** @@ -748,6 +809,7 @@ export function validateRecord( const mediaStrict = options.mediaValueShapeStrict === true; const valueStrict = options.valueShapeStrict === true; const messages = options.messages; + const onAdmitted = options.onAdmittedValueShapeViolation; if (mode === 'insert') { // Walk all declared fields — required check applies even when @@ -755,7 +817,7 @@ export function validateRecord( for (const [name, def] of Object.entries(fields)) { if (SKIP_FIELDS.has(name)) continue; if (def.system || def.readonly) continue; - const err = validateOne(name, def, data[name], false, mediaStrict, messages, valueStrict); + const err = validateOne(name, def, data[name], false, mediaStrict, messages, valueStrict, onAdmitted); if (err) errors.push(err); } } else { @@ -784,7 +846,7 @@ export function validateRecord( // skipRequired: PATCH-omitted fields must not 400. (No def clone — the // registry's own field object flows through so the ADR-0104 value-shape // schema cache, keyed on def identity, hits.) - const err = validateOne(name, def, value, true, mediaStrict, messages, valueStrict); + const err = validateOne(name, def, value, true, mediaStrict, messages, valueStrict, onAdmitted); if (err) errors.push(err); } } diff --git a/packages/platform-objects/src/plugin.test.ts b/packages/platform-objects/src/plugin.test.ts index 932f044bea..ccb76d903d 100644 --- a/packages/platform-objects/src/plugin.test.ts +++ b/packages/platform-objects/src/plugin.test.ts @@ -12,6 +12,7 @@ import { SysMigration, SysMigrationJournal, SysSecret } from './system/index.js' function makeCtx() { const services = new Map(); const hooks: Array<() => Promise | void> = []; + const byEvent: Record Promise | void>> = {}; const logs: { info: string[]; warn: string[] } = { info: [], warn: [] }; const ctx: any = { logger: { @@ -26,9 +27,12 @@ function makeCtx() { return s as T; }, hook: (event: string, fn: () => Promise | void) => { + (byEvent[event] ??= []).push(fn); if (event === 'kernel:ready') hooks.push(fn); }, _flushReady: async () => { for (const h of hooks) await h(); }, + _flush: async (event: string) => { for (const h of byEvent[event] ?? []) await h(); }, + _events: () => Object.keys(byEvent), }; return ctx; } @@ -169,4 +173,79 @@ describe('PlatformObjectsPlugin: fresh-datastore attestation (#3438, ADR-0104)', expect(invalidated).toBe(1); }); + + /** + * #4769 — the attestation has to run after the boot's own data has landed. + * `kernel:ready` alone was too early for a deployment that seeds: the + * certificate went in while rows contradicting it were still being written, + * and the next boot enforced it against them. + */ + describe('timing: after this boot has finished writing (#4769)', () => { + it('subscribes to app:seeded, the settle point for this boot own seed', async () => { + const plugin = new PlatformObjectsPlugin(); + const ctx = makeCtx(); + ctx.registerService('objectql', engineWith(true)); + + await plugin.init(ctx); + await plugin.start(ctx); + + expect(ctx._events()).toContain('app:seeded'); + }); + + it('attests when the seed settles, without waiting for kernel:ready', async () => { + const engine = engineWith(true); + const plugin = new PlatformObjectsPlugin(); + const ctx = makeCtx(); + ctx.registerService('objectql', engine); + await plugin.init(ctx); + await plugin.start(ctx); + + await ctx._flush('app:seeded'); + + expect(engine.rows.map((r: any) => r.id).sort()).toEqual([ + 'adr-0104-file-references', + 'adr-0104-value-shapes', + ]); + }); + + it('the two passes are one idempotent call — kernel:ready adds nothing after app:seeded', async () => { + const engine = engineWith(true); + const plugin = new PlatformObjectsPlugin(); + const ctx = makeCtx(); + ctx.registerService('objectql', engine); + await plugin.init(ctx); + await plugin.start(ctx); + + await ctx._flush('app:seeded'); + await ctx._flushReady(); + + expect(engine.rows).toHaveLength(2); + }); + + /** + * The seed's counterexample reaches the attestation through the engine, + * so a boot that wrote an off-shape value certifies nothing — the + * end-to-end shape of the #4769 repro, one layer up from the engine's own + * tests. + */ + it('a boot whose seed wrote a violating value attests nothing for that gate', async () => { + const engine = engineWith(true); + engine.valueShapeViolationsAdmitted = () => ({ + 'adr-0104-file-references': { + count: 10, + first: { object: 'showcase_task', field: 'cover', type: 'image', detail: 'Expected an opaque sys_file id' }, + }, + }); + const plugin = new PlatformObjectsPlugin(); + const ctx = makeCtx(); + ctx.registerService('objectql', engine); + await plugin.init(ctx); + await plugin.start(ctx); + + await ctx._flush('app:seeded'); + await ctx._flushReady(); + + expect(engine.rows.map((r: any) => r.id)).toEqual(['adr-0104-value-shapes']); + }); + }); }); diff --git a/packages/platform-objects/src/plugin.ts b/packages/platform-objects/src/plugin.ts index 7b671aabcc..490a50810e 100644 --- a/packages/platform-objects/src/plugin.ts +++ b/packages/platform-objects/src/plugin.ts @@ -43,7 +43,12 @@ import type { II18nService, IObjectQLEngine } from '@objectstack/spec/contracts' * platform-objects. Registered here, that error message is true. * - **Fresh-datastore attestation** (#3438, ADR-0104 2026-07-30) — * travels with the ledger registration: a store this boot created from - * empty is attested at `kernel:ready`, whichever services are composed. + * empty is attested once that boot's own data has settled + * (`app:seeded`, falling back to `kernel:ready` for kernels that never + * seed), whichever services are composed. Not before: emptiness settles + * a claim about CONTENT, and a boot that certifies itself and then seeds + * rows contradicting the certificate leaves every later boot enforcing + * it against data this one wrote (#4769). * - **Translation bundles** — `SetupAppTranslations` (the static Setup * App + sys_* dashboards) and `MetadataFormsTranslations` * (`metadataForms.*` for object/field/agent/flow/view configuration @@ -95,17 +100,23 @@ export class PlatformObjectsPlugin { // ── Fresh-datastore attestation (#3438, ADR-0104 2026-07-30) ──────── // A store this process just created from empty can hold no legacy // value, so the data migrations that exist to find and convert them - // are settled here before they are ever run — recorded now, while that + // are settled here before they are ever run — recorded while that // emptiness is still an observed fact rather than something a later // scan would have to infer. Without it every new deployment would // start lax and stay lax until someone ran a command that, for them, // does nothing: the warn regime would never die out. // + // Timing is load-bearing (#4769): the inference is "empty, therefore + // nothing here violates", and it stops holding the moment this boot + // writes. So the attestation waits for the boot's own data, and + // `attestFreshDatastore` refuses any id this boot has already + // contradicted. + // // This plugin owns the call because it registers `sys_migration` // (above; #4243 — moved here with the registration from // service-storage). A store that was found rather than created attests // nothing and keeps producing evidence by scan. - ctx?.hook?.('kernel:ready', async () => { + const attest = async () => { let engine: IObjectQLEngine | undefined; try { engine = ctx.getService?.('objectql'); @@ -128,7 +139,20 @@ export class PlatformObjectsPlugin { `[platform-objects] fresh-datastore attestation skipped (${err?.message ?? err})`, ); } - }); + }; + + // #4769 — run it as soon as the boot's own data has settled, and not + // before. `app:seeded` is that moment for a deployment that seeds: it + // fires when the inline seed finishes, including the background + // continuation of one that overran `OS_INLINE_SEED_BUDGET_MS`. Attesting + // ahead of it certified a store as clean and then filled it with the rows + // that disprove the certificate, which the NEXT boot enforced against. + // `kernel:ready` stays as the backstop for every kernel that never seeds + // (it is the same moment as before for those). Both land in the same + // idempotent call: the first one to find an id unattested and + // uncontradicted writes it, the other finds the row and skips. + ctx?.hook?.('app:seeded', attest); + ctx?.hook?.('kernel:ready', attest); ctx?.hook?.('kernel:ready', async () => { let i18n: II18nService | undefined; diff --git a/packages/platform-objects/src/system/migration-flag.test.ts b/packages/platform-objects/src/system/migration-flag.test.ts index 8015ceed0f..7b206a42d5 100644 --- a/packages/platform-objects/src/system/migration-flag.test.ts +++ b/packages/platform-objects/src/system/migration-flag.test.ts @@ -196,4 +196,72 @@ describe('fresh-datastore attestation (ADR-0104, 2026-07-30 addendum)', () => { expect(logger.warn).toHaveBeenCalled(); expect(logger.info).not.toHaveBeenCalled(); }); + + /** + * #4769. "Created empty" licenses a claim about CONTENT, and the boot doing + * the attesting is also the boot doing the seeding — so the claim has to be + * checked against what that boot has already written, not against the + * emptiness it remembers. One admitted value is a complete disproof. + */ + describe('a boot may not attest a contract it has already broken (#4769)', () => { + const VIOLATED = { + count: 10, + first: { + object: 'showcase_task', + field: 'cover', + type: 'image', + detail: 'Expected an opaque sys_file id', + }, + }; + + it('does not attest an id this boot has written violating values for', async () => { + const engine = fakeEngine(); + engine.valueShapeViolationsAdmitted = () => ({ [MIGRATION]: VIOLATED }); + const logger = { info: vi.fn(), warn: vi.fn() }; + + const attested = await attestFreshDatastore(engine, { logger }); + + expect(attested).not.toContain(MIGRATION); + expect(engine.tables.sys_migration.find((r) => r.id === MIGRATION)).toBeUndefined(); + expect(await isDataMigrationVerified(engine, MIGRATION)).toBe(false); + // The operator is told which value cost them the gate, and what closes it. + const warning = String(logger.warn.mock.calls[0]?.[0] ?? ''); + expect(warning).toContain('showcase_task.cover'); + expect(warning).toContain('os migrate files-to-references --apply'); + }); + + /** + * The two ids are attested at the same moment on the same evidence, but + * they stand for two different facts — a bad `cover` says nothing about + * whether a `location` is well formed. One contradiction must not sink + * the other gate, and must not spare its own. + */ + it('declines only the contradicted id, and still attests the other', async () => { + const engine = fakeEngine(); + engine.valueShapeViolationsAdmitted = () => ({ [MIGRATION]: VIOLATED }); + + const attested = await attestFreshDatastore(engine); + + expect(attested).toEqual( + CREATION_ATTESTED_MIGRATION_IDS.filter((id) => id !== MIGRATION) as unknown as string[], + ); + expect(await isDataMigrationVerified(engine, 'adr-0104-value-shapes')).toBe(true); + }); + + it('attests normally when the boot admitted nothing', async () => { + const engine = fakeEngine(); + engine.valueShapeViolationsAdmitted = () => ({}); + + expect(await attestFreshDatastore(engine)).toEqual([...CREATION_ATTESTED_MIGRATION_IDS]); + }); + + it('an engine that cannot report reads as no counterexample (older build, fake)', async () => { + const engine = fakeEngine(); + engine.valueShapeViolationsAdmitted = () => { + throw new Error('not implemented'); + }; + + expect(await attestFreshDatastore(engine)).toEqual([...CREATION_ATTESTED_MIGRATION_IDS]); + }); + }); }); diff --git a/packages/platform-objects/src/system/migration-flag.ts b/packages/platform-objects/src/system/migration-flag.ts index 5fddddb21e..480e365033 100644 --- a/packages/platform-objects/src/system/migration-flag.ts +++ b/packages/platform-objects/src/system/migration-flag.ts @@ -3,6 +3,7 @@ import { CREATION_ATTESTED_MIGRATION_IDS, DATA_MIGRATION_FLAG_OBJECT, + FILE_REFERENCES_MIGRATION_ID, isDataMigrationFlagVerified, type DataMigrationFlag, } from '@objectstack/spec/system'; @@ -24,12 +25,31 @@ import { const SYSTEM_CTX = { isSystem: true } as const; +/** + * What this boot has admitted against one migration's contract (#4769). The + * engine's shape, restated structurally rather than imported: this package + * defines schemas and must not take a runtime dependency on the engine to read + * one number. Mirrors `AdmittedValueShapeViolationTally` in + * `@objectstack/objectql`. + */ +export interface AdmittedMigrationViolations { + count: number; + first?: { object?: string; field?: string; type?: string; detail?: string }; +} + /** Engine surface the flag helpers need — duck-typed like the storage seams. */ export interface MigrationFlagEngine { getObject(name: string): unknown | undefined; find(object: string, options: Record): Promise>>; insert(object: string, data: Record, options?: Record): Promise; update(object: string, data: Record, options: Record): Promise; + /** + * Value-shape violations this boot has ADMITTED, keyed by migration id + * (#4769) — the counterexamples that forbid attesting. Optional so a fake or + * an older engine simply reports nothing; see {@link attestFreshDatastore} + * for why "cannot say" is handled the way it is. + */ + valueShapeViolationsAdmitted?(): Record; } /** @@ -161,10 +181,33 @@ export interface AttestationLogger { * already ships these migrations would start lax and stay lax until someone * ran a command that, for them, does nothing. * + * **The emptiness has to still be settling the question.** "Created empty" + * licenses a claim about CONTENT — no legacy value here — and that inference + * expires the moment the boot writes something. Before #4769 this function + * ignored that: it ran at `kernel:ready` while the same boot's seed was still + * landing rows, so a deployment certified itself and then immediately stored + * values contradicting the certificate. The first boot ran warn-first over + * data it kept; every later boot read the certificate, enforced, and rejected + * the very rows its predecessor had written — the same data, the same code, a + * restart the only difference. + * + * So the boot's own admissions are consulted first. The engine tallies every + * off-shape value it lets through, with the exact predicate strict mode uses, + * and one such value is a complete disproof: certifying needs a scan of + * everything, refuting needs a single counterexample. An id this boot has + * contradicted is NOT attested — the deployment stays warn-first, which is + * both true and recoverable, and the operator is told which value cost them + * the gate. An engine that cannot say (a fake, an older build) reports + * nothing, which reads as no counterexample: this is a best-effort + * bookkeeping path, and it was already the case that a store nobody could + * inspect got attested on the creator's word alone. + * * **Never overwrites.** A migration id that already has a row is skipped * untouched: a store with flag rows is by definition not one being created, * so a write here would be evidence about the wrong database — and it could - * only ever *raise* a gate the real evidence had closed. + * only ever *raise* a gate the real evidence had closed. (The engine owns the + * other direction: a certificate contradicted AFTER it was issued is revoked + * from the write path that contradicted it.) * * **Best-effort, deliberately diverging from this module's "writes fail * loudly" rule.** That rule fits the migration commands, whose entire output @@ -184,11 +227,34 @@ export async function attestFreshDatastore( const logger = options.logger; if (!engine.getObject(DATA_MIGRATION_FLAG_OBJECT)) return []; + let admitted: Record = {}; + try { + admitted = engine.valueShapeViolationsAdmitted?.() ?? {}; + } catch { + admitted = {}; + } + const now = new Date().toISOString(); const attested: string[] = []; for (const id of ids) { try { if (await readDataMigrationFlag(engine, id)) continue; // not ours to write + // #4769 — a boot may not prove a contract it has already broken. + const contradiction = admitted[id]; + if (contradiction && contradiction.count > 0) { + const at = contradiction.first; + const where = at?.object && at?.field ? `${at.object}.${at.field}` : 'a record'; + logger?.warn( + `[migration] NOT attesting '${id}' on this new datastore: this boot already wrote ` + + `${contradiction.count} value(s) that the migration's own contract rejects ` + + `(${where}${at?.detail ? `: ${at.detail}` : ''}). The store was created empty, but it ` + + 'is no longer empty and what it now holds contradicts the claim — the gate stays ' + + 'open (warn-first). Fix the data, then run `os migrate ' + + (id === FILE_REFERENCES_MIGRATION_ID ? 'files-to-references' : 'value-shapes') + + ' --apply` to close it on real evidence (ADR-0104).', + ); + continue; + } await engine.insert( DATA_MIGRATION_FLAG_OBJECT, {