Skip to content

Commit c381977

Browse files
xuyushun441-sysos-zhuangclaude
authored
feat(messaging): race-safe dedup + opt-in retention (ADR-0030 hardening) (#1458)
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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9a1205f commit c381977

8 files changed

Lines changed: 414 additions & 3 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/service-messaging": minor
3+
"@objectstack/platform-objects": patch
4+
---
5+
6+
Harden the notification pipeline: race-safe dedup + opt-in retention (ADR-0030).
7+
8+
**Race-safe dedup.** `sys_notification.dedup_key` is now declared a **UNIQUE**
9+
index (was a plain index), and `emit()` **converges on a unique-key conflict**:
10+
the pre-insert `dedup_key` check is a fast-path, but if a concurrent `emit`
11+
raced past it and inserted first, our insert hits the violation — we catch it
12+
and converge to the winner's event (a dedup hit) instead of throwing or
13+
double-emitting. This mirrors the delivery outbox's enqueue convergence and
14+
stops a record-change storm from producing duplicate bell notifications. SQL
15+
treats NULLs as distinct, so the common events with no `dedup_key` are
16+
unconstrained. (Enforcement is per-driver: where declared indexes are
17+
materialized the conflict path activates; drivers that don't materialize them
18+
fall back to the best-effort fast-path — the catch is simply never taken. Note
19+
the SQL driver currently doesn't sync declared object indexes, which already
20+
affects the delivery/receipt unique indexes — tracked separately.)
21+
22+
**Opt-in retention.** New `NotificationRetention` sweeper + plugin options
23+
`retentionDays` / `retentionSweepMs`. Every `emit()` writes a `sys_notification`
24+
event (plus delivery/materialization/receipt rows), so a high-frequency
25+
periodic flow grows the tables unbounded. When `retentionDays > 0`, a
26+
low-frequency sweep (default hourly, timer `unref`'d) bulk-deletes events,
27+
deliveries, inbox messages and receipts older than the cutoff — a notification
28+
ages out wholesale, keeping the model consistent (no dangling `notification_id`)
29+
and the bell (recent-only) unaffected. The delivery row's epoch-ms `created_at`
30+
vs the others' ISO `created_at` is handled per target. **Default off** — no
31+
notification data is deleted without explicit operator policy. Each target is
32+
isolated (one object's failure doesn't abort the sweep), and the sweep runs
33+
under a system context (retention is a cross-tenant operator policy).
34+
35+
Tests: +7 `service-messaging` cases (converge-on-conflict, non-conflict
36+
rethrow, retention cutoff-formatting per target, no-engine / non-positive
37+
no-ops, failure isolation, missing-count) — 102 passing.

packages/platform-objects/src/audit/sys-notification.object.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,11 @@ export const SysNotification = ObjectSchema.create({
137137

138138
indexes: [
139139
{ fields: ['topic', 'created_at'] },
140-
{ fields: ['dedup_key'] },
140+
// Idempotency spine (ADR-0030). UNIQUE so `emit()` dedup is race-safe: a
141+
// concurrent emit with the same dedup_key loses the insert and converges to
142+
// the winner (mirrors the delivery outbox). SQL treats NULLs as distinct, so
143+
// the (common) events with no dedup_key are unconstrained.
144+
{ fields: ['dedup_key'], unique: true },
141145
{ fields: ['source_object', 'source_id'] },
142146
],
143147
});

packages/services/service-messaging/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ export type {
102102
DispatchCluster,
103103
DispatchLockHandle,
104104
} from './dispatcher.js';
105+
export { NotificationRetention, DEFAULT_RETENTION_TARGETS } from './retention.js';
106+
export type { NotificationRetentionOptions, RetentionTarget, PruneOutcome } from './retention.js';
105107

106108
// Objects (metadata definitions)
107109
export {

packages/services/service-messaging/src/messaging-service-plugin.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { MessagingService } from './messaging-service.js';
77
import { createInboxChannel } from './inbox-channel.js';
88
import { SqlNotificationOutbox } from './sql-outbox.js';
99
import { NotificationDispatcher, type DispatchCluster } from './dispatcher.js';
10+
import { NotificationRetention } from './retention.js';
1011
import { createEmailChannel } from './email-channel.js';
1112
import { NotificationTemplateStore } from './template-renderer.js';
1213
import {
@@ -40,6 +41,17 @@ export interface MessagingServicePluginOptions {
4041
* `prefix.` entry for a prefix match (default none).
4142
*/
4243
mandatoryTopics?: readonly string[];
44+
/**
45+
* Retention window in days for the notification pipeline (ADR-0030
46+
* hardening). When set (> 0), a periodic sweep prunes events, deliveries,
47+
* inbox materializations and receipts older than this — bounding the
48+
* event-log growth from high-frequency periodic flows. **Opt-in**: unset (or
49+
* ≤ 0) disables retention (default), so no notification data is ever deleted
50+
* without explicit operator policy.
51+
*/
52+
retentionDays?: number;
53+
/** Retention sweep interval in ms (default 1 hour). Only used when `retentionDays` is set. */
54+
retentionSweepMs?: number;
4355
}
4456

4557
/**
@@ -73,6 +85,7 @@ export class MessagingServicePlugin implements Plugin {
7385

7486
private readonly options: Required<MessagingServicePluginOptions>;
7587
private dispatcher?: NotificationDispatcher;
88+
private retentionTimer?: ReturnType<typeof setInterval>;
7689

7790
constructor(options: MessagingServicePluginOptions = {}) {
7891
this.options = {
@@ -81,6 +94,8 @@ export class MessagingServicePlugin implements Plugin {
8194
partitionCount: 8,
8295
dispatchIntervalMs: 500,
8396
mandatoryTopics: [],
97+
retentionDays: 0,
98+
retentionSweepMs: 3_600_000,
8499
...options,
85100
};
86101
}
@@ -200,14 +215,40 @@ export class MessagingServicePlugin implements Plugin {
200215
});
201216
}
202217

218+
// Retention sweep (ADR-0030 hardening): opt-in pruning of the
219+
// notification pipeline so the event log can't grow unbounded. Runs once
220+
// at ready then on a low-frequency interval; the timer is unref'd so it
221+
// never keeps the process alive.
222+
if (this.options.retentionDays > 0 && typeof ctx.hook === 'function') {
223+
ctx.hook('kernel:ready', async () => {
224+
const retention = new NotificationRetention({ getData, logger: ctx.logger });
225+
const days = this.options.retentionDays;
226+
const sweep = () => {
227+
void retention.prune(days).catch((err) =>
228+
ctx.logger.warn(`[messaging] retention sweep failed: ${(err as Error)?.message ?? err}`),
229+
);
230+
};
231+
sweep();
232+
this.retentionTimer = setInterval(sweep, this.options.retentionSweepMs);
233+
this.retentionTimer.unref?.();
234+
ctx.logger.info(
235+
`[messaging] retention on (prune > ${days}d every ${Math.round(this.options.retentionSweepMs / 1000)}s)`,
236+
);
237+
});
238+
}
239+
203240
ctx.logger.info(
204241
`[messaging] service registered with channels: ${service.getRegisteredChannels().join(', ') || '(none)'}`,
205242
);
206243
}
207244

208-
/** Stop the dispatcher loop on shutdown. */
245+
/** Stop the dispatcher loop + retention sweep on shutdown. */
209246
async stop(): Promise<void> {
210247
await this.dispatcher?.stop();
211248
this.dispatcher = undefined;
249+
if (this.retentionTimer) {
250+
clearInterval(this.retentionTimer);
251+
this.retentionTimer = undefined;
252+
}
212253
}
213254
}

packages/services/service-messaging/src/messaging-service.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,5 +365,63 @@ describe('MessagingService', () => {
365365
expect(inbox.seen).toHaveLength(0); // no re-fan
366366
expect(data.inserts.some((i) => i.object === 'sys_notification')).toBe(false);
367367
});
368+
369+
it('converges to the winner when a concurrent emit wins the dedup_key unique index', async () => {
370+
// Simulate the race: the fast-path findOne misses (no prior event),
371+
// but the event insert hits the UNIQUE(dedup_key) violation because a
372+
// concurrent emit inserted first. We must catch it and converge to
373+
// that winner rather than throwing or double-emitting.
374+
let firstLookup = true;
375+
const engine = {
376+
async insert(object: string) {
377+
if (object === 'sys_notification') throw new Error('UNIQUE constraint failed: sys_notification.dedup_key');
378+
return { id: 'row' };
379+
},
380+
async find() { return []; },
381+
async findOne(object: string) {
382+
if (object !== 'sys_notification') return null;
383+
// First call = the fast-path miss; second = post-conflict lookup finds the winner.
384+
if (firstLookup) { firstLookup = false; return null; }
385+
return { id: 'evt_winner' };
386+
},
387+
async update() { return {}; },
388+
async delete() { return {}; },
389+
async count() { return 0; },
390+
async aggregate() { return []; },
391+
} as any;
392+
service = new MessagingService({ logger: silentLogger(), getData: () => engine });
393+
const inbox = recordingChannel('inbox');
394+
service.registerChannel(inbox.channel);
395+
396+
const result = await service.emit({
397+
topic: 'task.assigned',
398+
audience: ['user_1'],
399+
dedupKey: 'task.assigned:t_7:user_1',
400+
payload: { title: 'Assigned' },
401+
});
402+
403+
expect(result.deduped).toBe(true);
404+
expect(result.notificationId).toBe('evt_winner');
405+
expect(inbox.seen).toHaveLength(0); // loser does not re-fan
406+
});
407+
408+
it('rethrows an event insert error that is not a dedup conflict', async () => {
409+
// No dedupKey ⇒ no convergence path ⇒ a genuine write failure surfaces.
410+
const engine = {
411+
async insert() { throw new Error('disk full'); },
412+
async find() { return []; },
413+
async findOne() { return null; },
414+
async update() { return {}; },
415+
async delete() { return {}; },
416+
async count() { return 0; },
417+
async aggregate() { return []; },
418+
} as any;
419+
service = new MessagingService({ logger: silentLogger(), getData: () => engine });
420+
service.registerChannel(recordingChannel('inbox').channel);
421+
422+
await expect(
423+
service.emit({ topic: 't', audience: ['user_1'], payload: { title: 'x' } }),
424+
).rejects.toThrow('disk full');
425+
});
368426
});
369427
});

