diff --git a/.changeset/16974-inbox-message-actor-id.md b/.changeset/16974-inbox-message-actor-id.md new file mode 100644 index 0000000000..96048bdb3d --- /dev/null +++ b/.changeset/16974-inbox-message-actor-id.md @@ -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. diff --git a/packages/services/service-messaging/src/channel.ts b/packages/services/service-messaging/src/channel.ts index d608f5494a..7b2b4be3b8 100644 --- a/packages/services/service-messaging/src/channel.ts +++ b/packages/services/service-messaging/src/channel.ts @@ -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; } diff --git a/packages/services/service-messaging/src/digest.test.ts b/packages/services/service-messaging/src/digest.test.ts index 2867ef008f..27bc0b0d12 100644 --- a/packages/services/service-messaging/src/digest.test.ts +++ b/packages/services/service-messaging/src/digest.test.ts @@ -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(); + }); +}); diff --git a/packages/services/service-messaging/src/dispatcher.ts b/packages/services/service-messaging/src/dispatcher.ts index 3e74d15705..083c99761d 100644 --- a/packages/services/service-messaging/src/dispatcher.ts +++ b/packages/services/service-messaging/src/dispatcher.ts @@ -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 }, }; @@ -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, }; diff --git a/packages/services/service-messaging/src/inbox-channel.test.ts b/packages/services/service-messaging/src/inbox-channel.test.ts index 2ba87a8ef3..55b56fe6b7 100644 --- a/packages/services/service-messaging/src/inbox-channel.test.ts +++ b/packages/services/service-messaging/src/inbox-channel.test.ts @@ -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: () => {} } }; @@ -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 🎉', @@ -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); + }); +}); diff --git a/packages/services/service-messaging/src/inbox-channel.ts b/packages/services/service-messaging/src/inbox-channel.ts index eda0f60785..651620bb86 100644 --- a/packages/services/service-messaging/src/inbox-channel.ts +++ b/packages/services/service-messaging/src/inbox-channel.ts @@ -198,6 +198,7 @@ export function createInboxChannel(opts: InboxChannelOptions): MessagingChannel const row: Record = { user_id: userId, notification_id: n.notificationId ?? null, + actor_id: n.actorId ?? null, topic: n.topic, title, body_md: bodyMd, diff --git a/packages/services/service-messaging/src/messaging-service.test.ts b/packages/services/service-messaging/src/messaging-service.test.ts index e85f405327..b8ae0d6659 100644 --- a/packages/services/service-messaging/src/messaging-service.test.ts +++ b/packages/services/service-messaging/src/messaging-service.test.ts @@ -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', () => { diff --git a/packages/services/service-messaging/src/messaging-service.ts b/packages/services/service-messaging/src/messaging-service.ts index 36757546eb..276a8cff71 100644 --- a/packages/services/service-messaging/src/messaging-service.ts +++ b/packages/services/service-messaging/src/messaging-service.ts @@ -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, }; @@ -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) { diff --git a/packages/services/service-messaging/src/objects/inbox-message.object.ts b/packages/services/service-messaging/src/objects/inbox-message.object.ts index a52232344e..e34a181476 100644 --- a/packages/services/service-messaging/src/objects/inbox-message.object.ts +++ b/packages/services/service-messaging/src/objects/inbox-message.object.ts @@ -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, diff --git a/packages/services/service-messaging/src/outbox.ts b/packages/services/service-messaging/src/outbox.ts index 60f4e0dbe1..51114e2941 100644 --- a/packages/services/service-messaging/src/outbox.ts +++ b/packages/services/service-messaging/src/outbox.ts @@ -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; } diff --git a/packages/services/service-messaging/src/translations/en.objects.generated.ts b/packages/services/service-messaging/src/translations/en.objects.generated.ts index 4f4da6f80f..d965985200 100644 --- a/packages/services/service-messaging/src/translations/en.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/en.objects.generated.ts @@ -34,6 +34,10 @@ export const enObjects: NonNullable = { 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" }, diff --git a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts index da564ef664..9a168ea7f0 100644 --- a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts @@ -34,6 +34,10 @@ export const esESObjects: NonNullable = { 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" }, diff --git a/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts b/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts index 236b89382b..4b63ef7de7 100644 --- a/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts +++ b/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts @@ -58,6 +58,7 @@ export const esESGeneratedSourceHashes: Readonly> = { "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", diff --git a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts index 7a98675f39..51136382d9 100644 --- a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts @@ -34,6 +34,10 @@ export const jaJPObjects: NonNullable = { label: "配信レコード", help: "外部キー → sys_notification_delivery(アウトボックス行);P1 までは null" }, + actor_id: { + label: "操作者", + help: "イベントを引き起こしたユーザー(メンション者、担当者)。sys_notification.actor_id と同じ意味です。クライアントはローカルな比較だけで自分が発生させた通知を抑制できます。ダイジェスト行は構造上 null:まとめられたグループに単一の操作者は存在しません。" + }, topic: { label: "トピック" }, diff --git a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts index 1ec7c88828..00f6e7bf12 100644 --- a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts @@ -34,6 +34,10 @@ export const zhCNObjects: NonNullable = { label: "投递记录", help: "外键 → sys_notification_delivery(发件箱行);P1 之前为空" }, + actor_id: { + label: "执行人", + help: "触发该事件的用户(提及人、分配人)——与 sys_notification.actor_id 同语义。客户端据此可用纯本地比较抑制自己触发的回执。摘要行按构造为 null:合并后的一组没有单一执行人。" + }, topic: { label: "主题" },