OUT-3928 | Send task-marked-as-done email notifications to IUs - #1372
Conversation
Adds the IU-only 'task marked as done' email (OUT-3928), routed through the existing 5-minute grouped buffer per the PRD grouping rule. - add completion email templates (Completed, CompletedByIU, CompletedByCompanyMember, CompletedForCompanyByIU) and map them to GroupedEmailEventType.COMPLETED - gate completion emails on isIuEmailEnabled() - carry the real actor's senderType/company through the grouped flush so a client-actor completion isn't sent as senderId(client)/senderType(IU), which Copilot rejects and which wipes the whole window on retry failure - skip the client-notification dedup guard for IU recipients so CompletedByIU on a client-assigned task is no longer dropped by the client's leftover row Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR implements OUT-3928: IU-only "task marked as done" email notifications, routing all four completion actions through the existing 5-minute grouped email buffer. It also fixes two regressions found during review — a client-actor sender mismatch that could poison grouped emails, and a dedup guard that was incorrectly blocking IU-recipient completion notifications on client-assigned tasks.
Confidence Score: 5/5Safe to merge — the completion email paths are flag-gated on All completion actions are correctly routed to IU recipients, the grouped-email buffer carries the full sender identity from buffered rows, the dedup guard no longer blocks IU-recipient notifications on client-assigned tasks, and the flag gates ensure no production impact. Test coverage is thorough across all four completion actions and the new grouped-email sender passthrough. The only gap is a latent inconsistency in
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Task marked complete] --> B{Who completed it?}
B -->|IU| C{Assignee type?}
B -->|Client| D{Assignee type?}
C -->|company| E[CompletedForCompanyByIU\ncreate]
C -->|client or IU| F[CompletedByIU\ncreate]
D -->|company| G[CompletedByCompanyMember\ncreateBulkNotification]
D -->|other| H[Completed\ncreateBulkNotification]
E & F --> I{isIuEmailEnabled?}
G & H --> I
I -->|No| J[In-product only]
I -->|Yes| K[bufferGroupedEmailEvent\nrecipientIuId set]
K --> L[GroupedEmailEvents DB row\neventType=COMPLETED\nindividualEmail carries\nsenderId + senderType + senderCompanyId]
L --> M[flush-grouped-email job\n5-min delay]
M --> N{1 live event?}
N -->|Yes| O[sendIndividualEmail\noriginal payload verbatim]
N -->|No| P[senderFromEvents\npicks first event sender]
P --> Q[sendGroupedEmail\nwith real senderType/senderCompanyId]
Q --> R{Copilot rejects\nsenderCompanyId?}
R -->|Yes| S[Retry without\nsenderCompanyId]
R -->|No| T[IU receives\ngrouped digest email]
S --> T
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Task marked complete] --> B{Who completed it?}
B -->|IU| C{Assignee type?}
B -->|Client| D{Assignee type?}
C -->|company| E[CompletedForCompanyByIU\ncreate]
C -->|client or IU| F[CompletedByIU\ncreate]
D -->|company| G[CompletedByCompanyMember\ncreateBulkNotification]
D -->|other| H[Completed\ncreateBulkNotification]
E & F --> I{isIuEmailEnabled?}
G & H --> I
I -->|No| J[In-product only]
I -->|Yes| K[bufferGroupedEmailEvent\nrecipientIuId set]
K --> L[GroupedEmailEvents DB row\neventType=COMPLETED\nindividualEmail carries\nsenderId + senderType + senderCompanyId]
L --> M[flush-grouped-email job\n5-min delay]
M --> N{1 live event?}
N -->|Yes| O[sendIndividualEmail\noriginal payload verbatim]
N -->|No| P[senderFromEvents\npicks first event sender]
P --> Q[sendGroupedEmail\nwith real senderType/senderCompanyId]
Q --> R{Copilot rejects\nsenderCompanyId?}
R -->|Yes| S[Retry without\nsenderCompanyId]
R -->|No| T[IU receives\ngrouped digest email]
S --> T
Reviews (3): Last reviewed commit: "Merge branch 'feature/iu-email' into OUT..." | Re-trigger Greptile |
| 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 |
There was a problem hiding this comment.
Completed absent from isRecipientIu in create()
groupedEventTypeFor now maps NotificationTaskActions.Completed to GroupedEmailEventType.COMPLETED, but isRecipientIu in create() does not include Completed. No current call site passes Completed to create() (it always goes through createBulkNotification), so this is not a live bug — but if a future caller invokes create(Completed, task), isRecipientIu will be false, causing bufferGroupedEmailEvent to write recipientClientId = task.createdById (an IU ID) instead of recipientIuId, and buildNotificationDetails to produce recipientClientId in the individual email. The flush would then group those rows as CU events and silently mis-deliver them. Adding action === NotificationTaskActions.Completed to the isRecipientIu guard here would make the two methods consistent and safe against future callers.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good catch — fixed in 8403c94. Extracted a single isCompletionAction predicate now used by both create()'s isRecipientIu and groupedEventTypeFor, so the two can't drift, and create() now routes all four Completed* actions as IU. Also extended the completion test to cover all four so a future create(Completed) misroute would fail the suite.
…o 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>
|
Deployment failed with the following error: Learn More: https://vercel.link/multiple-function-regions |
|
@greptile please re review. |
|
@greptile review again please. |
Summary
Implements OUT-3928 — the IU-only "task marked as done" email. When a task is completed, IUs previously got only an in-product notification; this adds the email. Per the PRD's grouping rule ("group IU emails if >1 task notification in <5 minutes"), completions flow through the existing 5-minute grouped buffer using the reserved
GroupedEmailEventType.COMPLETED.Stacked on #1362 (OUT-3925) — base this PR on
OUT-3925-extend-grouped-email-iu-recipients, review that one first.What's in here
Completed,CompletedByIU,CompletedByCompanyMember,CompletedForCompanyByIUand maps all four toGroupedEmailEventType.COMPLETEDingroupedEventTypeFor.isIuEmailEnabled()(dev/staging only until OUT-3929 lands the platform preference), matching the assignment path.Two regressions found in review and fixed here
Client-actor completion poisoned the grouped email. The PRD's headline case is a client completing their own task. That buffers a row whose real sender is the client, but the flush hard-coded
senderType: 'internalUser'— producing asenderId(client)+senderType(IU)mismatch that Copilot rejects. On retry exhaustion the failure handler deletes the entire window, losing unrelated buffered emails for that IU. Fix:sendGroupedEmailnow carries the real actor'ssenderType/senderCompanyIdfrom the buffered row (defaulting to IU only when no sender is present), plus the single-companysenderCompanyIdretry the individual-send path already has.Dedup guard dropped IU completion emails on client-assigned tasks.
create()'s duplicate-notification guard is keyed on the client assignee but was gating the whole call, soCompletedByIUon a client-assigned task (recipient = creator IU) was skipped by the client's leftover assignment notification. Fix: the guard (and its query) now skip IU-recipient notifications.A third, prospective issue — the
?? !emailrecipient inference inbuildNotificationDetailsbecoming a landmine when OUT-3927 enables IU comment emails — is dormant in this ticket and deferred to OUT-3927.Test plan
yarn tsccleanIU_EMAIL_ALWAYS_ENABLED=true: client completes a task → creator IU receives the grouped/individual "task marked as done" email🤖 Generated with Claude Code