Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/16974-inbox-message-actor-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"@objectstack/service-messaging": minor
---

`sys_inbox_message` rows now carry **`actor_id`** — who caused the notification — and the actor travels there end to end from the `emit()` that raised the event.

Until now an inbox row could not answer "did I cause this?". The actor stopped one layer upstream on `sys_notification.actor_id`, and the shipped default permission sets grant a member no read on `sys_notification`, so the value was behind an FK hop into an object the reader cannot open. Consumers implementing the standard "do not notify me of my own action" rule had nothing to compare, and the visible failure was the notification that says *you* just did the thing you just did.

The path, one leg per seam, no new read anywhere:

- **`Notification.actorId?: string`** (`channel.ts`) — the per-recipient unit every channel implementation consumes gains an optional member, with the same semantics as `sys_notification.actor_id`.
- **`emit()`** projects `EmitInput.actorId` onto that unit on the P0 inline path, and **`enqueueDeliveries`** snapshots it into the delivery row's payload on the P1 outbox path — beside the rendered title/body, under the rule the enqueue path already states in its own comment: an event edited after enqueue cannot rewrite an in-flight send. `DeliveryPayload.actorId?: string` is declared rather than left to that type's index signature.
- **The dispatcher** reads it back off that snapshot in `processRow`. It deliberately does **not** re-read `sys_notification`, which would cost one read per delivery and break the snapshot rule.
- **The inbox channel** writes `actor_id: n.actorId ?? null`, and `sys_inbox_message` declares `actor_id` as a `sys_user` lookup.

**A digest row keeps `actor_id` null by construction.** A collapsed group has no single actor, so asserting "you caused this" over a message that also carries other people's events would be wrong; `processDigestGroup` sets no actor and the object's own description says so.

**Existing rows read `actor_id` null**, which a consumer's `row.actor_id === currentUserId` evaluates as "not mine" — the pre-change behaviour for rows written before this release. Nothing is backfilled: the value was never captured on those rows, so any backfill would be invented.
14 changes: 14 additions & 0 deletions packages/services/service-messaging/src/channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ export interface Notification {
readonly channels?: string[];
/** Optional deep-link surfaced as the inbox row's call-to-action. */
readonly actionUrl?: string;
/**
* User who caused the event (mentioner, assigner) — the same semantics as
* `sys_notification.actor_id`, projected onto the per-recipient unit so a
* channel can materialize it without reading the L2 event back.
*
* Carried end to end: `EmitInput.actorId` → here → (P1) the delivery row's
* snapshotted payload → back onto this field in the dispatcher → the
* `sys_inbox_message.actor_id` column. Consumers use it for the standard
* "do not notify me of my own action" rule, a purely local comparison.
*
* Absent on a digest delivery by construction: a collapsed group has no
* single actor.
*/
readonly actorId?: string;
/** Arbitrary structured payload carried to renderers / webhook receivers. */
readonly payload?: Record<string, unknown>;
}
Expand Down
63 changes: 63 additions & 0 deletions packages/services/service-messaging/src/digest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,66 @@ describe('NotificationDispatcher — digest collapse (P3b-2)', () => {
expect(pending.every((r) => (r.nextAttemptAt ?? 0) > now)).toBe(true);
});
});

/**
* #16974 — the dispatcher's leg of the actor's journey, with its own control.
*
* `processRow` reads the actor back off the delivery row's snapshot;
* `processDigestGroup` deliberately does NOT, because a collapsed group has no
* single actor and "you caused this" must not be asserted of a message that
* also carries other people's events. Both halves are pinned in one test on
* purpose: an absence is only evidence when the same tick shows the presence.
*/
describe('NotificationDispatcher — actor read-back (#16974)', () => {
it('restores the actor on an immediate row and leaves a digest group without one', async () => {
let now = Date.UTC(2026, 0, 1, 9, 0);
const windowAt = Date.UTC(2026, 0, 2, 0, 0);
const outbox = new MemoryNotificationOutbox(1, () => now);
const key = 'u1|inbox|2026-01-01';
for (let i = 0; i < 2; i++) {
await outbox.enqueue({
notificationId: `n${i}`, recipientId: 'u1', channel: 'inbox',
payload: { title: `Item ${i}`, actorId: `user_${i}` },
digestKey: key, notBefore: windowAt,
});
}
await outbox.enqueue({
notificationId: 'imm', recipientId: 'u1', channel: 'inbox',
payload: { title: 'Immediate', actorId: 'user_9' },
});

const rec = recordingChannel('inbox');
const d = dispatcher(outbox, [rec.channel], () => now);

// CONTROL — the immediate row's actor survives the outbox round trip.
await d.tick();
expect(rec.sent.map((n) => n.title)).toEqual(['Immediate']);
expect(rec.sent[0].actorId).toBe('user_9');

// SUBJECT — the collapsed group carries no actor, though every row in
// it has one. Null by construction, not by a missing snapshot.
now = windowAt;
await d.tick();
const digest = rec.sent[1];
expect(digest.title).toBe('You have 2 notifications');
expect(digest.actorId).toBeUndefined();
});

it('ignores a non-string actor on the snapshot rather than passing it through', async () => {
const now = () => 1000;
const outbox = new MemoryNotificationOutbox(1, now);
await outbox.enqueue({
notificationId: 'n', recipientId: 'u1', channel: 'inbox',
// A payload is a stored JSON column: a legacy or hand-edited row can
// hold anything. The seam is typed `string | undefined`, so a
// non-string must not reach it (and `actor_id` then materializes null).
payload: { title: 'T', actorId: 42 as unknown as string },
});

const rec = recordingChannel('inbox');
await dispatcher(outbox, [rec.channel], now).tick();

expect(rec.sent).toHaveLength(1);
expect(rec.sent[0].actorId).toBeUndefined();
});
});
7 changes: 7 additions & 0 deletions packages/services/service-messaging/src/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,10 @@ export class NotificationDispatcher {
severity: 'info',
recipients: [recipient],
channels: [channelName],
// No `actorId`: a collapsed group has no single actor, so a digest
// materializes with `actor_id` null by construction. ⛔ Do not pick
// the first row's actor — "you caused this" would then be asserted
// of a message that also carries other people's events.
payload: { digest: true, count: digest.count, items: digest.items },
};

Expand Down Expand Up @@ -339,6 +343,9 @@ export class NotificationDispatcher {
recipients: [row.recipientId],
channels: [row.channel],
actionUrl: typeof p.actionUrl === 'string' ? p.actionUrl : undefined,
// Read the actor back off the snapshot `enqueueDeliveries` took —
// no read of `sys_notification` here, by design.
actorId: typeof p.actorId === 'string' ? p.actorId : undefined,
payload: p,
};

Expand Down
88 changes: 88 additions & 0 deletions packages/services/service-messaging/src/inbox-channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import { describe, it, expect } from 'vitest';
import { createInboxChannel, INBOX_OBJECT, RECEIPT_OBJECT } from './inbox-channel.js';
import type { Delivery } from './channel.js';
import { assertEngineFindOnePredicate } from '@objectstack/metadata-core';
import { MessagingService } from './messaging-service.js';
import { MemoryNotificationOutbox } from './memory-outbox.js';
import { NotificationDispatcher } from './dispatcher.js';

