Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/adr-0104-d3w2-pr5b-enable-collection.md
Original file line numberDiff line numberDiff line change
@@ -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.
14 changes: 14 additions & 0 deletions .changeset/adr-0104-d3w2-pr5b-engine-flag-read.md
Original file line numberDiff line numberDiff line change
@@ -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.
6 changes: 4 additions & 2 deletions content/docs/deployment/cli.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand All@@ -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.
</Callout>

### Scaffolding
Expand Down
16 changes: 13 additions & 3 deletions content/docs/releases/v17.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)

Expand Down
19 changes: 13 additions & 6 deletions packages/objectql/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<boolean> {
async isFileReferencesMigrationVerified(): Promise<boolean> {
if (!this.fileReferencesMigrationVerified) {
this.fileReferencesMigrationVerified = (async () => {
if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return false;
Expand All@@ -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;
Expand Down
110 changes: 107 additions & 3 deletions packages/services/service-storage/src/attachment-lifecycle.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');
Expand DownExpand Up@@ -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();
Expand Down
62 changes: 52 additions & 10 deletions packages/services/service-storage/src/attachment-lifecycle.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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<boolean>,
): (object: string, rows: Array<Record<string, unknown>>) => Promise<Array<string | number>> {
return async (_object, rows) => {
const confirmed: Array<string | number> = [];
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<boolean> | 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;
Expand All@@ -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) {
Expand All@@ -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;
};
}
Expand Down
Loading
Loading