PROD: feature-iu-email - #1393
Conversation
Co-authored-by: Neil Raina <makeitraina@users.noreply.github.com>
…ents - Wire recipientIuId into bufferGroupedEmailEvent with a separate window query and IU-specific window key (avoids collisions with CU keys) - Teach flushGroupedEmail to group and flush IU recipient rows via recipientInternalUserId in the Copilot notification payload - Add isIuEmailEnabled() stub backed by IU_EMAIL_ALWAYS_ENABLED env var so IU emails can be tested before OUT-3929 (platform preference flag) ships - Remove disableEmail hard-block for IU assignees; gate on isIuEmailEnabled() - Fix buildNotificationDetails to accept explicit isIuRecipient flag so the payload sets recipientInternalUserId correctly when IU has email Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ipients Address PR review: pass recipient ids via conditional spread instead of explicit null, and drop the null type from bufferGroupedEmailEvent params. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rbitrary IU The flush job attributed grouped emails to resolveSenderId (an arbitrary workspace internal user). Copilot creates the notification but does not deliver the email to an IU recipient from an unrelated sender, so IU grouped emails (event count > 1) silently failed while single-event and client grouped emails worked. Use the sender captured on the buffered individualEmail and fall back to resolveSenderId only for pre-migration rows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getEmailDetails had no ReassignedToIU entry, so reassignment to an IU produced no email and was never buffered into the grouped queue — only the in-product notification fired. Add the template (same copy as ReassignedToClient) and a regression test asserting IU-recipient actions have email templates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- rename isIuRecipient to isAssignedToIu / isRecipientIu for clarity - bufferGroupedEmailEvent now takes recipientId + companyId + isRecipientIu and routes to the right column internally - collapse the duplicated window queries into one with a recipient filter fragment Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…il-iu-recipients OUT-3925, OUT-3926 | Extend grouped email system to support IU recipients
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>
…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>
…s-done-emails-to-ius OUT-3928 | Send task-marked-as-done email notifications to IUs
Implements OUT-3927. IUs now receive email (not just in-product) for new comments and thread replies, reusing the existing CU comment templates. - comment job: IU recipients now get email (gated on isIuEmailEnabled()) via the grouped buffer, consistent with CU comment emails - reply job: IU initiator branch now includes the email delivery target alongside inProduct, gated on isIuEmailEnabled() Removes the deferred routing landmine (was tracked for this ticket): the `?? !email` inference in buildNotificationDetails assumed "has email => client", which inverts once IUs get emails. Flipping the comment IU call to email:true would have routed IU comment emails to recipientClientId. Now: - createBulkNotification takes an explicit isRecipientIu opt; recipient type is declared by the caller (the same Commented action fans out to both CU and IU lists, so it can't be inferred from the action) - buildNotificationDetails uses isRecipientIu directly, no email-absence fallback - completion callers pass isRecipientIu: true explicitly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…quired Addresses Greptile review: defaulting isRecipientIu to false is a silent misrouting footgun — a future IU-recipient bulk caller that omits it would route to recipientClientId with no compile-time or runtime signal. Make isRecipientIu a required field on the opts object so every caller must declare recipient type. All existing CU callers now pass isRecipientIu: false explicitly; IU callers already pass true. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s-to-ius OUT-3927 | Send comment & reply email notifications to IUs
…settings
Implements OUT-3929. IUs can now toggle Product/Email per notification type on
the Assembly /settings/notifications page; the platform enforces those prefs at
send time. The app declares one setting per category (assigned/comment/completed),
fetches their stable ids, and wires them into IU sends.
- add notificationSettingId to the notification body + NotificationSetting(s)
schemas; CopilotAPI.getNotificationSettings() resolves the install by appId and
fetches installs/{id}/notification-settings
- resolveIuNotificationSetting({category}) maps a category -> declared setting by
label, returning { id, emailEnabled }; per-workspace cache (5m TTL, config only,
never IU prefs). Fails closed on error and when a category isn't declared yet:
no id, email withheld
- IU sends attach the category id to the in-product dispatch so the platform gates
in-product per IU. The email surface is gated app-side: a grouped summary is
cross-category and can't carry a per-category id, so an IU email is only buffered
when the category's declared setting enables the email surface. Grouped windows
stay cross-category. Reply job dispatches directly, so it just passes the id.
- remove the IU_EMAIL_ALWAYS_ENABLED env kill switch: IU sends always request both
surfaces and the platform is the sole gate
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Greptile Reworks IU notification gating per the platform's real mechanism (per-IU prefs are only enforced at send time via notificationSettingId; there's no read API yet). - resolver back to id-only (resolveIuNotificationSettingId); drops the surfaces/ emailEnabled buffer-time gate — gating now lives on the send + at flush - create()/createBulkNotification always buffer IU emails and attach the category id to the in-product dispatch (platform gates in-product per IU) and to the buffered email (for the flush single-category gate) - flush: attach notificationSettingId to an IU grouped summary only when the whole window is one category (mixed windows can't gate against one id); add a filterEventsForIuPreferences seam (pass-through, TODO(OUT-3929)) for when Assembly exposes a per-IU preference read endpoint - replies now buffer as COMMENT events (were direct sends) so they group + gate like top-level comments; IU in-product still fires immediately. Removes the ungated direct-reply path (Greptile P1) - Greptile P2: stop logging the raw token in _getNotificationSettings Windows stay cross-category (combined email). True per-IU gating of a mixed summary is blocked on the read endpoint being confirmed with Assembly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…efault getNotificationParties' CommentToIU case had no break and fell through to default, which strict-parses me().id and task.assigneeId — throwing (ZodError "Required") when me() returns null or the task is unassigned. The comment job only consumes recipientIds/senderCompanyId (both set in the CommentToIU case), so the fallthrough's values were unused anyway. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Logs the payload for both flush send paths (individual replay + grouped summary) via the trigger logger, for debugging IU/CU email delivery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a notificationSettingId is passed and the recipient IU has every requested surface turned off, the platform suppresses the notification: the call succeeds (2xx) with no created object. _createNotification now returns null in that case instead of throwing on the strict schema parse, and callers skip the DB save. Non-suppressible callers (reminders, webhook, backfill, validate-count) guard the null defensively (they never pass a settingId). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stop attaching notificationSettingId (set undefined; resolve calls commented) so IUs receive all email + in-product notifications without platform gating. Per-IU gating is blocked on Copilot exposing a preference-read endpoint; re-enable by restoring the commented resolve calls (and implementing flush-time preference filtering). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mated-triage-response-b1cc Short-circuit detail page HEAD probes
…feature/iu-email
…ks-app into OUT-3929-per-category-iu-notification-gating
…tification-gating OUT-3929 | Gate IU notifications via platform notification settings
Feature/IU-emails
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Deployment failed with the following error: Learn More: https://vercel.link/multiple-function-regions |
Greptile SummaryThis PR enables grouped email delivery for internal users. The main changes are:
Confidence Score: 4/5IU notification preferences and reply retry behavior need fixes before merging.
src/app/api/notification/notification.service.ts; src/jobs/notifications/flush-grouped-email.ts; src/jobs/notifications/send-reply-create-notifications.ts
|
| Filename | Overview |
|---|---|
| src/app/api/notification/notification.service.ts | Adds IU recipient routing and grouped-email buffering, but leaves the preference setting identifier unset. |
| src/jobs/notifications/flush-grouped-email.ts | Adds IU grouped-email flushing and sender preservation, while mixed category windows omit preference gating. |
| src/jobs/notifications/send-reply-create-notifications.ts | Buffers reply emails for grouped delivery and sends IU in-product notifications alongside the buffer operation. |
| src/utils/CopilotAPI.ts | Adds notification-settings retrieval and treats platform-suppressed notification creation as a null result. |
Reviews (1): Last reviewed commit: "Merge branch 'production' into main" | Re-trigger Greptile
| const notificationSettingId = undefined | ||
| // const notificationSettingId = isRecipientIu && category ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined |
There was a problem hiding this comment.
notificationSettingId is always left undefined while this change enables IU assignment, comment, reassignment, and completion emails. The buffered snapshot and the immediate in-product request therefore have no category ID, so Copilot cannot apply an IU's disabled-category preference and sends notifications that the recipient turned off.
There was a problem hiding this comment.
Got it — the TODO comment in the code already captures this, and it matches the ticket OUT-3929 for the follow-up. Makes sense to ship ungated for now.
| const singleCategorySettingId = (events: WindowEvent[]): string | undefined => { | ||
| const ids = events.map((e) => e.individualEmail?.notificationSettingId) | ||
| return ids.every((id) => id && id === ids[0]) ? (ids[0] ?? undefined) : undefined | ||
| } |
There was a problem hiding this comment.
Mixed windows lose preference gating
When one IU receives events from two categories during the same five-minute window, this function returns undefined and the grouped request is sent without a setting ID. Even after the buffering path starts supplying setting IDs, an IU who disabled one of those categories will receive the mixed summary because the pass-through filter does not remove disabled events.
There was a problem hiding this comment.
as of now, we are sending all emails to iu regardless the settingId. please read the comments too @greptile
There was a problem hiding this comment.
You're right, and I missed that. The comments explicitly document both limitations as intentional, known gaps — filterEventsForIuPreferences is a named seam with a TODO, and singleCategorySettingId calls out the mixed-window behavior directly. My comment was just restating what the code already says.
Shall I remember this as a rule: "Do not flag behavior that is already explicitly documented in code comments as a known limitation or future TODO"?
No description provided.