packages/services/service-messaging/src/messaging-service.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,29 @@ export class MessagingService {
197197
}
198198

199199
// 2) Write the L2 event (or synthesize an id when there is no data layer).
200-
const notificationId = await this.writeEvent(data, input);
200+
// The check at (1) is a fast-path. Where the driver materializes the
201+
// UNIQUE(dedup_key) index, it is the real guard: a concurrent emit
202+
// that raced past (1) and inserted first makes our insert hit the
203+
// unique violation — we catch it and converge to the winner's event
204+
// (treated as a dedup hit), so a record-change storm can't produce
205+
// duplicate notifications. Mirrors the delivery outbox's enqueue
206+
// convergence. (Drivers that don't enforce the index fall back to the
207+
// best-effort fast-path — the catch is then simply never taken.)
208+
let notificationId: string;
209+
try {
210+
notificationId = await this.writeEvent(data, input);
211+
} catch (err) {
212+
if (input.dedupKey && data) {
213+
const winner = await this.findEventByDedupKey(data, input.dedupKey);
214+
if (winner) {
215+
this.ctx.logger.info(
216+
`[messaging] emit: dedupKey '${input.dedupKey}' raced; converged to ${winner}`,
217+
);
218+
return { notificationId: winner, deduped: true, deliveries: [], delivered: 0, failed: 0 };
219+
}
220+
}
221+
throw err;
222+
}
201223

