From 518f13a8035de5ed1f8da21d79a6b6776943a96b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 09:15:51 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(objectql,service-queue):=20lifecycle=20?= =?UTF-8?q?settings=20=E8=A6=86=E7=9B=96=E4=B8=8D=E5=86=8D=E8=83=BD?= =?UTF-8?q?=E7=BB=95=E8=BF=87=E6=B6=88=E8=B4=B9=E8=80=85=E7=9A=84=E4=BF=9D?= =?UTF-8?q?=E7=95=99=E7=AA=97=E4=B8=8B=E9=99=90=20(#5195)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0057 P4 lets an operator override any object's retention window through the `lifecycle` settings namespace, and the only validation on that override was "does it parse". A retention window is not only the operator's business: other code can depend on the rows still being there. `sys_job_queue` is the worked example. `DbQueueAdapter` dedups publishes by comparing a terminal row's `created_at` against its idempotency window, and #5179 made the ordering an invariant by refusing — at construction — an idempotency window longer than the object's DECLARED retention. A settings override the constructor cannot see (`retention_overrides.sys_job_queue.maxAge = '1h'`) walks straight around it: completed rows are reaped an hour after they are written, publish keeps dedupping against 24h, and duplicate deliveries resume with nothing in any log. A consumer may now register a retention floor at runtime — `lifecycle.registerRetentionFloor(object, { policy, minWindowMs, declaredBy, consequence, remedy })` — declaring the shortest window its own contract survives: - an override below the floor, GLOBAL or TENANT-scoped, is REJECTED and the declared window keeps running. Not clamped to the floor: a clamp enforces a third number written in neither the declaration nor the settings, and it moves whenever an unrelated package changes its floor. Rejection has one fallback, the declaration, which is how an unparseable override already resolves ("never fail open into no bound at all"); - the rejection is `error`-level with the consequence AND the fix, because what it prevents leaves the system looking healthy; it is also on the sweep report as `floorViolations`, machine-readable, every sweep; - a DECLARED window below a floor is reported the same way and still enforced — refusing to reap would trade a broken consumer contract for the unbounded table #5179 just closed; - objects with no registered floor are untouched: P4 behaves exactly as before. Floors are runtime wiring, not spec surface — the same call ADR-0057's reap-guard amendment makes, plus a reason of their own: the queue's floor IS `DbQueueAdapterOptions.idempotencyWindowMs`, a per-kernel construction option, so a static key on the object's `lifecycle` block could only be a copy that drifts. No `packages/spec` change. `QueueServicePlugin` registers `sys_job_queue`'s floor on `kernel:ready` carrying the window the adapter was actually constructed with, so a non-default `db.idempotencyWindowMs` is covered too. The ordering is now enforced from both ends: the constructor rejects a too-long idempotency window, the floor rejects a too-short `maxAge`. Tests cover the rejected 1h override (global and tenant), a legal override still winning, an override exactly at the floor, objects with no floor being unaffected, ttl/retention floors staying separate, strictest-floor-wins, re-registration replacing, log-once/report-always, and the end-to-end queue scenario — including a test that REPRODUCES the bypass with no floor registered, so the harness is proven to be able to fail. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017MCKJaEomEqg4tvz4SzdNd --- .changeset/lifecycle-retention-floor.md | 74 +++++ ...057-system-data-lifecycle-and-retention.md | 45 +++ packages/objectql/src/index.ts | 3 + .../src/lifecycle/lifecycle-service.test.ts | 295 ++++++++++++++++++ .../src/lifecycle/lifecycle-service.ts | 287 ++++++++++++++++- .../src/lifecycle/lifecycle-settings.ts | 4 +- packages/services/service-queue/README.md | 10 +- .../service-queue/src/db-queue-adapter.ts | 58 ++++ .../src/job-queue-retention.test.ts | 221 ++++++++++++- .../service-queue/src/queue-service-plugin.ts | 44 +++ scripts/adr-anchors.json | 5 + 11 files changed, 1031 insertions(+), 15 deletions(-) create mode 100644 .changeset/lifecycle-retention-floor.md diff --git a/.changeset/lifecycle-retention-floor.md b/.changeset/lifecycle-retention-floor.md new file mode 100644 index 0000000000..f18598b5c9 --- /dev/null +++ b/.changeset/lifecycle-retention-floor.md @@ -0,0 +1,74 @@ +--- +"@objectstack/objectql": minor +"@objectstack/service-queue": patch +--- + +fix(objectql,service-queue): a `lifecycle` settings override can no longer undercut a consumer's retention floor (#5195) + +ADR-0057 P4 lets an operator override any object's retention window per +environment and per tenant through the `lifecycle` settings namespace. Until now +the only validation on that override was **does it parse** — and a retention +window is not only the operator's business: other code can depend on the rows +still being there. + +`sys_job_queue` is the worked example. `DbQueueAdapter` deduplicates publishes by +comparing a terminal row's `created_at` against its idempotency window, so the +dedup check only means anything while that row still exists; #5179 made the +ordering an invariant by refusing, at construction, an idempotency window longer +than the object's **declared** retention. A settings override the constructor +cannot see walks straight around it: + +```jsonc +// lifecycle → retention_overrides +{ "sys_job_queue": { "maxAge": "1h" } } +``` + +completed rows are reaped an hour after they are written, publish keeps +deduplicating against 24h, and duplicate deliveries resume **with nothing in any +log**. + +**New: retention floors.** A consumer may now declare, at runtime, the shortest +window its own contract survives: + +```ts +lifecycle.registerRetentionFloor('sys_job_queue', { + policy: 'retention', // or 'ttl' + minWindowMs: 24 * 60 * 60 * 1000, + declaredBy: 'com.objectstack.service.queue', + consequence: '…what silently breaks below it', + remedy: '…the settings change that makes an override legal', +}); +``` + +- An override below the floor — **global or tenant-scoped** — is **rejected**, + and the declared window keeps running. Not clamped to the floor: clamping + would enforce a third number written in neither the declaration nor the + settings, and that number would move whenever an unrelated package changed + its floor. Rejection has exactly one fallback, the declaration, which is + already how an unparseable override resolves. +- The rejection is `error`-level and carries both the consequence and the fix, + because what it prevents leaves the system looking entirely healthy. It is + also on the sweep report as `LifecycleSweepReport.floorViolations` — machine- + readable, every sweep. +- A **declared** window below a registered floor is reported the same way and + still enforced: refusing to reap would trade a broken consumer contract for + the unbounded table #5179 just closed. +- Objects with no registered floor are completely unaffected — P4 overrides + behave exactly as before. + +Floors are runtime wiring, not spec surface (the same call ADR-0057's reap-guard +amendment makes), plus a reason of their own: the queue's floor **is** +`DbQueueAdapterOptions.idempotencyWindowMs`, a per-kernel construction option, so +a static key on the object's `lifecycle` block could only ever be a second copy +of it that drifts. No `packages/spec` change. + +`QueueServicePlugin` registers `sys_job_queue`'s floor on `kernel:ready`, +carrying the window the adapter was actually constructed with — so a non-default +`db.idempotencyWindowMs` is covered too. The ordering is now enforced from both +sides: the constructor rejects a too-long `idempotencyWindowMs`, the floor +rejects a too-short `maxAge`. + +New exports from `@objectstack/objectql`: `LifecycleRetentionFloor`, +`LifecycleFloorViolation`, plus `LifecycleService.registerRetentionFloor()`. +`LifecycleLoggerLike` gained an optional `error()` (absent ⇒ falls back to +`warn`), and `LifecycleSweepReport` gained `floorViolations`. diff --git a/docs/adr/0057-system-data-lifecycle-and-retention.md b/docs/adr/0057-system-data-lifecycle-and-retention.md index 9457b5713b..7319035951 100644 --- a/docs/adr/0057-system-data-lifecycle-and-retention.md +++ b/docs/adr/0057-system-data-lifecycle-and-retention.md @@ -197,6 +197,51 @@ retried next sweep). Rules: dir), skipping `completed` sessions and vetoing on abort failure so the session's already-uploaded parts don't leak. +#### Amendment (#5195): retention floors — a consumer's lower bound on a P4 override + +P4 (§3.2, below) lets an operator override any object's window per environment +and per tenant through the `lifecycle` settings namespace. Until #5195 the only +validation on that override was *does it parse*, and a window is not only an +operator's decision: other code can depend on rows still being there. + +`sys_job_queue` is the worked example. `DbQueueAdapter` dedups publishes by +comparing a terminal row's `created_at` against its idempotency window, so the +dedup check only means anything while that row exists; #5179 made the ordering +an invariant by refusing, at construction, an idempotency window longer than +the object's **declared** retention. A settings override the constructor cannot +see (`retention_overrides.sys_job_queue.maxAge = '1h'`) walks straight around +it: completed rows are reaped an hour after they are written, publish keeps +dedupping against 24h, and duplicate deliveries resume with **nothing in any +log** — a one-day-old enforced invariant with a side door. + +So a consumer may register a **retention floor** at runtime +(`lifecycle.registerRetentionFloor(object, floor)`) declaring the shortest +window its own contract survives, plus the consequence and the fix. Rules: + +- An override — global **or** tenant-scoped — below the floor is **rejected**, + not clamped: the declared window keeps running, exactly as an unparseable + override already resolves ("never fail open into no bound at all"). Clamping + would enforce a third number written in neither the declaration nor the + settings, and that number would move whenever an unrelated package changed + its floor. +- The rejection is **`error`-level and says both halves** — what silently + breaks below the floor and which two settings make it legal — because the + failure it prevents leaves the system looking entirely healthy. It is also + on the sweep report (`floorViolations`), machine-readable, every sweep. +- Floors are **runtime wiring, not spec surface**, for the same reason reap + guards are, plus one of their own: the queue's floor *is* + `DbQueueAdapterOptions.idempotencyWindowMs`, a per-kernel construction + option, so a static key on the object's `lifecycle` block could only ever be + a second copy of it that drifts. A declaration says how long rows are kept; a + floor says how short a *consumer* can survive them being kept — different + authors, different lifetimes. +- The floor also covers the declaration itself: a declared window below a + registered floor is reported the same way, and still enforced — refusing to + reap would trade a broken consumer contract for the unbounded table #5179 + just closed. +- First consumer: `sys_job_queue` (service-queue), floored at the adapter's + configured idempotency window. + ### 3.4 Reclaim — driver space hygiene SQLite driver defaults to `auto_vacuum=INCREMENTAL` (shipped P0); the Reaper diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index c6b849c3d9..de63f7b704 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -148,6 +148,9 @@ export type { LifecycleSettingsLike, LifecycleGovernanceAlert, LifecycleReapGuard, + // [#5195] Consumer-declared floors on settings-driven window overrides. + LifecycleRetentionFloor, + LifecycleFloorViolation, } from './lifecycle/lifecycle-service.js'; export { parseLifecycleDuration } from './lifecycle/duration.js'; export { lifecycleSettingsManifest } from './lifecycle/lifecycle-settings.js'; diff --git a/packages/objectql/src/lifecycle/lifecycle-service.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.test.ts index 1a40f79017..ef707c61df 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.test.ts @@ -695,6 +695,301 @@ describe('LifecycleService.sweep — governance (P4)', () => { }); }); +// #5195 — ADR-0057 P4 lets an operator override any object's window through the +// `lifecycle` settings namespace, and until this the only validation on that +// override was "does it parse". That is a side door around #5179's invariant: +// `DbQueueAdapter` dedups `sys_job_queue` publishes against terminal rows by +// `created_at`, checks at CONSTRUCTION that its idempotency window is ≤ the +// object's DECLARED retention — and never sees a settings override. Set +// `maxAge: '1h'` and the rows the dedup check reads are reaped an hour after +// they are written, so duplicate deliveries resume with nothing in any log. +// +// A consumer now registers the shortest window its contract survives; an +// override below it is rejected (not clamped) and the declared window runs. +// Nothing here names sys_job_queue: the mechanism is the deliverable, the queue +// is only its first caller (pinned end-to-end in @objectstack/service-queue). +describe('LifecycleService — retention floors (#5195)', () => { + const FLOOR_MS = 24 * 3_600_000; // 24h, the queue's default dedup window + + const floor = (over: Partial[1]> = {}) => ({ + policy: 'retention' as const, + minWindowMs: FLOOR_MS, + declaredBy: 'com.example.consumer', + consequence: 'the consumer silently re-accepts work it already did.', + remedy: 'raise the override to ≥ 24h, or shorten the consumer window.', + ...over, + }); + + const QUEUE_LIKE: LifecycleObjectLike = { + name: 'app_work_queue', + lifecycle: { class: 'transient', retention: { maxAge: '7d', onlyWhen: { status: 'done' } } } as any, + }; + + function fakeSettings(values: Record, tenantValues: Record> = {}) { + return { + async get(_ns: string, key: string, ctx?: Record) { + const tenantId = ctx?.tenantId as string | undefined; + if (tenantId && tenantValues[tenantId] && key in tenantValues[tenantId]) { + return { value: tenantValues[tenantId][key], source: 'tenant' }; + } + if (key in values) return { value: values[key], source: 'global' }; + return { value: undefined, source: 'default' }; + }, + }; + } + + it('rejects a global override below the floor and keeps enforcing the declared window', async () => { + const error = vi.fn(); + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '1h' } } }); + const svc = service(engine, { + getSettings: () => settings, + logger: { ...silentLogger(), error }, + }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + // The 1h override never reaches the delete: rows still live 7d, so the + // consumer's 24h dedup window still has rows to dedup against. + expect(deletes).toHaveLength(1); + expect(deletes[0].where).toEqual({ created_at: { $lt: isoCutoff('7d') }, status: 'done' }); + expect(report.swept[0].cutoff).toBe(isoCutoff('7d')); + + // …and it is loud: machine-readable on the report, `error` in the log, + // carrying the consequence AND the fix (AGENTS.md degradation rule). + expect(report.floorViolations).toEqual([{ + object: 'app_work_queue', + policy: 'retention', + scope: 'global', + override: '1h', + offendingMs: 3_600_000, + floorMs: FLOOR_MS, + declaredBy: 'com.example.consumer', + appliedMs: 7 * 86_400_000, + }]); + expect(error).toHaveBeenCalledTimes(1); + const line = error.mock.calls[0]![0] as string; + expect(line).toContain('REJECTED'); + expect(line).toContain('com.example.consumer'); + expect(line).toContain('Consequence:'); + expect(line).toContain('Fix:'); + }); + + it('a legal override (≥ the floor) still wins over the declared window', async () => { + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + // 2d: shorter than the declared 7d, longer than the 24h floor — exactly the + // environment tuning P4 exists for. The floor bounds overrides, it does not + // abolish them. + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '2d' } } }); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + expect(deletes[0].where).toEqual({ created_at: { $lt: isoCutoff('2d') }, status: 'done' }); + expect(report.floorViolations).toEqual([]); + }); + + it('an override exactly AT the floor is legal (the bound is ≥, not >)', async () => { + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '24h' } } }); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + expect(deletes[0].where).toEqual({ created_at: { $lt: isoCutoff('24h') }, status: 'done' }); + expect(report.floorViolations).toEqual([]); + }); + + it('an object with no registered floor is untouched — 1h still applies', async () => { + const { engine, deletes } = captureEngine([ + QUEUE_LIKE, + { name: 'sys_job_run', lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } as any }, + ]); + const settings = fakeSettings({ + retention_overrides: { app_work_queue: { maxAge: '1h' }, sys_job_run: { maxAge: '1h' } }, + }); + const svc = service(engine, { getSettings: () => settings }); + // Floor on ONE object only. + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + const forObject = (name: string) => deletes.find((d) => d.object === name)!; + expect(forObject('app_work_queue').where.created_at.$lt).toBe(isoCutoff('7d')); + // No floor ⇒ P4 is unchanged: an aggressive override is the operator's call. + expect(forObject('sys_job_run').where.created_at.$lt).toBe(isoCutoff('1h')); + expect(report.floorViolations.map((v) => v.object)).toEqual(['app_work_queue']); + }); + + it('floors a TENANT-scoped override too — the same door one scope down', async () => { + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + (engine as any).find = async (object: string) => + object === 'sys_organization' ? [{ id: 'org_fast' }] : []; + const settings = fakeSettings( + {}, + { org_fast: { retention_overrides: { app_work_queue: { maxAge: '1h' } } } }, + ); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + // The tenant pass falls back to the window that DID pass the floor (7d), + // so no tenant can shorten its way past another package's contract. + expect(deletes[0].where).toEqual({ + created_at: { $lt: isoCutoff('7d') }, + organization_id: 'org_fast', + status: 'done', + }); + expect(report.floorViolations).toHaveLength(1); + expect(report.floorViolations[0]!.scope).toBe('tenant:org_fast'); + }); + + it('a retention floor does not reject a ttl override (policies are separate windows)', async () => { + const { engine, deletes } = captureEngine([ + { name: 'app_session', lifecycle: { class: 'transient', ttl: { field: 'expires_at', expireAfter: '7d' } } as any }, + ]); + const settings = fakeSettings({ retention_overrides: { app_session: { expireAfter: '1h' } } }); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_session', floor()); // policy: 'retention' + + const report = await svc.sweep(); + + expect(deletes[0].where).toEqual({ expires_at: { $lt: isoCutoff('1h') } }); + expect(report.floorViolations).toEqual([]); + + // The same floor declared for `ttl` DOES bite. + const second = captureEngine([ + { name: 'app_session', lifecycle: { class: 'transient', ttl: { field: 'expires_at', expireAfter: '7d' } } as any }, + ]); + const ttlSvc = service(second.engine, { getSettings: () => settings }); + ttlSvc.registerRetentionFloor('app_session', floor({ policy: 'ttl' })); + const ttlReport = await ttlSvc.sweep(); + expect(second.deletes[0].where).toEqual({ expires_at: { $lt: isoCutoff('7d') } }); + expect(ttlReport.floorViolations[0]!.policy).toBe('ttl'); + }); + + it('the strictest of several registered floors governs', async () => { + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '2d' } } }); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_work_queue', floor()); // 24h — 2d clears it + svc.registerRetentionFloor('app_work_queue', floor({ + minWindowMs: 3 * 86_400_000, + declaredBy: 'com.example.slow-consumer', + })); // 3d — 2d does not + + const report = await svc.sweep(); + + expect(deletes[0].where.created_at.$lt).toBe(isoCutoff('7d')); + expect(report.floorViolations[0]!.declaredBy).toBe('com.example.slow-consumer'); + }); + + it('re-registering the same (object, policy, declaredBy) replaces rather than accumulates', async () => { + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '2d' } } }); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_work_queue', floor({ minWindowMs: 3 * 86_400_000 })); + // A reconstructed adapter re-registers with a lowered window; the stale + // stricter floor must not linger and keep rejecting a now-legal override. + svc.registerRetentionFloor('app_work_queue', floor({ minWindowMs: 3_600_000 })); + + const report = await svc.sweep(); + + expect(deletes[0].where.created_at.$lt).toBe(isoCutoff('2d')); + expect(report.floorViolations).toEqual([]); + }); + + it('reports a DECLARED window below the floor, and still enforces it', async () => { + const error = vi.fn(); + const { engine, deletes } = captureEngine([ + { name: 'app_work_queue', lifecycle: { class: 'transient', retention: { maxAge: '6h' } } as any }, + ]); + const svc = service(engine, { logger: { ...silentLogger(), error } }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + // Refusing to reap would trade a broken consumer contract for the + // unbounded table #5179 closed — so the declaration still runs, loudly. + expect(deletes[0].where.created_at.$lt).toBe(isoCutoff('6h')); + expect(report.floorViolations).toEqual([{ + object: 'app_work_queue', + policy: 'retention', + scope: 'global', + offendingMs: 6 * 3_600_000, + floorMs: FLOOR_MS, + declaredBy: 'com.example.consumer', + appliedMs: 6 * 3_600_000, + }]); + expect(error).toHaveBeenCalledTimes(1); + expect(error.mock.calls[0]![0]).toContain('declared retention window'); + }); + + it('logs a standing violation once, but reports it on every sweep', async () => { + const error = vi.fn(); + const { engine } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '1h' } } }); + const svc = service(engine, { getSettings: () => settings, logger: { ...silentLogger(), error } }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const first = await svc.sweep(); + const second = await svc.sweep(); + + // Hourly repetition of an unchanged misconfiguration is what trains people + // to skim `error` — the report stays complete regardless. + expect(error).toHaveBeenCalledTimes(1); + expect(first.floorViolations).toHaveLength(1); + expect(second.floorViolations).toHaveLength(1); + }); + + it('falls back to warn when the logger has no error method', async () => { + const warn = vi.fn(); + const { engine } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: '1h' } } }); + const svc = service(engine, { + getSettings: () => settings, + logger: { info: () => {}, warn, debug: () => {} }, + }); + svc.registerRetentionFloor('app_work_queue', floor()); + + await svc.sweep(); + + expect(warn.mock.calls.some((c) => String(c[0]).includes('REJECTED'))).toBe(true); + }); + + it('refuses a malformed floor at registration — an unactionable rejection helps nobody', () => { + const { engine } = captureEngine([QUEUE_LIKE]); + const svc = service(engine); + expect(() => svc.registerRetentionFloor('app_work_queue', floor({ minWindowMs: 0 }))) + .toThrow(/positive finite minWindowMs/); + expect(() => svc.registerRetentionFloor('app_work_queue', floor({ minWindowMs: Number.NaN }))) + .toThrow(/positive finite minWindowMs/); + expect(() => svc.registerRetentionFloor('app_work_queue', floor({ policy: 'forever' as any }))) + .toThrow(/policy 'retention' or 'ttl'/); + expect(() => svc.registerRetentionFloor('app_work_queue', floor({ consequence: '' }))) + .toThrow(/declaredBy, consequence and remedy/); + expect(() => svc.registerRetentionFloor('app_work_queue', floor({ remedy: '' }))) + .toThrow(/declaredBy, consequence and remedy/); + expect(() => svc.registerRetentionFloor('', floor())).toThrow(/requires an object name/); + }); + + it('an unparseable override still keeps the declared window and is not a floor violation', async () => { + const { engine, deletes } = captureEngine([QUEUE_LIKE]); + const settings = fakeSettings({ retention_overrides: { app_work_queue: { maxAge: 'forever' } } }); + const svc = service(engine, { getSettings: () => settings }); + svc.registerRetentionFloor('app_work_queue', floor()); + + const report = await svc.sweep(); + + expect(deletes[0].where.created_at.$lt).toBe(isoCutoff('7d')); + expect(report.floorViolations).toEqual([]); + }); +}); + describe('LifecycleService timers', () => { it('start() sweeps after the initial delay and then on the interval; stop() disarms', async () => { vi.useFakeTimers(); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index b6da2e9f9a..ea291b7c9d 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -91,6 +91,9 @@ export interface LifecycleObjectLike { export interface LifecycleLoggerLike { info(msg: string, meta?: unknown): void; warn(msg: string, meta?: unknown): void; + /** Optional so a test double stays a two-method object; a real kernel logger + * always has it. Absent ⇒ {@link LifecycleService} falls back to `warn`. */ + error?(msg: string, meta?: unknown): void; debug?(msg: string, meta?: unknown): void; } @@ -168,6 +171,67 @@ const DEFAULT_GOVERNANCE: GovernanceSnapshot = { /** Cap on tenants scanned for per-tenant overrides each sweep. */ const TENANT_SCAN_LIMIT = 200; +/** + * [#5195] A **retention floor**: the shortest window a consumer's own contract + * can survive on an object it does not own. + * + * ADR-0057 P4 lets an operator override any object's window through the + * `lifecycle` settings namespace, and until #5195 the only validation on that + * override was "does it parse". That is a side door around exactly the kind of + * invariant #5179 had just made construction-time: `DbQueueAdapter` dedups + * `sys_job_queue` publishes by comparing `created_at` against its idempotency + * window and checks — at construction — that the window is ≤ the **declared** + * retention. A settings override the constructor cannot see (`maxAge: '1h'`) + * reaps the very rows the dedup check reads, and duplicate deliveries resume + * with nothing in any log. + * + * The floor is registered at **runtime** (`registerRetentionFloor`), the same + * shape as {@link LifecycleReapGuard} and for the same reason (ADR-0057 §3.3 + * amendment): the number is not a property of the declaration at all. The + * queue's floor IS `DbQueueAdapterOptions.idempotencyWindowMs` — a per-kernel + * construction option — so a static key on the object's `lifecycle` block could + * only ever be a second, drifting copy of it. Declaration says how long rows + * are kept; a floor says how short a *consumer* can survive them being kept. + */ +export interface LifecycleRetentionFloor { + /** Which window is floored: `retention` (`maxAge`, incl. the rotation + * fallback) or `ttl` (`expireAfter`). */ + policy: 'retention' | 'ttl'; + /** Shortest window, in ms, that keeps the registrar's contract true. */ + minWindowMs: number; + /** Who depends on it — named in the rejection so an operator knows who to + * talk to (e.g. `'com.objectstack.service.queue'`). */ + declaredBy: string; + /** What breaks below the floor, in operator terms. Required: an error line + * without a consequence is an error line nobody can act on. */ + consequence: string; + /** The config change that makes the override legal. Also required. */ + remedy: string; +} + +/** + * [#5195] A window that would have been enforced below a registered floor. + * Always reported per sweep (machine-readable), and logged at `error` once per + * distinct violation — the failure it prevents is silent duplicate work, which + * is a durability-class degradation, not a functional one. + */ +export interface LifecycleFloorViolation { + object: string; + policy: 'retention' | 'ttl'; + /** `'global'`, or `tenant:` for a tenant-scoped override. */ + scope: string; + /** The offending settings literal — absent when the **declaration itself** + * is what sits below the floor (there is no override to blame). */ + override?: string; + /** The offending window in ms. */ + offendingMs: number; + /** The floor that rejected it, and who registered it. */ + floorMs: number; + declaredBy: string; + /** The window actually enforced this sweep after the violation was handled. */ + appliedMs: number; +} + export interface LifecycleSweepEntry { object: string; class: string; @@ -193,6 +257,13 @@ export interface LifecycleSweepReport { reclaimed: string[]; /** Governance alerts raised this sweep (quota breaches, growth spikes). */ alerts: LifecycleGovernanceAlert[]; + /** + * [#5195] Windows rejected this sweep for sitting below a registered + * {@link LifecycleRetentionFloor}. Empty on every healthy sweep; non-empty + * means an override (or a declaration) is being overruled, and the entry + * says by whom. + */ + floorViolations: LifecycleFloorViolation[]; /** * [#4551] Read-only referential-integrity finding for this sweep, when the * engine offers the audit. Absent on an engine that does not (older engine, @@ -262,6 +333,15 @@ export class LifecycleService { private governance: GovernanceSnapshot = DEFAULT_GOVERNANCE; /** Per-object reap guards ({@link LifecycleReapGuard}). */ private readonly reapGuards = new Map(); + /** + * [#5195] Registered retention floors, keyed `object::policy::declaredBy` so + * a re-registration replaces rather than accumulates, while two independent + * consumers of one object both keep their say (the strictest wins). + */ + private readonly retentionFloors = new Map(); + /** Violations already logged, so a standing misconfiguration says it once + * (AGENTS.md degradation rule) while a CHANGED one speaks up again. */ + private readonly reportedFloorViolations = new Set(); /** * [#4747] The "the engine is going away" bit, handed to the work in flight. * @@ -341,6 +421,62 @@ export class LifecycleService { this.reapGuards.set(object, guard); } + /** + * [#5195] Register a {@link LifecycleRetentionFloor} for one object. + * + * From then on a settings override (global **or** tenant-scoped) that would + * shorten that object's window below the floor is REJECTED — the declared + * window stands — and the rejection is reported at `error` naming the + * registrar, the consequence and the fix. + * + * Rejected rather than clamped to the floor, deliberately. Clamping would + * enforce a third number that appears in neither the object's declaration nor + * the operator's settings, so nobody reading either surface could predict when + * rows actually disappear — and it would move whenever an unrelated package + * changed its floor. Rejection has exactly one fallback, the declaration, + * which is already the contract everywhere else in this file (an unparseable + * override resolves the same way: "never fail open into no bound at all"). + * The operator's intent is not silently half-honoured; it is refused, loudly, + * with the two settings that would make it legal. + * + * Registering a floor is a wiring act, so a malformed one throws here rather + * than degrading into an unactionable log line at 3am. + */ + registerRetentionFloor(object: string, floor: LifecycleRetentionFloor): void { + if (!object) throw new Error('[lifecycle] registerRetentionFloor requires an object name'); + if (floor?.policy !== 'retention' && floor?.policy !== 'ttl') { + throw new Error( + `[lifecycle] retention floor for ${object} must declare policy 'retention' or 'ttl' (got ${JSON.stringify(floor?.policy)})`, + ); + } + if (!Number.isFinite(floor.minWindowMs) || floor.minWindowMs <= 0) { + throw new Error( + `[lifecycle] retention floor for ${object} needs a positive finite minWindowMs (got ${String(floor.minWindowMs)})`, + ); + } + if (!floor.declaredBy || !floor.consequence || !floor.remedy) { + throw new Error( + `[lifecycle] retention floor for ${object} must name declaredBy, consequence and remedy — ` + + 'the rejection it produces is read by an operator who knows none of the three.', + ); + } + this.retentionFloors.set(`${object}::${floor.policy}::${floor.declaredBy}`, { ...floor, object }); + } + + /** The strictest floor registered for (object, policy) — every registrar's + * floor has to hold, so the largest one governs. */ + private floorFor( + object: string, + policy: 'retention' | 'ttl', + ): (LifecycleRetentionFloor & { object: string }) | undefined { + let strictest: (LifecycleRetentionFloor & { object: string }) | undefined; + for (const floor of this.retentionFloors.values()) { + if (floor.object !== object || floor.policy !== policy) continue; + if (!strictest || floor.minWindowMs > strictest.minWindowMs) strictest = floor; + } + return strictest; + } + /** * Apply every declared lifecycle policy once. Safe to call directly (the * dogfood growth gate and `db:clean`-style tooling do); re-entrant calls @@ -354,6 +490,7 @@ export class LifecycleService { errors: [], reclaimed: [], alerts: [], + floorViolations: [], }; if (this.sweeping || !this.enabled) return report; // [#4747] Torn down ⇒ there is no engine to sweep through, whatever the @@ -437,7 +574,10 @@ export class LifecycleService { this.opts.logger.info( `[lifecycle] sweep: ${report.swept.length} policy(ies) applied, ~${total} rows reaped, ` + `${report.reclaimed.length} datasource(s) reclaimed, ${report.errors.length} error(s), ` + - `${report.alerts.length} alert(s)`, + `${report.alerts.length} alert(s)` + + (report.floorViolations.length > 0 + ? `, ${report.floorViolations.length} window(s) overruled by a registered retention floor` + : ''), ); } return report; @@ -617,7 +757,14 @@ export class LifecycleService { const ov = this.governance.overrides[object] ?? {}; if (lc.ttl) { - const windowMs = this.effectiveWindowMs(ov.expireAfter, parseLifecycleDuration(lc.ttl.expireAfter), object); + const windowMs = this.effectiveWindowMs( + ov.expireAfter, + parseLifecycleDuration(lc.ttl.expireAfter), + object, + 'ttl', + 'global', + report, + ); outcomes.push(await this.reap(engine, object, lc, 'ttl', lc.ttl.field, windowMs, report)); } @@ -649,7 +796,14 @@ export class LifecycleService { // granularity, an explicit retention.maxAge trims to the day inside the // live shards — and immediately bounds a legacy table the Rotator just // adopted whole into its first shard. - const windowMs = this.effectiveWindowMs(ov.maxAge, parseLifecycleDuration(lc.retention.maxAge), object); + const windowMs = this.effectiveWindowMs( + ov.maxAge, + parseLifecycleDuration(lc.retention.maxAge), + object, + 'retention', + 'global', + report, + ); outcomes.push( await this.reap(engine, object, lc, 'retention', 'created_at', windowMs, report, lc.retention.onlyWhen), ); @@ -657,24 +811,126 @@ export class LifecycleService { // Rotation declared but the driver can't shard physically: the shard // window IS the bound — enforce the same window with an age-based reap // so the declaration is never inert. - const windowMs = this.effectiveWindowMs(ov.maxAge, lc.storage.shards * SHARD_UNIT_MS[lc.storage.unit], object); + const windowMs = this.effectiveWindowMs( + ov.maxAge, + lc.storage.shards * SHARD_UNIT_MS[lc.storage.unit], + object, + 'retention', + 'global', + report, + ); outcomes.push(await this.reap(engine, object, lc, 'rotation-fallback', 'created_at', windowMs, report)); } return outcomes; } - /** A governance override window beats the declared one — unless it fails to + /** + * A governance override window beats the declared one — unless it fails to * parse, in which case the declared window stands (never fail open into - * "no bound at all"). */ - private effectiveWindowMs(override: string | undefined, declaredMs: number, object: string): number { - if (!override) return declaredMs; + * "no bound at all") — or [#5195] unless it sits below a registered + * {@link LifecycleRetentionFloor}, in which case it is rejected the same way + * and for the same reason: an override that breaks a consumer's contract is + * not a shorter policy, it is an invalid one. + * + * `fallbackMs` is what an invalid override falls back to: the declared window + * at global scope, and the already-resolved global window for a tenant-scoped + * override (which has itself passed this check). + */ + private effectiveWindowMs( + override: string | undefined, + fallbackMs: number, + object: string, + policy: 'retention' | 'ttl', + scope: string, + report: LifecycleSweepReport, + ): number { + const floor = this.floorFor(object, policy); + + if (!override) { + // No override: the DECLARATION is what runs. A declaration below the + // floor is a different defect (the object and its consumer disagree at + // authoring time, which is where #5179's constructor guard catches the + // queue case) — reported here too, because a floor registered against an + // object nobody re-checked would otherwise be silently unmet. The sweep + // still runs it: refusing to reap would trade a broken consumer contract + // for an unbounded table, which is the defect #5179 just closed. + if (floor && fallbackMs < floor.minWindowMs) { + this.reportFloorViolation(report, { + object, + policy, + scope, + offendingMs: fallbackMs, + floorMs: floor.minWindowMs, + declaredBy: floor.declaredBy, + appliedMs: fallbackMs, + }, floor, 'declared'); + } + return fallbackMs; + } + + let overrideMs: number; try { - return parseLifecycleDuration(override); + overrideMs = parseLifecycleDuration(override); } catch { this.opts.logger.warn(`[lifecycle] invalid override window '${override}' for ${object}; keeping the declared window`); - return declaredMs; + return fallbackMs; + } + + if (floor && overrideMs < floor.minWindowMs) { + this.reportFloorViolation(report, { + object, + policy, + scope, + override, + offendingMs: overrideMs, + floorMs: floor.minWindowMs, + declaredBy: floor.declaredBy, + appliedMs: fallbackMs, + }, floor, 'override'); + return fallbackMs; } + + return overrideMs; + } + + /** + * Record a floor violation on the sweep report (always) and log it at + * `error` (once per distinct violation). + * + * `error`, not `warn`, by the AGENTS.md test: after this degradation the + * system looks entirely normal — the sweep reports success, the table shrinks + * on schedule — while the contract that override silently broke shows up + * later as duplicate work nobody can trace back to a settings edit. The line + * owes a consequence and a fix, and both come from the registrar rather than + * from guesswork here. + */ + private reportFloorViolation( + report: LifecycleSweepReport, + violation: LifecycleFloorViolation, + floor: LifecycleRetentionFloor, + kind: 'override' | 'declared', + ): void { + report.floorViolations.push(violation); + const dedupKey = `${violation.object}|${violation.policy}|${violation.scope}|${kind}|${violation.offendingMs}|${violation.floorMs}`; + if (this.reportedFloorViolations.has(dedupKey)) return; + this.reportedFloorViolations.add(dedupKey); + + const where = violation.scope === 'global' ? 'global scope' : violation.scope; + const head = + kind === 'override' + ? `[lifecycle] REJECTED the ${violation.policy} override '${violation.override}' (${violation.offendingMs}ms) on ` + + `${violation.object} at ${where}: it is below the ${violation.floorMs}ms floor registered by ` + + `'${floor.declaredBy}'. Enforcing the declared ${violation.appliedMs}ms window instead.` + : `[lifecycle] ${violation.object}'s declared ${violation.policy} window (${violation.offendingMs}ms) is below ` + + `the ${violation.floorMs}ms floor registered by '${floor.declaredBy}'; it is still being enforced as declared.`; + this.logError(`${head} Consequence: ${floor.consequence} Fix: ${floor.remedy}`); + } + + /** `error` where the logger has one; a duck-typed test double may not. */ + private logError(msg: string): void { + if (typeof this.opts.logger.error === 'function') this.opts.logger.error(msg); + else this.opts.logger.warn(msg); } /** @@ -785,7 +1041,16 @@ export class LifecycleService { // Tenant-level windows (P4): each overriding tenant gets its own // cutoff on its own rows… for (const t of tenantWindows) { - const tMs = this.effectiveWindowMs(t[overrideKey], windowMs, `${object} (tenant ${t.tenantId})`); + // [#5195] Tenant-scoped overrides go through the same floor: a + // per-tenant `maxAge: '1h'` is the identical side door, one scope down. + const tMs = this.effectiveWindowMs( + t[overrideKey], + windowMs, + object, + policy === 'ttl' ? 'ttl' : 'retention', + `tenant:${t.tenantId}`, + report, + ); const tCutoff = new Date(this.now() - tMs).toISOString(); accumulate(await reapWhere({ [field]: { $lt: tCutoff }, organization_id: t.tenantId, ...scope })); } diff --git a/packages/objectql/src/lifecycle/lifecycle-settings.ts b/packages/objectql/src/lifecycle/lifecycle-settings.ts index ecc33c0f27..d70335b7c7 100644 --- a/packages/objectql/src/lifecycle/lifecycle-settings.ts +++ b/packages/objectql/src/lifecycle/lifecycle-settings.ts @@ -44,7 +44,9 @@ export const lifecycleSettingsManifest = { default: {}, description: 'Per-object window overrides: { "": { "maxAge": "1y", "expireAfter": "30d" } }. ' + - 'Duration literals: h/d/w/y. Tenant-scoped — a regulated tenant sets years while dev keeps days (ADR-0057 §3.2).', + 'Duration literals: h/d/w/y. Tenant-scoped — a regulated tenant sets years while dev keeps days (ADR-0057 §3.2). ' + + 'An override BELOW a retention floor a consumer registered (e.g. the job queue\'s dedup window) is rejected at ' + + 'sweep time and logged at error — the declared window keeps running (#5195).', }, { type: 'json', diff --git a/packages/services/service-queue/README.md b/packages/services/service-queue/README.md index 720b32c67e..bb3244761d 100644 --- a/packages/services/service-queue/README.md +++ b/packages/services/service-queue/README.md @@ -90,8 +90,14 @@ Two consequences worth knowing: setting would start accepting duplicates the moment the row was swept. The `db` adapter throws at construction instead of degrading quietly. - **The window is overridable per environment** through the `lifecycle` - settings namespace (`maxAge` per object), like every other ADR-0057 policy. - Keep it ≥ your idempotency window. + settings namespace (`maxAge` per object), like every other ADR-0057 policy — + but **not below the idempotency window**. On startup this plugin registers a + *retention floor* with the `LifecycleService` carrying the window the adapter + was actually constructed with; a global or tenant-scoped override under it is + **rejected** at sweep time (the declared window keeps running) and logged at + `error` naming the consequence and the two settings that would make it legal. + So the ordering is enforced from both sides: the constructor rejects a + too-long `idempotencyWindowMs`, the floor rejects a too-short `maxAge`. ## Service API diff --git a/packages/services/service-queue/src/db-queue-adapter.ts b/packages/services/service-queue/src/db-queue-adapter.ts index 1afc51cbf6..7fad738054 100644 --- a/packages/services/service-queue/src/db-queue-adapter.ts +++ b/packages/services/service-queue/src/db-queue-adapter.ts @@ -44,6 +44,22 @@ export function completedRetentionWindowMs(): number { return lifecycleDurationMs(maxAge); } +/** + * [#5195] The shape `LifecycleService.registerRetentionFloor()` accepts. + * + * Restated here rather than imported: `@objectstack/objectql` is a + * devDependency of this package on purpose (the queue must not drag the engine + * into every install), and the registration is duck-typed at the call site the + * same way the storage service's reap guards are. + */ +export interface QueueRetentionFloor { + policy: 'retention'; + minWindowMs: number; + declaredBy: string; + consequence: string; + remedy: string; +} + export interface DbQueueAdapterOptions { /** Polling interval for the worker loop (ms, default 1000) */ pollIntervalMs?: number; @@ -147,6 +163,48 @@ export class DbQueueAdapter implements IQueueService { } } + /** The configured dedup window (ms) — the number the floor below is made of. */ + get idempotencyWindowMs(): number { + return this.opts.idempotencyWindowMs; + } + + /** + * [#5195] The retention floor `sys_job_queue` must satisfy for this adapter's + * dedup contract to mean anything, handed to `LifecycleService` + * (`registerRetentionFloor`) by `QueueServicePlugin`. + * + * The constructor check above only reads the object's **declaration**. ADR-0057 + * P4 lets an operator override that window per environment/tenant through the + * `lifecycle` settings namespace, which the constructor cannot see: set + * `lifecycle.retention_overrides.sys_job_queue.maxAge = '1h'` and completed + * rows vanish an hour after they are written while publish keeps dedupping + * against a 24h window — duplicate deliveries resume, with nothing in any log. + * Registering the floor is what closes that door, and it carries the number + * this adapter was actually CONSTRUCTED with rather than a static copy of the + * default (a per-kernel option cannot live in the object's declaration). + */ + retentionFloor(): QueueRetentionFloor { + const ms = this.opts.idempotencyWindowMs; + // Settings are authored as ADR-0057 duration literals, not milliseconds, so + // the remedy quotes one the operator can paste — rounded UP, since a + // rounded-down literal would be rejected by the very floor it is meant to + // satisfy. + const literal = `${Math.ceil(ms / 3_600_000)}h`; + return { + policy: 'retention', + minWindowMs: ms, + declaredBy: 'com.objectstack.service.queue', + consequence: + `DbQueueAdapter dedups sys_job_queue publishes by comparing created_at against its ${ms}ms ` + + 'idempotency window, so a shorter retention deletes the very rows that check reads — ' + + 'duplicate deliveries resume silently, with nothing in any log.', + remedy: + `set lifecycle.retention_overrides.sys_job_queue.maxAge to '${literal}' or longer, or lower ` + + "QueueServicePlugin's db.idempotencyWindowMs to the window you actually want (both are measured " + + 'from created_at).', + }; + } + // ── IQueueService ──────────────────────────────────────────────── async publish( diff --git a/packages/services/service-queue/src/job-queue-retention.test.ts b/packages/services/service-queue/src/job-queue-retention.test.ts index ebb5a7459b..11b291c239 100644 --- a/packages/services/service-queue/src/job-queue-retention.test.ts +++ b/packages/services/service-queue/src/job-queue-retention.test.ts @@ -26,9 +26,10 @@ // the engine. import { describe, it, expect, beforeEach } from 'vitest'; -import { assertEngineDeleteDispatch } from '@objectstack/objectql'; +import { assertEngineDeleteDispatch, LifecycleService } from '@objectstack/objectql'; import { SysJobQueue } from '@objectstack/platform-objects/audit'; import { DbQueueAdapter, completedRetentionWindowMs } from './db-queue-adapter.js'; +import { QueueServicePlugin } from './queue-service-plugin.js'; import { lifecycleDurationMs } from './common.js'; const QUEUE_TABLE = 'sys_job_queue'; @@ -65,6 +66,12 @@ function makeFakeEngine() { rows(): Row[] { return tables.get(QUEUE_TABLE) ?? []; }, + // [#5195] The two members `LifecycleService` needs to sweep through this + // fake for real, so the floor can be proven end-to-end rather than + // re-mirrored: the object set it iterates, and the driver hook it consults + // for space reclaim (none here). + registry: { getAllObjects: () => [SysJobQueue as any] }, + getDriverForObject: () => undefined, async find(table: string, opts: any = {}) { const t = tables.get(table) ?? []; let out = opts.where ? t.filter((r) => matches(r, opts.where)) : [...t]; @@ -309,3 +316,215 @@ describe('declared retention bounds sys_job_queue (#5179)', () => { expect(engine.rows()[0]!.status).toBe('completed'); }); }); + +// #5195 — the invariant above is enforced against the object's DECLARATION, +// which the adapter's constructor can read. ADR-0057 P4 also lets an operator +// override that window at runtime through the `lifecycle` settings namespace, +// and the constructor cannot see that at all: set +// `retention_overrides.sys_job_queue.maxAge = '1h'` and completed rows are +// reaped an hour after they are written while publish keeps dedupping against +// 24h — duplicate deliveries resume, silently. +// +// These run the REAL `LifecycleService` over the REAL `SysJobQueue` +// declaration, so what is pinned is the two packages agreeing, not this file's +// idea of either. +describe('a lifecycle settings override cannot undercut the dedup window (#5195)', () => { + const OVERRIDE_1H = { retention_overrides: { [QUEUE_TABLE]: { maxAge: '1h' } } }; + + function fakeSettings(values: Record) { + return { + async get(_ns: string, key: string) { + return key in values + ? { value: values[key], source: 'global' } + : { value: undefined, source: 'default' }; + }, + }; + } + + function lifecycle( + engine: ReturnType, + clock: ReturnType, + settings: unknown, + logger: { info(m: string): void; warn(m: string): void; error(m: string): void }, + ) { + return new LifecycleService({ + getEngine: () => engine as any, + logger, + now: () => clock.ms, + getSettings: () => settings as any, + }); + } + + function collectingLogger() { + const errors: string[] = []; + return { + errors, + info: () => {}, + warn: () => {}, + error: (m: string) => { errors.push(m); }, + }; + } + + /** Publish → deliver → age past the override window, then sweep. */ + async function ageACompletedMessage( + engine: ReturnType, + clock: ReturnType, + ): Promise<{ adapter: DbQueueAdapter; firstId: string }> { + const adapter = new DbQueueAdapter({ + engine, + clock, + options: { autoStart: false, pollIntervalMs: 60_000 }, + }); + await adapter.subscribe('billing', async () => { /* delivered */ }); + const firstId = await adapter.publish('billing', { invoice: 7 }, { idempotencyKey: 'inv-7' }); + await adapter.pollOnce(); + expect(engine.rows()[0]!.status).toBe('completed'); + // Past the 1h override, far inside both the 7d declaration and the 24h + // dedup window: the exact gap #5195 is about. + clock.advance(2 * HOUR); + return { adapter, firstId }; + } + + it('REPRODUCES the bypass when no floor is registered: the row is reaped and the duplicate lands', async () => { + const engine = makeFakeEngine(); + const clock = makeClock(Date.parse('2026-03-01T00:00:00.000Z')); + const { adapter, firstId } = await ageACompletedMessage(engine, clock); + + await lifecycle(engine, clock, fakeSettings(OVERRIDE_1H), collectingLogger()).sweep(); + + // Nothing kept the reaper off the row the dedup check reads… + expect(engine.rows()).toHaveLength(0); + // …so the same key publishes again, 2 hours into a 24 hour window. + const again = await adapter.publish('billing', { invoice: 7 }, { idempotencyKey: 'inv-7' }); + expect(again).not.toBe(firstId); + expect(engine.rows()).toHaveLength(1); + }); + + it('rejects the override once the adapter has registered its floor — dedup keeps holding', async () => { + const engine = makeFakeEngine(); + const clock = makeClock(Date.parse('2026-03-01T00:00:00.000Z')); + const { adapter, firstId } = await ageACompletedMessage(engine, clock); + const logger = collectingLogger(); + + const svc = lifecycle(engine, clock, fakeSettings(OVERRIDE_1H), logger); + svc.registerRetentionFloor(QUEUE_TABLE, adapter.retentionFloor()); + const report = await svc.sweep(); + + // The declared 7d window runs instead of the 1h override… + expect(engine.rows()).toHaveLength(1); + expect(report.swept[0]!.cutoff).toBe(new Date(clock.ms - 7 * DAY).toISOString()); + // …and the re-publish is still deduplicated to the original message. + const again = await adapter.publish('billing', { invoice: 7 }, { idempotencyKey: 'inv-7' }); + expect(again).toBe(firstId); + expect(engine.rows()).toHaveLength(1); + + // Loud, with the consequence and both fixes an operator needs. + expect(report.floorViolations).toHaveLength(1); + expect(report.floorViolations[0]).toMatchObject({ + object: QUEUE_TABLE, + policy: 'retention', + scope: 'global', + override: '1h', + floorMs: DEFAULT_IDEMPOTENCY_WINDOW_MS, + declaredBy: 'com.objectstack.service.queue', + }); + expect(logger.errors).toHaveLength(1); + expect(logger.errors[0]).toContain('duplicate deliveries resume silently'); + expect(logger.errors[0]).toContain('lifecycle.retention_overrides.sys_job_queue.maxAge'); + expect(logger.errors[0]).toContain('idempotencyWindowMs'); + }); + + it('a legal override (≥ the dedup window) still takes effect', async () => { + const engine = makeFakeEngine(); + const clock = makeClock(Date.parse('2026-03-01T00:00:00.000Z')); + const adapter = new DbQueueAdapter({ engine, clock, options: { autoStart: false } }); + // 2d: shorter than the declared 7d, longer than the 24h dedup window. + const svc = lifecycle( + engine, + clock, + fakeSettings({ retention_overrides: { [QUEUE_TABLE]: { maxAge: '2d' } } }), + collectingLogger(), + ); + svc.registerRetentionFloor(QUEUE_TABLE, adapter.retentionFloor()); + + const report = await svc.sweep(); + + expect(report.floorViolations).toEqual([]); + expect(report.swept[0]!.cutoff).toBe(new Date(clock.ms - 2 * DAY).toISOString()); + }); + + it('the floor carries the CONFIGURED idempotency window, not the default', () => { + const adapter = new DbQueueAdapter({ + engine: makeFakeEngine(), + options: { autoStart: false, idempotencyWindowMs: 3 * DAY }, + }); + // A static key on the object declaration could never say this — the number + // is a per-kernel construction option. + expect(adapter.idempotencyWindowMs).toBe(3 * DAY); + expect(adapter.retentionFloor()).toMatchObject({ + policy: 'retention', + minWindowMs: 3 * DAY, + declaredBy: 'com.objectstack.service.queue', + }); + + const engine = makeFakeEngine(); + const clock = makeClock(Date.parse('2026-03-01T00:00:00.000Z')); + const svc = lifecycle( + engine, + clock, + fakeSettings({ retention_overrides: { [QUEUE_TABLE]: { maxAge: '2d' } } }), + collectingLogger(), + ); + svc.registerRetentionFloor(QUEUE_TABLE, adapter.retentionFloor()); + // 2d clears the 24h default but NOT this adapter's 3d window. + return svc.sweep().then((report) => { + expect(report.floorViolations).toHaveLength(1); + expect(report.floorViolations[0]!.floorMs).toBe(3 * DAY); + }); + }); + + it('QueueServicePlugin registers the floor at kernel:ready (the wiring, not just the ability)', async () => { + const engine = makeFakeEngine(); + const clock = makeClock(Date.parse('2026-03-01T00:00:00.000Z')); + const svc = lifecycle(engine, clock, fakeSettings(OVERRIDE_1H), collectingLogger()); + + const readyHooks: Array<() => Promise> = []; + const services = new Map([ + ['manifest', { register: () => {} }], + ['objectql', engine], + ['lifecycle', svc], + ]); + const ctx: any = { + logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }, + getService: (name: string) => { + if (!services.has(name)) throw new Error(`no service '${name}'`); + return services.get(name); + }, + registerService: (name: string, s: unknown) => { services.set(name, s); }, + replaceService: (name: string, s: unknown) => { services.set(name, s); }, + hook: (name: string, fn: () => Promise) => { + if (name === 'kernel:ready') readyHooks.push(fn); + }, + }; + + const plugin = new QueueServicePlugin({ adapter: 'db', db: { pollIntervalMs: 60_000 } }); + await plugin.init(ctx); + for (const fn of readyHooks) await fn(); + await plugin.destroy(); + + // An unswept-by-1h row is the observable proof the plugin did the wiring — + // the ability to register a floor is worth nothing if nobody calls it. + engine.tables.set(QUEUE_TABLE, [{ + id: 'm_old', queue: 'q', status: 'completed', payload_json: '{}', + created_at: new Date(clock.ms - 3 * HOUR).toISOString(), + }]); + const report = await svc.sweep(); + + expect(engine.rows()).toHaveLength(1); + expect(report.floorViolations[0]).toMatchObject({ + object: QUEUE_TABLE, + floorMs: DEFAULT_IDEMPOTENCY_WINDOW_MS, + declaredBy: 'com.objectstack.service.queue', + }); + }); +}); diff --git a/packages/services/service-queue/src/queue-service-plugin.ts b/packages/services/service-queue/src/queue-service-plugin.ts index d90c346623..9e771ebe01 100644 --- a/packages/services/service-queue/src/queue-service-plugin.ts +++ b/packages/services/service-queue/src/queue-service-plugin.ts @@ -107,6 +107,17 @@ export class QueueServicePlugin implements Plugin { options: this.options.db, }); + // [#5195] Tell the LifecycleService how short sys_job_queue's retention + // may get. The adapter's constructor already refuses an idempotency + // window longer than the DECLARED retention (#5179), but ADR-0057 P4 + // overrides live in the `lifecycle` settings namespace, which the + // constructor never sees — an operator setting `maxAge: '1h'` would reap + // the rows publish dedups against and duplicate deliveries would resume + // silently. The floor carries the window this adapter was actually + // constructed with, so a non-default `db.idempotencyWindowMs` is covered + // too. + this.registerRetentionFloor(ctx, this.dbAdapter); + try { (ctx as any).replaceService?.('queue', this.dbAdapter); this.dbAdapter.start(); @@ -117,6 +128,39 @@ export class QueueServicePlugin implements Plugin { }); } + /** + * [#5195] Register the adapter's retention floor with the platform + * LifecycleService. Duck-typed and best-effort, exactly like the storage + * service's reap guards: a kernel without a lifecycle service has no sweeper + * either, so there is no override for anything to bypass. + */ + private registerRetentionFloor(ctx: PluginContext, adapter: DbQueueAdapter): void { + let lifecycle: any; + try { + lifecycle = ctx.getService('lifecycle'); + } catch { + lifecycle = undefined; + } + if (!lifecycle || typeof lifecycle.registerRetentionFloor !== 'function') return; + try { + lifecycle.registerRetentionFloor(SysJobQueue.name, adapter.retentionFloor()); + ctx.logger.info( + `QueueServicePlugin: registered a ${adapter.idempotencyWindowMs}ms retention floor on ${SysJobQueue.name} ` + + 'with the lifecycle service (settings overrides below it are rejected)', + ); + } catch (err) { + // A floor the service refused is a wiring bug in THIS plugin, not a + // degraded deployment — but it must not stop the queue from coming up. + ctx.logger.error( + 'QueueServicePlugin: the lifecycle service rejected the sys_job_queue retention floor. A `lifecycle` ' + + 'settings override may now shorten sys_job_queue.retention below the idempotency window, in which case ' + + 'publish would silently re-accept duplicates (#5195). Fix the floor registration, or keep ' + + 'lifecycle.retention_overrides.sys_job_queue unset.', + err as any, + ); + } + } + async destroy(): Promise { await this.dbAdapter?.stop(); } diff --git a/scripts/adr-anchors.json b/scripts/adr-anchors.json index 64ad269944..85804e1a74 100644 --- a/scripts/adr-anchors.json +++ b/scripts/adr-anchors.json @@ -1,6 +1,11 @@ { "//": "ADR anchors — see scripts/check-adr-anchors.mjs. Each entry pins the ADR ids that MUST stay referenced in a file whose behaviour an accepted ADR decided. Add an entry when an ADR's decision is realized in code that would look arbitrary (or wrong) to someone reading the file alone.", "anchors": [ + { + "file": "packages/objectql/src/lifecycle/lifecycle-service.ts", + "adrs": ["ADR-0057"], + "invariant": "ADR-0057 P4 settings overrides are bounded from below by consumer-registered retention floors (§3.3 amendment, #5195). An override under a floor — global or tenant-scoped — is REJECTED, never clamped to the floor: the declared window is the one fallback, the same resolution an unparseable override already gets, and a clamp would enforce a third number written in neither the declaration nor the settings. Floors are runtime wiring, not spec surface, for the reap-guard reason plus one of their own — the first floor IS `DbQueueAdapterOptions.idempotencyWindowMs`, a per-kernel construction option a static `lifecycle` key could only copy and drift from. The rejection is `error`-level with consequence and fix because what it prevents (duplicate delivery on a reaped dedup row) leaves the system looking healthy." + }, { "file": "packages/objectql/src/validation/rule-validator.ts", "adrs": ["ADR-0057", "ADR-0058"], From 97edc20cbab04367a4588dd6f866e4814919cfb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 10:29:26 +0000 Subject: [PATCH 2/2] fix(service-queue): type the `lifecycle` slot lookup instead of erasing it to `any` (#5195) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The floor registration added two `getService`-erasure sites to queue-service-plugin.ts (`let lifecycle: any` and `getService('lifecycle')`), growing the file's `check:slot-lookup` ratchet count 4 → 6. That file is grandfathered for its EXISTING sites only, and the baseline never grows. Fixed at the call site rather than by touching the baseline or adding an exemption: `LifecycleFloorRegistrar` declares the one method this package calls on the slot, so the registration is type-checked. That is not ratchet appeasement — `any` on this particular call is the worst place in the change to have it: a renamed or re-ordered `registerRetentionFloor` would compile, then throw at runtime inside the `try` that logs and continues, leaving the floor silently unregistered. That is exactly the silent bypass #5195 exists to close, reintroduced one layer up. `registerRetentionFloor` is optional on the interface on purpose: a kernel may carry a lifecycle service predating floors, so the runtime `typeof … === 'function'` probe is a real check and the type now says so, instead of an `any` hiding both the check and the call. Verified: `check:slot-lookup` back to 159 unswept sites, none new. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017MCKJaEomEqg4tvz4SzdNd --- .../service-queue/src/db-queue-adapter.ts | 25 +++++++++++++++++-- .../service-queue/src/queue-service-plugin.ts | 17 ++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/services/service-queue/src/db-queue-adapter.ts b/packages/services/service-queue/src/db-queue-adapter.ts index 7fad738054..1acc3816bb 100644 --- a/packages/services/service-queue/src/db-queue-adapter.ts +++ b/packages/services/service-queue/src/db-queue-adapter.ts @@ -49,8 +49,8 @@ export function completedRetentionWindowMs(): number { * * Restated here rather than imported: `@objectstack/objectql` is a * devDependency of this package on purpose (the queue must not drag the engine - * into every install), and the registration is duck-typed at the call site the - * same way the storage service's reap guards are. + * into every install), so its types are not available to this package's + * consumers at build time. */ export interface QueueRetentionFloor { policy: 'retention'; @@ -60,6 +60,27 @@ export interface QueueRetentionFloor { remedy: string; } +/** + * [#5195] The `lifecycle` slot's contract as THIS package consumes it — the one + * method `QueueServicePlugin` calls, and nothing else. + * + * Declared rather than erased to `any` at the lookup (#4127/#4251): `any` would + * switch off checking on the single call that carries the floor, so a rename or + * a changed argument order in `LifecycleService.registerRetentionFloor` would + * compile here and fail at runtime inside a `try` that logs and continues — + * i.e. the floor would silently not exist, which is precisely the silent + * bypass #5195 exists to close. + * + * `registerRetentionFloor` is **optional** on purpose, and that optionality is + * the honest part of the contract: a kernel may carry a lifecycle service that + * predates floors, so the runtime `typeof … === 'function'` probe below is a + * real check and the type says so, instead of an `any` that hides both the + * check and the call. + */ +export interface LifecycleFloorRegistrar { + registerRetentionFloor?(object: string, floor: QueueRetentionFloor): void; +} + export interface DbQueueAdapterOptions { /** Polling interval for the worker loop (ms, default 1000) */ pollIntervalMs?: number; diff --git a/packages/services/service-queue/src/queue-service-plugin.ts b/packages/services/service-queue/src/queue-service-plugin.ts index 9e771ebe01..86c742ad68 100644 --- a/packages/services/service-queue/src/queue-service-plugin.ts +++ b/packages/services/service-queue/src/queue-service-plugin.ts @@ -5,7 +5,7 @@ import { SysJobQueue } from '@objectstack/platform-objects/audit'; import { MemoryQueueAdapter } from './memory-queue-adapter.js'; import type { MemoryQueueAdapterOptions } from './memory-queue-adapter.js'; import { DbQueueAdapter } from './db-queue-adapter.js'; -import type { DbQueueAdapterOptions } from './db-queue-adapter.js'; +import type { DbQueueAdapterOptions, LifecycleFloorRegistrar } from './db-queue-adapter.js'; /** * Configuration options for the QueueServicePlugin. @@ -130,14 +130,19 @@ export class QueueServicePlugin implements Plugin { /** * [#5195] Register the adapter's retention floor with the platform - * LifecycleService. Duck-typed and best-effort, exactly like the storage - * service's reap guards: a kernel without a lifecycle service has no sweeper - * either, so there is no override for anything to bypass. + * LifecycleService. Best-effort: a kernel without a lifecycle service has no + * sweeper either, so there is no override for anything to bypass. + * + * The lookup is typed to {@link LifecycleFloorRegistrar} — the slot's + * contract as this package consumes it — rather than erased to `any` + * (#4127/#4251). The `typeof … === 'function'` probe stays because the method + * is genuinely optional (a lifecycle service predating floors), but it is now + * a check the compiler can see rather than one `any` was hiding. */ private registerRetentionFloor(ctx: PluginContext, adapter: DbQueueAdapter): void { - let lifecycle: any; + let lifecycle: LifecycleFloorRegistrar | undefined; try { - lifecycle = ctx.getService('lifecycle'); + lifecycle = ctx.getService('lifecycle'); } catch { lifecycle = undefined; }