fix(notifications): gate IU in-product too + resilient settings fetch

Two follow-ups on OUT-3929:

1. Gate the in-product surface as well (per updated requirement). The
   notificationSettingId is now attached to the immediate in-product IU dispatch
   in create()/createBulkNotification, not just the buffered email, so the
   platform gates both surfaces against the IU's preference. We don't gate
   ourselves; we just pass the id.

2. Address Greptile review:
   - P1: resolveTasksNotificationSettingId catches getNotificationSettings
     failures and returns undefined instead of throwing. A failure no longer
     skips buffering + the in-product dispatch (which silently dropped the whole
     notification). Undefined id = platform delivers all (fail-open); the failure
     is not cached so the next send retries. With in-product now gated too,
     fail-open is the coherent choice - failing closed would mean dropping the
     entire notification during a settings-API outage.
   - P2: drop redundant `?? undefined` after `.find(Boolean)` in the flush.
   - P2: add the standard console.info trace to _getNotificationSettings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: arpandhakal

src/app/api/notification/notification.service.test.ts

@@ -544,10 +544,13 @@ describe('guard: IU notification setting (OUT-3929)', () => {
     expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBeUndefined()
   })
 
-  it('does not stamp the setting id on the immediate in-product dispatch (only the buffered email)', async () => {
+  it('also stamps the setting id on the immediate in-product dispatch so the platform gates in-product', async () => {
     await buildService().create(NotificationTaskActions.Assigned, iuTask(), { disableEmail: false })
 
-    // in-product notification fires now; suppressing it would break the notification center
-    expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined()
+    const sent = mockCreateNotification.mock.calls[0][0]
+    expect(sent.notificationSettingId).toBe('setting_tasks')
+    // email diverted to the buffer; the immediate dispatch is in-product only, now gated by the setting
+    expect(sent.deliveryTargets.inProduct).toBeDefined()
+    expect(sent.deliveryTargets.email).toBeUndefined()
   })
 })

src/app/api/notification/notification.service.ts

@@ -74,14 +74,16 @@ export class NotificationService extends BaseService {
       const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail
 
       const groupedType = email && recipientId ? this.groupedEventTypeFor(action) : null
-      if (groupedType) {
-        const association = AssociationsSchema.parse(task.associations)?.[0]
-        // IU emails carry the Tasks notification setting so the platform enforces the IU's
-        // per-surface preference. Stored on the buffered email, so both the grouped summary and
-        // the single-event replay dispatch it.
-        const notificationSettingId = isRecipientIu
+      // IU notifications carry the Tasks notification setting so the platform gates BOTH the
+      // in-product and email surfaces against the IU's preference. Resolved once and attached to the
+      // buffered email (also drives the grouped summary + single-event replay) and the immediate
+      // in-product dispatch below.
+      const notificationSettingId =
+        isRecipientIu && email
           ? await resolveTasksNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId })
           : undefined
