feat(moderation): comprehensive warning system with severity, decay, and expiry#251
feat(moderation): comprehensive warning system with severity, decay, and expiry#251BillChirico merged 6 commits intomainfrom
Conversation
…and expiry - Add warnings table migration (011) with severity, points, expiry - Create warningEngine module: create/edit/remove/clear/expire warnings - Update /warn command: moderatorOnly, add severity option, link to warnings table - Add /warnings command: view user warnings with active/expired status - Add /editwarn command: edit warning reason/severity - Add /removewarn command: deactivate specific warning - Add /clearwarnings command: bulk deactivate all active warnings - Update escalation to count only active warnings from warnings table - Add warning expiry scheduler (60s poll for expired warnings) - Add API routes: GET /warnings, /warnings/user/:userId, /warnings/stats - Add config defaults: warnings.expiryDays=90, severityPoints - Update permission defaults: all warning commands as moderator level - Add comprehensive tests for warningEngine, all commands Closes #250
BillChirico
added
priority: high
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-251
|
🚅 Deployed to the volvox-bot-pr-251 environment in volvox-bot
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| router.use('/moderation', requireAuth(), auditLogMiddleware(), moderationRouter); | ||
|
|
||
| // Warning routes — require API secret or OAuth2 JWT | ||
| router.use('/warnings', requireAuth(), auditLogMiddleware(), warningsRouter); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
In general, the fix is to introduce a rate-limiting middleware (e.g., using express-rate-limit) and apply it to the affected routes so that even authenticated clients cannot make unbounded numbers of expensive requests. This middleware should be added without altering existing authentication or audit behavior.
The best single approach here is to (1) import express-rate-limit, (2) define a small set of limiter instances (for example, a “public” limiter and an “authenticated” limiter) in this file, and (3) apply the appropriate limiter to the router mounts. Specifically, for the issue CodeQL flagged on the /warnings route, we add the authenticated limiter into the middleware chain directly before requireAuth() or immediately after it. To keep behavior consistent and improve security more broadly, we can also rate-limit other authenticated routes in the same router, and optionally add a lighter limiter to public routes such as /auth and /community. All changes should be made inside src/api/index.js: add the import at the top, define the limiters near the router creation, and then update the router.use(...) lines to include those limiters in the middleware lists, keeping the rest of the logic and ordering intact.
Concretely:
- At the top of
src/api/index.js, addimport rateLimit from 'express-rate-limit';. - After
const router = Router();, define e.g.:const publicLimiter = rateLimit({ windowMs: 60_000, max: 60 });const authLimiter = rateLimit({ windowMs: 60_000, max: 120 });
- Update
router.use('/warnings', requireAuth(), auditLogMiddleware(), warningsRouter);to includeauthLimiter(e.g.,router.use('/warnings', authLimiter, requireAuth(), auditLogMiddleware(), warningsRouter);). - Optionally but consistently, apply
authLimiterto all the authenticated routes in this file andpublicLimiterto public routes like/authand/community.
| @@ -47,7 +47,8 @@ | ||
| "winston": "^3.19.0", | ||
| "winston-daily-rotate-file": "^5.0.0", | ||
| "winston-transport": "^4.9.0", | ||
| "ws": "^8.19.0" | ||
| "ws": "^8.19.0", | ||
| "express-rate-limit": "^8.3.0" | ||
| }, | ||
| "pnpm": { | ||
| "overrides": { |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.3.0 | None |
|
|
||
| // Apply rate limiter and guild-scoped authorization | ||
| router.use(warningsRateLimit); | ||
| router.use(adaptGuildIdParam, requireGuildModerator); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 9 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| router.get('/', async (req, res) => { | ||
| const { guildId, userId, active, severity } = req.query; | ||
|
|
||
| if (!guildId) { | ||
| return res.status(400).json({ error: 'guildId is required' }); | ||
| } | ||
|
|
||
| const page = Math.max(1, parseInt(req.query.page, 10) || 1); | ||
| const limit = Math.min(100, Math.max(1, parseInt(req.query.limit, 10) || 25)); | ||
| const offset = (page - 1) * limit; | ||
|
|
||
| try { | ||
| const pool = getPool(); | ||
|
|
||
| const conditions = ['guild_id = $1']; | ||
| const values = [guildId]; | ||
| let paramIdx = 2; | ||
|
|
||
| if (userId) { | ||
| conditions.push(`user_id = $${paramIdx++}`); | ||
| values.push(userId); | ||
| } | ||
|
|
||
| if (active !== undefined) { | ||
| conditions.push(`active = $${paramIdx++}`); | ||
| values.push(active === 'true'); | ||
| } | ||
|
|
||
| if (severity) { | ||
| conditions.push(`severity = $${paramIdx++}`); | ||
| values.push(severity); | ||
| } | ||
|
|
||
| const where = conditions.join(' AND '); | ||
|
|
||
| const [warningsResult, countResult] = await Promise.all([ | ||
| pool.query( | ||
| `SELECT * FROM warnings | ||
| WHERE ${where} | ||
| ORDER BY created_at DESC | ||
| LIMIT $${paramIdx} OFFSET $${paramIdx + 1}`, | ||
| [...values, limit, offset], | ||
| ), | ||
| pool.query(`SELECT COUNT(*)::integer AS total FROM warnings WHERE ${where}`, values), | ||
| ]); | ||
|
|
||
| const total = countResult.rows[0]?.total ?? 0; | ||
| const pages = Math.ceil(total / limit); | ||
|
|
||
| info('Warnings listed via API', { guildId, page, limit, total }); | ||
|
|
||
| return res.json({ | ||
| warnings: warningsResult.rows, | ||
| total, | ||
| page, | ||
| limit, | ||
| pages, | ||
| }); | ||
| } catch (err) { | ||
| logError('Failed to list warnings', { error: err.message, guildId }); | ||
| return res.status(500).json({ error: 'Failed to fetch warnings' }); | ||
| } | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 9 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| router.get('/user/:userId', async (req, res) => { | ||
| const { userId } = req.params; | ||
| const { guildId } = req.query; | ||
|
|
||
| if (!guildId) { | ||
| return res.status(400).json({ error: 'guildId is required' }); | ||
| } | ||
|
|
||
| try { | ||
| const pool = getPool(); | ||
|
|
||
| const [warningsResult, statsResult, bySeverityResult] = await Promise.all([ | ||
| pool.query( | ||
| `SELECT * FROM warnings | ||
| WHERE guild_id = $1 AND user_id = $2 | ||
| ORDER BY created_at DESC | ||
| LIMIT 50`, | ||
| [guildId, userId], | ||
| ), | ||
| pool.query( | ||
| `SELECT | ||
| COUNT(*)::integer AS active_count, | ||
| COALESCE(SUM(points), 0)::integer AS active_points | ||
| FROM warnings | ||
| WHERE guild_id = $1 AND user_id = $2 AND active = TRUE`, | ||
| [guildId, userId], | ||
| ), | ||
| pool.query( | ||
| `SELECT severity, COUNT(*)::integer AS count | ||
| FROM warnings | ||
| WHERE guild_id = $1 AND user_id = $2 AND active = TRUE | ||
| GROUP BY severity`, | ||
| [guildId, userId], | ||
| ), | ||
| ]); | ||
|
|
||
| const bySeverity = {}; | ||
| for (const row of bySeverityResult.rows) { | ||
| bySeverity[row.severity] = row.count; | ||
| } | ||
|
|
||
| info('User warning summary fetched via API', { guildId, userId }); | ||
|
|
||
| return res.json({ | ||
| userId, | ||
| activeCount: statsResult.rows[0]?.active_count ?? 0, | ||
| activePoints: statsResult.rows[0]?.active_points ?? 0, | ||
| bySeverity, | ||
| warnings: warningsResult.rows, | ||
| }); | ||
| } catch (err) { | ||
| logError('Failed to fetch user warnings', { error: err.message, guildId, userId }); | ||
| return res.status(500).json({ error: 'Failed to fetch user warnings' }); | ||
| } | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the problem, the /warnings/user/:userId route should be protected with the existing warningsRateLimit middleware so that requests to this endpoint are rate-limited before any database access occurs. This aligns with the established pattern in the file (using warningsRateLimit for warning endpoints) and doesn’t change the route’s core functionality other than throttling excessive use.
Concretely, in src/api/routes/warnings.js, modify the route definition starting on line 186 from router.get('/user/:userId', async (req, res) => { to include the warningsRateLimit middleware, e.g. router.get('/user/:userId', warningsRateLimit, async (req, res) => {. No new imports are needed because warningsRateLimit is already defined in this file using the imported rateLimit helper. The rest of the handler logic remains unchanged. This single change ensures that all database access within this route is subject to the configured 120-requests-per-15-minutes limit, addressing all three CodeQL alert variants since they all reference database calls within the same handler.
| @@ -183,7 +183,7 @@ | ||
| * "400": | ||
| * description: Missing guildId | ||
| */ | ||
| router.get('/user/:userId', async (req, res) => { | ||
| router.get('/user/:userId', warningsRateLimit, async (req, res) => { | ||
| const { userId } = req.params; | ||
| const { guildId } = req.query; | ||
|
|
| router.get('/stats', async (req, res) => { | ||
| const { guildId } = req.query; | ||
|
|
||
| if (!guildId) { | ||
| return res.status(400).json({ error: 'guildId is required' }); | ||
| } | ||
|
|
||
| try { | ||
| const pool = getPool(); | ||
|
|
||
| const [totalResult, activeResult, bySeverityResult, topUsersResult] = await Promise.all([ | ||
| pool.query('SELECT COUNT(*)::integer AS total FROM warnings WHERE guild_id = $1', [guildId]), | ||
| pool.query( | ||
| 'SELECT COUNT(*)::integer AS total FROM warnings WHERE guild_id = $1 AND active = TRUE', | ||
| [guildId], | ||
| ), | ||
| pool.query( | ||
| `SELECT severity, COUNT(*)::integer AS count | ||
| FROM warnings | ||
| WHERE guild_id = $1 AND active = TRUE | ||
| GROUP BY severity`, | ||
| [guildId], | ||
| ), | ||
| pool.query( | ||
| `SELECT user_id, COUNT(*)::integer AS count, SUM(points)::integer AS points | ||
| FROM warnings | ||
| WHERE guild_id = $1 AND active = TRUE | ||
| GROUP BY user_id | ||
| ORDER BY points DESC | ||
| LIMIT 10`, | ||
| [guildId], | ||
| ), | ||
| ]); | ||
|
|
||
| const bySeverity = {}; | ||
| for (const row of bySeverityResult.rows) { | ||
| bySeverity[row.severity] = row.count; | ||
| } | ||
|
|
||
| return res.json({ | ||
| totalWarnings: totalResult.rows[0]?.total ?? 0, | ||
| activeWarnings: activeResult.rows[0]?.total ?? 0, | ||
| bySeverity, | ||
| topUsers: topUsersResult.rows, | ||
| }); | ||
| } catch (err) { | ||
| logError('Failed to fetch warning stats', { error: err.message, guildId }); | ||
| return res.status(500).json({ error: 'Failed to fetch warning stats' }); | ||
| } | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the problem, the /warnings/stats route should be protected with the existing warningsRateLimit middleware so that requests to this DB-heavy endpoint are throttled per client, reducing the risk of denial-of-service from excessive requests. Generally, this is done in Express by inserting a rate-limiting middleware function as an argument to router.get before the async handler.
The best minimal fix, without changing existing behavior, is to reuse the already-defined warningsRateLimit constant (line 17) for the /stats route, just like any other warning API route would. Concretely, in src/api/routes/warnings.js at line 265, change router.get('/stats', async (req, res) => { to router.get('/stats', warningsRateLimit, async (req, res) => {. This introduces no new imports, no configuration changes, and does not alter the business logic of the handler; it only adds rate limiting before the handler executes, addressing all four CodeQL alerts tied to the DB queries inside this route.
| @@ -262,7 +262,7 @@ | ||
| * "200": | ||
| * description: Warning stats | ||
| */ | ||
| router.get('/stats', async (req, res) => { | ||
| router.get('/stats', warningsRateLimit, async (req, res) => { | ||
| const { guildId } = req.query; | ||
|
|
||
| if (!guildId) { |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR implements a comprehensive warning system for Discord bot moderation, featuring manual warning commands with configurable severity levels, persistent storage, warning lifecycle management (edit/remove/clear), expiration policies, escalation triggers, API endpoints for querying warning history, and dashboard UI controls for configuration. 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. 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ 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 |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @BillChirico. The following files were modified: * `src/api/routes/warnings.js` * `src/commands/clearwarnings.js` * `src/commands/editwarn.js` * `src/commands/removewarn.js` * `src/commands/warn.js` * `src/commands/warnings.js` * `src/modules/moderation.js` * `src/modules/warningEngine.js` These files were kept as they were: * `src/index.js` These files were ignored: * `tests/commands/clearwarnings.test.js` * `tests/commands/editwarn.test.js` * `tests/commands/removewarn.test.js` * `tests/commands/warn.test.js` * `tests/commands/warnings.test.js` * `tests/modules/warningEngine.test.js` These file types are not supported: * `.gitignore` * `TASK.md` * `biome.json` * `config.json`
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-251
- Add WarningsConfig type (expiryDays, severityPoints, dmNotification, maxPerPage) - Add warning system controls to moderation section in config editor - Wire up updateWarningsField callback in config-editor.tsx - Warning config saves via existing moderation SAFE_CONFIG_KEYS path
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-251
|
| Filename | Overview |
|---|---|
| src/modules/warningEngine.js | Core warning lifecycle module — well-structured with try/catch on all CRUD functions, but editWarning allows editing inactive warnings due to a missing AND active = TRUE guard in the UPDATE query. |
| src/commands/warnings.js | New /warnings display command — correctly paginated and embedded, but hardcodes perPage = 10 instead of reading config.moderation.warnings.maxPerPage, making the dashboard's per-page setting a dead control. |
| src/commands/warn.js | Updated /warn command — severity, createWarning integration, and escalation wiring look correct; minor redundancy in extracting _severity inside extractOptions that is never consumed downstream. |
| src/api/routes/warnings.js | New warnings API — proper auth, rate limiting, parameterized queries, and error handling; severity query param is not validated against the allowed enum before use, leading to silent empty results for invalid values. |
| migrations/011_warnings.cjs | Clean migration — creates warnings table with appropriate CHECK constraint on severity, three targeted partial indexes for expiry polling, guild/user lookups, and active-only filtering; down safely cascades. |
| src/modules/moderation.js | Escalation updated to count active warnings from the new warnings table with a graceful fallback to mod_cases if the table doesn't exist yet (error code 42P01); well-handled backward compatibility. |
| web/src/types/config.ts | Adds WarningsConfig with expiryDays, severityPoints, and maxPerPage fields; maxPerPage is exposed in the dashboard but never read by the backend bot, making it a misleading dead field. |
Sequence Diagram
sequenceDiagram
participant Mod as Moderator
participant Bot as Discord Bot
participant WE as warningEngine.js
participant DB as PostgreSQL
participant Sched as ExpiryScheduler
Mod->>Bot: /warn user reason severity
Bot->>Bot: executeModAction → createCase()
Bot->>WE: createWarning(guildId, data, config)
WE->>DB: INSERT INTO warnings (severity, points, expires_at)
DB-->>WE: warning row
WE-->>Bot: warning row
Bot->>Bot: checkEscalation() — queries active warnings only
Bot-->>Mod: ✅ User warned (Case #N)
Mod->>Bot: /editwarn id reason severity
Bot->>WE: editWarning(guildId, id, updates, config)
WE->>DB: SELECT for audit trail
WE->>DB: UPDATE warnings SET reason/severity/points
DB-->>WE: updated row
WE-->>Bot: updated warning
Bot-->>Mod: ✅ Warning #N updated
Mod->>Bot: /removewarn id reason
Bot->>WE: removeWarning(guildId, id, removedBy, reason)
WE->>DB: UPDATE SET active=FALSE, removed_at=NOW() WHERE active=TRUE
DB-->>WE: deactivated row
Bot-->>Mod: ✅ Warning #N removed
Sched->>WE: processExpiredWarnings() [every 60s]
WE->>DB: UPDATE SET active=FALSE WHERE expires_at <= NOW()
DB-->>WE: rowCount expired
Comments Outside Diff (3)
-
src/commands/warnings.js, line 58 (link)perPageis hardcoded to10on line 58, but the dashboard exposes a configurablewarnings.maxPerPagefield (WarningsConfig.maxPerPageinweb/src/types/config.ts, editable inconfig-editor.tsx). Admins who set this value via the dashboard will see no effect because the command never reads it from config.This also requires adding a
getConfigimport at the top of the file. Without this, the dashboard setting is a dead control. -
src/modules/warningEngine.js, line 222-228 (link)The UPDATE query in
editWarninghas noAND active = TRUEguard, so a moderator calling/editwarnon a previously removed or expired warning ID will silently update it without any indication that the warning is no longer active. The function will return the updated (but still inactive) row, and the command will respond with a success message — confusing the moderator into thinking the edit was meaningful.In contrast, both
removeWarning()(line 267) andclearWarnings()(line 302) correctly checkAND active = TRUEwhen deactivating warnings. -
src/api/routes/warnings.js, line 120-123 (link)The
severityvalue fromreq.queryis passed directly into a parameterized query without being validated against the allowed set['low', 'medium', 'high']. While SQL injection is prevented by parameterization, an invalid value (e.g.severity=critical) will silently return 0 results instead of a400 Bad Request, making API clients harder to debug.
Last reviewed commit: 6705986
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config.json`:
- Around line 163-167: The entries "warnings", "editwarn", "removewarn", and
"clearwarnings" inside the allowedCommands object are mis-indented; update their
indentation to align with the other keys (e.g., "warn") so all entries in
allowedCommands have consistent whitespace/indent level; locate the
allowedCommands object and adjust the lines for the keys "warnings", "editwarn",
"removewarn", and "clearwarnings" to match the same indentation as its
neighboring entries.
In `@src/api/routes/warnings.js`:
- Around line 22-31: The request must validate presence of guildId before
running requireGuildModerator to avoid unnecessary permission lookups and
incorrect 403/502 responses; update adaptGuildIdParam (or add a small validator)
to check req.query.guildId and, if missing, respond with a 400 error
immediately, otherwise set req.params.id=req.query.guildId and call next(); then
ensure the router uses this validator/adaptor before requireGuildModerator
(router.use(adaptGuildIdParam, requireGuildModerator) or split into two
middlewares) so requireGuildModerator never executes with an undefined
req.params.id.
In `@src/commands/warn.js`:
- Around line 51-66: afterCase currently runs after createCase has committed,
which can leave an orphaned mod_cases row if createWarning fails; modify the
flow so the case and warning inserts are one atomic unit: either move the case
creation and warning creation into a single transaction at the DB layer (so both
inserts succeed or both rollback) or, if you cannot change createCase, add
compensating cleanup in afterCase by catching errors from createWarning and
deleting the created mod_cases row (use caseData.id) to avoid orphaned rows and
ensure checkEscalation’s fallback path won’t count stale cases; reference
afterCase, createCase, createWarning and checkEscalation when making the change.
- Around line 21-30: The choice labels in the .addStringOption builder for
setName('severity') currently hard-code point totals (e.g., "Low (1 point)");
remove those totals from the addChoices names so they are neutral (e.g., "Low",
"Medium", "High"), and rely on the configurable severityPoints mapping at
runtime to determine point values (refer to severityPoints and the command
handler that consumes the 'severity' option) — if you need to inform users of
actual points, format and display the resolved points in the command's
follow-up/response rather than embedding them in the choice labels.
In `@src/commands/warnings.js`:
- Around line 63-72: The current rendering in the warnings.map block always
shows inactive warnings as "❌ Inactive"; change the status derivation to inspect
w.active, w.expires_at and w.removed_at/removal_reason so you can display
distinct labels (e.g., '✅ Active' when w.active is true; '⌛ Expired' when not
active and expires_at is set and in the past; '🗑️ Removed' plus removal_reason
when removed_at is set). Update the status variable creation inside the
warnings.map callback (where timestamp, status, reason, caseRef are computed) to
prefer removed_at/removal_reason, then expires_at, then fallback to a generic
inactive label so moderators can tell expired vs manually cleared warnings.
- Around line 13-22: The command currently hard-caps warning results to limit:25
and provides no way to fetch older pages; add a page parameter to the
SlashCommandBuilder (e.g., addIntegerOption named "page" with default 1) and use
it to compute an offset or cursor when calling getWarnings() (e.g., offset =
(page - 1) * limit) so you pass pagination through instead of truncating; update
getWarnings() signature/call sites to accept and use offset or a cursor
parameter (or return a nextCursor) and wire that through the handler (see data,
the command execute handler around getWarnings(), and getWarnings() itself) so
moderators can request arbitrary pages.
In `@src/modules/moderation.js`:
- Around line 309-326: The catch currently treats any error from the pool.query
against the warnings table as a signal to use the legacy mod_cases fallback;
change it so the fallback is only used when the error indicates the warnings
table is missing (e.g., check err.code === '42P01' or err.message includes
'relation "warnings" does not exist'), otherwise log the unexpected error
(include the error object) and rethrow/abort so we don't silently use mod_cases;
keep references to pool.query, warnCount, and the mod_cases fallback query when
implementing this conditional handling.
In `@src/modules/warningEngine.js`:
- Around line 171-211: editWarning currently overwrites reason/severity and only
updates updated_at, losing who edited and previous values; update the DB schema
(or add a new warnings_edits/history table per migrations/011_warnings.cjs) to
store editor identity, timestamp, and previous fields, then change editWarning
to (1) SELECT the current warning row by guildId/warningId, (2) INSERT a new
audit row into the history table with editor_id, edited_at, previous_reason,
previous_severity, previous_points, and any other originals, (3) perform the
UPDATE of warnings (preserving your dynamic SET logic), and (4) wrap the
SELECT/INSERT/UPDATE in a transaction to ensure atomicity; reference
editWarning, the warnings table, and the new warnings_edits (or history) table
when implementing.
- Around line 122-123: The active-only filter currently only adds "active =
TRUE" which leaves rows with expires_at <= NOW() considered active; update the
predicate built when activeOnly is true (the branch that pushes into conditions)
to require active = TRUE AND (expires_at IS NULL OR expires_at > NOW()) so
expired warnings are excluded; make the same replacement wherever the
active-only condition is applied (the other occurrence that pushes into
conditions for escalations) to ensure /warnings and escalation totals ignore
expired warnings.
- Around line 115-132: getWarnings currently only accepts limit so callers can
only fetch the newest slice; add pagination by extending the options parameter
(e.g., accept offset or page) and apply it in the SQL query using an OFFSET
clause. Update the getWarnings signature to read an offset (or page + compute
offset from limit) alongside limit and activeOnly, validate/normalize the value,
then modify the query to append "OFFSET $N" and pass the offset in the values
array (ensuring the parameter index matches values.length + 1). Ensure callers
can request subsequent pages by using the new option and keep existing behavior
when offset is omitted.
In `@TASK.md`:
- Around line 19-98: TASK.md describes an outdated warning design (mentions
007_warnings.cjs, mod_cases, warningDecay.js, decayDays, and admin-only
commands) but the PR implements migrations/011_warnings.cjs, a separate warnings
table, src/modules/warningEngine.js, expiryDays, and moderator-level controls;
update TASK.md to reflect the actual implementation by replacing the outdated
migration/DB/engine/setting names and permission guidance with the real
artifacts (migrations/011_warnings.cjs, warnings table,
src/modules/warningEngine.js, config key expiryDays, and that warn commands are
moderator-level), remove or reconcile conflicting instructions about mod_cases
and admin-only routes, and ensure the config snippet and command permission note
match the current codebase.
- Around line 3-118: The TASK.md violates markdownlint rules MD022/MD031 due to
missing blank lines around headings and the fenced JSON block; fix by inserting
a single blank line before and after each top-level and secondary heading (e.g.,
"## Branch:", "## Closes:", "## Context", "## What Exists", "## Deliverables",
and each subheading like "### 1. Database Migration") and ensure there's a blank
line both above and below the ```json fenced block in the "Config Schema"
section so the code fence is isolated from surrounding text.
In `@tests/commands/editwarn.test.js`:
- Around line 49-73: Add a new test case in tests/commands/editwarn.test.js that
mirrors the reason-only test but supplies only a severity update: use
createInteraction({ reason: null, severity: 'high' }) (or mock
interaction.options.getInteger/ getString appropriately to return null for
reason and the severity value), call execute(interaction), assert editWarning
was called with 'guild1', 1, { severity: 'high' }, expect.any(Object), and
assert interaction.editReply was called with a message containing 'updated';
reference existing helpers createInteraction, execute, editWarning and
interaction.editReply to place the new test next to the other edit-warning
tests.
In `@tests/commands/warn.test.js`:
- Line 125: The test currently only asserts that the mock checkEscalation was
invoked; change that to assert the escalation handoff was called with the
correct guild and member identifiers by replacing
expect(checkEscalation).toHaveBeenCalled() with a call-argument assertion (e.g.
toHaveBeenCalledWith) that pins the expected guild ID and member ID (use the
test's expectedGuildId/expectedMemberId or the variables used to create the
warning) and include any remaining parameters using expect.any(...) as
appropriate so the mock verifies the correct escalation target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 887a0ad8-2e65-47c7-919b-52f157fe73d0
📒 Files selected for processing (21)
.gitignoreTASK.mdbiome.jsonconfig.jsonmigrations/011_warnings.cjssrc/api/index.jssrc/api/routes/warnings.jssrc/commands/clearwarnings.jssrc/commands/editwarn.jssrc/commands/removewarn.jssrc/commands/warn.jssrc/commands/warnings.jssrc/index.jssrc/modules/moderation.jssrc/modules/warningEngine.jstests/commands/clearwarnings.test.jstests/commands/editwarn.test.jstests/commands/removewarn.test.jstests/commands/warn.test.jstests/commands/warnings.test.jstests/modules/warningEngine.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: Greptile Review
- GitHub Check: Docker Build Validation
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,cjs,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only with
import/exportsyntax, never CommonJS except in migration files (.cjs)
Files:
src/commands/warn.jssrc/commands/warnings.jssrc/commands/editwarn.jssrc/modules/moderation.jstests/commands/editwarn.test.jssrc/commands/clearwarnings.jssrc/api/routes/warnings.jstests/commands/removewarn.test.jssrc/index.jssrc/api/index.jstests/modules/warningEngine.test.jssrc/commands/removewarn.jsmigrations/011_warnings.cjstests/commands/clearwarnings.test.jssrc/modules/warningEngine.jstests/commands/warnings.test.jstests/commands/warn.test.js
**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,mjs,jsx,ts,tsx}: Use single quotes for strings in code, double quotes only allowed in JSON files
Always end statements with semicolons
Use 2-space indentation, enforced by Biome
Files:
src/commands/warn.jssrc/commands/warnings.jssrc/commands/editwarn.jssrc/modules/moderation.jstests/commands/editwarn.test.jssrc/commands/clearwarnings.jssrc/api/routes/warnings.jstests/commands/removewarn.test.jssrc/index.jssrc/api/index.jstests/modules/warningEngine.test.jssrc/commands/removewarn.jstests/commands/clearwarnings.test.jssrc/modules/warningEngine.jstests/commands/warnings.test.jstests/commands/warn.test.js
src/**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,mjs,jsx,ts,tsx}: Usesrc/logger.jsWinston logger singleton, never useconsole.*methods
Use safe Discord message methods:safeReply(),safeSend(),safeEditReply()instead of direct Discord.js methods
Use parameterized SQL queries, never string interpolation for database queries
Files:
src/commands/warn.jssrc/commands/warnings.jssrc/commands/editwarn.jssrc/modules/moderation.jssrc/commands/clearwarnings.jssrc/api/routes/warnings.jssrc/index.jssrc/api/index.jssrc/commands/removewarn.jssrc/modules/warningEngine.js
src/commands/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Export slash command builder and execute function from each command file
Files:
src/commands/warn.jssrc/commands/warnings.jssrc/commands/editwarn.jssrc/commands/clearwarnings.jssrc/commands/removewarn.js
src/modules/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Create new modules for features with corresponding config sections in
config.jsonand entries inSAFE_CONFIG_KEYS
Files:
src/modules/moderation.jssrc/modules/warningEngine.js
tests/**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain 80% code coverage threshold minimum, never lower this threshold
Files:
tests/commands/editwarn.test.jstests/commands/removewarn.test.jstests/modules/warningEngine.test.jstests/commands/clearwarnings.test.jstests/commands/warnings.test.jstests/commands/warn.test.js
src/api/routes/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Create API routes with proper authentication middleware, mount in
src/api/server.js, and document in OpenAPI spec
Files:
src/api/routes/warnings.js
migrations/**/*.cjs
📄 CodeRabbit inference engine (AGENTS.md)
Use
.cjsfile extension for database migrations, use sequential migration numbering (001, 002, etc.) with node-pg-migrate
Files:
migrations/011_warnings.cjs
🧠 Learnings (4)
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/commands/**/*.{js,mjs} : Export slash command builder and execute function from each command file
Applied to files:
src/commands/warnings.jssrc/commands/editwarn.jssrc/commands/clearwarnings.jssrc/commands/removewarn.jstests/commands/clearwarnings.test.jstests/commands/warnings.test.jstests/commands/warn.test.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/**/*.{js,mjs,jsx,ts,tsx} : Use safe Discord message methods: `safeReply()`, `safeSend()`, `safeEditReply()` instead of direct Discord.js methods
Applied to files:
src/commands/warnings.jssrc/commands/editwarn.jssrc/commands/clearwarnings.jstests/commands/warn.test.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/api/routes/**/*.{js,mjs} : Create API routes with proper authentication middleware, mount in `src/api/server.js`, and document in OpenAPI spec
Applied to files:
src/api/routes/warnings.jssrc/api/index.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to web/**/*.{ts,tsx} : Next.js 16 web dashboard uses App Router with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Applied to files:
src/api/index.js
🪛 GitHub Check: CodeQL
src/api/routes/warnings.js
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 88-150: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 182-236: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 261-310: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
src/api/index.js
[failure] 64-64: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
🪛 markdownlint-cli2 (0.21.0)
TASK.md
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 39-39: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 50-50: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 65-65: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 77-77: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 88-88: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 100-100: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 113-113: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (16)
.gitignore (1)
45-45: LGTM — this keeps local worktree artifacts out of version control.biome.json (1)
12-13: Looks good — this keeps Biome from traversing local worktree content.src/api/index.js (1)
62-64: LGTM – Route mounting follows established patterns.The warningsRouter is correctly mounted with
requireAuth()andauditLogMiddleware(), consistent with other protected moderation routes.Regarding rate limiting: this has already been flagged by static analysis. Consider adding rate limiting middleware to protect against abuse, similar to how other sensitive routes should be protected.
config.json (1)
119-127: LGTM – Warning configuration defaults are sensible.The 90-day expiry window and severity point structure (low=1, medium=2, high=3) align with the PR objectives for configurable decay/expiry.
src/index.js (2)
56-59: LGTM – Scheduler lifecycle integration is correct.The warning expiry scheduler is properly imported and integrated into both startup (line 488) and graceful shutdown (line 292) flows, consistent with other background schedulers like tempban and scheduled messages.
Also applies to: 292-292
488-488: Code already implements defensive error handling correctly.The
startWarningExpiryScheduler()function insrc/modules/warningEngine.js(lines 307–329) already follows the defensive pattern you were concerned about. The initial expiry check (lines 311–313) and the recurring poll (lines 319–325) both wrapprocessExpiredWarnings()in.catch()to log errors gracefully without crashing the bot. No changes needed.tests/commands/clearwarnings.test.js (1)
1-60: LGTM – Test coverage is solid.The test suite covers the essential scenarios for the
clearwarningscommand:
- Export validation (name, moderatorOnly)
- Successful bulk clear with count verification
- Zero active warnings case
- Error handling path
The mock setup is appropriate and
afterEachcleanup prevents test pollution.tests/commands/removewarn.test.js (1)
1-58: LGTM – Test coverage is comprehensive.The test suite properly validates the
removewarncommand:
- Export metadata verification
- Successful warning removal with correct parameters
- Not-found scenario (null return)
- Database error handling
Structure is consistent with other command test files.
tests/commands/warnings.test.js (1)
1-92: LGTM – Test coverage is well-structured.The test suite covers the
warningsview command effectively:
- Export metadata checks
- Empty state messaging
- Embed rendering with warning data (including mixed active/expired warnings)
- Error handling for database failures
The mock interaction includes
displayAvatarURLwhich is needed for embed generation—good attention to detail.src/commands/editwarn.js (2)
33-33: Clarify:moderatorOnlyexport is metadata only—enforcement relies on config.Based on the relevant code snippets from
src/utils/permissions.jsandsrc/index.js, themoderatorOnlyexport is not read at runtime for permission enforcement. Access control is determined solely byconfig.permissions.allowedCommands['editwarn'].The config.json correctly maps
editwarnto'moderator', so this works as intended. The export serves as documentation and potentially for future tooling (e.g., command help generation).
39-81: LGTM – Command implementation is solid.The
executefunction correctly:
- Defers with ephemeral reply for responsive UX
- Validates that at least one update field is provided
- Passes config to
editWarningfor severity point recalculation- Handles not-found case gracefully
- Logs the edit action with relevant context
- Catches and logs errors without exposing internals to users
migrations/011_warnings.cjs (2)
17-55: LGTM – Well-designed schema with proper indexing.The migration is comprehensive:
- Table includes all necessary fields for warning lifecycle management (severity, points, active, expiry, removal tracking)
CHECKconstraint onseverityensures data integrity at the database level- Partial indices (
WHERE active = TRUE) optimize the most common query patterns (escalation checks, expiry polling)- Foreign key to
mod_caseswithON DELETE SET NULLcorrectly preserves warnings if a case is deleted- Audit trail fields (
removed_at,removed_by,removal_reason) support the PR's immutable audit trail requirement
57-60: LGTM – Down migration is correct.
CASCADEensures associated indices are also dropped with the table.tests/modules/warningEngine.test.js (1)
28-412: Coverage here is strong.This suite exercises severity mapping, expiry calculation, CRUD paths, expiry processing, and scheduler idempotency, which gives the new warning engine good regression protection.
src/commands/clearwarnings.js (1)
27-55: LGTM.The command matches
clearWarnings()'s row-count contract cleanly and handles the zero-result path without extra complexity.src/commands/removewarn.js (1)
29-65: LGTM.The success and not-found flows are straightforward, and the reply uses the deactivated warning row returned by the engine correctly.
- Add updateEscalationThresholds callback for managing threshold rules - Threshold editor shows when escalation is enabled - Each rule: warns count, time window (days), action (timeout/kick/ban) - Timeout rules get duration picker (5m to 28d) - Add/remove threshold rules with + button and remove link - Conditionally hides duration field for non-timeout actions
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-251
Docstrings generation was requested by @BillChirico. The following files were modified: * `src/api/routes/warnings.js` * `src/commands/clearwarnings.js` * `src/commands/removewarn.js` * `src/index.js` * `src/modules/moderation.js` * `src/modules/warningEngine.js` * `web/src/components/dashboard/config-editor.tsx` * `web/src/components/dashboard/config-sections/ModerationSection.tsx` These files were kept as they were: * `src/commands/editwarn.js` * `src/commands/warn.js` * `src/commands/warnings.js` These files were ignored: * `tests/commands/clearwarnings.test.js` * `tests/commands/editwarn.test.js` * `tests/commands/removewarn.test.js` * `tests/commands/warn.test.js` * `tests/commands/warnings.test.js` * `tests/modules/warningEngine.test.js` These file types are not supported: * `.gitignore` * `TASK.md` * `biome.json` * `config.json`
railway-app
bot
temporarily deployed
to
volvox-bot / volvox-bot-pr-251
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (9)
src/commands/warn.js (2)
21-30:⚠️ Potential issue | 🟡 MinorKeep the severity choice labels neutral.
severityPointsis configurable, so labels likeLow (1 point)andHigh (3 points)become wrong as soon as a guild overrides the defaults.Suggested change
- { name: 'Low (1 point)', value: 'low' }, - { name: 'Medium (2 points)', value: 'medium' }, - { name: 'High (3 points)', value: 'high' }, + { name: 'Low', value: 'low' }, + { name: 'Medium', value: 'medium' }, + { name: 'High', value: 'high' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/warn.js` around lines 21 - 30, The severity choice labels in the command option currently hard-code point counts (e.g., "Low (1 point)"), which can be incorrect when guilds override severityPoints; update the .addStringOption chain in src/commands/warn.js (the builder that calls .setName('severity')/.addChoices(...)) to use neutral labels such as "Low", "Medium", "High" (or "Low severity", etc.) while keeping the same choice values ('low','medium','high'), so the UI remains accurate regardless of configurable point weights.
54-69:⚠️ Potential issue | 🟠 MajorPersist the case row and warning row atomically.
afterCaseruns aftercreateCase()has already committed. IfcreateWarning()fails here, you leave amod_caseswarn behind without the linked warning lifecycle row, andcheckEscalation()can still count that legacy case via its fallback path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/warn.js` around lines 54 - 69, afterCase currently creates the warning after createCase has already committed, risking an orphaned mod_cases row if createWarning fails; modify the flow so creation of the mod case and its warning are executed inside a single database transaction (e.g., have createCase accept an optional callback or parameter to create the warning within its transaction or refactor createCase to create both records before committing), use the same transaction context when calling createWarning (or move warning creation into createCase), and ensure any failure rolls back both the case and warning so checkEscalation cannot see a legacy case without a warning.src/modules/moderation.js (1)
311-327:⚠️ Potential issue | 🟠 MajorOnly use the legacy fallback when the
warningstable is actually missing.This still catches every failure from the
warningsquery and silently countsmod_casesinstead. A transient query error can therefore resurrect removed/expired legacy warns and trigger false escalations.Suggested fix
- } catch { - // Fallback: warnings table may not exist yet during migration rollout + } catch (err) { + const isMissingWarningsTable = + err?.code === '42P01' || + /relation ["']warnings["'] does not exist/i.test(err?.message ?? ''); + if (!isMissingWarningsTable) { + logError('Failed to query active warnings for escalation', { + error: err.message, + guildId, + targetId, + threshold, + }); + throw err; + } + + // Fallback: warnings table may not exist yet during migration rollout const { rows } = await pool.query( `SELECT COUNT(*)::integer AS count FROM mod_cases WHERE guild_id = $1 AND target_id = $2 AND action = 'warn'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/moderation.js` around lines 311 - 327, The current try/catch around the pool.query for the warnings table silently falls back to querying mod_cases for any error; change it so only a missing-table error triggers the legacy fallback: in the catch for the warnings query (around the pool.query call that sets warnCount), inspect the caught error (e.g., error.code === '42P01' or error.message includes 'relation "warnings" does not exist') and only then run the mod_cases fallback query and set warnCount, otherwise rethrow or propagate the error so transient/query errors are not swallowed; reference the pool.query call that queries warnings, the catch block, the mod_cases fallback query, warnCount, and threshold.withinDays when making the change.src/modules/warningEngine.js (3)
122-123:⚠️ Potential issue | 🟠 MajorExclude already-expired rows from “active” reads.
Both queries only check
active = TRUE. Until the scheduler flips those rows, warnings withexpires_at <= NOW()still show up as active history and still inflate active-point totals.Also applies to: 146-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/warningEngine.js` around lines 122 - 123, The active-only filter currently pushes "active = TRUE" but doesn't exclude rows already expired; update the condition logic where "conditions" is built (the branch checking "activeOnly") to push a compound predicate that also requires "expires_at > NOW()" (e.g. "active = TRUE AND expires_at > NOW()"); make the same change in the second occurrence that builds conditions (the other block around lines 146-152) so both queries exclude rows with expires_at <= NOW().
115-132:⚠️ Potential issue | 🟠 Major
getWarnings()still cannot serve page 2+.The accessor only accepts
limit, so callers can only fetch the newest slice. That leaves the command/API layer unable to implement real pagination once a user has more than one page of warnings.Pagination shape that preserves current behavior
export async function getWarnings(guildId, userId, options = {}) { const pool = getPool(); - const { activeOnly = false, limit = 50 } = options; + const { activeOnly = false, limit = 50, offset = 0 } = options; + const safeLimit = Math.min(Math.max(Number(limit) || 50, 1), 100); + const safeOffset = Math.max(Number(offset) || 0, 0); const conditions = ['guild_id = $1', 'user_id = $2']; const values = [guildId, userId]; @@ const { rows } = await pool.query( `SELECT * FROM warnings WHERE ${conditions.join(' AND ')} ORDER BY created_at DESC - LIMIT $${values.length + 1}`, - [...values, limit], + LIMIT $${values.length + 1} + OFFSET $${values.length + 2}`, + [...values, safeLimit, safeOffset], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/warningEngine.js` around lines 115 - 132, getWarnings currently only accepts a limit so callers cannot fetch pages beyond the first; add a pagination parameter (e.g., options.offset = 0) to getWarnings and use it in the SQL by appending "OFFSET $N" where $N is values.length+1, and pass the offset in the values array after limit (or pass [ ...values, limit, offset ] if you add offset after limit); update the function signature/defaults to const { activeOnly = false, limit = 50, offset = 0 } = options and ensure the query uses LIMIT $M and OFFSET $N with corresponding parameter placeholders so callers can request page slices.
172-212:⚠️ Potential issue | 🟠 Major
editWarning()still overwrites the only copy of the record.This updates
reason/severityin place and only bumpsupdated_at, so you lose who edited the warning and what the previous values were. That does not satisfy the immutable audit-trail requirement for/editwarn.src/api/routes/warnings.js (1)
24-29:⚠️ Potential issue | 🟡 MinorReject missing
guildIdbeforerequireGuildModeratorruns.
requireGuildModeratorreadsreq.params.idviasrc/api/routes/guilds.js:289-293, butadaptGuildIdParam()currently callsnext()even whenguildIdis absent. That means the request can hit authz with an undefined guild and return the wrong status before the documented 400 check in each handler.Suggested middleware adjustment
-function adaptGuildIdParam(req, _res, next) { - if (req.query.guildId) { - req.params.id = req.query.guildId; - } - next(); +function adaptGuildIdParam(req, res, next) { + if (!req.query.guildId) { + return res.status(400).json({ error: 'guildId is required' }); + } + req.params.id = req.query.guildId; + return next(); }Also applies to: 31-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/warnings.js` around lines 24 - 29, adaptGuildIdParam currently always calls next() even when req.query.guildId is missing, which allows requireGuildModerator to read undefined req.params.id and return incorrect status; update adaptGuildIdParam to validate presence of req.query.guildId and when absent short-circuit with a 400 bad request (or call next with a validation error) so the per-handler 400 check is enforced, and when present set req.params.id = req.query.guildId before calling next(); apply the same change to the duplicate at lines 31-33.src/commands/warnings.js (2)
13-22:⚠️ Potential issue | 🟠 MajorExpose paging on
/warningsinstead of truncating at 25.The command still has no
pageoption, and the handler always callsgetWarnings(..., { activeOnly, limit: 25 }), so moderators can never reach older warning history from the command surface.Also applies to: 51-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/warnings.js` around lines 13 - 22, Add a paginated "page" option to the SlashCommandBuilder and wire it into the handler so getWarnings can fetch older pages instead of always limiting to 25; specifically, update the exported data (the SlashCommandBuilder used in data) to add an IntegerOption named "page" (default 1) and in the command handler compute offset = (page - 1) * pageSize and call getWarnings(userId, { activeOnly, limit: pageSize, offset }) instead of getWarnings(..., { activeOnly, limit: 25 }); apply the same change for the other warnings command variant referenced around the getWarnings(...) calls (lines with getWarnings and options).
63-72:⚠️ Potential issue | 🟠 MajorShow expired and manually removed warnings as different statuses.
Every inactive record is rendered as
❌ Inactive, which hides whether it decayed naturally or was explicitly cleared by staff.Possible rendering fix
- const status = w.active ? '✅ Active' : '❌ Inactive'; + const expired = w.expires_at && new Date(w.expires_at) <= new Date(); + const status = w.active + ? '✅ Active' + : w.removed_at + ? '🗑️ Removed' + : expired + ? '⌛ Expired' + : '❌ Inactive';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/warnings.js` around lines 63 - 72, The current status calculation in the warnings.map block always collapses inactive warnings into '❌ Inactive'; update the status logic in that mapping (where status is computed from w.active) to distinguish inactive states: keep '✅ Active' for w.active === true, then check for an expiration indicator (e.g., w.expired === true or w.expired_at !== null) and render something like '⏳ Expired' for decayed/expired warnings, and check for a manual-cleared indicator (e.g., w.removed_by or w.cleared_by) and render something like '🧑⚖️ Cleared' or '❌ Cleared by staff' for staff-removed warnings; adjust the ternary/branch around status (and update the template string using severityLabel, w.points, <t:...:R>, and reason) so each inactive case shows a distinct label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/editwarn.js`:
- Around line 21-30: The choice labels in the addStringOption for
setName('severity') currently hard-code point counts (e.g., "Low (1 point)")
which can drift from configurable severityPoints; update the .addChoices entries
used in the addStringOption chain (the severity option) to neutral labels (e.g.,
"Low", "Medium", "High") or otherwise omit point counts so the UI doesn't
promise specific point values tied to guild settings.
- Around line 54-59: The edit path must record who made the change: update the
editWarning function signature to accept a modifiedBy (or modified_by)
parameter, update its DB update to set modified_by = modifiedBy (alongside
updated_at = NOW()), and ensure any internal typing/exports reflect the new
param; then update the caller in editwarn.js to pass the moderator's id (e.g.,
interaction.user.id) when calling editWarning(interaction.guild.id, warningId,
updates, config, modifiedBy). Also mirror how removeWarning()/clearWarnings()
record removed_by so the audit trail is consistent.
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 1377-1389: The input is turning a stored null (meaning "never")
into the default 90 via value={draftConfig.moderation?.warnings?.expiryDays ??
90}; change the rendering and change the onChange mapping so null is displayed
as 0 and entering 0 stores null: for the input with id "warn-expiry" use
something like value={draftConfig.moderation?.warnings?.expiryDays === null ? 0
: draftConfig.moderation?.warnings?.expiryDays ?? 90} and adjust the onChange
handler (the one calling updateWarningsField('expiryDays', ...)) to set null
only when the parsed number === 0 (and handle NaN appropriately), so 0 displays
but persists as null while undefined still falls back to 90.
- Around line 1367-1374: The new ToggleSwitch is writing to
moderation.warnings.dmNotification (via
draftConfig.moderation?.warnings?.dmNotification and
updateWarningsField('dmNotification', ...)) while an existing "DM Notifications
-> warn" control uses a different authoritative setting, causing contradictory
defaults; pick one source of truth: either switch this control to read/write the
same key the existing DM Notifications control uses, or remove/rename this
duplicate and clearly separate behaviors, and make the default value consistent
(align the ToggleSwitch default with the existing control's default rather than
always using true) so UI state is never conflicting.
In `@web/src/components/dashboard/config-sections/ModerationSection.tsx`:
- Around line 205-215: The new "DM user on warn" Switch in ModerationSection
currently reads/writes draftConfig.moderation?.warnings?.dmNotification and
defaults to true, which conflicts with the existing "DM Notifications"
fieldset's warn toggle (same behavior but different source). Consolidate to a
single source of truth by wiring this Switch to the same property the "DM
Notifications" fieldset uses (remove the duplicate dmNotification key) or, if
they must remain distinct, rename and document the distinct purpose and align
defaults; update the onCheckedChange handler (currently
onWarningsChange?.('dmNotification', v)) to use the unified property and ensure
the default value matches the existing control so the UI cannot show
contradictory states.
- Around line 223-230: The number input for expiryDays currently uses
draftConfig.moderation?.warnings?.expiryDays ?? 90 which hides an explicit null
by showing 90; change the input's value to preserve null (e.g., use an empty
string instead of 90) so a saved "never" (null) remains distinguishable and
entering 0 maps to null via onWarningsChange; update the value binding in
ModerationSection.tsx for the element with id "warn-expiry" to use
draftConfig.moderation?.warnings?.expiryDays ?? '' while keeping the existing
onChange handler and onWarningsChange logic.
---
Duplicate comments:
In `@src/api/routes/warnings.js`:
- Around line 24-29: adaptGuildIdParam currently always calls next() even when
req.query.guildId is missing, which allows requireGuildModerator to read
undefined req.params.id and return incorrect status; update adaptGuildIdParam to
validate presence of req.query.guildId and when absent short-circuit with a 400
bad request (or call next with a validation error) so the per-handler 400 check
is enforced, and when present set req.params.id = req.query.guildId before
calling next(); apply the same change to the duplicate at lines 31-33.
In `@src/commands/warn.js`:
- Around line 21-30: The severity choice labels in the command option currently
hard-code point counts (e.g., "Low (1 point)"), which can be incorrect when
guilds override severityPoints; update the .addStringOption chain in
src/commands/warn.js (the builder that calls
.setName('severity')/.addChoices(...)) to use neutral labels such as "Low",
"Medium", "High" (or "Low severity", etc.) while keeping the same choice values
('low','medium','high'), so the UI remains accurate regardless of configurable
point weights.
- Around line 54-69: afterCase currently creates the warning after createCase
has already committed, risking an orphaned mod_cases row if createWarning fails;
modify the flow so creation of the mod case and its warning are executed inside
a single database transaction (e.g., have createCase accept an optional callback
or parameter to create the warning within its transaction or refactor createCase
to create both records before committing), use the same transaction context when
calling createWarning (or move warning creation into createCase), and ensure any
failure rolls back both the case and warning so checkEscalation cannot see a
legacy case without a warning.
In `@src/commands/warnings.js`:
- Around line 13-22: Add a paginated "page" option to the SlashCommandBuilder
and wire it into the handler so getWarnings can fetch older pages instead of
always limiting to 25; specifically, update the exported data (the
SlashCommandBuilder used in data) to add an IntegerOption named "page" (default
1) and in the command handler compute offset = (page - 1) * pageSize and call
getWarnings(userId, { activeOnly, limit: pageSize, offset }) instead of
getWarnings(..., { activeOnly, limit: 25 }); apply the same change for the other
warnings command variant referenced around the getWarnings(...) calls (lines
with getWarnings and options).
- Around line 63-72: The current status calculation in the warnings.map block
always collapses inactive warnings into '❌ Inactive'; update the status logic in
that mapping (where status is computed from w.active) to distinguish inactive
states: keep '✅ Active' for w.active === true, then check for an expiration
indicator (e.g., w.expired === true or w.expired_at !== null) and render
something like '⏳ Expired' for decayed/expired warnings, and check for a
manual-cleared indicator (e.g., w.removed_by or w.cleared_by) and render
something like '🧑⚖️ Cleared' or '❌ Cleared by staff' for staff-removed
warnings; adjust the ternary/branch around status (and update the template
string using severityLabel, w.points, <t:...:R>, and reason) so each inactive
case shows a distinct label.
In `@src/modules/moderation.js`:
- Around line 311-327: The current try/catch around the pool.query for the
warnings table silently falls back to querying mod_cases for any error; change
it so only a missing-table error triggers the legacy fallback: in the catch for
the warnings query (around the pool.query call that sets warnCount), inspect the
caught error (e.g., error.code === '42P01' or error.message includes 'relation
"warnings" does not exist') and only then run the mod_cases fallback query and
set warnCount, otherwise rethrow or propagate the error so transient/query
errors are not swallowed; reference the pool.query call that queries warnings,
the catch block, the mod_cases fallback query, warnCount, and
threshold.withinDays when making the change.
In `@src/modules/warningEngine.js`:
- Around line 122-123: The active-only filter currently pushes "active = TRUE"
but doesn't exclude rows already expired; update the condition logic where
"conditions" is built (the branch checking "activeOnly") to push a compound
predicate that also requires "expires_at > NOW()" (e.g. "active = TRUE AND
expires_at > NOW()"); make the same change in the second occurrence that builds
conditions (the other block around lines 146-152) so both queries exclude rows
with expires_at <= NOW().
- Around line 115-132: getWarnings currently only accepts a limit so callers
cannot fetch pages beyond the first; add a pagination parameter (e.g.,
options.offset = 0) to getWarnings and use it in the SQL by appending "OFFSET
$N" where $N is values.length+1, and pass the offset in the values array after
limit (or pass [ ...values, limit, offset ] if you add offset after limit);
update the function signature/defaults to const { activeOnly = false, limit =
50, offset = 0 } = options and ensure the query uses LIMIT $M and OFFSET $N with
corresponding parameter placeholders so callers can request page slices.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51e1d8b6-6725-4c0d-80fd-7e1d3e1c03fd
📒 Files selected for processing (11)
src/api/routes/warnings.jssrc/commands/clearwarnings.jssrc/commands/editwarn.jssrc/commands/removewarn.jssrc/commands/warn.jssrc/commands/warnings.jssrc/modules/moderation.jssrc/modules/warningEngine.jsweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/config-sections/ModerationSection.tsxweb/src/types/config.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,cjs,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only with
import/exportsyntax, never CommonJS except in migration files (.cjs)
Files:
src/commands/clearwarnings.jssrc/commands/warn.jssrc/commands/editwarn.jssrc/api/routes/warnings.jssrc/modules/warningEngine.jssrc/commands/warnings.jssrc/commands/removewarn.jssrc/modules/moderation.js
**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,mjs,jsx,ts,tsx}: Use single quotes for strings in code, double quotes only allowed in JSON files
Always end statements with semicolons
Use 2-space indentation, enforced by Biome
Files:
src/commands/clearwarnings.jssrc/commands/warn.jssrc/commands/editwarn.jssrc/api/routes/warnings.jssrc/modules/warningEngine.jsweb/src/types/config.tsweb/src/components/dashboard/config-sections/ModerationSection.tsxsrc/commands/warnings.jssrc/commands/removewarn.jsweb/src/components/dashboard/config-editor.tsxsrc/modules/moderation.js
src/**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,mjs,jsx,ts,tsx}: Usesrc/logger.jsWinston logger singleton, never useconsole.*methods
Use safe Discord message methods:safeReply(),safeSend(),safeEditReply()instead of direct Discord.js methods
Use parameterized SQL queries, never string interpolation for database queries
Files:
src/commands/clearwarnings.jssrc/commands/warn.jssrc/commands/editwarn.jssrc/api/routes/warnings.jssrc/modules/warningEngine.jssrc/commands/warnings.jssrc/commands/removewarn.jssrc/modules/moderation.js
src/commands/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Export slash command builder and execute function from each command file
Files:
src/commands/clearwarnings.jssrc/commands/warn.jssrc/commands/editwarn.jssrc/commands/warnings.jssrc/commands/removewarn.js
src/api/routes/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Create API routes with proper authentication middleware, mount in
src/api/server.js, and document in OpenAPI spec
Files:
src/api/routes/warnings.js
src/modules/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Create new modules for features with corresponding config sections in
config.jsonand entries inSAFE_CONFIG_KEYS
Files:
src/modules/warningEngine.jssrc/modules/moderation.js
web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Next.js 16 web dashboard uses App Router with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Files:
web/src/types/config.tsweb/src/components/dashboard/config-sections/ModerationSection.tsxweb/src/components/dashboard/config-editor.tsx
🧠 Learnings (3)
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/commands/**/*.{js,mjs} : Export slash command builder and execute function from each command file
Applied to files:
src/commands/clearwarnings.jssrc/commands/editwarn.jssrc/commands/warnings.jssrc/commands/removewarn.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/**/*.{js,mjs,jsx,ts,tsx} : Use safe Discord message methods: `safeReply()`, `safeSend()`, `safeEditReply()` instead of direct Discord.js methods
Applied to files:
src/commands/clearwarnings.jssrc/commands/editwarn.jssrc/commands/warnings.jssrc/commands/removewarn.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/api/routes/**/*.{js,mjs} : Create API routes with proper authentication middleware, mount in `src/api/server.js`, and document in OpenAPI spec
Applied to files:
src/api/routes/warnings.js
🪛 GitHub Check: CodeQL
src/api/routes/warnings.js
[failure] 33-33: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 90-152: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 184-238: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 263-312: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
🔇 Additional comments (2)
src/commands/warn.js (1)
33-33: Config entries are correct—warning commands are properly protected.The warning commands (
warn,editwarn,removewarn,clearwarnings) all have explicitallowedCommandsentries in config.json with"moderator"permission level, so the dispatcher does enforce permissions correctly. The runtime permission check reads fromconfig.permissions.allowedCommands[commandName], and all four commands are present there.The
moderatorOnlyexport is not used by the dispatcher (only in tests), but this does not affect permission enforcement—the config entries provide the actual protection. No action needed.src/modules/warningEngine.js (1)
307-339: Scheduler start/stop guards look solid.The duplicate-start guard, in-flight poll guard, and explicit
stopWarningExpiryScheduler()cleanup are all sensible for the startup/shutdown flow wired insrc/index.js.
web/src/components/dashboard/config-sections/ModerationSection.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
web/src/components/dashboard/config-sections/ModerationSection.tsx (2)
203-217:⚠️ Potential issue | 🟠 MajorConflicting DM notification controls.
This new "DM user on warn" switch writes to
moderation.warnings.dmNotification(defaulting totrue), but the existing "DM Notifications" fieldset at lines 106-119 already has awarntoggle that writes tomoderation.dmNotifications.warn(defaulting tofalse). The UI shows two separate controls for conceptually the same behavior with opposite defaults, creating contradictory state.Consolidate to a single source of truth or clearly differentiate the behaviors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboard/config-sections/ModerationSection.tsx` around lines 203 - 217, The two DM notification switches conflict: one uses draftConfig.moderation?.warnings?.dmNotification with onWarningsChange and defaults true, while the other uses draftConfig.moderation?.dmNotifications?.warn with a different default; pick a single source of truth (either warnings.dmNotification or dmNotifications.warn) and update the UI and handlers to use that property consistently — e.g., remove the duplicate "DM user on warn" Switch or repurpose it with a distinct name, update the checked value and onChange to the chosen property (adjust onWarningsChange or the dmNotifications handler accordingly), and ensure the default value is consistent across the code paths so the two controls no longer contradict each other.
223-234:⚠️ Potential issue | 🟠 Major
nullexpiry (never expire) is indistinguishable from the default.The input uses
value={... ?? 90}which renders a savednull(meaning "never expire") as90. Users cannot distinguish a configured "never expire" from an unconfigured default, and the UI cannot represent this state correctly.Suggested fix
<Input id="warn-expiry" type="number" min={0} placeholder="90 (0 = never)" - value={draftConfig.moderation?.warnings?.expiryDays ?? 90} + value={ + draftConfig.moderation?.warnings?.expiryDays === null + ? 0 + : (draftConfig.moderation?.warnings?.expiryDays ?? 90) + } onChange={(e) => { const val = parseInt(e.target.value, 10); onWarningsChange?.('expiryDays', Number.isNaN(val) || val <= 0 ? null : val); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboard/config-sections/ModerationSection.tsx` around lines 223 - 234, The input currently displays null (meaning "never expire") as the numeric default because value uses `... ?? 90`; change the value logic to explicitly render an empty string for a saved null so users can see "never": use something like `draftConfig.moderation?.warnings?.expiryDays === null ? '' : (draftConfig.moderation?.warnings?.expiryDays ?? 90)`. Update the onChange handler in ModerationSection (the Input with id "warn-expiry") to treat an empty string as null (call onWarningsChange?.('expiryDays', null)) and otherwise parse the number as before (keeping the existing <=0 -> null behavior).web/src/components/dashboard/config-editor.tsx (2)
1539-1546:⚠️ Potential issue | 🟠 MajorConflicting DM notification controls.
Same issue as in
ModerationSection.tsx: this "DM user on warn" toggle writes tomoderation.warnings.dmNotification(defaulttrue), while the "DM Notifications" fieldset above (lines 1179-1192) has awarntoggle writing tomoderation.dmNotifications.warn(defaultfalse). The two controls present contradictory defaults and may confuse both users and the save logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboard/config-editor.tsx` around lines 1539 - 1546, There are two conflicting controls for DM notifications: the ToggleSwitch in config-editor.tsx currently reads/writes draftConfig.moderation?.warnings?.dmNotification via updateWarningsField (default true) while an earlier fieldset uses moderation.dmNotifications.warn (default false); consolidate them to a single canonical field and default (pick either moderation.warnings.dmNotification or moderation.dmNotifications.warn), update the ToggleSwitch and the other control to read/write the same property, adjust their default fallback to be consistent, and ensure the save/serialization logic and any functions like updateWarningsField or the handler that updates moderation.dmNotifications are updated to reference the chosen property so the UI and persisted config are aligned.
1549-1563:⚠️ Potential issue | 🟠 Major
nullexpiry (never expire) displays as90, hiding the configured "never" state.Line 1557 uses
value={... ?? 90}, so a savednull(meaning "never expire") renders as90. Users cannot see or edit this state correctly.Suggested fix
<input id="warn-expiry" type="number" min={0} className="..." placeholder="90 (0 = never)" - value={draftConfig.moderation?.warnings?.expiryDays ?? 90} + value={ + draftConfig.moderation?.warnings?.expiryDays === null + ? 0 + : (draftConfig.moderation?.warnings?.expiryDays ?? 90) + } onChange={(e) => { const val = parseInt(e.target.value, 10); updateWarningsField('expiryDays', Number.isNaN(val) || val <= 0 ? null : val); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboard/config-editor.tsx` around lines 1549 - 1563, The input for warning expiry currently uses value={draftConfig.moderation?.warnings?.expiryDays ?? 90} so a saved null (meaning "never") displays as 90; change the controlled value to reflect null by mapping null to an empty string (e.g., value={draftConfig.moderation?.warnings?.expiryDays ?? ''}) and update the onChange handler in the same component (the warn-expiry <input> that calls updateWarningsField) to convert an empty string back to null while parsing numbers (e.g., treat '' as null, otherwise parseInt and if NaN or <=0 use null), keeping the placeholder "90 (0 = never)" and disabled={saving} logic intact.src/modules/warningEngine.js (3)
143-159:⚠️ Potential issue | 🟠 MajorActive stats include logically-expired warnings.
getActiveWarningStatsonly checksactive = TRUEbut ignoresexpires_at. Until the scheduler runs, warnings past their expiry inflate the count and points, affecting escalation decisions and UI display.🛠️ Suggested fix
const { rows } = await pool.query( `SELECT COUNT(*)::integer AS count, COALESCE(SUM(points), 0)::integer AS points FROM warnings - WHERE guild_id = $1 AND user_id = $2 AND active = TRUE`, + WHERE guild_id = $1 AND user_id = $2 AND active = TRUE + AND (expires_at IS NULL OR expires_at > NOW())`, [guildId, userId], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/warningEngine.js` around lines 143 - 159, getActiveWarningStats currently counts any row with active = TRUE even if expires_at has passed; update the SQL in getActiveWarningStats to also filter out logically-expired warnings by adding a condition like (expires_at IS NULL OR expires_at > now()) to the WHERE clause so only currently valid warnings contribute to COUNT and SUM(points), leaving the return shape unchanged.
115-135:⚠️ Potential issue | 🟠 MajorPagination and expired-warning filtering are missing.
No offset parameter: Callers can only fetch the first page. The
/warningscommand and API need to request subsequent pages.Expired warnings included: When
activeOnly=true, rows withexpires_at <= NOW()are still returned until the scheduler flipsactive. This inflates counts and can cause incorrect escalations.🛠️ Suggested fix
export async function getWarnings(guildId, userId, options = {}) { const pool = getPool(); - const { activeOnly = false, limit = 50 } = options; + const { activeOnly = false, limit = 50, offset = 0 } = options; + const safeLimit = Math.min(Math.max(Number(limit) || 50, 1), 100); + const safeOffset = Math.max(Number(offset) || 0, 0); const conditions = ['guild_id = $1', 'user_id = $2']; const values = [guildId, userId]; if (activeOnly) { conditions.push('active = TRUE'); + conditions.push('(expires_at IS NULL OR expires_at > NOW())'); } const { rows } = await pool.query( `SELECT * FROM warnings WHERE ${conditions.join(' AND ')} ORDER BY created_at DESC - LIMIT $${values.length + 1}`, - [...values, limit], + LIMIT $${values.length + 1} + OFFSET $${values.length + 2}`, + [...values, safeLimit, safeOffset], ); return rows; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/warningEngine.js` around lines 115 - 135, getWarnings currently only supports limit and returns warnings whose expires_at may already be past; add an offset parameter (e.g., options.offset = 0) and include it in the SQL using LIMIT $N OFFSET $M so callers can page results, and when activeOnly is true add an additional condition to filter out expired warnings (e.g., "expires_at > NOW()") in the WHERE clause before building the parameter list; update the call site(s) that use getWarnings if needed and ensure the parameter ordering passed to pool.query matches the $ placeholders (function: getWarnings, symbols: activeOnly, expires_at, created_at, pool.query).
172-213:⚠️ Potential issue | 🟠 MajorEdit audit trail is not preserved.
editWarningoverwritesreason/severityand only updatesupdated_at. The PR objectives require "edit reason with immutable audit trail" and "tracked edits with actor/timestamp/original-value." Consider storing edit history (editor ID, timestamp, previous values) in a separate table or JSONB column.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/warningEngine.js` around lines 172 - 213, editWarning currently overwrites reason/severity and only updates updated_at so edit audit history is lost; change editWarning to capture the previous row before updating and persist an immutable audit record (either insert into a new warning_edits table with columns like warning_id, editor_id, edited_at, previous_values JSONB, field_names, or append a new object to a JSONB edits array column on the warnings row) and make the update/insert inside a single transaction; add an editorId parameter to editWarning (and pass it through callers), record previous reason/severity/points and actor/timestamp/original-value into the audit destination, then perform the existing UPDATE (still recalculating points via getSeverityPoints when severity changes) and return the updated row.src/modules/moderation.js (1)
313-334:⚠️ Potential issue | 🟠 MajorBare catch swallows all query failures, not just missing-table errors.
The
catchblock at line 325 silently falls back tomod_casesfor any error from thewarningsquery—including transient network failures, connection exhaustion, or syntax errors. This can cause escalation to use stalemod_casesdata (which includes removed/expired warnings), leading to incorrect auto-timeouts or bans.Reserve the fallback for the specific PostgreSQL error indicating the table doesn't exist (
42P01/relation "warnings" does not exist); otherwise log and rethrow.🛠️ Suggested fix
- } catch { - // Fallback: warnings table may not exist yet during migration rollout + } catch (err) { + // Fallback only if warnings table doesn't exist (migration not yet applied) + const isTableMissing = + err.code === '42P01' || /relation "warnings" does not exist/i.test(err.message); + if (!isTableMissing) { + logError('Escalation query failed', { error: err.message, guildId, targetId }); + return null; + } const { rows } = await pool.query(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/moderation.js` around lines 313 - 334, The current catch around the warnings table query in src/modules/moderation.js (the block that sets warnCount via pool.query for warnings with threshold.withinDays) swallows all errors and blindly falls back to mod_cases; change it to only fall back when the Postgres "relation does not exist" error is returned (error.code === '42P01' or error.message includes 'relation \"warnings\" does not exist'), and for other errors log the error (using your existing logger or console.error) and rethrow so transient/query errors are not masked; keep the same fallback query to mod_cases only in the specific missing-table branch.src/api/routes/warnings.js (1)
26-35:⚠️ Potential issue | 🟡 MinorValidate
guildIdbefore authorization middleware runs.When
req.query.guildIdis missing,adaptGuildIdParamsetsreq.params.idtoundefinedand callsnext(). ThenrequireGuildModeratorattempts authorization with an undefined guild ID, potentially causing a 403 or 502 instead of the documented 400.Note: The CodeQL "missing rate limiting" warning on line 35 is a false positive—
warningsRateLimitis applied viarouter.use()on line 34 before the authorization middleware.🛠️ Suggested fix
-function adaptGuildIdParam(req, _res, next) { - if (req.query.guildId) { - req.params.id = req.query.guildId; - } - next(); +function adaptGuildIdParam(req, res, next) { + if (!req.query.guildId) { + return res.status(400).json({ error: 'guildId is required' }); + } + req.params.id = req.query.guildId; + return next(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/warnings.js` around lines 26 - 35, adaptGuildIdParam currently sets req.params.id to undefined when req.query.guildId is missing, which lets requireGuildModerator run and produce a 403/502; update adaptGuildIdParam to validate req.query.guildId and short-circuit with a 400 Bad Request if it's absent (i.e., call next(new Error/HTTPError with status 400 and a clear "missing guildId" message) or otherwise only set req.params.id when the query value exists) so authorization middleware (requireGuildModerator) never runs with an undefined guild ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/removewarn.js`:
- Line 23: The exported metadata "moderatorOnly" in removewarn.js is only
decorative and the dispatcher enforces access via
guildConfig.permissions.allowedCommands; remove reliance on moderatorOnly and
instead ensure the command name used by removewarn.js is added to the guild's
permission config (guildConfig.permissions.allowedCommands) or update the
dispatcher to check moderatorOnly — specifically, either (A) add the removewarn
command key to guildConfig.permissions.allowedCommands for appropriate guilds so
permissions are enforced, or (B) modify the dispatcher logic that resolves
allowed commands to honor the exported moderatorOnly flag from removewarn.js
(adjust the permission check surrounding commandName resolution).
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 1218-1226: The code clones
draftConfig.moderation?.escalation?.thresholds inside the .map callback which
creates a new thresholds array per iteration and leads to
stale-closure/inefficient behavior in the inner onChange handlers; fix it by
hoisting the clone outside the .map (create one const thresholds =
[...(draftConfig.moderation?.escalation?.thresholds ?? [])] before mapping) or,
better, switch the handlers to a functional update that reads the latest state
(setDraftConfig(prev => { const thresholds =
[...(prev.moderation?.escalation?.thresholds ?? [])]; thresholds[idx] =
{...thresholds[idx], /* changed field */}; return {...prev, moderation:
{...prev.moderation, escalation: {...prev.moderation.escalation, thresholds}}};
})), and update each onChange handler (the ones mutating
warns/withinDays/action/duration) to use that single shared clone or the
functional update so closures always work with the current array.
---
Duplicate comments:
In `@src/api/routes/warnings.js`:
- Around line 26-35: adaptGuildIdParam currently sets req.params.id to undefined
when req.query.guildId is missing, which lets requireGuildModerator run and
produce a 403/502; update adaptGuildIdParam to validate req.query.guildId and
short-circuit with a 400 Bad Request if it's absent (i.e., call next(new
Error/HTTPError with status 400 and a clear "missing guildId" message) or
otherwise only set req.params.id when the query value exists) so authorization
middleware (requireGuildModerator) never runs with an undefined guild ID.
In `@src/modules/moderation.js`:
- Around line 313-334: The current catch around the warnings table query in
src/modules/moderation.js (the block that sets warnCount via pool.query for
warnings with threshold.withinDays) swallows all errors and blindly falls back
to mod_cases; change it to only fall back when the Postgres "relation does not
exist" error is returned (error.code === '42P01' or error.message includes
'relation \"warnings\" does not exist'), and for other errors log the error
(using your existing logger or console.error) and rethrow so transient/query
errors are not masked; keep the same fallback query to mod_cases only in the
specific missing-table branch.
In `@src/modules/warningEngine.js`:
- Around line 143-159: getActiveWarningStats currently counts any row with
active = TRUE even if expires_at has passed; update the SQL in
getActiveWarningStats to also filter out logically-expired warnings by adding a
condition like (expires_at IS NULL OR expires_at > now()) to the WHERE clause so
only currently valid warnings contribute to COUNT and SUM(points), leaving the
return shape unchanged.
- Around line 115-135: getWarnings currently only supports limit and returns
warnings whose expires_at may already be past; add an offset parameter (e.g.,
options.offset = 0) and include it in the SQL using LIMIT $N OFFSET $M so
callers can page results, and when activeOnly is true add an additional
condition to filter out expired warnings (e.g., "expires_at > NOW()") in the
WHERE clause before building the parameter list; update the call site(s) that
use getWarnings if needed and ensure the parameter ordering passed to pool.query
matches the $ placeholders (function: getWarnings, symbols: activeOnly,
expires_at, created_at, pool.query).
- Around line 172-213: editWarning currently overwrites reason/severity and only
updates updated_at so edit audit history is lost; change editWarning to capture
the previous row before updating and persist an immutable audit record (either
insert into a new warning_edits table with columns like warning_id, editor_id,
edited_at, previous_values JSONB, field_names, or append a new object to a JSONB
edits array column on the warnings row) and make the update/insert inside a
single transaction; add an editorId parameter to editWarning (and pass it
through callers), record previous reason/severity/points and
actor/timestamp/original-value into the audit destination, then perform the
existing UPDATE (still recalculating points via getSeverityPoints when severity
changes) and return the updated row.
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 1539-1546: There are two conflicting controls for DM
notifications: the ToggleSwitch in config-editor.tsx currently reads/writes
draftConfig.moderation?.warnings?.dmNotification via updateWarningsField
(default true) while an earlier fieldset uses moderation.dmNotifications.warn
(default false); consolidate them to a single canonical field and default (pick
either moderation.warnings.dmNotification or moderation.dmNotifications.warn),
update the ToggleSwitch and the other control to read/write the same property,
adjust their default fallback to be consistent, and ensure the
save/serialization logic and any functions like updateWarningsField or the
handler that updates moderation.dmNotifications are updated to reference the
chosen property so the UI and persisted config are aligned.
- Around line 1549-1563: The input for warning expiry currently uses
value={draftConfig.moderation?.warnings?.expiryDays ?? 90} so a saved null
(meaning "never") displays as 90; change the controlled value to reflect null by
mapping null to an empty string (e.g.,
value={draftConfig.moderation?.warnings?.expiryDays ?? ''}) and update the
onChange handler in the same component (the warn-expiry <input> that calls
updateWarningsField) to convert an empty string back to null while parsing
numbers (e.g., treat '' as null, otherwise parseInt and if NaN or <=0 use null),
keeping the placeholder "90 (0 = never)" and disabled={saving} logic intact.
In `@web/src/components/dashboard/config-sections/ModerationSection.tsx`:
- Around line 203-217: The two DM notification switches conflict: one uses
draftConfig.moderation?.warnings?.dmNotification with onWarningsChange and
defaults true, while the other uses
draftConfig.moderation?.dmNotifications?.warn with a different default; pick a
single source of truth (either warnings.dmNotification or dmNotifications.warn)
and update the UI and handlers to use that property consistently — e.g., remove
the duplicate "DM user on warn" Switch or repurpose it with a distinct name,
update the checked value and onChange to the chosen property (adjust
onWarningsChange or the dmNotifications handler accordingly), and ensure the
default value is consistent across the code paths so the two controls no longer
contradict each other.
- Around line 223-234: The input currently displays null (meaning "never
expire") as the numeric default because value uses `... ?? 90`; change the value
logic to explicitly render an empty string for a saved null so users can see
"never": use something like `draftConfig.moderation?.warnings?.expiryDays ===
null ? '' : (draftConfig.moderation?.warnings?.expiryDays ?? 90)`. Update the
onChange handler in ModerationSection (the Input with id "warn-expiry") to treat
an empty string as null (call onWarningsChange?.('expiryDays', null)) and
otherwise parse the number as before (keeping the existing <=0 -> null
behavior).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49f7f51a-5099-42bb-b02a-c236487db0c9
📒 Files selected for processing (8)
src/api/routes/warnings.jssrc/commands/clearwarnings.jssrc/commands/removewarn.jssrc/index.jssrc/modules/moderation.jssrc/modules/warningEngine.jsweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/config-sections/ModerationSection.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,cjs,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only with
import/exportsyntax, never CommonJS except in migration files (.cjs)
Files:
src/index.jssrc/api/routes/warnings.jssrc/modules/warningEngine.jssrc/commands/removewarn.jssrc/modules/moderation.jssrc/commands/clearwarnings.js
**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,mjs,jsx,ts,tsx}: Use single quotes for strings in code, double quotes only allowed in JSON files
Always end statements with semicolons
Use 2-space indentation, enforced by Biome
Files:
src/index.jssrc/api/routes/warnings.jssrc/modules/warningEngine.jssrc/commands/removewarn.jssrc/modules/moderation.jsweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/config-sections/ModerationSection.tsxsrc/commands/clearwarnings.js
src/**/*.{js,mjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,mjs,jsx,ts,tsx}: Usesrc/logger.jsWinston logger singleton, never useconsole.*methods
Use safe Discord message methods:safeReply(),safeSend(),safeEditReply()instead of direct Discord.js methods
Use parameterized SQL queries, never string interpolation for database queries
Files:
src/index.jssrc/api/routes/warnings.jssrc/modules/warningEngine.jssrc/commands/removewarn.jssrc/modules/moderation.jssrc/commands/clearwarnings.js
src/api/routes/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Create API routes with proper authentication middleware, mount in
src/api/server.js, and document in OpenAPI spec
Files:
src/api/routes/warnings.js
src/modules/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Create new modules for features with corresponding config sections in
config.jsonand entries inSAFE_CONFIG_KEYS
Files:
src/modules/warningEngine.jssrc/modules/moderation.js
src/commands/**/*.{js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Export slash command builder and execute function from each command file
Files:
src/commands/removewarn.jssrc/commands/clearwarnings.js
web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Next.js 16 web dashboard uses App Router with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design
Files:
web/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/config-sections/ModerationSection.tsx
🧠 Learnings (4)
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/api/routes/**/*.{js,mjs} : Create API routes with proper authentication middleware, mount in `src/api/server.js`, and document in OpenAPI spec
Applied to files:
src/api/routes/warnings.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/commands/**/*.{js,mjs} : Export slash command builder and execute function from each command file
Applied to files:
src/commands/removewarn.jssrc/commands/clearwarnings.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/**/*.{js,mjs,jsx,ts,tsx} : Use safe Discord message methods: `safeReply()`, `safeSend()`, `safeEditReply()` instead of direct Discord.js methods
Applied to files:
src/commands/removewarn.jssrc/commands/clearwarnings.js
📚 Learning: 2026-03-05T18:07:15.752Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T18:07:15.752Z
Learning: Applies to src/api/utils/configAllowlist.js : Maintain `SAFE_CONFIG_KEYS` for writable config sections via API and `READABLE_CONFIG_KEYS` for read-only sections, add new config sections to SAFE to enable saves
Applied to files:
web/src/components/dashboard/config-editor.tsx
🪛 GitHub Check: CodeQL
src/api/routes/warnings.js
[failure] 35-35: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 92-154: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 186-240: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 265-314: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
🔇 Additional comments (28)
web/src/components/dashboard/config-sections/ModerationSection.tsx (2)
254-284: LGTM!The severity points grid correctly reads from
severityPointswith appropriate defaults, merges changes immutably, and clamps values to a minimum of 1. The implementation is clean and handles the three-level map consistently.
236-252: LGTM!The warnings-per-page input correctly clamps values between 1 and 25, with a sensible default of 10. The implementation handles edge cases appropriately.
web/src/components/dashboard/config-editor.tsx (4)
727-747: LGTM!The
updateWarningsFieldcallback correctly initializes default values when warnings config doesn't exist and uses immutable update patterns. The defaults align with the documented config schema.
611-627: LGTM!The
updateEscalationThresholdscallback correctly updates the escalation thresholds array immutably, preserving other escalation fields.
1582-1611: LGTM!The severity points grid correctly handles reading, defaults, and immutable updates with a minimum value of 1. Implementation is consistent with the
ModerationSection.tsxversion.
1334-1352: LGTM!The "Add threshold" button correctly clones the existing thresholds array before pushing a new default threshold with sensible initial values. The implementation handles the empty-state case properly.
src/modules/moderation.js (3)
217-228: LGTM!The expanded JSDoc clearly documents the function's behavior, including the new log_message_id storage and return semantics.
285-296: LGTM!The JSDoc accurately describes the escalation evaluation, threshold matching, and return contract.
338-338: LGTM!The escalation reason message now accurately reflects "active warns" terminology, aligning with the new warnings-based counting.
src/modules/warningEngine.js (7)
1-27: LGTM!Module header, imports, default severity points mapping, and scheduler state variables are well-structured.
28-53: LGTM!
getSeverityPointsandcalculateExpiryare clean utility functions with proper config fallback handling.
55-104: LGTM!
createWarningcorrectly inserts all required fields, calculates points/expiry from config, and logs the creation event.
215-243: LGTM!
removeWarningcorrectly soft-deletes by settingactive = FALSEand records removal metadata.
245-273: LGTM!
clearWarningsbulk-deactivates with proper metadata and logging.
275-299: LGTM!
processExpiredWarningsuses a single UPDATE to deactivate all expired rows efficiently, with error handling that doesn't crash the scheduler.
301-340: LGTM!Scheduler implementation follows the same pattern as tempban scheduler with concurrency guard, immediate startup check, and proper cleanup on stop.
src/commands/removewarn.js (2)
1-22: LGTM!Command definition correctly exports the
SlashCommandBuilderwith appropriate options and validation (setMinValue(1)for ID).
33-68: LGTM!Execute function correctly defers ephemerally, handles not-found cases, logs success with target user context, and has proper error handling with safe reply fallback.
src/index.js (3)
56-59: LGTM!Import follows existing module import patterns and correctly references the new warning engine exports.
282-294: LGTM!
stopWarningExpiryScheduleris correctly placed in the shutdown sequence alongside other scheduler stops. The ordering (triage → cleanup → tempban → warnings → scheduler → github) is appropriate.
486-491: LGTM!
startWarningExpiryScheduleris correctly started inside theif (dbPool)block, ensuring it only runs when database is available, consistent withstartTempbanScheduler.src/api/routes/warnings.js (4)
16-17: LGTM!Rate limiter is properly configured with reasonable limits (120 req/15 min).
92-154: LGTM!The GET
/route correctly implements pagination with bounds checking, dynamic WHERE clause building with parameterized queries, parallel count query, and comprehensive logging. The CodeQL warning is a false positive since rate limiting is applied at the router level.
186-240: LGTM!The user warnings endpoint efficiently batches three parallel queries (warnings, stats, severity counts) and returns a well-structured response. CodeQL warning is a false positive.
265-314: LGTM!The stats endpoint provides useful aggregate data with top users ranking. Four parallel queries are efficient. CodeQL warning is a false positive since rate limiting is applied at router level.
src/commands/clearwarnings.js (3)
1-21: LGTM!Command definition correctly exports
SlashCommandBuilderwith required user option and optional reason.
29-57: LGTM!Execute function correctly handles the zero-warnings case, logs success with count, and has proper error handling with safe reply fallback.
21-21: 🧹 Nitpick | 🔵 TrivialNote:
moderatorOnlyis metadata only.Same as
removewarn.js—the dispatcher enforces permissions via guild config, not this flag. Ensure the command is configured inpermissions.allowedCommands.⛔ Skipped due to learnings
Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-03-05T18:07:15.752Z Learning: All community features must be gated behind `config.<feature>.enabled` flag, mod commands always available regardless of config state
- warningEngine: add try/catch with Winston logging to all CRUD functions - warningEngine: add offset param to getWarnings for pagination - warningEngine: filter expired-but-unprocessed warnings in active queries - warningEngine: log previous values on edit for audit trail - warningEngine: set removed_at in processExpiredWarnings - moderation: only fall back to mod_cases on table-not-found (42P01) - moderation: filter expired warnings in escalation count - warn/editwarn: remove hardcoded point values from severity labels - warnings: add page option, show expired vs removed status - config.json: fix indentation on warning command permissions - dashboard: remove duplicate DM warn toggle (uses existing dmNotifications.warn) - dashboard: fix null expiry rendering (show 0 instead of 90) - dashboard: fix stale closure in escalation threshold editor - types: remove dmNotification from WarningsConfig (conflicts with existing) - tests: fix editWarning mocks for audit trail SELECT, add offset test - tests: add getInteger mock to warnings command, severity-only edit test - Remove TASK.md from tracked files
🧹 Preview Environment Cleaned UpThe Railway preview environment for this PR has been removed. Environment: |
Summary
Comprehensive warning system for moderators and admins — full lifecycle management with severity levels, point-based escalation, automatic expiry/decay, and audit trail.
Closes #250
What changed
New Commands (all moderator-level)
/warn/warnings <user>/editwarn <id> [reason] [severity]/removewarn <id> [reason]/clearwarnings <user> [reason]Warning Engine (
src/modules/warningEngine.js)Database Migration (
011_warnings.cjs)warningstable with severity, points, expiry, soft-delete supportmod_casesunchanged)Escalation Updates (
src/modules/moderation.js)checkEscalation()now only counts active warnings (excludes expired/removed)API Routes (
src/api/routes/warnings.js)GET /api/v1/guilds/:guildId/warnings— list warnings (paginated, filterable)GET /api/v1/guilds/:guildId/warnings/user/:userId— user-specific warningsGET /api/v1/guilds/:guildId/warnings/stats— guild warning statisticsConfig Defaults
{ "moderation": { "warnings": { "expiryDays": 90, "severityPoints": { "low": 1, "medium": 2, "high": 3 } } } }Tests
tests/modules/warningEngine.test.js— 29 tests (CRUD, expiry, scheduler, severity points)tests/commands/warn.test.js— updated for moderatorOnly + severitytests/commands/warnings.test.js— history display + error handlingtests/commands/editwarn.test.js— edit flow + validationtests/commands/removewarn.test.js— soft-delete flowtests/commands/clearwarnings.test.js— bulk clear flowValidation
pnpm lint✅pnpm test✅ (3685 passed, 1 pre-existing failure in server.test.js)