feat(digest): daily digest notification mode#2614
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR introduces a daily/twice-daily/weekly digest notification mode as an alternative to real-time alerts. The implementation is full-stack: Convex schema and mutations for storing digest preferences, a new The overall architecture is solid and follows established patterns in the codebase (relay shared-secret auth, timing-safe comparisons, per-channel SSRF guards). However, two functional bugs were found in the new cron script:
Additional minor findings:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Browser / Client
participant Edge as Vercel Edge (api/notification-channels.ts)
participant Convex as Convex HTTP (convex/http.ts)
participant DB as Convex DB (alertRules)
participant Cron as Railway Cron (seed-digest-notifications.mjs)
participant Redis as Upstash Redis
participant Ch as Notification Channel
Note over Client,DB: Setting digest mode
Client->>Edge: POST /api/notification-channels {action: set-digest-settings}
Edge->>Edge: Validate Clerk JWT
Edge->>Convex: POST /relay/notification-channels {action: set-digest-settings, userId, digestMode, ...}
Convex->>Convex: Verify RELAY_SHARED_SECRET
Convex->>DB: setDigestSettingsForUser (upsert alertRules)
DB-->>Convex: ok
Convex-->>Edge: {ok: true}
Edge-->>Client: {ok: true}
Note over Cron,Ch: Every 30 min — digest cron run
Cron->>Convex: GET /relay/digest-rules (Bearer RELAY_SECRET)
Convex->>DB: getDigestRules (by_enabled index + in-memory filter)
DB-->>Convex: rules[]
Convex-->>Cron: rules[]
loop For each rule
Cron->>Redis: GET digest:last-sent:v1:{userId}:{variant}
Redis-->>Cron: lastSentAt
Cron->>Cron: isDue(rule, lastSentAt)?
Cron->>Redis: ZRANGEBYSCORE digest:accumulator:v1:{variant} windowStart now
Redis-->>Cron: hashes[]
Cron->>Redis: pipeline HGETALL story:track:v1:{hash} × N
Redis-->>Cron: track data
Cron->>Redis: pipeline SMEMBERS story:sources:v1:{hash} × top30
Redis-->>Cron: sources
Cron->>Convex: POST /relay/channels {userId}
Convex-->>Cron: channels[]
Cron->>Ch: send (Telegram / Slack / Discord / Email)
Ch-->>Cron: ok / 403 / 404
alt Channel gone (403/404/410)
Cron->>Convex: POST /relay/deactivate {userId, channelType}
end
Cron->>Redis: SET digest:last-sent:v1:{userId}:{variant} EX 691200
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * 7. Updates digest:last-sent:v1:${userId}:${variant} | ||
| */ | ||
| import { createRequire } from 'node:module'; | ||
| import { createHash } from 'node:crypto'; |
There was a problem hiding this comment.
createHash is imported from node:crypto but never referenced anywhere in the file.
Suggested change
| import { createHash } from 'node:crypto'; | |
| // (remove this line entirely) |
Comment on lines
+50
to
+64
| async function upstashRest(...args) { | ||
| const res = await fetch(`${UPSTASH_URL}/${args.map(encodeURIComponent).join('/')}`, { | ||
| method: 'POST', | ||
| headers: { | ||
| Authorization: `Bearer ${UPSTASH_TOKEN}`, | ||
| 'User-Agent': 'worldmonitor-digest/1.0', | ||
| }, | ||
| }); | ||
| if (!res.ok) { | ||
| console.warn(`[digest] Upstash error ${res.status} for command ${args[0]}`); | ||
| return null; | ||
| } | ||
| const json = await res.json(); | ||
| return json.result; | ||
| } |
There was a problem hiding this comment.
upstashRest has no timeout
upstashPipeline guards its fetch with AbortSignal.timeout(15000), but upstashRest (used for ZRANGEBYSCORE, GET, and SET calls) has no timeout. A slow or hung Upstash connection would stall the entire cron process indefinitely. Consider adding the same guard:
Suggested change
| async function upstashRest(...args) { | |
| const res = await fetch(`${UPSTASH_URL}/${args.map(encodeURIComponent).join('/')}`, { | |
| method: 'POST', | |
| headers: { | |
| Authorization: `Bearer ${UPSTASH_TOKEN}`, | |
| 'User-Agent': 'worldmonitor-digest/1.0', | |
| }, | |
| }); | |
| if (!res.ok) { | |
| console.warn(`[digest] Upstash error ${res.status} for command ${args[0]}`); | |
| return null; | |
| } | |
| const json = await res.json(); | |
| return json.result; | |
| } | |
| async function upstashRest(...args) { | |
| const res = await fetch(`${UPSTASH_URL}/${args.map(encodeURIComponent).join('/')}`, { | |
| method: 'POST', | |
| headers: { | |
| Authorization: `Bearer ${UPSTASH_TOKEN}`, | |
| 'User-Agent': 'worldmonitor-digest/1.0', | |
| }, | |
| signal: AbortSignal.timeout(10000), | |
| }); | |
| if (!res.ok) { | |
| console.warn(`[digest] Upstash error ${res.status} for command ${args[0]}`); | |
| return null; | |
| } | |
| const json = await res.json(); | |
| return json.result; | |
| } |
Comment on lines
+63
to
+65
| return json.result; | ||
| } | ||
|
|
There was a problem hiding this comment.
digestHour has no range validation
The schema comment says // 0-23 local hour but neither the Convex validator (v.number()), the HTTP action, nor the edge function enforce that digestHour falls in 0–23. Storing digestHour: 100 would pass all validation and silently prevent every digest from ever firing (no hour will match 100). A bounds check should be added at the Convex mutation level:
// convex/alertRules.ts — setDigestSettings / setDigestSettingsForUser args
digestHour: v.optional(v.number()),Consider replacing with a union of literals or adding an explicit check:
handler: async (ctx, args) => {
if (args.digestHour !== undefined && (args.digestHour < 0 || args.digestHour > 23 || !Number.isInteger(args.digestHour))) {
throw new ConvexError("digestHour must be an integer 0–23");
}
// ...
koala73
added a commit
that referenced
this pull request
Apr 2, 2026
…ugh internal mutation - convex/alertRules.ts: add IANA timezone validation to setDigestSettingsForUser (internalMutation called by http.ts); the public mutation already validated but the edge/relay path bypassed it - preferences-content.ts: add VITE_DIGEST_CRON_ENABLED browser flag; when =0, disable the digest mode select and show only Real-time with a note so users cannot enter a blackhole state where the relay skips their rule and the cron never runs Addresses P1 and P2 review findings on #2614
- convex/schema.ts: add digestMode/digestHour/digestTimezone to alertRules - convex/alertRules.ts: setDigestSettings mutation, setDigestSettingsForUser internal mutation, getDigestRules internal query - convex/http.ts: GET /relay/digest-rules for Railway cron; set-digest-settings action in /relay/notification-channels - cache-keys.ts: DIGEST_LAST_SENT_KEY + DIGEST_ACCUMULATOR_TTL (48h); fix accumulator EXPIRE to use 48h instead of 7-day STORY_TTL - notification-relay.cjs: skip digest-mode rules in processEvent — prevents daily/weekly users from receiving both real-time and digest messages - seed-digest-notifications.mjs: new Railway cron (every 30 min) — queries due rules, ZRANGEBYSCORE accumulator, batch HGETALL story tracks, derives phase, formats digest per channel, updates digest:last-sent - notification-channels.ts: DigestMode type, digest fields on AlertRule, setDigestSettings() client function - api/notification-channels.ts: set-digest-settings action
… on confirmed delivery isDue() only checked a single hour slot, so twice_daily users got one digest per day instead of two. Now checks both primaryHour and (primaryHour+12)%24 for twice_daily. All four send functions returned void and errors were swallowed, causing dispatched=true to be set unconditionally. Replaced with boolean returns and anyDelivered guard so lastSentKey is only written when at least one channel confirms a 2xx delivery.
…Hour, minor cleanup /relay/deactivate was rejecting channelType="discord" with 400, so stale Discord webhooks were never auto-deactivated. Added "discord" to the validation guard. Added 0-23 integer bounds check for digestHour in both setDigestSettings mutations to reject bad values at the DB layer rather than silently storing them. Removed unused createHash import and added AbortSignal.timeout(10000) to upstashRest to match upstashPipeline and prevent cron hangs.
…ation, and Digest Mode UI - seed-digest-notifications.mjs: exit 0 when DIGEST_CRON_ENABLED=0 so Railway cron does not error on intentionally disabled runs - convex/alertRules.ts: validate digestTimezone via Intl.DateTimeFormat; throw ConvexError with descriptive message for invalid IANA strings - preferences-content.ts: add Digest Mode section with mode select (realtime/ daily/twice_daily/weekly), delivery hour select, and timezone input; details panel hidden in realtime mode; wired to setDigestSettings with 800ms debounce Fixes gaps F, G, I from docs/plans/2026-04-02-003-fix-news-alerts-pr-gaps-plan.md
…ugh internal mutation - convex/alertRules.ts: add IANA timezone validation to setDigestSettingsForUser (internalMutation called by http.ts); the public mutation already validated but the edge/relay path bypassed it - preferences-content.ts: add VITE_DIGEST_CRON_ENABLED browser flag; when =0, disable the digest mode select and show only Real-time with a note so users cannot enter a blackhole state where the relay skips their rule and the cron never runs Addresses P1 and P2 review findings on #2614
Dark theme (#0a0a0a bg, #111 cards), #4ade80 green accent, 4px top bar, table-based logo header, severity-bucketed story cards with colored left borders, stats row (total/critical/high), green CTA button. Plain text fallback preserved for Telegram/Slack/Discord channels.
Covers three paths flagged as untested by reviewers: - VITE_DIGEST_CRON_ENABLED gates digest-mode options and usDigestDetails visibility - setDigestSettings (public) validates digestTimezone via Intl.DateTimeFormat - setDigestSettingsForUser (internalMutation) also validates digestTimezone to prevent silent bypass through the edge-to-Convex path
koala73
force-pushed
the
feat/news-alerts-daily-digest
branch
from
184bb72 to
d29dc39
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
notification-relay.cjsskips digest-mode rules so users don't get double-notifiedseed-digest-notifications.mjsruns every 30 min, compiles digests from the Redis accumulator (E3), and dispatches to all configured channelsChanges by layer
Convex (schema/mutations/HTTP)
schema.ts:digestMode/digestHour/digestTimezoneoptional fields onalertRules(absent = realtime)constants.ts:digestModeValidatorunion (realtime | daily | twice_daily | weekly)alertRules.ts:setDigestSettingsmutation,setDigestSettingsForUserinternal mutation,getDigestRulesinternal queryhttp.ts:GET /relay/digest-rules(Railway auth viaRELAY_SHARED_SECRET);set-digest-settingsaction in/relay/notification-channelsShared/server
cache-keys.ts:DIGEST_LAST_SENT_KEY+DIGEST_ACCUMULATOR_TTL(48h); fixes accumulatorEXPIREto use 48h instead of 7-daySTORY_TTLRelay
notification-relay.cjs: adds!r.digestMode || r.digestMode === 'realtime'guard toprocessEventfilterNew Railway cron
seed-digest-notifications.mjs: fetches due rules →ZRANGEBYSCOREaccumulator → batchHGETALLstory tracks → derive phase → format per channel → dispatch → updatedigest:last-sent:v1:${userId}:${variant}(8-day TTL)isDue()with per-mode minimum intervals (23h daily, 11h twice-daily, 6.5d weekly) to prevent double-sends on 30-min cron cadencelastSentAtfrom RedisClient
notification-channels.ts:DigestModetype, digest fields onAlertRule,setDigestSettings()functionapi/notification-channels.ts:set-digest-settingsaction in Vercel edgeDependencies
Stacks on top of PR #2604 (E3 story tracking) — the
digest:accumulator:v1:${variant}sorted set written there is the data source for digest content.Test plan
npm run typecheck+npm run typecheck:api— PASSnpm run test:data(2729 tests) — PASSnode --test tests/edge-functions.test.mjs(146 tests) — PASSnpm run lint:md— 0 errorsnpm run version:check— OKdigest:accumulatorhas entries in RedisdigestMode: dailyon a test rule, runseed-digest-notifications.mjsmanually, confirm delivery anddigest:last-sentkey writtenPost-Deploy Monitoring & Validation
[digest] Cron run start/completelines in Railway cron service logs every 30 min[digest] Sent N stories to <userId>on scheduled runs;[digest] No digest rules foundwhen no users have opted in yet[digest] Fatal:exits with code 1 (Railway restarts); repeatedUpstash errormeans Redis connectivity issuedigestMode: daily, check RedisZCARD digest:accumulator:v1:full> 0, then run cron manually and verifydigest:last-sent:v1:<userId>:fullis set