+      if (groupedType) {
+        const association = AssociationsSchema.parse(task.associations)?.[0]
         await this.bufferGroupedEmailEvent({
           task,
           recipientId,
@@ -110,6 +112,7 @@ export class NotificationService extends BaseService {
         { inProduct, email },
         senderCompanyId,
         isRecipientIu,
+        notificationSettingId,
       )
       if (groupedType) notificationDetails.deliveryTargets = { inProduct }
       if (!inProduct && !notificationDetails.deliveryTargets?.email) return
@@ -256,6 +259,7 @@ export class NotificationService extends BaseService {
             { inProduct, email },
             opts?.senderCompanyId,
             isRecipientIu,
+            notificationSettingId,
           )
           if (groupedType) notificationDetails.deliveryTargets = { inProduct }
 
@@ -730,9 +734,8 @@ export class NotificationService extends BaseService {
     deliveryTargets: NotificationRequestBody['deliveryTargets'],
     senderCompanyId?: string,
     isRecipientIu?: boolean,
-    // Set only for IU email payloads: the platform gates each surface against the IU's preference.
-    // Intentionally omitted on the immediate in-product dispatch so notification-center delivery
-    // (and our InternalUserNotification tracking) is never suppressed.
+    // Set for IU payloads so the platform gates each requested surface (in-product and email)
+    // against the IU's preference. Suppression is enforced platform-side; we just pass the id.
     notificationSettingId?: string,
   ): NotificationRequestBody {
     const associations = AssociationsSchema.parse(task.associations)

src/app/api/notification/resolveNotificationSettingId.test.ts

@@ -42,6 +42,20 @@ describe('resolveTasksNotificationSettingId', () => {
     expect(getNotificationSettings).toHaveBeenCalledTimes(1)
   })
 
+  it('falls back to undefined when the settings fetch fails, without caching the failure', async () => {
+    const getNotificationSettings = jest
+      .fn()
+      .mockRejectedValueOnce(new Error('copilot 5xx'))
+      .mockResolvedValueOnce({ notifications: [{ id: 'setting_tasks', label: 'x', surfaces: ['email'] }] })
+    const copilot = buildCopilot(getNotificationSettings)
+
+    // first call fails to resolve → no suppression, and the failure is not cached
+    expect(await resolveTasksNotificationSettingId({ copilot, workspaceId: 'ws_1' })).toBeUndefined()
+    // next call retries and succeeds
+    expect(await resolveTasksNotificationSettingId({ copilot, workspaceId: 'ws_1' })).toBe('setting_tasks')
+    expect(getNotificationSettings).toHaveBeenCalledTimes(2)
+  })
+
   it('caches per workspace independently', async () => {
     const getNotificationSettings = jest.fn().mockResolvedValue({
       notifications: [{ id: 'setting_tasks', label: 'New task assigned', surfaces: ['email'] }],

src/app/api/notification/resolveNotificationSettingId.ts

@@ -1,4 +1,5 @@
 import { CopilotAPI } from '@/utils/CopilotAPI'
+import { serializeError } from '@/utils/serializeError'
 
 // The Tasks app declares a single notification setting for now, so every IU notification shares
 // the sole declared setting's id. When we split into per-category settings (task assigned vs
@@ -22,10 +23,18 @@ export const resolveTasksNotificationSettingId = async ({
   const cached = cache.get(workspaceId)
   if (cached && cached.expiresAt > Date.now()) return cached.id
 
-  const { notifications } = await copilot.getNotificationSettings()
-  const id = notifications[0]?.id
-  cache.set(workspaceId, { id, expiresAt: Date.now() + CACHE_TTL_MS })
-  return id
+  try {
+    const { notifications } = await copilot.getNotificationSettings()
+    const id = notifications[0]?.id
+    cache.set(workspaceId, { id, expiresAt: Date.now() + CACHE_TTL_MS })
+    return id
+  } catch (e) {
+    // Never let settings resolution block delivery: fall back to no suppression (pre-OUT-3929
+    // behavior — email is sent, just not gated by preference). Don't cache the failure so the next
+    // send retries rather than staying degraded for the whole TTL.
+    console.error('resolveTasksNotificationSettingId | failed to resolve; sending without suppression', serializeError(e))
+    return undefined
+  }
 }
 
 // Test seam: clear the per-workspace cache between cases.

src/jobs/notifications/flush-grouped-email.ts

@@ -238,7 +238,7 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) =>
         senderCompanyId: sender?.senderCompanyId,
         recipientInternalUserId: group.recipientIuId,
         // All IU rows in a window share the single Tasks setting; read it off any buffered email.
-        notificationSettingId: liveEvents.map((e) => e.individualEmail?.notificationSettingId).find(Boolean) ?? undefined,
+        notificationSettingId: liveEvents.map((e) => e.individualEmail?.notificationSettingId).find(Boolean),
         copilot,
       })
       sent += 1

src/utils/CopilotAPI.ts

@@ -352,6 +352,7 @@ export class CopilotAPI {
   // (installs aren't in the token, so match by appId) then fetches its settings. Returns an empty
   // list when the app has no install/settings, so callers safely fall back to no suppression.
   async _getNotificationSettings(): Promise<NotificationSettingsResponse> {
+    console.info('CopilotAPI#_getNotificationSettings', this.token)
     const appId = z.string({ message: 'Missing AppID in environment' }).parse(APP_ID)
     const installs = await this.copilot.listAppInstalls()
     const install = installs.find((entry) => entry.appId === appId)