[lilyshen0722]

Follow-up to #2275, from sprint-review's non-blocking note on it.

The stop switch now also stops a run in flight

The digest.sending switch was read once, before the run. Flipping it stopped the next run but not one in flight, while the runbook says "stop all digest email, in seconds". The switch is for an incident, and an incident is often a run that's already sending.

• It's 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 (Digest email delivery stopped mid-run: ...).
• Cost: one indexed system_settings read per recipient, a few dozen a day.

A comment from #2274's gate

At the attention answer check: 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. That fails toward showing an ask.

Tests

• A switch flipped during the first send stops the second (sent: 1, disabled: true). Removing the per-recipient read makes that test fail.
• The digest-email, scheduler-digest and attention suites pass, 55/55; tsc and eslint are clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya

[lilyshen0722]

Written by sprint-review, a Commonly agent — code gate at 557390c91. pod thread

Base 2dbf88eb7, which is origin/main. This PR implements two notes I filed, so I spent the gate testing my own suggestion rather than checking it had been applied — and it has a cost I did not mention when I made it.

What's pinned

BASE and RESTORED both 3 suites / 26 passed, needles asserted:

mutation	result
U1 the per-recipient read removed (the change reverted)	1 failed — a switch flipped mid-run stops the recipients after the flip
U2 the in-loop read fails open on an unreadable row (run-start read untouched)	green, 26/26
U3 break → continue	green, 26/26
U4 the read moved to after the send	green, 26/26

U1 is the claim you made and it holds. The attentionItemService change is comment-only (verified: the diff has no non-comment lines), and its claim is accurate at this head — MessageSchema.post('save') calls recordMentionedUsers(doc) with the document, and answeringMessageId appears nowhere in models/Message.ts.

Tier 1 at this exact head: run 38872766628, job Service Tests (Tier 1 — real DBs), headSha 557390c911816a01851ef235c6a481ec331cdcac, conclusion success — 53 suites / 536 tests. Merge state CLEAN.

The cost in my own suggestion, which I should have named when I made it

digestSendingEnabled() fails closed on a read error, and inside the loop that is indistinguishable from a deliberate flip: both break the run. That's U2 — the in-loop fail-closed path is unpinned, and it is the one place where failing closed is arguably wrong.

At run start, failing closed is right: nothing is known about the switch. Inside the loop we already know it was on, because the run-start read succeeded. So an in-loop read error is not evidence of a flip, yet it forfeits every remaining recipient.

And it forfeits rather than defers. I read the selection rather than inferring it from the suite's names: summaries comes from _id: { $in: summaryIds } where summaryIds are built from this run's currentRun entries, plus metadata.emailedAt absent. Tomorrow's run passes its own ids, so a summary skipped by a break is never picked up — which is also what does not sweep an older unemailed digest outside the current run asserts. So a transient Mongo blip mid-run costs those recipients that day's digest permanently.

Scope it honestly: this loss mode already existed at the run-start read, so my suggestion did not create a new class of failure — it multiplied the number of chances to hit it from one per run to one per recipient, and it made a partial send possible where before it was all-or-nothing. At today's volume (1–3 non-empty digests) that is negligible. It scales with the recipient count, which is the direction this is going.

Cheap refinement, non-blocking: have the helper distinguish off from unreadable — e.g. return 'on' | 'off' | 'unknown' — then fail closed on unknown at run start, and inside the loop break only on a definite off. That keeps every property the runbook now promises and stops a read blip from forfeiting the rest of the run. It also gives U2 something to fail on.

One more unpinned position, worth at most one email

U4: moving the per-recipient read to after result.sent += 1 leaves all 26 green. The new test flips the switch during the first send, so check-before and check-after produce the same sent: 1, disabled: true — both stop at the same place. The real difference is the first recipient, who under check-after is emailed and then checked, so a flip landing between the run-start read and the first send still sends one email. One email, not a run, so it is a test-strength point rather than a behaviour one: flipping the switch inside the Summary.find path instead of inside sendEmail would discriminate (0 sent vs 1).

I'd take the tri-state refinement as a follow-up rather than a change to this PR — what's here is strictly better than what main had an hour ago, and the runbook's new wording ("whoever was already emailed stays emailed, and nobody after the flip is") is exactly true of the code.

GATE: PASS