diff --git a/.changeset/adr-0104-d3w2-pr5b-enable-collection.md b/.changeset/adr-0104-d3w2-pr5b-enable-collection.md new file mode 100644 index 0000000000..7b1e4fa271 --- /dev/null +++ b/.changeset/adr-0104-d3w2-pr5b-enable-collection.md @@ -0,0 +1,36 @@ +--- +"@objectstack/service-storage": minor +--- + +feat(storage): released field files enter collection on deployments that verified their file migration — ADR-0104 D3 wave 2 PR-5b (#3459) + +The gated, final step of the file-as-reference sequence. On a deployment whose +`adr-0104-file-references` flag is verified (`os migrate files-to-references +--apply`, #3617), releasing a field file's ownership — clearing the field, or +deleting the owning record — now also tombstones the file +(`status='deleted'` + `deleted_at`), which starts the `sys_file` lifecycle's +declared 30-day grace window and, at its end, hands the row to the reap sweep. +Re-referencing the id inside the window revives it, exactly like re-attaching +an attachment. + +**The two halves ship together, deliberately.** The same change extends the +reap guard's sweep-time re-verify beyond `sys_attachment` join rows to the +ownership columns: a tombstoned file whose `ref_*` columns name a current +owner (re-claimed in the window, or a release/claim race) is un-tombstoned and +vetoed. Tombstoning released files without that re-verify would have turned +every release into a *guaranteed* byte delete — the guard's old check consults +a table that is always empty for field files. This pairing was the standing +hard constraint on #3459, locked by regression tests on both halves. + +**Nothing changes for a deployment that has not migrated.** Release keeps +clearing the ownership columns only, and released files are retained forever. +Every way of not knowing — no flag row, an unreadable table, an engine that +cannot be asked — reads as "not verified": the gate fails closed, toward +retention. And the guard re-reads the flag *fresh* at sweep time (not the +release path's memoized read), so a later failing migration run — a database +that has drifted — closes the gate for already-written tombstones too, without +a restart. Attachments-scope collection is unchanged and needs no flag. + +The irreversible moment is therefore per deployment: day 30 after *that* +deployment verified its migration and released a file — never the upgrade +itself. diff --git a/.changeset/adr-0104-d3w2-pr5b-engine-flag-read.md b/.changeset/adr-0104-d3w2-pr5b-engine-flag-read.md new file mode 100644 index 0000000000..424efb7b82 --- /dev/null +++ b/.changeset/adr-0104-d3w2-pr5b-engine-flag-read.md @@ -0,0 +1,14 @@ +--- +"@objectstack/objectql": minor +--- + +feat(objectql): `engine.isFileReferencesMigrationVerified()` is public — one memoized flag read for both in-process consumers (#3459 PR-5b) + +The memoized per-deployment read of the `adr-0104-file-references` migration +flag was private to the engine's media value-shape enforcement. The storage +service's release path now asks the same question — may a released field file +be tombstoned? — so the method is public and the release hooks reach it as an +optional duck-typed member (an older engine or a test fake reads as "not +verified", failing closed). One read, one invalidation +(`invalidateDataMigrationFlags()`), no way for the two consumers to see +different answers. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index a8ed63fd2f..2f51c1db70 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -657,7 +657,7 @@ Exit status is `0` only when the self-check passes, so CI can gate on it. | Once verified | Effect | | :--- | :--- | | **Media value shapes** | A malformed `file` / `image` / `avatar` / `video` / `audio` value is **rejected** (`400 invalid_type`) instead of warned about. Set `OS_ALLOW_LAX_MEDIA_VALUES=1` to re-open leniency while diagnosing. | -| **Released-file collection** | Not yet shipped — the flag is the gate it will read. | +| **Released-file collection** | A field file whose one owning record lets go (the field is cleared or the record deleted) is tombstoned into the declared 30-day grace window; re-referencing the id within the window revives it, and after it the platform sweep reclaims the row and its bytes. Unverified deployments keep every released file forever. | Other value classes are unaffected: a `lookup` or `location` value keeps its own warn-first rollout, because this migration is evidence about *file* values and @@ -670,7 +670,9 @@ later run that *fails* its self-check clears the flag's verified state, so a database that has drifted closes its own gate. A running server reads the flag once; after migrating, **restart it** for -enforcement to take effect. +enforcement (and release-time tombstoning) to take effect. The sweep's final +delete check re-reads the flag fresh, so a later failing run stops collection +without a restart. ### Scaffolding diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 0ab0823350..203d7d0ef0 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -906,9 +906,19 @@ os migrate files-to-references --apply # converts, verifies, records the flag The run backfills legacy file-field values (inline metadata blobs, own-resolver URLs, `data:` URIs) into owned `sys_file` references and reconciles the ownership ledger against what records actually hold. The **deployment-level flag it -records** — never the platform version — is what may later authorise -irreversible behaviour, and media value shapes enforce only once *this* -deployment has verified its own migration. +records** — never the platform version — is what authorises irreversible +behaviour, and media value shapes enforce only once *this* deployment has +verified its own migration. + +**Released-file collection is live behind that same flag** (#3459). On a +verified deployment, a field file whose one owning record lets go — the field +cleared, or the record deleted — is tombstoned into the declared 30-day grace +window; re-referencing the id inside the window revives it, and past it the +platform sweep re-verifies at delete time that nothing holds the file (join +rows, ownership columns, and a fresh read of the flag itself) before +reclaiming the row and its bytes. A deployment that never migrates keeps every +released file forever: upgrading is not consent — passing your own +migration's self-check is. ### Approvals: dynamic approver routing (#3447) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index f366d8b2a2..ab7d117af5 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -2203,15 +2203,21 @@ export class ObjectQL implements IDataEngine { * * Every way of not knowing answers `false` — no storage service (so no * `sys_migration` object registered), no row, an unreadable table, a - * malformed row. That is the same posture the flag's other consumer takes: - * enforcement derives from evidence, and absent evidence is not permission. - * Here "no" means media value shapes keep warning instead of rejecting, so - * a deployment that cannot be asked keeps writing. + * malformed row. Enforcement derives from evidence, and absent evidence is + * not permission. Here "no" means media value shapes keep warning instead + * of rejecting, so a deployment that cannot be asked keeps writing. + * + * Public because the flag's other in-process consumer reads it through this + * same memoized seam: the storage service's release path (#3459 PR-5b) asks + * it whether a released field file may be tombstoned, duck-typed as an + * optional method so a fake or an older engine reads as "not verified". + * One read, one invalidation (`invalidateDataMigrationFlags`), no way for + * the two consumers to see different answers. * * Costs nothing on a kernel without the storage objects: the registry lookup * short-circuits before any query. */ - private async isFileReferencesMigrationVerified(): Promise { + async isFileReferencesMigrationVerified(): Promise { if (!this.fileReferencesMigrationVerified) { this.fileReferencesMigrationVerified = (async () => { if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return false; @@ -2234,7 +2240,8 @@ export class ObjectQL implements IDataEngine { if (verified) { this.logger.info( '[value-shape] this deployment has verified the file-as-reference migration — ' + - 'media value shapes are enforced (ADR-0104 D1 / #3617)', + 'media value shapes are enforced and released field files may be collected ' + + '(ADR-0104 / #3617)', ); } return verified; diff --git a/packages/services/service-storage/src/attachment-lifecycle.test.ts b/packages/services/service-storage/src/attachment-lifecycle.test.ts index 656d185ab1..6f0c0da328 100644 --- a/packages/services/service-storage/src/attachment-lifecycle.test.ts +++ b/packages/services/service-storage/src/attachment-lifecycle.test.ts @@ -180,13 +180,13 @@ describe('installAttachmentLifecycleHooks — tombstoning', () => { describe('createSysFileReapGuard', () => { const storage = () => ({ delete: vi.fn(async () => {}) }) as any; - it('confirms zero-ref tombstones after deleting the bytes', async () => { + it('confirms zero-ref attachment tombstones after deleting the bytes — no migration flag needed', async () => { const engine = fakeEngine({ attachments: [], files: [] }); const s = storage(); const guard = createSysFileReapGuard(engine, () => s, silentLogger()); const confirmed = await guard('sys_file', [ - { id: 'f1', key: 'attachments/f1.bin', status: 'deleted' }, + { id: 'f1', key: 'attachments/f1.bin', status: 'deleted', scope: 'attachments' }, ]); expect(s.delete).toHaveBeenCalledWith('attachments/f1.bin'); @@ -217,13 +217,117 @@ describe('createSysFileReapGuard', () => { const guard = createSysFileReapGuard(engine, () => s, logger); const confirmed = await guard('sys_file', [ - { id: 'f1', key: 'attachments/f1.bin', status: 'deleted' }, + { id: 'f1', key: 'attachments/f1.bin', status: 'deleted', scope: 'attachments' }, ]); expect(confirmed).toEqual([]); expect(logger.warn).toHaveBeenCalled(); }); + // ── Field-file lineage (#3459 PR-5b) ────────────────────────────── + // The gated, irreversible half-pair: released field files only become byte + // deletes when (a) nothing owns them at sweep time AND (b) this deployment's + // file-as-reference migration flag is verified, re-read fresh each sweep. + + it('reaps a released field file when the deployment gate is open', async () => { + const engine = fakeEngine({ attachments: [], files: [] }); + const s = storage(); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), async () => true); + + const confirmed = await guard('sys_file', [ + { id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user', ref_object: null, ref_id: null }, + ]); + + expect(s.delete).toHaveBeenCalledWith('user/f1.png'); + expect(confirmed).toEqual(['f1']); + }); + + /** + * R4 REGRESSION (the "two halves ship together" lock, #3459 PR-5b). A + * tombstoned file whose ownership columns name a current owner — re-claimed + * inside the grace window, or a release/claim race — must be un-tombstoned + * and vetoed, exactly like an attachment that regained join rows. Without + * this, every release would be a guaranteed byte delete. + */ + it('vetoes and un-tombstones a field file that regained an owner (ownership re-verify)', async () => { + const engine = fakeEngine({ + attachments: [], + files: [{ id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user' }], + }); + const s = storage(); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), async () => true); + + const confirmed = await guard('sys_file', [ + { id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user', ref_object: 'product', ref_id: 'p1', ref_field: 'image' }, + ]); + + expect(confirmed).toEqual([]); + expect(s.delete).not.toHaveBeenCalled(); + expect(engine.updates[0].data).toMatchObject({ id: 'f1', status: 'committed', deleted_at: null }); + }); + + it('vetoes field-file tombstones when no gate callback is wired (fail closed)', async () => { + const engine = fakeEngine({ attachments: [], files: [] }); + const s = storage(); + const logger = silentLogger(); + const guard = createSysFileReapGuard(engine, () => s, logger); + + const confirmed = await guard('sys_file', [ + { id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user' }, + ]); + + expect(confirmed).toEqual([]); + expect(s.delete).not.toHaveBeenCalled(); + // Kept tombstoned — the observed release stands; only deletion is withheld. + expect(engine.updates).toHaveLength(0); + expect(logger.info).toHaveBeenCalledWith(expect.stringContaining('files-to-references')); + }); + + it('vetoes field-file tombstones while the deployment gate is closed (flag regression)', async () => { + const engine = fakeEngine({ attachments: [], files: [] }); + const s = storage(); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), async () => false); + + const confirmed = await guard('sys_file', [ + { id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user' }, + ]); + + expect(confirmed).toEqual([]); + expect(s.delete).not.toHaveBeenCalled(); + }); + + it('a failing gate read vetoes field files but never blocks attachment reaps', async () => { + const engine = fakeEngine({ attachments: [], files: [] }); + const s = storage(); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), async () => { + throw new Error('sys_migration unreadable'); + }); + + const confirmed = await guard('sys_file', [ + { id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user' }, + { id: 'a1', key: 'attachments/a1.bin', status: 'deleted', scope: 'attachments' }, + ]); + + expect(confirmed).toEqual(['a1']); + expect(s.delete).toHaveBeenCalledTimes(1); + expect(s.delete).toHaveBeenCalledWith('attachments/a1.bin'); + }); + + it('reads the gate once per sweep batch', async () => { + const engine = fakeEngine({ attachments: [], files: [] }); + const s = storage(); + const isOpen = vi.fn(async () => true); + const guard = createSysFileReapGuard(engine, () => s, silentLogger(), isOpen); + + await guard('sys_file', [ + { id: 'f1', key: 'user/f1.png', status: 'deleted', scope: 'user' }, + { id: 'f2', key: 'user/f2.png', status: 'deleted', scope: 'user' }, + { id: 'f3', key: 'user/f3.png', status: 'deleted', scope: 'user' }, + ]); + + expect(isOpen).toHaveBeenCalledTimes(1); + }); + it('confirms abandoned pending uploads with best-effort byte cleanup', async () => { const engine = fakeEngine({ attachments: [], files: [] }); const s = storage(); diff --git a/packages/services/service-storage/src/attachment-lifecycle.ts b/packages/services/service-storage/src/attachment-lifecycle.ts index 1549ab4224..72b96ea088 100644 --- a/packages/services/service-storage/src/attachment-lifecycle.ts +++ b/packages/services/service-storage/src/attachment-lifecycle.ts @@ -24,9 +24,14 @@ import type { IStorageService } from '@objectstack/spec/contracts'; * confirming the row delete. Detection and scheduling stay inside the * single platform sweep — ADR-0057 §3.3, no bespoke sweeper. * - * Only `scope === 'attachments'` files are ever tombstoned: `Field.file` / - * `Field.image` / avatar uploads use other scopes and reference files from - * record columns the join-row count cannot see. + * The hooks in THIS file only ever tombstone `scope === 'attachments'` files: + * `Field.file` / `Field.image` / avatar uploads use other scopes and reference + * files from record columns the join-row count cannot see. Field-owned files + * have their own tombstone seam — `releaseOwnership` in + * `file-reference-lifecycle.ts`, active only on a deployment that has verified + * its file-as-reference migration (#3617) — and the reap guard below + * re-verifies their ownership columns, and re-reads that deployment flag, at + * sweep time (#3459 PR-5b). */ /** Engine surface these installers need — duck-typed like the other @@ -196,21 +201,47 @@ export function installAttachmentLifecycleHooks( * * - `pending`: the upload was never completed; bytes may or may not exist. * Best-effort byte delete, then confirm. - * - `deleted`: re-verify ZERO sys_attachment references at sweep time. - * References found (hook bypass, restore) → un-tombstone and veto. - * Zero references → delete bytes; a byte-delete failure vetoes so the - * row is retried next sweep (the row is the only pointer to the bytes — - * dropping it first would leak the bytes forever). + * - `deleted`: re-verify at sweep time that nothing holds the file on + * EITHER surface — zero `sys_attachment` join rows AND empty ownership + * 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 + * 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 + * bytes — dropping it first would leak the bytes forever). * - anything else: veto (shouldn't be a candidate; fail toward retention). + * + * `isCollectionOpen` absent (an older caller, a test fake) reads as "gate + * closed": field-file tombstones are kept, attachments behave as always. */ export function createSysFileReapGuard( engine: AttachmentLifecycleEngine, getStorage: () => IStorageService | null | undefined, logger: AttachmentLifecycleLogger, + isCollectionOpen?: () => Promise, ): (object: string, rows: Array>) => Promise> { return async (_object, rows) => { const confirmed: Array = []; const storage = getStorage(); + // One fresh flag read per sweep batch, taken lazily so a batch with no + // field-file tombstone costs nothing. + let gate: Promise | undefined; + const collectionOpen = () => + (gate ??= (async () => { + if (typeof isCollectionOpen !== 'function') return false; + try { + return (await isCollectionOpen()) === true; + } catch { + return false; // unreadable evidence → the gate is closed + } + })()); + let keptGateClosed = 0; for (const row of rows) { const id = row?.id as string | number | undefined; if (id === undefined || id === null) continue; @@ -234,17 +265,22 @@ export function createSysFileReapGuard( limit: 1, context: { ...SYSTEM_CTX }, }); - if (refs?.length) { + const owned = row.ref_object != null && row.ref_id != null && row.ref_id !== ''; + if (refs?.length || owned) { await engine.update( 'sys_file', { id, status: 'committed', deleted_at: null }, { context: { ...SYSTEM_CTX } }, ); logger.info( - `[storage] reap guard: sys_file ${id} regained references since tombstoning — un-tombstoned, not reaped`, + `[storage] reap guard: sys_file ${id} regained ${refs?.length ? 'references' : 'an owner'} since tombstoning — un-tombstoned, not reaped`, ); continue; } + if (row.scope !== 'attachments' && !(await collectionOpen())) { + keptGateClosed += 1; + continue; + } if (storage && typeof row.key === 'string' && row.key) await storage.delete(row.key); confirmed.push(id); } catch (err) { @@ -256,6 +292,12 @@ export function createSysFileReapGuard( } // Not a state this guard reaps — veto (fail toward retention). } + if (keptGateClosed > 0) { + 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\`)`, + ); + } return confirmed; }; } diff --git a/packages/services/service-storage/src/file-reference-lifecycle.test.ts b/packages/services/service-storage/src/file-reference-lifecycle.test.ts index fc1d7893cc..87aefd1d14 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.test.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.test.ts @@ -358,18 +358,20 @@ describe('File Reference Ownership (ADR-0104 D3 wave 2)', () => { }); /** - * R4 REGRESSION. Releasing must NOT tombstone: `deleted_at` is what makes a - * row a reap candidate, and the reap guard's sweep-time re-verify still - * only consults `sys_attachment` (always empty for a field file). Setting - * the tombstone here without extending that guard in the same change turns - * every released file into a guaranteed byte delete. + * R4 REGRESSION (updated by #3459 PR-5b). Release may tombstone ONLY on a + * deployment whose file-as-reference migration is verified — and every way + * of not knowing (no engine method, an unverified flag, a failing read) + * must keep release tombstone-free, because `deleted_at` is what makes a + * row a reap candidate. The matching half — the reap guard re-verifying + * the ownership columns at sweep time — lives in attachment-lifecycle and + * shipped in the same change; its own regression tests are there. */ - it('never tombstones on release — the file stays as retained as it was', async () => { + it('never tombstones on release when the engine cannot attest the migration (fail closed)', async () => { const engine = fakeEngine({ files: [file({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' })], records: { product: [{ id: 'p1', image: 'file_a' }] }, }); - install(engine); + install(engine); // fake engine has no isFileReferencesMigrationVerified await driveDelete(engine, 'product', { id: 'p1' }); @@ -382,6 +384,79 @@ describe('File Reference Ownership (ADR-0104 D3 wave 2)', () => { expect(w.arg).not.toHaveProperty('deleted_at'); } }); + + it('never tombstones on release while the deployment is unverified', async () => { + const engine = fakeEngine({ + files: [file({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' })], + records: { product: [{ id: 'p1', image: 'file_a' }] }, + }); + engine.isFileReferencesMigrationVerified = async () => false; + install(engine); + + await driveDelete(engine, 'product', { id: 'p1' }); + + expect(engine.tables.sys_file[0]).toMatchObject({ status: 'committed', ref_id: null }); + expect(engine.tables.sys_file[0].deleted_at).toBeUndefined(); + }); + + it('a failing flag read keeps release tombstone-free (unreadable evidence is not permission)', async () => { + const engine = fakeEngine({ + files: [file({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' })], + records: { product: [{ id: 'p1', image: 'file_a' }] }, + }); + engine.isFileReferencesMigrationVerified = async () => { + throw new Error('sys_migration unreadable'); + }; + install(engine); + + await driveDelete(engine, 'product', { id: 'p1' }); + + expect(engine.tables.sys_file[0]).toMatchObject({ status: 'committed', ref_id: null }); + }); + + it('tombstones on release once the deployment has verified its migration (#3459 PR-5b)', async () => { + const engine = fakeEngine({ + files: [file({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' })], + records: { product: [{ id: 'p1', image: 'file_a' }] }, + }); + engine.isFileReferencesMigrationVerified = async () => true; + install(engine); + + await driveDelete(engine, 'product', { id: 'p1' }); + + const row = engine.tables.sys_file[0]; + expect(row).toMatchObject({ ref_object: null, ref_id: null, ref_field: null, status: 'deleted' }); + expect(typeof row.deleted_at).toBe('string'); + }); + + it('tombstones when an update replaces the field value, on a verified deployment', async () => { + const engine = fakeEngine({ + files: [file({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' })], + records: { product: [{ id: 'p1', image: 'file_a' }] }, + }); + engine.isFileReferencesMigrationVerified = async () => true; + install(engine); + + await driveUpdate(engine, 'product', 'p1', { image: null }); + + expect(engine.tables.sys_file[0]).toMatchObject({ ref_id: null, status: 'deleted' }); + }); + + it('releases a non-committed file without tombstoning it, even when verified', async () => { + // A `pending` row already has its own never-completed reap policy; + // tombstoning it here would hand it to the wrong lifecycle. + const engine = fakeEngine({ + files: [file({ status: 'pending', ref_object: 'product', ref_id: 'p1', ref_field: 'image' })], + records: { product: [{ id: 'p1', image: 'file_a' }] }, + }); + engine.isFileReferencesMigrationVerified = async () => true; + install(engine); + + await driveDelete(engine, 'product', { id: 'p1' }); + + expect(engine.tables.sys_file[0]).toMatchObject({ status: 'pending', ref_id: null }); + expect(engine.tables.sys_file[0].deleted_at).toBeUndefined(); + }); }); // ── Exclusive ownership / copy-on-claim ────────────────────────── diff --git a/packages/services/service-storage/src/file-reference-lifecycle.ts b/packages/services/service-storage/src/file-reference-lifecycle.ts index 6811417602..7ac670481f 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.ts @@ -36,13 +36,16 @@ import type { IStorageService } from '@objectstack/spec/contracts'; * * ## What this module does NOT do * - * It records ownership and it releases ownership. It never tombstones and - * never deletes bytes. The `scope === 'attachments'` guardrail in - * `attachment-lifecycle.ts` — which is what keeps field-referenced files out - * of the reap entirely — is untouched here, so shipping this module cannot - * delete anything. Turning released files into reap candidates is a separate, - * gated change that must also extend the reap guard's sweep-time re-verify in - * the same commit; see {@link releaseOwnership}. + * It records ownership, releases ownership, and — only on a deployment that + * has verified its file-as-reference migration (#3617) — tombstones a + * released file so the platform sweep can collect it after the declared + * grace window. It never deletes bytes itself: reclaiming them stays with the + * `sys_file` reap guard in `attachment-lifecycle.ts`, whose sweep-time + * re-verify covers the ownership columns for exactly this lineage. Those two + * halves — the tombstone here, the re-verify there — shipped in the same + * change (#3459 PR-5b) and must stay together; see {@link releaseOwnership}. + * On a deployment that has not migrated, nothing here ever tombstones, so + * nothing this module does can lead to a deleted byte. * * ## Dormancy * @@ -83,6 +86,11 @@ export interface FileReferenceEngine { findOne(object: string, options: Record): Promise | null>; insert(object: string, data: Record, options?: Record): Promise; update(object: string, data: Record, options: Record): Promise; + /** Memoized read of this deployment's `adr-0104-file-references` migration + * flag (#3617) — the gate on tombstoning released files. Optional: an + * engine without it (an older engine, a test fake) reads as "not verified", + * so release stays tombstone-free. Fail closed, never open. */ + isFileReferencesMigrationVerified?(): Promise; } export interface FileReferenceLogger { @@ -420,35 +428,71 @@ async function claimFile( await engine.update('sys_file', patch, { context: { ...SYSTEM_CTX } }); } +/** + * May a released file enter collection on this deployment? True only when the + * engine attests the file-as-reference migration verified (#3617). Every way + * of not knowing — no such engine method, a read failure — answers false and + * keeps release tombstone-free: absent evidence is not permission. The engine + * memoizes the underlying flag read (one query per process; a flag recorded + * by a separate `os migrate` process is picked up on restart or via + * `invalidateDataMigrationFlags()`). + */ +async function collectionOpen(engine: FileReferenceEngine): Promise { + if (typeof engine.isFileReferencesMigrationVerified !== 'function') return false; + try { + return (await engine.isFileReferencesMigrationVerified()) === true; + } catch { + return false; + } +} + /** * Give up ownership of `rows` — the single seam through which a field-owned * file becomes unreferenced. * - * Today this only clears the ownership columns; the file becomes unowned and - * stays exactly as retained as it was before (no tombstone, so nothing makes - * it a reap candidate). Enabling collection means extending THIS function to - * also set `status='deleted'` + `deleted_at` — and that change is only safe in - * the same commit that teaches the `sys_file` reap guard to re-verify the - * ownership columns at sweep time. Doing the tombstone half alone would make - * every released file reapable while the guard still re-verifies only - * `sys_attachment` (always empty for field files), i.e. a guaranteed byte - * delete rather than a merely risky one. Keep both halves together. + * On a deployment that has verified its file-as-reference migration + * (`os migrate files-to-references --apply`, #3617), a released committed + * file is also tombstoned — `status='deleted'` + `deleted_at` — which starts + * the `sys_file` lifecycle's declared grace window and, at its end, hands the + * row to the reap guard. That guard's sweep-time re-verify covers the + * ownership columns for exactly this lineage, and the two halves shipped in + * the same change (#3459 PR-5b) deliberately: tombstoning released files + * while the guard still re-verified only `sys_attachment` — always empty for + * a field file — would turn every release into a guaranteed byte delete + * rather than a risky one. Do not separate them. + * + * On a deployment that has NOT verified (or where the engine cannot say), + * this only clears the ownership columns and the file stays exactly as + * retained as it was. A release the memoized flag read lets through after a + * recorded regression still cannot delete anything: the guard re-reads the + * flag fresh at sweep time and vetoes while the gate is closed. + * + * Only a `committed` file is tombstoned: `pending` rows already carry their + * own never-completed reap policy, and re-tombstoning a `deleted` row would + * reset its grace clock. */ async function releaseOwnership( engine: FileReferenceEngine, logger: FileReferenceLogger, rows: Array>, ): Promise { + if (rows.length === 0) return; + const collect = await collectionOpen(engine); for (const row of rows) { const id = row?.id; if (id == null) continue; + const tombstone = collect && row.status === 'committed'; try { - await engine.update( - 'sys_file', - { id, ref_object: null, ref_id: null, ref_field: null }, - { context: { ...SYSTEM_CTX } }, + const patch: Record = { id, ref_object: null, ref_id: null, ref_field: null }; + if (tombstone) { + patch.status = 'deleted'; + patch.deleted_at = new Date().toISOString(); + } + await engine.update('sys_file', patch, { context: { ...SYSTEM_CTX } }); + logger.debug?.( + `[storage] file reference: released ownership of sys_file ${String(id)}` + + (tombstone ? ' (tombstoned — grace window started)' : ''), ); - logger.debug?.(`[storage] file reference: released ownership of sys_file ${String(id)}`); } catch (err) { // Bookkeeping must never break a user's write. A missed release only // means the file stays owned — it lingers rather than being collected, diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 0b0952496a..24624fe867 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -34,7 +34,8 @@ import { SysAttachment } from '@objectstack/platform-objects/audit'; // its first consuming domain: the ADR-0104 file-as-reference row gates this // service's released-file collection (#3459 PR-5b) and the strict media // value-shape default (#3438). -import { SysMigration } from '@objectstack/platform-objects/system'; +import { SysMigration, isDataMigrationVerified } from '@objectstack/platform-objects/system'; +import { FILE_REFERENCES_MIGRATION_ID } from '@objectstack/spec/system'; import { SwappableStorageService } from './swappable-storage-service.js'; import { resolveStorageTarget, @@ -301,16 +302,23 @@ export class StorageServicePlugin implements Plugin { // Field-reference ownership (ADR-0104 D3 wave 2) — keeps // sys_file.ref_object/ref_id/ref_field in step with what records hold, // and copies bytes rather than sharing a row when a second field slot - // writes an already-owned id. Records ownership only: it never - // tombstones, so the `scope==='attachments'` reap guardrail above - // still keeps field-referenced files out of collection entirely. + // writes an already-owned id. On a deployment whose file-as-reference + // migration is verified (#3617), releasing ownership also tombstones + // the file into the declared grace window (#3459 PR-5b); the reap + // guard below re-verifies the ownership columns — and re-reads the + // deployment flag, fresh — before any byte is deleted. installFileReferenceHooks(engine as any, () => this.storage, ctx.logger); try { const lifecycle = ctx.getService('lifecycle'); if (lifecycle && typeof lifecycle.registerReapGuard === 'function') { lifecycle.registerReapGuard( 'sys_file', - createSysFileReapGuard(engine as any, () => this.storage, ctx.logger), + createSysFileReapGuard(engine as any, () => this.storage, ctx.logger, () => + // 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), + ), ); // Abort the backend multipart upload before an abandoned/terminal // sys_upload_session row is reaped, so its parts don't leak (#2970).