[#400] Add issue reminder cron and Blade issue page#424
Open
[#400] Add issue reminder cron and Blade issue page#424
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Next.js issue detail page, a new daily issue-reminders cron that posts grouped Discord reminders, new cron env vars and scheduling, API eager-loading for issue relations, and a DB relation linking Issue → Roles for team lookup. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as Cron Scheduler
participant Cron as IssueReminders
participant DB as Database
participant Roles as Roles Table
participant Formatter as Message Formatter
participant Webhook as Discord Webhook
Scheduler->>Cron: trigger daily @ 09:00
Cron->>DB: fetch non-finished dated issues (with relations)
DB-->>Cron: issues (includes team, visibility, assignees)
Cron->>Roles: resolve team -> discord role & channel mappings
Cron->>Cron: compute reminder bucket (14/7/3/1/overdue)
Cron->>Formatter: group by channel and reminder day, build markdown with links & mentions
Formatter-->>Cron: messages per channel (split to <=2000 chars)
Cron->>Webhook: POST each message chunk to configured webhooks (with retries)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds daily issue reminder support in the cron app and introduces a minimal Blade issue detail page for reminder deep-links, alongside the necessary DB/API relation updates to fetch team visibility and assignees.
Changes:
- Add a new
issue-reminderscron that groups issues by due buckets (14/7/3/1/overdue) and formats Discord-ready reminder messages with mentions + Blade links. - Add
/issues/[id]Blade page to display issue details fromapi.issues.getIssue. - Extend DB relations and API
getIssueto include owning team, visible teams, and assignees.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schemas/relations.ts | Adds Issue -> Roles relation for owning team. |
| packages/api/src/routers/issues.ts | Expands getIssue query to include team + visibility + assignees. |
| apps/cron/src/index.ts | Schedules the new issue-reminders cron. |
| apps/cron/src/env.ts | Adds required env vars for issue reminder webhooks and Blade base URL. |
| apps/cron/src/crons/issue-reminders.ts | Implements reminder grouping/formatting and (currently commented) webhook sending. |
| apps/blade/src/app/issues/[id]/page.tsx | Adds minimal issue detail page for reminder links. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/cron/src/env.ts (1)
12-16: Validate the new URL-shaped env vars as URLs.
z.string()accepts malformed webhook endpoints andBLADE_URL, so config mistakes won't surface until reminder delivery or link generation. Tightening these toz.string().url()will fail fast during startup instead.Suggested diff
- DISCORD_WEBHOOK_ISSUE_TEAMS: z.string(), - DISCORD_WEBHOOK_ISSUE_DIRECTORS: z.string(), - DISCORD_WEBHOOK_ISSUE_DESIGN: z.string(), - DISCORD_WEBHOOK_ISSUE_HACKORG: z.string(), - BLADE_URL: z.string(), + DISCORD_WEBHOOK_ISSUE_TEAMS: z.string().url(), + DISCORD_WEBHOOK_ISSUE_DIRECTORS: z.string().url(), + DISCORD_WEBHOOK_ISSUE_DESIGN: z.string().url(), + DISCORD_WEBHOOK_ISSUE_HACKORG: z.string().url(), + BLADE_URL: z.string().url(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/env.ts` around lines 12 - 16, The listed environment schema entries (DISCORD_WEBHOOK_ISSUE_TEAMS, DISCORD_WEBHOOK_ISSUE_DIRECTORS, DISCORD_WEBHOOK_ISSUE_DESIGN, DISCORD_WEBHOOK_ISSUE_HACKORG, and BLADE_URL) are currently defined with z.string() and should be tightened to z.string().url() so malformed webhook endpoints and BLADE_URL are rejected at startup; update the env schema in apps/cron/src/env.ts to replace z.string() with z.string().url() for those symbols so the zod validation fails fast on invalid URLs.apps/cron/src/crons/issue-reminders.ts (1)
25-35: Avoid keying channel routing off seeded role UUIDs.These values couple the cron to one specific
Roles.idset. If a role is recreated or IDs differ across environments,getIssueReminderChannel()returnsnulland the reminder is silently dropped. A stable key like role name, enum, or config-backed mapping is safer here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/crons/issue-reminders.ts` around lines 25 - 35, The ISSUE_TEAM_CHANNEL_MAP currently keys off hard-coded Role UUIDs which breaks when Roles.id changes; replace those UUID keys with a stable identifier (e.g., role name or an enum) and move mapping into config or a constants enum (instead of literal UUIDs), then update the lookup call in getIssueReminderChannel() to resolve role -> stable identifier (e.g., read role.name or role.enumValue) before using ISSUE_TEAM_CHANNEL_MAP, and add a safe fallback/log when no mapping exists; ensure references to ISSUE_TEAM_CHANNEL_MAP and getIssueReminderChannel() are updated together so the map keys and the lookup key type match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blade/src/app/issues/`[id]/page.tsx:
- Around line 17-18: Validate the route param before fetching and translate
missing/invalid issues to the route not-found state: ensure the extracted id
from params is checked (e.g., using your app's ID validation helper or a simple
regex/parse) and return the route not-found (call the framework's notFound
handler) if the id is malformed, then wrap the api.issues.getIssue({ id }) call
in a try/catch and call notFound() when the API throws a "not found" or returns
no record so stale/missing /issues/[id] links produce a clean 404.
In `@apps/cron/src/crons/issue-reminders.ts`:
- Around line 235-237: The cron issueReminders is scheduled
(issueReminders.schedule() is registered) but the actual delivery is disabled
because the WebhookClient.send call in issue-reminders.ts is commented out;
either re-enable the send call to deliver reminders or gate scheduling behind an
explicit dry-run/feature flag. Update the code in issue-reminders.ts to restore
the WebhookClient({ url: ISSUE_REMINDER_WEBHOOK_URLS[channel] }).send({ content:
msg }) call (or add a configurable flag such as ISSUE_REMINDERS_ENABLED/dryRun
that the scheduler checks before scheduling/executing reminders) so scheduled
runs perform real sends only when enabled.
- Around line 160-164: formatIssueReminder currently builds a Markdown masked
link which Discord webhooks do not render; update the function
(formatIssueReminder) to use a plain URL instead of `[name](url)` by
concatenating target.issueName, the URL from getIssueUrl(target.issueId), and
mentions from getIssueMentionTargets(target) into a plain-text string like " -
{issueName}: {issueUrl} {mentions}" so Discord will auto-link the URL.
---
Nitpick comments:
In `@apps/cron/src/crons/issue-reminders.ts`:
- Around line 25-35: The ISSUE_TEAM_CHANNEL_MAP currently keys off hard-coded
Role UUIDs which breaks when Roles.id changes; replace those UUID keys with a
stable identifier (e.g., role name or an enum) and move mapping into config or a
constants enum (instead of literal UUIDs), then update the lookup call in
getIssueReminderChannel() to resolve role -> stable identifier (e.g., read
role.name or role.enumValue) before using ISSUE_TEAM_CHANNEL_MAP, and add a safe
fallback/log when no mapping exists; ensure references to ISSUE_TEAM_CHANNEL_MAP
and getIssueReminderChannel() are updated together so the map keys and the
lookup key type match.
In `@apps/cron/src/env.ts`:
- Around line 12-16: The listed environment schema entries
(DISCORD_WEBHOOK_ISSUE_TEAMS, DISCORD_WEBHOOK_ISSUE_DIRECTORS,
DISCORD_WEBHOOK_ISSUE_DESIGN, DISCORD_WEBHOOK_ISSUE_HACKORG, and BLADE_URL) are
currently defined with z.string() and should be tightened to z.string().url() so
malformed webhook endpoints and BLADE_URL are rejected at startup; update the
env schema in apps/cron/src/env.ts to replace z.string() with z.string().url()
for those symbols so the zod validation fails fast on invalid URLs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 35f65c99-0d94-4da8-9de8-3b7a8d1d3db1
📒 Files selected for processing (6)
apps/blade/src/app/issues/[id]/page.tsxapps/cron/src/crons/issue-reminders.tsapps/cron/src/env.tsapps/cron/src/index.tspackages/api/src/routers/issues.tspackages/db/src/schemas/relations.ts
…nders enabled flag
…nightHacks/forge into sprint-01/cron-issue-reminders
…ng when channel doesnt exist
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cron/src/crons/issue-reminders.ts`:
- Around line 82-99: The getIssueReminderDay function currently normalizes dates
with setHours which uses the process local timezone; change it to normalize both
the due date and now to UTC by building UTC midnight timestamps (use
Date.UTC(year, month, day) or equivalent) and use those UTC ms values to compute
diffDays, ensuring consistent reminder bucketing; update the similar
normalization at the other occurrence (lines referenced in the review) and add a
short comment documenting the UTC assumption for cron runs and date parsing;
keep references to ISSUE_REMINDER_DAYS and return behavior unchanged.
- Around line 173-185: formatChannelReminderMsg can produce messages >2000 chars
which causes Discord webhook sends to fail and abort the whole cron; update the
send logic (the webhook send call near the block that invokes
formatChannelReminderMsg) to (1) split the returned string into 2000-char-safe
chunks (preserving section boundaries from ISSUE_REMINDER_DAY_ORDER /
ISSUE_REMINDER_DAY_LABELS and using formatIssueReminder for lines), (2) send
each chunk independently to the webhook in a try/catch so one failure doesn't
stop other channels, and (3) add retry-with-backoff (e.g., limited attempts with
increasing delay) around each chunk send while logging failures via the same
logger (include the channel/webhook id and error). Ensure you reference
formatChannelReminderMsg, formatIssueReminder, and the webhook send call so the
changes are applied where messages are constructed and dispatched.
- Around line 167-170: The issue is that user-controlled issueName is inserted
raw into webhook content and Discord will parse mentions/newlines; update
formatIssueReminder to sanitize the title by escaping Discord mention syntax
(<@...>, <@&...>, <!here>, <!everyone>) and replacing newlines with a single
space, e.g. operate on target.issueName before composing the string in
formatIssueReminder; then when sending the webhook (where you currently use
getIssueMentionTargets and post the message around lines handling sending), set
the Discord webhook payload's allowedMentions to explicitly include only the
resolved user IDs and role IDs returned by getIssueMentionTargets (do not rely
on default parsing), so only intended assignees/teams can be pinged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 560135b9-6611-47a8-95ee-09b3b826e8fc
📒 Files selected for processing (2)
apps/blade/src/app/issues/[id]/page.tsxapps/cron/src/crons/issue-reminders.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/blade/src/app/issues/[id]/page.tsx
…ok message length
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/cron/src/crons/issue-reminders.ts (4)
393-394:formatChannelReminderMsgresult is unused beyond the null check.
formatChannelReminderMsgbuilds the full message string, but the actual sending usessplitChannelReminderMessages. This is a minor redundancy—you could simplify by checking ifsplitChannelReminderMessagesreturns an empty array instead:Suggested simplification
- const msg = formatChannelReminderMsg(groupedChannel); - if (!msg) continue; - const chunks = splitChannelReminderMessages(groupedChannel); + if (chunks.length === 0) continue; + for (const chunk of chunks) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/crons/issue-reminders.ts` around lines 393 - 394, The code currently calls formatChannelReminderMsg(groupedChannel) and only uses it to check for null while the actual send uses splitChannelReminderMessages; remove this redundant call and instead call splitChannelReminderMessages(groupedChannel) (or call splitChannelReminderMessages(formatChannelReminderMsg(groupedChannel)) if that overload exists) and check that the returned array is non-empty before proceeding; update the loop to use the split messages directly for sending and remove the unused msg variable and its null check.
26-36: Hardcoded team UUIDs reduce maintainability.These UUIDs couple the cron tightly to specific database records. If teams change or new ones are added, this file must be updated and redeployed.
Consider moving this mapping to environment variables or the database (e.g., adding a
discordChannelcolumn to the team/roles table) so channel routing can be configured without code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/crons/issue-reminders.ts` around lines 26 - 36, The hardcoded mapping ISSUE_TEAM_CHANNEL_MAP ties team UUIDs to channels in code; move this out of source by loading the mapping at runtime (e.g., from environment/config or the DB) and use ISSUE_REMINDER_CHANNELS as the target enum; implement a loader function (e.g., loadIssueTeamChannelMap) that reads a JSON env var or queries the teams table (add a discordChannel column if using DB), validate keys are known team IDs and values map to keyof typeof ISSUE_REMINDER_CHANNELS, and replace direct use of ISSUE_TEAM_CHANNEL_MAP with the loaded map wherever referenced.
310-316: MoveWebhookClientinstantiation outside the retry loop.Creating a new
WebhookClienton each retry attempt is slightly inefficient. While webhooks are stateless, reusing the client avoids repeated object creation.Suggested refactor
const sendIssueReminderChunk = async ( channel: keyof typeof ISSUE_REMINDER_CHANNELS, webhookUrl: string, chunk: { content: string; targets: IssueReminderTarget[] }, ) => { const webhookId = getWebhookId(webhookUrl); + const webhook = new WebhookClient({ url: webhookUrl }); for (let attempt = 1; attempt <= ISSUE_REMINDER_SEND_ATTEMPTS; attempt++) { try { - await new WebhookClient({ url: webhookUrl }).send({ + await webhook.send({ content: chunk.content, allowedMentions: getAllowedMentions(chunk.targets), }); return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/crons/issue-reminders.ts` around lines 310 - 316, The loop currently creates a new WebhookClient on every retry; instantiate the WebhookClient once before the for-loop using the existing webhookUrl (e.g., const webhook = new WebhookClient({ url: webhookUrl })) and then call webhook.send(...) inside the retry loop (passing chunk.content and allowedMentions from getAllowedMentions(chunk.targets)); keep the retry logic and return behavior unchanged and reference ISSUE_REMINDER_SEND_ATTEMPTS, WebhookClient, webhookUrl, send, chunk.content, and getAllowedMentions when making this change.
333-333: Document the cron schedule's timezone assumption.The schedule
"0 9 * * *"runs at 9 AM in the server's local timezone. Since date calculations use UTC (viagetUtcMidnightTimestamp), there could be edge-case mismatches if the server isn't running in UTC.Suggested documentation
-}).addCron("0 9 * * *", async () => { +// Runs daily at 09:00 UTC. Date math uses UTC midnight to ensure consistent bucket assignment. +}).addCron("0 9 * * *", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cron/src/crons/issue-reminders.ts` at line 333, The cron schedule string "0 9 * * *" passed to addCron runs in the server's local timezone but the code uses UTC-based calculations via getUtcMidnightTimestamp; add a short comment immediately above the addCron invocation noting this timezone assumption and the potential mismatch, and then either change the cron configuration to run in UTC (use your scheduler's TZ option or prefix with TZ=UTC) or ensure the host process runs in UTC so the schedule aligns with getUtcMidnightTimestamp; reference addCron and getUtcMidnightTimestamp in the comment so future readers can spot the coupling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cron/src/crons/issue-reminders.ts`:
- Around line 267-283: The loop that builds message chunks (using sectionLines,
targets, sectionChunkContent and MAX_DISCORD_MESSAGE_LENGTH) doesn't handle the
case where a single formatted reminder line (from formatIssueReminder) plus
header already exceeds MAX_DISCORD_MESSAGE_LENGTH; detect when line length +
header length > MAX_DISCORD_MESSAGE_LENGTH and in that case truncate the
per-issue content (e.g., sanitize and trim the issue title / assignee list
produced by formatIssueReminder to a safe max) or replace it with a shortened
placeholder before adding to a chunk so you never attempt to push an individual
message part larger than MAX_DISCORD_MESSAGE_LENGTH; ensure the truncation
happens where you build nextSectionChunkContent and update sectionChunkTargets
accordingly so the rest of the chunking logic remains unchanged.
---
Nitpick comments:
In `@apps/cron/src/crons/issue-reminders.ts`:
- Around line 393-394: The code currently calls
formatChannelReminderMsg(groupedChannel) and only uses it to check for null
while the actual send uses splitChannelReminderMessages; remove this redundant
call and instead call splitChannelReminderMessages(groupedChannel) (or call
splitChannelReminderMessages(formatChannelReminderMsg(groupedChannel)) if that
overload exists) and check that the returned array is non-empty before
proceeding; update the loop to use the split messages directly for sending and
remove the unused msg variable and its null check.
- Around line 26-36: The hardcoded mapping ISSUE_TEAM_CHANNEL_MAP ties team
UUIDs to channels in code; move this out of source by loading the mapping at
runtime (e.g., from environment/config or the DB) and use
ISSUE_REMINDER_CHANNELS as the target enum; implement a loader function (e.g.,
loadIssueTeamChannelMap) that reads a JSON env var or queries the teams table
(add a discordChannel column if using DB), validate keys are known team IDs and
values map to keyof typeof ISSUE_REMINDER_CHANNELS, and replace direct use of
ISSUE_TEAM_CHANNEL_MAP with the loaded map wherever referenced.
- Around line 310-316: The loop currently creates a new WebhookClient on every
retry; instantiate the WebhookClient once before the for-loop using the existing
webhookUrl (e.g., const webhook = new WebhookClient({ url: webhookUrl })) and
then call webhook.send(...) inside the retry loop (passing chunk.content and
allowedMentions from getAllowedMentions(chunk.targets)); keep the retry logic
and return behavior unchanged and reference ISSUE_REMINDER_SEND_ATTEMPTS,
WebhookClient, webhookUrl, send, chunk.content, and getAllowedMentions when
making this change.
- Line 333: The cron schedule string "0 9 * * *" passed to addCron runs in the
server's local timezone but the code uses UTC-based calculations via
getUtcMidnightTimestamp; add a short comment immediately above the addCron
invocation noting this timezone assumption and the potential mismatch, and then
either change the cron configuration to run in UTC (use your scheduler's TZ
option or prefix with TZ=UTC) or ensure the host process runs in UTC so the
schedule aligns with getUtcMidnightTimestamp; reference addCron and
getUtcMidnightTimestamp in the comment so future readers can spot the coupling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 67a47b7d-7a23-4484-ba94-b535851db7a4
📒 Files selected for processing (1)
apps/cron/src/crons/issue-reminders.ts
| DISCORD_WEBHOOK_ISSUE_DIRECTORS: z.string().url().optional(), | ||
| DISCORD_WEBHOOK_ISSUE_DESIGN: z.string().url().optional(), | ||
| DISCORD_WEBHOOK_ISSUE_HACKORG: z.string().url().optional(), | ||
| ISSUE_REMINDERS_ENABLED: z.enum(["true", "false"]).optional(), |
Contributor
There was a problem hiding this comment.
Instead of optional, .default("false")? Probably out of scope
| export const issueReminders = new CronBuilder({ | ||
| name: "issue-reminders", | ||
| color: 2, | ||
| // This addCron schedule runs in the host timezone; keep the host in UTC so it aligns with getUtcMidnightTimestamp grouping. |
Contributor
There was a problem hiding this comment.
Server should be on eastern time. Dylan can confirm this. But the crons run as we would expect, not on UTC.
| const webhookId = getWebhookId(webhookUrl); | ||
| const webhook = new WebhookClient({ url: webhookUrl }); | ||
|
|
||
| for (let attempt = 1; attempt <= ISSUE_REMINDER_SEND_ATTEMPTS; attempt++) { |
Contributor
There was a problem hiding this comment.
I think this is kind of useless to be honest. We don't expect the webhook request to fail, right?
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
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.
Why
Adds issue reminder cron support and a minimal Blade issue page for reminder links.
What
Closes: #400
issue-reminderscron/issues/[id]issues.getIssueto include assignees, visible teams, and owning teamNotes:
ISSUE_REMINDERS_ENABLEDgates delivery, so reminders stay disabled by default until production webhook config is ready.Test Plan
Teamsand mentioned the assigned userDesignand mentioned the team role/issues/[id]Checklist
db:pushbefore mergingSummary by CodeRabbit
New Features
Chores