feat: switch code quality setup to ultracite and biome#769
feat: switch code quality setup to ultracite and biome#769ahmadk953 wants to merge 15 commits intodevelopmentfrom
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
ahmadk953
added
enhancement
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces ESLint/Prettier with Ultracite/Biome across tooling, CI, and editor settings; adds Ultracite docs and standards; refactors DB/Redis table/type usage and many DB functions; centralizes logging handlers; introduces interaction validation/safe responses and refactors numerous commands, events, and utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "yarn check/fix"
participant Ultracite
participant Biome as "Biome (engine)"
participant Config as "biome.jsonc"
User->>CLI: run lint/format command
CLI->>Ultracite: invoke ultracite
Ultracite->>Config: load biome.jsonc
Config-->>Ultracite: return extensions/rules
Ultracite->>Biome: delegate analysis/fixes
Biome-->>Ultracite: return diagnostics/fixes
Ultracite->>CLI: report/apply fixes
CLI-->>User: display results
sequenceDiagram
participant Git
participant Husky
participant LintStaged
participant Ultracite
participant TSC as "TypeScript (tsc)"
Git->>Husky: pre-commit hook triggers
Husky->>LintStaged: run lint-staged
LintStaged->>Ultracite: run `yarn ultracite fix` on staged files
Ultracite-->>LintStaged: apply fixes
LintStaged->>TSC: run `yarn type-check` for staged .ts
TSC-->>LintStaged: return type results
LintStaged-->>Husky: finish
Husky-->>Git: allow or block commit
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Ultracite reportFound 0 issue(s): ERROR 0 | WARN 0 | NOTE 0 |
There was a problem hiding this comment.
Pull request overview
This PR migrates the repo’s code-quality toolchain from ESLint/Prettier to Ultracite (Biome backend), updating CI/editor/docs accordingly, and applies a large set of auto-format/refactor changes across the TypeScript codebase to match the new standards.
Changes:
- Replace ESLint/Prettier with Ultracite/Biome (new
biome.jsonc, new scripts, updated lint-staged/Husky, updated CI workflow). - Apply Biome-driven formatting and some small refactors across many
src/modules (logging, giveaways, counting, commands, db functions, etc.). - Add/expand Ultracite standards documentation and agent/editor hook configuration.
Reviewed changes
Copilot reviewed 108 out of 109 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/util/undeployCommands.ts | Formatting-only change (logger call punctuation/line breaks). |
| src/util/telemetry/otelLogTransport.ts | Refactors/formatting; extracts helper functions for OTEL attribute cleaning. |
| src/util/notificationHandler.ts | Replaces enum with as const object + type alias; formatting tweaks. |
| src/util/logging/utils.ts | Type/loop refactors (for...of), tweaks to channel field typing, import order changes. |
| src/util/logging/types.ts | Reorders interface members/types for consistency with formatter. |
| src/util/logging/handlers/roleHandlers.ts | New logging handler module for role-related audit log embeds. |
| src/util/logging/handlers/purgeHandler.ts | New purge transcript generation + attachment send helper. |
| src/util/logging/handlers/moderationHandlers.ts | New handler module for moderation/counting log embed fields. |
| src/util/logging/handlers/messageHandlers.ts | New handler module for message delete/edit embed fields and link button. |
| src/util/logging/handlers/memberHandlers.ts | New handler module for member join/leave + name-change embed fields. |
| src/util/logging/handlers/channelHandlers.ts | New handler module for channel update/create/delete embed fields (incl. perm diffs). |
| src/util/logging/constants.ts | Formatting + numeric separator style for embed colors. |
| src/util/logger.ts | Refactors fatal error handling/transport flush logic; formatting updates. |
| src/util/levelingSystem.ts | Imports/schema usage cleanup, formatting, minor TS typing adjustments. |
| src/util/giveaways/utils.ts | Formatting + small control-flow refactors in requirements/session helpers. |
| src/util/giveaways/types.ts | Reorders interface members to match formatter rules. |
| src/util/giveaways/modals.ts | Formatting-only changes (trailing commas/line breaks). |
| src/util/giveaways/dropdowns.ts | Formatting-only changes (trailing commas/line breaks). |
| src/util/giveaways/builder.ts | Formatting + minor boolean expression refactors and numeric separators. |
| src/util/factManager.ts | Changes scheduling function signature + formatting updates. |
| src/util/eventLoader.ts | Import ordering + formatting tweaks. |
| src/util/deployCommand.ts | Formatting + minor control-flow simplification; numeric separators; import ordering. |
| src/util/counting/types.ts | Reorders interface members to match formatter rules. |
| src/util/counting/helpers.ts | Formatting + adds top-level regex constants and refactors some logic structure. |
| src/util/counting/durationUtils.ts | Formatting-only changes. |
| src/util/counting/constants.ts | Formatting-only changes. |
| src/util/configLoader.ts | Import ordering + formatting tweaks. |
| src/util/achievementManager.ts | Formatting + minor control-flow refactors; safer parseInt usage. |
| src/util/achievementCardGenerator.ts | Import ordering + formatting tweaks. |
| src/types/EventTypes.ts | Reorders interface members. |
| src/types/ConfigTypes.ts | Reorders interface members (no semantic change intended). |
| src/structures/ExtendedClient.ts | Makes some properties readonly; formatting tweaks. |
| src/events/roleEvents.ts | Import ordering tweak. |
| src/events/ready.ts | Import ordering + removes await on sync redis connect + fact scheduling call update. |
| src/events/reactionEvents.ts | Import ordering + early return formatting. |
| src/events/memberEvents.ts | Import ordering + early return formatting. |
| src/events/interactionCreate.ts | Refactors control flow, introduces constant regex, formatting updates. |
| src/events/channelEvents.ts | Refactors permission change detection helpers and formatting. |
| src/discord-bot.ts | Formatting and minor control-flow brace changes. |
| src/db/schema.ts | Import ordering + interface member ordering + formatting. |
| src/db/functions/utils/moderationUtils.ts | Formatting + == null simplification. |
| src/db/functions/moderationFunctions.ts | Switch from schema.* namespace usage to named imports; formatting. |
| src/db/functions/memberFunctions.ts | Switch from schema.* namespace usage to named imports; formatting. |
| src/db/functions/factFunctions.ts | Switch from schema.* namespace usage to named imports; formatting/structure. |
| src/db/functions/achievementFunctions.ts | Switch from schema.* namespace usage to named imports; formatting. |
| src/commands/util/xp.ts | Formatting + early-return braces + minor string formatting changes. |
| src/commands/util/server.ts | Formatting + early-return braces. |
| src/commands/util/rules.ts | Formatting + numeric separators for colors. |
| src/commands/util/restart.ts | Import ordering + early-return braces + formatting. |
| src/commands/util/reload-config.ts | Import ordering + early-return braces + numeric separators for colors. |
| src/commands/util/recalculate-levels.ts | Early-return braces + formatting. |
| src/commands/util/ping.ts | Formatting-only change. |
| src/commands/util/members.ts | Refactors pagination handler logic slightly; formatting + numeric separators. |
| src/commands/util/help.ts | Refactors category rendering loops + changes handleSpecificCommand call style. |
| src/commands/util/backend-manager.ts | Formatting + adds default subcommand handling; minor status color refactor. |
| src/commands/testing/test-leave.ts | Early-return braces + formatting. |
| src/commands/testing/test-join.ts | Early-return braces + formatting. |
| src/commands/moderation/warn.ts | Import ordering + early-return braces + formatting. |
| src/commands/moderation/unmute.ts | Import ordering + early-return braces + formatting. |
| src/commands/moderation/unban.ts | Import ordering + early-return braces + formatting. |
| src/commands/moderation/mute.ts | Import ordering + early-return braces + formatting. |
| src/commands/moderation/kick.ts | Import ordering + early-return braces + formatting. |
| src/commands/moderation/ban.ts | Import ordering + early-return braces + formatting. |
| src/commands/fun/rank.ts | Import ordering + early-return braces + formatting. |
| src/commands/fun/leaderboard.ts | Refactors pagination handling functions; formatting + numeric separators. |
| src/commands/fun/giveaway.ts | Import ordering + adds default subcommand handling; formatting + numeric separators. |
| skills-lock.json | Adds Ultracite “skills” lock metadata. |
| package.json | Replaces lint/format scripts with check/fix/type-check; swaps deps to Biome/Ultracite. |
| eslint.config.mjs | Removes ESLint flat config (migration away from ESLint). |
| drizzle.config.ts | Formatting-only changes. |
| biome.jsonc | Adds Biome config extending Ultracite preset + import organization settings. |
| README.md | Updates developer commands to check/fix/type-check. |
| CONTRIBUTING.md | Updates contribution workflow docs for Ultracite/Biome and new scripts. |
| AGENTS.md | Adds Ultracite standards reference document. |
| .prettierrc.yml | Removes Prettier config. |
| .prettierignore | Removes Prettier ignore file. |
| .lintstagedrc.mjs | Migrates lint-staged tasks to Ultracite + separate TS type-check trigger. |
| .husky/commit-msg | Updates commit-msg hook command invocation. |
| .github/workflows/ultracite.yml | Adds CI workflow for Ultracite/Biome check + SARIF upload + PR comment. |
| .github/workflows/eslint.yml | Removes ESLint workflow. |
| .github/pull_request_template.md | Updates testing checklist to new scripts. |
| .github/hooks/ultracite.json | Adds post-tool-use hook to auto-run yarn fix (Ultracite). |
| .github/copilot-instructions.md | Updates documented workflow commands and adds Ultracite standards reference. |
| .github/PULL_REQUEST_TEMPLATE/refactor.md | Updates template checklists for new scripts. |
| .github/PULL_REQUEST_TEMPLATE/feature.md | Updates template checklists for new scripts. |
| .github/PULL_REQUEST_TEMPLATE/bug_fix.md | Updates template checklists for new scripts. |
| .devcontainer/devcontainer.json | Switches formatter/extensions to Biome + updates save actions. |
| .devcontainer/README.md | Updates devcontainer tool description to Ultracite/Biome. |
| .agents/skills/ultracite/references/code-standards.md | Adds detailed Ultracite/Biome code standards reference. |
| .agents/skills/ultracite/SKILL.md | Adds Ultracite skill documentation for agents/tools. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 109 files
Confidence score: 2/5
- There are multiple high-confidence, user-impacting risks, especially in
src/commands/fun/fact.tswhere using globalMAX(id)is race-prone and can attach moderation buttons to the wrong fact under concurrent submissions. src/events/messageEvents.tsandsrc/util/counting/countingManager.tsintroduce concrete counting correctness issues (event-order reordering and invalid non-numeric entries being silently ignored), which can cause visible behavior regressions.- Operational safety is also weakened by logging raw message content in
src/util/counting/countingManager.tsand allowing Biome failures to be ignored in.github/workflows/ultracite.yml, increasing privacy and quality-gate risk. - Pay close attention to
src/commands/fun/fact.ts,src/events/messageEvents.ts,src/util/counting/countingManager.ts, and.github/workflows/ultracite.yml- race conditions, counting-state integrity, sensitive logging, and CI enforcement are the main merge risks.
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/util/counting/countingManager.ts">
<violation number="1" location="src/util/counting/countingManager.ts:197">
P1: Custom agent: **Avoid Logging Sensitive Information**
Do not log raw message content; it can contain sensitive user data. Log only non-sensitive metadata instead.</violation>
<violation number="2" location="src/util/counting/countingManager.ts:200">
P1: Non-numeric counting messages are marked as `ignored`, which bypasses the invalid-number reset path and lets invalid entries pass silently.</violation>
</file>
<file name="src/events/messageEvents.ts">
<violation number="1" location="src/events/messageEvents.ts:321">
P1: Awaiting leveling before enqueuing counting messages can reorder counting processing under concurrent `messageCreate` events. Keep leveling non-blocking so queue insertion happens immediately in event order.</violation>
</file>
<file name=".lintstagedrc.mjs">
<violation number="1" location=".lintstagedrc.mjs:2">
P2: Build the lint-staged command without manually joining raw filenames; unquoted filenames can break on spaces/special characters.</violation>
</file>
<file name="src/commands/fun/fact.ts">
<violation number="1" location="src/commands/fun/fact.ts:218">
P1: Fetching the new fact ID via global `MAX(id)` is race-prone and can link moderation buttons to the wrong fact under concurrent submissions.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:20">
P3: `husky` is configured in both `postinstall` and `prepare`, which duplicates hook installation on dependency install.</violation>
</file>
<file name="src/commands/util/help.ts">
<violation number="1" location="src/commands/util/help.ts:50">
P2: Await the `handleSpecificCommand` promise so reply errors are caught by this command’s `try/catch` instead of becoming unhandled rejections.</violation>
</file>
<file name=".devcontainer/devcontainer.json">
<violation number="1" location=".devcontainer/devcontainer.json:70">
P2: YAML files are configured to use Biome as formatter even though Biome YAML support is not fully supported/stable; this can break format-on-save for YAML configs.</violation>
</file>
<file name=".github/workflows/ultracite.yml">
<violation number="1" location=".github/workflows/ultracite.yml:40">
P1: Do not ignore Biome CI failures; this allows the quality workflow to pass even when Biome reports error-level violations.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Dev as Developer
participant IDE as VS Code (Biome Ext)
participant Git as Git Hooks (Husky)
participant Ultra as Ultracite CLI (Biome)
participant CI as GitHub Actions
participant GH as GitHub PR UI
Note over Dev, IDE: Local Development Flow
Dev->>IDE: Save changed file
IDE->>Ultra: NEW: Format and fix (source.fixAll.biome)
Ultra-->>IDE: Applied fixes
IDE-->>Dev: Clean code in editor
Note over Dev, Ultra: Pre-commit Flow (Staged Files)
Dev->>Git: git commit
Git->>Ultra: CHANGED: yarn ultracite fix
Ultra->>Ultra: Format & Lint staged files
alt NEW: Files are .ts
Git->>Ultra: NEW: yarn type-check (tsc --noEmit)
end
Ultra-->>Git: Success/Fail
Git-->>Dev: Commit accepted/rejected
Note over Git, GH: Continuous Integration Flow
Dev->>GH: Push to branch / Open PR
GH->>CI: Trigger Ultracite Workflow
CI->>Ultra: CHANGED: yarn check
Ultra-->>CI: Lint/Format results (SARIF)
alt NEW: PR Reporting
CI->>GH: NEW: Post summary comment to PR
CI->>GH: NEW: Upload SARIF to Code Scanning
end
alt CHANGED: Lint/Format Errors
Ultra-->>CI: Exit Code 1
CI-->>GH: Status Check: FAILED
else Success
Ultra-->>CI: Exit Code 0
CI-->>GH: Status Check: PASSED
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
src/util/deployCommand.ts (1)
82-99:⚠️ Potential issue | 🟡 MinorValidate and migrate persisted deploy-state entries before casting.
readDeployStatereturnsparsed as DeployStatewithout validating entry shape. Older state files (withoutcommandCount) will deserialize successfully but violate the newDeployStateEntrycontract and can leakundefinedmetadata.Proposed fix
const readDeployState = (): DeployState => { @@ - return parsed as DeployState; + const normalizedEntries = Object.entries(parsed).flatMap(([key, value]) => { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return []; + } + + const maybe = value as Partial<DeployStateEntry>; + if (typeof maybe.hash !== 'string' || typeof maybe.updatedAt !== 'string') { + return []; + } + + return [[ + key, + { + hash: maybe.hash, + updatedAt: maybe.updatedAt, + commandCount: + typeof maybe.commandCount === 'number' ? maybe.commandCount : 0, + } satisfies DeployStateEntry, + ] as const]; + }); + + return Object.fromEntries(normalizedEntries) as DeployState;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/deployCommand.ts` around lines 82 - 99, readDeployState currently casts parsed JSON directly to DeployState, which allows older/malformed entries (missing DeployStateEntry fields like commandCount) to slip through; update readDeployState to iterate the parsed object entries, validate each value is an object, ensure required fields (e.g., commandCount) are present (initialize defaults such as commandCount = 0 and normalize metadata), discard or log malformed entries via logger.warn, and return the reconstructed DeployState map so the returned type conforms to DeployState/DeployStateEntry and prevents undefined metadata from leaking.src/commands/fun/leaderboard.ts (1)
104-123:⚠️ Potential issue | 🟠 MajorGuard against select-menu option overflow (>25 pages).
With
getLevelLeaderboard(100)andusersPerPageconfigurable as low as 1 (via thelimitoption), pages can reach 100, exceeding Discord's 25-option select menu limit. This breaks the dropdown at lines 122 and 190.Conditionally exclude the select menu when
pages.length > 25, falling back to button-only navigation:Suggested fix
+ const shouldShowSelectMenu = pages.length > 1 && pages.length <= 25; const components = - pages.length > 1 ? [getButtonActionRow(), getSelectMenuRow()] : []; + pages.length > 1 + ? shouldShowSelectMenu + ? [getButtonActionRow(), getSelectMenuRow()] + : [getButtonActionRow()] + : []; @@ await i.update({ embeds: [pages[currentPage]], - components: [getButtonActionRow(), getSelectMenuRow()], + components: shouldShowSelectMenu + ? [getButtonActionRow(), getSelectMenuRow()] + : [getButtonActionRow()], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/leaderboard.ts` around lines 104 - 123, The select menu can exceed Discord's 25-option limit when pages.length > 25; update the logic around getSelectMenuRow, pages, and components so you only build/add the StringSelectMenuBuilder if pages.length <= 25 (otherwise omit it and rely on getButtonActionRow for navigation); ensure getSelectMenuRow checks pages.length and components is computed as pages.length > 1 ? (pages.length <= 25 ? [getButtonActionRow(), getSelectMenuRow()] : [getButtonActionRow()]) : [] and apply the same guard wherever the select is recreated/used (e.g., the code paths that rebuild components later such as the handler referencing getSelectMenuRow).src/commands/util/reload-config.ts (1)
78-89:⚠️ Potential issue | 🟠 MajorDon't send the raw reload exception back to Discord.
errorcan include parser internals, file paths, and config values. Keep the detailed exception in logs and return a generic admin-facing failure message here.Suggested fix
const errorEmbed = new EmbedBuilder() .setTitle('❌ Configuration Reload Failed') .setColor(0xff_00_00) - .setDescription( - `Failed to reload configuration from disk:\n\`\`\`${error}\`\`\`` - ) + .setDescription( + 'Failed to reload configuration from disk. Check the server logs for details.' + ) .setTimestamp();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/reload-config.ts` around lines 78 - 89, The current error handling in reload-config.ts builds errorEmbed with the raw `error` and sends it via `interaction.editReply`; instead log the full `error` details (e.g., using your logger or console.error) and change the EmbedBuilder description to a generic admin-facing message like "Failed to reload configuration. Check server logs for details." before calling `interaction.editReply` so no parser internals, file paths, or config values are exposed to Discord.src/util/giveaways/utils.ts (1)
40-44:⚠️ Potential issue | 🟠 MajorThe current winner shuffle is biased and unfair for giveaway selection.
The comparator
sort(() => 0.5 - Math.random())does not produce a uniform permutation. This violates the comparator consistency contract and results in some participants being favored over others—a critical issue for fairness-dependent giveaway selection. Use Fisher-Yates instead.Suggested fix
- const shuffled = uniqueParticipants.sort(() => 0.5 - Math.random()); - return shuffled.slice(0, actualWinnerCount); + const shuffled = [...uniqueParticipants]; + for (let i = shuffled.length - 1; i > 0; i -= 1) { + const j = Math.floor(Math.random() * (i + 1)); + [shuffled[i], shuffled[j]] = [shuffled[j], shuffled[i]]; + } + return shuffled.slice(0, actualWinnerCount);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/giveaways/utils.ts` around lines 40 - 44, The current unbiased issue: replace the unsafe sort(() => 0.5 - Math.random()) approach (used on uniqueParticipants to produce shuffled and then slice by actualWinnerCount) with a proper Fisher‑Yates shuffle to produce a uniform random permutation; implement a function or inline logic that copies uniqueParticipants (to avoid mutating original if needed), performs an in-place Fisher‑Yates swap loop to shuffle the array, then return the first actualWinnerCount entries from that shuffled array (references: uniqueParticipants, actualWinnerCount, shuffled).src/commands/util/backend-manager.ts (1)
145-166:⚠️ Potential issue | 🟠 MajorThis reconnect path can report a false failure.
ensureRedisConnection()now only starts the reconnect attempt; it does not wait for Redis to be ready. CheckingisRedisConnected()immediately after it will usually still befalse, so/backend-manager rediscan tell admins the reconnect failed even when Redis comes back seconds later. Either wait for a concrete readiness check/ping, or phrase the response as “reconnect initiated.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/backend-manager.ts` around lines 145 - 166, The reconnect handler currently calls ensureRedisConnection() then immediately checks isRedisConnected(), which can report a false failure because ensureRedisConnection() merely starts async reconnecting; update handleRedisReconnect to either (A) await a concrete readiness check (e.g., call a new or existing ping/waitForReady function on the Redis client and only then call isRedisConnected()) so the success/failure messages reflect actual connectivity, or (B) change the replies to indicate the reconnect was initiated (e.g., "Reconnect initiated; will report when connection is restored") and avoid declaring failure immediately; reference handleRedisReconnect, ensureRedisConnection, and isRedisConnected when making the change.src/db/functions/giveawayFunctions.ts (1)
180-198:⚠️ Potential issue | 🟠 MajorAvoid read-modify-write on
participants.This path reads the participant array, mutates it in memory, and writes it back outside a transaction. Two users joining at the same time can read the same old array and the later update will overwrite the earlier one, silently dropping entries. Move the duplicate check + append into a transaction or an atomic database update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/functions/giveawayFunctions.ts` around lines 180 - 198, The current getGiveaway -> read/modify/write flow (involving getGiveaway, participants, db.update and giveawayTable) is racy: replace it with an atomic operation or run the duplicate check + append inside a transaction so concurrent joins cannot clobber each other; specifically, perform the existence/status check and the "already_entered" check and the append of `userId` (repeated `entries` times) inside a db.transaction or use a single atomic DB update that conditionally appends to the participants array (e.g., using the DB/ORM's array append/concat operator with a WHERE that ensures status='active' and the user isn't already present), and return appropriate results when the update affects zero rows to indicate inactive/already_entered.src/db/functions/moderationFunctions.ts (1)
73-109:⚠️ Potential issue | 🟡 MinorInconsistent error handling:
ensureDbInitialized()is outside the try-catch block.In
getMemberModerationHistory,ensureDbInitialized()(line 76) and the subsequentdbcheck are outside the try-catch, unlikeupdateMemberModerationHistorywhich wraps them. IfensureDbInitialized()throws, it won't be caught and handled byhandleDbError.🛠️ Proposed fix to wrap initialization in try-catch
export async function getMemberModerationHistory( discordId: string ): Promise<moderationTableTypes[]> { - await ensureDbInitialized(); - - if (!db) { - logger.error( - '[moderationDbFunctions] Database not initialized, cannot get member moderation history' - ); - throw new Error('Database not initialized'); - } - - const cacheKey = `${discordId}-moderationHistory`; - try { + await ensureDbInitialized(); + + if (!db) { + logger.error( + '[moderationDbFunctions] Database not initialized, cannot get member moderation history' + ); + throw new Error('Database not initialized'); + } + + const cacheKey = `${discordId}-moderationHistory`; + const moderationHistory = await withCache<moderationTableTypes[]>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/functions/moderationFunctions.ts` around lines 73 - 109, The function getMemberModerationHistory currently calls ensureDbInitialized() and checks db outside of the try/catch, so initialization errors bypass handleDbError; move the ensureDbInitialized() call and the subsequent db existence check inside the try block that surrounds the cache/db operations (or wrap the whole function body in a try/catch), so any errors from ensureDbInitialized() or the db check are caught and passed to handleDbError('Failed to get moderation history', error); keep the rest of the logic (cacheKey, withCache, withDbRetryDrizzle, and normalizeModerationDates) unchanged and mirror the error handling pattern used by updateMemberModerationHistory.src/commands/moderation/ban.ts (1)
72-77:⚠️ Potential issue | 🟡 MinorFix the indefinite-ban DM copy.
The string at Line 76 says "You been indefinitely banned...", which will be sent verbatim to users.
💬 Suggested copy fix
- : `You been indefinitely banned from ${guild.name}. Reason: ${reason}.` + : `You have been indefinitely banned from ${guild.name}. Reason: ${reason}.`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/moderation/ban.ts` around lines 72 - 77, In the ban DM send logic inside src/commands/moderation/ban.ts (the member.user.send call), fix the indefinite-ban message branch so it reads correctly (e.g., "You have been indefinitely banned from ${guild.name}. Reason: ${reason}.") instead of the current "You been indefinitely banned..."; update only the string used when banDuration is falsy in the ternary so grammar is corrected and punctuation stays consistent with the temporary-ban message.src/discord-bot.ts (1)
9-19:⚠️ Potential issue | 🟠 MajorMake the fallback logger non-throwing.
JSON.stringifystill throws on circular objects andBigInt. If startup fails with one of those payloads, this fallback path can crash while handling the original error, which defeats the whole point of having a last-resort logger.🛟 Suggested hardening
+const safeSerialize = (value: unknown): string => { + if (value instanceof Error) { + return value.stack ?? value.message; + } + if (typeof value === 'bigint') { + return value.toString(); + } + if (value && typeof value === 'object') { + try { + return JSON.stringify(value); + } catch { + return '[unserializable object]'; + } + } + return String(value); +}; + const _fallbackLogger = { log: (level: string, message?: unknown, ...meta: unknown[]) => { const prefix = level ? `[${level}]` : '[log]'; - const parts = [prefix, message]; + const parts = [prefix]; + if (typeof message !== 'undefined') { + parts.push(safeSerialize(message)); + } if (meta?.length) { - parts.push( - ...meta.map((m) => - typeof m === 'object' ? JSON.stringify(m) : String(m) - ) - ); + parts.push(...meta.map(safeSerialize)); } process.stderr.write(`${parts.join(' ')}\n`); }, error: (message?: unknown, ...meta: unknown[]) => { - const parts = [message]; + const parts: string[] = []; + if (typeof message !== 'undefined') { + parts.push(safeSerialize(message)); + } if (meta?.length) { - parts.push( - ...meta.map((m) => - typeof m === 'object' ? JSON.stringify(m) : String(m) - ) - ); + parts.push(...meta.map(safeSerialize)); } process.stderr.write(`${parts.join(' ')}\n`); }, };Also applies to: 21-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/discord-bot.ts` around lines 9 - 19, The fallback logger function log currently uses JSON.stringify which can throw on circular structures or BigInt; wrap the serialization in a try/catch inside the log function (and any sibling fallback logger block at lines 21-30) so that if JSON.stringify fails you fall back to a safe serializer: convert BigInt to string and replace circular references or non-serializable values with a safe placeholder (or simply call String(value)) and ensure the catch branch builds parts without throwing; finally always call process.stderr.write with the assembled string so the logger never throws while handling errors.src/util/giveaways/handlers.ts (1)
317-330:⚠️ Potential issue | 🟡 MinorInconsistent session expiry handling.
Unlike other handlers that use
getSessionOrReplyExpired, this function usesgetSessiondirectly and silently returns if the session is null. This means the user won't be notified that their session expired.🔧 Proposed fix for consistency
export async function handlePingRoleSelect( interaction: StringSelectMenuInteraction ): Promise<void> { const roleId = interaction.values[0]; - const session = await getSession(interaction.user.id); + const session = await getSessionOrReplyExpired(interaction); - if (!session) { - return; - } + if (!session) { + return; + } session.pingRoleId = roleId; await saveSession(interaction.user.id, session); await showBuilderStep(interaction, session); }Note: This requires changing the function signature slightly since
getSessionOrReplyExpiredexpects aBuilderInteractiontype. You may need to extend theBuilderInteractiontype alias to includeStringSelectMenuInteraction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/giveaways/handlers.ts` around lines 317 - 330, handlePingRoleSelect currently calls getSession and silently returns on null, causing no user feedback on expired sessions; change it to use getSessionOrReplyExpired so the user is notified. Update the handlePingRoleSelect signature to accept the BuilderInteraction type (or expand the BuilderInteraction type alias to include StringSelectMenuInteraction), then replace the getSession call with await getSessionOrReplyExpired(interaction) and early-return only when that helper indicates expiration; preserve the existing logic that sets session.pingRoleId, calls saveSession and then showBuilderStep.src/util/counting/helpers.ts (1)
57-68:⚠️ Potential issue | 🟠 MajorBackfill
totalCorrectduring migration.
totalCorrectwas added to the migrated shape, but this function never marks the payload as changed when that field is missing, andfinalDatastill falls back todata.totalCorrect. For legacy Redis blobs, the next successful count turnsundefined += 1intoNaN, so the stat stays corrupted.💡 Minimal fix
if ( !mutableData.mistakeTracker || typeof mutableData.mistakeTracker !== 'object' || Array.isArray(mutableData.mistakeTracker) ) { mutableData.mistakeTracker = {}; changed = true; } + + if ( + typeof mutableData.totalCorrect !== 'number' || + !Number.isFinite(mutableData.totalCorrect) || + mutableData.totalCorrect < 0 + ) { + mutableData.totalCorrect = 0; + changed = true; + } if (changed) { const finalData: CountingData = { @@ - totalCorrect: - typeof mutableData.totalCorrect === 'number' - ? mutableData.totalCorrect - : data.totalCorrect, + totalCorrect: mutableData.totalCorrect,Also applies to: 101-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/counting/helpers.ts` around lines 57 - 68, The migrateData function adds totalCorrect to the ExpectedCountingData shape but never treats a missing totalCorrect as a migration change, leaving finalData to fall back to the original data.totalCorrect (which can be undefined and later produce NaN). Update migrateData so that when data.totalCorrect is missing or not a number you set the migrated copy's totalCorrect to 0 and set the same "changed" flag used elsewhere in this function to true so finalData is taken from the validated/mutated copy (refer to the migrateData function, the ExpectedCountingData interface, the local mutated copy and the finalData/changing logic).src/util/giveaways/builder.ts (1)
102-117:⚠️ Potential issue | 🟠 MajorCatch block must handle deferred interactions with
editReply().After
startGiveawayBuilder()defers the interaction (line 24), the error handler at line 107 checks only!interaction.repliedbefore callinginteraction.reply(). In the deferred state, this call fails silently, leaving the user without error feedback.💡 Correct fallback
} catch (error) { logger.error( '[GiveawayManager] Error displaying giveaway builder step', error ); + if (interaction.deferred) { + try { + await interaction.editReply({ + content: 'There was an error updating the giveaway builder.', + embeds: [], + components: [], + }); + } catch (replyError) { + logger.error( + '[GiveawayManager] Failed to send error reply', + replyError + ); + } + } else if (!interaction.replied) { try { await interaction.reply({ content: 'There was an error updating the giveaway builder.', flags: ['Ephemeral'], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/giveaways/builder.ts` around lines 102 - 117, The catch in startGiveawayBuilder's error handling only checks interaction.replied and attempts interaction.reply(), which fails for deferred interactions; change the fallback to first check interaction.deferred (or interaction.replied) and call interaction.editReply() when the interaction was deferred (or already replied), otherwise call interaction.reply(); update the error branch around logger.error('[GiveawayManager] Error displaying giveaway builder step', error) and the nested reply attempt so it uses editReply for deferred interactions and preserves the existing try/catch/logger.error('[GiveawayManager] Failed to send error reply', replyError) behavior.
🟠 Major comments (23)
src/util/undeployCommands.ts-18-20 (1)
18-20:⚠️ Potential issue | 🟠 MajorUse structured metadata in log calls.
Line 19 logs a plain string, which makes filtering and correlation harder. Please include metadata (at minimum
guildId/clientId) and apply the same pattern to the other logger calls in this file.🔧 Suggested change
- logger.info( - '[UndeployCommands] Undeploying all commands from the Discord API...' - ); + logger.info('[UndeployCommands] Undeploying all commands from the Discord API...', { + guildId, + clientId, + });As per coding guidelines: "Use structured logging with metadata objects:
logger.info('message', { userId, commandName, guildId })and import logger fromsrc/util/logger.js."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/undeployCommands.ts` around lines 18 - 20, The log call in undeployCommands.ts uses a plain string; update logger calls (starting with the logger.info('[UndeployCommands] Undeploying all commands...') invocation) to use structured logging by passing a metadata object (at minimum { guildId, clientId }) as the second argument and apply this pattern to every other logger.* call in this file (e.g., any logger.info, logger.warn, logger.error occurrences); ensure the file imports the standard logger from src/util/logger.js and include those identifiers (guildId/clientId) available in the surrounding scope when constructing the metadata object.drizzle.config.ts-21-25 (1)
21-25:⚠️ Potential issue | 🟠 MajorFail fast in production when TLS cert loading fails.
Returning
undefinedforsslsilently downgrades DB connectivity to insecure mode. Keep this fallback only for local/dev, and throw in production.Suggested hardening
} catch (error) { + if (process.env.NODE_ENV === 'production') { + throw new Error( + 'Missing database TLS certificate at ./certs/rootCA.pem; refusing insecure connection.' + ); + } console.warn( 'Failed to load certificates for database, using insecure connection:', error ); return undefined; }Based on learnings, production requires TLS certificate in
certs/rootCA.pem.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@drizzle.config.ts` around lines 21 - 25, The current ssl certificate load block (where console.warn is called and the code returns undefined) silently falls back to insecure DB connections; update it so that when reading certs/rootCA.pem fails you only fallback to insecure mode in local/dev (e.g., NODE_ENV === 'development' or a dedicated DEV flag) but in production (process.env.NODE_ENV === 'production') throw an Error instead of returning undefined; locate the ssl-loading logic (the block that logs "Failed to load certificates for database, using insecure connection:" and the surrounding ssl variable/return) and change the control flow to throw in production with a clear message about missing certs/rootCA.pem while preserving the existing non-throwing warning/fallback for development.src/commands/util/recalculate-levels.ts-13-15 (1)
13-15:⚠️ Potential issue | 🟠 MajorUse shared interaction helpers in
execute(Line 13 onward).Please replace the manual guard with
await validateInteraction(interaction)and route replies throughsafelyRespond(...)for consistent interaction/error handling.Suggested patch
import { PermissionFlagsBits, SlashCommandBuilder } from 'discord.js'; import type { Command } from '@/types/CommandTypes.js'; +import { safelyRespond, validateInteraction } from '@/util/helpers.js'; import { recalculateUserLevels } from '@/util/levelingSystem.js'; import { logger } from '@/util/logger.js'; @@ execute: async (interaction) => { - if (!(interaction.isChatInputCommand() && interaction.guild)) { + if (!(await validateInteraction(interaction))) { return; } await interaction.deferReply({ flags: ['Ephemeral'] }); - await interaction.editReply('Recalculating levels...'); + await safelyRespond(interaction, 'Recalculating levels...'); @@ await recalculateUserLevels(); - await interaction.editReply('Levels recalculated successfully!'); + await safelyRespond(interaction, 'Levels recalculated successfully!'); @@ - await interaction.editReply('Failed to recalculate levels.'); + await safelyRespond(interaction, 'Failed to recalculate levels.'); } }, };As per coding guidelines: "Call
await validateInteraction(interaction)before processing interactions" and "UsesafelyRespond(interaction, content)fromsrc/util/helpers.tsfor all interaction responses."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/recalculate-levels.ts` around lines 13 - 15, Replace the manual guard in execute with the shared helpers: call await validateInteraction(interaction) at the start of the function to validate the incoming interaction, and replace direct returns/responses with safelyRespond(interaction, ...) so all replies and error messages use the centralized handler; update any early-return logic that checked interaction.isChatInputCommand() && interaction.guild to rely on validateInteraction and ensure every place that previously used interaction.reply or return now uses safelyRespond(interaction, { content: "...", ephemeral: true }) or safelyRespond(interaction, error) as appropriate.src/commands/util/user-info.ts-190-190 (1)
190-190:⚠️ Potential issue | 🟠 MajorUse
safelyRespond(...)instead of direct reply/editReply calls.Line 190 and Line 238 directly call
interaction.editReply(...); this bypasses the standardized response helper expected for command/event handlers.Suggested change
+import { safelyRespond } from '@/util/helpers.js'; ... if (!user) { - await interaction.editReply('User not found'); + await safelyRespond(interaction, 'User not found'); return; } ... - await interaction.editReply({ embeds: [embed] }); + await safelyRespond(interaction, { embeds: [embed] });As per coding guidelines: "Use
safelyRespond(interaction, content)fromsrc/util/helpers.tsfor all interaction responses; it handles reply vs. follow-up vs. skip logic and logs Discord API errors".Also applies to: 238-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/user-info.ts` at line 190, Replace direct calls to interaction.editReply(...) in the user-info command with the standardized helper safelyRespond(interaction, message): locate the occurrences where interaction.editReply('User not found') and the other editReply at the other branch are used, import safelyRespond from src/util/helpers.ts if not already imported, and change those calls to await safelyRespond(interaction, 'User not found') (and the corresponding message for the second editReply) so reply/follow-up logic and error handling are handled consistently.src/commands/util/user-info.ts-180-182 (1)
180-182:⚠️ Potential issue | 🟠 MajorAdd required
validateInteractiongate before processing.Line 180 currently does a manual guard, but this command still skips the required
await validateInteraction(interaction)pre-check used across command handlers.Suggested change
+import { validateInteraction } from '@/util/helpers.js'; ... execute: async (interaction) => { - if (!(interaction.isChatInputCommand() && interaction.guild)) { + if (!(interaction.isChatInputCommand() && interaction.guild)) { return; } + if (!(await validateInteraction(interaction))) { + return; + } await interaction.deferReply();As per coding guidelines: "Call
await validateInteraction(interaction)before processing interactions to ensure the interaction is in-guild, channel is available, and for component interactions the original message is fetchable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/user-info.ts` around lines 180 - 182, Replace the manual guard that checks interaction.isChatInputCommand() && interaction.guild with the standard pre-check: call await validateInteraction(interaction) at the start of the handler (before any processing) and return early if it indicates the interaction is invalid; remove or avoid duplicating the manual guard so the interaction is validated using validateInteraction and subsequent code can assume in-guild/channel/message availability. Ensure you reference the existing validateInteraction function and the current interaction handling block (which currently uses interaction.isChatInputCommand and interaction.guild) when making the change.src/commands/util/user-info.ts-194-194 (1)
194-194:⚠️ Potential issue | 🟠 MajorWrap DB access with consistent DB error handling.
Line 194 calls
getMember(user.id)directly without ahandleDbError(error, operation)path, which can leak inconsistent failures into the command flow.Suggested change
-import { getMember } from '@/db/db.js'; +import { getMember, handleDbError } from '@/db/db.js'; ... - const memberData = (await getMember(user.id)) ?? null; + let memberData: MemberData | null = null; + try { + memberData = (await getMember(user.id)) ?? null; + } catch (error) { + handleDbError(error, 'getMember(user.id)'); + await safelyRespond(interaction, 'Failed to fetch user data.'); + return; + }As per coding guidelines: "Import database functions from
src/db/db.ts, not direct DB access; usehandleDbError(error, operation)for consistent error handling".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/user-info.ts` at line 194, The call to getMember(user.id) in user-info command isn’t wrapped with the standardized DB error handling; replace the direct call to getMember with a try/catch that uses handleDbError(error, "getMember") (imported from src/db/db.ts per guidelines) and return or rethrow/handle the error consistently so memberData is set only on success (e.g., set memberData to null on handled DB errors); ensure you import both getMember and handleDbError from the centralized DB module and apply this pattern around the getMember(user.id) invocation in the file.src/commands/fun/leaderboard.ts-32-37 (1)
32-37:⚠️ Potential issue | 🟠 MajorUse the shared interaction guard/response helpers in this command path.
This flow still bypasses
await validateInteraction(interaction)and directly replies in the collector path. Please route interaction handling through the shared helpers to match project standards and keep response/error behavior consistent.As per coding guidelines, "Call
await validateInteraction(interaction)before processing interactions" and "UsesafelyRespond(interaction, content)fromsrc/util/helpers.tsfor all interaction responses."Also applies to: 171-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/leaderboard.ts` around lines 32 - 37, This command bypasses the project's shared interaction helpers: call await validateInteraction(interaction) immediately after confirming interaction.isChatInputCommand() && interaction.guild, and replace direct uses of interaction.deferReply()/interaction.reply() within the main flow and collector path (including the block around lines 171-176) with the shared helpers—use safelyRespond(interaction, { content }) for all responses and error cases and use the standardized defer/ack flow provided by validateInteraction/safelyRespond so responses and errors conform to project standards; update any collector callback that replies directly to first await validateInteraction(interaction) (or use the validated interaction) and then call safelyRespond instead of interaction.reply/deferReply.src/util/telemetry/otelLogTransport.ts-80-90 (1)
80-90:⚠️ Potential issue | 🟠 MajorAvoid mapping generic
name/stackmetadata to exception fields unconditionally.This currently rewrites ordinary metadata (e.g.,
nameon non-error logs) intoexception.typeand drops the original key, which can corrupt telemetry dimensions.💡 Suggested fix
-function extractCommonErrorField( - key: string, - value: unknown -): Record<string, unknown> | null { +function extractCommonErrorField( + key: string, + value: unknown, + allowErrorFieldProjection: boolean +): Record<string, unknown> | null { + if (!allowErrorFieldProjection) { + return null; + } if (key === 'stack' && typeof value === 'string') { return { 'exception.stacktrace': value }; } if (key === 'name' && typeof value === 'string') { return { 'exception.type': value }; } return null; }-function cleanAttributes( - meta: Record<string, unknown> -): Record<string, unknown> { +function cleanAttributes( + meta: Record<string, unknown>, + allowErrorFieldProjection = false +): Record<string, unknown> { const out: Record<string, unknown> = {}; for (const [k, v] of Object.entries(meta || {})) { @@ - const commonField = extractCommonErrorField(k, v); + const commonField = extractCommonErrorField( + k, + v, + allowErrorFieldProjection + );-const attrs = cleanAttributes(meta as Record<string, unknown>); +const attrs = cleanAttributes( + meta as Record<string, unknown>, + severityNumber >= 17 +);Also applies to: 126-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/telemetry/otelLogTransport.ts` around lines 80 - 90, The extractCommonErrorField function indiscriminately maps metadata keys 'stack' and 'name' to exception fields, which can corrupt non-error telemetry; update extractCommonErrorField to only map when the value is provably an Error-like value (e.g., for 'stack' require a string that looks like a stack trace such as containing '\n' or ' at ', and for 'name' require the caller indicate the source is an Error instance—add an optional parameter like isError:boolean or accept the original value object to check instanceof Error), and when you do map preserve the original key/value as well (return both the original key and the mapped exception field) instead of dropping the original metadata; apply the same guard logic to the other occurrence referenced around lines 126-129 so non-error logs are not remapped.src/commands/fun/fact.ts-191-196 (1)
191-196:⚠️ Potential issue | 🟠 MajorRace condition:
getLastInsertedFactId()can return wrong fact ID if concurrent submissions occur.Between
addFact()completing andgetLastInsertedFactId()executing, another user could submit a fact. TheMAX(id)query would then return the newer fact's ID, causing the approve/reject buttons to target the wrong submission.Modify
addFact()to return the inserted fact's ID using.returning():export async function addFact({...}): Promise<number> { // ... existing code ... const result = await db.insert(factTable).values({...}).returning(); return result[0].id; }Then use the returned ID directly instead of calling
getLastInsertedFactId().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/fact.ts` around lines 191 - 196, The current flow is vulnerable to a race because getLastInsertedFactId() can return a different user's ID; change addFact(...) to return the newly inserted fact ID by using the query builder's .returning() (e.g., have addFact return Promise<number> and return result[0].id after insert into factTable), then update the caller (the code that currently calls addFact({...}) and then calls getLastInsertedFactId()) to capture the returned id (e.g., const factId = await addFact(...)) and use that factId for wiring approve/reject buttons instead of calling getLastInsertedFactId().src/commands/util/ping.ts-9-12 (1)
9-12:⚠️ Potential issue | 🟠 MajorUse the shared interaction helpers here.
Line 10 still calls
interaction.reply()directly, and this handler never invokesvalidateInteraction(interaction). That bypasses the repo’s guard/response flow and can break on already-acknowledged or invalid interactions. As per coding guidelines, "src/{commands,events}/**/*.ts: UsesafelyRespond(interaction, content)fromsrc/util/helpers.tsfor all interaction responses" and "Callawait validateInteraction(interaction)before processing interactions".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/ping.ts` around lines 9 - 12, In the execute handler replace the direct interaction.reply call with the repo's helpers: first call await validateInteraction(interaction) at the start of execute, then return await safelyRespond(interaction, `🏓 Pong! Latency: ${Date.now() - interaction.createdTimestamp}ms`); also add/import validateInteraction and safelyRespond from src/util/helpers.ts and remove the direct interaction.reply usage in the execute function.src/commands/util/config.ts-121-129 (1)
121-129:⚠️ Potential issue | 🟠 MajorEmpty config sections can make
/configfail.
join('\n')androlesTextcan evaluate to''for present-but-empty sections, so the??fallback never fires. Discord rejects empty embed field values, which will break this command on sparse configs.Suggested fix
embed.addFields({ name: 'Channels', - value: channelsText ?? 'None configured', + value: channelsText || 'None configured', }); } @@ embed.addFields({ name: 'Roles', - value: rolesText ?? 'None configured', + value: rolesText || 'None configured', }); } @@ embed.addFields({ name: 'Counting', - value: countingText ?? 'Default settings', + value: countingText || 'Default settings', }); }Also applies to: 132-154, 177-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/config.ts` around lines 121 - 129, The embed field values (e.g., channelsText computed from displayConfig.channels) can be an empty string which bypasses the nullish coalescing and causes Discord to reject the embed; update the code that builds each section (channelsText, rolesText and the other similar variables used in embed.addFields) to treat empty strings as absent—e.g., compute a value variable and if value === '' or value.trim() === '' replace it with a fallback like 'None configured' before calling embed.addFields; apply the same change where rolesText and the other sections are assembled so no embed field value is ever an empty string.src/db/db.ts-358-369 (1)
358-369:⚠️ Potential issue | 🟠 MajorThis CA filename no longer matches the repo's TLS setup.
The repo guidance points production PostgreSQL/Redis TLS at
certs/psql-ca.pem, but this helper now readsrootCA.pem. That will push deployments into the insecure fallback path even when the expected certificate is present.Suggested fix
function loadDbSslOptions(): { ca: Buffer } | undefined { try { return { - ca: fs.readFileSync(path.resolve('./certs/rootCA.pem')), + ca: fs.readFileSync(path.resolve('./certs/psql-ca.pem')), }; } catch (error) {Based on learnings "Use PM2 process manager for production (
yarn startuses process namepoixpixel-discord-bot,yarn restartto reload); ensure TLS certificates are present atcerts/psql-ca.pemfor PostgreSQL and Redis".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/db.ts` around lines 358 - 369, The CA filename in loadDbSslOptions should be changed from 'rootCA.pem' to the repo-standard 'psql-ca.pem' so the function (loadDbSslOptions) reads the expected cert and doesn't fall back to insecure mode; update the fs.readFileSync(path.resolve('./certs/rootCA.pem')) call to read './certs/psql-ca.pem' (keeping the same try/catch and returned { ca: Buffer } shape and logger.warn behavior on failure).src/db/redis.ts-421-445 (1)
421-445:⚠️ Potential issue | 🟠 MajorDon't report a successful flush after hard delete failures.
If both
DELandUNLINKfail for a batch, the helper only logs and continues, butflushRedisCache()still reaches the success log. That can tell admins the cache was flushed even though keys were left behind. Bubble the failure, or aggregate batch errors and fail the flush at the end.Also applies to: 492-494
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/redis.ts` around lines 421 - 445, The deleteKeysInBatches helper currently swallows failures when both redis.del and redis.unlink fail, which allows flushRedisCache to incorrectly log success; modify deleteKeysInBatches (the function handling redis.del and redis.unlink) to propagate errors instead of just logging: either throw the batch error immediately when both operations fail or collect batch-level errors and throw an aggregated Error after the loop so flushRedisCache can catch it and log/handle failure; ensure you update flushRedisCache to catch the thrown error and avoid logging a successful flush when deletion errors occur. Use the existing symbols: deleteKeysInBatches, redis.del, redis.unlink, flushRedisCache, and logger.error to locate and implement the change.src/commands/util/backend-manager.ts-56-61 (1)
56-61:⚠️ Potential issue | 🟠 MajorValidate the interaction before the first reply.
The command now exits on the chat-input/guild guard only, then goes straight into
deferReply(). Please keep the repo-standardvalidateInteraction()check here as well so unavailable channels/interactions are rejected before the first Discord API call.As per coding guidelines, "Call
await validateInteraction(interaction)before processing interactions to ensure the interaction is in-guild, channel is available, and for component interactions the original message is fetchable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/backend-manager.ts` around lines 56 - 61, The execute handler currently checks interaction.isChatInputCommand() and interaction.guild but calls await interaction.deferReply(...) before calling validateInteraction; add a call to await validateInteraction(interaction) immediately after the existing guard and before deferReply so the interaction/channel availability and component state are validated first. Specifically, update the execute function (the method named execute) to invoke validateInteraction(interaction) after the guild/chat-input guard and only then call deferReply, ensuring validateInteraction is awaited and any failed validation returns early.src/commands/util/restart.ts-22-27 (1)
22-27:⚠️ Potential issue | 🟠 MajorValidate the interaction before deferring.
This path still goes straight from the chat-input/guild guard into
deferReply(). If the interaction is stale or its channel is unavailable, the first response call will fail instead of short-circuiting cleanly.As per coding guidelines, "Call
await validateInteraction(interaction)before processing interactions to ensure the interaction is in-guild, channel is available, and for component interactions the original message is fetchable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/restart.ts` around lines 22 - 27, The execute handler currently defers the reply immediately after the chat-input/guild guard; insert a call to await validateInteraction(interaction) right after that guard and before calling interaction.deferReply, and if validateInteraction returns false (or throws a handled validation error) short-circuit and return early; update the execute function to use validateInteraction(interaction) to ensure the interaction and channel are valid before calling interaction.deferReply or further processing.src/commands/util/help.ts-48-50 (1)
48-50:⚠️ Potential issue | 🟠 MajorAdd
awaitto handleeditReply()errors in specific-command path.
handleSpecificCommand()returns a promise frominteraction.editReply()on both code paths (lines 202–204 and 253). Line 50 drops theawait, meaning if Discord's API rejects, the error occurs outside the surroundingtry/catchblock and surfaces as an unhandled promise rejection.Fix
- handleSpecificCommand(interaction, client, commandName); + await handleSpecificCommand(interaction, client, commandName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/help.ts` around lines 48 - 50, The specific-command branch drops the await when calling handleSpecificCommand after deferring the reply, so any rejection from interaction.editReply() escapes the surrounding try/catch; change the call in the if (commandName) branch to await handleSpecificCommand(interaction, client, commandName) so the returned promise is awaited and errors are caught by the enclosing try/catch that surrounds interaction.deferReply and subsequent logic.src/db/schema.ts-13-21 (1)
13-21:⚠️ Potential issue | 🟠 MajorThese hand-written row types are already out of sync with the schema definitions.
memberTableTypes.currentlyInServeranddiscordUsernameare optional in the interface despitenotNull()in the table definition (lines 26-27)levelTableTypes.lastMessageTimestampis typed as optionalDatebut should beDate | nullsince the column is nullable (line 52)moderationTableTypes.durationis required in the interface despite the column lackingnotNull()and defaulting to empty string (line 75)Downstream DB functions cast query results using
as levelTableTypes[]andas moderationTableTypes[](e.g., levelFunctions.ts:535, moderationFunctions.ts:97), which masks these nullability mismatches and weakens type safety. Derive the interfaces from table definitions or align the field nullability exactly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 13 - 21, The handwritten row interfaces are out-of-sync with the table schema; update the types for memberTableTypes, levelTableTypes, and moderationTableTypes to exactly match column nullability (or better: derive them from the table definitions) — specifically make memberTableTypes.currentlyInServer and memberTableTypes.discordUsername non-optional (non-null), change levelTableTypes.lastMessageTimestamp to Date | null, and make moderationTableTypes.duration nullable/optional to reflect the column that lacks notNull() (or reflect the actual default if you prefer non-null empty-string); also remove or adjust unsafe casts like "as levelTableTypes[]" and "as moderationTableTypes[]" in levelFunctions.ts and moderationFunctions.ts and use the derived/updated types so the compiler enforces correct nullability.src/db/redis.ts-212-226 (1)
212-226:⚠️ Potential issue | 🟠 MajorUse atomic SET with EX to ensure TTL always applies.
The
set()function currently executesSETandEXPIREas separate commands. IfEXPIREfails afterSETsucceeds, the key persists without TTL—a data reliability issue. Additionally, the truthy check skipsttl: 0. ioredis supports atomic SET-with-expiry: useredis.set(key, value, 'EX', ttl)and gate onttl !== undefinedto handle all cases atomically.Possible fix
- await redis.set(`bot:${key}`, value); - if (ttl) { - await redis.expire(`bot:${key}`, ttl); - } + if (ttl !== undefined) { + await redis.set(`bot:${key}`, value, 'EX', ttl); + } else { + await redis.set(`bot:${key}`, value); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/redis.ts` around lines 212 - 226, The set() function currently calls redis.set(...) then redis.expire(...), which can leave keys without TTL if expire fails and also treats ttl: 0 as falsy; change set() (in src/db/redis.ts) to perform an atomic SET with expiry when a TTL is provided by checking ttl !== undefined and calling redis.set(`bot:${key}`, value, 'EX', ttl) (and remove the separate redis.expire call); keep the ensureRedisConnection() guard and return semantics the same.src/commands/moderation/warn.ts-25-30 (1)
25-30: 🛠️ Refactor suggestion | 🟠 MajorRun the shared interaction validation before deferring.
This early return only checks command type and guild, so it still skips the repo's standard validation path. Please call
validateInteraction(interaction)beforedeferReply()so invalid states are handled consistently instead of returning silently.As per coding guidelines, "Call
await validateInteraction(interaction)before processing interactions to ensure the interaction is in-guild, channel is available, and for component interactions the original message is fetchable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/moderation/warn.ts` around lines 25 - 30, The execute handler currently returns early before running shared validation; call await validateInteraction(interaction) immediately after entering execute and before await interaction.deferReply(...) so the repo's standard validation logic (in validateInteraction) runs for all interactions; update the execute function in the warn command to await validateInteraction(interaction) prior to deferring the reply and proceed only if it resolves.src/commands/moderation/ban.ts-35-40 (1)
35-40: 🛠️ Refactor suggestion | 🟠 MajorRun the shared interaction validation before deferring.
This early return only checks command type and guild, so it still skips the repo's standard validation path. Please call
validateInteraction(interaction)beforedeferReply()so invalid states are handled consistently instead of returning silently.As per coding guidelines, "Call
await validateInteraction(interaction)before processing interactions to ensure the interaction is in-guild, channel is available, and for component interactions the original message is fetchable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/moderation/ban.ts` around lines 35 - 40, The handler currently returns early after checking interaction.isChatInputCommand() and interaction.guild, skipping the repository's standard validation; update the execute function to call await validateInteraction(interaction) before calling interaction.deferReply({ flags: ['Ephemeral'] }) so all interactions go through the shared validation path (keep the existing type/guild checks if desired but ensure validateInteraction is awaited first); reference the execute function, validateInteraction, and deferReply to locate and update the code.src/events/messageEvents.ts-213-218 (1)
213-218:⚠️ Potential issue | 🟠 MajorReuse the counting evaluator for restore checks.
The counting pipeline accepts arithmetic expressions via
sanitizeAndEval(), but this helper only doesNumber(trimmed). Deleted valid counts like1+1or3*4will never be restored even though they are accepted everywhere else in the counting flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/events/messageEvents.ts` around lines 213 - 218, The restore-check code currently converts the message with Number(trimmed) which rejects valid arithmetic like "1+1"; replace the Number(trimmed) usage with the shared evaluator sanitizeAndEval(trimmed) (the same function used by the counting pipeline), then validate the result with Number.isInteger(result) before returning; if sanitizeAndEval can throw or return non-numeric values, guard it with a try/catch or NaN check so only integer results proceed (refer to sanitizeAndEval and the parsed/result variable in this message handling block).src/util/counting/countingManager.ts-191-200 (1)
191-200:⚠️ Potential issue | 🟠 MajorDon't collapse every eval error into
ignored.
sanitizeAndEval()throws for invalid count attempts too — division by zero, leading zeros, empty parens, malformed operator sequences, etc. Catching everything here means inputs like01or1/0no longer trigger the warning/reset path and can be used to bypass counting enforcement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/counting/countingManager.ts` around lines 191 - 200, The current catch around sanitizeAndEval() treats all errors as "ignored", hiding malformed numeric inputs (e.g., leading zeros, division by zero) that should trigger the invalid/warn/reset path; change the logic so non-numeric messages are filtered before calling sanitizeAndEval() (e.g., a regex test on trimmed) and only map true non-numeric errors to the ignored path, while any errors thrown by sanitizeAndEval() (identify via the thrown error or by calling sanitizeAndEval() only for inputs that pass the numeric-ish test) should be handled as invalid counting attempts (return { isValid: false, reason: 'invalid' } or rethrow to let the CountingManager's invalid/warn/reset logic run). Use the sanitizeAndEval and CountingManager context and update the catch branch to distinguish non-numeric vs evaluation/parsing errors accordingly.src/events/messageEvents.ts-151-198 (1)
151-198:⚠️ Potential issue | 🟠 MajorEliminate duplicate audit log fetch in messageDelete handler.
The handler calls
fetchAuditLogs()twice per deleted message: once inshouldAllowRestoreCountingMessage()(line 158) to validate restore eligibility against author/channel, and again at line 263 to retrieve the executor for logging. During bulk deletes this becomes a significant performance issue. Reuse the matched entry from the first lookup or cache the result to avoid the second fetch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/events/messageEvents.ts` around lines 151 - 198, The audit log is being fetched twice; update shouldAllowRestoreCountingMessage to return the matched audit-log entry (or its executor) along with the boolean decision so the caller can reuse it instead of calling fetchAuditLogs() again: locate the matching variable inside shouldAllowRestoreCountingMessage (currently assigned from entries.find) and return an object like { allowed: boolean, matchingEntry?: AuditLogEntry } or { allowed: boolean, executor?: User } (or a tuple), update the call sites in the messageDelete handler to consume the returned executor/matchingEntry (use its executor for logging) and remove the second fetchAuditLogs() call; keep existing error handling and fallback behavior.
…mprove error handling
…e message response logic
…Respond and validateInteraction
…n and executor tracking
…ck trace handling
There was a problem hiding this comment.
11 issues found across 46 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/events/messageEvents.ts">
<violation number="1" location="src/events/messageEvents.ts:267">
P2: Moderator attribution regressed for most `messageDelete` logs because executor is now only derived from counting-message restoration logic.</violation>
</file>
<file name="src/util/counting/countingManager.ts">
<violation number="1" location="src/util/counting/countingManager.ts:215">
P1: Early return in catch exits before local variables (like `expected`) are initialized, risking incorrect state or runtime errors.</violation>
</file>
<file name="src/db/functions/utils/moderationUtils.ts">
<violation number="1" location="src/db/functions/utils/moderationUtils.ts:16">
P1: `createdAt` fallback uses an unsafe cast to return the original value, which can leak invalid/non-Date data instead of a normalized date value.</violation>
</file>
<file name="src/events/interactionCreate.ts">
<violation number="1" location="src/events/interactionCreate.ts:333">
P2: `handleInteractionError` is now async, but its caller does not await it, creating a floating promise in the catch path.</violation>
</file>
<file name="CONTRIBUTING.md">
<violation number="1" location="CONTRIBUTING.md:78">
P3: The docs incorrectly describe commitlint as a pre-commit check; it runs on the commit-msg hook.</violation>
</file>
<file name="src/db/functions/levelFunctions.ts">
<violation number="1" location="src/db/functions/levelFunctions.ts:79">
P2: `getUserLevel` returns a synthetic `id: 0` instead of the actual DB row after insert, which can return invalid/stale user-level data.</violation>
</file>
<file name="src/util/telemetry/otelLogTransport.ts">
<violation number="1" location="src/util/telemetry/otelLogTransport.ts:96">
P2: The `isError` check makes the `'name'` branch unreachable because this function is only called after non-Error values are filtered, so `isError` is always false.</violation>
</file>
<file name="src/db/functions/memberFunctions.ts">
<violation number="1" location="src/db/functions/memberFunctions.ts:238">
P2: Guard against empty `updateFields` before calling `.set(...)`; the current signature allows `{ discordId }` only, which can produce an empty update payload and fail at runtime.</violation>
</file>
<file name="src/commands/fun/achievements.ts">
<violation number="1" location="src/commands/fun/achievements.ts:55">
P2: Use the component interaction (`i`) for unauthorized collector responses; responding with the original command interaction can overwrite the main reply and leave the clicked interaction unacknowledged.</violation>
</file>
<file name="src/db/functions/moderationFunctions.ts">
<violation number="1" location="src/db/functions/moderationFunctions.ts:27">
P3: JSDoc parameter tags no longer match the refactored function signature (single `moderation` object), which makes API docs misleading.</violation>
</file>
<file name="src/util/counting/constants.ts">
<violation number="1" location="src/util/counting/constants.ts:43">
P2: This introduces a duplicate math-validation regex. Keep a single shared regex source so `processCountingMessage` and `sanitizeAndEval` cannot diverge over time.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } as const; | ||
| }; | ||
|
|
||
| export const numericLikeRegex = /^[\d+\-*/()\s]+$/; |
There was a problem hiding this comment.
P2: This introduces a duplicate math-validation regex. Keep a single shared regex source so processCountingMessage and sanitizeAndEval cannot diverge over time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/util/counting/constants.ts, line 43:
<comment>This introduces a duplicate math-validation regex. Keep a single shared regex source so `processCountingMessage` and `sanitizeAndEval` cannot diverge over time.</comment>
<file context>
@@ -38,4 +38,6 @@ export const MILESTONE_REACTIONS = {
-} as const;
+};
+
+export const numericLikeRegex = /^[\d+\-*/()\s]+$/;
</file context>
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/util/helpers.ts (1)
162-175:⚠️ Potential issue | 🟠 MajorMove moderation table deactivation before the early return check in
executeUnmute.The early return at line 162–164 skips the
UPDATE moderationTable ... active=falseblock. If the member is missing from the database or already marked unmuted, mute rows remainactive=true. SinceloadActiveMutesqueries formoderationTable.active = true, these stale rows re-trigger on the next startup, causing phantom unmutes or duplicate scheduling.Deactivate the mute rows before checking
currentlyMuted, or restructure the logic to ensure moderation rows are always cleaned up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/helpers.ts` around lines 162 - 175, In executeUnmute, the current early return after checking getMember(userId)?.currentlyMuted prevents running the db.update(...) that sets moderationTable.active = false; move or copy the moderationTable deactivation (the db.update(moderationTable).set({ active: false })... where(...) call) to run before the early return so that moderation rows for action 'mute' and discordId userId are always marked inactive (use the same where clause with eq(moderationTable.discordId, userId), eq(moderationTable.action, 'mute'), eq(moderationTable.active, true)); optionally keep the currentlyMuted check afterward to skip the Discord unmute call but ensure the DB cleanup happens first.
🧹 Nitpick comments (3)
src/commands/util/server.ts (1)
29-32: Use a Discord timestamp forcreatedAt.
${interaction.guild.createdAt}callsDate.prototype.toString(), which yields an engine/locale-dependent string like"Wed Apr 09 2025 10:30:00 GMT+0000". You already exportmsToDiscordTimestampfrom@/util/helpers.js; using it gives users a localized, hover-for-relative timestamp:Suggested tweak
- await safelyRespond( - interaction, - `The server **${interaction.guild.name}** has **${interaction.guild.memberCount}** members and was created on **${interaction.guild.createdAt}**. It is **${new Date().getFullYear() - interaction.guild.createdAt.getFullYear()}** years old.` - ); + const { guild } = interaction; + await safelyRespond( + interaction, + `The server **${guild.name}** has **${guild.memberCount}** members and was created on ${msToDiscordTimestamp(guild.createdTimestamp)}. It is **${new Date().getFullYear() - guild.createdAt.getFullYear()}** years old.` + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/server.ts` around lines 29 - 32, The message uses interaction.guild.createdAt directly which prints a locale-dependent Date string; replace that with the exported helper msToDiscordTimestamp to produce a Discord-formatted timestamp. Update the template in the safelyRespond call (where interaction is used) to call msToDiscordTimestamp(interaction.guild.createdAt.getTime()) (and keep the existing memberCount and age calculation), ensuring you import msToDiscordTimestamp from "@/util/helpers.js" if not already imported.src/db/functions/memberFunctions.ts (1)
225-251: Guard against emptyupdateFields.If a caller invokes
updateMember({ discordId })with no other keys,updateFieldsis{}anddb.update(memberTable).set({})will throw at runtime (Drizzle requires at least one column inSET). Current callers look safe, but a one-line guard makes this robust against future misuse:Proposed guard
const { discordId, ...updateFields } = updates; + if (Object.keys(updateFields).length === 0) { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/functions/memberFunctions.ts` around lines 225 - 251, The updateMember function must guard against an empty updateFields object before calling db.update(memberTable).set(updateFields); check the destructured updateFields (from updates) and if Object.keys(updateFields).length === 0, log an appropriate message via logger (e.g., "[memberDbFunctions] no update fields provided for discordId: ...") and either throw a clear Error('No update fields provided') or return early—make the behavior consistent with other DB error handling; update the branch in updateMember that reaches db.update(memberTable).set(...) to perform this check and avoid calling Drizzle with an empty SET.src/db/functions/utils/moderationUtils.ts (1)
13-20: Misleadingas Datecast increatedAtfallback.When
createdAtis null or NaN, the fallback(record.createdAt as Date)assertsDatebut the underlying value could benull,undefined, or an invalid string (since we only reach that branch whennew Date(record.createdAt)was itself null/NaN). Downstream code that relies on theDatetype will receive a non-Datevalue.Either keep the real input type in the return type, or fall back consistently (e.g., to
nulllikeexpiresAt, or tonew Date(0)/ current time as a sentinel). At minimum the asymmetric handling betweencreatedAtandexpiresAtdeserves an inline note.Possible normalization
return { ...record, createdAt: createdAt != null && !Number.isNaN(createdAt.getTime()) ? createdAt - : (record.createdAt as Date), + : (record.createdAt ?? null), expiresAt: expiresAt != null && !Number.isNaN(expiresAt.getTime()) ? expiresAt : null, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/functions/utils/moderationUtils.ts` around lines 13 - 20, The createdAt fallback in moderationUtils.ts currently forces (record.createdAt as Date) which can be null/undefined/invalid; make it consistent with expiresAt by returning null when createdAt is missing/NaN (i.e., change the createdAt branch to yield null instead of casting record.createdAt), and update any related return type/interface to allow createdAt to be Date | null (or choose a clear sentinel Date and apply it to both createdAt and expiresAt consistently); ensure references to createdAt, expiresAt and record.createdAt in the moderationUtils functions reflect the chosen nullable/sentinel contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ultracite.yml:
- Around line 38-39: The Biome SARIF generation step ("Generate Biome SARIF
report") is limited to 20 diagnostics; update the run command that currently
calls `yarn biome ci --reporter sarif --reporter-file biome-results.sarif ./src`
to include the `--max-diagnostics=none` flag so the SARIF output contains all
findings (i.e., change the step's run invocation to add
`--max-diagnostics=none`).
- Around line 3-18: The workflow triggers on pull_request but later steps in the
ultracite job perform write operations and will fail for forked PRs because the
token is read-only; add a guard to skip write steps for forked PRs by checking
the pull_request head repo fork flag (e.g., use an if condition such as
"github.event.pull_request == null || github.event.pull_request.head.repo.fork
== false") on the write steps (or the ultracite job) so writes only run for
non-fork PRs and other events; update any step that posts comments or uploads
results to include this condition.
- Around line 144-149: The workflow step named "Upload analysis results to
GitHub" currently uses if: always() and tries to upload sarif_file:
biome-results.sarif even when the file doesn't exist; change the step's
condition to only run when the SARIF file exists by replacing the if: always()
guard with a hashFiles check (e.g., test that hashFiles('biome-results.sarif')
is not empty) so the upload-sarif@v4 action is executed only when
biome-results.sarif is present and the original biome failure isn't masked by a
ConfigurationError.
In `@package.json`:
- Line 19: Add a "prepare" lifecycle script to package.json so Husky hooks are
installed automatically during install: update the "scripts" object that
currently contains "setup": "husky" by adding "prepare": "husky install" (you
can keep "setup": "husky" for manual runs if desired) so that running npm/yarn
install will run the Husky installation step without requiring contributors to
run "setup" manually.
In `@src/commands/fun/counting.ts`:
- Around line 96-104: The catch block currently echoes raw error details to the
user; instead log the Error instance using the project's logger (e.g.,
logger.error(error, { action: 'setCount', count, interactionId: interaction.id
})) and send a generic failure message via safelyRespond (for example: "Failed
to set the count. Please try again later.") without including the error string;
update the catch around setCount/ safelyRespond to follow this pattern and
ensure you import/ use the logger from src/util/logger.js.
- Around line 313-323: The select menu construction in getSelectRow creates one
option per page using StringSelectMenuBuilder and doesn't guard Discord's
25-option limit, so add a check on pages.length and only build/add the
StringSelectMenu when pages.length <= 25 (otherwise omit the select component
but still return the pagination buttons); update both the getSelectRow used in
the listbans handler and the analogous select construction in the listwarnings
handler to follow this pattern, referencing pages and currentPage to preserve
default selection when the select is present and ensuring the returned
ActionRow(s) still include the navigation buttons even if the select is omitted.
In `@src/commands/fun/fact.ts`:
- Around line 132-136: Several places in src/commands/fun/fact.ts are calling
interaction.reply / i.reply / interaction.editReply directly for text-only
responses (e.g., the guard that checks i.user.id !== interaction.user.id and
similar blocks around the ranges noted); replace those direct calls with the
shared helper safelyRespond(interaction, { content: '...', flags: [...] } or
safelyRespond(interaction, '...') as appropriate so you reuse centralized
response/error handling from src/util/helpers.ts, while keeping direct
interaction.update / interaction.editReply only where embeds or component
updates are actually required (do the replacements for the occurrences around
lines 132-136, 259-270, 280-290, 301-312, and 374-383).
- Around line 192-249: The code currently calls addFact(...) (factId) before
verifying the approval channel, which can persist a pending fact that never gets
moderator buttons; change the flow in the non-admin branch so you first locate
and validate approvalChannel (check
interaction.guild?.channels.cache.get(config.channels.factApproval) and
approvalChannel?.isTextBased()) and only then call addFact(...) to create the
pending fact and send the embed/buttons; if the channel is missing or not
text-based, do not call addFact — instead log the error
(logger.error('[FactCommand] Fact approval channel not found or is not a text
channel')) and respond to the user that submission failed or to contact staff,
ensuring addFact, approvalChannel, and interaction.editReply are updated
accordingly.
- Around line 94-114: The subcommand dispatch in the switch (handling subcommand
variable and calling handleSubmitFact, handleApproveFact, handleDeleteFact,
handlePendingFacts, handlePostFact) must be wrapped in a try/catch so that if
any handler throws after the interaction was deferred the user still gets a
final response; update the code to call the handlers inside a try block and in
the catch log the error and call safelyRespond(interaction, { content: 'An error
occurred while processing your request.' }) (or similar contextual message) to
send a final reply, mirroring the pattern used in counting.ts.
In `@src/commands/fun/rank.ts`:
- Line 5: The import for safelyRespond and validateInteraction currently uses
the path alias without the required extension; update the import of
"@/util/helpers" so it includes the .js extension (i.e., import safelyRespond
and validateInteraction from "@/util/helpers.js") to comply with ESM/project
rules and match other imports in this file.
In `@src/commands/util/config.ts`:
- Around line 30-35: The current masking logic only hides one DB secret because
it uses "else if" between output.database.poolingDbConnectionString and
output.database.directDbConnectionString; change that to two independent checks
so both output.database.poolingDbConnectionString and
output.database.directDbConnectionString are masked regardless of the other’s
presence (i.e., replace the "else if" with a separate "if" for
directDbConnectionString) so both secrets are redacted before posting the
config.
In `@src/commands/util/user-info.ts`:
- Around line 128-155: The current code uses muteModerations[0] and
banModerations[0] which may be the newest row but not the currently active
moderation; update buildCurrentMuteField and buildCurrentBanField to search for
the active moderation row (e.g., const currentMute = muteModerations.find(m =>
m.active) and const currentBan = banModerations.find(m => m.active)), and if no
active row exists fall back to the first entry (muteModerations[0] /
banModerations[0]) or return null as appropriate so the embed shows the true
active reason/duration; keep the rest of the field construction in
buildCurrentMuteField/buildCurrentBanField unchanged.
In `@src/db/functions/factFunctions.ts`:
- Around line 37-52: The insert currently returns result[0]?.id ?? 0 which
silently coerces a missing returning row to 0; instead, in the function that
inserts into factTable (the add fact logic that calls db.insert(...).returning({
id: factTable.id })), detect when result[0] is undefined and throw or call
handleDbError with a clear message (e.g., "No row returned after insert") so
callers don't receive a bogus factId of 0; keep the
invalidateCache('unused-facts') call when insert succeeds and only return the
numeric id when result[0].id is present.
In `@src/events/messageEvents.ts`:
- Around line 321-335: The execute function should be declared async and must
await the async work so the event handler doesn't resolve early: change the
execute: (message: Message): Promise<void> => { ... } to an async function,
await handleLevelingMessage(message), and replace any Promise.resolve() return
statements with simple returns (e.g., return;). Also ensure you still enqueue
counting messages (countingQueue.push) and call processCountingQueue() as
before; if processCountingQueue is async consider awaiting it as well. Use the
identifiers execute, handleLevelingMessage, countingQueue, and
processCountingQueue to locate and update the code.
In `@src/util/counting/countingManager.ts`:
- Around line 200-260: The catch invokes the inner helper handleRollbackOrReset
while it closes over const expected which is declared after the try/catch,
causing a TDZ ReferenceError when sanitizeAndEval throws; fix by moving the
expected initialization (const expected = data.currentCount + 1) above the
try/catch so it's defined before handleRollbackOrReset can run, or refactor
handleRollbackOrReset to accept expected as a parameter (or convert it to a
const arrow function) and pass expected in; update references inside
handleRollbackOrReset (expectedCount, logging) accordingly and keep use of
setCount, resetCounting and handleMistake unchanged.
In `@src/util/helpers.ts`:
- Around line 564-586: In safelyRespond, the code paths for interaction.deferred
and interaction.replied call interaction.editReply but the comments state they
should send a follow-up (ephemeral by default); fix this by replacing the
editReply calls with interaction.followUp({ content, flags: ephemeral ?
MessageFlags.Ephemeral : undefined }) for the deferred and replied branches to
match the documented behavior, or if editReply was intended, update the comments
to state that the original reply is edited and remove any reference to
follow-up/ephemeral semantics; reference interaction.deferred,
interaction.replied, safelyRespond, followUp, editReply, and
MessageFlags.Ephemeral when making the change.
In `@src/util/telemetry/otelLogTransport.ts`:
- Around line 134-143: The code marks Error objects early in the loop (the v
instanceof Error branch in otelLogTransport.ts) and continues, so
extractCommonErrorField(k, v, v instanceof Error) never sees isError=true and
the "name" → "exception.type" mapping in extractCommonErrorField is effectively
dead; fix by making isError derived from sibling metadata instead of v
instanceof Error — e.g., before the per-key logic detect an error-shaped record
(presence of a string "stack" and/or "message") and set a local isErrorForRecord
flag, then call extractCommonErrorField(k, v, isErrorForRecord) or remove the
isError gate inside extractCommonErrorField so that when a top-level "name"
string appears alongside an error-looking "stack" it maps to exception.type;
update extractErrorAttributes usage accordingly so true Error instances still
prefer extractErrorAttributes but flattened Winston-style records get the
name→exception.type mapping.
- Around line 59-61: isNumericKey currently treats empty or whitespace-only
strings as numeric because Number('') === 0; update isNumericKey to only return
true for non-empty, trimmed strings that consist entirely of digits (i.e.,
integer indices) so whitespace or empty keys are not considered numeric. Locate
the isNumericKey function and replace the Number(...) check with a stricter
validation such as trimming the key and matching it against an all-digits
pattern (e.g., /^\d+$/) to ensure only true numeric splat indices pass.
---
Outside diff comments:
In `@src/util/helpers.ts`:
- Around line 162-175: In executeUnmute, the current early return after checking
getMember(userId)?.currentlyMuted prevents running the db.update(...) that sets
moderationTable.active = false; move or copy the moderationTable deactivation
(the db.update(moderationTable).set({ active: false })... where(...) call) to
run before the early return so that moderation rows for action 'mute' and
discordId userId are always marked inactive (use the same where clause with
eq(moderationTable.discordId, userId), eq(moderationTable.action, 'mute'),
eq(moderationTable.active, true)); optionally keep the currentlyMuted check
afterward to skip the Discord unmute call but ensure the DB cleanup happens
first.
---
Nitpick comments:
In `@src/commands/util/server.ts`:
- Around line 29-32: The message uses interaction.guild.createdAt directly which
prints a locale-dependent Date string; replace that with the exported helper
msToDiscordTimestamp to produce a Discord-formatted timestamp. Update the
template in the safelyRespond call (where interaction is used) to call
msToDiscordTimestamp(interaction.guild.createdAt.getTime()) (and keep the
existing memberCount and age calculation), ensuring you import
msToDiscordTimestamp from "@/util/helpers.js" if not already imported.
In `@src/db/functions/memberFunctions.ts`:
- Around line 225-251: The updateMember function must guard against an empty
updateFields object before calling db.update(memberTable).set(updateFields);
check the destructured updateFields (from updates) and if
Object.keys(updateFields).length === 0, log an appropriate message via logger
(e.g., "[memberDbFunctions] no update fields provided for discordId: ...") and
either throw a clear Error('No update fields provided') or return early—make the
behavior consistent with other DB error handling; update the branch in
updateMember that reaches db.update(memberTable).set(...) to perform this check
and avoid calling Drizzle with an empty SET.
In `@src/db/functions/utils/moderationUtils.ts`:
- Around line 13-20: The createdAt fallback in moderationUtils.ts currently
forces (record.createdAt as Date) which can be null/undefined/invalid; make it
consistent with expiresAt by returning null when createdAt is missing/NaN (i.e.,
change the createdAt branch to yield null instead of casting record.createdAt),
and update any related return type/interface to allow createdAt to be Date |
null (or choose a clear sentinel Date and apply it to both createdAt and
expiresAt consistently); ensure references to createdAt, expiresAt and
record.createdAt in the moderationUtils functions reflect the chosen
nullable/sentinel contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d89e5123-3b11-4345-9a9e-12cfafef280b
📒 Files selected for processing (46)
.devcontainer/devcontainer.json.github/workflows/ultracite.yml.husky/commit-msgCONTRIBUTING.mdpackage.jsonsrc/commands/fun/achievements.tssrc/commands/fun/counting.tssrc/commands/fun/fact.tssrc/commands/fun/giveaway.tssrc/commands/fun/leaderboard.tssrc/commands/fun/rank.tssrc/commands/moderation/ban.tssrc/commands/moderation/kick.tssrc/commands/moderation/mute.tssrc/commands/moderation/unban.tssrc/commands/moderation/unmute.tssrc/commands/moderation/warn.tssrc/commands/testing/test-join.tssrc/commands/testing/test-leave.tssrc/commands/util/backend-manager.tssrc/commands/util/config.tssrc/commands/util/help.tssrc/commands/util/manage-achievement.tssrc/commands/util/members.tssrc/commands/util/ping.tssrc/commands/util/purge.tssrc/commands/util/recalculate-levels.tssrc/commands/util/reload-config.tssrc/commands/util/restart.tssrc/commands/util/rules.tssrc/commands/util/server.tssrc/commands/util/user-info.tssrc/commands/util/xp.tssrc/db/functions/factFunctions.tssrc/db/functions/levelFunctions.tssrc/db/functions/memberFunctions.tssrc/db/functions/moderationFunctions.tssrc/db/functions/utils/moderationUtils.tssrc/db/redis.tssrc/db/schema.tssrc/events/interactionCreate.tssrc/events/messageEvents.tssrc/util/counting/constants.tssrc/util/counting/countingManager.tssrc/util/helpers.tssrc/util/telemetry/otelLogTransport.ts
🚧 Files skipped from review as they are similar to previous changes (27)
- .husky/commit-msg
- src/commands/util/ping.ts
- src/commands/testing/test-join.ts
- src/util/counting/constants.ts
- src/commands/util/rules.ts
- src/commands/moderation/unmute.ts
- src/commands/testing/test-leave.ts
- src/commands/moderation/unban.ts
- src/commands/util/restart.ts
- src/commands/util/members.ts
- src/commands/util/reload-config.ts
- .devcontainer/devcontainer.json
- src/commands/moderation/kick.ts
- src/commands/util/manage-achievement.ts
- src/commands/moderation/ban.ts
- src/commands/util/xp.ts
- CONTRIBUTING.md
- src/commands/moderation/warn.ts
- src/commands/fun/leaderboard.ts
- src/events/interactionCreate.ts
- src/commands/fun/achievements.ts
- src/commands/util/backend-manager.ts
- src/db/functions/levelFunctions.ts
- src/commands/fun/giveaway.ts
- src/db/schema.ts
- src/db/redis.ts
- src/commands/util/purge.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Seer Code Review
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Path alias
@/*maps tosrc/*and imports MUST append.jsextension (e.g.,import { foo } from '@/util/helpers.js')Import database functions from
src/db/db.ts, not direct DB access. Always usehandleDbError(error, operation)for consistent error handlingAll Redis keys MUST be prefixed with
'bot:'(e.g.,bot:counting:${guildId}). Always checkisRedisConnected()before non-critical caching operationsUse
safelyRespond(interaction, content)fromsrc/util/helpers.tsfor all interaction responses instead of directly calling interaction methodsCall
await validateInteraction(interaction)before processing interactions to ensure the interaction is still valid (in-guild, channel available, message fetchable for components)Use structured logging with
logger.info(),logger.error(), etc. fromsrc/util/logger.js, passing Error instances and metadata objects instead of string concatenationUse
typekeyword for imports only used in type positions (e.g.,import type { Guild } from 'discord.js')Never modify
achievementDefinitionsTabledirectly; seed once and update via admin commands only. Progress tracked inuserAchievementsTable.progressXP cooldown is stored in Redis as
bot:xp_cooldown:${userId}and enforced vialeveling.xpCooldownconfigurationUse modal UI for role input in giveaways when guild has >25 eligible roles (Discord's 25-option limit for select menus)
Don't import from
target/directory; always import fromsrc/with.jsextensions for TypeScript filesUse
constby default,letonly when reassignment is needed, never usevarUse arrow functions for callbacks and short functions
Prefer
for...ofloops over.forEach()and indexedforloopsUse optional chaining (
?.) and nullish coalescing (??) for safer property accessPrefer template literals over string concatenation
Use destructuring for object and array assignments
Always
awaitpromises in async functions and useasync/awaitsynta...
Files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/db/functions/moderationFunctions.tssrc/db/functions/factFunctions.tssrc/commands/util/user-info.tssrc/util/counting/countingManager.tssrc/db/functions/utils/moderationUtils.tssrc/commands/util/help.tssrc/util/telemetry/otelLogTransport.tssrc/db/functions/memberFunctions.tssrc/util/helpers.tssrc/events/messageEvents.ts
src/commands/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All commands in
src/commands/**/*.tsMUST default export an object withdata(SlashCommandBuilder) andasync execute(interaction)method, satisfying Command, OptionsCommand, or SubcommandCommand typesIf a command takes >3 seconds to execute, call
interaction.deferReply()immediately to prevent interaction token expiration
Files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/commands/util/user-info.tssrc/commands/util/help.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit types for function parameters and return values in TypeScript when they enhance clarity
Prefer
unknownoveranywhen the type is genuinely unknown in TypeScriptUse const assertions (
as const) for immutable values and literal types in TypeScriptLeverage TypeScript's type narrowing instead of type assertions
Files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/db/functions/moderationFunctions.tssrc/db/functions/factFunctions.tssrc/commands/util/user-info.tssrc/util/counting/countingManager.tssrc/db/functions/utils/moderationUtils.tssrc/commands/util/help.tssrc/util/telemetry/otelLogTransport.tssrc/db/functions/memberFunctions.tssrc/util/helpers.tssrc/events/messageEvents.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions in JavaScript/TypeScript
Prefer
for...ofloops over.forEach()and indexedforloops in JavaScript/TypeScriptUse optional chaining (
?.) and nullish coalescing (??) for safer property access in JavaScript/TypeScriptPrefer template literals over string concatenation in JavaScript/TypeScript
Use destructuring for object and array assignments in JavaScript/TypeScript
Use
constby default,letonly when reassignment is needed, nevervarin JavaScript/TypeScriptAlways
awaitpromises in async functions - don't forget to use the return valueUse
async/awaitsyntax instead of promise chains for better readability in JavaScript/TypeScriptHandle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Remove
console.log,debugger, andalertstatements from production codeThrow
Errorobjects with descriptive messages in JavaScript/TypeScript, not strings or other valuesUse
try-catchblocks meaningfully - don't catch errors just to rethrow themPrefer early returns over nested conditionals for error cases in JavaScript/TypeScript
Extract complex conditions into well-named boolean variables in JavaScript/TypeScript
Use early returns to reduce nesting in JavaScript/TypeScript
Prefer simple conditionals over nested ternary operators in JavaScript/TypeScript
Don't use
eval()or assign directly todocument.cookiein JavaScript/TypeScriptAvoid spread syntax in accumulators within loops in JavaScript/TypeScript for performance
Use top-level regex literals instead of creating them in loops in JavaScript/TypeScript for performance
Prefer specific imports over namespace imports in JavaScript/TypeScript
Files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/db/functions/moderationFunctions.tssrc/db/functions/factFunctions.tssrc/commands/util/user-info.tssrc/util/counting/countingManager.tssrc/db/functions/utils/moderationUtils.tssrc/commands/util/help.tssrc/util/telemetry/otelLogTransport.tssrc/db/functions/memberFunctions.tssrc/util/helpers.tssrc/events/messageEvents.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Use `yarn ultracite fix` to automatically format and lint code before committing
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Run `yarn type-check` to validate TypeScript types before committing
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Use `yarn dev` to start the bot in watch mode with auto-checking of command deployment
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Use `FORCE_COMMAND_DEPLOY=true yarn dev` only when necessary to force command re-registration; normally `yarn dev` handles this automatically
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Bot only registers commands in configured `config.guildId` (guild-specific), not global slash commands
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Gracefully degrade when Redis is unavailable; check `isRedisConnected()` before non-critical caching operations
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Database migrations use `npx drizzle-kit generate` to generate SQL and `npx drizzle-kit migrate` to apply pending migrations
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Production uses TLS certificate at `certs/psql-ca.pem` for PostgreSQL connections; `drizzle.config.ts` uses `database.directDbConnectionString` for DDL operations
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Use PM2 process manager for production with `yarn start` and `yarn restart` commands; process name is `poixpixel-discord-bot`
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:09.737Z
Learning: Pre-commit hooks via Husky run `yarn ultracite fix` on staged JS/TS/JSON files and `yarn type-check` when staged `.ts` files are present
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:31.694Z
Learning: Keep functions focused and under reasonable cognitive complexity limits
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:31.694Z
Learning: Group related code together and separate concerns in code organization
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:31.694Z
Learning: Validate and sanitize user input for security
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:31.694Z
Learning: Use descriptive names for functions, variables, and types - write self-documenting code
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:31.694Z
Learning: Write code that is accessible, performant, type-safe, and maintainable - focus on clarity and explicit intent over brevity
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot
Timestamp: 2026-04-19T05:49:31.694Z
Learning: Add comments for complex logic, but prefer self-documenting code
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Pre-commit hooks run ESLint and Prettier via Husky and lint-staged
Applied to files:
package.json
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Use PM2 process manager for production (`yarn start` uses process name `poixpixel-discord-bot`, `yarn restart` to reload); ensure TLS certificates are present at `certs/psql-ca.pem` for PostgreSQL and Redis
Applied to files:
package.json
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/{commands,events}/**/*.ts : Call `await validateInteraction(interaction)` before processing interactions to ensure the interaction is in-guild, channel is available, and for component interactions the original message is fetchable
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/commands/util/user-info.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/{commands,events}/**/*.ts : Use `safelyRespond(interaction, content)` from `src/util/helpers.ts` for all interaction responses; it handles reply vs. follow-up vs. skip logic and logs Discord API errors
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/commands/util/user-info.tssrc/util/counting/countingManager.tssrc/commands/util/help.tssrc/util/helpers.tssrc/events/messageEvents.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/events/interactionCreate.ts : Call `processCommandAchievements()` and `processMessageAchievements()` only once per event (in `src/events/interactionCreate.ts` and `src/util/levelingSystem.ts` respectively) to batch-process all relevant achievements efficiently
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/events/messageEvents.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/commands/**/*.ts : If a command takes longer than 3 seconds, call `interaction.deferReply()` immediately to prevent interaction token expiration
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-10-05T20:23:31.976Z
Learnt from: ahmadk953
Repo: ahmadk953/poixpixel-discord-bot PR: 568
File: src/commands/util/help.ts:38-43
Timestamp: 2025-10-05T20:23:31.976Z
Learning: In the Poixpixel Discord Bot codebase, processCommandAchievements() is called centrally in the interactionCreate event handler (src/events/interactionCreate.ts) after command execution for all commands. Individual command files should NOT call processCommandAchievements() directly, as this would duplicate the achievement processing logic.
Applied to files:
src/commands/util/recalculate-levels.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/commands/**/*.ts : All commands in `src/commands/**/*.ts` MUST default export an object with `data` (SlashCommandBuilder) and `async execute(interaction)` properties, satisfying the `Command` type
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/commands/util/user-info.tssrc/commands/util/help.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/commands/**/*.ts : Use `SlashCommandBuilder` for command definitions with required `setName()` and `setDescription()` calls
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/fun/rank.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/fun/counting.tssrc/commands/moderation/mute.tssrc/commands/util/config.tssrc/commands/util/user-info.tssrc/commands/util/help.ts
📚 Learning: 2025-04-17T05:13:22.634Z
Learnt from: ahmadk953
Repo: ahmadk953/poixpixel-discord-bot PR: 343
File: src/commands/util/xp.ts:80-82
Timestamp: 2025-04-17T05:13:22.634Z
Learning: In Discord.js v14, the `ephemeral: true` option is deprecated and should be replaced with `flags: ['Ephemeral']` when using interaction methods like `deferReply()` and `reply()`.
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/moderation/mute.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-04-17T05:13:22.634Z
Learnt from: ahmadk953
Repo: ahmadk953/poixpixel-discord-bot PR: 343
File: src/commands/util/xp.ts:80-82
Timestamp: 2025-04-17T05:13:22.634Z
Learning: In Discord.js v14, the `ephemeral: true` option is deprecated and should be replaced with `flags: ['Ephemeral']` when using methods like `interaction.deferReply()` and `interaction.reply()`.
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/util/server.tssrc/commands/fun/fact.tssrc/commands/moderation/mute.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-04-17T05:13:52.029Z
Learnt from: ahmadk953
Repo: ahmadk953/poixpixel-discord-bot PR: 343
File: src/commands/moderation/ban.ts:36-37
Timestamp: 2025-04-17T05:13:52.029Z
Learning: In Discord.js v14, `ephemeral: true` is deprecated and should be replaced with `flags: ['Ephemeral']` when making messages ephemeral in interaction replies and deferred replies.
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/moderation/mute.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-04-17T05:13:52.029Z
Learnt from: ahmadk953
Repo: ahmadk953/poixpixel-discord-bot PR: 343
File: src/commands/moderation/ban.ts:36-37
Timestamp: 2025-04-17T05:13:52.029Z
Learning: In Discord.js v14.17+, `ephemeral: true` is deprecated and should be replaced with `flags: ['Ephemeral']` when making messages ephemeral in interaction replies and deferred replies.
Applied to files:
src/commands/util/recalculate-levels.tssrc/commands/moderation/mute.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/**/*.ts : Use structured logging with metadata objects: `logger.info('message', { userId, commandName, guildId })` and import logger from `src/util/logger.js`
Applied to files:
src/commands/util/recalculate-levels.tssrc/db/functions/moderationFunctions.tssrc/util/counting/countingManager.tssrc/db/functions/memberFunctions.tssrc/events/messageEvents.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/events/interactionCreate.ts : Use handler maps (object literals) in `src/events/interactionCreate.ts` for routing buttons, modals, and select menus instead of switch statements; add new handlers to `giveawayHandlers`, `modalHandlers`, `selectHandlers` objects with customId prefixes
Applied to files:
src/commands/fun/fact.tssrc/commands/util/help.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/{commands,util/giveaways}/**/*.ts : If a guild has more than 25 eligible roles for giveaway configuration, use a modal for role input instead of select menu due to Discord's 25-option limit
Applied to files:
src/commands/fun/counting.tssrc/commands/util/user-info.tssrc/commands/util/help.tssrc/util/helpers.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/**/*.ts : Import database functions from `src/db/db.ts`, not direct DB access; use `handleDbError(error, operation)` for consistent error handling
Applied to files:
src/db/functions/moderationFunctions.tssrc/db/functions/factFunctions.tssrc/db/functions/memberFunctions.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/**/*.ts : Use `type` keyword for imports only used in type positions: `import type { Guild } from 'discord.js'`
Applied to files:
src/util/counting/countingManager.tssrc/db/functions/memberFunctions.tssrc/util/helpers.ts
📚 Learning: 2025-12-25T06:04:00.591Z
Learnt from: CR
Repo: ahmadk953/poixpixel-discord-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-25T06:04:00.591Z
Learning: Applies to src/**/*.ts : All Redis keys MUST be prefixed with `'bot:'` (e.g., `bot:counting:${guildId}`); always check `isRedisConnected()` before non-critical Redis operations
Applied to files:
src/util/counting/countingManager.ts
📚 Learning: 2025-03-01T05:51:01.720Z
Learnt from: ahmadk953
Repo: ahmadk953/poixpixel-discord-bot PR: 294
File: src/events/channelEvents.ts:152-177
Timestamp: 2025-03-01T05:51:01.720Z
Learning: The `channelUpdate` function in `src/events/channelEvents.ts` intentionally skips logging when the only change is to a channel's position (using the condition `oldChannel.position !== newChannel.position` in the early-return check). This is by design to prevent log spam from channel position changes, which can be frequent during server reorganizations.
Applied to files:
src/events/messageEvents.ts
🔇 Additional comments (1)
package.json (1)
39-52: LGTM — dependency migration is aligned.Biome and Ultracite are present as dev dependencies, matching the new
check/fixscripts and lint-staged usage.
| execute: (message: Message): Promise<void> => { | ||
| try { | ||
| if (message.author.bot || !message.guild) return; | ||
| if (message.author.bot || !message.guild) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| void handleLevelingMessage(message); | ||
| handleLevelingMessage(message); | ||
|
|
||
| const countingChannelId = config.channels.counting; | ||
| if (message.channel.id === countingChannelId) { | ||
| countingQueue.push(message); | ||
| processCountingQueue(); | ||
| } | ||
|
|
||
| return Promise.resolve(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no fire-and-forget leveling call remains in messageEvents.ts.
rg -n -C 2 'handleLevelingMessage\(message\);' --iglob 'messageEvents.ts'Repository: ahmadk953/poixpixel-discord-bot
Length of output: 340
🏁 Script executed:
# Find and examine the handleLevelingMessage function definition
fd -t f 'levelingSystem.ts' --exec grep -l 'handleLevelingMessage' {} \;Repository: ahmadk953/poixpixel-discord-bot
Length of output: 57
🏁 Script executed:
# Also check the imports in messageEvents.ts to understand where it comes from
head -50 src/events/messageEvents.ts | grep -n 'import\|from'Repository: ahmadk953/poixpixel-discord-bot
Length of output: 573
🏁 Script executed:
# Get the full execute function to understand context
sed -n '321,340p' src/events/messageEvents.tsRepository: ahmadk953/poixpixel-discord-bot
Length of output: 629
🏁 Script executed:
# Find handleLevelingMessage definition in levelingSystem.ts
rg -n 'export.*handleLevelingMessage|function handleLevelingMessage|const handleLevelingMessage' src/util/levelingSystem.ts -A 5Repository: ahmadk953/poixpixel-discord-bot
Length of output: 57
🏁 Script executed:
# Get full context of levelingSystem.ts around handleLevelingMessage
rg -n 'handleLevelingMessage' src/util/levelingSystem.ts -B 2 -A 10Repository: ahmadk953/poixpixel-discord-bot
Length of output: 57
🏁 Script executed:
# Search entire codebase for handleLevelingMessage
rg -n 'handleLevelingMessage' src/Repository: ahmadk953/poixpixel-discord-bot
Length of output: 233
🏁 Script executed:
# Check what's exported from levelingSystem.ts
rg -n 'export' src/util/levelingSystem.ts | head -30Repository: ahmadk953/poixpixel-discord-bot
Length of output: 499
🏁 Script executed:
# Get the full definition of handleLevelingMessage in messageEvents.ts
sed -n '115,150p' src/events/messageEvents.tsRepository: ahmadk953/poixpixel-discord-bot
Length of output: 1044
Make the execute function async and await handleLevelingMessage(message).
The function at line 115 is async and contains await statements for XP processing, role assignment, and level-up achievements. Calling it without await at line 327 causes the event to resolve before this work completes. Update the executor to be async, await the call, and replace Promise.resolve() returns with simple return statements.
Suggested fix
- execute: (message: Message): Promise<void> => {
+ execute: async (message: Message): Promise<void> => {
try {
if (message.author.bot || !message.guild) {
- return Promise.resolve();
+ return;
}
- handleLevelingMessage(message);
+ await handleLevelingMessage(message);
const countingChannelId = config.channels.counting;
if (message.channel.id === countingChannelId) {
countingQueue.push(message);
processCountingQueue();
}
- return Promise.resolve();
+ return;
} catch (error) {
logger.error('[MessageEvents] Error handling message create', error);
- return Promise.resolve();
}
},📝 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.
| execute: (message: Message): Promise<void> => { | |
| try { | |
| if (message.author.bot || !message.guild) return; | |
| if (message.author.bot || !message.guild) { | |
| return Promise.resolve(); | |
| } | |
| void handleLevelingMessage(message); | |
| handleLevelingMessage(message); | |
| const countingChannelId = config.channels.counting; | |
| if (message.channel.id === countingChannelId) { | |
| countingQueue.push(message); | |
| processCountingQueue(); | |
| } | |
| return Promise.resolve(); | |
| execute: async (message: Message): Promise<void> => { | |
| try { | |
| if (message.author.bot || !message.guild) { | |
| return; | |
| } | |
| await handleLevelingMessage(message); | |
| const countingChannelId = config.channels.counting; | |
| if (message.channel.id === countingChannelId) { | |
| countingQueue.push(message); | |
| processCountingQueue(); | |
| } | |
| return; | |
| } catch (error) { | |
| logger.error('[MessageEvents] Error handling message create', error); | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/events/messageEvents.ts` around lines 321 - 335, The execute function
should be declared async and must await the async work so the event handler
doesn't resolve early: change the execute: (message: Message): Promise<void> =>
{ ... } to an async function, await handleLevelingMessage(message), and replace
any Promise.resolve() return statements with simple returns (e.g., return;).
Also ensure you still enqueue counting messages (countingQueue.push) and call
processCountingQueue() as before; if processCountingQueue is async consider
awaiting it as well. Use the identifiers execute, handleLevelingMessage,
countingQueue, and processCountingQueue to locate and update the code.
| // If the interaction was deferred, send an ephemeral follow-up instead of | ||
| // editing the original reply, since its visibility (ephemeral/public) | ||
| // cannot be changed after deferReply. | ||
| if (interaction.deferred) { | ||
| await interaction.followUp({ content, flags: ['Ephemeral'] }); | ||
| await interaction.editReply({ | ||
| content, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // If we've already replied, send a follow-up message (ephemeral by default) | ||
| if (interaction.replied) { | ||
| await interaction.followUp({ content, flags: ['Ephemeral'] }); | ||
| await interaction.editReply({ | ||
| content, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Fresh interaction: send the initial reply (ephemeral by default) | ||
| await interaction.reply({ content, flags: ['Ephemeral'] }); | ||
| await interaction.reply({ | ||
| content, | ||
| flags: ephemeral ? MessageFlags.Ephemeral : undefined, | ||
| }); |
There was a problem hiding this comment.
Comments contradict behavior in safelyRespond.
The comments above both branches claim a follow-up ("send an ephemeral follow-up", "send a follow-up message (ephemeral by default)"), but both branches call interaction.editReply({ content }) — editing the original reply, not following up, and without any ephemeral flag. Either update the code to use followUp({ content, flags: MessageFlags.Ephemeral }) when that is the intended semantics, or correct the comments to match the current edit-reply behavior. As-is, this will confuse future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util/helpers.ts` around lines 564 - 586, In safelyRespond, the code paths
for interaction.deferred and interaction.replied call interaction.editReply but
the comments state they should send a follow-up (ephemeral by default); fix this
by replacing the editReply calls with interaction.followUp({ content, flags:
ephemeral ? MessageFlags.Ephemeral : undefined }) for the deferred and replied
branches to match the documented behavior, or if editReply was intended, update
the comments to state that the original reply is edited and remove any reference
to follow-up/ephemeral semantics; reference interaction.deferred,
interaction.replied, safelyRespond, followUp, editReply, and
MessageFlags.Ephemeral when making the change.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Migrates the repo’s JS/TS code quality tooling from ESLint/Prettier to Ultracite (Biome backend), updating scripts, docs, and CI, plus applying broadly consistent formatting/lint-driven refactors across the codebase.
Changes:
- Replaced ESLint/Prettier tooling with Ultracite/Biome (new
biome.jsonc, updated scripts, removed ESLint config). - Updated devcontainer, hooks/lint-staged, and GitHub Actions workflow to run Biome/Ultracite checks and publish SARIF.
- Refactored and reorganized a number of bot utilities/handlers to match new lint/format rules (imports, control flow, constants, logging helpers).
Reviewed changes
Copilot reviewed 110 out of 111 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/undeployCommands.ts | Formatting-only adjustments consistent with new formatter. |
| src/util/telemetry/otelLogTransport.ts | Refactors log attribute cleaning and OTEL transport setup for Biome style. |
| src/util/notificationHandler.ts | Replaces enum with as const object + union type per Ultracite standards. |
| src/util/logging/utils.ts | Logging helper refactors and type tweaks for embed fields. |
| src/util/logging/types.ts | Type reordering/cleanup to align with formatting and handler refactors. |
| src/util/logging/handlers/roleHandlers.ts | New role audit log embed field builders. |
| src/util/logging/handlers/purgeHandler.ts | New purge transcript generation and attachment logging. |
| src/util/logging/handlers/moderationHandlers.ts | New moderation log embed field builder. |
| src/util/logging/handlers/messageHandlers.ts | New message delete/edit log embed field builders. |
| src/util/logging/handlers/memberHandlers.ts | New member join/leave and name update embed field builders. |
| src/util/logging/handlers/channelHandlers.ts | New channel create/update/delete embed field builders. |
| src/util/logging/constants.ts | Numeric literal formatting updates for action colors. |
| src/util/logger.ts | Refactors global error handling and transport flushing/shutdown. |
| src/util/levelingSystem.ts | Schema import refactor + formatting and minor typing refinements. |
| src/util/giveaways/utils.ts | Formatting and small logic reshapes for requirement checking and sessions. |
| src/util/giveaways/types.ts | Type/property order adjustments matching new code style. |
| src/util/giveaways/modals.ts | Formatting-only tweaks to modal builders. |
| src/util/giveaways/dropdowns.ts | Formatting-only tweaks to select menu builders. |
| src/util/factManager.ts | Minor scheduling signature refactor + formatting updates. |
| src/util/eventLoader.ts | Import reordering and small formatting changes. |
| src/util/deployCommand.ts | Formatting and small control-flow adjustments for lint compliance. |
| src/util/dataRetention.ts | Refactor into helper functions for readability/maintainability with new style. |
| src/util/counting/types.ts | Type/property order adjustments. |
| src/util/counting/helpers.ts | Adds regex constants and refactors validation/eval flow for lint rules. |
| src/util/counting/durationUtils.ts | Small formatting/early-return tweaks. |
| src/util/counting/constants.ts | Formatting plus new exported regex constant. |
| src/util/configLoader.ts | Import reordering and formatting in error output. |
| src/util/achievementManager.ts | Control-flow simplification and formatting; numeric literal formatting. |
| src/util/achievementCardGenerator.ts | Import ordering and formatting for Biome. |
| src/types/EventTypes.ts | Member order formatting/consistency changes. |
| src/types/ConfigTypes.ts | Reorders config fields; no semantic change intended. |
| src/structures/ExtendedClient.ts | Marks properties readonly and formatting updates. |
| src/events/roleEvents.ts | Import ordering/formatting adjustments. |
| src/events/ready.ts | Refactors init sequence usage (Redis/Fact scheduling call sites updated). |
| src/events/reactionEvents.ts | Formatting and early-return block style updates. |
| src/events/memberEvents.ts | Import ordering and interaction/logging formatting updates. |
| src/events/interactionCreate.ts | Refactors handlers and adds constants for regex; formatting and error handling tweaks. |
| src/events/channelEvents.ts | Refactors permission change extraction and adds helper for readability. |
| src/discord-bot.ts | Formatting and small control-flow adjustments for lint compliance. |
| src/db/schema.ts | Switches table “Types” interfaces to InferSelectModel type aliases for some tables. |
| src/db/functions/utils/moderationUtils.ts | Changes moderation date normalization logic. |
| src/db/functions/moderationFunctions.ts | Refactors to use new schema table exports/types and updates signatures. |
| src/db/functions/memberFunctions.ts | Refactors to use new schema table exports/types and updates signatures. |
| src/db/functions/factFunctions.ts | Refactors to use new schema table exports/types; addFact now returns inserted id. |
| src/db/functions/achievementFunctions.ts | Refactors to use new schema table exports/types and formatting adjustments. |
| src/commands/util/xp.ts | Updates interaction validation/response handling and formatting. |
| src/commands/util/server.ts | Uses shared validate/respond helpers and adds guild-only messaging. |
| src/commands/util/rules.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/util/restart.ts | Uses shared validate/respond helpers and formatting adjustments. |
| src/commands/util/reload-config.ts | Uses shared validate/respond helpers and formatting adjustments. |
| src/commands/util/recalculate-levels.ts | Uses shared validate/respond helpers and formatting adjustments. |
| src/commands/util/ping.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/util/members.ts | Refactors pagination interaction handling with shared helpers and formatting. |
| src/commands/util/help.ts | Refactors category iteration/collector callbacks and uses shared helpers. |
| src/commands/testing/test-leave.ts | Uses shared validate/respond helpers and adds guild-only messaging. |
| src/commands/testing/test-join.ts | Uses shared validate/respond helpers and adds guild-only messaging. |
| src/commands/moderation/warn.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/moderation/unmute.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/moderation/unban.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/moderation/mute.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/moderation/kick.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/moderation/ban.ts | Uses shared validate/respond helpers and formatting updates. |
| src/commands/fun/rank.ts | Refactors to shared validate/respond helpers; import ordering changes. |
| src/commands/fun/leaderboard.ts | Refactors pagination interaction handling with shared helpers and formatting. |
| skills-lock.json | Adds Ultracite skill lock metadata. |
| package.json | Replaces lint/format scripts with Ultracite/Biome scripts and adds dependencies. |
| eslint.config.mjs | Removes ESLint flat config (migration away from ESLint). |
| drizzle.config.ts | Formatting adjustments for Biome. |
| biome.jsonc | Adds Biome config extending Ultracite preset. |
| README.md | Updates developer commands from ESLint/Prettier to Ultracite/Biome. |
| CONTRIBUTING.md | Updates contributor workflow/docs for Ultracite/Biome and Husky setup. |
| AGENTS.md | Adds Ultracite code standards documentation for agents. |
| .prettierrc.yml | Removes Prettier config (migration away from Prettier). |
| .prettierignore | Removes Prettier ignore file (migration away from Prettier). |
| .lintstagedrc.mjs | Moves staged formatting/linting to yarn ultracite fix + type-check on TS. |
| .husky/commit-msg | Updates commit-msg hook to run via package script. |
| .github/workflows/ultracite.yml | Adds Ultracite/Biome workflow producing SARIF + PR comment. |
| .github/workflows/eslint.yml | Removes ESLint SARIF workflow (migration away from ESLint). |
| .github/pull_request_template.md | Updates checklist commands to yarn check/fix/type-check. |
| .github/hooks/ultracite.json | Adds PostToolUse hook to run yarn fix with a skip list. |
| .github/copilot-instructions.md | Updates repo instructions to Ultracite/Biome commands and standards. |
| .github/PULL_REQUEST_TEMPLATE/refactor.md | Updates checklist commands to new scripts. |
| .github/PULL_REQUEST_TEMPLATE/feature.md | Updates checklist commands to new scripts. |
| .github/PULL_REQUEST_TEMPLATE/bug_fix.md | Updates checklist commands to new scripts. |
| .devcontainer/devcontainer.json | Switches devcontainer editor defaults/extensions to Biome. |
| .devcontainer/README.md | Updates devcontainer docs to reference Ultracite/Biome. |
| .agents/skills/ultracite/references/code-standards.md | Adds Ultracite standards reference doc. |
| .agents/skills/ultracite/SKILL.md | Adds Ultracite “skill” metadata/documentation. |
| const transportsList: ClosableTransport[] = | ||
| (logger as unknown as { transports?: ClosableTransport[] }).transports ?? | ||
| []; | ||
| for (const transport of transportsList) { | ||
| const flushResult = invokeSafely(transport.flush); | ||
| if (isPromiseLike(flushResult)) { | ||
| closePromises.push(flushResult); | ||
| continue; | ||
| } | ||
|
|
||
| const closeResult = invokeSafely(transport.close); | ||
| if (isPromiseLike(closeResult)) { | ||
| closePromises.push(closeResult); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Calling invokeSafely(transport.flush) / invokeSafely(transport.close) drops the method this binding, which can break transports whose flush/close rely on instance state. Wrap these calls so they’re invoked as methods on the transport (e.g., call through a lambda or use .call(transport)), ensuring correct behavior during fatal shutdown.
| const hasMessage = | ||
| typeof (meta as Record<string, unknown>).message === 'string'; | ||
| const hasName = typeof (meta as Record<string, unknown>).name === 'string'; | ||
| const isErrorShapedRecord = | ||
| hasStack || hasMessage || (hasName && (hasStack || hasMessage)); | ||
|
|
||
| // Handle Error instances explicitly | ||
| if (v instanceof Error) { | ||
| out['exception.type'] = v.name; | ||
| out['exception.message'] = v.message; | ||
| if (v.stack) out['exception.stacktrace'] = v.stack; | ||
| for (const [k, v] of Object.entries(meta || {})) { | ||
| if (isNumericKey(k)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
extractCommonErrorField is passed v instanceof Error, but v instanceof Error is always false here due to the earlier continue. This causes regressions for common fields like name/stack when log metadata contains plain { name: 'TypeError', stack: '...' }. Consider removing the isError parameter (or deriving it from the overall metadata shape) and handling name/stack independently of Error instances.
| '*.{js,mjs,ts,mts}': [prettierCommand, buildEslintCommand], | ||
| '*.json': [prettierCommand], | ||
| '*.{js,mjs,ts,mts,json}': (filenames) => | ||
| `yarn ultracite fix ${filenames.join(' ')}`, |
There was a problem hiding this comment.
Building the command via filenames.join(' ') does not properly escape filenames (spaces, quotes, backslashes), which can cause lint-staged failures or incorrect file targeting. Prefer shell-escaping/quoting each filename (e.g., JSON.stringify per filename) or returning an array form where supported so filenames are passed safely.
| `yarn ultracite fix ${filenames.join(' ')}`, | |
| `yarn ultracite fix ${filenames.map((filename) => JSON.stringify(filename)).join(' ')}`, |
| const createdAt = | ||
| record.createdAt != null ? new Date(record.createdAt) : undefined; | ||
| record.createdAt == null ? null : new Date(record.createdAt); | ||
| const expiresAt = | ||
| record.expiresAt != null ? new Date(record.expiresAt) : undefined; | ||
| record.expiresAt == null ? null : new Date(record.expiresAt); | ||
|
|
||
| return { | ||
| ...record, | ||
| createdAt: Number.isNaN(createdAt?.getTime()) ? undefined : createdAt, | ||
| expiresAt: Number.isNaN(expiresAt?.getTime()) ? undefined : expiresAt, | ||
| createdAt: | ||
| createdAt != null && !Number.isNaN(createdAt.getTime()) | ||
| ? createdAt | ||
| : (record.createdAt as Date), | ||
| expiresAt: | ||
| expiresAt != null && !Number.isNaN(expiresAt.getTime()) | ||
| ? expiresAt | ||
| : null, |
There was a problem hiding this comment.
The fallback createdAt: (record.createdAt as Date) can return a non-Date value at runtime (e.g., a cached/serialized string), which defeats the purpose of normalization and can break downstream code expecting Date. Prefer guaranteeing createdAt is always a valid Date (e.g., parse then fall back to a safe default or throw), and avoid type casts that don’t enforce runtime validity.
| "undeploy-commands": "tsx src/util/undeployCommands.ts", | ||
| "lint": "eslint ./src && tsc --noEmit", | ||
| "type-check": "tsc --noEmit", | ||
| "clean": "rimraf ./target", | ||
| "format": "prettier --check --ignore-path .prettierignore .", | ||
| "format:fix": "prettier --write --ignore-path .prettierignore .", | ||
| "postinstall": "husky" | ||
| "check": "ultracite check", | ||
| "fix": "ultracite fix", | ||
| "setup": "husky", | ||
| "commitlint": "commitlint --edit" |
There was a problem hiding this comment.
The PR description’s 'How was this tested?' section indicates checks were run with yarn lint / yarn format:fix, but those scripts are removed in favor of yarn check / yarn fix / yarn type-check. Update the PR description/checklist to match the new commands so reviewers can trust the reported verification steps.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (19)
src/util/telemetry/otelLogTransport.ts (1)
129-138: SimplifyisErrorShapedRecord—hasNameis dead code, and the casts are redundant.The expression on line 137-138 simplifies to just
hasStack || hasMessage: the third disjunct(hasName && (hasStack || hasMessage))can never flip the result becausehasStack || hasMessageis already evaluated as the first term. SohasNameis computed but never affects the outcome, which is almost certainly not what was intended.Also,
metais already typedRecord<string, unknown>, so theas Record<string, unknown>casts on lines 130–133 (and line 135–136) are no-ops.If the intent was "treat a record with
nameplus any error-ish hint as error-shaped," thenhasNameis already subsumed; if the intent was something stricter (e.g., require bothnameandstacktogether), the predicate needs to be rewritten. Either way, the current form is misleading.♻️ Suggested simplification
- const hasStack = - (typeof (meta as Record<string, unknown>).stack === 'string' && - ((meta as Record<string, unknown>).stack as string).includes('\n')) || - (typeof (meta as Record<string, unknown>).stack === 'string' && - ((meta as Record<string, unknown>).stack as string).includes(' at ')); - const hasMessage = - typeof (meta as Record<string, unknown>).message === 'string'; - const hasName = typeof (meta as Record<string, unknown>).name === 'string'; - const isErrorShapedRecord = - hasStack || hasMessage || (hasName && (hasStack || hasMessage)); + const stackVal = meta.stack; + const hasErrorShapedStack = + typeof stackVal === 'string' && + (stackVal.includes('\n') || stackVal.includes(' at ')); + const hasMessage = typeof meta.message === 'string'; + const isErrorShapedRecord = hasErrorShapedStack || hasMessage;As per coding guidelines: "Extract complex conditions into well-named boolean variables" and "Leverage TypeScript's type narrowing instead of type assertions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/telemetry/otelLogTransport.ts` around lines 129 - 138, The isErrorShapedRecord predicate is needlessly complex: remove the redundant casts on meta and drop hasName (dead code) and set isErrorShapedRecord to simply hasStack || hasMessage; specifically, replace the current hasStack/hasMessage/hasName definitions to use meta (already typed) directly (e.g., typeof meta.stack === 'string' && checks for '\n' or ' at ') and then assign isErrorShapedRecord = hasStack || hasMessage, removing hasName and its use; if you intended a stricter rule instead, explicitly change the predicate to require hasName && (hasStack || hasMessage) so the intent is clear..github/workflows/ultracite.yml (1)
3-7: Bothpushandpull_requesttriggers will run the job twice on PR branches in this repo.For branches that exist in this repo and have an open PR, both events fire and the workflow runs twice (uploading SARIF twice as well, which CodeQL deduplicates by commit but still costs minutes). Consider scoping
push:to specific branches (e.g.,main,development) so PR runs are exclusively driven bypull_request. Not blocking, just noting the double-run cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ultracite.yml around lines 3 - 7, The workflow currently triggers on both push and pull_request which causes duplicate runs for PR branches; update the on: block in ultracite.yml to scope push to specific branches (e.g., change push: to push: branches: [main, development]) while keeping pull_request: and schedule: unchanged so PRs trigger only via pull_request and pushes only on your selected branches (refer to the on: push, on: pull_request, and on: schedule keys in the diff).CONTRIBUTING.md (1)
76-83: Code-block indentation under numbered items is inconsistent with the rest of the file.The new items at lines 76-82, 187-192, and 491-494 indent their nested paragraph and fenced code block by 2 spaces, while every other numbered item in this file (e.g., lines 60-62, 67, 73, 178) uses 3 spaces to align with the content position after
1.. Strict CommonMark parsers (and some doc renderers like GitBook, where these docs are now hosted) treat 2-space-indented content as a sibling block, breaking the list. GitHub itself is lenient, but the inconsistency is best avoided.📄 Proposed fix (lines 76-82)
-1. **Install Git hooks** - - After installing dependencies run the setup script to initialize Husky git hooks so local checks (commit-msg, pre-commit, etc.) run as expected: - - ```bash - yarn setup - ``` +1. **Install Git hooks** + + After installing dependencies run the setup script to initialize Husky git hooks so local checks (commit-msg, pre-commit, etc.) run as expected: + + ```bash + yarn setup + ```Apply the same 3-space adjustment to lines 187-192 and 491-494.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 76 - 83, List items under numbered entries use 2-space indentation causing list breakage; fix by increasing indentation for the nested paragraph and fenced code blocks to 3 spaces so they align under the numeral. In the "1. **Install Git hooks**" block change the two-space indents before the paragraph and the ```bash fenced block and its content to three spaces, and make the same 3-space adjustment for the other affected numbered entries (the blocks currently at the other two locations mentioned in the review). Ensure the paragraph and the opening/closing ``` lines as well as the code line ("yarn setup") are indented with three spaces to match the rest of the file.src/db/functions/factFunctions.ts (1)
21-26: Overly permissive parameter type foraddFact.Typing the input as
factTableTypes(the full select model) means callers can pass server-managed columns likeid,usedOn, or any timestamp fields, which are then silently ignored by destructuring. Consider narrowing to the input shape you actually accept, e.g.Pick<factTableTypes, 'content' | 'source' | 'addedBy'> & { approved?: boolean }, to make the contract explicit and prevent accidental misuse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/functions/factFunctions.ts` around lines 21 - 26, The addFact function accepts the entire factTableTypes which allows callers to pass server-managed fields (e.g., id, usedOn, timestamps) that are ignored; change its parameter type to a narrow input shape such as Pick<factTableTypes, 'content' | 'source' | 'addedBy'> & { approved?: boolean } (or define an explicit AddFactInput type) and update the function signature for addFact to use that type so the contract explicitly disallows server-managed columns.src/commands/util/members.ts (1)
63-66: Extract Discord's 25-option limit as a named constant.
25is Discord's hard limit onStringSelectMenuoptions and is replicated in at leastsrc/commands/util/members.tsandsrc/commands/fun/leaderboard.ts. Per guidelines on magic numbers, lifting this into a sharedMAX_SELECT_MENU_OPTIONS = 25 as const(e.g., insrc/util/constants.tsor alongside the helpers) would make both call sites self-documenting and easier to update if Discord ever changes the cap.As per coding guidelines: "Use meaningful variable names instead of magic numbers — extract constants with descriptive names".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/members.ts` around lines 63 - 66, Extract the magic number 25 into a named constant (e.g., MAX_SELECT_MENU_OPTIONS = 25 as const) and use it where Discord select-menu option limits are checked; update the condition in getSelectMenuRow (replace pages.length > 25) to pages.length > MAX_SELECT_MENU_OPTIONS and do the same replacement in src/commands/fun/leaderboard.ts (or wherever the same 25 is used) so the limit is self-documenting and centrally maintained.src/commands/util/config.ts (1)
305-305: Style nit: preferMessageFlags.Ephemeralfor consistency.Other files touched in this PR (e.g.,
src/util/helpers.ts,src/commands/fun/rank.ts) importMessageFlagsfromdiscord.jsand useMessageFlags.Ephemeral. The string-literal formflags: ['Ephemeral']is equivalent and supported, but mixing styles makes greps and refactors slightly harder. Consider standardizing onMessageFlags.Ephemeralhere (and at line 253).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/config.ts` at line 305, The defer reply call uses a string literal for flags—change await interaction.deferReply({ flags: ['Ephemeral'] }) (and the similar call at the other occurrence around line 253) to use the enum constant from discord.js by importing MessageFlags and passing flags: [MessageFlags.Ephemeral]; update the imports if needed and ensure both places consistently use MessageFlags.Ephemeral for style and easier refactors.src/db/functions/levelFunctions.ts (1)
79-94: Verify the post-insert SELECT runs against the same connection/transaction context.The new pattern of
INSERT ... ON CONFLICT DO NOTHINGfollowed by an independent SELECT (each through its ownwithDbRetryDrizzlecall) is susceptible to a brief race window: a concurrentdeleteUserLevelor pool failover between the two retries can cause the SELECT to return no row and triggerFailed to load user level after create. For a Discord bot this is extremely unlikely in practice, but if you want fully atomic semantics consider usingINSERT ... ON CONFLICT DO UPDATE SET <noop> RETURNING *(or wrapping both calls in a singledb.transaction) so the row is guaranteed to be returned in one round trip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/functions/levelFunctions.ts` around lines 79 - 94, The post-insert SELECT (inside withDbRetryDrizzle that assigns createdLevel) can observe a race because the INSERT ON CONFLICT DO NOTHING and the subsequent SELECT run as separate calls; update the logic in the function using either a single INSERT ... ON CONFLICT DO UPDATE SET <noop> RETURNING * to get the row in the same statement, or execute both statements inside a single db.transaction so the SELECT runs on the same connection/transaction; locate the code around withDbRetryDrizzle, createdLevel, and levelTable (and consider interactions with deleteUserLevel) and replace the two-call pattern with one of these atomic approaches.src/commands/util/user-info.ts (1)
132-154: Use??instead of||for fallback to align with coding guidelines.
Array.prototype.findreturns either an object orundefined, so||happens to work, but the codebase guidelines prefer nullish coalescing for safer property access.♻️ Proposed refactor
- const currentMute = - muteModerations.find((m) => m.active) || muteModerations[0]; + const currentMute = + muteModerations.find((m) => m.active) ?? muteModerations[0]; @@ - const currentBan = banModerations.find((m) => m.active) || banModerations[0]; + const currentBan = banModerations.find((m) => m.active) ?? banModerations[0];As per coding guidelines: "Use optional chaining (
?.) and nullish coalescing (??) for safer property access."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/user-info.ts` around lines 132 - 154, The fallback logic uses || which should be replaced with nullish coalescing (??) to follow guidelines: change the assignment of currentMute (currently "const currentMute = muteModerations.find((m) => m.active) || muteModerations[0];") to use ?? instead of ||, and likewise change currentBan in buildCurrentBanField (currently "const currentBan = banModerations.find((m) => m.active) || banModerations[0];") to use ??; this ensures undefined results from .find are handled safely without treating falsy values like 0 or '' as missing.src/events/messageEvents.ts (1)
344-363: DropPromise.resolve()in the asyncexecute.Now that
executeisasync, returningPromise.resolve()is redundant — a barereturn;resolves the same promise. Tightens the code without behavior change.♻️ Proposed refactor
execute: async (message: Message): Promise<void> => { try { if (message.author.bot || !message.guild) { - return Promise.resolve(); + return; } @@ - return Promise.resolve(); + return; } catch (error) { logger.error('[MessageEvents] Error handling message create', error); - return Promise.resolve(); + return; } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/events/messageEvents.ts` around lines 344 - 363, In the execute async function in messageEvents (execute: async (message: Message): Promise<void>), remove the redundant Promise.resolve() usages and replace them with bare returns: use plain return; after early exits (when message.author.bot || !message.guild) and at the end of the try block, and replace the Promise.resolve() in the catch block with a simple return; so the async function resolves naturally without unnecessary Promise.resolve() calls.src/commands/fun/achievements.ts (1)
580-590: Avoid theas Dateassertion via local narrowing.
isAchievementEarnedalready provesearnedAt != null, but TypeScript can't propagate the narrowing across the function boundary, so the cast remains. CapturingearnedAtin a local and narrowing inline removes the assertion.♻️ Proposed refactor
- if (isAchievementEarned(achievement)) { - const earnedDate = new Date(achievement.earnedAt as Date); - fieldValue += `\n✅ **Completed**: <t:${Math.floor( - earnedDate.getTime() / 1000 - )}:R>`; - } else { + const { earnedAt } = achievement; + if (earnedAt != null && new Date(earnedAt).getTime() > 0) { + const earnedDate = new Date(earnedAt); + fieldValue += `\n✅ **Completed**: <t:${Math.floor( + earnedDate.getTime() / 1000 + )}:R>`; + } else {As per coding guidelines: "Leverage TypeScript's type narrowing instead of type assertions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/achievements.ts` around lines 580 - 590, Replace the type assertion "achievement.earnedAt as Date" by first capturing and narrowing the property locally: inside the isAchievementEarned(achievement) branch read const earnedAt = achievement.earnedAt and rely on the local truthiness check (or the knowledge from isAchievementEarned) to create earnedDate = new Date(earnedAt); remove the "as Date" cast and use earnedDate where currently referenced (this touches the block that builds fieldValue when isAchievementEarned returns true, alongside symbols achievement.earnedAt and earnedDate).src/util/counting/constants.ts (1)
36-41: Restoreas constonMILESTONE_REACTIONS.The trailing
as constwas dropped in this change. Without it, the emoji values widen from string literals tostring, so consumers lose the literal type information that const assertions provide. Thekeyof typeof MILESTONE_REACTIONSusage inhelpers.ts(deriveMilestone) still works, but immutability and value-literal narrowing are lost.♻️ Proposed refactor
export const MILESTONE_REACTIONS = { normal: '✅', multiples25: '✨', multiples50: '⭐', multiples100: '🎉', -}; +} as const;As per coding guidelines: "Use const assertions (
as const) for immutable values and literal types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/counting/constants.ts` around lines 36 - 41, Restore the const assertion on MILESTONE_REACTIONS by appending "as const" to the exported object literal so the emoji values remain string literal types and the object is readonly; update the exported constant MILESTONE_REACTIONS declaration (the object with keys normal, multiples25, multiples50, multiples100) to use a const assertion to preserve literal types used by helpers like deriveMilestone.src/commands/util/help.ts (2)
211-215: UsesafelyRespondfor the "command not found" reply.This text-only response bypasses the centralized response/error helper used throughout this command. Prefer
safelyRespond(interaction, ...)to keep reply/follow-up logic and Discord error logging consistent.♻️ Proposed refactor
if (!cmd) { - return interaction.editReply({ - content: `Command \`${commandName}\` not found.`, - }); + await safelyRespond(interaction, `Command \`${commandName}\` not found.`); + return; }As per coding guidelines: "Use
safelyRespond(interaction, content)fromsrc/util/helpers.tsfor all interaction responses."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/help.ts` around lines 211 - 215, Replace the direct interaction.editReply call in the branch where cmd is falsy with the centralized helper: call safelyRespond(interaction, `Command \`${commandName}\` not found.`) (or safelyRespond(interaction, { content: `Command \`${commandName}\` not found.` }) if your helper expects an options object); this changes the code around the cmd check (variables: cmd, commandName, interaction) to use safelyRespond imported from src/util/helpers.ts so reply/follow-up handling and error logging remain consistent.
143-151: Don't silently swallow invalid component interactions inside the collector.When
validateInteraction(i)returnsfalse, the handler returns without acknowledging the component interaction, leaving the clicking user with a hanging "thinking…" / failed-interaction state. Mirror the pattern used at the top ofexecute(Lines 34-41) and respond ephemerally before exiting.🛠️ Suggested fix
collector.on('collect', async (i) => { if (!(await validateInteraction(i))) { + await safelyRespond(i, 'Invalid interaction. Please try again.', true); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/util/help.ts` around lines 143 - 151, When the collector callback returns early because validateInteraction(i) is false it currently leaves the component interaction unacknowledged; update the collector.on('collect', async (i) => { ... }) handler to call safelyRespond(i, 'Invalid interaction.', true) (or mirror the same ephemeral response used at the top of execute) before returning so the component is acknowledged; specifically change the branch that checks validateInteraction(i) to send an ephemeral response via safelyRespond and then return, referencing validateInteraction, safelyRespond, the collector.on('collect' handler, and the interaction/i variables to locate the code.src/commands/fun/fact.ts (1)
129-186: Drop redundantcreatePaginationButtonsparameter.
createPaginationButtonsis already imported at the top of the file (Line 17), so passing it as a parameter tohandlePaginationCollectis unnecessary indirection. Inline the import usage and simplify the signature.♻️ Proposed refactor
async function handlePaginationCollect( i: MessageComponentInteraction, interaction: ChatInputCommandInteraction, pages: EmbedBuilder[], - createPaginationButtons: ( - totalPages: number, - currentPage: number - ) => ActionRowBuilder<ButtonBuilder>, currentPage: number, setCurrentPage: (newPage: number) => void ): Promise<void> { @@ - collector.on('collect', async (i) => { - await handlePaginationCollect( - i, - interaction, - pages, - createPaginationButtons, - currentPage, - (newPage) => { - currentPage = newPage; - } - ); - }); + collector.on('collect', async (i) => { + await handlePaginationCollect( + i, + interaction, + pages, + currentPage, + (newPage) => { + currentPage = newPage; + } + ); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/fact.ts` around lines 129 - 186, The function handlePaginationCollect currently accepts createPaginationButtons as a parameter even though that helper is already imported; remove the createPaginationButtons parameter from handlePaginationCollect's signature and calls, and inside the function call the imported createPaginationButtons directly (i.e., replace createPaginationButtons(pages.length, newPage) with the imported function). Update all call sites of handlePaginationCollect to stop passing createPaginationButtons and ensure the components array still uses .toJSON() on the ActionRowBuilder returned by the imported createPaginationButtons; keep the rest of the parameter list and logic in handlePaginationCollect unchanged.src/util/counting/countingManager.ts (3)
191-198: Log message wording is slightly off.
VALID_MATH_EXPR_REpermits digits, operators, parentheses, and whitespace, so the rejected content isn't strictly "non-numeric" — it's "non-arithmetic". Consider tweaking to e.g.'Ignored non-arithmetic message'to keep debug logs accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/counting/countingManager.ts` around lines 191 - 198, The debug log message in CountingManager uses misleading wording ("Ignored non-numeric message") even though VALID_MATH_EXPR_RE allows operators and parentheses; update the logger.debug call to use a clearer message such as "Ignored non-arithmetic message" (keep the existing metadata: user and contentLength) so logs accurately reflect rejections of non-arithmetic input; locate the check around VALID_MATH_EXPR_RE.test(trimmed) and change only the string passed to logger.debug.
226-244: Extract the100rollback threshold/step into a named constant.Both the threshold check (
data.currentCount > 100) and the rollback step (Math.floor(data.currentCount / rollbackStep) * rollbackStep) hard-code100. The intent (rollback to nearest hundred when above 100, full reset otherwise) is non-obvious from a bare literal, and the two uses can drift if one is ever tweaked.Move into
src/util/counting/constants.ts, e.g.ROLLBACK_THRESHOLD = 100/ROLLBACK_STEP = 100, and import here.As per coding guidelines: "Use meaningful variable names instead of magic numbers - extract constants with descriptive names".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/counting/countingManager.ts` around lines 226 - 244, Extract the magic number 100 into named constants by adding ROLLBACK_THRESHOLD and ROLLBACK_STEP to src/util/counting/constants.ts and importing them into src/util/counting/countingManager.ts; then replace the hard-coded checks and calculations (the conditional data.currentCount > 100, the const rollbackStep = 100, and Math.floor(data.currentCount / rollbackStep) * rollbackStep) with the imported ROLLBACK_THRESHOLD and ROLLBACK_STEP so setCount(rollbackTo), the logger.debug call, and the returned rolledBackTo use the computed rollback based on those constants while preserving existing behavior; refer to symbols data.currentCount, setCount, logger.debug, rollbackTo and rolledBackTo to locate the changes.
222-261: Type thereasonparameter directly to drop the casts.
handleRollbackOrResetacceptsreason: stringand then assertsreason as CountingProcessInvalidReasonat both return sites. Typing the parameter asCountingProcessInvalidReasoneliminates the assertions, enables compile-time validation at every call site, and aligns with the guideline to "leverage type narrowing instead of type assertions".♻️ Suggested change
- async function handleRollbackOrReset( - reason: string - ): Promise<CountingProcessResult> { + async function handleRollbackOrReset( + reason: CountingProcessInvalidReason + ): Promise<CountingProcessResult> { if (data.currentCount > 100) { const rollbackStep = 100; const rollbackTo = Math.floor(data.currentCount / rollbackStep) * rollbackStep; await setCount(rollbackTo); logger.debug('[CountingManager] Invalid number caused rollback', { user: message.author.id.slice(-4), previousCount: data.currentCount, rolledBackTo: rollbackTo, reason, }); return { isValid: false, expectedCount: expected, - reason: reason as CountingProcessInvalidReason, + reason, rolledBackTo: rollbackTo, }; } await resetCounting(); logger.debug('[CountingManager] Invalid number caused reset', { user: message.author.id.slice(-4), previousCount: data.currentCount, reason, }); return { isValid: false, expectedCount: expected, - reason: reason as CountingProcessInvalidReason, + reason, rolledBackTo: 0, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util/counting/countingManager.ts` around lines 222 - 261, Change the handleRollbackOrReset function signature so its reason parameter is typed as CountingProcessInvalidReason instead of string (function: handleRollbackOrReset), then remove the redundant casts of reason as CountingProcessInvalidReason in both returned objects; update any call sites to pass a valid CountingProcessInvalidReason value (or refine their types) so callers compile without assertions. This ensures compile-time validation and eliminates the need for type assertions in the return objects (rolledBackTo branches) while keeping behavior unchanged.src/commands/fun/counting.ts (2)
270-506: Extract the duplicated pagination pipeline into a shared helper.
handleListBansandhandleListWarningsshare ~120 lines of essentially identical code: thegetSelectRowbuilder, components composition, the message-component collector, the user-id filter, thegetUpdatedPageFromComponentupdate step, and thesafeRemoveComponentscleanup. The only differences are thecustomIdprefix (counting_listbans_selectvscounting_listwarnings_select) and the embed array.Consolidate into a single helper (e.g., in
src/util/helpers.ts) likerunEmbedPaginator(interaction, pages, { selectCustomId, timeMs })that handlesgetSelectRow, components, and the collector lifecycle. This will also let you fix the duplicated magic number25(lines 315, 433) once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/counting.ts` around lines 270 - 506, handleListBans and handleListWarnings duplicate the entire pagination pipeline (getSelectRow, components composition, message creation, collector handling, user filter, page updates, and safeRemoveComponents); extract this into a shared helper like runEmbedPaginator(interaction, pages, { selectCustomId, timeMs }) placed in src/util/helpers.ts which implements the select/menu builder (replacing the hardcoded 25 with a constant), composes components using createPaginationButtons, wires the MessageComponentCollector, calls getUpdatedPageFromComponent to update currentPage, filters by interaction.user.id, defers/edits replies, and calls safeRemoveComponents on end; update both handlers to call runEmbedPaginator with their pages and their customId (e.g., 'counting_listbans_select'/'counting_listwarnings_select') and remove the duplicated blocks (references: handleListBans, handleListWarnings, getSelectRow, createPaginationButtons, getUpdatedPageFromComponent, safeRemoveComponents).
313-342: Replace the literal25with a named constant.Lines 315 and 433 both hard-code
25, which is Discord'sStringSelectMenuoption cap. Extract a module-level (or shared) constant such asMAX_SELECT_MENU_OPTIONS = 25to make the intent explicit and ensure both list views stay in sync if the limit ever changes.As per coding guidelines: "Use meaningful variable names instead of magic numbers - extract constants with descriptive names".
♻️ Suggested change
+const MAX_SELECT_MENU_OPTIONS = 25; + // ... const getSelectRow = () => { - if (pages.length > 25) { + if (pages.length > MAX_SELECT_MENU_OPTIONS) { return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/fun/counting.ts` around lines 313 - 342, The code uses the magic number 25 when building the select menu in getSelectRow (and elsewhere) which is Discord's option cap; introduce a shared module-level constant such as MAX_SELECT_MENU_OPTIONS = 25 and replace the literal 25 occurrences with that constant (e.g., update the conditional in getSelectRow that checks pages.length > 25 and any other hard-coded 25 checks) so the limit is explicit and consistent across uses (references: getSelectRow, pages.length, StringSelectMenuBuilder/select creation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ultracite.yml:
- Around line 78-90: Wrap the SARIF file read/parse around
JSON.parse(fs.readFileSync(reportPath, 'utf8')) in a try/catch to handle
partial/malformed SARIF; if parsing fails, catch the error, log a concise
message referencing reportPath and the error, and set runs/results to empty so
subsequent code that computes levelCounts ({ error, warning, note }) still runs
safely; ensure you keep the existing logic that defaults level to 'warning' and
updates levelCounts so the step doesn't throw and still produces a meaningful
result when parsing fails.
In `@src/commands/fun/achievements.ts`:
- Around line 341-349: The unauthorized-button handler inside
buttonCollector.on('collect', async (i: ButtonInteraction) => { ... }) sends the
ephemeral warning to the original interaction; change the call to safelyRespond
so it responds to the button interaction `i` (i.e., use safelyRespond(i, 'You
cannot use these buttons.', true)) so the clicking user receives the ephemeral
message (mirror the select-menu behavior used in the select collector).
In `@src/commands/fun/counting.ts`:
- Around line 117-120: The guard that checks for a missing guild (e.g., the
block with "const guild = interaction.guild; if (!guild) { return; }") returns
silently after interaction.deferReply() was already called in execute(), leaving
the interaction hung; update that guard (and the three other similar guards) to
call safelyRespond(interaction, "An error occurred") or a short generic error
message before returning so the deferred reply is fulfilled; ensure you use the
existing safelyRespond helper, and keep the guard early-return behavior after
calling safelyRespond to avoid further execution.
In `@src/commands/util/config.ts`:
- Around line 249-256: Replace the direct i.reply call in the collector's
'collect' handler with the project's interaction-guard pattern: first call await
validateInteraction(i) to ensure the interaction/channel/message is valid, then
call await safelyRespond(i, 'You cannot use this pagination.', true) (true =
ephemeral) instead of i.reply; keep the same early return behavior when
i.user.id !== interaction.user.id so the handler exits after safelyRespond.
Target the collector.on('collect', async (i: MessageComponentInteraction) => {
... }) block and update the non-invoker branch to use validateInteraction and
safelyRespond.
In `@src/db/functions/factFunctions.ts`:
- Around line 126-142: The branch in getRandomUnusedFact that resets usedOn then
unconditionally recurses can infinite-loop if there are no approved facts;
change it so after calling withDbRetryDrizzle(... update(factTable).set({
usedOn: null }).where(eq(factTable.approved, true))) you check the update result
(e.g., affected rows/count returned by the DB driver) or immediately re-query
facts (using the same logic that produced `facts`) and if zero rows were
affected or the re-query is still empty, do not recurse — instead return a
sentinel (null) or throw a clear error; only recurse when the reset actually
changed rows (or the re-query returns >0). Ensure you reference the existing
symbols factTable, withDbRetryDrizzle, invalidateCache, cacheKey, and
getRandomUnusedFact when making this change.
In `@src/db/functions/moderationFunctions.ts`:
- Around line 30-32: Update the inconsistent log message in
moderationFunctions.ts: in the function that logs "[moderationDbFunctions]
Database not initialized, update member moderation history" (the update member
moderation history path), change the message to include "cannot" so it matches
other messages (e.g., "[moderationDbFunctions] Database not initialized, cannot
update member moderation history").
In `@src/events/messageEvents.ts`:
- Around line 268-296: The code currently calls
shouldAllowRestoreCountingMessage twice causing duplicate guild.fetchAuditLogs
calls; change the flow to fetch audit-log once and reuse its result by either
(A) removing the standalone shouldAllowRestoreCountingMessage call and awaiting
maybeRestoreCountingMessage(...) and assign executor from its returned value, or
(B) perform a single shouldAllowRestoreCountingMessage(...) at the top, capture
{ allowed, executor } and pass those values into
maybeRestoreCountingMessage(message, guild, { allowed, executor }) (or adjust
maybeRestoreCountingMessage signature to accept them) so that
guild.fetchAuditLogs is only invoked once per delete event; update logger
handling to use the single fetched executor and preserve existing error
handling.
In `@src/util/helpers.ts`:
- Around line 246-252: The zero/negative-delay branch in schedule() invokes cb()
via setTimeout without awaiting or catching rejections, causing unhandled
promise rejections; change that branch to run the callback inside the same async
try/catch error-handling used by the main schedule path (e.g., wrap the
setTimeout handler in an async IIFE or call Promise.resolve(cb()).catch(...))
and forward errors to the same logger used by schedule (the same error logging
call used in the main await cb() catch block) so async rejections are logged
consistently for delayMs <= 0.
---
Nitpick comments:
In @.github/workflows/ultracite.yml:
- Around line 3-7: The workflow currently triggers on both push and pull_request
which causes duplicate runs for PR branches; update the on: block in
ultracite.yml to scope push to specific branches (e.g., change push: to push:
branches: [main, development]) while keeping pull_request: and schedule:
unchanged so PRs trigger only via pull_request and pushes only on your selected
branches (refer to the on: push, on: pull_request, and on: schedule keys in the
diff).
In `@CONTRIBUTING.md`:
- Around line 76-83: List items under numbered entries use 2-space indentation
causing list breakage; fix by increasing indentation for the nested paragraph
and fenced code blocks to 3 spaces so they align under the numeral. In the "1.
**Install Git hooks**" block change the two-space indents before the paragraph
and the ```bash fenced block and its content to three spaces, and make the same
3-space adjustment for the other affected numbered entries (the blocks currently
at the other two locations mentioned in the review). Ensure the paragraph and
the opening/closing ``` lines as well as the code line ("yarn setup") are
indented with three spaces to match the rest of the file.
In `@src/commands/fun/achievements.ts`:
- Around line 580-590: Replace the type assertion "achievement.earnedAt as Date"
by first capturing and narrowing the property locally: inside the
isAchievementEarned(achievement) branch read const earnedAt =
achievement.earnedAt and rely on the local truthiness check (or the knowledge
from isAchievementEarned) to create earnedDate = new Date(earnedAt); remove the
"as Date" cast and use earnedDate where currently referenced (this touches the
block that builds fieldValue when isAchievementEarned returns true, alongside
symbols achievement.earnedAt and earnedDate).
In `@src/commands/fun/counting.ts`:
- Around line 270-506: handleListBans and handleListWarnings duplicate the
entire pagination pipeline (getSelectRow, components composition, message
creation, collector handling, user filter, page updates, and
safeRemoveComponents); extract this into a shared helper like
runEmbedPaginator(interaction, pages, { selectCustomId, timeMs }) placed in
src/util/helpers.ts which implements the select/menu builder (replacing the
hardcoded 25 with a constant), composes components using
createPaginationButtons, wires the MessageComponentCollector, calls
getUpdatedPageFromComponent to update currentPage, filters by
interaction.user.id, defers/edits replies, and calls safeRemoveComponents on
end; update both handlers to call runEmbedPaginator with their pages and their
customId (e.g., 'counting_listbans_select'/'counting_listwarnings_select') and
remove the duplicated blocks (references: handleListBans, handleListWarnings,
getSelectRow, createPaginationButtons, getUpdatedPageFromComponent,
safeRemoveComponents).
- Around line 313-342: The code uses the magic number 25 when building the
select menu in getSelectRow (and elsewhere) which is Discord's option cap;
introduce a shared module-level constant such as MAX_SELECT_MENU_OPTIONS = 25
and replace the literal 25 occurrences with that constant (e.g., update the
conditional in getSelectRow that checks pages.length > 25 and any other
hard-coded 25 checks) so the limit is explicit and consistent across uses
(references: getSelectRow, pages.length, StringSelectMenuBuilder/select
creation).
In `@src/commands/fun/fact.ts`:
- Around line 129-186: The function handlePaginationCollect currently accepts
createPaginationButtons as a parameter even though that helper is already
imported; remove the createPaginationButtons parameter from
handlePaginationCollect's signature and calls, and inside the function call the
imported createPaginationButtons directly (i.e., replace
createPaginationButtons(pages.length, newPage) with the imported function).
Update all call sites of handlePaginationCollect to stop passing
createPaginationButtons and ensure the components array still uses .toJSON() on
the ActionRowBuilder returned by the imported createPaginationButtons; keep the
rest of the parameter list and logic in handlePaginationCollect unchanged.
In `@src/commands/util/config.ts`:
- Line 305: The defer reply call uses a string literal for flags—change await
interaction.deferReply({ flags: ['Ephemeral'] }) (and the similar call at the
other occurrence around line 253) to use the enum constant from discord.js by
importing MessageFlags and passing flags: [MessageFlags.Ephemeral]; update the
imports if needed and ensure both places consistently use MessageFlags.Ephemeral
for style and easier refactors.
In `@src/commands/util/help.ts`:
- Around line 211-215: Replace the direct interaction.editReply call in the
branch where cmd is falsy with the centralized helper: call
safelyRespond(interaction, `Command \`${commandName}\` not found.`) (or
safelyRespond(interaction, { content: `Command \`${commandName}\` not found.` })
if your helper expects an options object); this changes the code around the cmd
check (variables: cmd, commandName, interaction) to use safelyRespond imported
from src/util/helpers.ts so reply/follow-up handling and error logging remain
consistent.
- Around line 143-151: When the collector callback returns early because
validateInteraction(i) is false it currently leaves the component interaction
unacknowledged; update the collector.on('collect', async (i) => { ... }) handler
to call safelyRespond(i, 'Invalid interaction.', true) (or mirror the same
ephemeral response used at the top of execute) before returning so the component
is acknowledged; specifically change the branch that checks
validateInteraction(i) to send an ephemeral response via safelyRespond and then
return, referencing validateInteraction, safelyRespond, the
collector.on('collect' handler, and the interaction/i variables to locate the
code.
In `@src/commands/util/members.ts`:
- Around line 63-66: Extract the magic number 25 into a named constant (e.g.,
MAX_SELECT_MENU_OPTIONS = 25 as const) and use it where Discord select-menu
option limits are checked; update the condition in getSelectMenuRow (replace
pages.length > 25) to pages.length > MAX_SELECT_MENU_OPTIONS and do the same
replacement in src/commands/fun/leaderboard.ts (or wherever the same 25 is used)
so the limit is self-documenting and centrally maintained.
In `@src/commands/util/user-info.ts`:
- Around line 132-154: The fallback logic uses || which should be replaced with
nullish coalescing (??) to follow guidelines: change the assignment of
currentMute (currently "const currentMute = muteModerations.find((m) =>
m.active) || muteModerations[0];") to use ?? instead of ||, and likewise change
currentBan in buildCurrentBanField (currently "const currentBan =
banModerations.find((m) => m.active) || banModerations[0];") to use ??; this
ensures undefined results from .find are handled safely without treating falsy
values like 0 or '' as missing.
In `@src/db/functions/factFunctions.ts`:
- Around line 21-26: The addFact function accepts the entire factTableTypes
which allows callers to pass server-managed fields (e.g., id, usedOn,
timestamps) that are ignored; change its parameter type to a narrow input shape
such as Pick<factTableTypes, 'content' | 'source' | 'addedBy'> & { approved?:
boolean } (or define an explicit AddFactInput type) and update the function
signature for addFact to use that type so the contract explicitly disallows
server-managed columns.
In `@src/db/functions/levelFunctions.ts`:
- Around line 79-94: The post-insert SELECT (inside withDbRetryDrizzle that
assigns createdLevel) can observe a race because the INSERT ON CONFLICT DO
NOTHING and the subsequent SELECT run as separate calls; update the logic in the
function using either a single INSERT ... ON CONFLICT DO UPDATE SET <noop>
RETURNING * to get the row in the same statement, or execute both statements
inside a single db.transaction so the SELECT runs on the same
connection/transaction; locate the code around withDbRetryDrizzle, createdLevel,
and levelTable (and consider interactions with deleteUserLevel) and replace the
two-call pattern with one of these atomic approaches.
In `@src/events/messageEvents.ts`:
- Around line 344-363: In the execute async function in messageEvents (execute:
async (message: Message): Promise<void>), remove the redundant Promise.resolve()
usages and replace them with bare returns: use plain return; after early exits
(when message.author.bot || !message.guild) and at the end of the try block, and
replace the Promise.resolve() in the catch block with a simple return; so the
async function resolves naturally without unnecessary Promise.resolve() calls.
In `@src/util/counting/constants.ts`:
- Around line 36-41: Restore the const assertion on MILESTONE_REACTIONS by
appending "as const" to the exported object literal so the emoji values remain
string literal types and the object is readonly; update the exported constant
MILESTONE_REACTIONS declaration (the object with keys normal, multiples25,
multiples50, multiples100) to use a const assertion to preserve literal types
used by helpers like deriveMilestone.
In `@src/util/counting/countingManager.ts`:
- Around line 191-198: The debug log message in CountingManager uses misleading
wording ("Ignored non-numeric message") even though VALID_MATH_EXPR_RE allows
operators and parentheses; update the logger.debug call to use a clearer message
such as "Ignored non-arithmetic message" (keep the existing metadata: user and
contentLength) so logs accurately reflect rejections of non-arithmetic input;
locate the check around VALID_MATH_EXPR_RE.test(trimmed) and change only the
string passed to logger.debug.
- Around line 226-244: Extract the magic number 100 into named constants by
adding ROLLBACK_THRESHOLD and ROLLBACK_STEP to src/util/counting/constants.ts
and importing them into src/util/counting/countingManager.ts; then replace the
hard-coded checks and calculations (the conditional data.currentCount > 100, the
const rollbackStep = 100, and Math.floor(data.currentCount / rollbackStep) *
rollbackStep) with the imported ROLLBACK_THRESHOLD and ROLLBACK_STEP so
setCount(rollbackTo), the logger.debug call, and the returned rolledBackTo use
the computed rollback based on those constants while preserving existing
behavior; refer to symbols data.currentCount, setCount, logger.debug, rollbackTo
and rolledBackTo to locate the changes.
- Around line 222-261: Change the handleRollbackOrReset function signature so
its reason parameter is typed as CountingProcessInvalidReason instead of string
(function: handleRollbackOrReset), then remove the redundant casts of reason as
CountingProcessInvalidReason in both returned objects; update any call sites to
pass a valid CountingProcessInvalidReason value (or refine their types) so
callers compile without assertions. This ensures compile-time validation and
eliminates the need for type assertions in the return objects (rolledBackTo
branches) while keeping behavior unchanged.
In `@src/util/telemetry/otelLogTransport.ts`:
- Around line 129-138: The isErrorShapedRecord predicate is needlessly complex:
remove the redundant casts on meta and drop hasName (dead code) and set
isErrorShapedRecord to simply hasStack || hasMessage; specifically, replace the
current hasStack/hasMessage/hasName definitions to use meta (already typed)
directly (e.g., typeof meta.stack === 'string' && checks for '\n' or ' at ') and
then assign isErrorShapedRecord = hasStack || hasMessage, removing hasName and
its use; if you intended a stricter rule instead, explicitly change the
predicate to require hasName && (hasStack || hasMessage) so the intent is clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a6c9a348-fc4e-41ab-8ec1-0f58f4f384a0
📒 Files selected for processing (25)
.github/workflows/ultracite.ymlCONTRIBUTING.mdLICENSEREADME.mdSECURITY.mdsrc/commands/fun/achievements.tssrc/commands/fun/counting.tssrc/commands/fun/fact.tssrc/commands/fun/leaderboard.tssrc/commands/fun/rank.tssrc/commands/util/config.tssrc/commands/util/help.tssrc/commands/util/members.tssrc/commands/util/user-info.tssrc/db/functions/factFunctions.tssrc/db/functions/levelFunctions.tssrc/db/functions/memberFunctions.tssrc/db/functions/moderationFunctions.tssrc/events/interactionCreate.tssrc/events/messageEvents.tssrc/util/counting/constants.tssrc/util/counting/countingManager.tssrc/util/counting/helpers.tssrc/util/helpers.tssrc/util/telemetry/otelLogTransport.ts
✅ Files skipped from review due to trivial changes (3)
- SECURITY.md
- LICENSE
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/events/interactionCreate.ts
| const sarif = JSON.parse(fs.readFileSync(reportPath, 'utf8')); | ||
| const runs = sarif.runs ?? []; | ||
| const results = runs.flatMap((run) => run.results ?? []); | ||
|
|
||
| const levelCounts = { error: 0, warning: 0, note: 0 }; | ||
| for (const result of results) { | ||
| const level = result.level ?? 'warning'; | ||
| if (levelCounts[level] === undefined) { | ||
| levelCounts.note += 1; | ||
| } else { | ||
| levelCounts[level] += 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Wrap SARIF parsing in try/catch.
If biome ci writes a partial/malformed SARIF (e.g., when killed mid-write or hitting an internal error), JSON.parse throws and this whole step fails with a stack trace instead of a useful PR comment. Since the upload-sarif step is gated by hashFiles, a parse failure here would also leave reviewers with no signal at all.
🛡️ Proposed fix
- } else {
- const sarif = JSON.parse(fs.readFileSync(reportPath, 'utf8'));
- const runs = sarif.runs ?? [];
- const results = runs.flatMap((run) => run.results ?? []);
+ } else {
+ let runs = [];
+ try {
+ const sarif = JSON.parse(fs.readFileSync(reportPath, 'utf8'));
+ runs = sarif.runs ?? [];
+ } catch (err) {
+ core.warning(`Failed to parse SARIF report: ${err.message}`);
+ body += `Could not parse SARIF report: \`${err.message}\`.`;
+ runs = null;
+ }
+ if (runs !== null) {
+ const results = runs.flatMap((run) => run.results ?? []);(close the new conditional before the trailing } at line 115)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ultracite.yml around lines 78 - 90, Wrap the SARIF file
read/parse around JSON.parse(fs.readFileSync(reportPath, 'utf8')) in a try/catch
to handle partial/malformed SARIF; if parsing fails, catch the error, log a
concise message referencing reportPath and the error, and set runs/results to
empty so subsequent code that computes levelCounts ({ error, warning, note })
still runs safely; ensure you keep the existing logic that defaults level to
'warning' and updates levelCounts so the step doesn't throw and still produces a
meaningful result when parsing fails.
| buttonCollector.on('collect', async (i: ButtonInteraction) => { | ||
| if (i.user.id !== interaction.user.id) { | ||
| await i.reply({ | ||
| content: 'You cannot use these buttons.', | ||
| flags: ['Ephemeral'], | ||
| }); | ||
| await safelyRespond( | ||
| interaction, | ||
| 'You cannot use these buttons.', | ||
| true | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Send the unauthorized-button warning to the clicking user, not the original interaction.
When a non-invoker clicks a pagination button, the response should target i (the button interaction), but this passes interaction (the original chat-input). Compare with the select-menu collector at Line 302, which correctly uses safelyRespond(i, ...). As written, the editReply goes to the original interaction instead of replying ephemerally to the user who clicked.
🐛 Suggested fix
buttonCollector.on('collect', async (i: ButtonInteraction) => {
if (i.user.id !== interaction.user.id) {
- await safelyRespond(
- interaction,
- 'You cannot use these buttons.',
- true
- );
+ await safelyRespond(i, 'You cannot use these buttons.', true);
return;
}📝 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.
| buttonCollector.on('collect', async (i: ButtonInteraction) => { | |
| if (i.user.id !== interaction.user.id) { | |
| await i.reply({ | |
| content: 'You cannot use these buttons.', | |
| flags: ['Ephemeral'], | |
| }); | |
| await safelyRespond( | |
| interaction, | |
| 'You cannot use these buttons.', | |
| true | |
| ); | |
| return; | |
| } | |
| buttonCollector.on('collect', async (i: ButtonInteraction) => { | |
| if (i.user.id !== interaction.user.id) { | |
| await safelyRespond(i, 'You cannot use these buttons.', true); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/fun/achievements.ts` around lines 341 - 349, The
unauthorized-button handler inside buttonCollector.on('collect', async (i:
ButtonInteraction) => { ... }) sends the ephemeral warning to the original
interaction; change the call to safelyRespond so it responds to the button
interaction `i` (i.e., use safelyRespond(i, 'You cannot use these buttons.',
true)) so the clicking user receives the ephemeral message (mirror the
select-menu behavior used in the select collector).
| const guild = interaction.guild; | ||
| if (!guild) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Silent return after deferReply() leaves the interaction hanging.
Lines 117-120 (and similarly at 176-179, 211-214, 249-252) bail out without any reply. Because execute() already called interaction.deferReply() (line 650), the user is stuck on "thinking..." until the token expires. validateInteraction should make this branch unreachable, but if it ever fires it produces a hung interaction. Send a generic response via safelyRespond before returning.
♻️ Suggested fix (apply to all four guard sites)
- const guild = interaction.guild;
- if (!guild) {
- return;
- }
+ const { guild } = interaction;
+ if (!guild) {
+ await safelyRespond(interaction, 'This command can only be used in a server.');
+ return;
+ }📝 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.
| const guild = interaction.guild; | |
| if (!guild) { | |
| return; | |
| } | |
| const { guild } = interaction; | |
| if (!guild) { | |
| await safelyRespond(interaction, 'This command can only be used in a server.'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/fun/counting.ts` around lines 117 - 120, The guard that checks
for a missing guild (e.g., the block with "const guild = interaction.guild; if
(!guild) { return; }") returns silently after interaction.deferReply() was
already called in execute(), leaving the interaction hung; update that guard
(and the three other similar guards) to call safelyRespond(interaction, "An
error occurred") or a short generic error message before returning so the
deferred reply is fulfilled; ensure you use the existing safelyRespond helper,
and keep the guard early-return behavior after calling safelyRespond to avoid
further execution.
| collector.on('collect', async (i: MessageComponentInteraction) => { | ||
| if (i.user.id !== interaction.user.id) { | ||
| await i.reply({ | ||
| content: 'You cannot use this pagination.', | ||
| flags: ['Ephemeral'], | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent non-invoker reply pattern — should use validateInteraction + safelyRespond like the rest of the PR.
Other paginated commands updated in this PR (e.g., src/commands/util/members.ts and src/commands/fun/leaderboard.ts) gate the non-invoker response behind validateInteraction(i) and dispatch through safelyRespond(i, '...', true). Here i.reply(...) is called directly, which (a) bypasses the safe-respond error logging if the interaction is no longer repliable, and (b) skips the channel/message validity check. Aligning this site with the project pattern keeps behavior uniform.
♻️ Suggested change
collector.on('collect', async (i: MessageComponentInteraction) => {
if (i.user.id !== interaction.user.id) {
- await i.reply({
- content: 'You cannot use this pagination.',
- flags: ['Ephemeral'],
- });
+ if (await validateInteraction(i)) {
+ await safelyRespond(i, 'You cannot use this pagination.', true);
+ }
return;
}As per coding guidelines: "Use safelyRespond(interaction, content) from src/util/helpers.ts for all interaction responses" and "Call await validateInteraction(interaction) before processing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/util/config.ts` around lines 249 - 256, Replace the direct
i.reply call in the collector's 'collect' handler with the project's
interaction-guard pattern: first call await validateInteraction(i) to ensure the
interaction/channel/message is valid, then call await safelyRespond(i, 'You
cannot use this pagination.', true) (true = ephemeral) instead of i.reply; keep
the same early return behavior when i.user.id !== interaction.user.id so the
handler exits after safelyRespond. Target the collector.on('collect', async (i:
MessageComponentInteraction) => { ... }) block and update the non-invoker branch
to use validateInteraction and safelyRespond.
| if (facts.length === 0) { | ||
| await withDbRetryDrizzle( | ||
| async () => { | ||
| return await db | ||
| .update(schema.factTable) | ||
| .update(factTable) | ||
| .set({ usedOn: null }) | ||
| .where(eq(schema.factTable.approved, true)); | ||
| .where(eq(factTable.approved, true)); | ||
| }, | ||
| { | ||
| operationName: 'reset-used-facts', | ||
| forceRetry: true, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| await invalidateCache(cacheKey); | ||
| return await getRandomUnusedFact(); | ||
| } |
There was a problem hiding this comment.
Possible infinite recursion when no approved facts exist.
If there are zero approved facts in the table, the empty-facts branch resets usedOn for eq(approved, true) (matching nothing), invalidates the cache, and recurses. The next call again finds an empty facts array and recurses again — a stack-overflowing infinite loop. Add a base case that bails when the reset affected no rows (or when the post-reset re-query is still empty), and return a sentinel/throw rather than recursing unconditionally.
🛡️ Suggested guard
if (facts.length === 0) {
- await withDbRetryDrizzle(
+ const reset = await withDbRetryDrizzle(
async () => {
return await db
.update(factTable)
.set({ usedOn: null })
.where(eq(factTable.approved, true));
},
{
operationName: 'reset-used-facts',
forceRetry: true,
}
);
await invalidateCache(cacheKey);
+ // If nothing was reset, there are no approved facts at all — avoid recursing forever.
+ if (!reset || (reset as { rowCount?: number }).rowCount === 0) {
+ throw new Error('No approved facts available');
+ }
return await getRandomUnusedFact();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/functions/factFunctions.ts` around lines 126 - 142, The branch in
getRandomUnusedFact that resets usedOn then unconditionally recurses can
infinite-loop if there are no approved facts; change it so after calling
withDbRetryDrizzle(... update(factTable).set({ usedOn: null
}).where(eq(factTable.approved, true))) you check the update result (e.g.,
affected rows/count returned by the DB driver) or immediately re-query facts
(using the same logic that produced `facts`) and if zero rows were affected or
the re-query is still empty, do not recurse — instead return a sentinel (null)
or throw a clear error; only recurse when the reset actually changed rows (or
the re-query returns >0). Ensure you reference the existing symbols factTable,
withDbRetryDrizzle, invalidateCache, cacheKey, and getRandomUnusedFact when
making this change.
| logger.error( | ||
| '[moderationDbFunctions] Database not initialized, update member moderation history', | ||
| '[moderationDbFunctions] Database not initialized, update member moderation history' | ||
| ); |
There was a problem hiding this comment.
Minor: missing "cannot" in log message.
The other "Database not initialized" log messages in this PR use the form "... Database not initialized, cannot <action>". This one reads "... Database not initialized, update member moderation history" (verb missing). Tiny inconsistency.
- '[moderationDbFunctions] Database not initialized, update member moderation history'
+ '[moderationDbFunctions] Database not initialized, cannot update member moderation history'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/functions/moderationFunctions.ts` around lines 30 - 32, Update the
inconsistent log message in moderationFunctions.ts: in the function that logs
"[moderationDbFunctions] Database not initialized, update member moderation
history" (the update member moderation history path), change the message to
include "cannot" so it matches other messages (e.g., "[moderationDbFunctions]
Database not initialized, cannot update member moderation history").
| const { guild } = message; | ||
|
|
||
| let executor = undefined as User | undefined; | ||
| try { | ||
| const audit = await shouldAllowRestoreCountingMessage( | ||
| guild, | ||
| message.author?.id, | ||
| message.channelId, | ||
| message.client?.user?.id | ||
| ); | ||
| executor = audit.executor; | ||
| } catch (error) { | ||
| logger.warn( | ||
| '[MessageEvents] Could not determine audit-log executor for deleted message', | ||
| error | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const countingChannelId = config.channels.counting; | ||
| if ( | ||
| message.channelId === countingChannelId && | ||
| message.content && | ||
| message.author | ||
| ) { | ||
| const { author } = message; | ||
| const trimmed = message.content.trim(); | ||
| const parsed = Number(trimmed); | ||
| if (Number.isInteger(parsed)) { | ||
| const data = await getCountingData(); | ||
|
|
||
| let allowRestore = true; | ||
| try { | ||
| const logs = await guild.fetchAuditLogs({ | ||
| type: AuditLogEvent.MessageDelete, | ||
| limit: 5, | ||
| }); | ||
| const entries = Array.from(logs.entries.values()); | ||
|
|
||
| const matching = entries.find((e) => { | ||
| const target = e.target as { id?: string } | null; | ||
| const targetId = | ||
| target?.id ?? (e as { targetId?: string }).targetId; | ||
| const extra = e.extra as { | ||
| channel?: { id?: string }; | ||
| channelId?: string; | ||
| } | null; | ||
| const channelId = extra?.channel?.id ?? extra?.channelId; | ||
| if (!targetId) return false; | ||
| if (targetId !== author?.id) return false; | ||
| if (channelId && channelId !== message.channelId) return false; | ||
| return true; | ||
| }); | ||
|
|
||
| const executor = matching?.executor; | ||
| if ( | ||
| executor && | ||
| author && | ||
| executor.id !== author.id && | ||
| executor.id !== message.client?.user?.id | ||
| ) { | ||
| allowRestore = false; | ||
| } | ||
| } catch (error) { | ||
| logger.warn( | ||
| '[MessageEvents] Could not fetch audit logs when checking message delete; allowing restore by fallback', | ||
| error, | ||
| ); | ||
| allowRestore = true; | ||
| } | ||
|
|
||
| if (data.currentCount === parsed && allowRestore) { | ||
| const countingChannel = | ||
| guild.channels.cache.get(countingChannelId); | ||
| if (countingChannel?.isTextBased()) { | ||
| await countingChannel.send( | ||
| `🔁 Restoring deleted counting message: **${trimmed}** (originally by <@${message.author.id}>)`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| // Still attempt to restore counting messages when appropriate. | ||
| // We intentionally ignore the return value here to avoid overwriting | ||
| // the executor derived from audit logs for general moderator attribution. | ||
| await maybeRestoreCountingMessage(message, guild); | ||
| } catch (error) { | ||
| logger.error( | ||
| '[MessageEvents] Error attempting to restore deleted counting message', | ||
| error, | ||
| error | ||
| ); | ||
| } |
There was a problem hiding this comment.
Audit log is fetched twice per message delete; reuse the executor.
shouldAllowRestoreCountingMessage is invoked once at Line 272 to derive the moderator and again indirectly inside maybeRestoreCountingMessage at Line 290 (which itself calls shouldAllowRestoreCountingMessage). Both paths run the same guild.fetchAuditLogs(...) request. The comment at Lines 287-289 implies the duplication exists to avoid “overwriting” the executor, but both calls compute the same matching entry, so the second fetch is redundant Discord API traffic for every deletion event.
Either:
- Use the executor returned from
maybeRestoreCountingMessage(...)and skip the standaloneshouldAllowRestoreCountingMessagecall, or - Fetch the audit-log result once at the top of the handler and pass
{ allowed, executor }intomaybeRestoreCountingMessage.
♻️ Suggested refactor (option 1)
- let executor = undefined as User | undefined;
- try {
- const audit = await shouldAllowRestoreCountingMessage(
- guild,
- message.author?.id,
- message.channelId,
- message.client?.user?.id
- );
- executor = audit.executor;
- } catch (error) {
- logger.warn(
- '[MessageEvents] Could not determine audit-log executor for deleted message',
- error
- );
- }
-
- try {
- // Still attempt to restore counting messages when appropriate.
- // We intentionally ignore the return value here to avoid overwriting
- // the executor derived from audit logs for general moderator attribution.
- await maybeRestoreCountingMessage(message, guild);
- } catch (error) {
- logger.error(
- '[MessageEvents] Error attempting to restore deleted counting message',
- error
- );
- }
+ let executor: User | undefined;
+ try {
+ executor = await maybeRestoreCountingMessage(message, guild);
+ } catch (error) {
+ logger.error(
+ '[MessageEvents] Error attempting to restore deleted counting message',
+ error
+ );
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/events/messageEvents.ts` around lines 268 - 296, The code currently calls
shouldAllowRestoreCountingMessage twice causing duplicate guild.fetchAuditLogs
calls; change the flow to fetch audit-log once and reuse its result by either
(A) removing the standalone shouldAllowRestoreCountingMessage call and awaiting
maybeRestoreCountingMessage(...) and assign executor from its returned value, or
(B) perform a single shouldAllowRestoreCountingMessage(...) at the top, capture
{ allowed, executor } and pass those values into
maybeRestoreCountingMessage(message, guild, { allowed, executor }) (or adjust
maybeRestoreCountingMessage signature to accept them) so that
guild.fetchAuditLogs is only invoked once per delete event; update logger
handling to use the single fetched executor and preserve existing error
handling.
| if (delayMs <= 0) { | ||
| // Execute immediately (next tick) for zero / negative values | ||
| setTimeout(() => { | ||
| void cb(); | ||
| cb(); | ||
| }, 0); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Async errors from cb() are swallowed in the zero/negative-delay path.
The main schedule() path wraps await cb() in try/catch and logs failures, but the delayMs <= 0 branch invokes cb() without await/.catch(). If cb is async and rejects, this surfaces as an unhandled promise rejection rather than going through the same logger path. Make this branch behave consistently with schedule:
♻️ Suggested fix
if (delayMs <= 0) {
// Execute immediately (next tick) for zero / negative values
- setTimeout(() => {
- cb();
- }, 0);
+ setTimeout(async () => {
+ try {
+ await cb();
+ } catch (error) {
+ logger.error('[scheduleLargeTimeout] Callback error', error);
+ }
+ }, 0);
return;
}📝 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.
| if (delayMs <= 0) { | |
| // Execute immediately (next tick) for zero / negative values | |
| setTimeout(() => { | |
| void cb(); | |
| cb(); | |
| }, 0); | |
| return; | |
| } | |
| if (delayMs <= 0) { | |
| // Execute immediately (next tick) for zero / negative values | |
| setTimeout(async () => { | |
| try { | |
| await cb(); | |
| } catch (error) { | |
| logger.error('[scheduleLargeTimeout] Callback error', error); | |
| } | |
| }, 0); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util/helpers.ts` around lines 246 - 252, The zero/negative-delay branch
in schedule() invokes cb() via setTimeout without awaiting or catching
rejections, causing unhandled promise rejections; change that branch to run the
callback inside the same async try/catch error-handling used by the main
schedule path (e.g., wrap the setTimeout handler in an async IIFE or call
Promise.resolve(cb()).catch(...)) and forward errors to the same logger used by
schedule (the same error logging call used in the main await cb() catch block)
so async rejections are logged consistently for delayMs <= 0.
There was a problem hiding this comment.
2 issues found across 25 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/util/telemetry/otelLogTransport.ts">
<violation number="1" location="src/util/telemetry/otelLogTransport.ts:137">
P2: `hasMessage` alone makes records look like errors, so normal `name` fields can be incorrectly exported as `exception.type`.</violation>
</file>
<file name="src/commands/fun/fact.ts">
<violation number="1" location="src/commands/fun/fact.ts:117">
P2: Log the caught exception as structured metadata (`{ error }`) so stack/details are preserved by the logger formatter.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const isErrorShapedRecord = | ||
| hasStack || hasMessage || (hasName && (hasStack || hasMessage)); |
There was a problem hiding this comment.
P2: hasMessage alone makes records look like errors, so normal name fields can be incorrectly exported as exception.type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/util/telemetry/otelLogTransport.ts, line 137:
<comment>`hasMessage` alone makes records look like errors, so normal `name` fields can be incorrectly exported as `exception.type`.</comment>
<file context>
@@ -126,6 +126,17 @@ function cleanAttributes(
+ const hasMessage =
+ typeof (meta as Record<string, unknown>).message === 'string';
+ const hasName = typeof (meta as Record<string, unknown>).name === 'string';
+ const isErrorShapedRecord =
+ hasStack || hasMessage || (hasName && (hasStack || hasMessage));
+
</file context>
| const isErrorShapedRecord = | |
| hasStack || hasMessage || (hasName && (hasStack || hasMessage)); | |
| const isErrorShapedRecord = hasStack || (hasName && hasMessage); |
| return; | ||
| } | ||
| } catch (error) { | ||
| logger.error('[FactCommand] Error handling subcommand', error); |
There was a problem hiding this comment.
P2: Log the caught exception as structured metadata ({ error }) so stack/details are preserved by the logger formatter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/fun/fact.ts, line 117:
<comment>Log the caught exception as structured metadata (`{ error }`) so stack/details are preserved by the logger formatter.</comment>
<file context>
@@ -91,26 +91,34 @@ const command: SubcommandCommand = {
+ });
+ }
+ } catch (error) {
+ logger.error('[FactCommand] Error handling subcommand', error);
+ await safelyRespond(
+ interaction,
</file context>
| logger.error('[FactCommand] Error handling subcommand', error); | |
| logger.error('[FactCommand] Error handling subcommand', { error }); |
Pull Request
Summary
This pull request migrates the project from ESLint and Prettier to Ultracite (using Biome as the backend) as the standard for linting and formatting JavaScript/TypeScript code. It updates documentation, developer tooling, CI/CD workflows, and code standards to reflect this change. The main improvements include new and updated code standards, devcontainer and editor integration, workflow and hook updates, and revised project instructions.
Related issues
N/A
Type of change
What changed
Migration to Ultracite/Biome for Linting and Formatting:
.devcontainer/devcontainer.json,.devcontainer/README.md,.github/copilot-instructions.md,.github/pull_request_template.md,.github/PULL_REQUEST_TEMPLATE/bug_fix.md,.github/PULL_REQUEST_TEMPLATE/feature.md,.github/PULL_REQUEST_TEMPLATE/refactor.md,.github/workflows/eslint.yml, [1] [2] [3] [4] [5] [6] [7] [8]Documentation and Code Standards:
.agents/skills/ultracite/SKILL.md,.agents/skills/ultracite/references/code-standards.md, [1] [2].github/copilot-instructions.md, .github/copilot-instructions.mdL140-R266)Developer Tooling and Editor Integration:
.devcontainer/devcontainer.json, .devcontainer/devcontainer.jsonL21-R75)yarn fix(Ultracite auto-fix) after tool usage, skipping certain correctness rules for smoother workflow. (.github/hooks/ultracite.json, .github/hooks/ultracite.jsonR1-R10)CI/CD and Workflow Updates:
.github/workflows/eslint.yml, .github/workflows/eslint.ymlL1-L47).github/copilot-instructions.md, .github/copilot-instructions.mdL140-R266)Project Scripts and PR Templates:
yarn check,yarn fix, andyarn type-checkin place of previousyarn lintandyarn format:fixcommands. (.github/copilot-instructions.md,.github/pull_request_template.md,.github/PULL_REQUEST_TEMPLATE/bug_fix.md,.github/PULL_REQUEST_TEMPLATE/feature.md,.github/PULL_REQUEST_TEMPLATE/refactor.md, [1] [2] [3] [4] [5]How was this tested?
yarn dev/yarn no-deployyarn lintyarn format:fixyarn compileTest notes:
Database or cache changes
drizzle-kitbot:); includes graceful degradationDetails:
Breaking changes
Migration notes:
Security and privacy
Notes:
Checklist
CONTRIBUTING.mdfeat(bot): ...)username/feature-name)deferReply()usage where neededAdditional context for reviewers
Summary by cubic
Switched code quality to
ultracitewith Biome, replacing ESLint/Prettier. Updated scripts, CI, editor/devcontainer, and docs; broad refactors improve command safety, counting validation, DB types, and logging.Dependencies
ultracite(Biome) and addedbiome.jsonc.yarn check,yarn fix,yarn type-check,yarn setup,yarn commitlint; Husky +lint-stagednow runyarn ultracite fixand type-check on staged files.eslint.ymlwithultracite.yml(Biome SARIF output + optional PR comments).biomejs.biomeas the default formatter across file types.AGENTS.mdand.agents/skills/ultracite.Refactors
validateInteractionandsafelyRespondacross commands; improved select/menu handling and error paths.addFact).Written for commit 93e2860. Summary will update on new commits.