fix(digest): correct signedBy check, rollup per user, skip dead orgs

* fix(digest): correct signedBy check, rollup per user, skip dead orgs

The helper compared policy.signedBy[] against member.user.id, but signedBy
stores member ids (written by the portal accept action and read by every
other consumer). Users who had signed their policies were still receiving
daily digest emails — the comparison never matched. Switched to member.id
with an inline regression test covering the reported owner+employee case.

Also roll up pending policies across orgs by user.id so a person with
memberships in multiple orgs receives one email with per-org sections
instead of one email per org. Per-org unsubscribe is still honored: a
user opted out of policy notifications in org A only drops that org from
the rollup and still gets an email about org B.

Finally, match weekly-task-reminder's dead-org filter at the DB query —
hasAccess, onboardingCompleted, and at least one non-deactivated member
with a session updated in the last 90 days — so we stop sending to
abandoned orgs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(digest): guard array[0] access for noUncheckedIndexedAccess

DTS build failed because noUncheckedIndexedAccess types array[0] as
possibly undefined, even right after a length check. Destructure and
narrow with a truthy guard so strict TS accepts the template and the
shared subject helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 3 people

apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts

@@ -37,14 +37,21 @@ const itOnlyPolicy: DigestPolicy = {
 
 describe('computePendingPolicies', () => {
   it('returns no pending policies when member has signed all applicable policies', () => {
-    const policies: DigestPolicy[] = [{ ...allPolicy, signedBy: ['usr_alice'] }];
+    const policies: DigestPolicy[] = [{ ...allPolicy, signedBy: ['mem_alice'] }];
     expect(computePendingPolicies(alice, policies)).toEqual([]);
   });
 
   it('returns policies where the member id is missing from signedBy[]', () => {
     const policies: DigestPolicy[] = [
-      { ...allPolicy, signedBy: ['usr_bob'] },
-      { ...itOnlyPolicy, id: 'pol_2', name: 'Second', signedBy: ['usr_alice'] },
+      { ...allPolicy, signedBy: ['mem_bob'] },
+      { ...itOnlyPolicy, id: 'pol_2', name: 'Second', signedBy: ['mem_alice'] },
+    ];
+    expect(computePendingPolicies(alice, policies).map((p) => p.id)).toEqual(['pol_all']);
+  });
+
+  it('does not treat the member user id as a signature (signedBy stores member ids, not user ids)', () => {
+    const policies: DigestPolicy[] = [
+      { ...allPolicy, signedBy: ['usr_alice'] },
     ];
     expect(computePendingPolicies(alice, policies).map((p) => p.id)).toEqual(['pol_all']);
   });

apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts

@@ -101,7 +101,9 @@ export function computePendingPolicies(
   policies: DigestPolicy[],
 ): DigestPolicy[] {
   return policies.filter((policy) => {
-    if (policy.signedBy.includes(member.user.id)) return false;
+    // signedBy stores member ids (see apps/portal/src/actions/accept-policies.ts),
+    // not user ids — every other consumer checks against member.id.
+    if (policy.signedBy.includes(member.id)) return false;
     if (policy.visibility === 'ALL') return true;
     if (!member.department) return false;
     return policy.visibleToDepartments.includes(member.department);

apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.test.ts

@@ -51,7 +51,8 @@ const taskUnderTest = policyAcknowledgmentDigest as unknown as {
     emailsSent: number;
     emailsFailed: number;
     orgsProcessed: number;
-    emailsSkippedUnsubscribed: number;
+    recipients: number;
+    orgsSkippedUnsubscribed: number;
   }>;
 };
 
@@ -79,7 +80,7 @@ describe('policyAcknowledgmentDigest', () => {
           {
             id: 'pol_b',
             name: 'Backup',
-            signedBy: ['usr_alice'],
+            signedBy: ['mem_alice'],
             visibility: 'ALL',
             visibleToDepartments: [],
           },
@@ -111,7 +112,7 @@ describe('policyAcknowledgmentDigest', () => {
     expect(result).toMatchObject({
       success: true,
       emailsSent: 1,
-      emailsSkippedUnsubscribed: 0,
+      orgsSkippedUnsubscribed: 0,
     });
   });
 
@@ -124,7 +125,7 @@ describe('policyAcknowledgmentDigest', () => {
           {
             id: 'pol_a',
             name: 'Access Control',
-            signedBy: ['usr_alice'],
+            signedBy: ['mem_alice'],
             visibility: 'ALL',
             visibleToDepartments: [],
           },
@@ -333,11 +334,94 @@ describe('policyAcknowledgmentDigest', () => {
     expect(result).toMatchObject({
       success: true,
       emailsSent: 0,
-      emailsSkippedUnsubscribed: 1,
+      orgsSkippedUnsubscribed: 1,
     });
   });
 
-  it('sends a separate email per org when a user belongs to multiple orgs', async () => {
+  it('rolls up pending policies across orgs into a single email when a user belongs to multiple orgs', async () => {
+    mockFindMany.mockResolvedValueOnce([
+      {
+        id: 'org_1',
+        name: 'Acme',
+        policy: [
+          {
+            id: 'pol_a',
+            name: 'Access Control',
+            signedBy: [],
+            visibility: 'ALL',
+            visibleToDepartments: [],
+          },
+        ],
+        members: [
+          {
+            id: 'mem_1',
+            department: 'it',
+            user: {
+              id: 'usr_alice',
+              name: 'Alice',
+              email: 'alice@example.com',
+              role: null,
+            },
+          },
+        ],
+      },
+      {
+        id: 'org_2',
+        name: 'Beta',
+        policy: [
+          {
+            id: 'pol_b',
+            name: 'Backup',
+            signedBy: [],
+            visibility: 'ALL',
+            visibleToDepartments: [],
+          },
+          {
+            id: 'pol_c',
+            name: 'Change Mgmt',
+            signedBy: [],
+            visibility: 'ALL',
+            visibleToDepartments: [],
+          },
+        ],
+        members: [
+          {
+            id: 'mem_2',
+            department: 'hr',
+            user: {
+              id: 'usr_alice',
+              name: 'Alice',
+              email: 'alice@example.com',
+              role: null,
+            },
+          },
+        ],
+      },
+    ]);
+
+    const result = await taskUnderTest.run({ timestamp: new Date() } as never);
+
+    expect(mockSendEmailViaApi).toHaveBeenCalledTimes(1);
+    const call = mockSendEmailViaApi.mock.calls[0][0] as {
+      to: string;
+      subject: string;
+      organizationId: string;
+    };
+    expect(call.to).toBe('alice@example.com');
+    expect(call.subject).toBe(
+      'You have 3 policies to review across 2 organizations',
+    );
+    // x-organization-id falls back to the first org the user had policies in.
+    expect(call.organizationId).toBe('org_1');
+    expect(result).toMatchObject({
+      success: true,
+      orgsProcessed: 2,
+      recipients: 1,
+      emailsSent: 1,
+    });
+  });
+
+  it('drops a single org from the rollup when the user is unsubscribed there, but still emails about other orgs', async () => {
     mockFindMany.mockResolvedValueOnce([
       {
         id: 'org_1',
@@ -390,21 +474,137 @@ describe('policyAcknowledgmentDigest', () => {
         ],
       },
     ]);
+    // Alice is unsubscribed from policy notifications in org_1 only.
+    mockGetUnsubscribedEmails.mockImplementation(
+      async (_db, _emails, _pref, orgId) =>
+        orgId === 'org_1'
+          ? new Set<string>(['alice@example.com'])
+          : new Set<string>(),
+    );
 
     const result = await taskUnderTest.run({ timestamp: new Date() } as never);
 
-    expect(sendEmailViaApi).toHaveBeenCalledTimes(2);
-    const orgs = mockSendEmailViaApi.mock.calls
-      .map((c) => (c[0] as { organizationId: string }).organizationId)
-      .sort();
-    expect(orgs).toEqual(['org_1', 'org_2']);
+    expect(mockSendEmailViaApi).toHaveBeenCalledTimes(1);
+    const call = mockSendEmailViaApi.mock.calls[0][0] as {
+      subject: string;
+      organizationId: string;
+    };
+    expect(call.subject).toBe('You have 1 policy to review at Beta');
+    expect(call.organizationId).toBe('org_2');
     expect(result).toMatchObject({
       success: true,
       orgsProcessed: 2,
-      emailsSent: 2,
+      recipients: 1,
+      emailsSent: 1,
+      orgsSkippedUnsubscribed: 1,
     });
   });
 
+  it('does not send any email to a user who is unsubscribed in every org they belong to', async () => {
+    mockFindMany.mockResolvedValueOnce([
+      {
+        id: 'org_1',
+        name: 'Acme',
+        policy: [
+          {
+            id: 'pol_a',
+            name: 'A',
+            signedBy: [],
+            visibility: 'ALL',
+            visibleToDepartments: [],
+          },
+        ],
+        members: [
+          {
+            id: 'mem_1',
+            department: 'it',
+            user: {
+              id: 'usr_alice',
+              name: 'Alice',
+              email: 'alice@example.com',
+              role: null,
+            },
+          },
+        ],
+      },
+      {
+        id: 'org_2',
+        name: 'Beta',
+        policy: [
+          {
+            id: 'pol_b',
+            name: 'B',
+            signedBy: [],
+            visibility: 'ALL',
+            visibleToDepartments: [],
+          },
+        ],
+        members: [
+          {
+            id: 'mem_2',
+            department: 'hr',
+            user: {
+              id: 'usr_alice',
+              name: 'Alice',
+              email: 'alice@example.com',
+              role: null,
+            },
+          },
+        ],
+      },
+    ]);
+    mockGetUnsubscribedEmails.mockResolvedValue(
+      new Set<string>(['alice@example.com']),
+    );
+
+    const result = await taskUnderTest.run({ timestamp: new Date() } as never);
+
+    expect(mockSendEmailViaApi).not.toHaveBeenCalled();
+    expect(result).toMatchObject({
+      success: true,
+      recipients: 0,
+      emailsSent: 0,
+      orgsSkippedUnsubscribed: 2,
+    });
+  });
+
+  it('does not email a multi-role member (e.g. owner,employee) who has already signed every policy', async () => {
+    // Regression: signedBy stores member ids (from the portal accept action),
+    // not user ids. The digest must compare against member.id, otherwise
+    // every compliance-obligated member looks "pending" on every policy.
+    mockFindMany.mockResolvedValueOnce([
+      {
+        id: 'org_1',
+        name: 'Acme',
+        policy: Array.from({ length: 10 }, (_, i) => ({
+          id: `pol_${i}`,
+          name: `Policy ${i}`,
+          signedBy: ['mem_owner_employee'],
+          visibility: 'ALL',
+          visibleToDepartments: [],
+        })),
+        members: [
+          {
+            id: 'mem_owner_employee',
+            role: 'owner,employee',
+            department: 'eng',
+            user: {
+              id: 'usr_owner_employee',
+              name: 'Owner Employee',
+              email: 'multi@example.com',
+              role: null,
+            },
+          },
+        ],
+      },
+    ]);
+
+    const result = await taskUnderTest.run({ timestamp: new Date() } as never);
+
+    expect(mockSendEmailViaApi).not.toHaveBeenCalled();
+    expect(result).toMatchObject({ success: true, emailsSent: 0 });
+  });
+
   it('sends emails in batches of up to 25', async () => {
     // Create 60 members in one org, all with pending policies, all subscribed.
     const members = Array.from({ length: 60 }, (_, i) => ({
@@ -443,4 +643,40 @@ describe('policyAcknowledgmentDigest', () => {
     expect(mockSendEmailViaApi).toHaveBeenCalledTimes(60);
     expect(result).toMatchObject({ success: true, emailsSent: 60 });
   });
+
+  it('filters out dead orgs at the DB query (hasAccess, onboardingCompleted, 90-day session activity)', async () => {
+    mockFindMany.mockResolvedValueOnce([]);
+
+    await taskUnderTest.run({ timestamp: new Date() } as never);
+
+    expect(mockFindMany).toHaveBeenCalledTimes(1);
+    const args = mockFindMany.mock.calls[0][0] as {
+      where: {
+        hasAccess?: boolean;
+        onboardingCompleted?: boolean;
+        members?: {
+          some?: {
+            deactivated?: boolean;
+            user?: {
+              sessions?: { some?: { updatedAt?: { gte?: Date } } };
+            };
+          };
+        };
+      };
+    };
+
+    expect(args.where.hasAccess).toBe(true);
+    expect(args.where.onboardingCompleted).toBe(true);
+    expect(args.where.members?.some?.deactivated).toBe(false);
+    const gte = args.where.members?.some?.user?.sessions?.some?.updatedAt?.gte;
+    expect(gte).toBeInstanceOf(Date);
+
+    // Mirror the task's local-time setDate(-90) so the assertion survives
+    // DST transitions during the 90-day window.
+    const expected = new Date();
+    expected.setDate(expected.getDate() - 90);
+    expect(Math.abs((gte as Date).getTime() - expected.getTime())).toBeLessThan(
+      5_000,
+    );
+  });
 });