function silentCtx() {
return { logger: { info: () => {}, warn: () => {}, error: () => {} } };
Expand Down Expand Up @@ -72,6 +75,9 @@ describe('inbox channel', () => {
expect(data.inserts[0].row).toEqual({
user_id: 'user_42',
notification_id: null,
// #16974 — the column exists on every row; this delivery carries no
// actor, so it materializes null rather than being absent.
actor_id: null,
topic: 'deal.won',
title: 'Deal closed',
body_md: 'Acme signed 🎉',
Expand Down Expand Up @@ -383,3 +389,85 @@ describe('inbox channel', () => {
});
});
});

/**
* #16974 — the actor travels END TO END, and the last leg lands here.
*
* `sys_inbox_message` carried no actor at all, so a client could not answer
* "did I cause this?" without a read of `sys_notification` — an object the
* default permission sets do not grant a member. The ruling (issue #16974,
* decision batch #119 item 5) is that the actor travels with the delivery:
*
* EmitInput.actorId → Notification.actorId → (P1) the delivery row's
* snapshotted payload → back onto Notification in the dispatcher →
* sys_inbox_message.actor_id
*
* The legs before this file are pinned in `messaging-service.test.ts` (emit's
* P0 literal and the enqueue snapshot) and `dispatcher.test.ts` (the read-back,
* and the digest group's deliberate absence). Here we pin the channel's own leg
* and then one whole-path run through the real service, outbox and dispatcher,
* because four green legs do not prove a connected path.
*/
describe('inbox channel — actor materialization (#16974)', () => {
it('writes the notification actor onto the row', async () => {
const data = fakeData();
const ch = createInboxChannel({ getData: () => data.engine, now: () => '2026-06-01T00:00:00.000Z' });

await ch.send(silentCtx(), delivery({ actorId: 'user_9' }, 'user_42'));

expect(data.inserts[0].row.actor_id).toBe('user_9');
});

it('materializes null — never undefined — when the delivery carries no actor', async () => {
const data = fakeData();
const ch = createInboxChannel({ getData: () => data.engine, now: () => '2026-06-01T00:00:00.000Z' });

await ch.send(silentCtx(), delivery({}, 'user_42'));

// `in` rather than a truthiness check: an absent key and a null value
// are different rows to the driver, and the column is declared.
expect('actor_id' in data.inserts[0].row).toBe(true);
expect(data.inserts[0].row.actor_id).toBeNull();
});

it('carries the actor through emit → outbox snapshot → dispatcher → row (P1)', async () => {
const data = fakeData();
const outbox = new MemoryNotificationOutbox(1);
const inbox = createInboxChannel({ getData: () => data.engine, now: () => '2026-06-01T00:00:00.000Z' });

const service = new MessagingService({
logger: silentCtx().logger,
getData: () => data.engine,
outbox,
});
service.registerChannel(inbox);

await service.emit({
topic: 'deal.won',
audience: ['user_42'],
actorId: 'user_9',
payload: { title: 'Deal closed', body: 'Acme signed' },
});

// Nothing materialized yet — the dispatcher owns the send on this path.
expect(data.inserts.filter((i) => i.object === INBOX_OBJECT)).toHaveLength(0);
// The actor is on the DELIVERY ROW's snapshot, so an event edited after
// enqueue cannot rewrite who caused an in-flight send.
const [row] = await outbox.list();
expect(row.payload.actorId).toBe('user_9');

await new NotificationDispatcher({
nodeId: 'node-test',
outbox,
channels: { getChannel: (id: string) => (id === 'inbox' ? inbox : undefined) },
channelContext: silentCtx(),
intervalMs: 10_000,
}).tick();

const inboxRows = data.inserts.filter((i) => i.object === INBOX_OBJECT);
expect(inboxRows).toHaveLength(1);
expect(inboxRows[0].row.actor_id).toBe('user_9');
// …and no read of `sys_notification` was needed to get it there.
expect(data.findOnes.some((f) => f.object === 'sys_notification')).toBe(false);
});
});
1 change: 1 addition & 0 deletions packages/services/service-messaging/src/inbox-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export function createInboxChannel(opts: InboxChannelOptions): MessagingChannel
const row: Record<string, unknown> = {
user_id: userId,
notification_id: n.notificationId ?? null,
actor_id: n.actorId ?? null,
topic: n.topic,
title,
body_md: bodyMd,
Expand Down
64 changes: 64 additions & 0 deletions packages/services/service-messaging/src/messaging-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,70 @@ describe('MessagingService', () => {
expect(rows[0].payload).toMatchObject({ title: 'Deal closed', body: 'Acme', severity: 'info' });
expect(rows.map((r) => r.recipientId).sort()).toEqual(['user_1', 'user_2']);
});

// #16974 — the actor rides the SNAPSHOT, like the rendered content. The
// alternative the ruling rejected was re-reading `sys_notification` in
// the dispatcher: that costs a read per delivery and lets an event
// edited after enqueue rewrite who caused an in-flight send.
it('snapshots the actor onto every enqueued delivery row', async () => {
const outbox = new MemoryNotificationOutbox(1);
service = new MessagingService({ logger: silentLogger(), outbox });
service.registerChannel(recordingChannel('inbox').channel);

await service.emit({
topic: 'deal.won',
audience: ['user_1', 'user_2'],
actorId: 'user_9',
payload: { title: 'Deal closed' },
});

const rows = await outbox.list();
expect(rows.map((r) => r.payload.actorId)).toEqual(['user_9', 'user_9']);
});

it('leaves the snapshot actor undefined when the emit carries none', async () => {
const outbox = new MemoryNotificationOutbox(1);
service = new MessagingService({ logger: silentLogger(), outbox });
service.registerChannel(recordingChannel('inbox').channel);

await service.emit({ topic: 'deal.won', audience: ['user_1'], payload: { title: 'T' } });

expect((await outbox.list())[0].payload.actorId).toBeUndefined();
});
});

