Skip to content

Commit e7fea46

Browse files
claude[bot]claude
andauthored
fix(service-messaging): sys_notification_delivery reaps its terminal-failure rows after 7d, not 90d (#17611) (#17871)
Fixes #17611 Implements ruling **C** (director seat, decision batch #116 item 3, comment 5642380896; maintainer verbatim and untranslated: 「17611 同意」 to 「席位推荐 C 现在做,A 另立一卡;若要逐条审计选 C+B」). **B is not taken. A is not taken here** — it is priced on its own card, #17732, whose own ruling orders it second. Nothing in this PR touches `channel.ts`, `messaging-service.ts` or fan-out. ## What was wrong Fan-out writes one delivery row per `(event × recipient × channel)`. A tenant with no transport configured for one of those channels dead-letters that channel's row on its **first** attempt, and every `notify` writes another one. Production reading from the card: 2,876 `email`/`dead` rows against 2,876 `inbox`/`success`, `max(attempts) = 1`, zero pending, +316 rows/day. Those rows carry no work — nothing claims, retries or acks them again — yet they sat for the full 90d in the table the dispatcher's claim query reads on every hop. ## The change One declaration, on `packages/services/service-messaging/src/objects/notification-delivery.object.ts`: ```ts lifecycle: { class: 'telemetry', ttl: { field: 'created_at', expireAfter: '90d' }, retention: { maxAge: '7d', onlyWhen: { status: { $in: ['dead', 'suppressed'] } }, }, }, ``` Zero new contract, as the ruling requires: no `MessagingChannel` member, no new status value, no change to fan-out. `retention.onlyWhen` is an existing spec key with a live consumer and three landed precedents (`sys_job_queue`, `sys_automation_run`, `sys_upload_session`). ### One deviation from the literal one-key reading, and why the ruling's own words require it The ruling says the terminal scope lands "through the existing `retention.onlyWhen` key ... **while non-terminal rows keep the object's 90d**". Measured on the spec: `lifecycle.retention` is a **single optional block** (`packages/spec/src/data/object.zod.ts`), not a list. So `retention: { maxAge: '7d', onlyWhen: terminal }` **alone** would have left `pending` / `in_flight` / `success` with **no age bound at all** — unbounding the larger half of this table's growth (the card's own table shows 2,876 `success` rows at the same +316/day) on the very card that exists to bound it, and dropping a bound the object ships today. So the 7d scope sits **under** the table window rather than replacing it: the `ttl` leg restates the 90d the object has always declared, on the same `created_at` clock `retention` reaps by. Non-terminal rows keep exactly today's behaviour. This is still zero new contract and still one file: `retention` + `ttl` on one object is a landed shape with two in-repo precedents — `system-upload-session.object.ts:143-144` and `system-file.object.ts:180-181`, both of which likewise put the **short, scoped** window on `retention` and the other window on `ttl`. Both legs run: `LifecycleService.reapObject` (`packages/objectql/src/lifecycle/lifecycle-service.ts:1022` and `:1083`) takes `ttl` and `retention` in **independent** `if` branches, not an either/or. `retention`'s field is hard-coded `created_at` at `:1097`, which is what makes the two windows commensurable. ## Zone-2 assumptions, as measured | # | PM's assumption | Verdict | |---|---|---| | 1 | 90d `maxAge`, no `onlyWhen` on the delivery object | **HOLDS.** Key-position pattern `(^ or non-alpha)onlyWhen[space]*:` returns 0 in that file; positive control `sys-automation-run.object.ts:64` returns the `$in` shape; `readonlyWhen` negative control separates cleanly (1 hit repo-wide, elsewhere) | | 2 | File face = the object + its new test | **HELD**, plus the mandatory changeset and one generated ledger row (below) | | 3 | Terminal set is `dead` / `suppressed` | **HOLDS, with one measured note** — see below | | 4 | `retention.onlyWhen` is consumed by the Reaper at `lifecycle-service.ts:1500/1510` | **HOLDS.** `reap()` builds `scope = onlyWhen ?? {}` and spreads it into `reapWhere({ [field]: { $lt: cutoff }, ...scope })`. Not edited; exercised end to end by the new suite | | 5 | `Clause-②: no` | **HOLDS.** No published interface widened; the two keys used are already in the spec and already consumed | | 6 | Object last moved at `e4902d2b9`, nothing touching retention | **HOLDS** | ### Assumption 3, reconciled The `status` field declares **six** members (`notification-delivery.object.ts:98`): `pending`, `in_flight`, `success`, `failed`, `dead`, `suppressed`. The ack paths that write this object — `SqlNotificationOutbox.ack` (`sql-outbox.ts:264-275`) and `MemoryNotificationOutbox.ack` (`memory-outbox.ts:164-177`) — produce exactly `success | suppressed | dead | pending`, and `claim()` writes `in_flight`. ⇒ **`failed` is a declared-but-unwritten member for THIS object.** It is `sys_http_delivery`'s terminal status (`http-outbox.ts`, `sql-http-outbox.ts`, `objects/http-delivery.object.ts:66`), a different object in the same package. Naming it in the sweep scope would scope on a value that cannot occur. So the scope is the ruling's two, and no more — **not silently widened, not silently narrowed**. The object comment carries that reconciliation and the `sys_automation_run` #15223 lesson verbatim in intent: ⛔ widen this `$in` in the same change as a writer, because a widened writer against a narrow sweep scope means the new status is simply never aged out, silently, forever. `success` is terminal in the ordinary sense and is deliberately **outside** the scope — the ruling keeps delivery history at the table window, and the acceptance names `success` as a row that must survive. ## Tests `packages/services/service-messaging/src/notification-delivery-retention-sweep.test.ts` — the REAL declaration through the REAL provisioning pass (`applySystemFields`) through the REAL Reaper (`LifecycleService`) against a REAL SQL backend (live better-sqlite3), over a table the driver created from that same declaration. Same doctrine as the two sibling sweeps, `plugin-auth/sys-session-ttl-sweep.test.ts` and `service-storage/sys-upload-session-ttl-sweep.test.ts`. Seven rows, one sweep, four verdicts: | row | status | age | expected | |---|---|---|---| | `d_dead_30d` | `dead` | 30d | reaped | | `d_suppressed_30d` | `suppressed` | 30d | reaped | | `d_dead_2d` | `dead` | 2d | **kept** — inside the 7d window | | `d_success_30d` | `success` | 30d | **kept** | | `d_pending_30d` | `pending` | 30d | **kept** | | `d_in_flight_30d` | `in_flight` | 30d | **kept** | | `d_success_100d` | `success` | 100d | reaped — by the `ttl` leg | `d_dead_30d` and `d_success_30d` carry the **identical** `created_at`, so no age rule can separate their fates — only the `onlyWhen` status filter can. `d_dead_2d` is terminal but inside the window, so "everything terminal is reaped" fails too. `d_success_100d` is the regression guard on the table bound: it is what distinguishes "the 90d window was kept" from "it was traded away". ``` Test Files 1 passed (1) Tests 9 passed (9) ``` Package suite, `pnpm --filter @objectstack/service-messaging exec vitest run --maxWorkers=2`: ``` Test Files 41 passed (41) Tests 432 passed (432) ``` `pnpm --filter @objectstack/service-messaging typecheck` — exit 0, and `tsc --noEmit --listFiles` confirms the new test file is **in** the program (so the typecheck genuinely covers it, rather than excluding `**/*.test.ts` and reporting green about a file it never read). ## Ablation — the negative pin is not vacuous This lane's standing commitment: every negative pin gets an ablation. The mutation was applied to the **shipped file on disk**, after the fix was committed, under a restoring `trap ... EXIT INT TERM` with absolute paths. Mutation: delete the `onlyWhen` scoping line, keep `maxAge: '7d'`. On-disk proof, not an editor's exit code: ``` HEAD_BLOB = 1a4df87 DISK_BLOB_BEFORE = 1a4df87 (precondition: tree at HEAD) ANCHOR_COUNT_BEFORE = 1 ANCHOR_COUNT_AFTER = 0 DISK_BLOB_AFTER = acb0ec5e5c768bf847ba6e54a8334b08d69cec2d ``` No rebuild step is claimed and none is needed: the declaration under test is **this package's own source**, imported relatively (`./objects/notification-delivery.object.js`), so vitest reads `src` and not a dependency's `dist`. That the src mutation reddened the suite with no rebuild is itself the resolution evidence — had the subject resolved through a built artifact, the ablation would have stayed green and certified nothing. Result — **red, in the predicted direction** (4 of 9, and precisely the 4 that depend on the scoping): ``` ❯ src/notification-delivery-retention-sweep.test.ts (9 tests | 4 failed) × is exactly the ruled declaration × scopes on statuses the field declares AND the ack paths really write × NEGATIVE — pending / success / in_flight rows of the SAME age are untouched × one sweep, seven rows, the whole verdict in one assertion Test Files 1 failed (1) Tests 4 failed | 5 passed (9) ``` The load-bearing one is the behavioural failure, not the declaration `toEqual`: ``` AssertionError: expected [ 'd_dead_2d' ] to include 'd_success_30d' ``` Without `onlyWhen`, the same 7d retention takes every 30d row whatever its status — the non-terminal rows included. That is exactly the outcome the negative pins discriminate against, so they are measuring the filter and not the sweep's mere existence. The five that stayed green are the ones that should: the two positive controls, the 7d-window control, the `ttl` leg, and the two in-file counterfactuals (which supply their own declaration and are therefore insensitive to the file). Restore, verified by state and not by an exit code: ``` git diff HEAD -> empty git status --porcelain -> empty git hash-object OBJECT_PATH -> 1a4df87 (== HEAD blob) ``` ## Gates Derived from the real change set with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (re-derived after the ledger row landed, which added 8 families), then reconciled with `--ran` carrying exit codes: ``` Run reconciliation — 70 derived, 67 run, 3 NOT-MEASURED, 0 UNRUN. ``` **67 green.** The 3 NOT MEASURED all exit **3**, the code these gates use for `PREREQUISITE NOT MET` — each refuses to run without a whole-repo build closure, which is CI's `Build Core` / `TypeScript Type Check` run and not a per-card one: - `pnpm check:dual-build-cjs-loads` — "this gate reads built output, and some package has no dist/" (55 packages listed) - `pnpm check:i18n` — needs the CLI plus the build closure of all ten extract-config packages. ⚠️ Read as NOT MEASURED, though this diff moves **no** i18n source key: it adds comments and a `lifecycle` block, and touches no `label`, `pluralLabel` or `description` - `pnpm check:type-check-debt` — "17 workspace dependencies ... have no built type entry point on disk" ⛔ None of these three is a red, and none is a pass. Recorded as NOT MEASURED with the reason, per the gates' own instruction that exit 3 "is NOT a pass: nothing was measured". Repo-wide lint, run whole rather than narrowed — `pnpm exec eslint . --no-inline-config --format json`, exit code captured before any pipe, at `a6c8cd7f5`: ``` ESLINT_EXIT=0 FILES_LINTED=6657 ERRORS=0 WARNINGS=0 ``` One generated file is in the diff: `scripts/engine-double-contract.pinned.json` gains one row for the new suite's engine double, written by `node scripts/check-engine-double-contract.mjs --write` ("789 rows, 1 added or grown, 0 lost"). The double opens with `assertEngineDeleteDispatch(options)`, so it refuses exactly what the real engine refuses rather than re-deriving the rule. ## Changeset `.changeset/17611-terminal-delivery-retention.md`, `minor` on `@objectstack/service-messaging`. It states the operational consequence in the first paragraph, as acceptance item 2 requires: **dead and suppressed delivery rows are now deleted 7 days after creation, so any report or SLA reading that consulted them must read inside that window.** It also flags a second consequence the ruling did not name but the change creates: an operator override of `retention_overrides.maxAge` for this object used to move the whole table's window and now moves the terminal-failure window only. ## ⚠️ Acceptance item 3 — the carried confidence gap, stated and NOT closed Per the ruling's own instruction, this PR states the seat's confidence gap ① rather than closing it: **Nothing on this card proves the reporting deployment runs the lifecycle Reaper at all.** `OS_LIFECYCLE_DISABLED=1` or the plugin switch turns it off. The card's own arithmetic is consistent with either world: 2,876 rows at +316/day is about **nine days**, far short of any 90d window, so that reading can neither confirm nor falsify that the Reaper is sweeping there. If that deployment has lifecycle disabled, **C yields nothing there** and #17732 becomes the live card. This PR does not chase the reporter and does not attempt to close the gap — the ruling asked @hotlong for that reading on the card itself. What this PR does establish is the other half: that the declaration **is** honoured wherever the Reaper does run, measured end to end above rather than assumed. ## Acceptance notes Observations from reading the surface, filed nowhere and deliberately not acted on in this PR: - The ruling's precedent list spells the files by **object name** (`sys_job_queue.object.ts`, `sys_automation_run.object.ts`, `sys_session.object.ts`); on disk they are `sys-job-queue.object.ts`, `sys-automation-run.object.ts`, `sys-session.object.ts` (hyphens), and they live in `platform-objects` / `service-automation`, not beside each other. Cosmetic, and the shapes are exactly as the ruling describes. - `sys_notification_delivery`'s `status` field declares `failed`, which no writer of this object can produce (see Assumption 3 above). That is dead vocabulary on an authorable-looking enum rather than a defect with a repro, so it is noted here and not filed. Retiring it would be an ADR-0049 enforce-or-remove exercise on a field shared conceptually with `sys_http_delivery`, well outside this card. --- _Generated by [Claude Code](https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3a5eaea commit e7fea46

4 files changed

Lines changed: 377 additions & 2 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
'@objectstack/service-messaging': minor
3+
---
4+
5+
`sys_notification_delivery` reaps its terminal-failure rows after **7 days** instead of 90 (#17611)
6+
7+
**⚠️ Operational consequence, stated plainly: `dead` and `suppressed` delivery rows are now deleted 7 days after they were created.** Any report, SLA reading, dashboard or manual investigation that consulted them — "which notifications failed to send, and why" — must now read inside that window. Before this change those rows survived for 90 days. Nothing else about the table changes: `pending`, `in_flight` and `success` rows keep the same 90-day window they have always had, and no row is reaped sooner than before except the two terminal-failure statuses.
8+
9+
**What was wrong.** Fan-out writes one delivery row per `(event × recipient × channel)`. A tenant with no transport configured for one of those channels dead-letters that channel's row on its **first** attempt, and every `notify` writes another one. Measured on a production tenant: 2,876 `email`/`dead` rows against 2,876 `inbox`/`success` rows, `max(attempts) = 1`, zero pending, growing +316 rows/day. Those rows carry no work — nothing ever claims, retries or acks them again — but they sat in the table the dispatcher's claim query reads on every hop for the full 90-day window, so the cost of every claim rose linearly with time.
10+
11+
**The change** is one declaration on the object, using spec keys that already ship and are already consumed by the platform Reaper:
12+
13+
```ts
14+
lifecycle: {
15+
class: 'telemetry',
16+
ttl: { field: 'created_at', expireAfter: '90d' },
17+
retention: {
18+
maxAge: '7d',
19+
onlyWhen: { status: { $in: ['dead', 'suppressed'] } },
20+
},
21+
},
22+
```
23+
24+
`retention.onlyWhen` scopes the short window to the terminal-failure statuses — the same shape `sys_job_queue`, `sys_automation_run` and `sys_upload_session` already declare. No channel interface member, no new status value, no change to fan-out.
25+
26+
The `ttl` leg is not new behaviour: it restates the 90-day bound the object has always declared. `lifecycle.retention` is a single block, so scoping it to terminal rows would otherwise have left `pending` / `in_flight` / `success` with **no age bound at all** — unbounding the larger half of this table's growth on the very change that exists to bound it. Both legs run: `LifecycleService.reapObject` takes `ttl` and `retention` in independent branches. `success` is deliberately outside the scope; delivery history stays at the table window.
27+
28+
**If you override this object's lifecycle windows through the `lifecycle` settings namespace, re-read your configuration.** `retention_overrides.maxAge` for `sys_notification_delivery` used to move the whole table's window; it now moves the **terminal-failure** window only, and `expireAfter` moves the table window. An override left in place keeps parsing and keeps applying — to a narrower set of rows than it did before.
29+
30+
**⚠️ This is worth nothing where the Reaper does not run.** The whole benefit is delivered by `LifecycleService`, which `OS_LIFECYCLE_DISABLED=1` or the plugin switch turns off. A deployment with lifecycle disabled kept these rows forever before this change and keeps them forever after it; a declaration is not a sweeper. Check that the Reaper is enabled before reading this entry as a bound on your table.
Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// #17611 — the ruled declaration (director seat, decision batch #116 item 3,
4+
// maintainer verbatim and untranslated: 「17611 同意」 to 「席位推荐 C 现在做,A
5+
// 另立一卡」), driven end to end.
6+
//
7+
// The card: a tenant with no email transport dead-letters the `email` row of
8+
// every fanned-out `notify` on its first attempt, and those rows then sat in
9+
// the claim query's table for 90 days — 2,876 of them at +316/day on the
10+
// reported production tenant, carrying no work anybody would ever do.
11+
//
12+
// The fix under test is a DECLARATION, so this suite is driven exactly as the
13+
// two sibling sweeps next door (`plugin-auth/sys-session-ttl-sweep.test.ts`,
14+
// `service-storage/sys-upload-session-ttl-sweep.test.ts`): the REAL
15+
// declaration (`objects/notification-delivery.object.ts`) through the REAL
16+
// provisioning pass (`applySystemFields`) through the REAL Reaper
17+
// (`@objectstack/objectql` `LifecycleService`) against a REAL SQL backend
18+
// (`@objectstack/driver-sql`, live better-sqlite3), over a table this driver
19+
// created from that same declaration. Nothing here restates a window by hand —
20+
// the policy under test is `NotificationDelivery.lifecycle` itself, so an edit
21+
// that drops or narrows it reddens this file rather than silently voiding the
22+
// ruling.
23+
//
24+
// ## The two sides the ruling asked to see pinned, and the third this adds
25+
//
26+
// 1. TERMINAL rows past 7d ARE selected — `dead` and `suppressed`.
27+
// 2. `pending` / `success` under 90d are NOT — the acceptance's own words.
28+
// 3. ⚠️ …and a `success` row past 90d IS still reaped. Scoping `retention`
29+
// to terminal statuses would otherwise have UNBOUNDED the non-terminal
30+
// rows entirely (`retention` is one block), so this third leg is what
31+
// distinguishes "the table window was kept" from "the table window was
32+
// traded away on the card that exists to tighten it".
33+
//
34+
// ## Why the reading is discriminating
35+
//
36+
// `d_dead_30d` and `d_success_30d` carry the IDENTICAL `created_at`, so no age
37+
// rule can separate their fates — only the `onlyWhen` status filter can. And
38+
// `d_dead_2d` is terminal but inside the 7d window, so "everything terminal is
39+
// reaped" fails too: both halves of the scoped policy are load-bearing.
40+
//
41+
// ## The counterfactual
42+
//
43+
// `seeded({ lifecycle: … })` hands the SAME service the same rows under an
44+
// ABLATED declaration — the 7d retention with its `onlyWhen` removed — and the
45+
// non-terminal rows are reaped along with the terminal ones. That is what
46+
// rules out "the sweep would have produced this result anyway". ⚠️ It is
47+
// deliberately NOT described as a dist-level ablation: the declaration under
48+
// test is this package's own source, imported directly, so no rebuild is
49+
// involved and none is claimed. The on-disk ablation of the shipped file is
50+
// recorded in the PR body.
51+
52+
import { describe, it, expect, afterEach } from 'vitest';
53+
import { SqlDriver } from '@objectstack/driver-sql';
54+
import { LifecycleService, applySystemFields, assertEngineDeleteDispatch } from '@objectstack/objectql';
55+
import type { LifecycleEngineLike, LifecycleObjectLike } from '@objectstack/objectql';
56+
import type { DriverQuery } from '@objectstack/spec/contracts';
57+
import { NotificationDelivery } from './objects/notification-delivery.object.js';
58+
59+
const DAY_MS = 86_400_000;
60+
/** The instant the sweep runs. Every age below is expressed against it. */
61+
const SWEEP_AT_MS = Date.parse('2026-09-12T00:00:00.000Z');
62+
const agedDays = (days: number) => new Date(SWEEP_AT_MS - days * DAY_MS).toISOString();
63+
64+
/** Past the 7d terminal window, far inside the 90d table window. */
65+
const AGE_30D = agedDays(30);
66+
/** Inside BOTH windows. */
67+
const AGE_2D = agedDays(2);
68+
/** Past the 90d table window. */
69+
const AGE_100D = agedDays(100);
70+
71+
const openDrivers: SqlDriver[] = [];
72+
afterEach(async () => {
73+
while (openDrivers.length) {
74+
const d = openDrivers.pop();
75+
try {
76+
await d?.disconnect();
77+
} catch {
78+
/* noop */
79+
}
80+
}
81+
});
82+
83+
const silentLogger = { info: () => {}, warn: () => {}, debug: () => {}, error: () => {} };
84+
85+
/**
86+
* `LifecycleEngineLike` over a live `SqlDriver`. `delete` opens with ObjectQL's
87+
* own dispatch predicate so this double refuses exactly what the real engine
88+
* refuses (#4550) rather than re-deriving the rule.
89+
*/
90+
function sweepEngine(driver: SqlDriver, objects: LifecycleObjectLike[]): LifecycleEngineLike {
91+
return {
92+
registry: { getAllObjects: () => objects },
93+
getDriverForObject: () => driver,
94+
async find(object: string, options: any) {
95+
// Typed rather than erased to `any`: the driver silently DROPS an
96+
// unrecognised query key, so `tsc` is the only channel that can reject a
97+
// misspelt one here (#4918).
98+
const query: DriverQuery = { where: options?.where, limit: options?.limit };
99+
return driver.find(object, query);
100+
},
101+
async delete(object: string, options: any) {
102+
const dispatch = assertEngineDeleteDispatch(options);
103+
if (dispatch.kind === 'by-id') {
104+
const id = typeof dispatch.id === 'bigint' ? dispatch.id.toString() : dispatch.id;
105+
return (await driver.delete(object, id)) ? 1 : 0;
106+
}
107+
const query: DriverQuery = { where: options?.where };
108+
return driver.deleteMany(object, query);
109+
},
110+
} as LifecycleEngineLike;
111+
}
112+
113+
/** The object as the platform actually registers it (tenant field: organization_id). */
114+
function provisioned() {
115+
return applySystemFields(NotificationDelivery as any, { multiTenant: true }) as any;
116+
}
117+
118+
/**
119+
* The six rows the policy has to tell apart. Every one of them is a shape the
120+
* ack paths really write: `SqlNotificationOutbox.ack` / `MemoryNotificationOutbox.ack`
121+
* produce exactly `success | suppressed | dead | pending`, and `claim()` writes
122+
* `in_flight`.
123+
*/
124+
const ROWS: Array<{ id: string; status: string; created_at: string; channel: string }> = [
125+
// ── past the 7d terminal window ──────────────────────────────────────────
126+
{ id: 'd_dead_30d', status: 'dead', created_at: AGE_30D, channel: 'email' },
127+
{ id: 'd_suppressed_30d', status: 'suppressed', created_at: AGE_30D, channel: 'email' },
128+
// ── terminal, but INSIDE the 7d window ───────────────────────────────────
129+
{ id: 'd_dead_2d', status: 'dead', created_at: AGE_2D, channel: 'email' },
130+
// ── non-terminal, same age as the reaped terminal rows ───────────────────
131+
{ id: 'd_success_30d', status: 'success', created_at: AGE_30D, channel: 'inbox' },
132+
{ id: 'd_pending_30d', status: 'pending', created_at: AGE_30D, channel: 'inbox' },
133+
{ id: 'd_in_flight_30d', status: 'in_flight', created_at: AGE_30D, channel: 'inbox' },
134+
// ── non-terminal, past the 90d TABLE window ──────────────────────────────
135+
{ id: 'd_success_100d', status: 'success', created_at: AGE_100D, channel: 'inbox' },
136+
];
137+
138+
async function seeded(opts?: { lifecycle?: unknown }) {
139+
const schema = provisioned();
140+
const driver = new SqlDriver({
141+
client: 'better-sqlite3',
142+
connection: { filename: ':memory:' },
143+
useNullAsDefault: true,
144+
});
145+
openDrivers.push(driver);
146+
await driver.initObjects([schema]);
147+
148+
for (const r of ROWS) {
149+
await driver.create('sys_notification_delivery', {
150+
id: r.id,
151+
// Distinct per row: the object declares `(notification_id, recipient_id,
152+
// channel)` UNIQUE, which is the real dedup key — one delivery per
153+
// (event × recipient × channel). Reusing one event id here would be a
154+
// shape production cannot produce.
155+
notification_id: `n_${r.id}`,
156+
recipient_id: 'usr_1',
157+
channel: r.channel,
158+
status: r.status,
159+
attempts: r.status === 'pending' ? 0 : 1,
160+
partition_key: 0,
161+
created_at: r.created_at,
162+
updated_at: r.created_at,
163+
organization_id: 'org_A',
164+
});
165+
}
166+
167+
const object: LifecycleObjectLike = {
168+
name: NotificationDelivery.name,
169+
lifecycle:
170+
opts && 'lifecycle' in opts ? (opts.lifecycle as any) : (NotificationDelivery as any).lifecycle,
171+
fields: schema.fields,
172+
} as LifecycleObjectLike;
173+
174+
const service = new LifecycleService({
175+
getEngine: () => sweepEngine(driver, [object]),
176+
logger: silentLogger,
177+
now: () => SWEEP_AT_MS,
178+
initialDelayMs: 1,
179+
sweepIntervalMs: 10,
180+
} as any);
181+
182+
return { driver, service };
183+
}
184+
185+
const ALL_ROWS: DriverQuery = {};
186+
const survivors = async (driver: SqlDriver) =>
187+
(await driver.find('sys_notification_delivery', ALL_ROWS)).map((r: any) => r.id).sort();
188+
189+
describe('[#17611] sys_notification_delivery terminal retention — real declaration, real Reaper, live SQL', () => {
190+
it('is exactly the ruled declaration', () => {
191+
expect((NotificationDelivery as any).lifecycle).toEqual({
192+
class: 'telemetry',
193+
ttl: { field: 'created_at', expireAfter: '90d' },
194+
retention: {
195+
maxAge: '7d',
196+
onlyWhen: { status: { $in: ['dead', 'suppressed'] } },
197+
},
198+
});
199+
});
200+
201+
it('scopes on statuses the field declares AND the ack paths really write', () => {
202+
// A filter naming a value the writers never produce would compile to a
203+
// predicate matching nothing — the sweep would silently reap nothing.
204+
const scoped: string[] = (NotificationDelivery as any).lifecycle.retention.onlyWhen.status.$in;
205+
const declared: string[] = (NotificationDelivery.fields as any).status.options.map(
206+
(o: any) => (typeof o === 'string' ? o : o.value),
207+
);
208+
for (const s of scoped) expect(declared).toContain(s);
209+
// `success` is terminal too, and is deliberately OUT of the scope: the
210+
// ruling keeps delivery history at the table window.
211+
expect(scoped).not.toContain('success');
212+
});
213+
214+
it('POSITIVE — terminal rows past 7d are reaped, and the retention leg is recorded', async () => {
215+
const { driver, service } = await seeded();
216+
217+
const report = await service.sweep();
218+
219+
const left = await survivors(driver);
220+
expect(left).not.toContain('d_dead_30d');
221+
expect(left).not.toContain('d_suppressed_30d');
222+
expect(report.errors).toEqual([]);
223+
const retention = report.swept.find(
224+
(e: any) => e.object === 'sys_notification_delivery' && e.policy === 'retention',
225+
);
226+
expect(retention).toBeTruthy();
227+
});
228+
229+
it('NEGATIVE — pending / success / in_flight rows of the SAME age are untouched', async () => {
230+
const { driver, service } = await seeded();
231+
232+
await service.sweep();
233+
234+
const left = await survivors(driver);
235+
// Identical `created_at` to the two rows just reaped, so nothing but the
236+
// `onlyWhen` status filter can be what spared them.
237+
expect(left).toContain('d_success_30d');
238+
expect(left).toContain('d_pending_30d');
239+
expect(left).toContain('d_in_flight_30d');
240+
});
241+
242+
it('NEGATIVE — a terminal row INSIDE the 7d window is untouched', async () => {
243+
const { driver, service } = await seeded();
244+
245+
await service.sweep();
246+
247+
// Without this the suite would pass for a policy that reaped every
248+
// terminal row at any age.
249+
expect(await survivors(driver)).toContain('d_dead_2d');
250+
});
251+
252+
it('the TABLE window survived — a non-terminal row past 90d is still reaped by the ttl leg', async () => {
253+
const { driver, service } = await seeded();
254+
255+
const report = await service.sweep();
256+
257+
expect(await survivors(driver)).not.toContain('d_success_100d');
258+
const ttl = report.swept.find(
259+
(e: any) => e.object === 'sys_notification_delivery' && e.policy === 'ttl',
260+
);
261+
expect(ttl).toBeTruthy();
262+
});
263+
264+
it('one sweep, seven rows, the whole verdict in one assertion', async () => {
265+
const { driver, service } = await seeded();
266+
267+
await service.sweep();
268+
269+
expect(await survivors(driver)).toEqual([
270+
'd_dead_2d',
271+
'd_in_flight_30d',
272+
'd_pending_30d',
273+
'd_success_30d',
274+
]);
275+
});
276+
277+
it('COUNTERFACTUAL — drop `onlyWhen` and the same sweep takes the non-terminal rows too', async () => {
278+
// The naive policy: a 7d table-wide retention. This is what the negative
279+
// pins above have to discriminate against, so they are not vacuous.
280+
const { driver, service } = await seeded({
281+
lifecycle: {
282+
class: 'telemetry',
283+
ttl: { field: 'created_at', expireAfter: '90d' },
284+
retention: { maxAge: '7d' },
285+
},
286+
});
287+
288+
await service.sweep();
289+
290+
expect(await survivors(driver)).toEqual(['d_dead_2d']);
291+
});
292+
293+
it('COUNTERFACTUAL — with no lifecycle declaration the same sweep reaps nothing', async () => {
294+
const { driver, service } = await seeded({ lifecycle: null });
295+
296+
const report = await service.sweep();
297+
298+
expect(await survivors(driver)).toEqual(ROWS.map((r) => r.id).sort());
299+
expect(report.swept.filter((e: any) => e.object === 'sys_notification_delivery')).toEqual([]);
300+
});
301+
});

‎packages/services/service-messaging/src/objects/notification-delivery.object.ts‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,49 @@ export const NotificationDelivery = ObjectSchema.create({
2424
icon: 'send',
2525
isSystem: true,
2626
managedBy: 'engine-owned',
27-
// ADR-0057: pipeline telemetry — same 90d window as sys_notification.
27+
// ADR-0057: pipeline telemetry. TWO windows, because this table
28+
// interleaves rows that still carry work with rows that never will.
29+
//
30+
// [#17611] A tenant with no transport configured for a fanned-out channel
31+
// dead-letters that channel's row on its FIRST attempt (`attempts: 1`),
32+
// and every `notify` writes one such row forever after. Measured on a
33+
// production tenant: 2,876 `email`/`dead` rows at +316/day, zero pending.
34+
// Those rows carry no work — nothing ever claims, retries or acks them
35+
// again — yet they sat in the claim query's table for the full 90d.
36+
//
37+
// `retention.onlyWhen` scopes the SHORT window to the terminal-FAILURE
38+
// statuses, the same shape `sys_job_queue`, `sys_automation_run` and
39+
// `sys_upload_session` already declare. `success` is deliberately NOT in
40+
// the scope: the ruling keeps delivery history at the table window.
41+
//
42+
// ⚠️ The 7d scope does not REPLACE the table's bound, it sits under it —
43+
// `retention` is a single block, so scoping it would have left every
44+
// non-terminal row (`pending`, `in_flight`, `success`) with no age bound
45+
// at all, unbounding the larger half of this table's growth on the very
46+
// card that exists to bound it. The `ttl` leg restates the 90d window the
47+
// object has always declared, on the same `created_at` clock `retention`
48+
// reaps by, so non-terminal rows keep exactly today's behaviour. Both legs
49+
// run: `LifecycleService.reapObject` takes `ttl` and `retention` in
50+
// independent `if`s, not an either/or.
51+
//
52+
// ⛔ The `$in` list is a third copy of a vocabulary the WRITERS own
53+
// (`SqlNotificationOutbox.ack` / `MemoryNotificationOutbox.ack`), so it
54+
// must be widened in the same change as a writer — the `sys_automation_run`
55+
// lesson: a widened writer against a narrow sweep scope means the new
56+
// status is simply never aged out, silently, forever. Today those two are
57+
// the only terminal-failure statuses either ack path can produce.
58+
// `failed` is a legal member of the `status` field below but NO writer of
59+
// THIS object ever sets it (it is `sys_http_delivery`'s terminal status),
60+
// so naming it here would scope the sweep on a value that cannot occur.
2861
lifecycle: {
2962
class: 'telemetry',
30-
retention: { maxAge: '90d' },
63+
// The table window — every row, same 90d as sys_notification.
64+
ttl: { field: 'created_at', expireAfter: '90d' },
65+
// The terminal-failure window — rows that will never carry work again.
66+
retention: {
67+
maxAge: '7d',
68+
onlyWhen: { status: { $in: ['dead', 'suppressed'] } },
69+
},
3170
},
3271
description: 'Durable per-recipient × channel delivery outbox (ADR-0030 Layer 4).',
3372
titleFormat: '{channel} → {recipient_id}',

‎scripts/engine-double-contract.pinned.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3726,6 +3726,11 @@
37263726
"verb": "findOne",
37273727
"pinned": 6
37283728
},
3729+
{
3730+
"file": "packages/services/service-messaging/src/notification-delivery-retention-sweep.test.ts",
3731+
"verb": "delete",
3732+
"pinned": 1
3733+
},
37293734
{
37303735
"file": "packages/services/service-messaging/src/notification-schema-conformance.test.ts",
37313736
"verb": "findOne",

0 commit comments

Comments
 (0)