Skip to content

feat(messaging): race-safe dedup + opt-in retention (ADR-0030 hardening)#1458

Merged
xuyushun441-sys merged 1 commit into
mainfrom
feat/notification-dedup-unique-and-retention
Jun 1, 2026
Merged

feat(messaging): race-safe dedup + opt-in retention (ADR-0030 hardening)#1458
xuyushun441-sys merged 1 commit into
mainfrom
feat/notification-dedup-unique-and-retention

Conversation

@xuyushun441-sys
Copy link
Copy Markdown
Contributor

What

Two correctness/ops gaps in the ADR-0030 notification pipeline, flagged as remaining hardening work.

1. Race-safe dedup

emit() idempotency was a non-transactional check-then-insert against a non-unique sys_notification.dedup_key index — under a record-change storm two concurrent emits with the same dedupKey could both write events → duplicate bell notifications.

  • dedup_key is now declared a UNIQUE index. SQL treats NULLs as distinct, so the (common) events with no dedup_key are unconstrained.
  • emit() converges on a unique-key conflict: the pre-insert check stays as a fast-path, but if a concurrent emit inserted first, our insert hits the violation — caught and converged to the winner's event (a dedup hit) instead of throwing or double-emitting. Mirrors the delivery outbox's enqueue convergence.
  • Strictly an improvement: before a conflict would throw (or, unenforced, double-emit); now it converges.

Enforcement note. The SQL driver currently doesn't materialize declared object indexes (indexes: false) — verified in dev sqlite, where no declared index exists (only PK autoindexes), so the delivery/receipt unique indexes the existing P1/ADR-0030 convergence relies on aren't enforced there either. This change is consistent with that established convention (declare the unique index; converge on the conflict where a driver enforces it; fall back to the fast-path where it doesn't). The driver-side index-sync gap is pre-existing and platform-wide — flagged for separate follow-up.

2. Opt-in retention

Every emit() writes a sys_notification event (+ delivery/materialization/receipt), so a high-frequency periodic flow grows the tables unbounded.

  • New NotificationRetention sweeper + plugin options retentionDays / retentionSweepMs.
  • When retentionDays > 0, a low-frequency sweep (default hourly, timer unref'd) bulk-deletes events, deliveries, inbox messages and receipts older than the cutoff — a notification ages out wholesale (no dangling notification_id); the bell (recent-only) is unaffected.
  • Delivery's epoch-ms created_at vs the others' ISO created_at is handled per target.
  • Default off — no notification data is ever deleted without explicit operator policy. Targets are isolated (one failure doesn't abort the sweep); the sweep runs under a system (cross-tenant) context.

Tests

+7 @objectstack/service-messaging cases (converge-on-conflict, non-conflict rethrow, retention cutoff-formatting per target, no-engine / non-positive no-ops, failure isolation, missing-count). 102 messaging tests + 58 platform-objects tests green; package builds clean.

🤖 Generated with Claude Code

Two correctness/ops gaps in the notification pipeline.

Race-safe dedup:
- `sys_notification.dedup_key` is now a UNIQUE index (was plain). SQL treats
  NULLs as distinct, so the common no-dedup_key events are unconstrained.
- `emit()` converges on a unique-key conflict: the pre-insert check is a
  fast-path; if a concurrent emit inserted first, our insert hits the violation
  and we converge to the winner's event (a dedup hit) rather than throwing or
  double-emitting. Mirrors the delivery outbox's enqueue convergence; stops a
  record-change storm from producing duplicate bell notifications. Enforcement
  is per-driver (the catch is simply never taken where the index isn't
  materialized — the same situation the delivery/receipt unique indexes are in
  today; tracked separately).

Opt-in retention:
- New `NotificationRetention` sweeper + plugin `retentionDays`/`retentionSweepMs`.
  Every emit writes an event (+ delivery/materialization/receipt), so a
  high-frequency periodic flow grows the tables unbounded. When
  `retentionDays > 0`, a low-frequency sweep (default hourly, timer unref'd)
  bulk-deletes rows older than the cutoff across all four objects — a
  notification ages out wholesale (no dangling notification_id), the bell
  (recent-only) is unaffected. Delivery's epoch-ms created_at vs the others' ISO
  created_at is handled per target. Default OFF — no data deleted without
  explicit operator policy. Targets are isolated; sweep runs under a system
  (cross-tenant) context.

Tests: +7 service-messaging cases — 102 passing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jun 1, 2026 8:46am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Jun 1, 2026
@xuyushun441-sys xuyushun441-sys merged commit c381977 into main Jun 1, 2026
12 checks passed
@xuyushun441-sys xuyushun441-sys deleted the feat/notification-dedup-unique-and-retention branch June 1, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants