Skip to content

fix(digest): a board line needs something to have happened; a switch stops sending in seconds - #2275

Merged
lilyshen0722 merged 1 commit into
mainfrom
growth/digest-board-moved
Oct 16, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
growth/digest-board-moved

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Found by a read-only dry run of the deployed recap digest (b6e4922, live since 07:44:49Z) before its first scheduled run, 2026-10-17 06:00Z.

What the dry run found

One email was due, to an outside stranger who'd already had one email from us. Its entire content was our own seeded onboarding checklist:

- TASK-002 Give your agent its first task is pending (My Workspace)
- TASK-001 Connect your first agent is pending (My Workspace)
- TASK-003 Invite a teammate is pending (My Workspace)

The starter tasks were created inside the 24h window, so the recap's board (updatedAt >= since) counted them. No agent had done anything. That's a nag from us, not "what your agents did".

The fix

  • A board line needs the task to have moved: a status other than pending, or an update note (lastUpdate). A task created and never touched is skipped.
  • A stop switch for digest sending, with no deploy and no restart:
    • It's one system_settings row: digest.sending = { enabled: false }.
    • No row means on, and an unreadable switch sends nothing (it fails closed).
    • Generation still runs; only the send stops.
    • Before this, the only lever for a bad run was "land a fix before 06:00Z".
  • docs/runbooks/daily-digest.md: the one-line flip, plus the rule to dry-run digest content on live data before it ships. That rule caught both content bugs so far, and both had passed every unit test.

Live data, same dry run, with this rule

before with this PR
candidates · active in 20h · away 51 · 35 · 16 51 · 34 · 17
nonempty 3 (1 outside) 1 (internal)
would be emailed 1 (outside, the checklist) 0

The remaining internal sample keeps only real movement: two agents' posts, and "TASK-001 Connect your first agent is done".

Tests

  • Board rule:
    • an untouched pending task is not news;
    • a pending task with an update note is;
    • a done task is.
  • Switch:
    • off, it sends nothing and stamps nothing;
    • on again, it sends;
    • unreadable, it sends nothing.
  • Mutations, each turning a test red:
    • the switch ignored;
    • the switch failing open;
    • untouched tasks kept.
  • Ran locally on Node 22: the digest-email, scheduler-digest and digest suites, 26/26. tsc and eslint are clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya

…stops sending in seconds

The live dry run of the deployed recap digest (b6e4922, read-only) found
one email due at the 2026-10-17 06:00Z run, to an outside stranger, and its
whole content was our own seeded onboarding checklist: three "TASK-00x ...
is pending" rows. The starter tasks were created inside the 24h window, so
the recap's board counted them. No agent had done anything.

- A board line now needs the task to have moved: a status other than
  pending, or an update note. A task created and never touched is skipped.
  On the same live data that leaves 0 emails; an internal sample keeps its
  agents' posts and a starter task that was completed.
- A stop switch for digest sending, with no deploy and no restart: one
  system_settings row, digest.sending = { enabled: false }. No row means on;
  an unreadable switch sends nothing. Generation still runs. The flip is in
  docs/runbooks/daily-digest.md, with the rule to dry-run content on live
  data before it ships.

Tests: an untouched pending task is not news, a pending task with an
update and a done task are; the switch off sends nothing and stamps
nothing, on again sends; an unreadable switch sends nothing. Mutations:
the switch ignored, the switch failing open, and untouched tasks kept
each red an arm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0157AG2wo8mJYqKezXNs48Ya

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Base b6e49229a (#2267's merge commit). origin/main has since moved to 95f7d4e35, but it has touched neither digest file — git diff b6e49229a origin/main -- dailyDigestService.ts digestEmailService.ts is empty — and merge-tree against current main gives 0 conflicts.

Both arms of the board rule are independently pinned

BASE and RESTORED both 4 suites / 26 passed, needles asserted at 1:

mutation failures
W4 untouched pending tasks kept (the defect restored) 1 — a task created and never touched is not news; one that moved is
W5 the update-note arm dropped (status alone decides) 1 — same test
W6 the status arm dropped (an update note alone decides) 3, incl. the digest is the recap: what waits on you, then agent work, then the board
W1 the switch ignored entirely 2 — sends nothing when the sending switch is off…, …when the switch cannot be read
W2 the switch fails open on an unreadable row 1 — sends nothing when the switch cannot be read
W3 an absent row read as OFF (the no-row default inverted) 8 across the send suite
W7 the neighbouring private-pod filter dropped (regression check) 2 — a public pod's … stay out, an agent update or board move with no pod stays out

W5 and W6 both red matters: status !== 'pending' || lastUpdate is a disjunction, and a disjunction is where a single green mutation usually means one arm is decorative. Here each arm carries tests the other does not.

moved() is exhaustive over the real status set, and reads fields the projection actually carries

Two things that would make this fix quietly partial, both checked rather than assumed:

  • The status set is closed. models/Task.ts declares TaskStatus = 'pending' | 'claimed' | 'done' | 'blocked' with default: 'pending', and the schema enum is the same four. So !== 'pending' is not a guess about one spelling of "not started" — pending is the only one, and claimed / done / blocked all represent something having happened. Had there been a second idle status (todo, backlog), this filter would have passed every test and still shipped the nag.
  • status and lastUpdate survive the read. The board rows come from activityService.getRecap, whose Task.find(...).select('podId taskId title status updatedAt updates') includes status and updates, and whose mapper emits lastUpdate as an object or null. A field absent from that projection would make its predicate undefined and the guard inert — worth confirming because the W4–W6 suites build recaps directly and so cannot see a projection change.

The switch covers the whole send surface

Enumerated rather than inferred. sendDigestEmails has exactly one caller — schedulerService.ts:207, inside the 0 6 * * * job — and the gate is the first statement in it, so there is no second path that sends a digest. Across the backend, sendEmail( has five non-test call sites: this one, onboardingAlertService.ts:137 (a single operator RECIPIENT(), not users), routes/admin/users.ts, and two in authController (verification / reset). So the digest is the only scheduled person-facing mail, and digest.sending stops all of it — the runbook's "Stop all digest email" is accurate, not overreach.

Fail-closed is real: the catch returns false, and row?.value?.enabled !== false means a missing row is on. The runbook's flip writes a boolean false directly, so the strict comparison matches the documented path.

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

Non-blocking

The switch is read once per run, so it stops the next run rather than one in flight. digestSendingEnabled() is called before the per-user loop; flipping the row at 06:00:30 does not halt the remainder of a run already sending. With ~51 candidates that window is seconds, so this is a wording point rather than a behaviour one — but the runbook says "Stop all digest email, in seconds" and someone reaching for it mid-incident will read that as "stop what is happening right now". Either say "takes effect from the next run" in the runbook, or move the check inside the loop (one indexed read per recipient, which at this volume is free). I'd take the wording change.

Everything the dry run found is addressed, and the 0-email figure on live data is the author's measurement, not mine — I verified the rule that produces it, not the live board.

GATE: PASS

@lilyshen0722
lilyshen0722 added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit 2dbf88e Oct 16, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant