fix(app-shell): an inbox message the signed-in user caused does not pop

.changeset/8667-inbox-own-receipts-silent.md

@@ -0,0 +1,20 @@
+---
+'@object-ui/app-shell': patch
+---
+
+The inbox arrival announcement (the in-page toast, or the desktop notification
+in a hidden tab) no longer fires for a message the signed-in user caused
+themselves (objectui#8667). Assigning yourself a task, or any flow that lists
+its initiator among the recipients, still lands in the inbox and still counts
+on the bell badge; it just does not pop.
+
+The inbox feed now maps the `actor_id` column that `sys_inbox_message` carries
+since objectstack#16974 onto `InboxNotification.actor_id`. It is a bare
+`sys_user` id, because the feed reads that lookup without expanding it. The
+arrival filter then skips a new, unread row whose actor is the signed-in user's
+id. The skipped row is still remembered as seen, so a later poll does not
+announce it either.
+
+Only a concrete id equal to the signed-in user's suppresses. A digest row
+carries no actor (`null`), and a server that predates the column sends no
+`actor_id` at all; both announce exactly as before.

packages/app-shell/src/hooks/__tests__/inboxArrivals.test.ts

@@ -124,6 +124,65 @@ describe('claimInboxArrivals — only UNREAD rows announce', () => {
   });
 });
 