202224
// 3) Resolve the audience to recipient user ids (RecipientResolver owns
203225
// role:/team:/owner_of:/email→id expansion).
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { NotificationRetention, DEFAULT_RETENTION_TARGETS } from './retention.js';
5+
6+
function silentLogger() {
7+
return { info: () => {}, warn: () => {} };
8+
}
9+
10+
/** A fake engine capturing every `delete(object, options)` call. */
11+
function captureEngine(deleteImpl?: (object: string, opts: any) => any) {
12+
const deletes: Array<{ object: string; where: any; multi: any; context: any }> = [];
13+
return {
14+
deletes,
15+
engine: {
16+
async find() { return []; },
17+
async findOne() { return null; },
18+
async insert() { return {}; },
19+
async update() { return {}; },
20+
async delete(object: string, opts: any) {
21+
deletes.push({ object, where: opts?.where, multi: opts?.multi, context: opts?.context });
22+
return deleteImpl ? deleteImpl(object, opts) : { deletedCount: 1 };
23+
},
24+
async count() { return 0; },
25+
async aggregate() { return []; },
26+
} as any,
27+
};
28+
}
29+
30+
const FIXED_NOW = 1_700_000_000_000; // fixed clock for deterministic cutoffs
31+
32+
describe('NotificationRetention', () => {
33+
it('prunes every target older than the cutoff, formatting the cutoff per target', async () => {
34+
const { engine, deletes } = captureEngine();
35+
const retention = new NotificationRetention({
36+
getData: () => engine,
37+
logger: silentLogger(),
38+
now: () => FIXED_NOW,
39+
});
40+
41+
const outcomes = await retention.prune(30);
42+
43+
// One bulk delete per default target (receipt, inbox, delivery, event).
44+
expect(deletes.map((d) => d.object)).toEqual(DEFAULT_RETENTION_TARGETS.map((t) => t.object));
45+
46+
const cutoffMs = FIXED_NOW - 30 * 86_400_000;
47+
const cutoffIso = new Date(cutoffMs).toISOString();
48+
for (const d of deletes) {
49+
expect(d.multi).toBe(true);
50+
// Cross-tenant system context — retention is an operator policy.
51+
expect(d.context).toEqual({ isSystem: true });
52+
}
53+
// The delivery row stores epoch-ms; everything else stores ISO strings.
54+
const byObject = Object.fromEntries(deletes.map((d) => [d.object, d.where]));
55+
expect(byObject['sys_notification_delivery']).toEqual({ created_at: { $lt: cutoffMs } });
56+
expect(byObject['sys_notification']).toEqual({ created_at: { $lt: cutoffIso } });
57+
expect(byObject['sys_inbox_message']).toEqual({ created_at: { $lt: cutoffIso } });
58+
expect(byObject['sys_notification_receipt']).toEqual({ created_at: { $lt: cutoffIso } });
59+
60+
expect(outcomes.every((o) => o.deleted === 1 && !o.error)).toBe(true);
61+
});
62+
63+
it('no-ops when there is no data engine', async () => {
64+
const retention = new NotificationRetention({ getData: () => undefined, logger: silentLogger() });
65+
expect(await retention.prune(30)).toEqual([]);
66+
});
67+
68+
it('no-ops for a non-positive retention window', async () => {
69+
const { engine, deletes } = captureEngine();
70+
const retention = new NotificationRetention({ getData: () => engine, logger: silentLogger() });
71+
expect(await retention.prune(0)).toEqual([]);
72+
expect(await retention.prune(-5)).toEqual([]);
73+
expect(deletes).toHaveLength(0);
74+
});
75+
76+
it('isolates a failing target — the rest of the sweep still runs', async () => {
77+
const { engine, deletes } = captureEngine((object) => {
78+
if (object === 'sys_inbox_message') throw new Error('boom');
79+
return { deletedCount: 2 };
80+
});
81+
const retention = new NotificationRetention({
82+
getData: () => engine,
83+
logger: silentLogger(),
84+
now: () => FIXED_NOW,
85+
});
86+
87+
const outcomes = await retention.prune(7);
88+
89+
// All four were attempted despite the inbox failure.
90+
expect(deletes).toHaveLength(4);
91+
const failed = outcomes.find((o) => o.object === 'sys_inbox_message');
92+
expect(failed?.error).toContain('boom');
93+
const ok = outcomes.filter((o) => o.object !== 'sys_inbox_message');
94+
expect(ok.every((o) => o.deleted === 2)).toBe(true);
95+
});
96+
97+
it('reports an undefined count when the driver returns no count', async () => {
98+
const { engine } = captureEngine(() => ({}));
99+
const retention = new NotificationRetention({
100+
getData: () => engine,
101+
logger: silentLogger(),
102+
now: () => FIXED_NOW,
103+
targets: [{ object: 'sys_notification', tsField: 'created_at', format: 'iso' }],
104+
});
105+
const outcomes = await retention.prune(1);
106+
expect(outcomes).toEqual([{ object: 'sys_notification', deleted: undefined }]);
107+
});
108+
});

0 commit comments

Comments
 (0)