refactor(notifications): make create() route all completion actions to IU

Greptile flagged that groupedEventTypeFor maps all four Completed* actions to
COMPLETED, but create()'s isRecipientIu only covered CompletedByIU and
CompletedForCompanyByIU. Latent today (Completed/CompletedByCompanyMember only
reach createBulkNotification), but a future create(Completed) call would buffer
the IU recipient as a CU row and mis-deliver.

Extract a single isCompletionAction predicate used by both create() and
groupedEventTypeFor so the two can't drift, and extend the completion test to
cover all four actions.

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

Author: arpandhakal

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

@@ -372,29 +372,33 @@ describe('guard: IU wiring boundaries', () => {
 })
 
 describe('guard: IU completion emails', () => {
-  it.each([NotificationTaskActions.CompletedByIU, NotificationTaskActions.CompletedForCompanyByIU])(
-    'buffers a %s email as a COMPLETED IU event and keeps the in-product notification immediate',
-    async (action) => {
-      await buildService().create(action, makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }))
+  // Every completion action routes to an IU; create() must buffer them as IU rows regardless of
+  // which one is passed, so the guard stays consistent with groupedEventTypeFor.
+  it.each([
+    NotificationTaskActions.CompletedByIU,
+    NotificationTaskActions.CompletedForCompanyByIU,
+    NotificationTaskActions.Completed,
+    NotificationTaskActions.CompletedByCompanyMember,
+  ])('buffers a %s email as a COMPLETED IU event and keeps the in-product notification immediate', async (action) => {
+    await buildService().create(action, makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }))
 
-      expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1)
-      const row = mockGroupedCreateMany.mock.calls[0][0].data[0]
-      expect(row).toMatchObject({
-        recipientIuId: '33333333-3333-3333-3333-333333333333',
-        recipientClientId: null,
-        recipientCompanyId: null,
-        eventType: GroupedEmailEventType.COMPLETED,
-      })
-      expect(row.individualEmail.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333')
+    expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1)
+    const row = mockGroupedCreateMany.mock.calls[0][0].data[0]
+    expect(row).toMatchObject({
+      recipientIuId: '33333333-3333-3333-3333-333333333333',
+      recipientClientId: null,
+      recipientCompanyId: null,
+      eventType: GroupedEmailEventType.COMPLETED,
+    })
+    expect(row.individualEmail.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333')
 
-      expect(mockCreateNotification).toHaveBeenCalledTimes(1)
-      const sent = mockCreateNotification.mock.calls[0][0]
-      expect(sent.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333')
-      expect(sent.recipientClientId).toBeUndefined()
-      expect(deliveryTargetsOf(0).inProduct).toBeDefined()
-      expect(deliveryTargetsOf(0).email).toBeUndefined()
-    },
-  )
+    expect(mockCreateNotification).toHaveBeenCalledTimes(1)
+    const sent = mockCreateNotification.mock.calls[0][0]
+    expect(sent.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333')
+    expect(sent.recipientClientId).toBeUndefined()
+    expect(deliveryTargetsOf(0).inProduct).toBeDefined()
+    expect(deliveryTargetsOf(0).email).toBeUndefined()
+  })
 
   it('does not buffer and still routes the in-product CompletedByIU notification to the IU when email is disabled', async () => {
     const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null })

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

@@ -39,11 +39,8 @@ export class NotificationService extends BaseService {
       const isAssignedToIu =
         task.assigneeType === AssigneeType.internalUser &&
         (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU)
-      // Completion notifications go to the task creator, an IU
-      const isRecipientIu =
-        isAssignedToIu ||
-        action === NotificationTaskActions.CompletedByIU ||
-        action === NotificationTaskActions.CompletedForCompanyByIU
+      // Completion notifications always go to IUs (task creator, or IUs with access)
+      const isRecipientIu = isAssignedToIu || this.isCompletionAction(action)
 
       // 1. Check for existing notification. Skip if duplicate. This dedup is keyed on the client
       // assignee, so it must not gate IU-recipient notifications (e.g. CompletedByIU on a
@@ -616,7 +613,17 @@ export class NotificationService extends BaseService {
     })
   }
 
+  private isCompletionAction(action: NotificationTaskActions): boolean {
+    return (
+      action === NotificationTaskActions.Completed ||
+      action === NotificationTaskActions.CompletedByIU ||
+      action === NotificationTaskActions.CompletedByCompanyMember ||
+      action === NotificationTaskActions.CompletedForCompanyByIU
+    )
+  }
+
   private groupedEventTypeFor(action: NotificationTaskActions): GroupedEmailEventType | null {
+    if (this.isCompletionAction(action)) return GroupedEmailEventType.COMPLETED
     switch (action) {
       case NotificationTaskActions.Assigned:
       case NotificationTaskActions.AssignedToCompany:
@@ -627,11 +634,6 @@ export class NotificationService extends BaseService {
         return GroupedEmailEventType.SHARED
       case NotificationTaskActions.Commented:
         return GroupedEmailEventType.COMMENT
-      case NotificationTaskActions.Completed:
-      case NotificationTaskActions.CompletedByIU:
-      case NotificationTaskActions.CompletedByCompanyMember:
-      case NotificationTaskActions.CompletedForCompanyByIU:
-        return GroupedEmailEventType.COMPLETED
       default:
         return null
     }