feat: scheduled announcements — /announce command (#42)#102
feat: scheduled announcements — /announce command (#42)#102BillChirico merged 10 commits intomainfrom
Conversation
Create scheduled_messages table with columns for one-time and recurring scheduled announcements, plus a partial index on next_run for enabled messages. Part of #42
Implement minimal cron parser and scheduler that polls every 60s for due messages. Handles one-time (disable after send) and recurring (compute next_run from cron) messages. No npm dependencies added. Part of #42
- /announce once <time> <channel> <message> — one-time scheduled message - /announce recurring <cron> <channel> <message> — recurring via cron - /announce list — show all scheduled for guild - /announce cancel <id> — cancel by ID (author or moderator) - Natural time parsing: 'in Xh', 'in Xm', 'tomorrow HH:MM', 'YYYY-MM-DD HH:MM' - Moderator-only permission check via isModerator() Part of #42
Start scheduled message scheduler after client ready (alongside tempban scheduler). Stop it on graceful shutdown. Part of #42
35 new tests covering: - Command: once/recurring/list/cancel subcommands, permission denial - Time parsing: relative, tomorrow, ISO formats - Cron parser: all 5 required patterns plus validation - Scheduler: fires due messages, disables one-time, updates recurring, skips disabled, polls on interval Part of #42
|
Claude finished @BillChirico's task in 2m 29s —— View job Reviewing PR #102: feat: scheduled announcements — /announce command
ResultsPrevious fix commits addressed the critical issues from earlier reviews (deferReply, guild-scoped cancel, list truncation, parseTime validation, cron error handling, timezone column removal, allowedCommands config). 5 issues remain — 1 warning and 4 nitpicks. See the review comments for details.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds scheduled announcements: a DB migration creating Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
🔴 Critical (1)
- Missing
deferReply()before DB operations (src/commands/announce.js:135-147) — All subcommands hit the database without deferring. Discord's 3-second interaction timeout will cause silent failures on slow queries. Every other DB-hitting command in the codebase defers first.
🟡 Warnings (3)
timezonecolumn stored but never used (migrations/002_scheduled-messages.cjs:19,src/modules/scheduler.js:17-19) — The migration adds a timezone column (defaultAmerica/New_York), but cron computation uses server-localDatemethods. The column is misleading.handleListoutput unbounded (src/commands/announce.js:285-295) — With many scheduled messages, output exceeds 2000 chars and is silently truncated bysafeReply. Users lose data with no indication.- No documentation updates — AGENTS.md states "Keep docs up to date — this is non-negotiable." None of
README.md,AGENTS.md(Key Files table),config.json(allowedCommands), or.env.examplewere updated for the new command/module/table.
🔵 Nitpick (1)
- Cancel query should filter by
guild_idin SQL (src/commands/announce.js:303-305) — Currently fetches across all guilds then checks in app code. Addingguild_idto WHERE is more efficient.
AI fix prompt (copy-paste into an AI agent)
Fix the following issues in the feat/scheduled-announcements branch of VolvoxLLC/volvox-bot:
-
src/commands/announce.js— Addawait interaction.deferReply({ ephemeral: true })after theisModeratorcheck (around line 145) and before getting the subcommand. Then change ALLsafeReply(interaction, ...)calls inhandleOnce,handleRecurring,handleList, andhandleCanceltosafeEditReply(interaction, ...). ImportsafeEditReplyfrom../utils/safeSend.js. -
src/commands/announce.jslines 303-305 — Change the cancel query to includeguild_id:'SELECT id, author_id, guild_id FROM scheduled_messages WHERE id = $1 AND guild_id = $2 AND enabled = true'with params[id, interaction.guildId]. Remove the separate guild_id check on lines 319-326. -
src/commands/announce.jslines 285-295 — Cap the list output to avoid silent truncation. After fetching rows, slice to at most 15 entries. If there are more, append a line like"… and {remaining} more. Use cancel to remove old entries.". -
migrations/002_scheduled-messages.cjsline 19 — Remove thetimezone TEXT NOT NULL DEFAULT 'America/New_York'column since it's not used by any code. If timezone support is planned for later, add it in a future migration when the code actually uses it. -
config.json— Add"announce": "moderator"to thepermissions.allowedCommandsobject. -
AGENTS.md— Add entries forsrc/commands/announce.jsandsrc/modules/scheduler.jsto the Key Files table. Addscheduled_messagesto the Database Tables section.
|
| Filename | Overview |
|---|---|
| src/commands/announce.js | Solid implementation with good input validation (parseTime validates day-of-month correctly). Permission checks are proper. Minor: no rate limiting on message creation, no upfront warning that long messages will be split. |
| src/modules/scheduler.js | Core scheduler logic works well. Has re-entrancy guard and 2-year cron search limit. Multiple bot instances would cause duplicate sends (architectural concern). Previous threads cover most issues (cron errors, send failures). |
| migrations/002_scheduled-messages.cjs | Clean migration with proper index on next_run. Previous threads note embed_json unused, timezone removed, and missing guild_id index for list queries. |
| tests/commands/announce.test.js | Comprehensive test coverage with 17 tests covering all subcommands, permission checks, time parsing formats, and error cases. |
| tests/modules/scheduler.test.js | Excellent test coverage with 18 tests for cron parsing, next run computation, and scheduler lifecycle. Uses fake timers properly. |
Sequence Diagram
sequenceDiagram
participant User
participant Discord
participant AnnounceCmd as /announce command
participant DB as PostgreSQL
participant Scheduler
participant Channel
User->>Discord: /announce once "in 2h" #general "Hello"
Discord->>AnnounceCmd: interaction
AnnounceCmd->>AnnounceCmd: parseTime("in 2h")
AnnounceCmd->>AnnounceCmd: validate future time
AnnounceCmd->>DB: INSERT scheduled_message
DB-->>AnnounceCmd: id=42
AnnounceCmd-->>User: ✅ Scheduled #42
Note over Scheduler: Polls every 60s
loop Every 60s
Scheduler->>DB: SELECT * WHERE next_run <= NOW()
DB-->>Scheduler: [message #42, ...]
Scheduler->>Channel: safeSend("Hello")
Channel-->>Scheduler: sent
Scheduler->>DB: UPDATE enabled=false (one-time)
Note over Scheduler: For recurring: compute next_run via cron
end
User->>Discord: /announce cancel 42
Discord->>AnnounceCmd: interaction
AnnounceCmd->>DB: SELECT message #42
AnnounceCmd->>AnnounceCmd: check permissions
AnnounceCmd->>DB: UPDATE enabled=false
AnnounceCmd-->>User: ✅ Cancelled #42
Last reviewed commit: a6957d5
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/002_scheduled-messages.cjs`:
- Around line 11-24: Add a CHECK constraint on the scheduled_messages table to
enforce valid schedule modes: ensure rows either represent a one-time message
(one_time = true AND cron_expression IS NULL) or a recurring message (one_time =
false AND cron_expression IS NOT NULL). Update the CREATE TABLE in
migrations/002_scheduled-messages.cjs by adding a CHECK constraint referencing
scheduled_messages, one_time, and cron_expression to guarantee only those two
valid combinations are allowed.
In `@src/commands/announce.js`:
- Around line 146-148: Call getPool() and handle the case where it fails
(missing DATABASE_URL) before proceeding to execute subcommands; instead of
letting getPool() throw during command handling, wrap the pool acquisition in a
try/catch (or check process.env.DATABASE_URL) at the start of the announce
handler, and if the pool is unavailable send a clear user-facing reply via
interaction.reply or interaction.followUp explaining the DB is not configured,
then return early so interaction.options.getSubcommand() and subsequent logic do
not run; refer to getPool(), interaction.options.getSubcommand(), and the
announce command handler when making the change.
- Around line 186-191: The INSERT into scheduled_messages (via the pool.query
call) stores next_run as server-local ISO and omits any guild timezone info;
update the scheduling logic to derive the guild timezone (from guild settings or
a default), compute nextRun in that timezone then convert to UTC for storage,
and add a timezone/tz column value to the INSERT so the DB knows the original
zone; apply the same change to the other scheduling INSERT block (the one around
the later block), ensuring you reference the pool.query call and the
scheduled_messages table, compute using a timezone-aware library (e.g.,
luxon/moment-timezone) and persist both next_run (UTC) and the guild timezone
string in the INSERT.
- Around line 292-295: The assembled announce list content (`content = \`📋
**Scheduled Messages (${rows.length})**\n\n${lines.join('\n\n')}\``) can exceed
Discord's 2000-char limit; use the existing splitMessage() utility to split that
string into chunks and then send each chunk via safeReply/interaction (first
chunk as the initial reply and subsequent chunks as follow-ups or additional
safeReply calls) so every piece is below the limit; update the `announce list`
branch where `safeReply(interaction, { content: ..., ephemeral: true })` is used
to iterate over splitMessage(content) and send each chunk.
- Around line 103-125: The parseTime logic currently accepts out-of-range
components; update parseTime to strictly validate numeric ranges before
returning a Date: for tomorrowMatch ensure parsed hour (tomorrowMatch[1]) is
0–23 and minutes (tomorrowMatch[2] or 0) are 0–59 and return null on invalid;
for isoMatch validate year, month (1–12), hour 0–23 and minute 0–59 and that the
requested day is valid for that month/year (account for leap years) before
constructing/returning d; alternatively, after creating d compare
d.getFullYear(), d.getMonth()+1, d.getDate(), d.getHours(), and d.getMinutes()
against parsed components from isoMatch/tomorrowMatch and return null if any
mismatch.
In `@src/modules/scheduler.js`:
- Around line 157-161: The next_run calculation ignores the schedule's timezone
(scheduled_messages.timezone) causing drift; update the call to getNextCronRun
to include the schedule timezone (e.g., getNextCronRun(msg.cron_expression, new
Date(), msg.timezone)) and modify the getNextCronRun implementation to interpret
the cron expression using that timezone (using a tz-aware cron/parser option)
and return a UTC Date so you can continue to store nextRun.toISOString(); ensure
all references to getNextCronRun and tests are updated to accept the new
timezone parameter.
- Around line 67-71: The cron step parsing accepts out-of-range bases; update
the parsing logic that handles "const [base, step] = field.split('/')" to
validate the resolved startNum is within the allowed bounds (min..max) when base
!== '*'. After computing startNum (and stepNum), add a check that startNum is
not NaN and min <= startNum <= max and throw the same Error (`Invalid cron step
"${field}" for ${names[i]}`) if it is out of range so invalid bases like "99/5"
are rejected.
- Around line 136-169: The SELECT→send→UPDATE flow in the scheduled_messages
processing is race-prone; change it to an atomic claim pattern using a DB
transaction and row locking so each due row is claimed by one worker: begin a
transaction with pool.query, SELECT FROM scheduled_messages WHERE enabled = true
AND next_run <= NOW() FOR UPDATE SKIP LOCKED (or update a claim column like
locked_by/locked_at in the same transaction) to mark the row as claimed, commit,
then fetch the channel and call safeSend; perform the post-send updates
(disabling one_time or computing nextRun via getNextCronRun) inside a
transaction or ensure they operate on the claimed row (using id) so duplicate
sends cannot occur; reference functions/vars: pool.query, scheduled_messages,
client.channels.fetch, safeSend, getNextCronRun.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
migrations/002_scheduled-messages.cjssrc/commands/announce.jssrc/index.jssrc/modules/scheduler.jstests/commands/announce.test.jstests/modules/scheduler.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (4)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESM modules only — useimport/export, neverrequire()
Usenode:protocol for Node.js builtins (e.g.import { readFileSync } from 'node:fs')
Always use semicolons
Use single quotes for strings
Use 2-space indentation
No TypeScript — use plain JavaScript with JSDoc comments for documentation
Files:
src/modules/scheduler.jstests/modules/scheduler.test.jssrc/index.jssrc/commands/announce.jstests/commands/announce.test.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Always use Winston for logging — import{ info, warn, error }from../logger.js
NEVER useconsole.log,console.warn,console.error, or anyconsole.*method in src/ files
Pass structured metadata to Winston logging calls (e.g.info('Message processed', { userId, channelId }))
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig(guildId?)fromsrc/modules/config.jsto read config
UsesetConfigValue(path, value, guildId?)fromsrc/modules/config.jsto update config at runtime
UsesplitMessage()utility for messages exceeding Discord's 2000-character limit
UsesafeSend()wrapper for outgoing Discord messages to sanitize mentions and enforce allowedMentions
Files:
src/modules/scheduler.jssrc/index.jssrc/commands/announce.js
tests/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Test files must achieve at least 80% code coverage on statements, branches, functions, and lines
Files:
tests/modules/scheduler.test.jstests/commands/announce.test.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/commands/*.js: Slash commands must export bothdata(SlashCommandBuilder) andexecute(interaction)function
ExportadminOnly = trueon command files to mark mod-only commands
UseparseDuration()fromsrc/utils/duration.jsfor duration-based commands (timeout, tempban, slowmode)
Files:
src/commands/announce.js
🧠 Learnings (5)
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*.js : Slash commands must export both `data` (SlashCommandBuilder) and `execute(interaction)` function
Applied to files:
src/commands/announce.js
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*.js : Use `parseDuration()` from `src/utils/duration.js` for duration-based commands (timeout, tempban, slowmode)
Applied to files:
src/commands/announce.jstests/commands/announce.test.js
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*mod*.js : Moderation commands must follow the pattern: deferReply → validate inputs → sendDmNotification → execute Discord action → createCase → sendModLogEmbed → checkEscalation
Applied to files:
src/commands/announce.js
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*.js : Export `adminOnly = true` on command files to mark mod-only commands
Applied to files:
src/commands/announce.js
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*mod*.js : Send DM notifications to moderation targets *before* executing kicks/bans (once kicked/banned, users cannot receive DMs from the bot)
Applied to files:
src/commands/announce.js
🧬 Code graph analysis (5)
src/modules/scheduler.js (3)
src/db.js (1)
getPool(142-147)src/utils/safeSend.js (1)
safeSend(116-123)src/logger.js (1)
info(230-232)
tests/modules/scheduler.test.js (3)
src/db.js (1)
getPool(142-147)src/modules/scheduler.js (3)
parseCron(25-86)getNextCronRun(95-122)startScheduler(183-198)src/utils/safeSend.js (1)
safeSend(116-123)
src/index.js (4)
src/modules/triage.js (2)
stopTriage(447-467)client(45-45)src/modules/moderation.js (3)
stopTempbanScheduler(435-441)client(90-90)client(98-136)src/modules/scheduler.js (2)
stopScheduler(203-209)startScheduler(183-198)src/modules/memory.js (1)
client(95-95)
src/commands/announce.js (6)
src/modules/config.js (1)
getConfig(282-313)src/utils/permissions.js (2)
isModerator(143-173)getPermissionError(182-184)src/utils/safeSend.js (1)
safeReply(138-145)src/db.js (1)
getPool(142-147)src/logger.js (2)
info(230-232)warn(237-239)src/modules/scheduler.js (2)
parseCron(25-86)getNextCronRun(95-122)
tests/commands/announce.test.js (3)
src/db.js (1)
getPool(142-147)src/utils/permissions.js (1)
isModerator(143-173)src/commands/announce.js (1)
parseTime(85-128)
🔇 Additional comments (4)
src/index.js (1)
52-52: Scheduler lifecycle wiring looks solid.Startup/shutdown integration matches existing background-service orchestration and keeps teardown symmetric.
Also applies to: 265-270, 452-455
tests/modules/scheduler.test.js (1)
57-143: Good coverage on scheduler behavior and cron parsing.The suite exercises parse/compute paths, invalid cron handling, immediate poll, recurring updates, and stop semantics effectively.
Also applies to: 145-252
tests/commands/announce.test.js (1)
121-329: Comprehensive command-path tests.Nice coverage of permission gate, CRUD-like scheduling flows, and time parsing edge cases.
Also applies to: 331-391
src/commands/announce.js (1)
16-71:⚠️ Potential issue | 🟠 Major
--embedsupport is missing from the command surface and persistence path.PR objectives require optional rich embeds, but the command schema and handlers currently only accept/store plain text
message.⛔ Skipped due to learnings
Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-25T02:39:33.506Z Learning: Applies to src/commands/*mod*.js : Moderation commands must follow the pattern: deferReply → validate inputs → sendDmNotification → execute Discord action → createCase → sendModLogEmbed → checkEscalationLearnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-02-25T02:39:33.506Z Learning: Applies to src/commands/*.js : Slash commands must export both `data` (SlashCommandBuilder) and `execute(interaction)` function
…n, parseTime validation, cron error handling
There was a problem hiding this comment.
Review Summary
🔴 Critical (1)
announcemissing fromconfig.jsonallowedCommands— The command advertises "Moderator only" but is not listed inconfig.json'spermissions.allowedCommands. ThehasPermission()fallback insrc/utils/permissions.js:96-100defaults toisGuildAdmin()(admin-only) for unlisted commands. This means moderators who are not admins are silently denied before the command's ownisModerator()check runs.
🟡 Warnings (3)
-
timezonecolumn stored but never used (migrations/002_scheduled-messages.cjs:19) — The column defaults toAmerica/New_YorkbutgetNextCronRun()uses server-localDatemethods. Remove it or implement timezone support. -
parseTimeISO date overflow (src/commands/announce.js:123-126) —2026-02-31 10:00silently becomes March 3. Missing round-trip validation (d.getMonth() !== month - 1 || d.getDate() !== day). -
AGENTS.md not updated — Per project guidelines ("Keep docs up to date — this is non-negotiable"), the Key Files table needs entries for
src/commands/announce.jsandsrc/modules/scheduler.js, and the Database Tables section needsscheduled_messages.
🔵 Nitpicks (2)
-
Cron step base bounds not validated (
src/modules/scheduler.js:70) —99/5accepted silently instead of throwing. -
Non-standard cron day-of-month / day-of-week AND logic (
src/modules/scheduler.js:107-112) — Should be documented so users don't expect standard cron OR behavior.
AI fix prompt (copy-paste into an AI agent)
Fix the following issues in the feat/scheduled-announcements branch of VolvoxLLC/volvox-bot:
-
config.json— Add"announce": "moderator"topermissions.allowedCommands(after"modlog": "moderator"). Without this, the command defaults to admin-only in thehasPermission()check and moderators are silently denied. -
src/commands/announce.jslines 123-126 — Add round-trip date validation after constructing the ISO Date to reject overflowed dates like Feb 31. Afterif (Number.isNaN(d.getTime())) return null;, add:if (d.getMonth() !== month - 1 || d.getDate() !== day) return null; -
migrations/002_scheduled-messages.cjsline 19 — Removetimezone TEXT NOT NULL DEFAULT 'America/New_York'since no code reads or writes this column. Add it back in a future migration when timezone-aware cron evaluation is implemented. -
AGENTS.md— In the Key Files table, add rows for:src/commands/announce.js— "Slash command for scheduling one-time and recurring messages"src/modules/scheduler.js— "Scheduled message polling (60s interval) with minimal cron parser"
In the Database Tables section, add:scheduled_messages— "Scheduled announcements — one-time and recurring (cron). Polled every 60s by the scheduler"
-
src/modules/scheduler.jsline 70 — Change the step validation to also check base bounds:if (Number.isNaN(stepNum) || stepNum <= 0 || Number.isNaN(startNum) || (base !== '*' && (startNum < min || startNum > max))) { -
src/modules/scheduler.js— Add a JSDoc note ongetNextCronRun()orparseCron()documenting that this uses AND logic for all fields (including day-of-month and day-of-week), which differs from standard cron's OR behavior for those two fields.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Prevents silent date overflow where e.g. 2026-02-31 would silently become 2026-03-03. Uses Date(year, month, 0).getDate() to get the correct upper bound for each month/year combination.
There was a problem hiding this comment.
Remaining Issues
Previous fix commits (deferReply, guild-scoped cancel, list truncation, parseTime validation, cron error handling, allowedCommands) all look good. Four issues remain:
🟡 Warning (2)
timezonecolumn is dead code (migrations/002_scheduled-messages.cjs:19) — Stored with defaultAmerica/New_Yorkbut never read or used by any code.getNextCronRun()uses server-localDatemethods, ignoring this column. Misleading.- AGENTS.md not updated — Per project guidelines: "Keep docs up to date — this is non-negotiable." Key Files table needs entries for
src/commands/announce.jsandsrc/modules/scheduler.js. Database Tables section needsscheduled_messages.
🔵 Nitpick (3)
3. embed_json column unused (migrations/002_scheduled-messages.cjs:16) — Never populated or read. Remove until needed.
4. Cron step base bounds not validated (src/modules/scheduler.js:70) — 99/5 silently accepted.
5. Non-standard cron AND logic undocumented (src/modules/scheduler.js:95) — Should note that day-of-month + day-of-week uses AND, not standard cron OR.
AI fix prompt
Fix the following issues in the feat/scheduled-announcements branch of VolvoxLLC/volvox-bot:
-
migrations/002_scheduled-messages.cjsline 19 — Removetimezone TEXT NOT NULL DEFAULT 'America/New_York'since no code reads or writes this column. Add it back in a future migration when timezone-aware cron evaluation is implemented. -
migrations/002_scheduled-messages.cjsline 16 — Removeembed_json JSONB,since no code reads or writes this column. Add it back in a future migration when embed support is implemented. -
AGENTS.md— In the Key Files table (after thesrc/modules/moderation.jsrow), add:src/commands/announce.js| Slash command for scheduling one-time and recurring messagessrc/modules/scheduler.js| Scheduled message polling (60s interval) with minimal cron parser
In the Database Tables section (aftermod_scheduled_actions), add:scheduled_messages| Scheduled announcements — one-time and recurring (cron). Polled every 60s by the scheduler
-
src/modules/scheduler.jsline 70 — Change the step validation to also check base bounds:if (Number.isNaN(stepNum) || stepNum <= 0 || Number.isNaN(startNum) || (base !== '*' && (startNum < min || startNum > max))) { -
src/modules/scheduler.jsline 95 — Add a JSDoc note ongetNextCronRun()documenting that this uses AND logic for all fields (including day-of-month and day-of-week), which differs from standard cron's OR behavior for those two fields.
There was a problem hiding this comment.
Review Summary
Good progress — the fix commits (deferReply, guild-scoped cancel, list truncation, parseTime validation, cron error handling, timezone column removal, allowedCommands) addressed the major issues from previous reviews. Five items remain:
🟡 Warning (1)
- AGENTS.md not updated — Per project guidelines ("Keep docs up to date — this is non-negotiable"), Key Files table needs entries for
src/commands/announce.jsandsrc/modules/scheduler.js, and Database Tables needsscheduled_messages.
🔵 Nitpick (4)
- Cron step base bounds not validated (
src/modules/scheduler.js:70) —99/5silently accepted instead of throwing. - Non-standard cron AND logic undocumented (
src/modules/scheduler.js:95) — Day-of-month + day-of-week uses AND, not standard cron OR. Should be noted in JSDoc. embed_jsoncolumn unused (migrations/002_scheduled-messages.cjs:16) — Never populated or read by any code. Dead schema.getPool()null check is dead code (src/commands/announce.js:155-157) —getPool()throws, never returns null. Wrap in try/catch or remove the unreachable branch.
AI fix prompt (copy-paste into an AI agent)
Fix the following issues in the feat/scheduled-announcements branch of VolvoxLLC/volvox-bot:
-
AGENTS.md— In the Key Files table (after thesrc/modules/moderation.jsrow), add:src/commands/announce.js| Slash command for scheduling one-time and recurring messagessrc/modules/scheduler.js| Scheduled message polling (60s interval) with minimal cron parser
In the Database Tables section (aftermod_scheduled_actions), add:scheduled_messages| Scheduled announcements — one-time and recurring (cron). Polled every 60s by the scheduler
-
src/modules/scheduler.jsline 70 — Change the step validation to also check base bounds:if (Number.isNaN(stepNum) || stepNum <= 0 || Number.isNaN(startNum) || (base !== '*' && (startNum < min || startNum > max))) { -
src/modules/scheduler.jsline 95 — Replace the existing JSDoc with one that documents the AND logic for day-of-month/day-of-week fields (differs from standard cron OR behavior). -
migrations/002_scheduled-messages.cjsline 16 — Removeembed_json JSONB,since no code reads or writes this column. Add it back in a future migration when embed support is implemented. -
src/commands/announce.jslines 154-158 — Replace thegetPool()+ null check with a try/catch that catches the throw fromgetPool()and replies with the DB unavailable message.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/002_scheduled-messages.cjs`:
- Around line 26-28: Add an index on scheduled_messages.guild_id so list queries
by guild avoid sequential scans: update the migration (alongside the existing
pgm.sql call that creates idx_scheduled_next_run) to execute a CREATE INDEX IF
NOT EXISTS for guild_id on the scheduled_messages table (optionally create it as
a partial index WHERE enabled = true if the /announce list only shows enabled
messages). Use the same migration file and pgm.sql call pattern and give the new
index a clear name like idx_scheduled_guild_id (or
idx_scheduled_guild_id_enabled for the partial index) so it’s easy to find and
idempotent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
config.jsonmigrations/002_scheduled-messages.cjs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: claude-review
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*mod*.js : Send DM notifications to moderation targets *before* executing kicks/bans (once kicked/banned, users cannot receive DMs from the bot)
Applied to files:
config.json
📚 Learning: 2026-02-25T02:39:33.506Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T02:39:33.506Z
Learning: Applies to src/commands/*.js : Export `adminOnly = true` on command files to mark mod-only commands
Applied to files:
config.json
🔇 Additional comments (2)
config.json (1)
122-123: LGTM!The addition of
"announce": "moderator"correctly grants moderator-level access to the new/announcecommand, following the established permission pattern in this config.migrations/002_scheduled-messages.cjs (1)
11-24: Missing CHECK constraint for schedule mode consistency.The
one_timeandcron_expressioncolumns can be stored in invalid combinations (e.g.,one_time = truewith a cron expression, orone_time = falsewithout one). This was already flagged in a previous review.
There was a problem hiding this comment.
Review Summary
Previous fix commits addressed the critical issues from earlier reviews (deferReply, guild-scoped cancel, list truncation, parseTime validation, cron error handling, timezone column, allowedCommands). Five items remain:
🟡 Warning (1)
- Send failure causes infinite 60s retry loop (
src/modules/scheduler.js:140-179) — WhensafeSendthrows or channel is null (deleted), the catch block only logs. One-time messages stay enabled, recurring messages don't advancenext_run. Both retry every 60s indefinitely.
🔵 Nitpick (4)
2. getPool() null check is dead code (src/commands/announce.js:154-157) — getPool() throws, never returns null. Wrap in try/catch to show the DB-specific error.
3. embed_json column unused (migrations/002_scheduled-messages.cjs:16) — Never populated or read.
4. Cron step base bounds not validated (src/modules/scheduler.js:70) — 99/5 silently accepted.
5. Cron AND logic undocumented (src/modules/scheduler.js:88-94) — Day-of-month + day-of-week uses AND, not standard cron OR.
AI fix prompt (copy-paste into an AI agent)
Fix the following issues in the feat/scheduled-announcements branch of VolvoxLLC/volvox-bot:
-
src/modules/scheduler.jslines 140-179 — InpollScheduledMessages, whensafeSendthrows (outer catch block, lines 174-179), advance state to prevent infinite retries: for one-time messages, disable them (UPDATE scheduled_messages SET enabled = false WHERE id = $1); for recurring messages, compute and set the nextnext_runviagetNextCronRun. Also, whenchannelis null (line 143-148), disable the message instead of justcontinue-ing — a deleted channel is a permanent failure. -
src/commands/announce.jslines 154-158 — Replace thegetPool()+if (!pool)null check with a try/catch.getPool()throws when pool is null (seesrc/db.js:142-147), so the null check is unreachable dead code. Use:let pool; try { pool = getPool(); } catch { await safeEditReply(interaction, { content: '❌ Database is not available.' }); return; } -
migrations/002_scheduled-messages.cjsline 16 — Removeembed_json JSONB,since no code reads or writes this column. -
src/modules/scheduler.jsline 70 — Add base bounds validation: change toif (Number.isNaN(stepNum) || stepNum <= 0 || Number.isNaN(startNum) || (base !== '*' && (startNum < min || startNum > max))) { -
src/modules/scheduler.jslines 88-94 — Add a JSDoc note ongetNextCronRun()documenting that this uses AND logic for all fields (including day-of-month and day-of-week), which differs from standard cron's OR behavior.
1 participant
Summary
Add the
/announceslash command for scheduling one-time and recurring messages in Discord channels.New Files
migrations/002_scheduled-messages.cjs— schema migration forscheduled_messagestablesrc/modules/scheduler.js— scheduler polling + minimal cron parser (zero new dependencies)src/commands/announce.js— slash command with 4 subcommandstests/commands/announce.test.js— 17 tests for the commandtests/modules/scheduler.test.js— 18 tests for the schedulerFeatures
/announce once <time> <channel> <message>— schedule a one-time messagein 2h,in 30m,tomorrow 09:00,2026-03-15 14:00/announce recurring <cron> <channel> <message>— recurring via 5-field cron/announce list— list active scheduled messages for the guild/announce cancel <id>— cancel by ID (author or moderator)isModerator()index.jsstartup/shutdown lifecycleTesting
Closes #42