diff --git a/.changeset/archiver-honours-declared-ttl.md b/.changeset/archiver-honours-declared-ttl.md new file mode 100644 index 0000000000..6baaaaef88 --- /dev/null +++ b/.changeset/archiver-honours-declared-ttl.md @@ -0,0 +1,39 @@ +--- +"@objectstack/objectql": patch +--- + +**Behaviour change:** a `lifecycle` that declares both `ttl` and `archive` now +has its **`ttl` enforced** — the Archiver selects the rows it moves by the +declared TTL cutoff (`ttl.field` past `ttl.expireAfter`) instead of by +`created_at` age (#10347). + +That pair has always parsed — ADR-0057 §3.5 is satisfied because `ttl` is a +bounding policy, and the `archive.after === retention.maxAge` refine only fires +when `retention` is present — but it did nothing: `LifecycleService.reapObject` +returns into `archiveObject` before its `ttl` branch is reachable, so no reap on +`ttl.field` ever ran and the Archiver copied and hot-deleted by `created_at` age +alone. Declared, not enforced. What the author wrote is now what executes; they +no longer have to discover that the two keys cannot usefully be written +together. + +**Lifecycles that declare `archive` without `ttl` are unaffected** — they keep +selecting rows by `created_at` past `archive.after`, unchanged. Every +archive-declaring object shipped with the platform (`sys_audit_log`, +`sys_metadata_audit`) is that shape, so no bundled object changes behaviour. + +Two details of the new selection, both deliberate: + +- A row whose `ttl.field` is **null or absent is retained, not archived**. `$lt` + is a positive comparison and a value that is not there satisfies none of them + (the platform-wide null answer settled in #5298/#5299), which is also the + right reading: a row with no expiry stamp has not been given one, and treating + "absent" as "expired at the epoch" would archive exactly the rows whose expiry + the author has not yet decided. +- The cold-side `archive.keep` prune is unchanged. It bounds how long **archived** + rows survive in cold storage, not which hot rows are due, and it still measures + from `created_at` under either policy. + +If you declare `retention` beside `ttl` and `archive`, the TTL cutoff is what +selects: the age window no longer separately bounds the hot store for that +triple. Whether the Archiver should honour both windows is a separate open +question, filed as #10527 rather than decided here. diff --git a/packages/objectql/src/lifecycle/lifecycle-service.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.test.ts index 2c8f41f81d..d05fb7b8ae 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.test.ts @@ -946,6 +946,169 @@ describe('LifecycleService.sweep — Archiver (P3)', () => { expect(hot.bulkDeleted).toEqual([]); expect(report.skipped).toEqual([{ object: 'sys_audit_log', reason: 'archive-pending' }]); }); + + /* ------------------------------------------------------------------ * + * [#10347] The Archiver honours a declared `ttl`. + * + * The property under test is "the declared ttl cutoff GOVERNED which rows + * moved", and it is invisible to a suite that only asserts rows were + * archived — such a suite passes identically against the `created_at`-only + * behaviour this card changes. Two things make the cases below able to fail: + * + * 1. `filteringHotStore` really evaluates the `where` the Archiver sends. + * `hotStore()` above deliberately ignores it (its subjects are batching + * and teardown, not selection), so a control built on that fake returns + * every row under either policy and can never distinguish them. + * 2. The rows are chosen so the two policies DISAGREE about which are due, + * in both directions — one row `created_at` age would move and the ttl + * would not, and one the reverse. + * ------------------------------------------------------------------ */ + + /** The card's own example. Both windows are '90d' ON PURPOSE: the cutoff + * INSTANT is then identical under either policy, so the only thing that can + * separate them is which COLUMN is read. */ + const TTL_ARCHIVE_OBJ: LifecycleObjectLike = { + name: 'sys_audit_log', + lifecycle: { + class: 'audit', + ttl: { field: 'expires_at', expireAfter: '90d' }, + archive: { after: '90d', to: 'archive', keep: '7y' }, + } as any, + }; + + const DAY = 86_400_000; + const at = (deltaMs: number) => new Date(FIXED_NOW + deltaMs).toISOString(); + + /** + * Four rows. `created_at` age and the `expires_at` ttl disagree on the first + * two in opposite directions; the last two carry no expiry stamp at all + * (null, then the key absent) while being old enough for the age policy. + */ + const disagreeingRows = () => [ + // Age says move it — 400 days old. The ttl says it has not expired yet. + { id: 'old-unexpired', created_at: at(-400 * DAY), expires_at: at(+30 * DAY) }, + // The reverse: one day old, so age keeps it — but its stamp expired 400 + // days ago, so the declared ttl says it is due. + { id: 'young-expired', created_at: at(-DAY), expires_at: at(-400 * DAY) }, + // No expiry stamp: old by age, undecided by ttl. + { id: 'null-stamp', created_at: at(-400 * DAY), expires_at: null }, + { id: 'absent-stamp', created_at: at(-400 * DAY) }, + ]; + + /** + * A hot store that EVALUATES the archiver's `where` and records it. `$lt` is + * applied with the platform's settled null answer — a value that is not + * there satisfies no positive comparison (#5298/#5299, every backend's + * `nullValueSatisfiesOperator` ends `default: return false`) — so the + * null/absent rows are decided by that contract, not by a JS accident. + */ + function filteringHotStore(rows: Array>) { + const wheres: Array> = []; + const bulkDeleted: Array> = []; + let remaining = [...rows]; + const matches = (row: Record, where: Record) => + Object.entries(where).every(([field, cond]) => { + const value = row[field]; + if (value === null || value === undefined) return false; + return String(value) < String(cond.$lt); + }); + return { + wheres, + bulkDeleted, + remaining: () => remaining.map((r) => r.id), + driver: { + name: 'default', + find: async (_object: string, query: any) => { + wheres.push(query.where); + return remaining.filter((r) => matches(r, query.where)).slice(0, query.limit ?? remaining.length); + }, + upsert: async () => ({}), + bulkDelete: async (_object: string, ids: Array) => { + bulkDeleted.push(ids); + remaining = remaining.filter((r) => !ids.includes(r.id as string)); + }, + deleteMany: async () => 0, + }, + }; + } + + it('DISCRIMINATING CONTROL: a declared ttl decides which rows move — not created_at age', async () => { + const cold = coldStore(); + const hot = filteringHotStore(disagreeingRows()); + const { engine } = captureEngine([TTL_ARCHIVE_OBJ], { + driver: hot.driver, + datasources: { archive: cold.driver }, + }); + + const report = await service(engine).sweep(); + + // The candidate read is issued against the DECLARED ttl field. + expect(hot.wheres).toEqual([{ expires_at: { $lt: isoCutoff('90d') } }]); + // Only the expired row moves. `old-unexpired` is what makes this a control: + // the `created_at`-only Archiver copies it, and the ttl the author declared + // says it is not due for another 30 days. + expect(cold.upserts.map((r) => r.id)).toEqual(['young-expired']); + expect(hot.bulkDeleted).toEqual([['young-expired']]); + expect(hot.remaining()).toEqual(['old-unexpired', 'null-stamp', 'absent-stamp']); + + const entry = report.swept.find((e) => e.policy === 'archive'); + expect(entry?.archived).toBe(1); + expect(entry?.cutoff).toBe(isoCutoff('90d')); + expect(report.skipped).toEqual([]); + }); + + it('a row whose ttl.field is null or absent is NOT due at the epoch — it is retained', async () => { + // Stated as its own case because it is a DECISION, not a side effect: a row + // with no expiry stamp has not been given one, and archiving it would move + // exactly the rows whose expiry the author has not decided yet. The three + // rows here are all past `archive.after` by age, so a fix that reached for + // `created_at` — or read a missing stamp as 0 — would copy all three. + const cold = coldStore(); + const hot = filteringHotStore([ + { id: 'null-stamp', created_at: at(-400 * DAY), expires_at: null }, + { id: 'absent-stamp', created_at: at(-400 * DAY) }, + { id: 'expired', created_at: at(-400 * DAY), expires_at: at(-91 * DAY) }, + ]); + const { engine } = captureEngine([TTL_ARCHIVE_OBJ], { + driver: hot.driver, + datasources: { archive: cold.driver }, + }); + + const report = await service(engine).sweep(); + + expect(cold.upserts.map((r) => r.id)).toEqual(['expired']); + expect(hot.remaining()).toEqual(['null-stamp', 'absent-stamp']); + expect(report.swept.find((e) => e.policy === 'archive')?.archived).toBe(1); + }); + + it('POSITIVE CONTROL: archive WITHOUT ttl still moves rows by created_at age', async () => { + // Every archive-declaring object shipped today is this shape (`sys_audit_log`, + // `sys_metadata_audit`: retention + archive, no ttl). It is fed the SAME + // rows as the discriminating control, so the two cases answer differently + // on the same input: a fix leaking into this path would copy + // `young-expired` (which has an expired stamp) and skip the two stampless + // rows, and this expectation would fail. + const cold = coldStore(); + const hot = filteringHotStore(disagreeingRows()); + const { engine } = captureEngine([AUDIT_OBJ], { + driver: hot.driver, + datasources: { archive: cold.driver }, + }); + + const report = await service(engine).sweep(); + + expect(hot.wheres).toEqual([{ created_at: { $lt: isoCutoff('90d') } }]); + expect(cold.upserts.map((r) => r.id)).toEqual(['old-unexpired', 'null-stamp', 'absent-stamp']); + expect(hot.bulkDeleted).toEqual([['old-unexpired', 'null-stamp', 'absent-stamp']]); + expect(hot.remaining()).toEqual(['young-expired']); + + const entry = report.swept.find((e) => e.policy === 'archive'); + expect(entry?.archived).toBe(3); + expect(entry?.cutoff).toBe(isoCutoff('90d')); + // The cold-side `keep` prune is a bound on the ARCHIVE, not on which hot + // rows are due: it stays on `created_at` under either policy. + expect(cold.coldDeletes).toEqual([{ where: { created_at: { $lt: isoCutoff('7y') } } }]); + }); }); describe('LifecycleService.sweep — space reclaim', () => { diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index d8ef023e4f..c53fb4a494 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -21,7 +21,9 @@ import type { * oldest shard. Until a driver advertises rotation support, declared * rotation falls back to an age-based reap bounded by `shards × unit`. * - **Archiver** (P3): copies audit-class cold rows to the declared archive - * datasource, then deletes them from the hot store. **Safety rule:** an + * datasource, then deletes them from the hot store. Cold is `created_at` + * past `archive.after`, or [#10347] `ttl.field` past `ttl.expireAfter` + * when the object declares a `ttl` beside its `archive`. **Safety rule:** an * object that declares `archive` is never hot-deleted unless the archive * copy succeeded — a compliance ledger must not be dropped unarchived. * @@ -910,6 +912,13 @@ export class LifecycleService { // deletion happens ONLY for rows the Archiver has copied to the cold // store; when the archive datasource isn't registered, rows are retained // (never dropped unarchived) and the object is reported as skipped. + // + // [#10347] This return is not a policy DROP. A lifecycle may declare `ttl` + // beside `archive` — that pair parses — and until this card the ttl branch + // below was simply unreachable for it, so the declared per-row expiry never + // ran anywhere. {@link archiveObject} now applies that window itself (see + // its `dueField` note), so the hand-off carries the policy instead of + // discarding it. if (lc.archive) { return this.archiveObject(engine, obj, lc, report); } @@ -1124,6 +1133,9 @@ export class LifecycleService { /** * Archiver (ADR-0057 §3.3 / P3): copy rows past `archive.after` from the * hot store to the archive datasource, then delete the copied rows hot. + * [#10347] When the object ALSO declares `ttl`, the declared per-row expiry + * is what selects candidates — `ttl.field` past its `expireAfter` window — + * instead of `created_at` past `archive.after`. * Batched (500 × 20 per sweep) so a large backlog drains across sweeps * without one long-locking pass. Copies are per-row idempotent upserts, so * a sweep interrupted between copy and hot-delete re-converges. When @@ -1158,7 +1170,44 @@ export class LifecycleService { await cold.syncSchema(object, obj); } - const cutoff = new Date(this.now() - parseLifecycleDuration(archive.after)).toISOString(); + // [#10347] WHICH ROWS ARE DUE. `archive` alone moves rows by age from + // `created_at`, bounded by `archive.after` — unchanged. But a lifecycle may + // also declare `ttl` beside `archive`: ADR-0057 §3.5's refine is satisfied + // (`ttl` IS a bounding policy) and the `archive.after === retention.maxAge` + // refine only fires when `retention` is present, so the pair parses. It used + // to do nothing at all — `reapObject` returns into this method before its + // ttl branch — leaving the author with a declared expiry that never ran + // while the Archiver moved rows by `created_at` age alone. + // + // Maintainer ruling 2026-08-20: what the author declared is what executes. + // When `ttl` is declared the Archiver selects candidates by the TTL cutoff + // on `ttl.field` and hands them to the same copy → hot-delete pair, so the + // expiry the author wrote decides which rows move. Nothing else about the + // Archiver changes: `archive`-only objects keep selecting by + // `created_at`/`archive.after`, and the cold-side `keep` prune below is a + // bound on the ARCHIVE (how long cold rows survive), not on which hot rows + // are due — it stays on `created_at` either way. + // + // A row whose `ttl.field` is NULL or absent is NOT due, and is retained. + // `$lt` is a positive comparison, and a value that is not there satisfies + // none of them — the platform-wide answer settled in #5298/#5299 and + // spelled out in every backend's `nullValueSatisfiesOperator` + // (`default: return false`). That is also the answer this method wants: a + // row with no expiry stamp has not been GIVEN one, so reading "absent" as + // "expired at the epoch" would archive exactly the rows whose expiry the + // author has not decided yet — against the retain-first posture that makes + // this method refuse to hot-delete anything the cold store has not taken. + // + // ⚠️ Deliberately NOT decided here: `retention` declared beside `ttl` + + // `archive`. The ttl cutoff selects, so the age window (`archive.after`, + // which the spec pins equal to `retention.maxAge`) no longer separately + // bounds the hot store for that triple. Whether the Archiver should union + // the two windows, or the triple be refused at parse time (a + // `packages/spec` accept-set question, outside this card's fence), is + // #10527 rather than a choice this diff makes silently. + const dueField = lc.ttl ? lc.ttl.field : 'created_at'; + const dueWindow = lc.ttl ? lc.ttl.expireAfter : archive.after; + const cutoff = new Date(this.now() - parseLifecycleDuration(dueWindow)).toISOString(); let archived = 0; for (let batch = 0; batch < ARCHIVE_MAX_BATCHES_PER_SWEEP; batch++) { // [#4747] Leg boundary, per batch — the same check the reap loop makes @@ -1175,7 +1224,7 @@ export class LifecycleService { // store unchanged. if (this.abort.aborted) break; const rows = await hot.find(object, { - where: { created_at: { $lt: cutoff } }, + where: { [dueField]: { $lt: cutoff } }, limit: ARCHIVE_BATCH_SIZE, }); if (!rows.length) break;