fix(service-messaging): sys_notification_delivery reaps its terminal-failure rows after 7d, not 90d (#17611)

[claude]

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:

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           = 1a4df874968cc15cc0694cb8ebf05f80ece9b196
DISK_BLOB_BEFORE    = 1a4df874968cc15cc0694cb8ebf05f80ece9b196   (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 -> 1a4df874968cc15cc0694cb8ebf05f80ece9b196   (== 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

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 34 pages)
• 1 name(s) were too generic to anchor anything (single lowercase words)
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ed8dea17bd510100320ab42dbac6ec2a78e99deb → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 860bf110a6d28bb1fb88b13bec0142b17dfa015c — the merge of head a6c8cd7f55a2687d91a9cb392ad55fc04f0fe068 into base ed8dea17bd510100320ab42dbac6ec2a78e99deb, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 860bf110a6d28bb1fb88b13bec0142b17dfa015c && git checkout 860bf110a6d28bb1fb88b13bec0142b17dfa015c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ed8dea17bd510100320ab42dbac6ec2a78e99deb a6c8cd7f55a2687d91a9cb392ad55fc04f0fe068 && git checkout -B drift-repro ed8dea17bd510100320ab42dbac6ec2a78e99deb && git merge --no-ff a6c8cd7f55a2687d91a9cb392ad55fc04f0fe068

node scripts/docs-audit/affected-docs.mjs --json ed8dea17bd510100320ab42dbac6ec2a78e99deb

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.