+describe('claimInboxArrivals — what the signed-in user caused does not announce (objectui#8667)', () => {
+  // Two CONCRETE, distinguishable actors. `USER` is the reader; `OTHER` is
+  // someone else. Two absent actors would compare equal and prove nothing.
+  const OTHER = 'u_bob';
+
+  it('suppresses the row the reader caused and announces the one someone else caused', () => {
+    claimInboxArrivals(USER, [row('m1')]);
+
+    const arrivals = claimInboxArrivals(USER, [
+      row('m3', { actor_id: USER }),
+      row('m2', { actor_id: OTHER }),
+      row('m1'),
+    ]);
+
+    expect(arrivals.map((a) => a.id)).toEqual(['m2']);
+  });
+
+  it('decides by WHO IS READING: the same row is silent to its actor and announces to anyone else', () => {
+    // Alice assigned the task, so the row carries her id. To Alice it is noise…
+    claimInboxArrivals(USER, [row('m1')]);
+    expect(claimInboxArrivals(USER, [row('m2', { actor_id: USER }), row('m1')])).toEqual([]);
+
+    // …and to Bob, reading a row with the very same actor id, it is news.
+    __resetInboxArrivals();
+    claimInboxArrivals(OTHER, [row('m1')]);
+    const arrivals = claimInboxArrivals(OTHER, [row('m2', { actor_id: USER }), row('m1')]);
+    expect(arrivals.map((a) => a.id)).toEqual(['m2']);
+  });
+
+  it('still REMEMBERS the suppressed row, so it cannot announce on a later poll', () => {
+    claimInboxArrivals(USER, [row('m1')]);
+    claimInboxArrivals(USER, [row('m2', { actor_id: USER }), row('m1')]);
+
+    expect([...inboxArrivalMemory().seen]).toEqual(['m2', 'm1']);
+    // The discriminating poll drops the actor: had the row been SKIPPED rather
+    // than remembered, nothing would stop it announcing now.
+    expect(claimInboxArrivals(USER, [row('m2'), row('m1')])).toEqual([]);
+  });
+
+  it('announces a row whose actor is NULL: a digest row has no single actor, which is not "me"', () => {
+    claimInboxArrivals(USER, [row('m1')]);
+
+    const arrivals = claimInboxArrivals(USER, [row('m2', { actor_id: null }), row('m1')]);
+
+    expect(arrivals.map((a) => a.id)).toEqual(['m2']);
+  });
+
+  it('announces a row with NO actor_id key at all, so an older server behaves exactly as before', () => {
+    claimInboxArrivals(USER, [row('m1')]);
+    const legacy = row('m2');
+    // The control is only a control if the key is genuinely absent.
+    expect(Object.hasOwn(legacy, 'actor_id')).toBe(false);
+
+    const arrivals = claimInboxArrivals(USER, [legacy, row('m1')]);
+
+    expect(arrivals.map((a) => a.id)).toEqual(['m2']);
+  });
+});
+
 describe('claimInboxArrivals — a claim is a claim: the second consumer of one snapshot gets nothing', () => {
   it('hands the arrivals to the first scanner only', () => {
     claimInboxArrivals(USER, [row('m1')]);

packages/app-shell/src/hooks/__tests__/sharedInboxFeed.rowShape.test.tsx

@@ -9,10 +9,20 @@
  * `InboxNotification` (`layout/inboxGrouping.ts`) used to declare an
  * `actor_name` that was dead at both ends: `mergeInboxRows` below never mapped
  * it, neither consumer (`InboxPopover`, `useHomeInbox`) read it, and
- * `sys_inbox_message` declares no actor column for it to be mapped FROM. The
+ * `sys_inbox_message` then had no actor column for it to be mapped FROM. The
  * declaration was removed; `inboxGrouping.test.ts` carries the TYPE PIN that
  * stops it being re-declared.
  *
+ * ## The actor that DID come back, and why that is not a reversal
+ *
+ * objectui#8667 adds `actor_id` to the mapped set. The objectui#5203 retirement
+ * rested on "no column to map from", and for an actor ID that stopped being
+ * true when `sys_inbox_message` gained the `actor_id` lookup
+ * (objectstack#16974). It is still true for a NAME, so `actor_name` stays out
+ * and the second case below still holds. The route is the one this suite
+ * exists to enforce: one more EXPLICIT field in the producer's literal and one
+ * more entry in `DECLARED_KEYS`, never a pass-through.
+ *
  * A type pin alone cannot see the direction this contract is most likely to rot
  * in, because that direction does not go through the type at all: the producer
  * builds its row from an explicit field-by-field literal, and the cheap
@@ -50,12 +60,17 @@ vi.mock('@object-ui/auth', async (importOriginal) => ({
  * hypothetical future column the retired field was an invitation to add) — the
  * point of including it is that the producer's answer must be the same either
  * way: an unmapped column does not reach the UI shape.
+ *
+ * `actor_id` IS a real column (objectstack#16974) and it IS mapped. Its value is
+ * a concrete id distinct from the recipient's `user_id`, so the mapping
+ * assertion below cannot be satisfied by copying the wrong column.
  */
 const RAW_ROW = {
   id: 'ibx_1',
   user_id: 'u1',
   notification_id: 'ntf_1',
   delivery_id: 'dlv_1',
+  actor_id: 'u_zhangsan',
   topic: 'collab.assignment',
   title: 'Assigned to you: Ship it',
   body_md: 'Zhang San assigned you a task.',
@@ -65,9 +80,21 @@ const RAW_ROW = {
   actor_name: 'Li Si',
 };
 
+/**
+ * The same row as a server older than the `actor_id` column returns it: the key
+ * is ABSENT, not null. It is the control for "an older server behaves exactly
+ * as before".
+ */
+const LEGACY_ROW: Record<string, unknown> = { ...RAW_ROW, id: 'ibx_legacy' };
+delete LEGACY_ROW.actor_id;
+
+/** What the fake adapter answers the `sys_inbox_message` read with. */
+let inboxRows: unknown[] = [RAW_ROW];
+
 /** The keys `mergeInboxRows` maps — the whole of `InboxNotification`. */
 const DECLARED_KEYS = [
   'action_url',
+  'actor_id',
   'body',
   'created_at',
   'id',
@@ -80,7 +107,7 @@ const DECLARED_KEYS = [
 
 const fakeAdapter = {
   find: (object: string) => {
-    if (object === 'sys_inbox_message') return Promise.resolve({ data: [RAW_ROW] });
+    if (object === 'sys_inbox_message') return Promise.resolve({ data: inboxRows });
     // Receipts and activity answer emptily — neither is this suite's subject.
     return Promise.resolve({ data: [] });
   },
@@ -97,6 +124,7 @@ beforeEach(() => {
   vi.useFakeTimers();
   // Module-scoped stores outlive any one render tree.
   __resetSharedUserFeeds();
+  inboxRows = [RAW_ROW];
   // Approvals degrade to 0 (404) so nothing here depends on the REST feed.
   vi.stubGlobal('fetch', vi.fn(() => Promise.resolve(new Response('{}', { status: 404 }))));
 });
@@ -122,6 +150,24 @@ describe('objectui#5203 — the inbox producer maps fields, it does not pass row
     expect(row.type).toBe('collab.assignment');
     expect(row.body).toBe('Zhang San assigned you a task.');
     expect(row.action_url).toBe('/apps/crm/showcase_task/t_42');
+    // The actor, from ITS column: not the recipient's `user_id` ('u1').
+    expect(row.actor_id).toBe('u_zhangsan');
+  });
+
+  it('produces the SAME key set from an older server\'s row that has no `actor_id`, with the actor null', async () => {
+    // The control has to be real: the key is absent from the raw row.
+    expect(Object.hasOwn(LEGACY_ROW, 'actor_id')).toBe(false);
+    inboxRows = [LEGACY_ROW];
+
+    const { result } = renderHook(() => useSharedInboxFeed());
+    await settle();
+
+    expect(result.current.status).toBe('ready');
+    const row = result.current.value[0];
+    expect(row.id).toBe('ibx_legacy');
+    expect(Object.keys(row).sort()).toEqual(DECLARED_KEYS);
+    // "No known actor" — the value the arrival filter announces exactly as before.
+    expect(row.actor_id).toBeNull();
   });
 
   it('does not carry `actor_name` even when the raw row has one', async () => {

packages/app-shell/src/hooks/__tests__/useInboxArrivalNotifier.test.tsx

@@ -302,6 +302,33 @@ describe('constraint 2 — several messages in one cycle announce once', () => {
   });
 });
 
+describe('constraint 4 — a message the signed-in user caused does not pop (objectui#8667)', () => {
+  // The comparand is the signed-in user's ID as `useAuth` reports it
+  // ('u_alice' in this file's mock), not their name or email. Both actors are
+  // concrete and distinct, so neither case can pass by comparing two absences.
+  it('stays silent for a self-caused row, then announces the next row someone else caused', async () => {
+    const view = mountNotifier({ notifications: [row('m1')], status: 'ready' });
+    await settle();
+
+    view.rerender({ notifications: [row('m2', { actor_id: 'u_alice' }), row('m1')], status: 'ready' });
+    await settle();
+    expect(presentNotificationToast).not.toHaveBeenCalled();
+
+    view.rerender({
+      notifications: [row('m3', { actor_id: 'u_bob' }), row('m2', { actor_id: 'u_alice' }), row('m1')],
+      status: 'ready',
+    });
+    await settle();
+
+    // Exactly the other person's row: the self-caused one was remembered, not
+    // held back to be announced alongside it.
+    expect(presentNotificationToast).toHaveBeenCalledTimes(1);
+    expect(presentNotificationToast.mock.calls[0][0]).toMatchObject({
+      title: 'Assigned to you: m3',
+    });
+  });
+});
+
 describe('constraint 5 — toast and desktop notification are mutually exclusive', () => {
   it('a VISIBLE tab gets the toast and NO system notification', async () => {
     FakeNotification.permission = 'granted';

packages/app-shell/src/hooks/inboxArrivals.ts

@@ -15,7 +15,7 @@
  * row" into anything a user could perceive. Announcing is the presentation
  * layer's job, and it is entirely a question about DIFFS — which is why the
  * decision lives here as a pure module rather than inline in the hook that
- * presents: the three rules that decide it are the three ways the feature gets
+ * presents: the four rules that decide it are the four ways the feature gets
  * user-hostile, and each one is worth a test that can fail on its own.
  *
  *  1. **The first answered read never announces.** Historical unread at login
@@ -34,6 +34,15 @@
  *     `(topic, title)` rule (`groupNotifications`) rather than a second rule
  *     invented here — the bell already answers "how many distinct things is
  *     this really?" that way, and two answers to that question would drift.
+ *  4. **What the user caused themselves does not announce (objectui#8667).**
+ *     Assigning yourself a task, or any flow that lists its initiator among the
+ *     recipients, writes an inbox row whose `actor_id` is the reader's own id.
+ *     The message is right and the moment is noise: the user is looking at the
+ *     thing they just did. Such a row is still SEEN, so it cannot announce on
+ *     a later poll either. Only a concrete id equal to the signed-in user's
+ *     suppresses; `null` (a digest row, which has no single actor) and an
+ *     absent value (a server older than the column, objectstack#16974) mean
+ *     "no known actor" and announce exactly as before.
  *
  * ## Why the seen set is module-scoped rather than a ref
  *
@@ -104,7 +113,10 @@ export function rememberSeen(
  * announcements are switched on — a user who enables toasts mid-session must
  * not be greeted by every message that arrived while they were off.
  *
- * @param key   Session identity the memory belongs to — the signed-in user id.
+ * @param key   The signed-in user id. It is the identity the memory belongs to
+ *              AND the comparand for rule 4: the same id the feed filters
+ *              `sys_inbox_message.user_id` by, and the id space the inbox
+ *              channel writes `actor_id` in.
  * @param rows  The feed's current rows, newest first. Only pass rows from a
  *              snapshot whose status is `ready`: a `loading` or `error`
  *              snapshot carries the LAST value, and treating that as this
@@ -125,12 +137,28 @@ export function claimInboxArrivals(
 
   const known = new Set(memory.seen);
   // Unseen AND unread — the card's own definition of what arrived. A row that
-  // is new to this session but already read was consumed elsewhere.
-  const arrivals = rows.filter((row) => !known.has(row.id) && !row.is_read);
+  // is new to this session but already read was consumed elsewhere. And not
+  // caused by the reader (rule 4). Every row, suppressed or not, is still
+  // remembered below: `ids` is the whole window.
+  const arrivals = rows.filter(
+    (row) => !known.has(row.id) && !row.is_read && !causedBy(row, key),
+  );
   memory.seen = rememberSeen(memory.seen, ids);
   return arrivals.length > 0 ? arrivals : NO_ARRIVALS;
 }
 
+/**
+ * Did `userId` cause this row themselves (rule 4)?
+ *
+ * Only a concrete id equal to `userId` answers yes. `null` and `undefined`
+ * never do: they mean "no known actor", and reading either one as anything
+ * beyond "announce as before" would let a digest row, or a row from a server
+ * older than the column, be silenced on a guess.
+ */
+function causedBy(row: InboxNotification, userId: string): boolean {
+  return typeof row.actor_id === 'string' && row.actor_id === userId;
+}
+
 /**
  * What the session currently remembers — a read-only view for pins, so a test
  * can distinguish "primed and announced nothing" from "never scanned".

packages/app-shell/src/hooks/sharedUserFeeds.ts

@@ -737,6 +737,16 @@ function mergeInboxRows(rows: unknown[], receipts: unknown[]): InboxNotification
       action_url: (m.action_url as string) ?? null,
       is_read: rec ? READ_STATES.has(rec.state) : false,
       created_at: m.created_at as string | undefined,
+      // A bare `sys_user` id (objectui#8667). The column is a lookup, and a
+      // lookup arrives as the referenced RECORD only when the read asks for it
+      // with `$expand`; the `sys_inbox_message` read in `useSharedInboxFeed`
+      // passes none, so what arrives is the id the inbox channel stored, or
+      // `null` on a digest row (a collapsed group has no single actor). Only a
+      // string is an id, the same test the upstream dispatcher applies before
+      // it lets an actor through. A server older than the column
+      // (objectstack#16974) sends no key at all, and that maps to `null` too:
+      // "no known actor", which announces exactly as it did before.
+      actor_id: typeof m.actor_id === 'string' ? m.actor_id : null,
     } satisfies InboxNotification;
   });
 }

packages/app-shell/src/layout/inboxGrouping.ts

@@ -18,22 +18,30 @@
  * (`hooks/sharedUserFeeds.ts`) produces, and it is the single producer of every
  * row both the bell (`InboxPopover`) and Home's action centre render.
  *
- * Every member below is mapped by that producer and read by at least one of
- * those two consumers, and that agreement is what this interface is for. A
- * field declared here but filled by nobody is not documentation — it is a
- * standing invitation to wire it up, and two rounds of them have now been
- * removed: `source_object`/`source_id` (objectui#5190, see `action_url` below)
- * and `actor_name` (objectui#5203).
+ * Every member below is mapped by that producer and read by at least one
+ * consumer of its rows — those two surfaces, or the arrival announcer the bell
+ * mounts (`hooks/useInboxArrivalNotifier.ts`) — and that agreement is what this
+ * interface is for. A field declared here but filled by nobody is not
+ * documentation — it is a standing invitation to wire it up, and two rounds of
+ * them have now been removed: `source_object`/`source_id` (objectui#5190, see
+ * `action_url` below) and `actor_name` (objectui#5203).
  *
  * `actor_name` was dead at BOTH ends — `mergeInboxRows` never mapped it, no
- * consumer read it, and `sys_inbox_message` declares no actor column for it to
+ * consumer read it, and `sys_inbox_message` then had no actor column for it to
  * be mapped FROM. Beware that the same NAME is alive on unrelated shapes in
  * this package: the `sys_activity` -> `ActivityItem` map in
  * `hooks/sharedUserFeeds.ts` and the approval activity rows in
  * `hooks/useRecordApprovals.ts` both carry a real `actor_name`, so a grep for
- * the bare name conflates three different fields. Naming an actor on an inbox
- * row is a capability expansion (a column on `sys_inbox_message`, then a
- * producer that maps it), not a re-declaration here.
+ * the bare name conflates three different fields.
+ *
+ * Naming an actor on an inbox row was, this comment said, a capability
+ * expansion (a column on `sys_inbox_message`, then a producer that maps it),
+ * not a re-declaration here — and that is the route `actor_id` below took: the
+ * column landed first (objectstack#16974), then the producer mapped it
+ * (objectui#8667). The objectui#5203 retirement rested on "no column to map
+ * from", which no longer holds for an actor ID. It still holds for a NAME: the
+ * column is a lookup to `sys_user`, nothing on the row carries a display name,
+ * and `actor_name` stays retired.
  */
 export interface InboxNotification {
   id: string;
@@ -62,6 +70,19 @@ export interface InboxNotification {
   action_url?: string | null;
   is_read?: boolean;
   created_at?: string;
+  /**
+   * Who caused the event (the assigner, the mentioner): a bare `sys_user` id,
+   * mapped from `sys_inbox_message.actor_id` (objectstack#16974). Its reader is
+   * the arrival filter (`claimInboxArrivals` in `hooks/inboxArrivals.ts`),
+   * which compares it with the signed-in user's id so that a row the user
+   * caused themselves does not pop (objectui#8667).
+   *
+   * `null` means "no known actor" and nothing stronger. A digest row carries
+   * none by construction (a collapsed group has no single actor), and a server
+   * older than the column sends no key, which the producer maps to `null` as
+   * well. Both announce exactly as a row did before this field existed.
+   */
+  actor_id?: string | null;
 }
 
 /**