fix(service-messaging)!: mark-read writes a read receipt only for a notification delivered to that user

.changeset/22026-read-receipt-recipient-only.md

@@ -0,0 +1,17 @@
+---
+'@objectstack/service-messaging': minor
+---
+
+fix(service-messaging)!: mark-read writes a read receipt only for a notification delivered to that user
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) A runtime narrowing on the inbox mark-read path, not a metadata change: no spec key, export, option, response field or stored shape is removed, renamed or re-shaped, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. What narrows is which ids produce a receipt row: an id never delivered to the caller now writes none and is not counted, while every delivered id behaves as before. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this path and this diff adds none (not registered / already-registered); and no published interface or type changes (not runtime-interface-only). -->
+
+**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention: `MessagingService.markRead` — the method behind the notifications mark-read door, and behind `markReadAsCaller` — writes a `read` receipt only for a notification that was delivered to that user. A read receipt belongs to a recipient (ADR-0030 keys it by recipient).
+
+- **Delivered** means a receipt keyed on that user already exists (it is flipped to `read` in place, as before), or the user's inbox holds a message for that notification. The inbox message is enough on its own, because the inbox channel's `delivered` receipt is best-effort.
+- **Any other id** writes no receipt, is not counted in `readCount`, and its notification's organization is not read. The response shape `{ success, readCount }` is unchanged.
+- **`markAllRead`** is unchanged: every id it sweeps comes from the user's own inbox.
+
+What changes for you: nothing in what you write. A `readCount` lower than the number of ids sent means some of them were not notifications delivered to that user, and nothing was written for those.

packages/services/service-messaging/src/inbox-system-context.ts

@@ -8,7 +8,8 @@
  * `countUnreadTotal`'s total), `readReceiptStates`
  * (`sys_notification_receipt`), and mark-read / mark-all-read:
  * `unreadNotificationIds`, `upsertReadReceipt` (its read, its update and its
- * insert) and `notificationOrganization`.
+ * insert), `inboxMessageNames` (the delivery check's `sys_inbox_message` read)
+ * and `notificationOrganization`.
  *
  * Before this, each of those calls reached the data engine with no context at
  * all — no principal and no opt-in — and passed the security middleware only

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

@@ -1,7 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { describe, it, expect, beforeEach } from 'vitest';
-import { MessagingService } from './messaging-service.js';
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { MessagingService, NOTIFICATION_EVENT_OBJECT } from './messaging-service.js';
 import { InboxCallerError } from './inbox-caller.js';
 import { MemoryNotificationOutbox } from './memory-outbox.js';
 import type { Delivery, MessagingChannel, SendResult } from './channel.js';
@@ -1394,3 +1394,137 @@ describe('MessagingService — plugin-facing inbox read scoped to the authentica
         });
     });
 });
+
+/**
+ * [#22026] A read receipt belongs to a RECIPIENT (ADR-0030 keys it by
+ * recipient). `markRead` inserts a `read` receipt only for a notification
+ * delivered to that user: a receipt keyed on them already exists (it is
+ * flipped, as before), or their inbox holds a message for it. Any other id
+ * writes no row, is not counted, and never has its event's organization read.
+ *
+ * Both halves are pinned on the same notification, so the only difference
+ * between the green half and the refused half is who asks. The doubles are
+ * this file's stateful `inboxEngine`, observed through spies.
+ */
+describe('[#22026] markRead writes a read receipt only for a notification delivered to that user', () => {
+    const logger = silentLogger();
+
+    /**
+     * One notification event, delivered to `u_recipient` only: their inbox
+     * message exists and its best-effort `delivered` receipt never landed —
+     * the case the insert limb exists for.
+     */
+    function deliveredToOne() {
+        const engine = inboxEngine({
+            inbox: [{ id: 'm1', user_id: 'u_recipient', notification_id: 'n1', title: 'A', created_at: '1' }],
+        });
+        engine.store[NOTIFICATION_EVENT_OBJECT] = [{ id: 'n1', organization_id: 'org_1' }];
+        const findOne = vi.spyOn(engine, 'findOne');
+        const insert = vi.spyOn(engine, 'insert');
+        const svc = new MessagingService({ logger, getData: () => engine });
+        return { engine, findOne, insert, svc };
+    }
+    const callsOn = (spy: { mock: { calls: any[][] } }, object: string) =>
+        spy.mock.calls.filter(([o]) => o === object);
+
+    it('the recipient half: an inbox message alone is delivery, so the receipt persists, stamped, and counts 1', async () => {
+        const { engine, findOne, svc } = deliveredToOne();
+
+        expect(await svc.markRead('u_recipient', ['n1'])).toEqual({ success: true, readCount: 1 });
+
+        expect(engine.store.sys_notification_receipt).toHaveLength(1);
+        expect(engine.store.sys_notification_receipt[0]).toMatchObject({
+            notification_id: 'n1', user_id: 'u_recipient', channel: 'inbox', state: 'read', organization_id: 'org_1',
+        });
+        expect(callsOn(findOne, NOTIFICATION_EVENT_OBJECT)).toHaveLength(1);
+        expect((await svc.listInbox('u_recipient')).unreadCount).toBe(0);
+    });
+
+    it('⛔ the non-recipient half: the same notification writes no row, counts 0, and its organization is never read', async () => {
+        const { engine, findOne, insert, svc } = deliveredToOne();
+
+        expect(await svc.markRead('u_other', ['n1'])).toEqual({ success: true, readCount: 0 });
+
+        expect(engine.store.sys_notification_receipt).toEqual([]);
+        expect(callsOn(insert, 'sys_notification_receipt')).toHaveLength(0);
+        expect(callsOn(findOne, NOTIFICATION_EVENT_OBJECT)).toHaveLength(0);
+        // Nothing of the recipient's moved either.
+        expect((await svc.listInbox('u_recipient')).unreadCount).toBe(1);
+    });
+
+    it('⛔ the in-process door answers the same: markReadAsCaller for a non-recipient counts 0 and writes nothing', async () => {
+        const { engine, svc } = deliveredToOne();
+
+        expect(await svc.markReadAsCaller({ userId: 'u_other' }, ['n1'])).toEqual({ success: true, readCount: 0 });
+        expect(engine.store.sys_notification_receipt).toEqual([]);
+    });
+
+    it('a mixed call counts only the delivered id, and writes only its receipt', async () => {
+        const { engine, svc } = deliveredToOne();
+
+        expect(await svc.markRead('u_recipient', ['n1', 'n_never_delivered'])).toEqual({ success: true, readCount: 1 });
+        expect(engine.store.sys_notification_receipt.map((r: any) => r.notification_id)).toEqual(['n1']);
+    });
+
+    it('the receipt half: a delivered receipt keyed on the user is flipped in place, with no inbox message (unchanged)', async () => {
+        const engine = inboxEngine({
+            receipts: [{ id: 'r1', notification_id: 'n2', user_id: 'u_recipient', channel: 'inbox', state: 'delivered' }],
+        });
+        const insert = vi.spyOn(engine, 'insert');
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        expect(await svc.markRead('u_recipient', ['n2'])).toEqual({ success: true, readCount: 1 });
+        expect(engine.store.sys_notification_receipt).toHaveLength(1);
+        expect(engine.store.sys_notification_receipt[0]).toMatchObject({ id: 'r1', state: 'read' });
+        expect(insert).not.toHaveBeenCalled();
+    });
+
+    it('the delivery check reads only the caller’s own inbox, under the system opt-in, projecting the id', async () => {
+        const { findOne, svc } = deliveredToOne();
+
+        await svc.markRead('u_other', ['n1']);
+
+        const inboxReads = callsOn(findOne, 'sys_inbox_message');
+        expect(inboxReads).toHaveLength(1);
+        expect(inboxReads[0][1]).toEqual({ where: { user_id: 'u_other', notification_id: 'n1' }, fields: ['id'] });
+        expect(inboxReads[0][2]).toEqual({ context: { isSystem: true } });
+    });
+
+    it('an inbox row with no notification_id: naming its row id writes nothing, and the row lists unread as it always did', async () => {
+        // listInbox looks read-state up only through a row's notification_id,
+        // so this row read as unread before the check existed, whatever
+        // receipt a mark-read wrote under its row id. It still does.
+        const engine = inboxEngine({
+            inbox: [{ id: 'm_no_event', user_id: 'u_recipient', title: 'X', created_at: '1' }],
+        });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        expect(await svc.markRead('u_recipient', ['m_no_event'])).toEqual({ success: true, readCount: 0 });
+        expect(engine.store.sys_notification_receipt).toEqual([]);
+        const listed = await svc.listInbox('u_recipient');
+        expect(listed.notifications).toMatchObject([{ id: 'm_no_event', read: false }]);
+        expect(listed.unreadCount).toBe(1);
+    });
+
+    it('markAllRead needs no check of its own: every id it sweeps comes from the user’s own inbox, so each is delivered', async () => {
+        const engine = inboxEngine({
+            inbox: [
+                { id: 'm1', user_id: 'u_recipient', notification_id: 'n1', title: 'A', created_at: '1' },
+                { id: 'm2', user_id: 'u_recipient', notification_id: 'n2', title: 'B', created_at: '2' },
+                { id: 'm3', user_id: 'u_other', notification_id: 'n3', title: 'C', created_at: '3' },
+            ],
+            receipts: [{ id: 'r1', notification_id: 'n1', user_id: 'u_recipient', channel: 'inbox', state: 'delivered' }],
+        });
+        const svc = new MessagingService({ logger, getData: () => engine });
+
+        // n1 flips its delivered receipt; n2 has only its inbox message and is inserted.
+        expect(await svc.markAllRead('u_recipient')).toEqual({ success: true, readCount: 2 });
+        const receipts = engine.store.sys_notification_receipt;
+        expect(receipts.map((r: any) => [r.notification_id, r.user_id, r.state])).toEqual([
+            ['n1', 'u_recipient', 'read'],
+            ['n2', 'u_recipient', 'read'],
+        ]);
+        expect((await svc.listInbox('u_recipient')).unreadCount).toBe(0);
+        expect((await svc.listInbox('u_other')).unreadCount).toBe(1);
+    });
+});

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

@@ -697,8 +697,11 @@ export class MessagingService {
      * Mark specific notifications read by upserting their inbox receipts to
      * `read`. Updates the existing `delivered` receipt in place (keyed
      * `(notification_id, user_id, channel:'inbox')`); inserts one only when
-     * absent. `ids` are notification (event) ids. Returns the REST contract
-     * shape (`MarkNotificationsReadResponseSchema`): `{ success, readCount }`.
+     * absent AND the notification was delivered to that user — see
+     * {@link upsertReadReceipt}. An id that was never delivered to the user
+     * writes nothing and is not counted. `ids` are notification (event) ids.
+     * Returns the REST contract shape (`MarkNotificationsReadResponseSchema`):
+     * `{ success, readCount }`.
      *
      * **This is the REST door's method** — `INotificationService.markRead?`,
      * called by `runtime/src/domains/notifications.ts`, which binds `userId`
@@ -884,7 +887,26 @@ export class MessagingService {
         return ids;
     }
 
-    /** Upsert a `read` receipt for one notification; returns 1 when it persisted. */
+    /**
+     * Upsert a `read` receipt for one notification; returns 1 when it
+     * persisted, 0 when the notification was never delivered to the user.
+     *
+     * [#22026] A read receipt belongs to a RECIPIENT (ADR-0030 keys it by
+     * recipient). A notification counts as delivered to this user when either
+     * half holds, both read under INBOX_SYSTEM_CONTEXT and keyed on `user_id`:
+     *
+     *  - a receipt keyed `(notification_id, user_id, channel:'inbox')` exists —
+     *    the inbox channel's `delivered` receipt, or one already flipped. That
+     *    is `flipToRead`'s own read: the receipt key IS the delivered-receipt
+     *    key, so when it finds nothing there is no delivered receipt to find;
+     *  - an inbox message for this user names the notification. It must
+     *    suffice on its own, because the `delivered` receipt is best-effort
+     *    (`writeDeliveredReceipt` logs and moves on, and the inbox row stands).
+     *
+     * Neither half ⇒ nothing is written, nothing is counted, and the event's
+     * organization is never read: an id the user was never sent cannot become
+     * a receipt of theirs, nor stamp one with that notification's organization.
+     */
     private async upsertReadReceipt(
         data: IDataEngine,
         userId: string,
@@ -907,8 +929,13 @@ export class MessagingService {
 
         if (await flipToRead()) return 1;
 
-        // No receipt yet — insert one. findOne→insert is check-then-act, so a
-        // concurrent mark-read (or the best-effort `delivered` write still in
+        // No receipt keyed on this user, so no `delivered` one either: the
+        // inbox message is the only delivery evidence left. Without it the id
+        // was never delivered to this user — write nothing, count nothing.
+        if (!(await this.inboxMessageNames(data, userId, notificationId))) return 0;
+
+        // Delivered, and no receipt yet — insert one. findOne→insert is
+        // check-then-act, so a concurrent mark-read (or the best-effort `delivered` write still in
         // flight) can win the (notification_id, user_id, channel) unique index
         // between our read and write. Treat that collision as "someone else
         // created it" and flip the now-present row to `read` instead of failing.
@@ -952,6 +979,27 @@ export class MessagingService {
         }
     }
 
+    /**
+     * [#22026] Does this user's inbox hold a message for `notificationId`? The
+     * inbox half of {@link upsertReadReceipt}'s delivery check.
+     *
+     * Keyed on `user_id` under INBOX_SYSTEM_CONTEXT, projecting `id` alone: the
+     * answer is a yes/no about the caller's own rows. NOT best-effort — a read
+     * that throws propagates to `markRead`, which logs it and leaves the id
+     * uncounted with no row written, the direction a check must fail in.
+     */
+    private async inboxMessageNames(
+        data: IDataEngine,
+        userId: string,
+        notificationId: string,
+    ): Promise<boolean> {
+        const message = await data.findOne(INBOX_OBJECT, {
+            where: { user_id: userId, notification_id: notificationId },
+            fields: ['id'],
+        }, { context: INBOX_SYSTEM_CONTEXT });
+        return message != null;
+    }
+
     /**
      * [#11303] The organization of the `sys_notification` a receipt is about,
      * or `null`.

packages/services/service-messaging/src/read-receipt-organization.test.ts

@@ -2,7 +2,7 @@
 
 import { describe, it, expect } from 'vitest';
 import { MessagingService, NOTIFICATION_EVENT_OBJECT } from './messaging-service.js';
-import { RECEIPT_OBJECT } from './inbox-channel.js';
+import { INBOX_OBJECT, RECEIPT_OBJECT } from './inbox-channel.js';
 import { assertEngineFindOnePredicate } from '@objectstack/metadata-core';
 
 /**
@@ -36,6 +36,11 @@ function silentLogger(): any {
  * A data engine holding one `sys_notification` row and recording every insert.
  * `findOne` answers the notification row for the event object and `undefined`
  * for the receipt (so `markRead` takes the insert limb, not the flip limb).
+ *
+ * [#22026] The reader is a RECIPIENT: their inbox holds a message for the
+ * notification, and only its best-effort `delivered` receipt never landed —
+ * the case this insert limb exists for. A reader with no inbox message gets no
+ * receipt at all (`messaging-service.test.ts`, the `[#22026]` block).
  */
 function engineWithNotification(notificationOrg: string | null) {
     const inserted: Array<{ object: string; row: Record<string, unknown> }> = [];
@@ -49,6 +54,9 @@ function engineWithNotification(notificationOrg: string | null) {
             if (object === NOTIFICATION_EVENT_OBJECT) {
                 return { id: 'evt_pin', organization_id: notificationOrg };
             }
+            if (object === INBOX_OBJECT && opts?.where?.user_id === 'user_1' && opts?.where?.notification_id === 'evt_pin') {
+                return { id: 'msg_pin' };
+            }
             void opts;
             return undefined;
         },