From 557390c911816a01851ef235c6a481ec331cdcac Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:17:59 -0700 Subject: [PATCH] fix(digest): the stop switch also stops a run that is already sending sprint-review on #2275 (non-blocking): the digest.sending switch was read once, before the run, so flipping it stopped the next run and not one in flight, while the runbook said "stop all digest email, in seconds". The switch is for an incident, and an incident is often a run that is already sending. It is now read again before each recipient: whoever was already emailed stays emailed, and nobody after the flip is. The runbook says so, and names the mid-run log line. Also from sprint-review on #2274: a comment at the attention answer check that only the PG write carries answeringMessageId. The Mongo fallback's post-save hook passes the document, so with PG down an unclaimed addressed answer still opens a row, which fails toward showing an ask. Test: a switch flipped during the first send stops the second. Mutation: the per-recipient read removed reds it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya --- .../unit/services/digestEmailService.test.js | 17 +++++++++++++++++ backend/services/attentionItemService.ts | 3 +++ backend/services/digestEmailService.ts | 12 +++++++++++- docs/runbooks/daily-digest.md | 11 ++++++++--- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/backend/__tests__/unit/services/digestEmailService.test.js b/backend/__tests__/unit/services/digestEmailService.test.js index feca70acfc..941a9485a6 100644 --- a/backend/__tests__/unit/services/digestEmailService.test.js +++ b/backend/__tests__/unit/services/digestEmailService.test.js @@ -211,6 +211,23 @@ describe('digestEmailService', () => { .resolves.toEqual(expect.objectContaining({ disabled: false, sent: 1 })); }); + // sprint-review on #2275: the switch is for an incident, and an incident is + // often a run that is already sending. + it('a switch flipped mid-run stops the recipients after the flip', async () => { + const first = await createUser(); + const second = await createUser(); + const digests = [await createDigest(first), await createDigest(second)]; + mockSendEmail.mockImplementationOnce(async () => { + await SystemSetting.create({ key: digestEmailService.DIGEST_SENDING_KEY, value: { enabled: false } }); + return { data: { data: { succeeded: 1 } } }; + }); + + const result = await digestEmailService.sendDigestEmails(digests.map(runEntry)); + + expect(result).toEqual(expect.objectContaining({ sent: 1, disabled: true })); + expect(mockSendEmail).toHaveBeenCalledTimes(1); + }); + it('sends nothing when the switch cannot be read', async () => { const user = await createUser(); const digest = await createDigest(user); diff --git a/backend/services/attentionItemService.ts b/backend/services/attentionItemService.ts index 9401743a20..777adf1c3d 100644 --- a/backend/services/attentionItemService.ts +++ b/backend/services/attentionItemService.ts @@ -107,6 +107,9 @@ const peopleThisAgentIsAnswering = async ( instanceId: String(author.botMetadata?.instanceId || 'default'), replyToMessageId: message?.reply_to_message_id ?? message?.replyToMessageId, // Carried from the post, never stored on the row (agentAnswerService). + // Only the PG write carries it: the Mongo fallback's post-save hook + // (models/Message.ts) passes the document, so with PG down an unclaimed + // addressed answer still opens a row. That fails toward showing an ask. answeringMessageId: message?.answeringMessageId, }); } catch (error) { diff --git a/backend/services/digestEmailService.ts b/backend/services/digestEmailService.ts index b9ac9f81c0..1a3c352ffa 100644 --- a/backend/services/digestEmailService.ts +++ b/backend/services/digestEmailService.ts @@ -10,7 +10,9 @@ const SystemSetting = require('../models/SystemSetting'); /** * The switch that stops digest SENDING in seconds, with no deploy and no * restart: a system_settings row { key: 'digest.sending', value: { enabled: - * false } }. No row means on. Generation still runs; only the send stops. + * false } }. No row means on. Generation still runs; only the send stops. It + * is read before the run and again before each recipient, so flipping it + * also stops a run that is already sending. * docs/runbooks/daily-digest.md has the one-line flip. Earned 2026-10-16, * when the only fallback for a bad run was "land a fix before 06:00Z". */ @@ -156,6 +158,14 @@ export class DigestEmailService { result.eligible = summaries.length; for (const summary of summaries) { + // Read again per recipient (sprint-review on #2275): the switch is for an + // incident, and an incident is often a run that is already sending. + // eslint-disable-next-line no-await-in-loop + if (!await digestSendingEnabled()) { + console.warn(`Digest email delivery stopped mid-run: switched off (${DIGEST_SENDING_KEY}).`); + result.disabled = true; + break; + } const userId = summary.metadata?.userId; try { if (!userId) { diff --git a/docs/runbooks/daily-digest.md b/docs/runbooks/daily-digest.md index 0e3c791809..455e08388f 100644 --- a/docs/runbooks/daily-digest.md +++ b/docs/runbooks/daily-digest.md @@ -11,7 +11,10 @@ digest off. Sending is controlled by one `system_settings` row. A missing row means sending is **on**. Flipping it needs no deploy and no restart, and survives -later deploys. Generation still runs; only the send stops. +later deploys. Generation still runs; only the send stops. The row is read +before the run and again before each recipient, so flipping it also stops a +run that is already sending: whoever was already emailed stays emailed, and +nobody after the flip is. ```bash # OFF @@ -27,8 +30,10 @@ m.connect(process.env.MONGO_URI).then(async () => { ``` To turn it back on, run the same command with `enabled: true`, or delete the -row. If the row can't be read, nothing is sent: the switch fails closed. The -run logs `Digest email delivery skipped: switched off (digest.sending).` +row. If the row can't be read, nothing is sent: the switch fails closed. A run +logs `Digest email delivery skipped: switched off (digest.sending).` when it +starts switched off, and `Digest email delivery stopped mid-run: ...` when the +switch is flipped while it's sending. ## Before a change to digest content ships