// #16974 — the P0 (inline fan-out) leg. The same actor reaches the channel
// whether or not an outbox is configured; otherwise the column would be
// populated on one deployment shape and null on the other.
describe('emit() actor projection (P0)', () => {
it('projects EmitInput.actorId onto the Notification handed to every channel', async () => {
const inbox = recordingChannel('inbox');
const email = recordingChannel('email');
service = new MessagingService({ logger: silentLogger() });
service.registerChannel(inbox.channel);
service.registerChannel(email.channel);

await service.emit({
topic: 'deal.won',
audience: ['user_1'],
actorId: 'user_9',
channels: ['inbox', 'email'],
payload: { title: 'Deal closed' },
});

expect(inbox.seen[0].notification.actorId).toBe('user_9');
expect(email.seen[0].notification.actorId).toBe('user_9');
});

it('leaves the Notification actor undefined when the emit carries none', async () => {
const inbox = recordingChannel('inbox');
service = new MessagingService({ logger: silentLogger() });
service.registerChannel(inbox.channel);

await service.emit({ topic: 'deal.won', audience: ['user_1'], payload: { title: 'T' } });

expect(inbox.seen[0].notification.actorId).toBeUndefined();
});
});

describe('emit() L2 event persistence', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/services/service-messaging/src/messaging-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,9 @@ export class MessagingService {
recipients,
channels: input.channels,
actionUrl: actionUrlFor(input, payload),
// Who caused it, projected onto the per-recipient unit so a channel
// can materialize it without reading `sys_notification` back.
actorId: input.actorId,
payload: input.payload,
};

Expand Down Expand Up @@ -981,6 +984,10 @@ export class MessagingService {
body: str(payload.body) ?? '',
severity: input.severity ?? 'info',
actionUrl: actionUrlFor(input, payload),
// Snapshot the actor too: the dispatcher reads it back in
// `processRow` rather than re-reading `sys_notification`, which
// would cost a read per delivery and break the snapshot rule above.
actorId: input.actorId,
};
const deliveries: DeliveryOutcome[] = [];
for (const { recipient, channels, notBefore, digest } of targets) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ export const InboxMessage = ObjectSchema.create({
description: 'FK → sys_notification_delivery (outbox row); null until P1',
}),

actor_id: Field.lookup('sys_user', {
label: 'Actor',
required: false,
description:
'User who caused the event (mentioner, assigner) — same semantics as sys_notification.actor_id. Lets a client suppress its own receipts with a purely local comparison. Null on a digest row by construction: a collapsed group has no single actor.',
}),

topic: Field.text({
label: 'Topic',
searchable: true,
Expand Down
8 changes: 8 additions & 0 deletions packages/services/service-messaging/src/outbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ export interface DeliveryPayload {
body?: string;
severity?: 'info' | 'warning' | 'critical';
actionUrl?: string;
/**
* User who caused the event, snapshotted alongside the rendered content so
* the dispatcher can put it back on the `Notification` without a read per
* delivery. Declared rather than left to the index signature below: a typed
* key beats an untyped record (Prime Directive #12), and the dispatcher's
* read of it is then type-checked at both ends.
*/
actorId?: string;
[k: string]: unknown;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
label: "Delivery",
help: "FK → sys_notification_delivery (outbox row); null until P1"
},
actor_id: {
label: "Actor",
help: "User who caused the event (mentioner, assigner) — same semantics as sys_notification.actor_id. Lets a client suppress its own receipts with a purely local comparison. Null on a digest row by construction: a collapsed group has no single actor."
},
topic: {
label: "Topic"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
label: "Entrega",
help: "FK → sys_notification_delivery (fila de outbox); nulo hasta P1"
},
actor_id: {
label: "Actor",
help: "Usuario que provocó el evento (quien menciona, quien asigna); misma semántica que sys_notification.actor_id. Permite que un cliente silencie sus propios acuses con una comparación puramente local. Nulo en una fila de resumen por construcción: un grupo colapsado no tiene un único actor."
},
topic: {
label: "Tema"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const esESGeneratedSourceHashes: Readonly<Record<string, string>> = {
"objects.sys_http_delivery.fields.url.label": "501585a180652bdf",
"objects.sys_http_delivery.label": "302f263363fe99ab",
"objects.sys_http_delivery.pluralLabel": "340f81dc6ea64987",
"objects.sys_inbox_message.fields.actor_id.label": "b155813f8a7f06e3",
"objects.sys_notification_delivery.description": "6e93eafd2b3ec57a",
"objects.sys_notification_delivery.fields.attempts.label": "096d2dbc7038a926",
"objects.sys_notification_delivery.fields.channel.label": "979883fe73ee88cb",
Expand Down
Loading
Loading