diff --git a/.changeset/adr0104-lax-deviation-marker.md b/.changeset/adr0104-lax-deviation-marker.md new file mode 100644 index 0000000000..524f8346d1 --- /dev/null +++ b/.changeset/adr0104-lax-deviation-marker.md @@ -0,0 +1,63 @@ +--- +"@objectstack/spec": minor +"@objectstack/platform-objects": minor +"@objectstack/objectql": minor +"@objectstack/service-storage": minor +--- + +fix(objectql): a value admitted by an `OS_ALLOW_LAX_*` escape hatch stops released field files from being collected (#4797) + +`recordDataMigrationRun`'s contract says a deployment whose data has regressed +since it last verified closes its own gate. That only happened when a migration +was re-run — nothing told the ledger when the data actually regressed. + +Normally nothing has to. Once `sys_migration` records a verified ADR-0104 +migration the write path is strict, a non-conforming value is refused, and the +certificate cannot go stale. **The operator escape hatches are the exception, +and they exist precisely to relax a deployment that has already verified.** With +`OS_ALLOW_MEDIA_VALUES` / `OS_ALLOW_LAX_MEDIA_VALUES` / `OS_ALLOW_LAX_VALUE_SHAPES` +on, a non-conforming value is admitted and persisted while the row still reads +`verified_at` non-null, `blocking: 0`. Turn the switch off — or let any other +process or machine run without it — and strict returns to reject the very data +this deployment stored. Meanwhile the `adr-0104-file-references` row also governs +reclamation of released field files, so the reap guard kept **deleting bytes** on +the strength of a certificate that was no longer true, with nothing in the ledger +saying so. + +**A lax-admitted write now records a deviation.** The engine's admit path — the +same sink that already tallies counterexamples for #4769 — stamps +`sys_migration.deviation_observed_at` (plus a `deviation_detail` naming the +object, field, type and parse issue) on the migration whose contract the value +broke. + +**The marker gates the irreversible path, and only that.** Authority is withdrawn +in proportion to reversibility: + +| behaviour | reversible? | predicate | while a deviation stands | +| --- | --- | --- | --- | +| strict value-shape enforcement (#3438) | a rejected write is retried | `isDataMigrationFlagVerified` | continues | +| tombstoning a released file (#3459 PR-5b) | lifted on re-attach | `isDataMigrationFlagVerified` | continues | +| reap guard's byte delete | **never** | `authorisesIrreversibleAction` | **refuses** | + +A certificate is not a boolean; it is authority over a set of behaviours, and the +two halves are withdrawn on different evidence. One admitted write is a complete +disproof of "nothing here violates this contract" — enough to stop deleting data +forever. It is *not* evidence of the same order as the full-store scan that +earned the certificate, so it does not revoke it: doing that would turn an +explicitly temporary switch into a one-way door, forcing a full re-migration on +anyone who used the escape hatch once. + +Recording without gating was rejected for the opposite reason — a marker no code +consumes is a declared-but-unenforced field, and the bytes get deleted regardless. + +**Getting back to full authority is the documented route.** A real +`os migrate files-to-references --apply` / `os migrate value-shapes --apply` run +walks the whole store again, which *is* evidence of the same order, and clears +the marker. + +Additive and backward compatible. A `sys_migration` row written before these +columns existed reads as "no deviation observed", so upgrading never retroactively +closes a gate a deployment earned — the marker only ever closes it on an observed +deviation. `isDataMigrationFlagVerified` is unchanged and keeps its existing +consumers; the new `authorisesIrreversibleAction` (spec) and `mayActIrreversibly` +(platform-objects) are the stronger pair, and the reap guard is their one caller. diff --git a/content/docs/references/system/migration.mdx b/content/docs/references/system/migration.mdx index e6029da0e8..6745557bbe 100644 --- a/content/docs/references/system/migration.mdx +++ b/content/docs/references/system/migration.mdx @@ -100,6 +100,8 @@ Deployment-level record that a data migration ran here and its self-check passed | **blocking** | `integer` | ✅ | Blocking discrepancies reported by the last self-check. The gate requires 0 | | **advisory** | `integer` | optional | Advisory findings from the last run (external URLs, stale owners, …) — cost storage or need a modelling decision, never block the gate | | **details** | `string` | optional | JSON-encoded counts from the last run, for diagnostics | +| **deviation_observed_at** | `string \| null` | optional | When this deployment last ADMITTED a value the verified contract rejects, via an OS_ALLOW_LAX_* escape hatch. Does not clear verified_at — it withdraws the irreversible half of what the certificate authorises (#4797) | +| **deviation_detail** | `string \| null` | optional | JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse issue), for diagnostics | --- diff --git a/packages/objectql/src/adr0104-attestation-evidence.test.ts b/packages/objectql/src/adr0104-attestation-evidence.test.ts index 4e0832f90c..d380d13156 100644 --- a/packages/objectql/src/adr0104-attestation-evidence.test.ts +++ b/packages/objectql/src/adr0104-attestation-evidence.test.ts @@ -277,6 +277,12 @@ describe('ADR-0104 fresh-datastore attestation vs. the boot that seeds (#4769)', * 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. + * + * That is still true, and it is the reason #4797 answers this case with a + * deviation marker instead of a revocation — the certificate survives and + * only the irreversible authority it carried is withheld. This test owns + * the revocation half; the marker half is pinned in + * `adr0104-lax-deviation-marker.test.ts`. */ it('never revokes a flag on a store this boot did not create', async () => { const store = newStore(); @@ -288,10 +294,11 @@ describe('ADR-0104 fresh-datastore attestation vs. the boot that seeds (#4769)', 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. + // Counted (the fact is true), and the CERTIFICATE 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(); + expect(row?.blocking).toBe(0); } finally { vi.unstubAllEnvs(); } diff --git a/packages/objectql/src/adr0104-lax-deviation-marker.test.ts b/packages/objectql/src/adr0104-lax-deviation-marker.test.ts new file mode 100644 index 0000000000..ded5ff361a --- /dev/null +++ b/packages/objectql/src/adr0104-lax-deviation-marker.test.ts @@ -0,0 +1,415 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0104 / #4797 — a scanned certificate that its own deployment overturned. + * + * #4769 closed the case where a boot certifies a store it then contradicts. + * This is the other half, and it starts from a certificate that was earned + * honestly: `os migrate … --apply` walked the whole store and found it clean. + * + * Nothing should be able to make that certificate stale, because once it + * holds the write path is strict and a non-conforming value is refused. The + * `OS_ALLOW_LAX_*` escape hatches are the exception, and they exist precisely + * to relax a deployment that has ALREADY verified. With one on: the value is + * admitted and persisted, `sys_migration` still reads `verified_at` non-null + * with `blocking: 0`, and the moment the switch goes off — or another process + * runs without it — strict returns and rejects the very data this deployment + * stored. Worse, the same row governs reclamation of released field files, so + * bytes keep being deleted on the strength of a certificate that is now false. + * + * The ruling is to withdraw authority IN PROPORTION TO REVERSIBILITY. A + * certificate is not a boolean; it is authority over a set of behaviours. One + * admitted write is not evidence of the same order as a full scan, so it does + * not revoke the certificate (that would make an explicitly temporary switch a + * one-way door). It does record a deviation, and while that marker stands the + * unrecoverable half — deleting bytes — is withheld while everything + * recoverable keeps running. + * + * So each test below names which half it is pinning, and the NEGATIVE pins + * matter as much as the positive ones: a marker anything but the admit path + * can set is a foot-gun, not a safety device. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { ObjectQL } from './engine'; +import { + authorisesIrreversibleAction, + isDataMigrationFlagVerified, + FILE_REFERENCES_MIGRATION_ID, + VALUE_SHAPES_MIGRATION_ID, + type DataMigrationFlag, +} from '@objectstack/spec/system'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +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; +} + +/** Counts the ledger updates a run issues, so the cost pin can be measured. */ +interface DriverProbe { + migrationUpdates: number; + migrationInserts: number; +} + +function makeDriver(store: Store, probe: DriverProbe): 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() {}, + // A store that already existed: this boot created nothing, so the + // fresh-datastore path of #4769 is out of scope by construction and every + // ledger row here is evidence by SCAN. + getSchemaSyncStats: () => ({ created: 0, existing: 2 }), + 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) { + if (object === 'sys_migration') probe.migrationInserts += 1; + const row = { ...data }; + rowsOf(store, object).push(row); + return row; + }, + async update(object: string, id: string, data: any) { + if (object === 'sys_migration') probe.migrationUpdates += 1; + 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; +} + +const TASK = { + name: 'showcase_task', + fields: { + id: { type: 'text' }, + title: { type: 'text' }, + cover: { type: 'image' }, + place: { type: 'location' }, + }, +}; + +/** `sys_migration` as the ledger writer and both gates see it. */ +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' }, + deviation_observed_at: { type: 'datetime' }, + deviation_detail: { type: 'textarea' }, + }, +}; + +function boot(store: Store, probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }): ObjectQL { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver(store, probe), true); + engine.registerApp({ + id: 'showcase_pkg', + name: 'Showcase', + objects: [TASK, FLAG_OBJECT], + } as any); + return engine; +} + +/** + * The row a REAL `os migrate … --apply` leaves behind: evidence by scan, no + * `attested: datastore-created-empty` anywhere near it. This is the row #4769 + * deliberately refuses to touch, and the row this card is about. + */ +function scanCertificate(id: string): Record { + const now = new Date().toISOString(); + return { + id, + last_run_at: now, + verified_at: now, + applied_at: now, + blocking: 0, + advisory: 0, + details: JSON.stringify({ scanned_records: 12_000 }), + deviation_observed_at: null, + deviation_detail: null, + }; +} + +const OFF_SHAPE_COVER = 'https://cdn.example.com/placeholder-cover.png'; + +function flagRow(store: Store, id: string): Record | undefined { + return rowsOf(store, 'sys_migration').find((r) => r.id === id); +} + +/** The row as the spec predicates read it. */ +function asFlag(row: Record | undefined): DataMigrationFlag | null { + if (!row) return null; + return row as unknown as DataMigrationFlag; +} + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe('the producer: a lax-admitted write records a deviation (#4797)', () => { + it('marks the row the admitted value contradicts, and leaves verified_at standing', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const engine = boot(store); + + // The operator's escape hatch, on a deployment that HAS verified — the + // only configuration in which this window exists at all. + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Lax', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + + await vi.waitFor(() => { + const row = flagRow(store, FILE_REFERENCES_MIGRATION_ID); + expect(row?.deviation_observed_at).toBeTruthy(); + // The certificate itself is NOT torn up: a single write's observation + // does not overturn a walk of the whole store. + expect(row?.verified_at).toBeTruthy(); + expect(row?.blocking).toBe(0); + // And it names the value, so an operator can find what closed the gate. + const detail = JSON.parse(String(row?.deviation_detail ?? '{}')); + expect(detail.observed).toBe('lax-admitted-violating-value'); + expect(detail).toMatchObject({ object: 'showcase_task', field: 'cover', type: 'image' }); + }); + }); + + it('marks the migration whose contract was broken, and only that one', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + rowsOf(store, 'sys_migration').push(scanCertificate(VALUE_SHAPES_MIGRATION_ID)); + const engine = boot(store); + + // A `location` is `{lat, lng}`; the display string is the legacy shape. + vi.stubEnv('OS_ALLOW_LAX_VALUE_SHAPES', '1'); + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Map', place: '40.7128,-74.0060' }), + ).resolves.toBeDefined(); + + await vi.waitFor(() => { + expect(flagRow(store, VALUE_SHAPES_MIGRATION_ID)?.deviation_observed_at).toBeTruthy(); + }); + // The file migration attested a different fact and was not contradicted — + // borrowing the deviation across the two would withdraw authority nothing + // disproved. + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeFalsy(); + }); + + it('costs one ledger update per migration id per process, not one per lax write', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }; + const engine = boot(store, probe); + + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + for (let i = 0; i < 5; i++) { + await engine.insert('showcase_task', { id: `t${i}`, title: 'Lax', cover: OFF_SHAPE_COVER }); + } + + await vi.waitFor(() => { + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeTruthy(); + }); + // The marker is a latch, not a counter. Five admitted values, one write — + // otherwise a lax deployment pays a ledger round-trip per row, which is + // how a safety device gets switched off for being expensive. + expect(probe.migrationUpdates).toBe(1); + }); +}); + +describe('the reversibility split: which authority the marker withdraws (#4797)', () => { + it('withdraws the irreversible half and leaves the recoverable half intact', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const engine = boot(store); + + const before = asFlag(flagRow(store, FILE_REFERENCES_MIGRATION_ID)); + expect(isDataMigrationFlagVerified(before)).toBe(true); + expect(authorisesIrreversibleAction(before)).toBe(true); + + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + await engine.insert('showcase_task', { id: 't1', title: 'Lax', cover: OFF_SHAPE_COVER }); + await vi.waitFor(() => { + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeTruthy(); + }); + + const after = asFlag(flagRow(store, FILE_REFERENCES_MIGRATION_ID)); + // Recoverable behaviour — strict enforcement once the switch is off, + // tombstoning a released file into its grace window — keeps its authority. + expect(isDataMigrationFlagVerified(after)).toBe(true); + // Unrecoverable behaviour — deleting the bytes — loses it. + expect(authorisesIrreversibleAction(after)).toBe(false); + }); + + it('still enforces strictly once the escape hatch is off — the marker is not a second lax switch', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + + const lax = boot(store); + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + await lax.insert('showcase_task', { id: 't1', title: 'Lax', cover: OFF_SHAPE_COVER }); + await vi.waitFor(() => { + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeTruthy(); + }); + vi.unstubAllEnvs(); + + // Switch off, next process. The certificate still stands, so the write + // path is strict — which is the whole reason the marker exists, and would + // be undone by a marker that relaxed enforcement instead of reclamation. + const strict = boot(store); + await expect( + strict.insert('showcase_task', { id: 't2', title: 'Bad', cover: OFF_SHAPE_COVER }), + ).rejects.toThrow(/invalid image value/i); + }); +}); + +describe('the negative pins: nothing but the admit path may set the marker (#4797)', () => { + it('a dry-run preview never marks — a preview that gates a later reclamation is a side effect', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }; + const engine = boot(store, probe); + + // `validate()` reports the same admission as a WARNING on a lax + // deployment. It writes nothing, so it must withdraw nothing. + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + const preview = await engine.validate('showcase_task', { + id: 't1', title: 'Preview', cover: OFF_SHAPE_COVER, + }); + expect(preview.results?.[0]?.warnings?.length).toBeGreaterThan(0); + + await new Promise((r) => setTimeout(r, 20)); + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeFalsy(); + expect(probe.migrationUpdates).toBe(0); + }); + + it('a REJECTED write never marks — nothing was admitted, so nothing was contradicted', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }; + const engine = boot(store, probe); + + // No escape hatch: the certificate holds and strict refuses the value. + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Bad', cover: OFF_SHAPE_COVER }), + ).rejects.toThrow(/invalid image value/i); + + await new Promise((r) => setTimeout(r, 20)); + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeFalsy(); + expect(probe.migrationUpdates).toBe(0); + }); + + it('a conforming write never marks, escape hatch or not', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }; + const engine = boot(store, probe); + + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + await engine.insert('showcase_task', { + id: 't1', title: 'Fine', cover: 'file_01H0000000000000000000', + }); + + await new Promise((r) => setTimeout(r, 20)); + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeFalsy(); + expect(probe.migrationUpdates).toBe(0); + }); + + it('never inserts a row: no certificate means no authority to withdraw', async () => { + const store = newStore(); + const probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }; + const engine = boot(store, probe); // ledger registered, but empty + + // Warn-first deployment: the value is admitted because nothing was ever + // certified. Fabricating a row here would invent a migration run that + // never happened, and reclamation is already closed by the missing row. + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Legacy', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + + await new Promise((r) => setTimeout(r, 20)); + expect(rowsOf(store, 'sys_migration')).toHaveLength(0); + expect(probe.migrationInserts).toBe(0); + expect(probe.migrationUpdates).toBe(0); + }); + + it('leaves an already-unverified row alone — its verdict has been replaced, not weakened', async () => { + const store = newStore(); + const failed = scanCertificate(FILE_REFERENCES_MIGRATION_ID); + failed.verified_at = null; + failed.blocking = 3; + rowsOf(store, 'sys_migration').push(failed); + const probe: DriverProbe = { migrationUpdates: 0, migrationInserts: 0 }; + const engine = boot(store, probe); + + await expect( + engine.insert('showcase_task', { id: 't1', title: 'Legacy', cover: OFF_SHAPE_COVER }), + ).resolves.toBeDefined(); + + await new Promise((r) => setTimeout(r, 20)); + // Both gates are already closed by `verified_at: null`; a marker here + // would be a diagnostic nobody reads, written on every lax deployment. + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeFalsy(); + expect(probe.migrationUpdates).toBe(0); + }); +}); + +describe('the witness window reopens when the certificate is re-earned (#4797)', () => { + it('marks again after an in-process migration run cleared the marker', async () => { + const store = newStore(); + rowsOf(store, 'sys_migration').push(scanCertificate(FILE_REFERENCES_MIGRATION_ID)); + const engine = boot(store); + + vi.stubEnv('OS_ALLOW_LAX_MEDIA_VALUES', '1'); + await engine.insert('showcase_task', { id: 't1', title: 'Lax', cover: OFF_SHAPE_COVER }); + await vi.waitFor(() => { + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeTruthy(); + }); + + // The operator fixes the data and re-runs the migration in-process; the + // run clears the marker and tells the engine to re-read. + const row = flagRow(store, FILE_REFERENCES_MIGRATION_ID)!; + row.deviation_observed_at = null; + row.deviation_detail = null; + engine.invalidateDataMigrationFlags(); + + // A deviation AFTER that must be seen. Without reopening the window the + // once-per-process latch would silence every later deviation for the life + // of the process — a deployment could re-earn its gate and lose it again + // with nothing noticing. + await engine.insert('showcase_task', { id: 't2', title: 'Lax again', cover: OFF_SHAPE_COVER }); + await vi.waitFor(() => { + expect(flagRow(store, FILE_REFERENCES_MIGRATION_ID)?.deviation_observed_at).toBeTruthy(); + }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index e055b7f0e2..6a50e7da2d 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -5028,6 +5028,16 @@ export class ObjectQL implements IObjectQLEngine { private readonly retractedCreationAttestations = new Set(); /** Serializes retraction writes so concurrent violations issue one update. */ private creationAttestationRetraction: Promise = Promise.resolve(); + /** + * Ids whose deviation marker this process has already written (#4797). + * Reset by {@link invalidateDataMigrationFlags}, which is what a host calls + * after running a migration in-process — a re-earned certificate opens a + * fresh witness window, and without the reset the next admitted value in the + * same process would be silently unrecorded. + */ + private readonly recordedDeviations = new Set(); + /** Serializes deviation writes so concurrent violations issue one update. */ + private deviationRecording: Promise = Promise.resolve(); /** * The sink `validateRecord` reports admitted violations to. Built per write @@ -5049,9 +5059,126 @@ export class ObjectQL implements IObjectQLEngine { first: { object, field: violation.field, type: violation.type, detail: violation.detail }, }); } + // Two consumers of the same counterexample, answering different questions. + // The deviation marker goes first because it is the one that applies to + // EVERY verified deployment (#4797); the retraction below applies only to + // the narrow fresh-datastore case (#4769) and closes the gate outright + // there. Each is the other's backstop if one write fails. + this.recordObservedDeviation(migrationId); this.retractCreationAttestation(migrationId); } + /** + * Record that this deployment has ADMITTED a value its own verified contract + * rejects (#4797) — without touching `verified_at`. + * + * ## The window + * + * Once a certificate holds, the write path is strict and a non-conforming + * value cannot land, so the certificate cannot go stale on its own. The + * operator escape hatches are the exception, and they exist *precisely* to + * relax a deployment that has already verified: with + * `OS_ALLOW_LAX_MEDIA_VALUES` / `OS_ALLOW_LAX_VALUE_SHAPES` on, the value is + * admitted and persisted while `sys_migration` still reads `verified_at` + * non-null, `blocking: 0`. Turn the switch off — or let any other process or + * machine run without it — and strict returns to reject the very data this + * deployment stored. Meanwhile the `adr-0104-file-references` gate, which + * also governs reclamation of released field files, keeps deleting bytes on + * the strength of a certificate that is now false. + * + * ## Why a marker rather than a revocation + * + * {@link retractCreationAttestation} clears `verified_at`, and is right to: + * its target is a certificate issued on the inference "created empty, so + * clean", which a single counterexample fully disproves. A certificate + * earned by `os migrate … --apply` is a walk of the whole store, and one + * admitted write is not evidence of that order — overturning it would make a + * deliberately temporary switch into a one-way door, forcing a full + * re-migration on anyone who used the escape hatch once. + * + * So the authority is withdrawn in proportion to reversibility. The marker + * leaves every recoverable behaviour running (strict enforcement once the + * switch is off, tombstoning, throttling — a rejected write is retried, a + * tombstone is lifted on re-attach) and stops only what cannot be undone: + * `authorisesIrreversibleAction` is false while it stands, so the reap + * guard's byte delete refuses. A real apply-mode run clears it. + * + * ## Cost + * + * At most one ledger read+update per migration id per process, never awaited + * by the write that triggered it, and skipped entirely once the marker is + * standing. A lax write is therefore not a ledger round-trip; the FIRST lax + * write of each class is, and only while the row is still verified. + * + * Deliberately never inserts. No row means nothing was certified, so there + * is no authority to withdraw — and inserting one would fabricate a run that + * never happened. + */ + private recordObservedDeviation(migrationId: string): void { + if (this.recordedDeviations.has(migrationId)) return; + if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return; + this.recordedDeviations.add(migrationId); + this.deviationRecording = this.deviationRecording + .then(async () => { + const rows = await this.find(DATA_MIGRATION_FLAG_OBJECT, { + where: { id: migrationId }, + limit: 1, + context: { isSystem: true } as ExecutionContext, + }); + const row: any = rows?.[0]; + if (!row || row.id !== migrationId) return; // nothing certified — no authority to withdraw + if (row.deviation_observed_at != null && String(row.deviation_observed_at) !== '') return; // already standing + // Only a row that currently authorises something can have authority + // withdrawn. An unverified row already denies both halves, so marking + // it would add a diagnostic nobody gates on — and the next apply run + // would clear it before anything read it. + const verified = isDataMigrationFlagVerified({ + id: migrationId, + last_run_at: String(row.last_run_at ?? ''), + verified_at: row.verified_at == null ? null : String(row.verified_at), + blocking: typeof row.blocking === 'number' ? row.blocking : Number(row.blocking ?? Number.NaN), + }); + if (!verified) return; + const tally = this.admittedValueShapeViolations.get(migrationId); + const now = new Date().toISOString(); + await this.update( + DATA_MIGRATION_FLAG_OBJECT, + { + id: migrationId, + deviation_observed_at: now, + deviation_detail: JSON.stringify({ + observed: 'lax-admitted-violating-value', + ...(tally?.first ?? {}), + }), + updated_at: now, + }, + { context: { isSystem: true } as ExecutionContext }, + ); + this.logger.warn( + `[value-shape] '${migrationId}': this deployment is recorded as verified, but an ` + + 'escape hatch (OS_ALLOW_LAX_MEDIA_VALUES / OS_ALLOW_LAX_VALUE_SHAPES) just admitted a ' + + `value that contract rejects (${tally?.first.object}.${tally?.first.field}: ` + + `${tally?.first.detail}). The certificate stands for everything recoverable, but ` + + 'irreversible actions are withheld — released field files are no longer collected, so ' + + 'no byte is deleted on evidence this deployment has contradicted. Fix the data and run ' + + '`os migrate ' + + (migrationId === FILE_REFERENCES_MIGRATION_ID ? 'files-to-references' : 'value-shapes') + + ' --apply` to clear it (ADR-0104 / #4797).', + ); + }) + .catch((err: any) => { + // Bookkeeping must never surface as a write failure. Failing to record + // is the dangerous direction — it leaves the reclamation gate open on + // a certificate we now know is stale — so say so loudly. + this.logger.warn( + `[value-shape] could not record the observed deviation for '${migrationId}' ` + + `(${err?.message ?? err}) — the ledger still authorises irreversible collection while ` + + 'this deployment holds a value its own contract rejects; run the migration to ' + + 're-derive the gate (#4797)', + ); + }); + } + /** * Tear up a creation attestation this boot has just contradicted (#4769). * @@ -5157,11 +5284,19 @@ export class ObjectQL implements IObjectQLEngine { * 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 * effect without a restart. + * + * Also reopens the deviation witness window (#4797). The run that prompted + * this call cleared any standing marker, so the "already recorded, don't + * write again" guard would otherwise silence the next admitted value for the + * life of the process — a deployment could re-earn its certificate and then + * deviate again with nothing noticing. Costs at most one further ledger + * write per migration id per re-run. */ invalidateDataMigrationFlags(): void { this.fileReferencesMigrationVerified = null; this.valueShapesMigrationVerified = null; this.migrationGatesAnnounced = false; + this.recordedDeviations.clear(); } /** diff --git a/packages/platform-objects/src/apps/translations/en.objects.generated.ts b/packages/platform-objects/src/apps/translations/en.objects.generated.ts index 1378347f03..d6674f1bd2 100644 --- a/packages/platform-objects/src/apps/translations/en.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/en.objects.generated.ts @@ -3139,6 +3139,14 @@ export const enObjects: NonNullable = { label: "Details (JSON)", help: "JSON-encoded counts from the last run, for diagnostics." }, + deviation_observed_at: { + label: "Deviation Observed At", + help: "When this deployment last ADMITTED a value its own verified contract rejects, through an OS_ALLOW_LAX_* escape hatch. Deliberately does NOT clear verified_at: it withdraws only the irreversible half of what the certificate authorises — byte deletion stops, validation and tombstoning continue. Cleared by the next apply-mode run." + }, + deviation_detail: { + label: "Deviation Detail (JSON)", + help: "JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse issue), so an operator can find the value that closed the irreversible gate." + }, created_at: { label: "Created At" }, diff --git a/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts b/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts index 1bc959fd12..28902a28e9 100644 --- a/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts @@ -3139,6 +3139,14 @@ export const esESObjects: NonNullable = { label: "Details (JSON)", help: "JSON-encoded counts from the last run, for diagnostics." }, + deviation_observed_at: { + label: "Deviation Observed At", + help: "When this deployment last ADMITTED a value its own verified contract rejects, through an OS_ALLOW_LAX_* escape hatch. Deliberately does NOT clear verified_at: it withdraws only the irreversible half of what the certificate authorises — byte deletion stops, validation and tombstoning continue. Cleared by the next apply-mode run." + }, + deviation_detail: { + label: "Deviation Detail (JSON)", + help: "JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse issue), so an operator can find the value that closed the irreversible gate." + }, created_at: { label: "Created At" }, diff --git a/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts b/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts index 92a404dcf5..de728fa2aa 100644 --- a/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts @@ -3139,6 +3139,14 @@ export const jaJPObjects: NonNullable = { label: "Details (JSON)", help: "JSON-encoded counts from the last run, for diagnostics." }, + deviation_observed_at: { + label: "Deviation Observed At", + help: "When this deployment last ADMITTED a value its own verified contract rejects, through an OS_ALLOW_LAX_* escape hatch. Deliberately does NOT clear verified_at: it withdraws only the irreversible half of what the certificate authorises — byte deletion stops, validation and tombstoning continue. Cleared by the next apply-mode run." + }, + deviation_detail: { + label: "Deviation Detail (JSON)", + help: "JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse issue), so an operator can find the value that closed the irreversible gate." + }, created_at: { label: "Created At" }, diff --git a/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts b/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts index 543ba17def..39c01b9b0d 100644 --- a/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts @@ -3139,6 +3139,14 @@ export const zhCNObjects: NonNullable = { label: "详情(JSON)", help: "最近一次运行的计数,JSON 编码,供诊断用。" }, + deviation_observed_at: { + label: "Deviation Observed At", + help: "When this deployment last ADMITTED a value its own verified contract rejects, through an OS_ALLOW_LAX_* escape hatch. Deliberately does NOT clear verified_at: it withdraws only the irreversible half of what the certificate authorises — byte deletion stops, validation and tombstoning continue. Cleared by the next apply-mode run." + }, + deviation_detail: { + label: "Deviation Detail (JSON)", + help: "JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse issue), so an operator can find the value that closed the irreversible gate." + }, created_at: { label: "创建时间" }, diff --git a/packages/platform-objects/src/system/index.ts b/packages/platform-objects/src/system/index.ts index fa69351d89..c2feebe519 100644 --- a/packages/platform-objects/src/system/index.ts +++ b/packages/platform-objects/src/system/index.ts @@ -17,6 +17,7 @@ export { SysMigrationJournal } from './sys-migration-journal.object.js'; export { readDataMigrationFlag, isDataMigrationVerified, + mayActIrreversibly, recordDataMigrationRun, attestFreshDatastore, CREATION_ATTESTATION_DETAIL, diff --git a/packages/platform-objects/src/system/migration-flag.ts b/packages/platform-objects/src/system/migration-flag.ts index 480e365033..a93da9b803 100644 --- a/packages/platform-objects/src/system/migration-flag.ts +++ b/packages/platform-objects/src/system/migration-flag.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { + authorisesIrreversibleAction, CREATION_ATTESTED_MIGRATION_IDS, DATA_MIGRATION_FLAG_OBJECT, FILE_REFERENCES_MIGRATION_ID, @@ -80,6 +81,14 @@ export async function readDataMigrationFlag( blocking: typeof row.blocking === 'number' ? row.blocking : Number(row.blocking ?? Number.NaN), advisory: typeof row.advisory === 'number' ? row.advisory : undefined, details: typeof row.details === 'string' ? row.details : undefined, + // [#4797] Absent on a row written before this column existed, which + // reads as "no deviation observed" — the same answer as an unmarked new + // row, and the right one: nothing recorded a deviation because nothing + // was watching for one. Such a row is no more authorised than it was + // before, since the irreversible gate still requires `verified_at`. + deviation_observed_at: + row.deviation_observed_at == null ? null : String(row.deviation_observed_at), + deviation_detail: typeof row.deviation_detail === 'string' ? row.deviation_detail : undefined, }; } catch { return null; @@ -98,6 +107,28 @@ export async function isDataMigrationVerified( return isDataMigrationFlagVerified(await readDataMigrationFlag(engine, migrationId)); } +/** + * May `migrationId`'s certificate authorise an IRREVERSIBLE action here — one + * whose effect cannot be undone if the certificate turns out to be stale + * (#4797)? + * + * Strictly stronger than {@link isDataMigrationVerified}: everything that one + * requires, plus no deviation observed since the certificate was earned. Read + * this at the moment of irreversibility and nowhere else; a caller about to do + * something recoverable should keep asking the weaker question, because + * withdrawing recoverable authority on one admitted write is exactly the + * one-way door the ruling rejected. + * + * Fails toward retention through the same funnel as its sibling — an + * unreadable row answers `null`, which answers `false`. + */ +export async function mayActIrreversibly( + engine: MigrationFlagEngine, + migrationId: string, +): Promise { + return authorisesIrreversibleAction(await readDataMigrationFlag(engine, migrationId)); +} + /** Outcome of one gated (apply-mode) migration run. */ export interface DataMigrationRunOutcome { migrationId: string; @@ -119,6 +150,24 @@ export interface DataMigrationRunOutcome { * regressed since it last verified closes its own gate. Dry runs must not * call this: recording is what distinguishes a gated migration from a script * whose output can be ignored. + * + * A run also CLEARS any deviation marker (#4797). The marker records that one + * admitted write contradicted the certificate; a run is a fresh walk of the + * whole store, which is evidence of the same order as the certificate itself + * and therefore supersedes it in both directions. That is what keeps the + * escape hatch from becoming a one-way door: `os migrate … --apply` is the + * documented way back to full authority, and it must actually restore it. + * + * Cleared on a FAILING run too, deliberately. A failing run sets + * `verified_at: null`, which already closes both gates, so leaving the marker + * behind would only strand a stale diagnostic on a row whose verdict has been + * replaced wholesale. + * + * The one race worth naming: a lax write admitted *while* the scan is running + * can set the marker just before this clears it, and its counterexample is + * then only visible in the scan's own findings. Turn the `OS_ALLOW_LAX_*` + * switches off before re-running — which is what the operator is doing anyway + * when they run the migration to re-earn the gate. */ export async function recordDataMigrationRun( engine: MigrationFlagEngine, @@ -134,6 +183,8 @@ export async function recordDataMigrationRun( blocking: outcome.blocking, advisory: outcome.advisory, details: outcome.details === undefined ? undefined : JSON.stringify(outcome.details), + deviation_observed_at: null, + deviation_detail: null, }; const row: Record = { diff --git a/packages/platform-objects/src/system/sys-migration.object.ts b/packages/platform-objects/src/system/sys-migration.object.ts index 6d6d644f66..9edc91f483 100644 --- a/packages/platform-objects/src/system/sys-migration.object.ts +++ b/packages/platform-objects/src/system/sys-migration.object.ts @@ -13,16 +13,24 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; * from the installed version — it is evidence each deployment produces by * running `os migrate ` against its own database. * - * Consumers that would act irreversibly on migrated data gate on - * `isDataMigrationFlagVerified` (verified_at set AND blocking = 0) — e.g. the - * ADR-0104 file-as-reference row (`adr-0104-file-references`) gates released- - * file collection (#3459 PR-5b) and the strict media value-shape default - * (#3438). No row / failed self-check → consumers stay in their safe legacy - * posture: files are retained forever, lax values keep warning. Fail toward - * retention, per deployment. + * Consumers gate on the row rather than the version, and WHICH predicate they + * gate on depends on what they are about to do (#4797): + * + * - recoverable behaviour — the strict media value-shape default (#3438), + * tombstoning a released file into its grace window (#3459 PR-5b) — reads + * `isDataMigrationFlagVerified` (verified_at set AND blocking = 0); + * - irreversible behaviour — the reap guard's byte delete — reads + * `authorisesIrreversibleAction`, which additionally requires that no + * deviation has been observed since the certificate was earned. + * + * No row / failed self-check → consumers stay in their safe legacy posture: + * files are retained forever, lax values keep warning. Fail toward retention, + * per deployment. * * Writes flow through `recordDataMigrationRun` (system context, from the - * migration commands) — the API surface is read-only diagnostics. + * migration commands) — the API surface is read-only diagnostics. The one + * other writer is the engine's admit path, which sets `deviation_observed_at` + * and nothing else (#4797). * * Registered by `PlatformObjectsPlugin` (`./plugin.ts`) — the ledger is * platform infrastructure, present on every kernel that composes the platform @@ -97,6 +105,24 @@ export const SysMigration = ObjectSchema.create({ description: 'JSON-encoded counts from the last run, for diagnostics.', }), + deviation_observed_at: Field.datetime({ + label: 'Deviation Observed At', + readonly: true, + description: + 'When this deployment last ADMITTED a value its own verified contract rejects, through an ' + + 'OS_ALLOW_LAX_* escape hatch. Deliberately does NOT clear verified_at: it withdraws only the ' + + 'irreversible half of what the certificate authorises — byte deletion stops, validation and ' + + 'tombstoning continue. Cleared by the next apply-mode run.', + }), + + deviation_detail: Field.text({ + label: 'Deviation Detail (JSON)', + readonly: true, + description: + 'JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse ' + + 'issue), so an operator can find the value that closed the irreversible gate.', + }), + created_at: Field.datetime({ label: 'Created At', readonly: true, diff --git a/packages/services/service-storage/src/attachment-lifecycle.ts b/packages/services/service-storage/src/attachment-lifecycle.ts index 6b8753401f..90be7c144b 100644 --- a/packages/services/service-storage/src/attachment-lifecycle.ts +++ b/packages/services/service-storage/src/attachment-lifecycle.ts @@ -213,11 +213,17 @@ export function installAttachmentLifecycleHooks( * columns (`ref_*`). Either found (hook bypass, restore, re-claim) → * un-tombstone and veto. A tombstone outside the `attachments` scope is * field-file lineage (#3459 PR-5b) and additionally requires this - * deployment's `adr-0104-file-references` flag to be verified — re-read - * fresh each sweep via `isCollectionOpen`, so a regression recorded since - * (a later failing migration run clears `verified_at`) stops - * already-written tombstones from becoming byte deletes, without a - * restart. A closed gate vetoes but does NOT un-tombstone: the observed + * deployment's `adr-0104-file-references` flag to authorise an + * IRREVERSIBLE action — re-read fresh each sweep via `isCollectionOpen`, + * so anything recorded since stops already-written tombstones from + * becoming byte deletes, without a restart. Two things close it: a later + * failing migration run clearing `verified_at`, and a deviation observed + * on a still-verified deployment — a value an `OS_ALLOW_LAX_*` escape + * hatch admitted against the very contract the certificate asserts + * (#4797). The second is why the caller supplies `mayActIrreversibly` + * rather than `isDataMigrationVerified`: the recoverable consumers of the + * same flag keep running on the certificate, and only the byte delete + * stops. A closed gate vetoes but does NOT un-tombstone: the observed * release stands; only the permission to delete is withheld. * Clear on both counts → delete bytes; a byte-delete failure vetoes so * the row is retried next sweep (the row is the only pointer to the @@ -300,9 +306,18 @@ export function createSysFileReapGuard( // Not a state this guard reaps — veto (fail toward retention). } if (keptGateClosed > 0) { + // The guard is handed a boolean, so it cannot name WHICH of the two + // closed the gate — and naming only the first was wrong once #4797 + // added the second: a deployment whose `verified_at` is plainly set + // would be told its migration "is not verified" and sent hunting. Both + // causes are stated, and they share one remedy, so the instruction is + // unambiguous either way. `sys_migration` has the answer. logger.info( `[storage] reap guard: kept ${keptGateClosed} released field file(s) — this deployment's ` + - `file-as-reference migration is not verified (run \`os migrate files-to-references --apply\`)`, + `file-as-reference migration is not verified, or a deviation has been observed since it ` + + `was (a value an OS_ALLOW_LAX_* escape hatch admitted against the migration's own ` + + `contract). Either way: fix the data, then run \`os migrate files-to-references --apply\`. ` + + `See sys_migration.verified_at / deviation_observed_at (ADR-0104 / #4797)`, ); } return confirmed; diff --git a/packages/services/service-storage/src/lax-deviation-reclamation-gate.test.ts b/packages/services/service-storage/src/lax-deviation-reclamation-gate.test.ts new file mode 100644 index 0000000000..712ddc8873 --- /dev/null +++ b/packages/services/service-storage/src/lax-deviation-reclamation-gate.test.ts @@ -0,0 +1,248 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0104 / #4797 — the CONSUMER half: byte deletion stops while a deviation + * stands. + * + * The marker is the point of the card, but a marker with no consumer is worse + * than nothing: the ledger would declare "this deployment deviated" and the + * reclamation sweep would delete the bytes exactly as before, on a certificate + * the deployment's own data has contradicted. Unconditional "record and warn" + * was rejected for precisely that reason. + * + * So these tests are about which predicate the irreversible path reads. The + * existing reap-guard tests already pin "gate closed ⇒ veto" with a stub + * callback; what they cannot see is that the callback the storage plugin + * actually supplies is the STRONGER one. A row that is `verified_at`-set, + * `blocking: 0`, and carrying a deviation marker passes + * `isDataMigrationVerified` and fails `mayActIrreversibly` — and it is the + * second that must reach the byte delete. Wiring it back to the first is the + * regression these pin. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { + isDataMigrationVerified, + mayActIrreversibly, + recordDataMigrationRun, + type MigrationFlagEngine, +} from '@objectstack/platform-objects/system'; +import { FILE_REFERENCES_MIGRATION_ID } from '@objectstack/spec/system'; +import { createSysFileReapGuard, type AttachmentLifecycleEngine } from './attachment-lifecycle.js'; + +const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() }); +const storage = () => ({ delete: vi.fn(async () => {}) }) as any; + +/** A `sys_migration` row as a real `os migrate … --apply` leaves it. */ +function verifiedRow(overrides: Record = {}): Record { + const now = new Date().toISOString(); + return { + id: FILE_REFERENCES_MIGRATION_ID, + last_run_at: now, + verified_at: now, + applied_at: now, + blocking: 0, + advisory: 0, + details: JSON.stringify({ scanned_records: 12_000 }), + deviation_observed_at: null, + deviation_detail: null, + ...overrides, + }; +} + +/** The ledger-reading surface both predicates run against. */ +function ledgerEngine(rows: Array>) { + const tables: Record>> = { sys_migration: rows }; + const engine: MigrationFlagEngine & { tables: typeof tables } = { + getObject: (name: string) => (name in tables ? { name } : undefined), + async find(object, options: any) { + const where = options?.where ?? {}; + return tables[object].filter((r) => Object.entries(where).every(([k, v]) => r[k] === v)); + }, + async insert(object, data) { + tables[object].push({ ...data }); + return data; + }, + async update(object, data: any, options?: any) { + // Held to the real engine's dispatch predicate, so this double cannot be + // looser than `ObjectQL.update` about what counts as a by-id write. + assertEngineUpdateDispatch(data, options); + const row = tables[object].find((r) => r.id === data.id); + if (row) Object.assign(row, data); + return row; + }, + tables, + }; + return engine; +} + +/** The file half of the sweep — enough for the reap guard's re-verification. */ +function reapEngine() { + const tables: Record>> = { sys_attachment: [], sys_file: [] }; + const updates: Array<{ object: string; data: any }> = []; + const engine: AttachmentLifecycleEngine & { updates: typeof updates } = { + registerHook() {}, + async find(object: string, options: any) { + const where = options?.where ?? {}; + return tables[object].filter((r) => Object.entries(where).every(([k, v]) => r[k] === v)); + }, + async findOne(object: string, options: any) { + const where = options?.where ?? {}; + return tables[object].find((r) => Object.entries(where).every(([k, v]) => r[k] === v)) ?? null; + }, + async update(object: string, data: any, options?: any) { + assertEngineUpdateDispatch(data, options); + updates.push({ object, data }); + return data; + }, + updates, + } as any; + return engine; +} + +/** One released field file, tombstoned and past its grace window. */ +const RELEASED_FIELD_FILE = { + id: 'f1', + key: 'user/f1.png', + status: 'deleted', + scope: 'user', + ref_object: null, + ref_id: null, +}; + +describe('the two predicates disagree exactly where it matters (#4797)', () => { + it('a deviation on a verified row keeps the certificate and withdraws the irreversible half', async () => { + const engine = ledgerEngine([verifiedRow({ deviation_observed_at: new Date().toISOString() })]); + + // Still certified — strict enforcement and tombstoning keep running. + await expect(isDataMigrationVerified(engine, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(true); + // But nothing may be deleted forever on it. + await expect(mayActIrreversibly(engine, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(false); + }); + + it('an untouched certificate authorises both halves', async () => { + const engine = ledgerEngine([verifiedRow()]); + await expect(isDataMigrationVerified(engine, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(true); + await expect(mayActIrreversibly(engine, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(true); + }); + + it('a row written before the column existed reads as "no deviation", not as unreadable', async () => { + const legacy = verifiedRow(); + delete legacy.deviation_observed_at; + delete legacy.deviation_detail; + const engine = ledgerEngine([legacy]); + // Upgrading the platform must not retroactively close a gate a deployment + // earned; the marker only ever closes it on an OBSERVED deviation. + await expect(mayActIrreversibly(engine, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(true); + }); +}); + +describe('the reclamation gate refuses to delete bytes while a deviation stands (#4797)', () => { + it('vetoes the released field file, deletes nothing, and leaves the tombstone in place', async () => { + const ledger = ledgerEngine([verifiedRow({ deviation_observed_at: new Date().toISOString() })]); + const engine = reapEngine(); + const s = storage(); + // Wired exactly as `storage-service-plugin.ts` wires it. + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), () => + mayActIrreversibly(ledger, FILE_REFERENCES_MIGRATION_ID), + ); + + const confirmed = await guard('sys_file', [{ ...RELEASED_FIELD_FILE }]); + + // THE PIN. Before this change the same row was confirmed and its bytes + // deleted, because the gate read `verified_at` alone. + expect(confirmed).toEqual([]); + expect(s.delete).not.toHaveBeenCalled(); + // Withholding permission is not the same as denying the release: the + // tombstone stands, so the file is collected once the gate re-opens. + expect(engine.updates).toHaveLength(0); + }); + + it('reaps the same row once the deviation is cleared — the gate is withheld, not revoked', async () => { + const ledger = ledgerEngine([verifiedRow({ deviation_observed_at: new Date().toISOString() })]); + const engine = reapEngine(); + const s = storage(); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), () => + mayActIrreversibly(ledger, FILE_REFERENCES_MIGRATION_ID), + ); + + expect(await guard('sys_file', [{ ...RELEASED_FIELD_FILE }])).toEqual([]); + + // The operator fixes the data and re-runs the migration — the documented + // way back, and it must actually restore full authority or the escape + // hatch has become the one-way door the ruling rejected. + await recordDataMigrationRun(ledger, { + migrationId: FILE_REFERENCES_MIGRATION_ID, + passed: true, + blocking: 0, + applied: true, + details: { scanned_records: 12_000 }, + }); + + expect(await guard('sys_file', [{ ...RELEASED_FIELD_FILE }])).toEqual(['f1']); + expect(s.delete).toHaveBeenCalledWith('user/f1.png'); + }); + + it('still reaps ATTACHMENT-scope tombstones — they never rode on this flag', async () => { + const ledger = ledgerEngine([verifiedRow({ deviation_observed_at: new Date().toISOString() })]); + const engine = reapEngine(); + const s = storage(); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), () => + mayActIrreversibly(ledger, FILE_REFERENCES_MIGRATION_ID), + ); + + const confirmed = await guard('sys_file', [ + { id: 'a1', key: 'attachments/a1.bin', status: 'deleted', scope: 'attachments' }, + ]); + + // The deviation is evidence about ADR-0104 field-file values. Letting it + // reach a lifecycle that never gated on the flag would be the borrowed- + // evidence error in the other direction. + expect(confirmed).toEqual(['a1']); + expect(s.delete).toHaveBeenCalledWith('attachments/a1.bin'); + }); +}); + +describe('a migration re-run clears the marker (#4797)', () => { + it('clears it on a passing apply run and restores irreversible authority', async () => { + const ledger = ledgerEngine([verifiedRow({ + deviation_observed_at: new Date().toISOString(), + deviation_detail: JSON.stringify({ observed: 'lax-admitted-violating-value' }), + })]); + + await expect(mayActIrreversibly(ledger, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(false); + + await recordDataMigrationRun(ledger, { + migrationId: FILE_REFERENCES_MIGRATION_ID, + passed: true, + blocking: 0, + applied: true, + }); + + const row = ledger.tables.sys_migration[0]; + expect(row.deviation_observed_at).toBeNull(); + expect(row.deviation_detail).toBeNull(); + await expect(mayActIrreversibly(ledger, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(true); + }); + + it('clears it on a FAILING run too — that run replaces the verdict wholesale', async () => { + const ledger = ledgerEngine([verifiedRow({ + deviation_observed_at: new Date().toISOString(), + })]); + + await recordDataMigrationRun(ledger, { + migrationId: FILE_REFERENCES_MIGRATION_ID, + passed: false, + blocking: 4, + applied: true, + }); + + const row = ledger.tables.sys_migration[0]; + expect(row.deviation_observed_at).toBeNull(); + // The gate is closed by the failure itself, not by a stale marker. + expect(row.verified_at).toBeNull(); + await expect(mayActIrreversibly(ledger, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(false); + await expect(isDataMigrationVerified(ledger, FILE_REFERENCES_MIGRATION_ID)).resolves.toBe(false); + }); +}); diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index e1f0627fc5..2298ea7c8d 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -34,7 +34,7 @@ import { SysAttachment } from '@objectstack/platform-objects/audit'; // `PlatformObjectsPlugin` (#4243), not by this service — this service only // consumes the ADR-0104 file-as-reference flag, which gates its released-file // collection (#3459 PR-5b). -import { isDataMigrationVerified } from '@objectstack/platform-objects/system'; +import { mayActIrreversibly } from '@objectstack/platform-objects/system'; import { FILE_REFERENCES_MIGRATION_ID } from '@objectstack/spec/system'; import { SwappableStorageService } from './swappable-storage-service.js'; import { @@ -324,7 +324,17 @@ export class StorageServicePlugin implements Plugin { // Fresh read each sweep — deliberately NOT the engine's // memoized one: this sits at the moment of irreversibility, // so a regressed gate must close without a restart. - isDataMigrationVerified(engine as any, FILE_REFERENCES_MIGRATION_ID), + // + // [#4797] And the STRONGER of the two predicates, because what + // a `true` here authorises is a byte delete. + // `isDataMigrationVerified` answers "was this deployment + // certified" — the right question for the recoverable + // consumers (strict enforcement, tombstoning), and the wrong + // one here: an `OS_ALLOW_LAX_*` escape hatch can admit a value + // the certificate forbids without disturbing `verified_at`, and + // this gate would go on deleting released field files on a + // certificate the deployment's own data has contradicted. + mayActIrreversibly(engine as any, FILE_REFERENCES_MIGRATION_ID), ), ); // Abort the backend multipart upload before an abandoned/terminal diff --git a/packages/spec/api-surface/system.json b/packages/spec/api-surface/system.json index cb989fdaea..c625aa85c0 100644 --- a/packages/spec/api-surface/system.json +++ b/packages/spec/api-surface/system.json @@ -756,6 +756,7 @@ "WorkerStats (type)", "WorkerStatsSchema (const)", "audienceAllows (function)", + "authorisesIrreversibleAction (function)", "azureBlobStorageExample (const)", "defineBook (function)", "defineEmailTemplateDefinition (function)", @@ -766,6 +767,7 @@ "docAudienceAllows (function)", "emailTemplateForm (const)", "gcsStorageExample (const)", + "hasObservedDeviation (function)", "hasPlatformObjectPrefix (function)", "inProcessServiceMessage (function)", "interpolateValidationMessage (function)", diff --git a/packages/spec/authorable-surface/system.json b/packages/spec/authorable-surface/system.json index bd41665e5e..d836187808 100644 --- a/packages/spec/authorable-surface/system.json +++ b/packages/spec/authorable-surface/system.json @@ -296,6 +296,8 @@ "system/DataMigrationFlag:applied_at", "system/DataMigrationFlag:blocking", "system/DataMigrationFlag:details", + "system/DataMigrationFlag:deviation_detail", + "system/DataMigrationFlag:deviation_observed_at", "system/DataMigrationFlag:id", "system/DataMigrationFlag:last_run_at", "system/DataMigrationFlag:verified_at", diff --git a/packages/spec/export-origins/system.json b/packages/spec/export-origins/system.json index fd153db9f0..00dc41ac73 100644 --- a/packages/spec/export-origins/system.json +++ b/packages/spec/export-origins/system.json @@ -756,6 +756,7 @@ "WorkerStats": "src/system/worker.zod.ts#WorkerStats (type)", "WorkerStatsSchema": "src/system/worker.zod.ts#WorkerStatsSchema (const)", "audienceAllows": "src/system/book.zod.ts#audienceAllows (function)", + "authorisesIrreversibleAction": "src/system/migration.zod.ts#authorisesIrreversibleAction (function)", "azureBlobStorageExample": "src/system/object-storage.zod.ts#azureBlobStorageExample (const)", "defineBook": "src/system/book.zod.ts#defineBook (function)", "defineEmailTemplateDefinition": "src/system/email-template.zod.ts#defineEmailTemplateDefinition (function)", @@ -766,6 +767,7 @@ "docAudienceAllows": "src/system/book.zod.ts#docAudienceAllows (function)", "emailTemplateForm": "src/system/email-template.form.ts#emailTemplateForm (const)", "gcsStorageExample": "src/system/object-storage.zod.ts#gcsStorageExample (const)", + "hasObservedDeviation": "src/system/migration.zod.ts#hasObservedDeviation (function)", "hasPlatformObjectPrefix": "src/system/constants/platform-object-names.ts#hasPlatformObjectPrefix (function)", "inProcessServiceMessage": "src/system/core-services.zod.ts#inProcessServiceMessage (function)", "interpolateValidationMessage": "src/system/validation-message.ts#interpolateValidationMessage (function)", diff --git a/packages/spec/src/system/migration.zod.ts b/packages/spec/src/system/migration.zod.ts index 636ecd13fa..2c3a63e613 100644 --- a/packages/spec/src/system/migration.zod.ts +++ b/packages/spec/src/system/migration.zod.ts @@ -202,6 +202,10 @@ export const DataMigrationFlagSchema = lazySchema(() => z.object({ .describe('Advisory findings from the last run (external URLs, stale owners, …) — cost storage or need a modelling decision, never block the gate'), details: z.string().optional() .describe('JSON-encoded counts from the last run, for diagnostics'), + deviation_observed_at: z.string().datetime().nullable().optional() + .describe('When this deployment last ADMITTED a value the verified contract rejects, via an OS_ALLOW_LAX_* escape hatch. Does not clear verified_at — it withdraws the irreversible half of what the certificate authorises (#4797)'), + deviation_detail: z.string().nullable().optional() + .describe('JSON-encoded first counterexample behind deviation_observed_at (object, field, type, parse issue), for diagnostics'), }).describe('Deployment-level record that a data migration ran here and its self-check passed — the evidence gate consumers read instead of the platform version')); export type DataMigrationFlag = z.input; @@ -217,6 +221,62 @@ export function isDataMigrationFlagVerified(flag: DataMigrationFlag | null | und return flag.verified_at != null && flag.verified_at !== '' && flag.blocking === 0; } +/** + * Has this deployment admitted a value its own verified contract rejects, + * since that contract was last verified (#4797)? + * + * The counterexample arrives through an operator escape hatch — + * `OS_ALLOW_LAX_MEDIA_VALUES` / `OS_ALLOW_LAX_VALUE_SHAPES` — which exist + * precisely to relax a deployment that has ALREADY verified. So the window is + * real: the value is admitted and persisted, the ledger still reads + * `verified_at` non-null with `blocking: 0`, and the moment the switch goes + * off (or another process runs without it) strict returns and the same data + * starts being rejected. The certificate was true when it was written and is + * false now, and nothing in the ledger said so. + * + * Recorded on its own column rather than by clearing `verified_at`, because a + * single admitted write is not the same evidence as a failed full scan. + * Overturning a scan of the whole store on one observation is the wrong order + * of magnitude, and it would turn an explicitly temporary switch into a + * one-way door: one use of the escape hatch would force a full re-migration. + */ +export function hasObservedDeviation(flag: DataMigrationFlag | null | undefined): boolean { + if (!flag) return false; + return flag.deviation_observed_at != null && flag.deviation_observed_at !== ''; +} + +/** + * Does a flag row authorise an IRREVERSIBLE action — deleting bytes that + * cannot be brought back (#4797)? + * + * A certificate is not a boolean; it is authority over a *set of behaviours*, + * and the two halves of that set are withdrawn on different evidence. So there + * are two arbiters, not one: + * + * - {@link isDataMigrationFlagVerified} authorises the RECOVERABLE half — + * strict value-shape enforcement (#3438), tombstoning a released file into + * its grace window (#3459). Every one of those is undoable: a rejected + * write is retried, a tombstone is lifted on re-attach. + * - this one authorises the UNRECOVERABLE half — the reap guard's byte + * delete. It requires everything the first requires AND that no deviation + * has been observed since the certificate was earned. + * + * The asymmetry is the whole design. An admitted counterexample is enough to + * stop deleting data forever; it is NOT enough to overturn a full-store scan, + * so the recoverable behaviours keep running and the operator's escape hatch + * keeps being an escape hatch rather than a one-way door. A real + * `os migrate … --apply` re-run — a fresh scan of everything, which is + * evidence of the same order as the certificate — clears the marker and + * restores the irreversible half. + * + * A marker with no consumer would be worse than nothing: the ledger would + * declare a fact ("this deployment deviated") that no code acts on, while the + * bytes get deleted exactly as before. This function is that consumer. + */ +export function authorisesIrreversibleAction(flag: DataMigrationFlag | null | undefined): boolean { + return isDataMigrationFlagVerified(flag) && !hasObservedDeviation(flag); +} + // --- Migration journal (ADR-0119 D2, #4617) --- // // The flag above and the journal below answer DIFFERENT questions, and