feat: add moderation command suite (v2.0.0)#20
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a full moderation subsystem: 15 admin-only slash commands, a central moderation module with DB schema and tempban scheduler, duration utilities, README/config updates, small tooling file tweaks, startup lifecycle integration, and extensive unit tests across commands and modules. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 41
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.gitignore (1)
5-5: 🧹 Nitpick | 🔵 TrivialDuplicate
coverage/entry.
coverage/is listed on both Line 5 and Line 22. Remove one of them.Also applies to: 22-22
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 97-108: The documented Moderation Command Pattern in AGENTS.md is
out of order compared to the implementations in src/modules/moderation.js and
command files like kick.js, tempban.js and warn.js: the code sends DM
notifications before executing the Discord action (see pitfall `#7`). Update the
list so that "sendDmNotification()" appears before "Execute the Discord action
(ban, kick, timeout, etc.)", ensuring steps reference createCase(),
sendModLogEmbed(), and checkEscalation() in the same relative order as the
implementation.
In `@README.md`:
- Line 149: Update the stale README description for the `moderation.enabled`
field so it accurately reflects its current scope: replace "Enable spam
detection" with a concise phrase like "Enable moderation features" (or
equivalent) to indicate it governs the full moderation suite (warn, kick, ban,
case management, etc.); locate the `moderation.enabled` entry in the README
table and update its description text accordingly.
In `@src/commands/ban.js`:
- Around line 55-63: The current flow checks moderator vs target via
checkHierarchy but misses verifying the bot's own role; before calling
guild.members.ban() (after checkHierarchy and before sendDmNotification or the
ban), add a bot-self hierarchy check that compares interaction.guild.me (or
interaction.client.user) / interaction.guild.members.cache.get(client.user.id)
highest role against the target member's highest role, and if the bot does not
outrank the target return a clear interaction.editReply message like "I cannot
ban this member because my role is not high enough." Also ensure the catch block
for guild.members.ban() surfaces a friendly message when the bot lacks
permission or hierarchy rather than raw Discord errors.
- Around line 86-94: The catch block currently assumes interaction.deferred and
calls interaction.reply in the else branch which can itself throw; update the
error handler in the catch to always attempt interaction.editReply (since
deferReply is called earlier) and wrap the reply/edit in its own try/catch to
avoid unhandled rejections: build the error message (content), then try await
interaction.editReply(content) and if that fails fall back to a safe await
interaction.reply({ content, ephemeral: true }) inside the inner catch, and
ensure you still call logError('Command error', { error: err.message, command:
'ban' }) — modify the catch surrounding the names interaction.deferred,
deferReply, editReply, and reply accordingly.
In `@src/commands/case.js`:
- Around line 44-57: The slash command option builder for the 'type' filter (the
addStringOption chain where setName('type') and addChoices are used) is missing
valid action types; update the addChoices call for the 'type' option to include
the additional choices 'lock', 'unlock', 'purge', and 'untimeout' so users can
filter mod_cases by those actions in the UI.
- Around line 228-242: The inline channel-key mapping in the case handling block
duplicates the ACTION_LOG_CHANNEL_KEY constant; replace the inline object used
to derive channelKey with a reference to ACTION_LOG_CHANNEL_KEY from the
moderation module (i.e., import ACTION_LOG_CHANNEL_KEY and use
ACTION_LOG_CHANNEL_KEY[caseRow.action] where the current inline mapping is
used), and add the import for ACTION_LOG_CHANNEL_KEY at the top of the file to
keep the routing logic centralized.
- Around line 15-24: Remove the duplicated ACTION_COLORS and channelKey mapping
from case.js and instead export ACTION_COLORS and ACTION_LOG_CHANNEL_KEY from
src/modules/moderation.js (ensure ACTION_COLORS includes purge, lock, unlock and
any other action types), then import those constants into case.js and replace
the local definitions with the imported symbols (ACTION_COLORS and
ACTION_LOG_CHANNEL_KEY). Also update the list subcommand's filter choices in
case.js to include the missing action types untimeout, lock, unlock, and purge
so the choices cover all valid actions.
In `@src/commands/history.js`:
- Around line 24-25: Move the call to getPool() inside the existing try block
that handles the command execution (i.e., after interaction.deferReply()) and
remove the top-level call; call getPool() there (or await it if appropriate) so
any thrown error is caught by the catch. In the catch handler, ensure you send a
user-facing response (e.g., interaction.editReply or interaction.followUp)
indicating the DB error so the deferred interaction doesn't stay stuck. Update
references to the pooled variable (pool) to use the one from inside the try
block.
In `@src/commands/lock.js`:
- Around line 61-63: The catch block in lock.js is logging errors without the
command metadata; update the logError call inside the catch of the Lock command
to include command: 'lock' in the metadata object (same pattern used in
kick.js/tempban.js) so the call in the catch where logError('Lock command
failed', { error: err.message }) becomes the same shape but with command: 'lock'
added.
- Around line 30-34: Move the pre-checks and option parsing into the execute
function's try block so failures are caught consistently: wrap
interaction.deferReply({ ephemeral: true }), the channel resolution using
interaction.options.getChannel('channel') || interaction.channel, and reason =
interaction.options.getString('reason') inside the existing try in execute,
mirroring the pattern used in tempban.js, and adjust any early returns to throw
or return inside that try so interaction.followUp or error logging runs from the
catch.
- Around line 14-18: Update both lock.js and unlock.js to restrict the channel
picker to text channels by extending the existing addChannelOption chain with
.addChannelTypes(ChannelType.GuildText) (ensure ChannelType is imported from
discord.js if not already), and fix the error logging call around the current
logger/processLogger.error usage (the one referenced at line 62 in both files)
to include the structured metadata key command with the appropriate value
('lock' for lock.js and 'unlock' for unlock.js) so the error logs match the
format used in kick.js/tempban.js.
In `@src/commands/modlog.js`:
- Around line 149-164: Wrap the body of handleView (and similarly handleDisable)
in a try-catch so runtime errors from getConfig() or config mutators like
setConfigValue do not bubble to Discord; inside the try keep current logic
(calling getConfig(), building EmbedBuilder, and interaction.reply) and in catch
log the error and send an ephemeral interaction.reply with a simple
user-friendly failure message (e.g., "Failed to load mod log configuration") so
the user gets a clear response and the error is recorded; reference the
functions handleView, handleDisable, getConfig, setConfigValue, and
interaction.reply when making the changes.
- Around line 170-179: The loop in handleDisable is setting the string 'null'
instead of the actual null value, so moderation logging channels remain truthy;
change the calls in handleDisable to pass null (not the string) to
setConfigValue for each key in the keys array (e.g.,
moderation.logging.channels.default, warns, bans, kicks, timeouts, purges,
locks) so sendModLogEmbed and handleView see a true null and clear/render
channels properly.
- Around line 32-46: The switch in execute(interaction) lacks a default branch
so unknown subcommands leave the interaction unanswered; add a default case to
the switch that sends a safe fallback reply (e.g. interaction.reply or
interaction.followUp with an ephemeral "Unknown subcommand" or similar message)
and optionally log the unexpected subcommand value (use
interaction.options.getSubcommand()) to aid debugging; ensure the reply is only
sent if the interaction hasn't already been replied to to avoid errors.
- Around line 1-17: The module imports only the info logger but needs the error
logger for proper error reporting; update the import from logger.js to include
error (alongside info) and then use error in the sub-handlers (handleSetup,
handleView, handleDisable) when logging exceptions or in any added try-catch
blocks so errors are recorded consistently.
In `@src/commands/purge.js`:
- Around line 109-114: The current fetch-then-filter logic in the purge handler
(around channel.messages.fetch, fetched, fourteenDaysAgo, filtered) only fetches
`count` messages then filters (so `/purge user count:50` may delete far fewer
than 50); change the implementation to paginate and accumulate fetched messages
until you have scanned the requested `count` messages (use the `before`
parameter on channel.messages.fetch in a loop, updating a `lastId` cursor and
stopping when scanned >= count or no more messages), then apply the user/date
filter and bulk-delete the results; alternatively, if you prefer the simpler
route, update the command description to explicitly state that `count` is the
number of messages scanned rather than guaranteed deleted.
- Around line 149-179: Add a moderation case for purge actions and reuse the
shared mod-log helper instead of building the embed inline: call createCase(...)
with the purge action details (include actor from interaction.user, target
channel id, reason/metadata such as deleted.size and subcommand/filter) right
after a successful purge, then call sendModLogEmbed(...) (or extend it to accept
extraFields) to resolve the log channel and send an embed that includes the
Deleted count and Filter field; remove the duplicated
channel-resolution/embed-sending block and ensure you pass interaction.client,
interaction.user, channel.id, deleted.size and subcommand to the helper so purge
actions appear in /case and /history like other moderation commands.
In `@src/commands/slowmode.js`:
- Line 43: When computing seconds = Math.min(Math.floor(ms / 1000), 21600) in
the slowmode handler, detect whether the input was capped (i.e., computedSeconds
< requestedSeconds) and include a clear note in the response to the moderator
that the requested duration was reduced to the 6-hour maximum; because
deferReply is used you can append this note to the existing success reply (or
send a followUp) so the reply reads like "Slowmode set to **6 hours** (requested
7 hours, capped to max)". Update the slowmode command logic around the seconds
calculation and the reply path to build and send this appended message whenever
capping occurs.
- Around line 29-61: The slowmode execute function currently changes channel
rate limit but does not create a moderation case or post to the mod log; update
execute to call createCase(...) for both enabling and disabling slowmode (use
action like "slowmode" with fields: moderator = interaction.user, target =
channel.id, duration = seconds or null, and reason from options if available)
and then call the project's mod-log posting helper (e.g., postToModLog or
sendModLog) with the same case details so the change appears in audit logs;
ensure these calls occur after a successful channel.setRateLimitPerUser and
include the created case ID in the mod-log entry and in the reply message.
In `@src/commands/softban.js`:
- Around line 56-58: The DM check is using the wrong key — shouldSendDm(config,
'ban') ties softban notifications to the general ban toggle; update the call in
the softban flow to shouldSendDm(config, 'softban') so softban DMs are
controlled independently (or, if grouping is intentional, add an inline comment
next to shouldSendDm(config, 'ban') explaining this design choice); update any
related documentation or config schema keys if needed and ensure
sendDmNotification(target, 'softban', reason, interaction.guild.name) remains
unchanged.
- Around line 60-65: The softban flow currently awaits
interaction.guild.members.ban(...) then awaits
interaction.guild.members.unban(...), which can leave the user permanently
banned if the unban fails; change this by isolating the unban in a
retry/recovery block: after calling interaction.guild.members.ban(target.id, {
deleteMessageSeconds: deleteMessageDays * 86400, reason: reason || undefined }),
attempt interaction.guild.members.unban(target.id, 'Softban') inside a small
retry loop (e.g., 3 attempts with short delays) and catch errors per-attempt; if
retries exhaust, log a clear error including target.id and reason and
enqueue/schedule a background retry or persist a record so an operator or a
worker can reattempt unban later — ensure all references use
interaction.guild.members.unban and target.id so the fix is applied to the exact
call site.
In `@src/commands/tempban.js`:
- Around line 95-99: Extract the raw INSERT into a new exported helper in the
moderation module: add a scheduleAction(guildId, action, targetId, caseId,
executeAt) function that uses the same DB pool/query to insert into
mod_scheduled_actions and returns/throws on error; export it alongside
createCase. In tempban.js replace the inline pool.query block (which currently
calls getPool() and inserts into mod_scheduled_actions) with a call to
moderation.scheduleAction(interaction.guild.id, 'unban', user.id, caseData.id,
expiresAt). Ensure to import the new scheduleAction symbol from the moderation
module and keep error handling consistent with other moderation helpers.
- Around line 44-46: The call to interaction.deferReply currently sits outside
the try/catch in execute, so any exception from deferReply is unhandled and
makes the catch's handling of interaction.deferred meaningless; move the
interaction.deferReply({ ephemeral: true }) call inside the try block before the
rest of the logic so the catch can handle both deferred and non-deferred states,
and adjust the catch to correctly branch on interaction.deferred (preserve the
existing else branch so it is reachable); apply the same change to the other
similar block around the second usage (the code referenced in the review) so
both deferReply calls are inside their try blocks and handled by their catches.
In `@src/commands/timeout.js`:
- Around line 47-50: Add an explicit upper-bound check after parsing the
duration: when using parseDuration(durationStr) inside the timeout command
(where durationMs is computed), verify durationMs is <= 2419200000 (Discord's
28-day limit) and if not, return await interaction.editReply with a clear error
like '❌ Duration exceeds Discord's 28-day timeout limit.' This validation should
live alongside the existing invalid-format guard in the timeout command so
values like "60d" are rejected before calling the Discord API.
In `@src/commands/unban.js`:
- Around line 37-44: The case record is storing targetTag as the raw snowflake
(userId); update the unban flow to resolve the user's human-readable tag before
calling createCase by attempting to fetch the user (e.g., using
client.users.fetch(userId) or resolving via
interaction.guild.members.fetch/resolved user) and set targetTag to the
fetchedUser.tag; wrap the fetch in a try/catch and fall back to the original
userId if the fetch fails so createCase(action:'unban', targetId:userId,
targetTag:resolvedTag, moderatorId:interaction.user.id,
moderatorTag:interaction.user.tag, reason) always receives a sensible targetTag.
In `@src/commands/warn.js`:
- Around line 76-83: In the catch block handling command errors (the catch
around the warn command that calls logError and replies via interaction), stop
echoing err.message back to the user; instead send a generic user-facing message
like "❌ Failed to execute command. Please contact an administrator." while
preserving full error details only in the logError call (logError('Command
error', { error: err.message, command: 'warn' })); update the
interaction.editReply/interaction.reply branches to use the generic message and
ensure no internal error strings are interpolated into user-visible content.
In `@src/db.js`:
- Around line 145-148: The explicit CREATE INDEX for idx_mod_cases_guild_case on
table mod_cases is redundant because the UNIQUE(guild_id, case_number)
constraint already creates an implicit unique index; remove the CREATE INDEX
statement (the block that creates idx_mod_cases_guild_case) so you don't double
the index and incur extra write overhead, leaving the UNIQUE constraint as the
sole index for (guild_id, case_number).
In `@src/index.js`:
- Around line 339-341: The tempban scheduler is started unconditionally which
causes getPool() to throw when the DB isn't initialized; only start the
scheduler when the database is configured/initialized (use the same guard used
for DB-dependent features). Wrap the call to startTempbanScheduler(client) in
the same condition that you use to initialize the DB (e.g. check
process.env.DATABASE_URL or a boolean like dbInitialized), or check a helper
such as isDatabaseInitialized() before calling startTempbanScheduler so
getPool() is never invoked when the DB is absent.
In `@src/modules/moderation.js`:
- Around line 289-340: pollTempbans creates an unban case with targetTag set to
row.target_id (a numeric snowflake) instead of the user's
username#discriminator; fetch the user before building the case (e.g. await
client.users.fetch(row.target_id)) and use the fetched user's tag for targetTag,
falling back to row.target_id if the fetch fails or returns null, and ensure any
fetch errors are caught so the rest of the unban/case logic proceeds.
- Around line 193-201: The catch block after attempting to update mod_cases
(using getPool() and pool.query with log_message_id and caseData.id) is
currently silent; replace the empty catch with one that captures the error
(e.g., catch (err)) and logs a warning including context: the operation
("storing log_message_id"), the case id (caseData.id), the message id
(sentMessage.id), and the error details; use the project logger if available (or
console.warn/error) so the failure is recorded for debugging.
- Around line 292-295: The SELECT against mod_scheduled_actions is unbounded and
can return a huge result set; change the query used in pool.query (the one
selecting WHERE executed = FALSE AND execute_at <= NOW()) to acquire a bounded,
locked batch—wrap it in a transaction and use FOR UPDATE SKIP LOCKED with a
LIMIT (e.g., LIMIT 50) so you only claim a small number of rows each poll and
avoid overlapping polls; update the code paths that call pool.query and process
the rows to rely on this transaction/lock pattern and handle remaining rows in
subsequent polls.
- Around line 72-79: getNextCaseNumber followed by a separate INSERT causes a
race condition under concurrent commands; instead, remove the separate MAX read
and perform an atomic INSERT that computes the next case number in a subquery
inside createCase (e.g. INSERT ... VALUES (..., (SELECT
COALESCE(MAX(case_number),0)+1 FROM mod_cases WHERE guild_id = $X)) RETURNING
case_number) so the database assigns the next number atomically; update
createCase to use that single-statement insert and keep the UNIQUE(guild_id,
case_number) constraint as a safety net and handle/propagate any transient
unique-violation errors if they occur.
In `@src/utils/duration.js`:
- Around line 51-62: formatDuration currently returns "0 seconds" for inputs not
evenly divisible by any unit (e.g., 1500ms); update the JSDoc for the
formatDuration function to clearly state its contract: it accepts a number of
milliseconds, only formats values that are exact multiples of units from
UNIT_LIST, returns a singular/plural unit string for exact matches, and
otherwise returns "0 seconds"; also mention the relationship with parseDuration
(that parseDuration + formatDuration is a round-trip for parseable inputs) so
future callers won't be surprised by the silent fallback.
In `@tests/commands/kick.test.js`:
- Around line 30-35: The shared mockMember object is defined once at describe
scope and can leak mutations between tests; move its creation into the test
factory (createInteraction) so each test gets a fresh instance — remove the
top-level mockMember, instantiate a new mockMember inside createInteraction
(with id, user, roles, and kick: vi.fn().mockResolvedValue()), and return or
attach it to the interaction object used by tests; also keep vi.clearAllMocks()
in afterEach but ensure tests reference the interaction-specific mockMember (not
a shared variable) so property mutations don't persist across tests.
In `@tests/commands/softban.test.js`:
- Around line 30-61: Add a new test in tests/commands/softban.test.js that
simulates options.getMember returning null by modifying the createInteraction
stub (or creating a variant) so getMember: vi.fn().mockReturnValue(null); then
invoke the softban command handler with that interaction and assert that
guild.members.ban and guild.members.unban are not called and that the
interaction receives the expected error response (check interaction.reply or
interaction.editReply is called with the non-resolvable-member message). Ensure
the test references the existing createInteraction helper, options.getMember,
guild.members.ban, guild.members.unban, and interaction.reply/editReply mocks so
it fails if null handling is missing.
In `@tests/commands/unban.test.js`:
- Around line 78-87: Add a new test case that simulates the Discord API failing
by making guild.members.unban reject (e.g., mockRejectedValueOnce(new
Error('unban failed'))) while leaving createCase to resolve, then call
execute(interaction) and assert interaction.editReply was called with
expect.stringContaining('Failed to execute'); locate mocks in the existing test
file by referencing guild.members.unban, createCase, createInteraction and
execute to add this explicit error-path test so the catch block behavior is
covered.
In `@tests/commands/untimeout.test.js`:
- Around line 28-33: The shared mockMember object can leak state across tests;
change the test setup so a fresh mock member is created per test (e.g., add a
beforeEach that constructs mockMember or implement a createMockMember() factory
used by createInteraction), ensure the mock's timeout is set via
vi.fn().mockResolvedValue(...) on each creation, and keep vi.clearAllMocks() in
afterEach to clear mock histories—this guarantees tests reference a new
mockMember object each run and prevents cross-test mutations.
In `@tests/commands/warn.test.js`:
- Around line 69-87: Add an assertion that verifies checkEscalation is invoked
with the correct arguments: assert it's called with the bot client instance, the
guild id ('guild1'), the target id ('user1'), and the case payload (use
expect.objectContaining to match the case fields like action: 'warn' and
targetTag). Update the test in warn.test.js (the 'should warn a user
successfully' case that calls execute and currently asserts checkEscalation was
called) to include this to catch incorrect wiring between execute and
checkEscalation.
In `@tests/index.test.js`:
- Around line 256-262: Update the fragile settling sequence in
tests/index.test.js to document the exact required hop count and why: replace
the vague comment with a specific note like "requires 3 microtask hops (three
await Promise.resolve()) plus one macrotask (setImmediate) to match startup()'s
current awaits — if startup() adds/removes awaits update this count" and/or
extract the four awaits into a named helper (e.g., settleStartupHops or
flushStartupMicrotasks) referenced from tests so future changes to startup() are
obvious and centralized; mention startup() by name in the comment so maintainers
know what to update.
In `@tests/modules/moderation.test.js`:
- Around line 490-571: Add a test for pollTempbans error path by copying the
existing "should process expired tempbans on poll" test but make the
guild.members.unban (or guilds.fetch) reject (e.g., mockRejectedValue(new
Error('fail'))), run the scheduler with vi.useFakeTimers and
startTempbanScheduler, advance timers to trigger the poll, then
stopTempbanScheduler; finally assert that the DB update marking the row executed
= TRUE was issued by checking mockPool.query calls include the UPDATE that sets
executed = TRUE for the returned row id (referencing pollTempbans /
startTempbanScheduler / stopTempbanScheduler and mockPool.query to locate where
to inject the failing mock and verify the executed=true update).
- Around line 45-56: The beforeEach currently calls
getPool.mockReturnValue(mockPool) and then vi.clearAllMocks(), which is
misleading because clearing mocks after setting the return makes it look like
you just wiped the setup; change the order so vi.clearAllMocks() runs first,
then create mockPool and call getPool.mockReturnValue(mockPool) to set the
stubbed implementation; keep the afterEach teardown (stopTempbanScheduler and
vi.restoreAllMocks) as-is.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (44)
.gitignoreAGENTS.mdREADME.mdbiome.jsonconfig.jsonsrc/commands/ban.jssrc/commands/case.jssrc/commands/history.jssrc/commands/kick.jssrc/commands/lock.jssrc/commands/modlog.jssrc/commands/purge.jssrc/commands/slowmode.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/timeout.jssrc/commands/unban.jssrc/commands/unlock.jssrc/commands/untimeout.jssrc/commands/warn.jssrc/db.jssrc/index.jssrc/modules/moderation.jssrc/utils/duration.jstests/commands/ban.test.jstests/commands/case.test.jstests/commands/history.test.jstests/commands/kick.test.jstests/commands/lock.test.jstests/commands/modlog.test.jstests/commands/purge.test.jstests/commands/slowmode.test.jstests/commands/softban.test.jstests/commands/tempban.test.jstests/commands/timeout.test.jstests/commands/unban.test.jstests/commands/unlock.test.jstests/commands/untimeout.test.jstests/commands/warn.test.jstests/db.test.jstests/index.test.jstests/modules/ai.test.jstests/modules/moderation.test.jstests/utils/duration.test.js
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Use ESM syntax withimport/export; never userequire()
Always usenode:protocol when importing Node.js builtins (e.g.,import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Always use Winston logging viaimport { info, warn, error } from '../logger.js'— NEVER useconsole.log(),console.warn(),console.error(), or anyconsole.*method in src/ files
Pass structured metadata when logging:info('Message processed', { userId, channelId })
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig()fromsrc/modules/config.jsto read config; usesetConfigValue(key, value)to update at runtime
UsesplitMessage()utility for messages exceeding Discord's 2000-character limit
No TypeScript — use plain JavaScript with JSDoc comments for documentation
Files:
src/commands/case.jssrc/commands/untimeout.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/unban.jssrc/commands/lock.jssrc/commands/modlog.jssrc/commands/purge.jssrc/commands/kick.jssrc/commands/warn.jssrc/commands/history.jssrc/commands/ban.jssrc/commands/slowmode.jssrc/utils/duration.jssrc/db.jssrc/commands/unlock.jssrc/index.jssrc/commands/timeout.jssrc/modules/moderation.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Slash commands must export
data(SlashCommandBuilder) andexecute(interaction)function
Files:
src/commands/case.jssrc/commands/untimeout.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/unban.jssrc/commands/lock.jssrc/commands/modlog.jssrc/commands/purge.jssrc/commands/kick.jssrc/commands/warn.jssrc/commands/history.jssrc/commands/ban.jssrc/commands/slowmode.jssrc/commands/unlock.jssrc/commands/timeout.js
tests/**/*.{js,test.js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for testing with
pnpm test; test coverage provider is@vitest/coverage-v8
Files:
tests/commands/unlock.test.jstests/commands/ban.test.jstests/commands/softban.test.jstests/modules/ai.test.jstests/commands/tempban.test.jstests/commands/kick.test.jstests/index.test.jstests/commands/slowmode.test.jstests/commands/modlog.test.jstests/commands/lock.test.jstests/commands/warn.test.jstests/commands/purge.test.jstests/commands/untimeout.test.jstests/commands/case.test.jstests/utils/duration.test.jstests/commands/history.test.jstests/commands/timeout.test.jstests/commands/unban.test.jstests/modules/moderation.test.jstests/db.test.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Check
config.yourModule.enabledbefore processing in module handlers
Files:
src/modules/moderation.js
🧠 Learnings (14)
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/commands/*.js : Slash commands must export `data` (SlashCommandBuilder) and `execute(interaction)` function
Applied to files:
src/commands/case.jstests/commands/unlock.test.jstests/commands/ban.test.jssrc/commands/untimeout.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/unban.jstests/commands/softban.test.jssrc/commands/lock.jstests/commands/tempban.test.jssrc/commands/modlog.jssrc/commands/purge.jssrc/commands/kick.jstests/commands/slowmode.test.jssrc/commands/warn.jssrc/commands/history.jssrc/commands/ban.jssrc/commands/slowmode.jstests/commands/warn.test.jstests/commands/purge.test.jstests/commands/case.test.jstests/commands/history.test.jstests/commands/timeout.test.jssrc/commands/unlock.jstests/commands/unban.test.jssrc/commands/timeout.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Commands are auto-discovered from `src/commands/` on startup; after adding a command, run `pnpm run deploy` to register with Discord
Applied to files:
src/commands/purge.jssrc/commands/ban.jsAGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use `splitMessage()` utility for messages exceeding Discord's 2000-character limit
Applied to files:
src/commands/purge.jsAGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to tests/**/*.{js,test.js} : Use Vitest for testing with `pnpm test`; test coverage provider is `vitest/coverage-v8`
Applied to files:
.gitignoretests/commands/purge.test.jsbiome.jsontests/utils/duration.test.js
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/**/*.{ts,tsx} : Use `date-fns` for date manipulation and formatting throughout the application
Applied to files:
src/utils/duration.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use single quotes for strings (enforced by Biome)
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use 2-space indentation (enforced by Biome)
Applied to files:
biome.json
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json,css,md} : After changing or editing any files, run the complete validation workflow: `pnpm format && pnpm typecheck && pnpm lint && pnpm build` before committing
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : No TypeScript — use plain JavaScript with JSDoc comments for documentation
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use ESM syntax with `import`/`export`; never use `require()`
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Graceful shutdown is handled in `src/index.js`; use error classes and logging before throwing errors in other modules
Applied to files:
src/index.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Module handlers must be registered in `src/modules/events.js` to wire them to Discord events
Applied to files:
src/index.jsAGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Update Key Files table in AGENTS.md when adding a new command or module
Applied to files:
AGENTS.md
📚 Learning: 2026-02-11T17:18:14.598Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-11T17:18:14.598Z
Learning: See AGENTS.md for full project context, architecture, and coding guidelines
Applied to files:
AGENTS.md
🧬 Code graph analysis (31)
src/commands/case.js (5)
src/modules/moderation.js (13)
ACTION_COLORS(17-29)pool(73-73)pool(74-77)pool(96-96)pool(99-116)pool(194-194)pool(234-234)pool(237-242)pool(291-291)pool(292-295)embed(139-143)embed(170-183)config(307-307)src/commands/history.js (7)
data(10-13)data(10-13)pool(25-25)pool(28-31)embed(56-63)user(24-24)lines(37-45)src/modules/config.js (1)
getConfig(130-132)src/db.js (2)
pool(12-12)getPool(187-192)src/logger.js (1)
info(216-218)
tests/commands/unlock.test.js (2)
src/commands/unlock.js (1)
execute(30-65)src/modules/moderation.js (2)
createCase(95-127)sendModLogEmbed(159-208)
src/commands/untimeout.js (3)
src/modules/moderation.js (5)
config(307-307)reason(247-247)checkHierarchy(382-387)createCase(95-127)sendModLogEmbed(159-208)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/commands/softban.js (4)
src/index.js (2)
interaction(178-178)config(52-52)src/modules/moderation.js (7)
config(307-307)reason(247-247)checkHierarchy(382-387)shouldSendDm(395-397)sendDmNotification(137-150)createCase(95-127)sendModLogEmbed(159-208)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/commands/tempban.js (5)
src/modules/moderation.js (16)
config(307-307)reason(247-247)member(252-252)checkHierarchy(382-387)shouldSendDm(395-397)sendDmNotification(137-150)createCase(95-127)pool(73-73)pool(74-77)pool(96-96)pool(99-116)pool(194-194)pool(234-234)pool(237-242)pool(291-291)pool(292-295)src/modules/config.js (1)
getConfig(130-132)src/utils/duration.js (2)
parseDuration(23-36)formatDuration(51-62)src/db.js (1)
getPool(187-192)src/logger.js (1)
info(216-218)
tests/commands/softban.test.js (1)
src/modules/moderation.js (3)
sendDmNotification(137-150)createCase(95-127)checkHierarchy(382-387)
src/commands/lock.js (4)
src/commands/unlock.js (6)
execute(30-65)channel(32-32)reason(33-33)notifyEmbed(40-45)config(48-48)caseData(49-56)src/modules/moderation.js (5)
channel(167-167)reason(247-247)config(307-307)createCase(95-127)sendModLogEmbed(159-208)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
tests/commands/tempban.test.js (3)
src/modules/moderation.js (1)
sendDmNotification(137-150)src/db.js (1)
getPool(187-192)src/utils/duration.js (1)
parseDuration(23-36)
src/commands/modlog.js (2)
src/modules/config.js (3)
i(360-360)setConfigValue(141-225)getConfig(130-132)src/logger.js (1)
info(216-218)
src/commands/purge.js (2)
src/logger.js (1)
info(216-218)src/modules/config.js (1)
getConfig(130-132)
tests/commands/kick.test.js (2)
src/commands/kick.js (5)
data(17-23)data(17-23)adminOnly(25-25)adminOnly(25-25)execute(31-77)src/modules/moderation.js (3)
sendDmNotification(137-150)createCase(95-127)checkHierarchy(382-387)
src/commands/kick.js (3)
src/modules/moderation.js (7)
config(307-307)reason(247-247)checkHierarchy(382-387)shouldSendDm(395-397)sendDmNotification(137-150)createCase(95-127)sendModLogEmbed(159-208)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
tests/commands/slowmode.test.js (2)
src/commands/slowmode.js (1)
channel(31-31)src/utils/duration.js (1)
parseDuration(23-36)
src/commands/warn.js (3)
src/commands/ban.js (6)
data(17-31)data(17-31)config(43-43)reason(45-45)hierarchyError(56-56)caseData(71-78)src/modules/moderation.js (8)
config(307-307)reason(247-247)checkHierarchy(382-387)shouldSendDm(395-397)sendDmNotification(137-150)createCase(95-127)sendModLogEmbed(159-208)checkEscalation(221-283)src/logger.js (1)
info(216-218)
src/commands/history.js (2)
src/db.js (1)
getPool(187-192)src/logger.js (1)
info(216-218)
tests/commands/modlog.test.js (1)
src/commands/modlog.js (2)
subcommand(33-33)execute(32-46)
tests/commands/lock.test.js (2)
src/commands/lock.js (1)
execute(30-65)src/modules/moderation.js (2)
createCase(95-127)sendModLogEmbed(159-208)
src/commands/ban.js (2)
src/modules/moderation.js (8)
config(307-307)reason(247-247)member(252-252)checkHierarchy(382-387)shouldSendDm(395-397)sendDmNotification(137-150)createCase(95-127)sendModLogEmbed(159-208)src/logger.js (1)
info(216-218)
src/commands/slowmode.js (1)
src/utils/duration.js (3)
ms(33-33)parseDuration(23-36)formatDuration(51-62)
tests/commands/warn.test.js (2)
src/commands/warn.js (5)
data(18-24)data(18-24)adminOnly(26-26)adminOnly(26-26)execute(32-85)src/modules/moderation.js (5)
sendDmNotification(137-150)createCase(95-127)sendModLogEmbed(159-208)checkEscalation(221-283)checkHierarchy(382-387)
src/utils/duration.js (2)
src/modules/moderation.js (1)
ms(255-255)src/commands/slowmode.js (1)
ms(37-37)
src/db.js (2)
src/commands/history.js (2)
pool(25-25)pool(28-31)src/modules/moderation.js (4)
pool(73-73)pool(74-77)pool(96-96)pool(99-116)
tests/commands/untimeout.test.js (1)
src/modules/moderation.js (2)
createCase(95-127)checkHierarchy(382-387)
tests/commands/case.test.js (2)
src/commands/case.js (6)
subcommand(110-110)data(26-78)data(26-78)adminOnly(80-80)adminOnly(80-80)execute(107-131)src/db.js (1)
getPool(187-192)
tests/utils/duration.test.js (1)
src/utils/duration.js (2)
parseDuration(23-36)formatDuration(51-62)
tests/commands/timeout.test.js (1)
src/utils/duration.js (1)
parseDuration(23-36)
src/commands/unlock.js (4)
src/commands/lock.js (6)
execute(30-65)channel(32-32)reason(33-33)notifyEmbed(40-45)config(48-48)caseData(49-56)src/commands/unban.js (4)
execute(27-61)reason(33-33)config(31-31)caseData(37-44)src/modules/moderation.js (5)
channel(167-167)reason(247-247)config(307-307)createCase(95-127)sendModLogEmbed(159-208)src/logger.js (1)
info(216-218)
src/index.js (1)
src/modules/moderation.js (2)
stopTempbanScheduler(368-374)startTempbanScheduler(348-363)
tests/commands/unban.test.js (2)
src/commands/unban.js (1)
execute(27-61)src/modules/moderation.js (1)
createCase(95-127)
tests/modules/moderation.test.js (1)
src/modules/moderation.js (12)
getNextCaseNumber(72-79)createCase(95-127)member(252-252)sendDmNotification(137-150)embed(139-143)embed(170-183)config(307-307)sendModLogEmbed(159-208)checkEscalation(221-283)checkHierarchy(382-387)shouldSendDm(395-397)startTempbanScheduler(348-363)
src/modules/moderation.js (4)
src/db.js (2)
pool(12-12)getPool(187-192)src/logger.js (1)
info(216-218)src/modules/config.js (1)
getConfig(130-132)src/utils/duration.js (2)
ms(33-33)parseDuration(23-36)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/index.test.js (1)
509-519:⚠️ Potential issue | 🟡 MinorAdd assertion for
stopTempbanSchedulerin SIGINT shutdown test.The test at lines 509–519 asserts
closeDbandclient.destroybut omits verifying thatstopTempbanScheduler()is called. Sinceindex.jsline 247 invokes it during graceful shutdown (documented in AGENTS.md pitfall#10), the test should assert:expect(mocks.moderation.stopTempbanScheduler).toHaveBeenCalled();Without this assertion, a regression where
stopTempbanScheduler()is accidentally removed would go undetected.
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 92-95: The markdown linter warns about ordered-list prefix (MD029)
where the numbered steps (steps referencing adminOnly, src/commands/, pnpm run
deploy, and permissions.allowedCommands in config.json) intentionally resume
numbering; to silence the warning either renumber the list using consistent "1."
prefixes for each step or add a localized linter suppression around the list
(e.g. insert <!-- markdownlint-disable MD029 --> before the list and <!--
markdownlint-enable MD029 --> after) so the intentional continuation remains
while preventing MD029 noise.
In `@src/commands/ban.js`:
- Around line 39-42: The call to interaction.deferReply is outside the try in
execute, so if it throws (e.g., expired token) the error skips the catch and
makes the later else branch (checking interaction.deferred) dead; move await
interaction.deferReply({ ephemeral: true }) inside the try block at the start of
execute (same pattern as tempban.js), ensure subsequent logic relies on
interaction.deferred only when appropriate, and remove or adjust the unreachable
else branch (lines referencing interaction.deferred) so errors are handled by
the catch and the flow matches tempban's behavior.
In `@src/commands/case.js`:
- Around line 259-279: handleDelete currently permanently removes rows from
mod_cases with no persistent audit record; modify it to create an immutable
audit entry before deletion (or implement soft-delete) by either inserting a row
into a new audit table (e.g., mod_case_audits or mod_case_deletes) with case
details, deleter id/tag and timestamp inside handleDelete, or alter the
mod_cases schema to add deleted_at and deleted_by and perform an UPDATE instead
of DELETE; ensure the insert/update happens before the DELETE (if keeping hard
delete) and keep the existing info(...) call for immediate logging while
returning the same user-facing reply in handleDelete.
- Around line 118-121: The catch block in src/commands/case.js logs the error
via logError('Case command failed', { error: err.message, subcommand }) but the
user-facing reply (interaction.editReply) omits err.message; update the catch
handler to include the error message in the reply (e.g., change the reply text
to include err.message or `❌ Failed to execute case command: ${err.message}`) so
admins see the failure details while keeping the existing logError call and
subcommand context intact.
- Around line 104-121: The switch over subcommand in the main case handler
(where getSubcommand() is used) lacks a default branch, so if an unexpected
value is returned none of the handlers (handleView, handleList, handleReason,
handleDelete) run and the interaction is never replied to; add a default case
that logs the unexpected subcommand (using the same logError or console logger)
and sends a timely interaction.editReply (or interaction.reply if not deferred)
with a clear error message to avoid leaving the user "thinking." Ensure the
default branch mirrors the catch-path behavior by including the subcommand in
the log and a user-facing failure message.
In `@src/commands/history.js`:
- Around line 26-31: The query in src/commands/history.js currently uses SELECT
* against mod_cases which pulls unnecessary large columns; change the SQL in the
call that uses getPool().query(...) to explicitly select only the needed columns
(created_at, case_number, action, reason) instead of *, so the code retrieves
only those four fields for the ORDER BY created_at DESC LIMIT 25 query against
mod_cases.
In `@src/commands/modlog.js`:
- Around line 100-151: The shared EmbedBuilder instance (embed) is being mutated
inside collector.on('collect') via embed.setDescription(...) which can race with
the collector 'end' handler; instead, create a fresh embed for each
interaction/update to avoid shared-state mutation: when handling
collector.on('collect') (and in the collector 'end' handler), replace uses of
embed.setDescription(...) with a new EmbedBuilder constructed from the original
embed's data (e.g. new EmbedBuilder({ ...embed.toJSON(), description: '...' })
or equivalent) and pass that new embed into i.update and the final update so
each update uses its own EmbedBuilder instance.
- Around line 195-211: Move the await interaction.deferReply({ ephemeral: true
}) call inside the try block of the handleDisable function so any error from
deferReply is caught; specifically, open the try at the start of handleDisable,
call await interaction.deferReply(...) there, then proceed to the keys loop that
calls setConfigValue for moderation.logging.channels.* and the info/editReply
calls, leaving the existing catch which calls logError('Modlog disable failed',
...) and edits the reply — this ensures deferReply exceptions are handled by the
same catch.
In `@src/commands/purge.js`:
- Around line 110-175: The catch block currently treats any post-delete error
(from createCase or sendModLogEmbed) as if bulkDelete failed; refactor by moving
the audit-trail logic (createCase and sendModLogEmbed) into its own try/catch
after the bulkDelete call so that interaction.editReply is always sent with the
actual deletion result (use deleted.size and fetched.size), and only audit
failures are caught/logged (use logError with a message like "Purge audit-trail
failed" and include err.message and filterDetail) while still returning the
successful deletion message to the user; keep bulkDelete and its success logging
(info('Purge executed', ...)) as-is and ensure the outer catch only handles
failures from the delete operation itself.
In `@src/commands/slowmode.js`:
- Around line 94-97: The catch block in src/commands/slowmode.js logs the error
and calls interaction.editReply but doesn't guard against a rejected promise if
the interaction expired; update the catch handler (where logError('Slowmode
command failed', { error: err.message, command: 'slowmode' }) is called) to
append .catch(() => {}) to the interaction.editReply(...) call so it matches the
pattern used in tempban.js and lock.js and prevents an unhandled rejection.
- Around line 34-40: Move the pre-try operations in execute (the call to
interaction.deferReply and the option resolution:
interaction.options.getChannel('channel'), getString('duration'),
getString('reason')) into the existing try block so they are caught by the catch
handler; specifically, inside the execute function relocate the deferReply and
the channel/duration/reason assignments into the try scope (matching the pattern
used in tempban.js and lock.js) to ensure any errors thrown during deferReply or
option parsing are handled by the catch.
In `@src/commands/tempban.js`:
- Around line 66-75: Add the missing bot-self role hierarchy check inside the if
(member) block in tempban.js: after the existing user
checkHierarchy(interaction.member, member) call and before
shouldSendDm/sendDmNotification, call checkHierarchy(interaction.guild.me,
member) (or the same helper used in ban.js to validate the bot's highest role)
and if it returns an error immediately return await interaction.editReply(...)
with the same "my role is not high enough" style message used in ban.js so the
bot fails fast with a clear message instead of a generic Discord API error.
- Around line 72-74: Change the DM toggle check to use the tempban key so
tempbans can be controlled independently: update the conditional that calls
shouldSendDm to use 'tempban' (the same action passed into sendDmNotification)
instead of 'ban'—look for the shouldSendDm(config, 'ban') check in the tempban
command and make it shouldSendDm(config, 'tempban') so it matches
sendDmNotification(member, 'tempban', ...).
In `@src/commands/unban.js`:
- Around line 56-59: The success reply is interpolating the raw snowflake userId
instead of the resolved human-readable targetTag; update the
interaction.editReply call in src/commands/unban.js to use targetTag (the
variable resolved earlier in lines 37–43) so the message reads `✅
**${targetTag}** has been unbanned. (Case #${caseData.case_number})`; also
consider updating the info log call (info('User unbanned', { target: userId, ...
})) to include targetTag for consistent logging.
- Around line 60-68: In the catch block that currently logs via logError and
attempts to notify the user, avoid awaiting interaction.editReply and
interaction.reply because if they reject you get an unhandled rejection; instead
call interaction.editReply(content).catch(() => {}) and interaction.reply({
content, ephemeral: true }).catch(() => {}) (i.e., remove the await and append
.catch(() => {})) so any failure to send the error response is swallowed,
matching the pattern used elsewhere (see interaction.editReply /
interaction.reply in this catch block and logError usage).
In `@src/db.js`:
- Around line 157-160: Change the full index on mod_scheduled_actions to a
partial index that only includes pending rows to keep it small: modify the
CREATE INDEX for idx_mod_scheduled_actions_pending to index execute_at (and/or
executed if needed) but add a WHERE executed = FALSE predicate so it only
contains pending actions queried by WHERE executed = FALSE AND execute_at <=
NOW(); update the SQL statement that creates idx_mod_scheduled_actions_pending
on table mod_scheduled_actions (columns executed, execute_at) to use the partial
index predicate.
In `@src/modules/moderation.js`:
- Around line 290-296: The current loop over thresholds issues one DB query per
threshold (see thresholds, pool.query, guildId, targetId, threshold.withinDays);
replace it with a single batched SQL query that computes counts for all
thresholds at once using COUNT(*) FILTER (WHERE created_at > NOW() - INTERVAL '1
day' * <n>) for each threshold or SUM(CASE WHEN created_at > ... THEN 1 ELSE 0
END) AS count_n, then map the returned columns back to the thresholds array;
update the code that reads { rows } to use the single-row result with the
per-threshold columns instead of running queries inside the for loop.
- Around line 448-453: checkHierarchy currently only compares moderator vs
target and can still allow attempts that fail because the bot's role is too low;
update the function checkHierarchy to also accept or derive the bot's
GuildMember (e.g., add a botMember parameter or fetch guild.me inside the
function) and then check botMember.roles.highest.position >
target.roles.highest.position; if the bot's role is equal or lower return a
clear error string like '❌ I cannot moderate a member with an equal or higher
role than mine.' Ensure you keep the existing moderator-vs-target check
(moderator.roles.highest.position) and return null only when both moderator and
bot have sufficient hierarchy to act.
In `@tests/commands/kick.test.js`:
- Around line 25-107: Add a unit test in kick.test.js that covers the branch
where interaction.options.getMember('user') returns null: update/create a test
(e.g., "should return when target is not in server") that uses the existing
createInteraction pattern but mocks interaction.options.getMember to return
null, then calls execute(interaction) and asserts interaction.editReply was
called with a message containing '❌ User is not in this server.'; reference the
getMember call and execute function to locate the code path to test.
In `@tests/commands/slowmode.test.js`:
- Around line 134-136: Replace the persistent mock on
interaction.channel.setRateLimitPerUser (currently using
vi.fn().mockRejectedValue(new Error('Missing permissions'))) with a one-time
rejection using mockRejectedValueOnce to improve test isolation; locate the mock
in the slowmode test where interaction.channel.setRateLimitPerUser is defined
and change it to vi.fn().mockRejectedValueOnce(new Error('Missing permissions'))
so it only affects this test and matches the pattern used in tempban/ban tests.
In `@tests/commands/tempban.test.js`:
- Around line 133-142: The test exposes an internal error message leak by
expecting the specific error text from createCase; change the tempban error
handling to match warn.js/case.js by removing err.message interpolation in the
catch path (the error handler in tempban.js that constructs `❌ Failed to
execute: ${err.message}`) and return a generic failure string instead, then
update this test (and other tests referencing leaky messages) to assert the
generic message (e.g., expect.stringContaining('Failed to execute')) rather than
the error details; also apply the same change to the other listed command files
(unban.js, untimeout.js, timeout.js, softban.js, kick.js, ban.js) so all
moderation commands use the secure generic error message.
In `@tests/commands/timeout.test.js`:
- Around line 70-89: Add a unit test that covers the early-return branch when
getMember() returns null: in the timeout command test, stub the
interaction/createInteraction so that the command's getMember() resolves to
null, call execute(interaction), then assert that interaction.deferReply was
called with { ephemeral: true }, interaction.editReply was called (indicating
the early return response), and that neither mockMember.timeout nor createCase
were invoked; use the same test helpers (execute, createInteraction, mockMember,
createCase) used elsewhere in the file to locate the code paths.
- Around line 36-41: The shared mockMember object declared at describe scope can
leak state between tests; move the mockMember definition into the
createInteraction factory (the same way kick/softban tests do) so each call to
createInteraction() returns a fresh mockMember, and update all tests in
timeout.test.js to destructure { interaction, mockMember } from
createInteraction(); keep existing vi.clearAllMocks() but remove reliance on a
shared mockMember to ensure test isolation.
In `@tests/commands/untimeout.test.js`:
- Around line 66-83: Add an assertion in the success test to verify that
sendModLogEmbed was called when execute removes a timeout: after calling await
execute(interaction) and the existing expectations, assert that sendModLogEmbed
(the mocked function) was invoked (e.g., calledOnce or calledWith matching the
guild id and a payload containing action: 'untimeout' and targetId: 'user1') so
the test covers the mod-log branch of the untimeout flow.
In `@tests/commands/warn.test.js`:
- Around line 32-119: Add a unit test for the branch where the target user is
not in the guild by mocking interaction.options.getMember to return null for
that test; call execute(interaction) and assert interaction.editReply was called
with '❌ User is not in this server.' and that
createCase/sendDmNotification/sendModLogEmbed were not called. You can reuse the
existing createInteraction() helper and override interaction.options.getMember
to return null inside the new "should reply when target not in guild" test;
reference the execute function and interaction.options.getMember to locate the
change.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (35)
AGENTS.mdREADME.mdsrc/commands/ban.jssrc/commands/case.jssrc/commands/history.jssrc/commands/lock.jssrc/commands/modlog.jssrc/commands/purge.jssrc/commands/slowmode.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/timeout.jssrc/commands/unban.jssrc/commands/unlock.jssrc/commands/warn.jssrc/db.jssrc/index.jssrc/modules/moderation.jssrc/utils/duration.jstests/commands/ban.test.jstests/commands/kick.test.jstests/commands/lock.test.jstests/commands/modlog.test.jstests/commands/purge.test.jstests/commands/slowmode.test.jstests/commands/softban.test.jstests/commands/tempban.test.jstests/commands/timeout.test.jstests/commands/unban.test.jstests/commands/unlock.test.jstests/commands/untimeout.test.jstests/commands/warn.test.jstests/db.test.jstests/index.test.jstests/modules/moderation.test.js
🧰 Additional context used
📓 Path-based instructions (4)
tests/**/*.{js,test.js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for testing with
pnpm test; test coverage provider is@vitest/coverage-v8
Files:
tests/commands/lock.test.jstests/commands/ban.test.jstests/commands/kick.test.jstests/db.test.jstests/commands/unban.test.jstests/commands/timeout.test.jstests/commands/purge.test.jstests/commands/untimeout.test.jstests/commands/unlock.test.jstests/commands/softban.test.jstests/commands/slowmode.test.jstests/commands/modlog.test.jstests/commands/tempban.test.jstests/index.test.jstests/commands/warn.test.jstests/modules/moderation.test.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Use ESM syntax withimport/export; never userequire()
Always usenode:protocol when importing Node.js builtins (e.g.,import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Always use Winston logging viaimport { info, warn, error } from '../logger.js'— NEVER useconsole.log(),console.warn(),console.error(), or anyconsole.*method in src/ files
Pass structured metadata when logging:info('Message processed', { userId, channelId })
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig()fromsrc/modules/config.jsto read config; usesetConfigValue(key, value)to update at runtime
UsesplitMessage()utility for messages exceeding Discord's 2000-character limit
No TypeScript — use plain JavaScript with JSDoc comments for documentation
Files:
src/utils/duration.jssrc/commands/tempban.jssrc/commands/warn.jssrc/commands/unban.jssrc/commands/ban.jssrc/commands/softban.jssrc/commands/modlog.jssrc/commands/slowmode.jssrc/commands/lock.jssrc/commands/case.jssrc/commands/unlock.jssrc/commands/purge.jssrc/index.jssrc/commands/history.jssrc/commands/timeout.jssrc/modules/moderation.jssrc/db.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Slash commands must export
data(SlashCommandBuilder) andexecute(interaction)function
Files:
src/commands/tempban.jssrc/commands/warn.jssrc/commands/unban.jssrc/commands/ban.jssrc/commands/softban.jssrc/commands/modlog.jssrc/commands/slowmode.jssrc/commands/lock.jssrc/commands/case.jssrc/commands/unlock.jssrc/commands/purge.jssrc/commands/history.jssrc/commands/timeout.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Check
config.yourModule.enabledbefore processing in module handlers
Files:
src/modules/moderation.js
🧠 Learnings (11)
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/**/*.{ts,tsx} : Use `date-fns` for date manipulation and formatting throughout the application
Applied to files:
src/utils/duration.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/commands/*.js : Slash commands must export `data` (SlashCommandBuilder) and `execute(interaction)` function
Applied to files:
tests/commands/ban.test.jssrc/commands/tempban.jssrc/commands/warn.jstests/commands/kick.test.jssrc/commands/unban.jssrc/commands/ban.jstests/commands/unban.test.jssrc/commands/softban.jssrc/commands/modlog.jstests/commands/timeout.test.jssrc/commands/slowmode.jssrc/commands/lock.jssrc/commands/case.jstests/commands/purge.test.jssrc/commands/unlock.jssrc/commands/purge.jstests/commands/unlock.test.jstests/commands/softban.test.jstests/commands/slowmode.test.jstests/commands/tempban.test.jssrc/commands/history.jssrc/commands/timeout.jstests/commands/warn.test.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Commands are auto-discovered from `src/commands/` on startup; after adding a command, run `pnpm run deploy` to register with Discord
Applied to files:
src/commands/unban.jsAGENTS.mdsrc/commands/purge.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Always use Winston logging via `import { info, warn, error } from '../logger.js'` — NEVER use `console.log()`, `console.warn()`, `console.error()`, or any `console.*` method in src/ files
Applied to files:
src/commands/modlog.js
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to src/**/*.{ts,tsx} : Use `reportError(context, error)` from `src/lib/logger.ts` to report errors to Sentry with context metadata, falling back to console.error if Sentry is disabled
Applied to files:
src/commands/modlog.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Graceful shutdown is handled in `src/index.js`; use error classes and logging before throwing errors in other modules
Applied to files:
src/commands/modlog.jssrc/index.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Update Key Files table in AGENTS.md when adding a new command or module
Applied to files:
AGENTS.md
📚 Learning: 2026-02-11T17:18:14.598Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-11T17:18:14.598Z
Learning: See AGENTS.md for full project context, architecture, and coding guidelines
Applied to files:
AGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Module handlers must be registered in `src/modules/events.js` to wire them to Discord events
Applied to files:
AGENTS.mdsrc/index.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use `splitMessage()` utility for messages exceeding Discord's 2000-character limit
Applied to files:
AGENTS.mdsrc/commands/purge.jssrc/commands/timeout.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to tests/**/*.{js,test.js} : Use Vitest for testing with `pnpm test`; test coverage provider is `vitest/coverage-v8`
Applied to files:
tests/commands/purge.test.js
🧬 Code graph analysis (24)
tests/commands/lock.test.js (1)
src/modules/moderation.js (2)
createCase(87-155)sendModLogEmbed(209-262)
src/utils/duration.js (3)
src/commands/slowmode.js (1)
ms(46-46)src/modules/moderation.js (1)
ms(309-309)src/commands/purge.js (1)
count(107-107)
tests/commands/ban.test.js (2)
src/commands/ban.js (4)
data(17-31)data(17-31)adminOnly(33-33)adminOnly(33-33)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(448-453)
tests/commands/kick.test.js (2)
src/commands/kick.js (1)
execute(31-77)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(448-453)
src/commands/unban.js (3)
src/modules/moderation.js (4)
config(376-376)reason(301-301)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
tests/commands/unban.test.js (2)
src/commands/unban.js (1)
execute(27-69)src/modules/moderation.js (1)
createCase(87-155)
src/commands/softban.js (3)
src/index.js (1)
config(52-52)src/modules/moderation.js (7)
config(376-376)reason(301-301)checkHierarchy(448-453)shouldSendDm(461-463)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/logger.js (1)
info(216-218)
tests/commands/timeout.test.js (3)
src/commands/timeout.js (1)
execute(35-98)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(448-453)src/utils/duration.js (1)
parseDuration(23-36)
src/commands/lock.js (3)
src/modules/moderation.js (5)
channel(217-217)reason(301-301)config(376-376)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/commands/case.js (3)
src/modules/moderation.js (13)
ACTION_COLORS(17-30)ACTION_COLORS(17-30)pool(88-88)pool(167-167)pool(168-174)pool(244-244)pool(288-288)pool(291-296)pool(351-351)pool(352-357)config(376-376)ACTION_LOG_CHANNEL_KEY(51-64)ACTION_LOG_CHANNEL_KEY(51-64)src/db.js (2)
pool(12-12)getPool(181-186)src/logger.js (1)
info(216-218)
tests/commands/purge.test.js (2)
src/commands/purge.js (6)
filtered(115-115)data(11-95)data(11-95)subcommand(106-106)deleted(144-144)execute(103-176)src/modules/moderation.js (2)
createCase(87-155)sendModLogEmbed(209-262)
tests/commands/untimeout.test.js (2)
src/commands/untimeout.js (1)
execute(25-67)src/modules/moderation.js (2)
createCase(87-155)checkHierarchy(448-453)
src/commands/unlock.js (4)
src/commands/lock.js (6)
data(11-23)data(11-23)channel(35-35)reason(36-36)config(54-54)caseData(55-62)src/commands/unban.js (3)
reason(33-33)config(31-31)caseData(45-52)src/modules/moderation.js (5)
channel(217-217)reason(301-301)config(376-376)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)
src/commands/purge.js (3)
src/modules/moderation.js (4)
channel(217-217)config(376-376)createCase(87-155)sendModLogEmbed(209-262)src/logger.js (1)
info(216-218)src/modules/config.js (2)
getConfig(130-132)err(30-30)
src/index.js (1)
src/modules/moderation.js (4)
stopTempbanScheduler(434-440)startTempbanScheduler(414-429)client(89-89)client(97-135)
tests/commands/unlock.test.js (2)
src/commands/unlock.js (1)
execute(31-78)src/modules/moderation.js (2)
createCase(87-155)sendModLogEmbed(209-262)
tests/commands/slowmode.test.js (4)
src/commands/slowmode.js (6)
ms(46-46)channel(36-36)data(12-26)data(12-26)adminOnly(28-28)adminOnly(28-28)src/modules/moderation.js (3)
ms(309-309)channel(217-217)createCase(87-155)src/utils/duration.js (2)
ms(33-33)parseDuration(23-36)src/commands/modlog.js (4)
data(19-24)data(19-24)adminOnly(26-26)adminOnly(26-26)
tests/commands/modlog.test.js (2)
src/commands/modlog.js (6)
subcommand(33-33)data(19-24)data(19-24)adminOnly(26-26)adminOnly(26-26)execute(32-51)src/modules/config.js (2)
getConfig(130-132)setConfigValue(141-225)
tests/commands/tempban.test.js (4)
src/index.js (1)
interaction(178-178)src/commands/tempban.js (1)
execute(44-116)src/modules/moderation.js (4)
sendDmNotification(187-200)createCase(87-155)scheduleAction(166-177)checkHierarchy(448-453)src/utils/duration.js (1)
parseDuration(23-36)
src/commands/history.js (3)
src/commands/unban.js (2)
execute(27-69)reason(33-33)src/db.js (1)
getPool(181-186)src/logger.js (1)
info(216-218)
src/modules/moderation.js (4)
src/db.js (2)
pool(12-12)getPool(181-186)src/logger.js (1)
info(216-218)src/modules/config.js (1)
getConfig(130-132)src/utils/duration.js (2)
ms(33-33)parseDuration(23-36)
tests/commands/warn.test.js (2)
src/commands/warn.js (1)
execute(32-85)src/modules/moderation.js (5)
sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)checkEscalation(275-337)checkHierarchy(448-453)
src/db.js (1)
src/modules/moderation.js (8)
pool(88-88)pool(167-167)pool(168-174)pool(244-244)pool(288-288)pool(291-296)pool(351-351)pool(352-357)
tests/modules/moderation.test.js (1)
src/modules/moderation.js (14)
createCase(87-155)scheduleAction(166-177)member(306-306)sendDmNotification(187-200)embed(189-193)embed(220-233)client(89-89)client(97-135)config(376-376)sendModLogEmbed(209-262)checkEscalation(275-337)checkHierarchy(448-453)shouldSendDm(461-463)startTempbanScheduler(414-429)
🪛 markdownlint-cli2 (0.20.0)
AGENTS.md
[warning] 92-92: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/2/3
(MD029, ol-prefix)
[warning] 93-93: Ordered list item prefix
Expected: 2; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 94-94: Ordered list item prefix
Expected: 3; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 95-95: Ordered list item prefix
Expected: 4; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
🔇 Additional comments (40)
src/utils/duration.js (1)
1-71: Clean utility with correct round-trip semantics.The parsing validation chain (type check → regex → positive safe-integer → finite result) and the format loop are both sound. The detailed JSDoc contract on
formatDurationaddresses the non-exact-multiple edge case well.tests/db.test.js (1)
109-116: LGTM — moderation schema assertions align withsrc/db.jschanges.The new checks correctly verify that both moderation tables and their indexes are created during initialization.
src/index.js (1)
339-342: LGTM — tempban scheduler correctly guarded by DB availability.The scheduler starts only when
dbPoolis present, and the shutdown path callsstopTempbanScheduler()unconditionally which is safe due to the null-check inside the function.src/db.js (1)
120-138: Schema formod_casesis well-structured.The advisory-lock-based case numbering (in
createCase) pairs correctly with theUNIQUE(guild_id, case_number)constraint, and the composite index on(guild_id, target_id, created_at)supports the escalation count query inmoderation.js.tests/commands/modlog.test.js (1)
1-234: Thorough test coverage for the modlog command's interactive setup flow.The collector-handler capture pattern via
_collectHandlersis well-designed, and the tests cover all key paths: subcommand dispatch, view/disable behavior, interactive category→channel selection, edge cases (no prior category, timeout vs. user-initiated end).tests/commands/purge.test.js (1)
56-67: NicemockCollectionhelper with recursivefiltersupport.The self-attaching
.filteron filtered results correctly emulates Discord.js Collection's chainable filter behavior. Clean approach for testing without pulling in the full Collection class.src/commands/warn.js (1)
1-85: Clean implementation that follows all project conventions.Exports
data,adminOnly, andexecutecorrectly. Uses ESM imports, Winston logging with structured metadata,getConfig(), and proper error handling with a generic user-facing message. The previous review concern about leakingerr.messagehas been addressed at Line 78.tests/commands/tempban.test.js (1)
84-107: Good coverage of the successful tempban flow.The test validates the full lifecycle: defer → DM → ban → case creation → schedule → mod log → reply. Assertions on
scheduleActionargs (Line 103) andcreateCasewithobjectContaining(Lines 95-102) are well-targeted.README.md (1)
15-15: Documentation is comprehensive and well-organized.The moderation suite features, configuration options, and command reference are clearly documented. The previous concern about stale
moderation.enableddescription has been addressed at Line 149.Also applies to: 149-166, 176-228
tests/commands/unlock.test.js (1)
1-145: Thorough test coverage for the unlock command.Covers all key paths: current channel unlock, specified channel, reason propagation, non-text channel rejection, and permission error handling. The permission overwrite assertion (
{ SendMessages: null }) correctly validates the reset behavior.src/commands/history.js (1)
21-76: PreviousgetPool()placement concern has been addressed; overall implementation is solid.
getPool()is now inside thetryblock (Line 27), the error handler uses a generic message (Line 75), and structured logging is used correctly (Lines 65-70).tests/commands/ban.test.js (1)
74-138: Good test coverage including bot role hierarchy check.The test at Lines 117-127 for bot role validation is a valuable edge case that other command tests don't cover. Lines 95-103 correctly verify that bans proceed even when the target isn't in the guild (hierarchy check skipped).
tests/commands/unban.test.js (2)
85-108: Previous review feedback addressed — unban API failure test added.Line 99-108 now covers the case where
guild.members.unbanrejects, and Lines 85-97 test the fallback to raw user ID when user fetch fails. Both are valuable edge-case tests.
1-120: Well-structured test suite with comprehensive coverage.Covers metadata exports, successful flow with user tag resolution, fallback behavior, and both API-level and DB-level error paths.
tests/commands/slowmode.test.js (1)
69-112: Solid coverage of slowmode behavior including the 6-hour cap.The cap test (Lines 102-112) correctly validates that durations exceeding Discord's 6-hour maximum are clamped to 21600 seconds, and the disable test (Lines 83-91) covers the "0" special case.
tests/commands/softban.test.js (1)
1-166: Well-structured test suite covering critical softban paths.Tests cover the happy path, unban retry/failure, null member, hierarchy check, and DB errors. The
mockMemberis properly scoped insidecreateInteraction, addressing prior feedback. No issues found.tests/commands/lock.test.js (1)
1-146: Good coverage for the lock command — tests look correct.Tests validate the main flow (defer, permission overwrite, embed notification, case creation, mod-log), specified channel targeting, reason propagation, non-text channel rejection, and error handling. No issues found.
src/commands/unlock.js (1)
1-78: Clean implementation, consistent with the lock command counterpart.The unlock command correctly resets
SendMessagestonull(removing the override rather than explicitly allowing), includes proper channel validation, notification embed, case creation, mod-log posting, and robust error handling with thedeferred || repliedguard. No issues.src/commands/softban.js (1)
1-115: Solid implementation addressing all prior review feedback.The
shouldSendDmnow correctly uses'softban'instead of'ban'. The unban retry loop (3 attempts, linear backoff) with clear error propagation is well-implemented. DM notification is correctly sent before the ban to ensure the bot still shares a server with the target. No issues found.src/commands/timeout.js (1)
35-97: Clean timeout implementation with all prior feedback addressed.The 28-day upper-bound validation (lines 52-55) correctly guards against Discord API errors. Duration parsing, hierarchy checks, DM notification ordering, and case creation with
expiresAtare all properly implemented. No issues.src/commands/ban.js (1)
1-112: Well-structured ban command with thorough hierarchy checks, bot-self check, and robust error handling.The overall flow (defer → fetch member → hierarchy check → bot check → DM → ban → case → log → reply) is correct and follows the documented moderation command pattern. The nested try-catch in the error handler (lines 102–110) properly guards against interaction expiry. The bot-self hierarchy check (lines 61–66) and permission-based error messaging (lines 96–100) are good additions.
src/commands/tempban.js (1)
44-116: Good structure overall —deferReplyinsidetry, error handler with.catch(() => {}), andscheduleActionproperly extracted.The previous review feedback has been well addressed:
deferReplyis inside thetryblock, the scheduled action insert has been extracted to ascheduleActionhelper in the moderation module, and the error handler gracefully handles both deferred and non-deferred states.AGENTS.md (1)
97-109: Moderation Command Pattern now correctly reflects the DM-before-action order — LGTM.The step ordering (validate → DM → execute action → create case → log → escalation) matches the actual implementation in command files and is consistent with Pitfall
#7.tests/commands/warn.test.js (1)
69-96: Good coverage of the successful warn flow with checkEscalation argument verification.The test now validates the full flow including DM, case creation, mod log, and escalation check with argument assertions (client, guildId, targetId, botId, botTag, config). This addresses the previous review feedback well.
tests/index.test.js (2)
165-173:settleStartupHopshelper is a clear improvement over inline microtask settling.The extracted helper with a descriptive comment documenting the hop count and when to update it is exactly what the previous review requested. Good maintainability improvement.
286-304: Moderation scheduler startup/skip assertions look correct.Both the positive case (line 292: scheduler started with DATABASE_URL) and negative case (line 303: scheduler not started without DATABASE_URL) are properly verified. Good integration testing.
src/commands/lock.js (1)
1-78: All prior feedback addressed — clean implementation.Channel type restriction via
addChannelTypes(line 18),deferReplyinsidetry(line 32–33), structuredcommandkey in error log (line 68), and the defensive runtime type check for the fallback channel (line 38) are all correct. The flow (lock → notify in channel → create case → mod log → reply) is consistent with the documented moderation pattern.src/commands/modlog.js (1)
32-51: Good: default case added, error imports present, all prior feedback addressed.The
executedispatcher,handleView/handleDisableerror handling, andnull(not'null'string) for disable are all correctly implemented.src/commands/slowmode.js (1)
40-93: Capping logic, case creation, and mod-log integration properly address prior feedback.The silent-cap issue is resolved with the
cappedflag and explicit user notification (line 87–89). ThecreateCase+sendModLogEmbedintegration (lines 64–78) brings slowmode in line with other moderation commands for audit trail consistency.src/commands/purge.js (1)
11-95: LGTM — command builder is well-structured.Subcommand definitions are clean, option descriptions clarify the scan-vs-delete distinction for filtered subcommands, and min/max values are properly set.
tests/modules/moderation.test.js (7)
1-67: Well-structured test setup.Mock ordering in
beforeEach(clear → setup) is correct. TheafterEachproperly stops the scheduler and restores timers/mocks to avoid inter-test leakage.
69-147: Good transactional coverage forcreateCaseandscheduleAction.Tests verify the full BEGIN → advisory lock → INSERT → COMMIT sequence and the ROLLBACK + release path on failure.
149-175:sendDmNotificationtests cover the key paths well.Happy path, fallback reason, and silent DM failure are all exercised.
177-344: ThoroughsendModLogEmbedcoverage.Seven test cases cover action-specific routing, default fallback, duration field, log_message_id persistence failure, missing config, null channels, fetch failure, and send failure — good edge-case discipline.
346-512:checkEscalationtests verify both timeout and ban paths.The disabled/no-thresholds/below-threshold/timeout/ban scenarios are well covered. Mock query chains are correctly sequenced against the createCase transaction flow.
514-549: LGTM —checkHierarchyandshouldSendDmtests.
551-708: Good scheduler test coverage including the unban-failure path.The four tempban scheduler tests cover idempotent start/stop, successful poll processing, concurrent-claim skip, and the failure-still-marks-executed resilience behavior.
src/modules/moderation.js (3)
87-155: Solid transactional case creation with advisory locking.The per-guild advisory lock serializes concurrent case-number generation, the atomic subquery prevents gaps/collisions, and the ROLLBACK +
client.release()infinallyensures proper cleanup. This addresses the earlier race-condition concern well.
209-262: Good layered error handling insendModLogEmbed.The nested try/catch for
log_message_idstorage means a DB write failure doesn't prevent the success path from returning the sent message, and failures are logged with structured context.
343-406:pollTempbansis well-hardened.The
schedulerPollInFlightguard (safe under Node's single-threaded model),LIMIT 50withORDER BY, optimistic claim viaUPDATE ... WHERE executed = FALSE RETURNING id, per-row error isolation, and user-tag resolution with fallback all address prior review feedback thoroughly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@src/commands/history.js`:
- Around line 72-75: The catch block in src/commands/history.js logs the error
with logError('Command error', ...) but calls interaction.editReply('❌ Failed to
fetch moderation history.') without a follow-up rejection handler; update the
error-path reply in the catch of the history command to append .catch(() => {})
to interaction.editReply to match other commands (warn, unban, untimeout,
softban) and avoid unhandled promise rejections.
In `@src/commands/slowmode.js`:
- Around line 63-76: The current flow calls channel.setRateLimitPerUser(...)
before createCase(...), so if createCase fails the slowmode is applied without
an audit record; either move createCase(...) to run before
channel.setRateLimitPerUser(...) (so the DB record exists prior to mutating the
guild), or keep the order but wrap createCase(...) in a try/catch and, on
failure, log the error and update the moderator reply to explicitly state that
the rate limit was successfully set but the case creation failed (include error
context). Locate and modify the block using setRateLimitPerUser, createCase,
interaction.user, and formatDuration to implement one of these two fixes.
In `@src/modules/moderation.js`:
- Around line 304-333: The code creates an escalation case even when the
enforcement action was skipped (e.g., threshold.action === 'timeout' but member
is null or parseDuration(threshold.duration) is falsy); update the try block in
the escalation flow to detect when no action was taken and abort before calling
createCase/sendModLogEmbed. Concretely, after resolving guild and member and
computing ms with parseDuration, add a guard that if threshold.action ===
'timeout' and (member is null or !ms) then log a descriptive message (use
logError or a debug logger) and return null; similarly ensure the 'ban' branch
only proceeds if guild.members.ban was actually invoked (or if an earlier check
fails, return null) so createCase and sendModLogEmbed are only called when an
enforcement action completed.
In `@tests/commands/ban.test.js`:
- Around line 30-35: The shared plain objects mockUser and mockMember at
describe scope can leak state across tests because vi.clearAllMocks() won't
reset them; change them to be created fresh for each test by replacing the
top-level consts with a factory or by instantiating new objects in beforeEach
(e.g., a createMockMember/createMockUser function or reassign mockMember = {...}
inside beforeEach) so each test gets an independent mockMember and mockUser
instance; update references in tests to call the factory or use the
beforeEach-provided variables (look for mockUser and mockMember in this test
file).
In `@tests/commands/lock.test.js`:
- Around line 1-146: The tests for lock and unlock duplicate the
createInteraction factory and mock setup; extract the shared scaffolding into a
reusable helper (e.g., createChannelCommandInteraction) and import it into both
tests to eliminate duplication. Replace the local createInteraction in
tests/commands/lock.test.js (and in unlock.test.js) with calls to the new
helper, keep the same override signature so existing tests still pass, and move
common vi.mock setups (modules/moderation.js, modules/config.js, logger.js) into
a shared test-utils file if appropriate; update imports in both test files to
reference the new helper and shared mocks.
In `@tests/commands/purge.test.js`:
- Around line 128-211: The filter tests (those using buildInteraction + execute
for the "user", "bot", "contains", "links", and "attachments" subcommands) only
assert the filtered collection passed to interaction.channel.bulkDelete but
don't simulate a non-empty deletion result or assert the reply; update each of
those tests to pass a matching deletedResult into buildInteraction (use a
collection containing the filtered messages from the messages input) and add an
assertion that interaction.editReply was called with a string containing the
expected deleted count; look for the tests calling buildInteraction, execute,
mockCollection, mockMessage, and interaction.channel.bulkDelete/editReply to
apply the change.
In `@tests/commands/tempban.test.js`:
- Around line 12-19: Update the test's config mock so dmNotifications includes a
tempban key set to true to match expected config shape; specifically modify the
vi.mock for getConfig used in tests/commands/tempban.test.js so the returned
moderation.dmNotifications object contains tempban: true (alongside warn, kick,
timeout, ban) to avoid a misleading omission that could break future tests if
shouldSendDm mocking is removed.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (27)
src/commands/ban.jssrc/commands/history.jssrc/commands/kick.jssrc/commands/lock.jssrc/commands/purge.jssrc/commands/slowmode.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/timeout.jssrc/commands/unban.jssrc/commands/unlock.jssrc/commands/untimeout.jssrc/commands/warn.jssrc/modules/moderation.jstests/commands/ban.test.jstests/commands/kick.test.jstests/commands/lock.test.jstests/commands/purge.test.jstests/commands/slowmode.test.jstests/commands/softban.test.jstests/commands/tempban.test.jstests/commands/timeout.test.jstests/commands/unban.test.jstests/commands/unlock.test.jstests/commands/untimeout.test.jstests/commands/warn.test.jstests/modules/moderation.test.js
🧰 Additional context used
📓 Path-based instructions (4)
tests/**/*.{js,test.js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for testing with
pnpm test; test coverage provider is@vitest/coverage-v8
Files:
tests/commands/kick.test.jstests/commands/warn.test.jstests/commands/untimeout.test.jstests/commands/purge.test.jstests/commands/timeout.test.jstests/commands/lock.test.jstests/commands/ban.test.jstests/commands/unban.test.jstests/commands/softban.test.jstests/modules/moderation.test.jstests/commands/unlock.test.jstests/commands/slowmode.test.jstests/commands/tempban.test.js
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Use ESM syntax withimport/export; never userequire()
Always usenode:protocol when importing Node.js builtins (e.g.,import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Always use Winston logging viaimport { info, warn, error } from '../logger.js'— NEVER useconsole.log(),console.warn(),console.error(), or anyconsole.*method in src/ files
Pass structured metadata when logging:info('Message processed', { userId, channelId })
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig()fromsrc/modules/config.jsto read config; usesetConfigValue(key, value)to update at runtime
UsesplitMessage()utility for messages exceeding Discord's 2000-character limit
No TypeScript — use plain JavaScript with JSDoc comments for documentation
Files:
src/commands/warn.jssrc/commands/softban.jssrc/commands/history.jssrc/commands/unban.jssrc/commands/kick.jssrc/commands/ban.jssrc/commands/untimeout.jssrc/commands/unlock.jssrc/commands/purge.jssrc/commands/timeout.jssrc/commands/slowmode.jssrc/commands/lock.jssrc/commands/tempban.jssrc/modules/moderation.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Slash commands must export
data(SlashCommandBuilder) andexecute(interaction)function
Files:
src/commands/warn.jssrc/commands/softban.jssrc/commands/history.jssrc/commands/unban.jssrc/commands/kick.jssrc/commands/ban.jssrc/commands/untimeout.jssrc/commands/unlock.jssrc/commands/purge.jssrc/commands/timeout.jssrc/commands/slowmode.jssrc/commands/lock.jssrc/commands/tempban.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Check
config.yourModule.enabledbefore processing in module handlers
Files:
src/modules/moderation.js
🧠 Learnings (3)
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/commands/*.js : Slash commands must export `data` (SlashCommandBuilder) and `execute(interaction)` function
Applied to files:
tests/commands/kick.test.jstests/commands/warn.test.jssrc/commands/warn.jssrc/commands/softban.jstests/commands/purge.test.jssrc/commands/history.jssrc/commands/unban.jssrc/commands/kick.jstests/commands/timeout.test.jstests/commands/lock.test.jstests/commands/ban.test.jssrc/commands/ban.jssrc/commands/untimeout.jssrc/commands/unlock.jstests/commands/unban.test.jssrc/commands/purge.jssrc/commands/timeout.jstests/commands/softban.test.jssrc/commands/slowmode.jssrc/commands/lock.jssrc/commands/tempban.jstests/commands/unlock.test.jstests/commands/slowmode.test.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to tests/**/*.{js,test.js} : Use Vitest for testing with `pnpm test`; test coverage provider is `vitest/coverage-v8`
Applied to files:
tests/commands/warn.test.jstests/commands/purge.test.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use `splitMessage()` utility for messages exceeding Discord's 2000-character limit
Applied to files:
src/commands/timeout.js
🧬 Code graph analysis (20)
tests/commands/kick.test.js (2)
src/commands/kick.js (5)
data(17-23)data(17-23)adminOnly(25-25)adminOnly(25-25)execute(31-74)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(449-457)
tests/commands/warn.test.js (1)
src/modules/moderation.js (5)
sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)checkEscalation(275-337)checkHierarchy(449-457)
src/commands/warn.js (3)
src/index.js (2)
interaction(178-178)config(52-52)src/modules/moderation.js (8)
config(376-376)reason(301-301)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)checkEscalation(275-337)src/logger.js (1)
info(216-218)
tests/commands/purge.test.js (1)
src/commands/purge.js (8)
filtered(114-114)data(11-95)data(11-95)adminOnly(97-97)adminOnly(97-97)subcommand(107-107)deleted(143-143)execute(103-175)
src/commands/history.js (3)
src/commands/modlog.js (4)
lines(177-179)row(75-75)embed(80-84)embed(172-175)src/db.js (1)
getPool(181-186)src/logger.js (1)
info(216-218)
src/commands/kick.js (3)
src/modules/moderation.js (7)
config(376-376)reason(301-301)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
tests/commands/timeout.test.js (3)
src/commands/timeout.js (1)
execute(35-95)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(449-457)src/utils/duration.js (1)
parseDuration(23-36)
tests/commands/lock.test.js (1)
src/modules/moderation.js (2)
createCase(87-155)sendModLogEmbed(209-262)
tests/commands/ban.test.js (2)
src/commands/ban.js (3)
data(17-31)data(17-31)execute(39-96)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(449-457)
src/commands/ban.js (2)
src/modules/moderation.js (8)
config(376-376)reason(301-301)member(306-306)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/logger.js (1)
info(216-218)
tests/commands/unban.test.js (2)
src/commands/unban.js (1)
execute(27-65)src/modules/moderation.js (1)
createCase(87-155)
src/commands/purge.js (2)
src/modules/moderation.js (4)
channel(217-217)config(376-376)createCase(87-155)sendModLogEmbed(209-262)src/logger.js (1)
info(216-218)
src/commands/timeout.js (4)
src/modules/moderation.js (7)
config(376-376)reason(301-301)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/utils/duration.js (2)
parseDuration(23-36)formatDuration(60-71)src/logger.js (1)
info(216-218)
src/commands/slowmode.js (4)
src/modules/moderation.js (6)
channel(217-217)reason(301-301)ms(309-309)config(376-376)createCase(87-155)sendModLogEmbed(209-262)src/utils/duration.js (3)
ms(33-33)parseDuration(23-36)formatDuration(60-71)src/modules/config.js (1)
getConfig(130-132)src/logger.js (1)
info(216-218)
src/commands/lock.js (4)
src/commands/unlock.js (6)
execute(31-73)channel(35-35)reason(36-36)notifyEmbed(46-51)config(54-54)caseData(55-62)src/modules/moderation.js (5)
channel(217-217)reason(301-301)config(376-376)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/commands/tempban.js (4)
src/index.js (2)
interaction(178-178)config(52-52)src/modules/moderation.js (7)
config(376-376)reason(301-301)member(306-306)shouldSendDm(465-467)createCase(87-155)scheduleAction(166-177)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/utils/duration.js (2)
parseDuration(23-36)formatDuration(60-71)
tests/modules/moderation.test.js (1)
src/modules/moderation.js (15)
stopTempbanScheduler(434-440)createCase(87-155)scheduleAction(166-177)member(306-306)sendDmNotification(187-200)embed(189-193)embed(220-233)client(89-89)client(97-135)config(376-376)sendModLogEmbed(209-262)checkEscalation(275-337)checkHierarchy(449-457)shouldSendDm(465-467)startTempbanScheduler(414-429)
tests/commands/unlock.test.js (2)
src/commands/unlock.js (1)
execute(31-73)src/modules/moderation.js (2)
createCase(87-155)sendModLogEmbed(209-262)
src/modules/moderation.js (3)
src/db.js (2)
pool(12-12)getPool(181-186)src/logger.js (1)
info(216-218)src/modules/config.js (1)
getConfig(130-132)
tests/commands/tempban.test.js (3)
src/commands/tempban.js (3)
data(19-36)data(19-36)execute(44-117)src/modules/moderation.js (4)
sendDmNotification(187-200)createCase(87-155)scheduleAction(166-177)checkHierarchy(449-457)src/utils/duration.js (1)
parseDuration(23-36)
🔇 Additional comments (34)
src/modules/moderation.js (3)
1-12: Imports and module setup look good.ESM syntax, Winston logging, no
console.*, structured imports — all aligned with coding guidelines.
87-155: Atomic case creation with advisory lock is well-implemented.The
pg_advisory_xact_lock+ atomicCOALESCE(MAX(...))subquery within a transaction properly prevents race conditions on case number assignment. Rollback and client release incatch/finallyare handled correctly.
343-406: Tempban polling with claim-before-process pattern is solid.The
schedulerPollInFlightguard,LIMIT 50,ORDER BY, and optimisticUPDATE ... AND executed = FALSE RETURNING idclaim pattern address prior review feedback well. Per-row error handling ensures one failure doesn't block others.src/commands/warn.js (1)
32-81: Clean implementation following established patterns.
deferReplyinsidetry/catch, sanitized error responses, hierarchy check with bot member, and proper sequencing of DM → case → log → escalation. All previous review feedback has been addressed.src/commands/untimeout.js (1)
25-63: Well-structured implementation, consistent with other commands.Proper null checks, hierarchy enforcement with bot member,
timeout(null, ...)to clear, and sanitized error handling. No issues found.src/commands/softban.js (1)
39-112: Well-implemented softban with proper retry logic and failure messaging.The 3-attempt unban retry with linear backoff, continued case creation on failure, and differentiated user feedback (success vs. "unban failed" warning) all address prior review feedback comprehensively. Error handling is consistent with other commands.
tests/commands/softban.test.js (1)
1-166: Comprehensive test coverage for softban.Good coverage of the key paths: success, retry, null member, persistent failure, hierarchy, and error handling. The fake timer usage for retry assertions is well done.
src/commands/unlock.js (1)
1-73: LGTM!Clean implementation following the established command pattern.
deferReplyis insidetry/catch, channel type is restricted both at the builder level and with a runtime guard, structured logging includes relevant metadata, and error handling is consistent with the rest of the suite.src/commands/kick.js (1)
1-74: LGTM!Well-structured kick command. DM notification is correctly sent before the kick action (important since the user leaves the guild), hierarchy check includes the bot member, and error handling is consistent.
src/commands/ban.js (1)
1-96: LGTM!All previously flagged issues have been addressed:
deferReplyis insidetry/catch, bot hierarchy check is included viacheckHierarchythird parameter, and error messages are sanitized. The ban-by-ID flow correctly handles users not currently in the guild.src/commands/tempban.js (1)
1-117: All other previously flagged concerns are resolved.
deferReplyis insidetry/catch,scheduleActionis extracted to the moderation module, bot hierarchy check is included viacheckHierarchythird parameter, and error messages are sanitized. The rest of the implementation looks correct.src/commands/timeout.js (1)
1-95: LGTM!The 28-day upper-bound validation (Lines 52–55) correctly addresses Discord's timeout limit. Implementation is clean and consistent with the rest of the moderation suite.
src/commands/lock.js (1)
1-73: LGTM!All previously flagged issues are resolved: channel type is restricted at the builder level (Line 18) with a runtime guard (Line 38),
deferReplyis insidetry/catch, and error logging includes thecommandkey. Implementation is symmetrical withunlock.js.tests/modules/moderation.test.js (4)
1-67: Well-structured test scaffolding and mock setup.Mock ordering in
beforeEach(clear → create → assign) correctly addresses the previous review feedback. TheafterEachproperly tears down the tempban scheduler and restores real timers, preventing cross-test leakage from module-level scheduler state.
69-133: Good transactional coverage forcreateCase.Both the happy path (BEGIN → advisory lock → INSERT → COMMIT → release) and the rollback-on-failure path are well tested, including verification that
client.release()is always called.
346-511: Thorough escalation testing with bothtimeoutandbanactions.The mock wiring for the multi-step escalation flow (warn count query → createCase transaction → sendModLogEmbed) is complex but correct. The sequential
mockResolvedValueOncechains on bothmockPool.queryandmockConnection.queryaccurately model the interleaved pool vs. connection calls.
653-727: Good coverage of scheduler edge cases including the failure-and-mark-executed path.The "skip already-claimed rows" (line 653) and "mark as executed even when unban fails" (line 686) tests are important resilience tests that guard against infinite retry loops and duplicate processing. Both were requested in previous reviews and are now properly addressed.
tests/commands/purge.test.js (3)
56-67: ClevermockCollectionhelper enabling chained.filter()calls.The recursive
filterbinding correctly mirrors Discord.jsCollection.filter()behavior needed by the purge command's two-stage filtering (14-day cutoff → subcommand-specific filter). This is a clean approach.
69-91: Good structural assertions for command metadata.Export checks (name, adminOnly, all 6 subcommands with exact count) catch accidental renames or missing subcommands early.
248-258: LGTM — error path validates graceful degradation.tests/commands/warn.test.js (1)
73-100: LGTM — Happy-path test now includescheckEscalationargument verification.The assertion at lines 89-98 validates the full argument wiring to
checkEscalation, addressing the earlier feedback. The use of bot credentials ('bot1','Bot#0001') as the escalation moderator is consistent with auto-escalation semantics.tests/commands/ban.test.js (2)
74-93: LGTM — successful ban flow thoroughly validated.Covers the full chain: defer → DM → ban API call with correct
deleteMessageSecondscomputation → case creation → reply.
95-103: Good edge-case: non-guild member bypasses hierarchy and proceeds to ban.This correctly exercises the try/catch around
members.fetchinsrc/commands/ban.js(lines 48-50), ensuring bans work for users who've already left the server.tests/commands/unlock.test.js (3)
25-48: Clean interaction factory with spread overrides.The
createInteraction(overrides = {})pattern with...overridesis ergonomic and avoids the need for separate factories per test scenario. Well done.
58-83: LGTM — comprehensive unlock flow validation.Covers the full chain: defer → permission reset (
SendMessages: null) → channel notification embed → case creation → mod log → reply.
125-133: Good guard: non-text channel rejection.Verifies the
ChannelType.GuildTextcheck insrc/commands/unlock.js(line 37) and ensures no case is created for unsupported channel types.tests/commands/kick.test.js (1)
30-62: Good refactor:mockMembernow created inside the factory.This addresses the previous review feedback about shared mutable state. Each test gets a fresh
mockMemberwith its ownkickmock.tests/commands/lock.test.js (1)
58-81: LGTM — correctSendMessages: falsefor lock (vs.nullfor unlock).The key behavioral difference between lock and unlock is correctly captured: lock denies with
false, unlock resets withnull.tests/commands/unban.test.js (3)
64-83: LGTM — full unban happy-path validated.Covers defer → unban API call → user tag resolution → case creation → reply. The user ID and reason assertions against the mock are precise.
85-97: Good edge-case: graceful fallback when user can't be resolved.This correctly exercises the try/catch in
src/commands/unban.js(lines 38-41) where the user may no longer exist in Discord's cache/API but the unban should still proceed with the raw ID as the tag.
99-108: Unban API failure test now present — addresses prior feedback.Ensures that a Discord API rejection during the unban call itself is caught and reported to the user gracefully.
tests/commands/timeout.test.js (1)
1-141: LGTM — test suite covers the key branches well.The test file properly mocks dependencies, validates exports, and covers the success path, invalid/over-limit durations, hierarchy rejection, and error handling. The shared
mockMemberisolation concern and missinggetMember()→nullbranch test were already noted in a prior review round.tests/commands/slowmode.test.js (1)
1-143: LGTM — solid coverage of the slowmode command paths.Tests cover exports, valid duration, disable via
"0", invalid duration, 6-hour cap rejection, channel override, and error handling. Assertions align with the current implementation insrc/commands/slowmode.js.src/commands/slowmode.js (1)
38-97: Clean implementation — prior review feedback addressed.
deferReplyis insidetry, the >6h duration is rejected (not silently capped), a moderation case is created, and the error handler guardseditReplywith.catch(() => {}). All consistent with the other command implementations.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Fix config key mismatch in tempban - Use targetTag in unban reply - Use partial index for scheduler - Batch threshold queries in moderation - Fix escalation phantom case - Add catch to history editReply - Handle createCase fail in slowmode - Fix various test issues (assertions, scaffolding, isolation)
Comprehensive moderation toolkit with 16 slash commands: Core actions: /warn, /kick, /timeout, /untimeout, /ban, /tempban, /unban, /softban Message mgmt: /purge (all, user, bot, contains, links, attachments) Case system: /case (view, list, reason, delete), /history Channel ctrl: /lock, /unlock, /slowmode Log config: /modlog (setup, view, disable) Infrastructure: - mod_cases + mod_scheduled_actions tables in PostgreSQL - Shared moderation module (case creation, DM notifications, mod log embeds, auto-escalation, tempban scheduler) - Duration parser utility (parse + format) - Config extensions (dmNotifications, escalation thresholds, log routing) - Tempban scheduler (60s polling) wired into startup/shutdown All commands: admin-only, ephemeral replies, hierarchy checks, error handling. 527 tests passing, 80%+ coverage on all metrics. Specs complete with as-built updates.
- parseDuration() now rejects non-safe-integer values and Infinity results - warn, kick, timeout, softban check for null getMember() (user left guild)
…nd tempban retry guard - untimeout.js: add null check for getMember() (missed in prior fix pass) - history.js: log errors in catch block instead of silently swallowing - moderation.js: mark failed scheduled tempban actions as executed to prevent infinite 60s retry loop
Specs live at bills-bot/.specs/ outside the project directory.
…tion, bot hierarchy) Move deferReply inside try-catch across 10 commands to prevent unhandled errors when the interaction expires. Sanitize all user-facing error messages to use a generic response instead of leaking internal details. Add botMember parameter to checkHierarchy for centralized bot role verification. Fix slowmode to reject >6h instead of silently capping. Fix softban to continue creating case on unban failure with warning. Add ChannelType.GuildText restriction to slowmode channel option.
Head branch was pushed to by a user without write access
AnExiledDev
force-pushed
the
feat/moderation-commands-v2
branch
from
9e03d65 to
2b4475e
Compare
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In `@biome.json`:
- Around line 2-4: Update the biome.json "files.includes" array to remove the
redundant negation patterns: delete "!node_modules", "!coverage", "!logs", and
"!data" from the includes entry so it only contains "src/**/*.js" and
"tests/**/*.js"; keep the "$schema" value unchanged and ensure the key name
"files.includes" is preserved.
In `@config.json`:
- Around line 36-41: The dmNotifications object is missing a softban key so
shouldSendDm(config, 'softban') will treat softban as false; add a "softban":
true (or false to intentionally disable) entry to the dmNotifications block
(e.g. mirror the "ban" value) or, if you intentionally want softban DMs
suppressed, add an inline comment next to the dmNotifications block documenting
that softban is intentionally omitted/handled via "ban" to avoid silent
behaviour; reference dmNotifications and the shouldSendDm/softban check to
locate the fix.
In `@src/commands/ban.js`:
- Line 86: The structured log call in the info invocation inside
src/commands/ban.js (info('User banned', { target: user.tag, moderator:
interaction.user.tag })) is missing guildId; update that structured payload to
include the guild ID (use interaction.guild.id or interaction.guild?.id to be
safe) so the log entry includes { guildId: <guild id>, target: user.tag,
moderator: interaction.user.tag } for parity with other commands like purge.
In `@src/commands/case.js`:
- Around line 98-121: Move the call to interaction.deferReply into the try block
inside execute so any errors from deferReply are caught; wrap
interaction.deferReply(...) at the top of the try (before the switch) and keep
the existing catch that calls logError('Case command failed', { error:
err.message, subcommand }) and interaction.editReply so editReply is only
attempted for interactions that were successfully deferred. Ensure you reference
the execute function and keep the switch handling
(handleView/handleList/handleReason/handleDelete) unchanged, only relocating
deferReply into the try block so failures are caught and handled.
In `@src/commands/history.js`:
- Around line 73-74: Replace the inconsistent user-facing error message in the
history command: inside the error handling where logError('Command error', {
error: err.message, command: 'history' }) and interaction.editReply(...) are
called, change the reply text to use the standardized string '❌ An error
occurred. Please try again or contact an administrator.' so it matches other
commands and preserves a uniform UX.
In `@src/commands/modlog.js`:
- Around line 57-91: Wrap the initial await interaction.reply(...) in
handleSetup with a try/catch so failures (expired interaction, network error)
don't propagate; in the catch block log the error (e.g., console.error or your
logger), attempt a safe ephemeral fallback message via interaction.followUp or
interaction.reply only if the interaction hasn't already been replied
to/deferred, and then return early to avoid running the rest of the setup logic.
In `@src/commands/slowmode.js`:
- Around line 42-60: The duration parser currently rejects "0s" so durationStr
=== '0s' ends up reported as invalid; modify the slowmode handling so that if
durationStr represents zero (e.g., '0', '0s', '0m', etc.) you treat it as the
disable case instead of calling parseDuration and erroring. Concretely, before
calling parseDuration or when parseDuration returns null, check durationStr for
a zero value (e.g., matches /^0\s*[smhd]?$/i) and set seconds = 0 (and continue
to disable slowmode) otherwise keep the existing parseDuration validation and
the existing interaction.editReply error path. Ensure references to durationStr,
parseDuration, seconds and interaction.editReply are updated accordingly.
In `@src/commands/softban.js`:
- Line 96: The structured log call info('User softbanned', { target:
target.user.tag, moderator: interaction.user.tag }); in src/commands/softban.js
is missing guildId; update that structured object to include guildId:
interaction.guild.id (or interaction.guild?.id) so it matches other moderation
commands (e.g., ban/kick handlers) and allows per-guild filtering in logs—ensure
you reference the same property name "guildId" used elsewhere.
In `@src/commands/unlock.js`:
- Around line 42-52: The permission change is applied before notification/case
creation and if channel.send or createCase throws the unlock persists without
audit info; update the unlock flow in unlock.js so after calling
channel.permissionOverwrites.edit(...) you immediately send the success reply to
the moderator and then wrap the post-action steps (channel.send(...) and
createCase(...)) in their own try/catch block—inside the catch, log the error
and send a follow-up or ephemeral warning to the moderator that
logging/notification failed while making clear the channel was unlocked;
reference channel.permissionOverwrites.edit, channel.send, and createCase to
locate and update the code.
In `@src/utils/duration.js`:
- Around line 60-61: The formatDuration function's initial guard allows NaN and
Infinity through because typeof NaN === 'number'; tighten the check by also
verifying Number.isFinite(ms). Update the condition in formatDuration to return
'0 seconds' when ms is not a number, not finite, or <= 0 (i.e., use a combined
check like typeof ms !== 'number' || !Number.isFinite(ms) || ms <= 0) so invalid
numeric values are handled explicitly before the loop.
In `@tests/commands/ban.test.js`:
- Around line 131-140: Add a new test that simulates a failure of the Discord
ban API by mocking guild.members.ban to reject, call execute with a created
interaction (createInteraction), and assert that createCase was not called and
interaction.editReply was invoked with a message containing 'An error occurred';
this mirrors the existing error test but replaces
createCase.mockRejectedValueOnce with
interaction.guild.members.ban.mockRejectedValueOnce(new Error('...')) and
includes the two expectations (createCase not called and generic error reply) to
verify the catch path when banning fails.
In `@tests/commands/case.test.js`:
- Around line 156-180: The two tests ("should truncate long reasons" and "should
handle cases with no reason") only assert that an embed was returned; update
each test to inspect the embed description returned by execute (capture the
argument passed to interaction.editReply, e.g., from the mocked interaction
returned by createInteraction and its editReply call) and assert that for the
longReasonCase the description includes the reason truncated to 50 characters
(use the same truncation logic used by the code under test) and that for
noReasonCase the description contains the string "No reason"; reference the
mocked objects and helpers (mockCaseRow, createInteraction, execute, getPool,
and interaction.editReply) to locate where to extract the embed from the call
and add the tightened expect assertions.
In `@tests/commands/modlog.test.js`:
- Around line 100-116: The test currently hardcodes
expect(setConfigValue).toHaveBeenCalledTimes(7) which couples it to the current
set of channel keys; change the assertion to derive the expected call count from
the source of truth (e.g., the mocked config channels object or a shared
constant) and assert dynamically — for example compute const expectedCount =
Object.keys(mockConfig['moderation.logging.channels']).length (or import the
categories constant) and use
expect(setConfigValue).toHaveBeenCalledTimes(expectedCount); also iterate those
keys to assert each specific setConfigValue was called with the corresponding
'moderation.logging.channels.<key>' and null, keeping existing checks for
interaction.editReply (references: setConfigValue, mock config object,
createInteraction, execute, interaction.editReply).
In `@tests/commands/tempban.test.js`:
- Around line 42-47: Remove the shared mockUser and mockMember objects from the
describe scope and instead instantiate fresh mockUser and mockMember inside the
existing createInteraction() factory (or create a small factory helper within
createInteraction) so each test gets a new copy; update tests to destructure or
pull mockUser/mockMember from createInteraction() where they are used, and
replace any direct references to the old describe-scope mockUser/mockMember with
the values returned by createInteraction() (ensure references to
roles.highest.position and user.tag/id remain the same shape).
In `@tests/commands/warn.test.js`:
- Around line 37-41: The shared mockMember object at describe scope can leak
state between tests; change the tests to create a fresh mockMember inside a
factory like createInteraction (as used in softban.test.js) instead of sharing
the module-scoped mockMember. Implement createInteraction to construct a new
mockMember with the same shape (id, user.tag, roles.highest.position) each time
and return the interaction object that uses it, then update tests in
warn.test.js to call createInteraction per test so each test gets an isolated
mockMember.
In `@tests/utils/duration.test.js`:
- Around line 95-138: Add a unit test to tests/utils/duration.test.js that
asserts formatDuration falls back to a smaller unit when the value isn't an
exact multiple of a larger unit (e.g., call formatDuration(90000) and expect '90
seconds'); place it near the other formatDuration tests (e.g., under 'uses the
largest fitting unit') and reference the formatDuration function so future
changes that try to produce compound units will be caught.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (44)
.gitignoreAGENTS.mdREADME.mdbiome.jsonconfig.jsonsrc/commands/ban.jssrc/commands/case.jssrc/commands/history.jssrc/commands/kick.jssrc/commands/lock.jssrc/commands/modlog.jssrc/commands/purge.jssrc/commands/slowmode.jssrc/commands/softban.jssrc/commands/tempban.jssrc/commands/timeout.jssrc/commands/unban.jssrc/commands/unlock.jssrc/commands/untimeout.jssrc/commands/warn.jssrc/db.jssrc/index.jssrc/modules/moderation.jssrc/utils/duration.jstests/commands/ban.test.jstests/commands/case.test.jstests/commands/history.test.jstests/commands/kick.test.jstests/commands/lock.test.jstests/commands/modlog.test.jstests/commands/purge.test.jstests/commands/slowmode.test.jstests/commands/softban.test.jstests/commands/tempban.test.jstests/commands/timeout.test.jstests/commands/unban.test.jstests/commands/unlock.test.jstests/commands/untimeout.test.jstests/commands/warn.test.jstests/db.test.jstests/index.test.jstests/modules/ai.test.jstests/modules/moderation.test.jstests/utils/duration.test.js
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.js: Use ESM syntax withimport/export; never userequire()
Always usenode:protocol when importing Node.js builtins (e.g.,import { readFileSync } from 'node:fs')
Always use semicolons in code
Use single quotes for strings (enforced by Biome)
Use 2-space indentation (enforced by Biome)
Always use Winston logging viaimport { info, warn, error } from '../logger.js'— NEVER useconsole.log(),console.warn(),console.error(), or anyconsole.*method in src/ files
Pass structured metadata when logging:info('Message processed', { userId, channelId })
Use custom error classes fromsrc/utils/errors.jsfor error handling
Always log errors with context before re-throwing
UsegetConfig()fromsrc/modules/config.jsto read config; usesetConfigValue(key, value)to update at runtime
UsesplitMessage()utility for messages exceeding Discord's 2000-character limit
No TypeScript — use plain JavaScript with JSDoc comments for documentation
Files:
src/commands/kick.jssrc/commands/untimeout.jssrc/commands/history.jssrc/commands/tempban.jssrc/commands/softban.jssrc/index.jssrc/commands/purge.jssrc/commands/timeout.jssrc/db.jssrc/commands/ban.jssrc/commands/lock.jssrc/commands/case.jssrc/commands/unban.jssrc/commands/slowmode.jssrc/modules/moderation.jssrc/utils/duration.jssrc/commands/unlock.jssrc/commands/warn.jssrc/commands/modlog.js
src/commands/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Slash commands must export
data(SlashCommandBuilder) andexecute(interaction)function
Files:
src/commands/kick.jssrc/commands/untimeout.jssrc/commands/history.jssrc/commands/tempban.jssrc/commands/softban.jssrc/commands/purge.jssrc/commands/timeout.jssrc/commands/ban.jssrc/commands/lock.jssrc/commands/case.jssrc/commands/unban.jssrc/commands/slowmode.jssrc/commands/unlock.jssrc/commands/warn.jssrc/commands/modlog.js
tests/**/*.{js,test.js}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for testing with
pnpm test; test coverage provider is@vitest/coverage-v8
Files:
tests/commands/tempban.test.jstests/utils/duration.test.jstests/commands/kick.test.jstests/commands/purge.test.jstests/modules/ai.test.jstests/commands/slowmode.test.jstests/commands/untimeout.test.jstests/commands/history.test.jstests/commands/ban.test.jstests/commands/unlock.test.jstests/db.test.jstests/modules/moderation.test.jstests/commands/timeout.test.jstests/commands/warn.test.jstests/commands/softban.test.jstests/index.test.jstests/commands/unban.test.jstests/commands/case.test.jstests/commands/lock.test.jstests/commands/modlog.test.js
src/modules/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Check
config.yourModule.enabledbefore processing in module handlers
Files:
src/modules/moderation.js
🧠 Learnings (15)
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/commands/*.js : Slash commands must export `data` (SlashCommandBuilder) and `execute(interaction)` function
Applied to files:
src/commands/kick.jssrc/commands/untimeout.jssrc/commands/history.jssrc/commands/tempban.jstests/commands/kick.test.jstests/commands/purge.test.jssrc/commands/softban.jssrc/commands/purge.jssrc/commands/timeout.jstests/commands/slowmode.test.jstests/commands/history.test.jssrc/commands/ban.jssrc/commands/lock.jssrc/commands/case.jssrc/commands/unban.jstests/commands/ban.test.jstests/commands/unlock.test.jssrc/commands/slowmode.jstests/commands/timeout.test.jstests/commands/warn.test.jstests/commands/softban.test.jstests/commands/unban.test.jstests/commands/case.test.jstests/commands/lock.test.jssrc/commands/unlock.jssrc/commands/warn.jssrc/commands/modlog.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to tests/**/*.{js,test.js} : Use Vitest for testing with `pnpm test`; test coverage provider is `vitest/coverage-v8`
Applied to files:
tests/utils/duration.test.jstests/commands/purge.test.js.gitignoretests/commands/warn.test.jsbiome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Graceful shutdown is handled in `src/index.js`; use error classes and logging before throwing errors in other modules
Applied to files:
src/index.jssrc/commands/modlog.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Module handlers must be registered in `src/modules/events.js` to wire them to Discord events
Applied to files:
src/index.jsAGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Commands are auto-discovered from `src/commands/` on startup; after adding a command, run `pnpm run deploy` to register with Discord
Applied to files:
src/commands/purge.jssrc/commands/unban.jsAGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use `splitMessage()` utility for messages exceeding Discord's 2000-character limit
Applied to files:
src/commands/timeout.jsAGENTS.md
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Update Key Files table in AGENTS.md when adding a new command or module
Applied to files:
AGENTS.md
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to src/**/*.{ts,tsx} : Use `date-fns` for date manipulation and formatting throughout the application
Applied to files:
src/utils/duration.js
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use single quotes for strings (enforced by Biome)
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use 2-space indentation (enforced by Biome)
Applied to files:
biome.json
📚 Learning: 2025-11-26T01:57:34.920Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:57:34.920Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json,css,md} : After changing or editing any files, run the complete validation workflow: `pnpm format && pnpm typecheck && pnpm lint && pnpm build` before committing
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : No TypeScript — use plain JavaScript with JSDoc comments for documentation
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Use ESM syntax with `import`/`export`; never use `require()`
Applied to files:
biome.json
📚 Learning: 2026-02-11T17:18:29.603Z
Learnt from: CR
Repo: BillChirico/bills-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-11T17:18:29.603Z
Learning: Applies to src/**/*.js : Always use Winston logging via `import { info, warn, error } from '../logger.js'` — NEVER use `console.log()`, `console.warn()`, `console.error()`, or any `console.*` method in src/ files
Applied to files:
src/commands/modlog.js
📚 Learning: 2026-02-04T02:20:09.131Z
Learnt from: CR
Repo: VolvoxCommunity/Volvox.Website PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-04T02:20:09.131Z
Learning: Applies to src/**/*.{ts,tsx} : Use `reportError(context, error)` from `src/lib/logger.ts` to report errors to Sentry with context metadata, falling back to console.error if Sentry is disabled
Applied to files:
src/commands/modlog.js
🧬 Code graph analysis (23)
src/commands/kick.js (2)
src/modules/moderation.js (7)
config(376-376)reason(301-301)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)
src/commands/tempban.js (4)
src/index.js (1)
config(52-52)src/modules/moderation.js (8)
config(376-376)reason(301-301)member(306-306)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)scheduleAction(166-177)sendModLogEmbed(209-262)src/utils/duration.js (2)
parseDuration(23-36)formatDuration(60-71)src/logger.js (1)
info(216-218)
tests/commands/tempban.test.js (4)
src/commands/tempban.js (5)
data(19-36)data(19-36)adminOnly(38-38)adminOnly(38-38)execute(44-117)src/index.js (1)
interaction(157-157)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)scheduleAction(166-177)src/utils/duration.js (1)
parseDuration(23-36)
tests/utils/duration.test.js (1)
src/utils/duration.js (2)
parseDuration(23-36)formatDuration(60-71)
tests/commands/kick.test.js (1)
src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(449-457)
tests/commands/purge.test.js (1)
src/commands/purge.js (6)
filtered(114-114)data(11-95)data(11-95)subcommand(107-107)deleted(143-143)execute(103-175)
src/commands/softban.js (3)
src/modules/moderation.js (7)
config(376-376)reason(301-301)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/index.js (1)
src/modules/moderation.js (4)
stopTempbanScheduler(434-440)startTempbanScheduler(414-429)client(89-89)client(97-135)
src/commands/timeout.js (3)
src/modules/moderation.js (7)
config(376-376)reason(301-301)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/utils/duration.js (2)
parseDuration(23-36)formatDuration(60-71)src/logger.js (1)
info(216-218)
src/db.js (1)
src/modules/moderation.js (8)
pool(88-88)pool(167-167)pool(168-174)pool(244-244)pool(288-288)pool(291-296)pool(351-351)pool(352-357)
src/commands/ban.js (4)
src/index.js (1)
config(52-52)src/modules/moderation.js (8)
config(376-376)reason(301-301)member(306-306)checkHierarchy(449-457)shouldSendDm(465-467)sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/commands/case.js (5)
src/modules/moderation.js (13)
ACTION_COLORS(17-30)ACTION_COLORS(17-30)pool(88-88)pool(167-167)pool(168-174)pool(244-244)pool(288-288)pool(291-296)pool(351-351)pool(352-357)config(376-376)ACTION_LOG_CHANNEL_KEY(51-64)ACTION_LOG_CHANNEL_KEY(51-64)src/commands/modlog.js (4)
execute(32-51)subcommand(33-33)row(75-75)config(169-169)src/modules/config.js (1)
getConfig(130-132)src/db.js (2)
pool(12-12)getPool(181-186)src/logger.js (1)
info(216-218)
src/commands/unban.js (3)
src/modules/moderation.js (4)
config(376-376)reason(301-301)createCase(87-155)sendModLogEmbed(209-262)src/modules/config.js (2)
getConfig(130-132)err(30-30)src/logger.js (1)
info(216-218)
src/commands/slowmode.js (2)
src/utils/duration.js (3)
ms(33-33)parseDuration(23-36)formatDuration(60-71)src/modules/config.js (2)
getConfig(130-132)err(30-30)
src/modules/moderation.js (5)
src/commands/history.js (2)
pool(26-26)pool(27-30)src/db.js (2)
pool(12-12)getPool(181-186)src/logger.js (1)
info(216-218)src/modules/config.js (1)
getConfig(130-132)src/utils/duration.js (2)
ms(33-33)parseDuration(23-36)
tests/modules/moderation.test.js (1)
src/modules/moderation.js (14)
createCase(87-155)scheduleAction(166-177)member(306-306)sendDmNotification(187-200)embed(189-193)embed(220-233)client(89-89)client(97-135)config(376-376)sendModLogEmbed(209-262)checkEscalation(275-337)checkHierarchy(449-457)shouldSendDm(465-467)startTempbanScheduler(414-429)
tests/commands/timeout.test.js (3)
src/commands/timeout.js (1)
execute(35-95)src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(449-457)src/utils/duration.js (1)
parseDuration(23-36)
tests/commands/warn.test.js (2)
src/commands/warn.js (1)
execute(32-82)src/modules/moderation.js (5)
sendDmNotification(187-200)createCase(87-155)sendModLogEmbed(209-262)checkEscalation(275-337)checkHierarchy(449-457)
tests/commands/softban.test.js (1)
src/modules/moderation.js (3)
sendDmNotification(187-200)createCase(87-155)checkHierarchy(449-457)
src/utils/duration.js (3)
src/commands/slowmode.js (1)
ms(49-49)src/modules/moderation.js (1)
ms(309-309)src/commands/purge.js (1)
count(108-108)
tests/commands/unban.test.js (2)
src/commands/unban.js (1)
execute(27-65)src/modules/moderation.js (1)
createCase(87-155)
tests/commands/case.test.js (2)
src/commands/case.js (2)
subcommand(101-101)execute(98-122)src/db.js (1)
getPool(181-186)
src/commands/modlog.js (2)
src/modules/config.js (4)
i(360-360)setConfigValue(141-225)err(30-30)getConfig(130-132)src/logger.js (1)
info(216-218)
🪛 markdownlint-cli2 (0.20.0)
AGENTS.md
[warning] 92-92: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/2/3
(MD029, ol-prefix)
[warning] 93-93: Ordered list item prefix
Expected: 2; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 94-94: Ordered list item prefix
Expected: 3; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 95-95: Ordered list item prefix
Expected: 4; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
🔇 Additional comments (46)
.gitignore (1)
24-26: LGTM!The
.specs/directory exclusion is appropriate for keeping spec documents tracked outside the repo.tests/commands/history.test.js (1)
1-105: Well-structured test suite with good coverage of happy path, empty, and error scenarios.Clean mock setup and assertions.
src/utils/duration.js (1)
1-71: Clean, well-documented utility module.The parsing and formatting logic is solid with good input validation and overflow guards.
README.md (1)
15-15: Comprehensive documentation update covering all new moderation features, config fields, and commands.The README additions are well-organized and match the implemented command suite.
Also applies to: 149-166, 176-229
AGENTS.md (1)
32-32: LGTM — Key Files table, Moderation Command Pattern, DB tables, and new pitfalls are well documented.Good additions that will help onboard future contributors (human and AI).
Also applies to: 41-41, 97-109, 111-116, 172-176
src/commands/case.js (2)
6-10: Good resolution of previously duplicated constants.
ACTION_COLORSandACTION_LOG_CHANNEL_KEYare now properly imported fromsrc/modules/moderation.js, and thelistsubcommand type choices include all action types. Clean fix.
118-121: Error handling approach is correct and consistent with the codebase.The reply is ephemeral (line 99), and the pattern here—logging the full error context (
err.message) while showing a generic message to the user—is consistent across all moderation commands (warn, history, modlog, timeout, etc.). The full error is captured in logs for admin debugging, so no additional context is needed in the user-facing reply. This is the established sanitization pattern and requires no changes.tests/db.test.js (1)
109-116: LGTM — moderation schema assertions are well-structured.The new assertions correctly mirror the schema additions in
src/db.jsand follow the same pattern as the existing table/index checks above.tests/commands/untimeout.test.js (1)
28-60: Good:createInteractionfactory creates a freshmockMemberper call.This addresses the earlier shared-state concern. Each test gets its own mock member instance.
src/index.js (1)
294-297: LGTM — Tempban scheduler correctly guarded bydbPoolcheck.The scheduler only starts when the database is available, and
stopTempbanScheduler()is safely called unconditionally during shutdown (it's a no-op when the scheduler wasn't started). The shutdown ordering (stop scheduler → close DB) is also correct.src/db.js (1)
120-160: Schema additions look correct and well-structured.The moderation tables align with the queries in
src/modules/moderation.js:createCaseinserts intomod_caseswith advisory locking for case-number serialization, andscheduleActioninserts intomod_scheduled_actionswith the matching column set. The FK frommod_scheduled_actions.case_idtomod_cases(id)provides proper referential integrity. The redundantidx_mod_cases_guild_caseindex from the prior review has been removed.tests/commands/modlog.test.js (1)
118-232: Thorough interactive setup flow coverage.The collector-based tests (done button, category → channel selection, channel-without-category guard, timeout, non-timeout end) cover the full interaction lifecycle well. The
_collectHandlerspattern for capturing and invoking collector callbacks is a clean approach.tests/commands/purge.test.js (1)
56-67:mockCollectionis a solid lightweight Collection stub.The recursive
.filterbinding ensures chained filters work correctly, which matches the purge command's multi-stage filtering (14-day cutoff → subcommand-specific filter). The nativeMap.sizeproperty serves the.sizechecks naturally.src/commands/unban.js (1)
1-65: Overall command structure is clean and follows established patterns.The deferReply-inside-try, user-tag resolution with fallback, case creation + mod-log flow, and generic error reply with
.catch(() => {})all align with the conventions used across the other moderation commands.tests/commands/tempban.test.js (1)
85-108: Success-path test has thorough assertions covering the full tempban lifecycle.The test validates the complete flow: deferred reply → DM notification → ban → case creation (with duration) → scheduled unban → success reply. Good end-to-end coverage.
src/commands/warn.js (1)
1-81: Clean implementation following established patterns.The command correctly defers inside try/catch, uses generic error messages (addressing prior feedback), passes
botMemberfor hierarchy checks, and follows all coding guidelines (ESM, Winston logging with structured metadata, single quotes, semicolons).src/commands/untimeout.js (1)
1-64: Clean implementation, follows the established command pattern.Correctly clears the timeout via
target.timeout(null, ...), skips DM notification and escalation (appropriate for an untimeout action), and handles errors generically.tests/utils/duration.test.js (1)
140-147: Nice round-trip tests — note the intentional representation change for7d.Line 143 asserts
formatDuration(parseDuration('7d'))→'1 week', which documents that the round-trip normalizes to the largest fitting unit. This is good to have explicitly.tests/commands/softban.test.js (1)
1-166: Good test coverage with well-structured retry and failure scenarios.The factory pattern for
createInteraction(returning{ interaction, mockMember }) is a nice improvement over shared-scope objects. The fake-timer usage for unban retry testing is correct, and the "unban keeps failing but still creates case" test (Line 129) validates an important resilience behavior mentioned in the PR objectives.tests/modules/ai.test.js (1)
14-14: Clean test setup addition.Adding
_setPoolGetter(null)inbeforeEachensures proper isolation between tests when the pool getter is swapped. The underscore-prefixed export convention clearly signals this is a test-only API.Also applies to: 36-43
tests/commands/unlock.test.js (1)
1-145: Solid test coverage for the unlock command.Good use of the
overridesparameter increateInteractionfor flexibility. Tests cover the key scenarios including current channel, specified channel, reason propagation, non-text channel rejection, and error handling.tests/commands/slowmode.test.js (1)
1-143: LGTM — good coverage of the slowmode command's core paths.The test suite covers valid duration, disable (
"0"), invalid input, the 6-hour cap rejection, channel overrides, and error handling. Structure is clean and consistent with other command test files in this PR.src/commands/purge.js (1)
11-95: Well-structured subcommand definitions with clear descriptions.The subcommand descriptions accurately state "Messages to scan" (rather than "messages to delete"), which sets correct user expectations for the fetch-then-filter behavior. The builder constraints (
setMinValue(1),setMaxValue(100)) are appropriate for Discord's bulk-delete limits.tests/commands/kick.test.js (1)
25-112: Good test structure with proper mock isolation.The
createInteractionfactory correctly creates a freshmockMemberper call (addressing the earlier shared-state concern), and the test suite covers the core happy path, hierarchy rejection, and error handling.One gap remains: the kick command's null check for
getMember('user')returningnull(i.e., user not in server) is untested. Consider adding a test for completeness.tests/commands/lock.test.js (1)
1-146: LGTM — comprehensive coverage of the lock command.Good coverage of default vs. specified channel, reason propagation, non-text channel rejection, and error handling. The mock structure is clean and consistent with the rest of the test suite.
src/commands/softban.js (1)
65-83: Good retry logic for the unban step with linear backoff.The 3-attempt retry with
250ms * attemptbackoff and per-attempt logging is a solid approach. Continuing to case creation regardless of unban outcome (with a clear warning to the moderator at Line 98-101) aligns well with the PR objectives.src/commands/ban.js (1)
39-95: Well-structured ban command — past review concerns addressed.All previously flagged issues are resolved:
deferReplyis insidetry,checkHierarchynow receivesinteraction.guild.members.mefor bot-self hierarchy checks, and the error handler uses.catch(() => {}). The silent catch onmembers.fetch(Lines 49-53) correctly enables banning users who have already left the server.config.json (1)
49-59: The review comment incorrectly states thatsoftbans,unbans, andslowmodeare missing from the logging channel mapping. In fact,ACTION_LOG_CHANNEL_KEYinsrc/modules/moderation.jsexplicitly maps all of these actions:
softban→'bans'channelunban→'bans'channelslowmode→'locks'channelThese actions do not fall back to the default channel; they have explicit mappings to existing log channel keys.
Likely an incorrect or invalid review comment.
tests/commands/unban.test.js (1)
1-120: LGTM!Solid test coverage for the unban command — covers the happy path, user-fetch fallback, Discord API failure, and DB error paths. The interaction factory correctly creates fresh mocks per test, and assertions align with the source implementation.
src/commands/unlock.js (1)
1-73: LGTM!Clean implementation that correctly addresses prior review feedback — channel type is restricted to
GuildTextat the option level,deferReplyis insidetry, error logging includes thecommandkey, andeditReplyin the catch has.catch(() => {}).src/commands/kick.js (1)
1-74: LGTM!Clean implementation following the established moderation command pattern. Notable: DM notification is correctly sent before the kick (Line 47-49), since the user won't be reachable after leaving the server. Hierarchy check includes
botMember, error handling is consistent with other commands, and structured logging metadata is present.src/commands/timeout.js (1)
1-95: LGTM!Well-structured implementation that correctly addresses the previously flagged 28-day upper-bound validation (Lines 52-55). The flow is sound: parse → validate bounds → hierarchy check → DM (before action) → timeout → case creation → mod log. Error handling is consistent with other commands.
tests/index.test.js (2)
45-48: LGTM!The moderation scheduler mocks are cleanly integrated into the existing test scaffolding — hoisted mock definition, module wiring, lifecycle resets, and startup behavior assertions all follow the established patterns. The new
startTempbanSchedulerassertions correctly validate that the scheduler is gated onDATABASE_URLavailability.Also applies to: 145-148, 232-233, 292-293, 303-304
165-173: Good improvement:settleStartupHopshelper addresses the fragile microtask settling concern.The named helper with a descriptive comment explaining the hop count and when to update it is a clear improvement over the previous inline approach. This directly addresses the prior review feedback.
src/commands/slowmode.js (1)
38-96: All prior review feedback has been addressed.
- Case creation and mod-log posting are now included (Lines 65-78).
- Duration >6h is rejected rather than silently capped (Lines 56-58).
deferReplyis insidetry(Line 40).- Error-path
editReplyhas.catch(() => {})(Line 95).- Error message is sanitized (Line 94) — no longer leaks
err.messageto the user.src/commands/lock.js (1)
1-73: LGTM! All prior review feedback has been addressed.
- Channel option is now restricted to
GuildText(Line 18) with a runtime guard (Line 38).deferReplyis insidetry(Line 33).- Error log includes
command: 'lock'(Line 68).- Error-path
editReplyhas.catch(() => {})(Line 71).Implementation is consistent with
unlock.js.src/commands/modlog.js (2)
19-51: LGTM — Command definition and execute dispatcher.The
SlashCommandBuilder,adminOnlyexport, andexecutedispatcher with default case are well-structured. Past review items for these sections have been addressed.
167-189: LGTM —handleViewerror handling.Properly wrapped in try/catch with logging and user-facing error reply. The
.catch(() => {})on the fallback reply is a good defensive pattern.src/commands/tempban.js (1)
44-116: LGTM — Overall execute structure.
deferReplyis inside the try block, hierarchy check includesbotMember, duration validation with early return, scheduling viascheduleAction, and error handling with sanitized user-facing messages are all well done. Past review items have been addressed.tests/modules/moderation.test.js (3)
1-67: LGTM — Test setup and mock configuration.Clean mock setup with proper ordering (clearAllMocks before mockReturnValue), comprehensive dependency mocking, and proper teardown in afterEach including scheduler cleanup and timer restoration.
69-133: LGTM — createCase tests.Both the happy path (atomic insert with BEGIN/advisory lock/COMMIT/release) and the failure path (ROLLBACK on insert error, release in finally) are properly verified.
571-728: LGTM — Tempban scheduler tests.Good coverage of idempotent start/stop, successful poll processing, already-claimed row skipping, and the failure-and-mark-executed path. The sequential mock chaining for DB queries is well-structured and matches the production code flow.
src/modules/moderation.js (4)
87-155: LGTM —createCasewith atomic case number generation.Per-guild advisory lock +
COALESCE(MAX(...), 0) + 1subquery within a transaction is a robust approach. ROLLBACK on failure with client release infinallyis correct.
343-406: LGTM —pollTempbansimplementation.The
schedulerPollInFlightguard,LIMIT 50withORDER BY, optimistic claim pattern, per-row error handling, and user tag resolution with fallback are all well done. Past review items have been addressed.
209-262: LGTM —sendModLogEmbedwith channel routing and error handling.Action-specific channel lookup with default fallback, proper error logging for both send and log-message-ID storage failures, and graceful null returns are correct.
449-457: LGTM —checkHierarchywith bot member support.Clean implementation with optional
botMemberparameter and clear error messages for both moderator and bot hierarchy failures.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "$schema": "https://biomejs.dev/schemas/2.3.14/schema.json", | ||
| "files": { | ||
| "includes": [ | ||
| "src/**/*.js", | ||
| "tests/**/*.js", | ||
| "!node_modules/**", | ||
| "!coverage/**", | ||
| "!logs/**", | ||
| "!data/**" | ||
| ] | ||
| "includes": ["src/**/*.js", "tests/**/*.js", "!node_modules", "!coverage", "!logs", "!data"] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Biome 2.3.14 schema version release documentation
💡 Result:
Biome 2.3.14 configuration schema (biome.json / biome.jsonc)
Schema version to use (matches Biome 2.3.14):
{
"$schema": "https://biomejs.dev/schemas/2.3.14/schema.json"
}This is shown in Biome’s “Getting started” guide (example config) for v2.3.14. [1]
Alternative (recommended to avoid hardcoding the URL version):
{
"$schema": "./node_modules/@biomejs/biome/configuration_schema.json"
}Biome documents this local schema file option in the configuration reference. [2]
Biome 2.3.14 release documentation (what changed)
Biome publishes the 2.3.14 changelog entry here (“All versions since 2.3.14” → section 2.3.14). It lists the patch changes included in 2.3.14. [3]
Sources: [1] (biomejs.dev) [2] (biomejs.dev) [3] (main--biomejs.netlify.app)
Citations:
- 1: https://biomejs.dev/es/guides/getting-started/?utm_source=openai
- 2: https://biomejs.dev/reference/configuration/?utm_source=openai
- 3: https://main--biomejs.netlify.app/internals/changelog/version/2-3-14...latest/?utm_source=openai
🌐 Web query:
Biome files.includes negation patterns exclude behavior
💡 Result:
In Biome, files.includes is an allowlist of glob patterns, with optional negated “exception” patterns to exclude matches.
- Negation syntax: patterns starting with
!exclude files/folders that would otherwise be included. (biomejs.dev) - You must include
**first if you want negations to work globally (otherwise the negated patterns may match nothing). Example:(biomejs.dev){ "files": { "includes": ["**", "!**/dist", "!**/*.generated.js"] } } - Order matters: patterns are processed in order, so you can “exclude, then re-include” (exceptions to exceptions), e.g.:
(biomejs.dev)
{ "files": { "includes": ["**", "!**/*.test.js", "**/special.test.js", "!test"] } } files.includesis a hard gate: if a file doesn’t matchfiles.includes, then tool-specific filters likelinter.includes/formatter.includescannot bring it back. (biomejs.dev)- Directory exclusion shorthand:
!test(exclude a directory) is supported infiles.includes; in tool-specificincludesyou typically need something like!/test/**. (biomejs.dev) - Force-ignore (
!!) for the scanner/indexing: use!!patternto ensure Biome never scans/indexes those paths (useful for build output dirs). (biomejs.dev)
Citations:
- 1: https://biomejs.dev/guides/configure-biome?utm_source=openai
- 2: https://biomejs.dev/guides/configure-biome?utm_source=openai
- 3: https://biomejs.dev/reference/configuration/?utm_source=openai
- 4: https://biomejs.dev/guides/configure-biome?utm_source=openai
- 5: https://biomejs.dev/reference/configuration/?utm_source=openai
- 6: https://biomejs.dev/reference/configuration/?utm_source=openai
Remove redundant exclusion patterns from files.includes.
The schema version 2.3.14 is valid and confirmed in Biome's official documentation. However, the negation patterns !node_modules, !coverage, !logs, and !data are unnecessary. Since includes already restricts files to src/**/*.js and tests/**/*.js, those directories are never matched in the first place. These negation patterns have no effect and should be removed for clarity.
"files": {
- "includes": ["src/**/*.js", "tests/**/*.js", "!node_modules", "!coverage", "!logs", "!data"]
+ "includes": ["src/**/*.js", "tests/**/*.js"]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$schema": "https://biomejs.dev/schemas/2.3.14/schema.json", | |
| "files": { | |
| "includes": [ | |
| "src/**/*.js", | |
| "tests/**/*.js", | |
| "!node_modules/**", | |
| "!coverage/**", | |
| "!logs/**", | |
| "!data/**" | |
| ] | |
| "includes": ["src/**/*.js", "tests/**/*.js", "!node_modules", "!coverage", "!logs", "!data"] | |
| "$schema": "https://biomejs.dev/schemas/2.3.14/schema.json", | |
| "files": { | |
| "includes": ["src/**/*.js", "tests/**/*.js"] |
🤖 Prompt for AI Agents
In `@biome.json` around lines 2 - 4, Update the biome.json "files.includes" array
to remove the redundant negation patterns: delete "!node_modules", "!coverage",
"!logs", and "!data" from the includes entry so it only contains "src/**/*.js"
and "tests/**/*.js"; keep the "$schema" value unchanged and ensure the key name
"files.includes" is preserved.
| "dmNotifications": { | ||
| "warn": true, | ||
| "timeout": true, | ||
| "kick": true, | ||
| "ban": true | ||
| }, |
There was a problem hiding this comment.
dmNotifications is missing a softban key — softban DMs will be silently disabled.
The softban command calls shouldSendDm(config, 'softban') (see src/commands/softban.js Line 56), which checks config.moderation.dmNotifications.softban. Since this key is absent, the check returns false and no DM is sent before a softban. If this is intentional (grouping softban under ban or deliberately suppressing DMs), add an inline comment. Otherwise, add the key:
🐛 Proposed fix
"dmNotifications": {
"warn": true,
"timeout": true,
"kick": true,
- "ban": true
+ "ban": true,
+ "softban": true
},🤖 Prompt for AI Agents
In `@config.json` around lines 36 - 41, The dmNotifications object is missing a
softban key so shouldSendDm(config, 'softban') will treat softban as false; add
a "softban": true (or false to intentionally disable) entry to the
dmNotifications block (e.g. mirror the "ban" value) or, if you intentionally
want softban DMs suppressed, add an inline comment next to the dmNotifications
block documenting that softban is intentionally omitted/handled via "ban" to
avoid silent behaviour; reference dmNotifications and the shouldSendDm/softban
check to locate the fix.
|
|
||
| await sendModLogEmbed(interaction.client, config, caseData); | ||
|
|
||
| info('User banned', { target: user.tag, moderator: interaction.user.tag }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Structured log missing guildId — same as softban.
For log-filtering parity with other commands (e.g., purge includes guildId), consider adding it here as well.
♻️ Suggested fix
- info('User banned', { target: user.tag, moderator: interaction.user.tag });
+ info('User banned', { guildId: interaction.guild.id, target: user.tag, moderator: interaction.user.tag });🤖 Prompt for AI Agents
In `@src/commands/ban.js` at line 86, The structured log call in the info
invocation inside src/commands/ban.js (info('User banned', { target: user.tag,
moderator: interaction.user.tag })) is missing guildId; update that structured
payload to include the guild ID (use interaction.guild.id or
interaction.guild?.id to be safe) so the log entry includes { guildId: <guild
id>, target: user.tag, moderator: interaction.user.tag } for parity with other
commands like purge.
| export async function execute(interaction) { | ||
| await interaction.deferReply({ ephemeral: true }); | ||
|
|
||
| const subcommand = interaction.options.getSubcommand(); | ||
|
|
||
| try { | ||
| switch (subcommand) { | ||
| case 'view': | ||
| await handleView(interaction); | ||
| break; | ||
| case 'list': | ||
| await handleList(interaction); | ||
| break; | ||
| case 'reason': | ||
| await handleReason(interaction); | ||
| break; | ||
| case 'delete': | ||
| await handleDelete(interaction); | ||
| break; | ||
| } | ||
| } catch (err) { | ||
| logError('Case command failed', { error: err.message, subcommand }); | ||
| await interaction.editReply('Failed to execute case command.'); | ||
| } |
There was a problem hiding this comment.
deferReply is outside the try/catch — contradicts the PR's own hardening objective.
The PR summary states commits "move deferReply inside try/catch across commands to avoid expired-interaction errors." Here, deferReply at Line 99 is before the try block at Line 103. If deferReply throws (e.g., expired interaction), the catch block calls editReply on a non-deferred interaction, which will also fail with an unhandled rejection.
Proposed fix
export async function execute(interaction) {
- await interaction.deferReply({ ephemeral: true });
-
- const subcommand = interaction.options.getSubcommand();
-
try {
+ await interaction.deferReply({ ephemeral: true });
+
+ const subcommand = interaction.options.getSubcommand();
+
switch (subcommand) {
case 'view':
await handleView(interaction);
break;🤖 Prompt for AI Agents
In `@src/commands/case.js` around lines 98 - 121, Move the call to
interaction.deferReply into the try block inside execute so any errors from
deferReply are caught; wrap interaction.deferReply(...) at the top of the try
(before the switch) and keep the existing catch that calls logError('Case
command failed', { error: err.message, subcommand }) and interaction.editReply
so editReply is only attempted for interactions that were successfully deferred.
Ensure you reference the execute function and keep the switch handling
(handleView/handleList/handleReason/handleDelete) unchanged, only relocating
deferReply into the try block so failures are caught and handled.
| logError('Command error', { error: err.message, command: 'history' }); | ||
| await interaction.editReply('❌ Failed to fetch moderation history.'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Error reply leaks a generic message instead of using the sanitized pattern from other commands.
Other commands in this PR use the standardized message '❌ An error occurred. Please try again or contact an administrator.'. This command uses a different string ('❌ Failed to fetch moderation history.'). While not a bug, inconsistency makes it harder to maintain a uniform user experience across the moderation suite.
🤖 Prompt for AI Agents
In `@src/commands/history.js` around lines 73 - 74, Replace the inconsistent
user-facing error message in the history command: inside the error handling
where logError('Command error', { error: err.message, command: 'history' }) and
interaction.editReply(...) are called, change the reply text to use the
standardized string '❌ An error occurred. Please try again or contact an
administrator.' so it matches other commands and preserves a uniform UX.
| it('should truncate long reasons', async () => { | ||
| const longReasonCase = { ...mockCaseRow, reason: 'A'.repeat(60) }; | ||
| const mockPool = { query: vi.fn().mockResolvedValue({ rows: [longReasonCase] }) }; | ||
| getPool.mockReturnValue(mockPool); | ||
|
|
||
| const interaction = createInteraction('list'); | ||
| await execute(interaction); | ||
|
|
||
| expect(interaction.editReply).toHaveBeenCalledWith( | ||
| expect.objectContaining({ embeds: expect.any(Array) }), | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle cases with no reason', async () => { | ||
| const noReasonCase = { ...mockCaseRow, reason: null }; | ||
| const mockPool = { query: vi.fn().mockResolvedValue({ rows: [noReasonCase] }) }; | ||
| getPool.mockReturnValue(mockPool); | ||
|
|
||
| const interaction = createInteraction('list'); | ||
| await execute(interaction); | ||
|
|
||
| expect(interaction.editReply).toHaveBeenCalledWith( | ||
| expect.objectContaining({ embeds: expect.any(Array) }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Truncation and null-reason tests only assert embed presence, not content.
The "should truncate long reasons" and "should handle cases with no reason" tests verify that an embed is returned but don't assert on the actual description content (e.g., that the reason is truncated to 50 chars or that "No reason" appears). This means the truncation/null-handling logic could break without these tests catching it.
Consider tightening the assertions:
Example assertion for truncation test
- expect(interaction.editReply).toHaveBeenCalledWith(
- expect.objectContaining({ embeds: expect.any(Array) }),
- );
+ const call = interaction.editReply.mock.calls[0][0];
+ const description = call.embeds[0].data.description;
+ expect(description).toContain('...');🤖 Prompt for AI Agents
In `@tests/commands/case.test.js` around lines 156 - 180, The two tests ("should
truncate long reasons" and "should handle cases with no reason") only assert
that an embed was returned; update each test to inspect the embed description
returned by execute (capture the argument passed to interaction.editReply, e.g.,
from the mocked interaction returned by createInteraction and its editReply
call) and assert that for the longReasonCase the description includes the reason
truncated to 50 characters (use the same truncation logic used by the code under
test) and that for noReasonCase the description contains the string "No reason";
reference the mocked objects and helpers (mockCaseRow, createInteraction,
execute, getPool, and interaction.editReply) to locate where to extract the
embed from the call and add the tightened expect assertions.
| describe('disable subcommand', () => { | ||
| it('should clear all log channels', async () => { | ||
| const interaction = createInteraction('disable'); | ||
| await execute(interaction); | ||
|
|
||
| expect(setConfigValue).toHaveBeenCalledTimes(7); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.default', null); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.warns', null); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.bans', null); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.kicks', null); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.timeouts', null); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.purges', null); | ||
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.locks', null); | ||
|
|
||
| expect(interaction.editReply).toHaveBeenCalledWith(expect.stringContaining('disabled')); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
setConfigValue call-count assertion is coupled to the current category list.
Line 105 asserts exactly 7 calls. If a new event category is added later, this test will fail even though the disable logic is correct. Consider deriving the expected count from the config mock's channel keys or a shared constant if one exists.
♻️ Possible resilient assertion
- expect(setConfigValue).toHaveBeenCalledTimes(7);
+ // Derive expected count from the mock config's channel keys
+ const channelKeys = Object.keys(
+ getConfig().moderation.logging.channels,
+ );
+ expect(setConfigValue).toHaveBeenCalledTimes(channelKeys.length);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('disable subcommand', () => { | |
| it('should clear all log channels', async () => { | |
| const interaction = createInteraction('disable'); | |
| await execute(interaction); | |
| expect(setConfigValue).toHaveBeenCalledTimes(7); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.default', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.warns', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.bans', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.kicks', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.timeouts', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.purges', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.locks', null); | |
| expect(interaction.editReply).toHaveBeenCalledWith(expect.stringContaining('disabled')); | |
| }); | |
| }); | |
| describe('disable subcommand', () => { | |
| it('should clear all log channels', async () => { | |
| const interaction = createInteraction('disable'); | |
| await execute(interaction); | |
| // Derive expected count from the mock config's channel keys | |
| const channelKeys = Object.keys( | |
| getConfig().moderation.logging.channels, | |
| ); | |
| expect(setConfigValue).toHaveBeenCalledTimes(channelKeys.length); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.default', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.warns', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.bans', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.kicks', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.timeouts', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.purges', null); | |
| expect(setConfigValue).toHaveBeenCalledWith('moderation.logging.channels.locks', null); | |
| expect(interaction.editReply).toHaveBeenCalledWith(expect.stringContaining('disabled')); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/commands/modlog.test.js` around lines 100 - 116, The test currently
hardcodes expect(setConfigValue).toHaveBeenCalledTimes(7) which couples it to
the current set of channel keys; change the assertion to derive the expected
call count from the source of truth (e.g., the mocked config channels object or
a shared constant) and assert dynamically — for example compute const
expectedCount = Object.keys(mockConfig['moderation.logging.channels']).length
(or import the categories constant) and use
expect(setConfigValue).toHaveBeenCalledTimes(expectedCount); also iterate those
keys to assert each specific setConfigValue was called with the corresponding
'moderation.logging.channels.<key>' and null, keeping existing checks for
interaction.editReply (references: setConfigValue, mock config object,
createInteraction, execute, interaction.editReply).
| const mockUser = { id: 'user1', tag: 'User#0001' }; | ||
| const mockMember = { | ||
| id: 'user1', | ||
| user: mockUser, | ||
| roles: { highest: { position: 5 } }, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Shared mockUser/mockMember at describe scope could leak mutations between tests.
These objects are defined once and reused across all tests. While vi.clearAllMocks() resets mock function call counts, it doesn't restore plain object properties. If a test were to mutate mockMember (e.g., reassign mockMember.roles), subsequent tests would see the mutation. Moving these into the createInteraction factory would be more resilient.
♻️ Suggested improvement
- const mockUser = { id: 'user1', tag: 'User#0001' };
- const mockMember = {
- id: 'user1',
- user: mockUser,
- roles: { highest: { position: 5 } },
- };
-
const createInteraction = () => {
+ const mockUser = { id: 'user1', tag: 'User#0001' };
+ const mockMember = {
+ id: 'user1',
+ user: mockUser,
+ roles: { highest: { position: 5 } },
+ };
+
return {
+ mockUser,
+ mockMember,
+ interaction: {
options: {Tests that need mockMember or mockUser can destructure them from createInteraction().
🤖 Prompt for AI Agents
In `@tests/commands/tempban.test.js` around lines 42 - 47, Remove the shared
mockUser and mockMember objects from the describe scope and instead instantiate
fresh mockUser and mockMember inside the existing createInteraction() factory
(or create a small factory helper within createInteraction) so each test gets a
new copy; update tests to destructure or pull mockUser/mockMember from
createInteraction() where they are used, and replace any direct references to
the old describe-scope mockUser/mockMember with the values returned by
createInteraction() (ensure references to roles.highest.position and user.tag/id
remain the same shape).
| const mockMember = { | ||
| id: 'user1', | ||
| user: { id: 'user1', tag: 'User#0001' }, | ||
| roles: { highest: { position: 5 } }, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Shared mockMember at describe scope — same pattern note as ban.test.js.
Consider aligning with the softban.test.js factory pattern that creates mockMember inside createInteraction to prevent potential state leakage in future tests.
🤖 Prompt for AI Agents
In `@tests/commands/warn.test.js` around lines 37 - 41, The shared mockMember
object at describe scope can leak state between tests; change the tests to
create a fresh mockMember inside a factory like createInteraction (as used in
softban.test.js) instead of sharing the module-scoped mockMember. Implement
createInteraction to construct a new mockMember with the same shape (id,
user.tag, roles.highest.position) each time and return the interaction object
that uses it, then update tests in warn.test.js to call createInteraction per
test so each test gets an isolated mockMember.
| describe('formatDuration', () => { | ||
| it('formats weeks', () => { | ||
| expect(formatDuration(604800000)).toBe('1 week'); | ||
| expect(formatDuration(1209600000)).toBe('2 weeks'); | ||
| }); | ||
|
|
||
| it('formats days', () => { | ||
| expect(formatDuration(86400000)).toBe('1 day'); | ||
| expect(formatDuration(172800000)).toBe('2 days'); | ||
| }); | ||
|
|
||
| it('formats hours', () => { | ||
| expect(formatDuration(3600000)).toBe('1 hour'); | ||
| expect(formatDuration(7200000)).toBe('2 hours'); | ||
| }); | ||
|
|
||
| it('formats minutes', () => { | ||
| expect(formatDuration(60000)).toBe('1 minute'); | ||
| expect(formatDuration(300000)).toBe('5 minutes'); | ||
| }); | ||
|
|
||
| it('formats seconds', () => { | ||
| expect(formatDuration(1000)).toBe('1 second'); | ||
| expect(formatDuration(30000)).toBe('30 seconds'); | ||
| }); | ||
|
|
||
| it('returns "0 seconds" for zero', () => { | ||
| expect(formatDuration(0)).toBe('0 seconds'); | ||
| }); | ||
|
|
||
| it('returns "0 seconds" for negative values', () => { | ||
| expect(formatDuration(-1000)).toBe('0 seconds'); | ||
| }); | ||
|
|
||
| it('returns "0 seconds" for non-number input', () => { | ||
| expect(formatDuration('abc')).toBe('0 seconds'); | ||
| expect(formatDuration(null)).toBe('0 seconds'); | ||
| }); | ||
|
|
||
| it('uses the largest fitting unit', () => { | ||
| expect(formatDuration(604800000)).toBe('1 week'); | ||
| expect(formatDuration(86400000)).toBe('1 day'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a test for durations that don't evenly fit a single unit.
formatDuration selects the largest unit where ms % unit.ms === 0. For a value like 90000 (1 min 30 sec), it would skip minutes (not evenly divisible) and return "90 seconds". A test documenting this behavior would clarify the contract and prevent regressions if the logic is later changed to support compound units.
Proposed additional test
it('falls back to smaller unit for non-exact multiples', () => {
// 1 minute 30 seconds = 90_000 ms; not evenly divisible by 60_000
expect(formatDuration(90000)).toBe('90 seconds');
});🤖 Prompt for AI Agents
In `@tests/utils/duration.test.js` around lines 95 - 138, Add a unit test to
tests/utils/duration.test.js that asserts formatDuration falls back to a smaller
unit when the value isn't an exact multiple of a larger unit (e.g., call
formatDuration(90000) and expect '90 seconds'); place it near the other
formatDuration tests (e.g., under 'uses the largest fitting unit') and reference
the formatDuration function so future changes that try to produce compound units
will be caught.
… (#83) * feat(dashboard): add Discord entity pickers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(dashboard): add diff view and polish - ConfigDiff.tsx: visual diff component showing before/after config changes with green additions and red deletions using the `diff` library - SystemPromptEditor.tsx: textarea with real-time character count, max length warning indicator, and accessible labeling - Toast notifications via sonner: success/error toasts on save, load failures, and reset actions positioned bottom-right - ResetDefaultsButton with confirmation dialog using Radix UI Dialog - ConfigEditor.tsx: full config editing page with AI, welcome message, and moderation sections; PATCH-based save with diff preview - Config API proxy route (GET/PATCH) following established analytics proxy pattern with guild admin authorization - Dialog UI component (shadcn/ui new-york style) - Added lint script to web/package.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * polish(config-editor): improve UX, accessibility, and edge case handling - Replace native checkboxes with styled toggle switches using proper role="switch" and aria-checked attributes - Add unsaved changes guard (beforeunload warning + yellow banner) - Add Ctrl/Cmd+S keyboard shortcut for saving - Block save when system prompt exceeds character limit - Rename misleading "Reset to Defaults" to "Discard Changes" with accurate dialog copy (reverts to last saved state, not factory defaults) - Add diff summary counts (+N / -N) to the pending changes card - Improve accessibility throughout: aria-labels on loading spinner, aria-describedby linking textareas to their hints, aria-invalid on over-limit prompt, aria-live on character counter, aria-hidden on decorative icons, role="region" on diff view - Memoize hasChanges and hasValidationErrors to avoid redundant JSON.stringify on every render - Validate PATCH body shape in API route before proxying upstream - Fix stale "bills-bot" prefix in guild-selection localStorage keys (missed during volvox rename) Closes #31 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(api): add config endpoints for Issue #31 Add REST API endpoints for managing bot configuration: - GET /api/v1/config - Retrieve current config (ai, welcome, spam, moderation) - PUT /api/v1/config - Update config with schema validation Features: - Type-safe schema validation for config sections - Flatten nested objects to dot-notation paths for persistence - requireGlobalAdmin middleware (API-secret or bot-owner OAuth) - Proper HTTP error codes (400 for validation, 401/403 for auth, 500 for errors) - Added PUT to CORS methods Tests: - 35 comprehensive tests covering auth, validation, types, edge cases - Tests for validateConfigSchema and flattenToLeafPaths exports Closes #31 * feat(api): add webhook notifications for config changes - Add notifyDashboardWebhook() fire-and-forget sender to PATCH /:id/config - POST /webhooks/config-update endpoint for dashboard to push config changes - Webhook uses DASHBOARD_WEBHOOK_URL env var with 5s timeout - Add comprehensive tests for webhook functionality * feat(dashboard): add config editor with Zustand state management Add a full config editor UI at /dashboard/config with: - Proxy API routes (GET + PATCH) for bot config at /api/guilds/:guildId/config - Zustand store for config state with fetch, update, and debounced saves - Accordion-based sections for ai, welcome, spam, moderation (read-only), triage - Recursive field renderer supporting booleans, numbers, strings, arrays, objects - shadcn/ui components: accordion, input, label, switch, textarea Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(dashboard): enhance Discord entity pickers with multi-select and Zustand - Add multi-select mode to ChannelSelector and RoleSelector (multiple?: boolean prop) - Create Zustand store for caching channels/roles per guild - Add dedicated bot API endpoints: GET /:id/channels and GET /:id/roles - Add Next.js proxy routes for channels and roles - Update AGENTS.md with new key files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove unused import in webhooks.js * fix: resolve all linting errors across codebase - Migrate biome config from 2.3.14 to 2.4.0 - Fix unused imports (triage.js, modAction.test.js) - Fix import ordering (auth.js, lock.js, unlock.js, ai.js, triage-respond.js, modAction.js, modAction.test.js) - Fix formatting across 19 files - Replace O(n²) spread in reduce with push (cli-process.test.js) - Use Object.hasOwn() instead of Object.prototype.hasOwnProperty (config-guild.test.js) All 1310 tests pass. * feat: add full config editing support for moderation and triage - Add moderation and triage to SAFE_CONFIG_KEYS in guilds.js, webhooks.js, and config.js making them writable via PATCH/PUT endpoints - Expand READABLE_CONFIG_KEYS to include all sections: ai, welcome, spam, moderation, triage, logging, memory, permissions - Add CONFIG_SCHEMA definitions for moderation and triage sections with full type validation - Update WritableConfigSection type to include moderation and triage - Remove moderation from READ_ONLY_SECTIONS in config-section.tsx - Update config-store.ts writable keys check - Add editable moderation section in dashboard config-editor with toggles for enabled, autoDelete, DM notifications, and escalation - Add editable triage section with fields for models, budgets, intervals, streaming, debug footer, and moderation log channel - Update all test assertions to reflect new writable sections Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): add webhook URL validation, schema validation on all write paths, atomic writes - Add shared validateWebhookUrl() utility that blocks SSRF (localhost, private IPs, link-local, IPv6 loopback) and enforces https in production - Wire URL validation into config.js notifyWebhook and guilds.js notifyDashboardWebhook - Export validateSingleValue() from config.js and apply it to the PATCH endpoint in guilds.js and the POST /config-update webhook endpoint - Add path length (<=200 chars) and depth (<=10 segments) limits to guilds.js PATCH and webhooks.js POST endpoints - Refactor PUT handler in config.js to track per-field write results: returns 200 on full success, 207 on partial failure, 500 when all fail - Add comprehensive tests for all new validations and error responses Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: extract shared config allowlist, webhook utility, and proxy helpers; remove dead code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(frontend): batch saves, fix race conditions, DRY constants, localStorage safety - Batch saveChanges into parallel PATCH requests grouped by section instead of sequential individual PATCHes (I5) - Add request sequence counter to Zustand config store to prevent stale PATCH responses from clobbering newer state (I6) - Centralize SYSTEM_PROMPT_MAX_LENGTH constant in types/config.ts and import in config-editor and system-prompt-editor (M2) - Wrap localStorage.getItem in try/catch for SSR safety (M3) - Fix channels.length / roles.length truthiness bug — use !== undefined instead of .length which is falsy for 0 (M5) - Replace JSON.stringify deep comparison with recursive deepEqual utility function (M8) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): mask sensitive config fields, block IPv4-mapped IPv6 SSRF, reject unknown config paths - Add SENSITIVE_FIELDS set and maskSensitiveFields utility to strip triage API keys (classifyApiKey, respondApiKey) from all GET config responses - Block SSRF via IPv4-mapped IPv6 addresses (::ffff:127.0.0.1, hex form ::ffff:7f00:1, and cloud metadata ::ffff:169.254.169.254) - Reject unknown config paths in validateSingleValue instead of silently accepting them without type checking - Add cache size limit (100 entries) to webhook URL validation cache - Guard flattenToLeafPaths against __proto__/constructor/prototype keys Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(backend): extract shared validators and getBotOwnerIds, add webhook utility tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(frontend): remove dead code, fix save flow, harden inputs and type guards - C4: delete 10 unused files (stores, UI components, dashboard selectors) and remove zustand, @radix-ui/react-accordion, @radix-ui/react-label, @radix-ui/react-switch from package.json - m8: replace ~85-line local GuildConfig interface with DeepPartial<BotConfig> (add DeepPartial utility to types/config.ts) - m4: harden isGuildConfig type guard to verify at least one expected section key (ai, welcome, spam) instead of just typeof === "object" - M6: fix computePatches to include top-level paths (remove incorrect fullPath.includes(".") guard that silently dropped top-level field changes) - M7: fix partial save to merge only succeeded sections into savedConfig on partial failure, preserving draft edits for failed sections; only call fetchConfig() on full success - m5: add min constraints to triage number inputs (budgets min=0, timeouts min=1, buffer/context sizes min=1) - m9: add e.returnValue = "" to beforeunload handler for modern browser support Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: fix 10-segment test to use valid schema path after strict validation * fix: wire default-personality.md as system prompt fallback Replace generic 'You are a helpful Discord bot.' fallback with the existing default-personality.md template, which provides the intended Volvox Bot personality, role constraints, and anti-abuse guardrails. * fix: merge default-personality into responder system prompt, remove dead template Merge personality, role, constraints, and anti-abuse from the unused default-personality.md into triage-respond-system.md (the responder's actual system prompt). Revert the fallback wiring in triage-prompt.js since personality now lives in the system prompt file where it belongs. Delete default-personality.md — no longer needed. * 📝 Add docstrings to `feat/config-editor-combined` Docstrings generation was requested by @BillChirico. The following files were modified: * `src/api/routes/auth.js` * `src/api/routes/config.js` * `src/api/routes/guilds.js` * `src/api/utils/configAllowlist.js` * `src/api/utils/validateConfigPatch.js` * `src/api/utils/validateWebhookUrl.js` * `src/commands/lock.js` * `src/commands/slowmode.js` * `src/commands/unlock.js` * `src/modules/memory.js` * `src/modules/triage-prompt.js` * `src/modules/triage-respond.js` * `src/modules/triage.js` * `src/utils/debugFooter.js` * `src/utils/permissions.js` * `web/src/app/api/guilds/[guildId]/channels/route.ts` * `web/src/app/api/guilds/[guildId]/config/route.ts` * `web/src/app/api/guilds/[guildId]/roles/route.ts` * `web/src/app/dashboard/config/page.tsx` * `web/src/components/dashboard/config-diff.tsx` * `web/src/components/dashboard/config-editor.tsx` * `web/src/components/dashboard/reset-defaults-button.tsx` * `web/src/components/dashboard/system-prompt-editor.tsx` * `web/src/components/providers.tsx` * `web/src/lib/bot-api-proxy.ts` These files were kept as they were: * `src/api/server.js` * `src/api/utils/webhook.js` These files were ignored: * `tests/api/routes/config.test.js` * `tests/api/routes/guilds.test.js` * `tests/api/routes/webhooks.test.js` * `tests/api/utils/validateWebhookUrl.test.js` * `tests/api/utils/webhook.test.js` * `tests/commands/tempban.test.js` * `tests/config-listeners.test.js` * `tests/modules/cli-process.test.js` * `tests/modules/config-guild.test.js` * `tests/utils/modAction.test.js` These file types are not supported: * `.env.example` * `AGENTS.md` * `biome.json` * `src/prompts/triage-respond-system.md` * `web/package.json` * fix(prompts): replace ambiguous 'classified' with 'triaged' (Thread #11) * fix(prompts): define concrete 'flagging' mechanism for moderation (Thread #12) * fix(prompts): add PII/credentials constraint to prevent echoing secrets (Thread #13) * fix: remove bracketed-IPv6 dead code in webhook URL validator URL.hostname already strips brackets from IPv6 addresses (new URL('http://[::1]').hostname === '::1'), so the hostname.startsWith('[') branch was unreachable dead code. Remove the bracketed-IPv6 branch and the '[::1]' entry from BLOCKED_IPV6 since it could never match. Addresses CodeRabbit review thread #9. * fix: sanitize webhook URL in warning logs to prevent credential exposure Strip query string and fragment from webhook URLs before including them in warning log messages. If a webhook URL contains tokens or API keys as query parameters, they would previously appear in logs. Now logs only origin + pathname (e.g. 'https://example.com/hook') instead of the full URL with sensitive query params. Addresses CodeRabbit review thread #10. * test(config): rename misleading 'Infinity' test name (Thread #14) * fix: normalize validateSingleValue to always return string[] Unknown config paths previously returned [{path, message}] objects while validateValue returned string[]. Callers now receive a consistent string[] in both cases, eliminating the need to handle two different shapes. * test(config): move PUT partial write handling inside config routes suite (Thread #15) * refactor: extract getGuildChannels helper to remove duplication GET /:id and GET /:id/channels shared identical channel-fetching loops with a duplicated MAX_CHANNELS constant. Extracted into a single getGuildChannels(guild) helper used by both handlers. * feat: include section (topLevelKey) in PATCH config webhook payload Downstream consumers of the DASHBOARD_WEBHOOK_URL can now use the 'section' field to selectively reload only the affected config section rather than refreshing the entire config. * fix: return promise in webhook config-update handler (async/await) The previous .then()/.catch() chain was not returned, so Express 5 could not auto-forward rejected promises to the error handler. Converted to async/await so errors propagate correctly. * feat: make JSON body size limit configurable via API_BODY_LIMIT env var Defaults to '100kb' when the env var is not set, preserving existing behaviour. Operators can now tune the limit without code changes. * test(validateWebhookUrl): strengthen cache eviction test to verify re-evaluation (Thread #16) * test(webhook): move vi.useRealTimers() to afterEach to prevent timer leak (Thread #17) * refactor: move CONFIG_SCHEMA/validateValue/validateSingleValue to utils/configValidation.js validateConfigPatch.js (utils) was importing validateSingleValue from routes/config.js — an inverted dependency. Created src/api/utils/configValidation.js as the canonical home for CONFIG_SCHEMA, validateValue, and validateSingleValue. - config.js now imports from ../utils/configValidation.js and re-exports validateSingleValue for backward compatibility with existing callers. - validateConfigPatch.js now imports from ./configValidation.js directly. * perf: split path string once in validateConfigPatchBody path.split('.') was called twice — once to extract topLevelKey and again for segments. Moved the single split to the top and derived topLevelKey from segments[0], avoiding the redundant allocation. * fix(#18): change lint script from tsc to biome check * fix(#19,#20): simplify params type; add PATCH body value check * fix(#21): add metadata export to config page * fix(#22): compute addedCount/removedCount inside useMemo loop * fix(#23,#24,#25,#26): tighten isGuildConfig; extract inputClasses; guard number inputs; rename DiscardChangesButton * fix(#27): change aria-live from polite to off on char counter * fix(#28): change Toaster theme from dark to system * fix(#29,#30): export BotApiConfig; return 504 on AbortError/TimeoutError * fix(#31): add one-time localStorage key migration from old key * fix(#32,#33,#34): SpamConfig JSDoc; collapse WritableConfigSection; fix SYSTEM_PROMPT_MAX_LENGTH JSDoc * fix: remove unused validateSingleValue import, fix biome formatting in config.js After the refactor, validateSingleValue is re-exported directly via 'export { } from' and no longer needs a local import binding. Also removed an extra blank line that biome flagged as a format error. * fix: add DNS resolution validation to prevent SSRF via DNS rebinding Add async validateDnsResolution() that resolves a webhook hostname via DNS and checks all resolved IPs against the existing blocked ranges before fetch. This closes the TOCTOU gap where a hostname could pass string-based validation then resolve to a private IP at request time (DNS rebinding attack). Changes: - Add validateDnsResolution() with resolve4/resolve6 checks - Integrate DNS check in fireAndForgetWebhook before fetch - Normalize IPv6 hostnames by stripping brackets (Node.js v22 retains them in URL.hostname, contrary to WHATWG spec) - Add comprehensive test coverage for DNS rebinding scenarios - Update webhook tests for async DNS validation flow Addresses CodeRabbit review thread #8. * fix: update test assertions for string[] return type from validateSingleValue * fix: mock validateDnsResolution in webhook integration tests After adding DNS resolution pinning in fireAndForgetWebhook, the config and guilds route tests need to mock validateDnsResolution to return true so fetch is actually called. * fix: address minor code review feedback - JSDoc, tests, caps * fix(frontend): address code review feedback - HTML, types, perf * fix(backend): address code review feedback - validation, logging, exports * fix: correct IPv6 validation for public addresses and literals * fix: restore classifier invocation in triage module * fix: address test failures in validateConfigPatch and triage-respond - Check for empty path segments before topLevelKey validation - Fix test to use valid nullable path (welcome.channelId) - Add mock cleanup between triage-respond tests * fix(validation): handle alertChannelId nullable and DNS edge cases * fix(security): prevent mask sentinel write-back and auth secret override 1. configAllowlist: Add isMasked() and stripMaskedWrites() to detect and filter out writes where sensitive fields contain the mask sentinel ('••••••••'). Prevents clients from accidentally overwriting real secrets with the placeholder returned by maskSensitiveFields(). 2. bot-api-proxy: Reorder header spread so x-api-secret is always set AFTER spreading options.headers, preventing caller-provided headers from overriding the server-side auth secret. Both fixes include comprehensive tests. * test: add missing test cases for mask sentinel, prototype pollution, DNS edge cases * refactor: simplify webhook validation for internal-only use * refactor: remove unused SSRF validation code Deleted validateWebhookUrl.js and its tests since webhooks are internal-only. Simplified webhook.js to just check URL format. * fix: prevent WebSearch notification failures from aborting response * fix: correct safeSend mock setup in triage-respond tests * fix(security): use own-property checks in config validation * fix: export MASK constant and clean up orphaned JSDoc * fix: report written sections in webhook, add roles endpoint test * fix: address remaining PR review feedback - Add nullable: true to triage.moderationLogChannel and debugFooterLevel - Add evalClient param to runResponder JSDoc - Convert SAFE_CONFIG_KEYS to Set for O(1) lookups - Reorder validation checks for consistent 400 responses - Update tests for Set-based SAFE_CONFIG_KEYS --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: AnExiledDev <AnExiledDev@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Summary
src/modules/moderation.js) — case creation, DM notifications, mod log embeds, auto-escalation, hierarchy checks, tempban schedulersrc/utils/duration.js) — parse "1h", "7d" ↔ ms with overflow guardsmod_casesandmod_scheduled_actionstables with indexes/modlog setupwith select menus for channel routing per event category.specs/v2.0.0/with as-built updatesStats
Test plan
pnpm test)pnpm lint)