Add dashboard permission access controls#360
Conversation
|
This PR was not deployed automatically as @MohsinCoding does not have access to the Railway project. In order to get automatic PR deploys, please add @MohsinCoding to your workspace on Railway. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds per-guild access-level resolution and moderator-level authorization, trusted internal request handling via API secret, a guild-directory React context, a DB migration to backfill audit_logs, editor character-limit enforcement, and broad test updates covering the new authorization, rate‑limit, and UI flows. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| '/:id/members/export', | ||
| membersRateLimit, | ||
| requireGuildAdmin, | ||
| requireGuildModerator, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| * $ref: "#/components/responses/ServiceUnavailable" | ||
| */ | ||
| router.get('/:id/members', membersRateLimit, requireGuildAdmin, validateGuild, async (req, res) => { | ||
| router.get('/:id/members', membersRateLimit, requireGuildModerator, validateGuild, async (req, res) => { |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
src/api/routes/members.js
Outdated
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
src/api/routes/members.js
Outdated
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
src/api/routes/members.js
Outdated
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| '/:id/tickets/stats', | ||
| ticketRateLimit, | ||
| requireGuildAdmin, | ||
| requireGuildModerator, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| '/:id/tickets/:ticketId', | ||
| ticketRateLimit, | ||
| requireGuildAdmin, | ||
| requireGuildModerator, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| * $ref: "#/components/responses/ServiceUnavailable" | ||
| */ | ||
| router.get('/:id/tickets', ticketRateLimit, requireGuildAdmin, validateGuild, async (req, res) => { | ||
| router.get('/:id/tickets', ticketRateLimit, requireGuildModerator, validateGuild, async (req, res) => { |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Copilot Autofix
AI 6 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
|
| Filename | Overview |
|---|---|
| src/api/middleware/trustedInternalRequest.js | New module that bypasses rate limiting for trusted internal (dashboard) requests; uses plain === comparison for the API secret instead of the constant-time crypto.timingSafeEqual used elsewhere. |
| src/api/routes/guilds.js | Adds getGuildAccessLevel for bot-config-aware permission resolution, updates requireGuildPermission to use it, and introduces the new /access batch endpoint; logic is correct and well-tested. |
| web/src/app/api/guilds/route.ts | New applyAccessLevels function fetches bot-evaluated access in 100-guild chunks; a mid-chunk failure drops access data already accumulated in earlier chunks before returning the original guild list. |
| web/src/lib/bot-api-proxy.ts | Cleanly refactored to add authorizeGuildModerator alongside authorizeGuildAdmin; introduces resolveGuildAccess that correctly falls back to OAuth-derived permissions when the bot API is unavailable. |
| web/src/lib/discord.server.ts | Adds maxRetryDelayMs and totalRetryBudgetMs caps to fetchWithRateLimit to prevent long Discord rate-limit windows from stalling requests; also parses x-ratelimit-reset-after header. |
| web/src/components/layout/guild-directory-context.tsx | New React context that centralizes guild fetching for the dashboard shell, reducing duplicate fetch calls from ServerSelector and Sidebar; properly handles abort on unmount. |
| web/src/components/layout/sidebar.tsx | Filters primary nav to Moderation/Members/Tickets for moderator-access guilds; null activeGuildAccess (during initial load) falls through to full nav, causing a brief unrestricted flicker for moderators. |
| src/api/middleware/auth.js | Extends API-secret auth to accept an optional x-discord-user-id header (validated as a Discord snowflake) so the dashboard can forward the acting user's identity for audit attribution. |
| src/utils/loadCommands.js | Switches from raw file path to file:// URL via pathToFileURL for ESM dynamic imports, fixing Windows compatibility. |
| migrations/014_audit_logs_user_tag_backfill.cjs | Repair migration that backfills the user_tag column and idx_audit_logs_guild_user index for databases that ran an older version of 013_audit_log.cjs; correctly uses IF NOT EXISTS. |
Sequence Diagram
sequenceDiagram
participant Browser
participant NextJS as Next.js Web
participant BotAPI as Bot API (Express)
participant Discord
Browser->>NextJS: GET /api/guilds
NextJS->>Discord: fetchMutualGuilds(accessToken)
Discord-->>NextJS: user guild list
NextJS->>BotAPI: GET /guilds/access?userId=X&guildIds=...
Note over NextJS,BotAPI: x-api-secret header (bypasses rate limit)
BotAPI->>BotAPI: getGuildAccessLevel(guild, userId)
BotAPI-->>NextJS: [{id, access}]
NextJS-->>Browser: guilds with resolved access level
Browser->>NextJS: GET /api/guilds/:id/members
NextJS->>BotAPI: authorizeGuildModerator(token, guildId)
Note over NextJS,BotAPI: resolveGuildAccess → /guilds/access
BotAPI-->>NextJS: access level
NextJS->>BotAPI: proxy GET /:id/members
BotAPI-->>NextJS: member list
NextJS-->>Browser: 200 OK
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/api/middleware/trustedInternalRequest.js
Line: 18-19
Comment:
**Plain string comparison for the shared secret**
`isTrustedInternalRequest` uses `providedSecret === expectedSecret` while `isValidSecret` in `auth.js` uses `crypto.timingSafeEqual` for the same `BOT_API_SECRET`. A timing-side-channel on this bypass path could leak the secret and allow an attacker to craft a valid API-secret request.
```suggestion
return (
typeof expectedSecret === 'string' &&
expectedSecret.length > 0 &&
typeof providedSecret === 'string' &&
providedSecret.length === expectedSecret.length &&
crypto.timingSafeEqual(Buffer.from(providedSecret), Buffer.from(expectedSecret))
);
```
`crypto` would need to be imported at the top of the file with `import crypto from 'node:crypto';`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: web/src/app/api/guilds/route.ts
Line: 49-56
Comment:
**Partial chunk failure silently discards successfully resolved access data**
When iterating over chunks of guild IDs, if the first chunk (e.g., guilds 1–100) succeeds and populates `accessMap`, but the second chunk request fails, the function returns the original `guilds` parameter before calling `.map(...)`. This drops the access data already accumulated in `accessMap`, leaving all guilds without augmented access levels rather than applying partial results.
Consider continuing with the accumulated `accessMap` instead of returning early on a per-chunk failure, or at minimum document that this is an intentional all-or-nothing fallback.
```suggestion
if (!response.ok) {
logger.warn('[api/guilds] Failed to fetch bot access levels', {
status: response.status,
statusText: response.statusText,
guildCount: guildIdChunk.length,
});
// Break out of the loop but still apply whatever we have in accessMap so far
break;
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: web/src/components/layout/sidebar.tsx
Line: 111-113
Comment:
**`activeGuildAccess === null` shows the full nav instead of a restricted view**
When `guildId` is set but the guild hasn't loaded into `guilds` yet (e.g., initial render or refresh), `activeGuildAccess` is `null`. The expression `activeGuildAccess === 'moderator' ? moderatorPrimaryNav : primaryNav` then falls through to `primaryNav`, so a moderator briefly sees the unrestricted nav before the guild list resolves. This is a cosmetic flicker rather than a security bypass (the API enforces access), but it may be confusing.
Consider defaulting to `moderatorPrimaryNav` when `activeGuildAccess` is `null` and the user has no resolved access yet, or guard with a loading state.
How can I resolve this? If you propose a fix, please make it concise.Reviews (13): Last reviewed commit: "Merge branch 'main' into mohsin/volvox-b..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR tightens dashboard access control by introducing explicit access levels (viewer/moderator/admin/bot-owner) and enforcing them consistently across the bot API, the Next.js proxy routes, and the dashboard UI. It also includes supporting fixes for migration ordering, Windows ESM command loading, and rate-limit retry behavior.
Changes:
- Add bot-evaluated guild access levels and enforce moderator/admin authorization in both the bot API and the Next.js proxy layer.
- Restrict moderator dashboard navigation to Moderation/Members/Tickets and add tests around role/access behavior.
- Improve rate-limit handling (retry caps/budgets), skip rate limiting for trusted internal (x-api-secret) requests, and fix Windows command module imports via
file://URLs.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| web/tests/lib/discord.test.ts | Adds coverage for rate-limit retry caps/budgets and fail-fast behavior. |
| web/tests/lib/bot-api-proxy-branches.test.ts | Adds branch tests for moderator permission detection and moderator authorization. |
| web/tests/hooks/use-guild-role.test.ts | Adds test ensuring explicit API access overrides permission-bit fallback. |
| web/tests/components/layout/sidebar.test.tsx | Adds test ensuring moderator-only access hides non-allowed sidebar pages. |
| web/tests/components/layout/server-selector.test.tsx | Updates/manageability copy expectations and adds moderator-access manageable test. |
| web/tests/api/guilds.test.ts | Adds test for augmenting /api/guilds with bot-evaluated access when configured. |
| web/src/types/discord.ts | Extends MutualGuild with optional access field. |
| web/src/lib/discord.server.ts | Introduces retry delay cap + total retry budget for Discord/bot API fetches. |
| web/src/lib/bot-api-proxy.ts | Adds moderator access model and authorization helpers backed by bot API access evaluation. |
| web/src/hooks/use-guild-role.ts | Uses explicit access field to derive dashboard role; updates manageable logic. |
| web/src/components/layout/sidebar.tsx | Hides non-moderation navigation for guilds with moderator-only access. |
| web/src/components/layout/server-selector.tsx | Updates empty-state copy for manageable server selection. |
| web/src/app/api/moderation/user/[userId]/history/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/moderation/stats/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/moderation/cases/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/moderation/cases/[id]/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/route.ts | Augments Discord mutual guilds with bot-evaluated access levels when configured. |
| web/src/app/api/guilds/[guildId]/tickets/stats/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/tickets/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/members/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/members/export/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/members/[userId]/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| web/src/app/api/guilds/[guildId]/members/[userId]/cases/route.ts | Switches auth gate from admin-only to moderator-or-higher. |
| tests/commands.test.js | Fixes Windows ESM dynamic imports by using pathToFileURL(...).href. |
| tests/api/routes/guilds.coverage.test.js | Adds coverage for new /guilds/access endpoint and updated permission behavior. |
| tests/api/middleware/redisRateLimit.test.js | Adds coverage for skipping rate limits on trusted internal (x-api-secret) requests. |
| tests/api/middleware/rateLimit.test.js | Adds coverage for skipping in-memory rate limits on trusted internal requests. |
| src/utils/loadCommands.js | Fixes Windows ESM command loading using pathToFileURL imports. |
| src/api/routes/tickets.js | Broadens ticket routes from admin-only to moderator-or-higher. |
| src/api/routes/members.js | Broadens member routes from admin-only to moderator-or-higher. |
| src/api/routes/guilds.js | Adds bot-side access resolution + new /guilds/access endpoint and integrates into auth middleware. |
| src/api/middleware/trustedInternalRequest.js | Adds helper for identifying trusted internal requests via shared API secret. |
| src/api/middleware/redisRateLimit.js | Skips Redis-backed rate limiting for trusted internal requests. |
| src/api/middleware/rateLimit.js | Skips in-memory rate limiting for trusted internal requests. |
| migrations/015_perf_indexes.cjs | Renumbers perf index migration comment to align with restored 014 slot. |
| migrations/014_audit_logs_user_tag_backfill.cjs | Adds repair migration preserving historical 014 slot and backfilling audit_logs drift. |
| export function getGuildDashboardRole(guild: MutualGuild): GuildDashboardRole { | ||
| if (guild.access === 'bot-owner') return 'admin'; | ||
| if (guild.access === 'admin') return 'admin'; | ||
| if (guild.access === 'moderator') return 'moderator'; | ||
| if (guild.access === 'viewer') return 'viewer'; | ||
| if (guild.owner) return 'owner'; | ||
|
|
There was a problem hiding this comment.
In getGuildDashboardRole, (1) checking guild.access before guild.owner means an owner can be reported as admin/viewer when access is present; if you want the owner label to remain authoritative, handle guild.owner first. (2) This helper currently treats MANAGE_GUILD as admin (later in the function), but the server-side checks in this PR treat Manage Guild as moderator (e.g. isModerator() and hasModeratorPermission()). Aligning these avoids showing admin-only UI when access isn’t provided.
| logger.warn( | ||
| `[discord] Rate limited on ${url}, retrying in ${waitMs}ms (attempt ${attempt + 1}/${MAX_RETRIES})`, | ||
| `[discord] Rate limited on ${url}, retrying in ${waitMs}ms ` + | ||
| `(attempt ${attempt + 1}/${maxRetries}, remaining budget ${remainingBudgetMs}ms)`, | ||
| ); |
There was a problem hiding this comment.
Same as above: the retry log prints (attempt ${attempt + 1}/${maxRetries}), but maxRetries is the retry count while attempt + 1 is the attempt count. This can exceed the denominator on the last retry (e.g. 4/3). Align the logged numerator/denominator with the actual meaning (attempts vs retries).
| <div className="px-2 py-3 text-center text-xs text-muted-foreground"> | ||
| <Server className="mx-auto mb-1 h-4 w-4" /> | ||
| You need mod or admin permissions to manage a server. | ||
| You need admin permissions to manage a server. |
There was a problem hiding this comment.
The empty-state copy says “You need admin permissions…”, but the component (and its comment just above) defines manageable servers as mod/admin/owner. Since moderators are explicitly supported, this message is misleading—consider changing it to mention moderator access (or reword to clarify it’s about full admin-only management).
| You need admin permissions to manage a server. | |
| You need moderator, admin, or owner permissions to manage a server. |
web/tests/api/guilds.test.ts
Outdated
| const realFetch = globalThis.fetch; | ||
| globalThis.fetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: async () => [{ id: "1", access: "moderator" }], | ||
| status: 200, | ||
| statusText: "OK", | ||
| } as Response); | ||
|
|
There was a problem hiding this comment.
This test replaces globalThis.fetch and only restores it at the end of the test body. If an assertion throws (or the test returns early), the restore won’t run and later tests may observe the mocked fetch. Prefer vi.spyOn(globalThis, 'fetch') with mockRestore() in afterEach, or wrap the override in try/finally so restoration is guaranteed.
| useEffect(() => { | ||
| const controller = new AbortController(); | ||
|
|
||
| (async () => { | ||
| try { | ||
| const response = await fetch('/api/guilds', { signal: controller.signal }); | ||
| if (!response.ok) return; | ||
| const data: unknown = await response.json(); | ||
| if (!Array.isArray(data)) return; | ||
| setGuilds( | ||
| data.filter( | ||
| (guild): guild is MutualGuild => | ||
| typeof guild === 'object' && | ||
| guild !== null && | ||
| typeof (guild as { id?: unknown }).id === 'string' && | ||
| typeof (guild as { name?: unknown }).name === 'string' && | ||
| typeof (guild as { permissions?: unknown }).permissions === 'string' && | ||
| typeof (guild as { owner?: unknown }).owner === 'boolean', | ||
| ), | ||
| ); | ||
| } catch { | ||
| // Keep full nav if the guild list cannot be loaded. | ||
| } | ||
| })(); | ||
|
|
||
| return () => controller.abort(); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
Sidebar fetches /api/guilds on mount to determine the active guild’s access, but ServerSelector already fetches the same endpoint in the same layout (DashboardShell). This creates duplicate requests (and duplicated runtime shape checks) on every dashboard load. Consider centralizing guild list/access into a shared hook/context (or persisting the selected guild’s access alongside the selected guild ID) so both components can reuse a single fetch/result.
| useEffect(() => { | |
| const controller = new AbortController(); | |
| (async () => { | |
| try { | |
| const response = await fetch('/api/guilds', { signal: controller.signal }); | |
| if (!response.ok) return; | |
| const data: unknown = await response.json(); | |
| if (!Array.isArray(data)) return; | |
| setGuilds( | |
| data.filter( | |
| (guild): guild is MutualGuild => | |
| typeof guild === 'object' && | |
| guild !== null && | |
| typeof (guild as { id?: unknown }).id === 'string' && | |
| typeof (guild as { name?: unknown }).name === 'string' && | |
| typeof (guild as { permissions?: unknown }).permissions === 'string' && | |
| typeof (guild as { owner?: unknown }).owner === 'boolean', | |
| ), | |
| ); | |
| } catch { | |
| // Keep full nav if the guild list cannot be loaded. | |
| } | |
| })(); | |
| return () => controller.abort(); | |
| }, []); |
| /** | ||
| * Verify that the incoming request is from the owner or an administrator of the specified guild. | ||
| * | ||
| * @param request - The incoming NextRequest containing the user's session/token. | ||
| * @param guildId - The Discord guild ID to authorize against. | ||
| * @param logPrefix - Prefix used when logging contextual error messages. | ||
| * @returns `null` if the requester is authorized; a `NextResponse` containing an error JSON otherwise. | ||
| * Possible responses: | ||
| * - 401 Unauthorized when the access token is missing or expired. | ||
| * - 502 Bad Gateway when mutual guilds cannot be verified. | ||
| * - 403 Forbidden when the user is neither the guild owner nor has administrator permission. | ||
| */ |
There was a problem hiding this comment.
The JSDoc for authorizeGuildAdmin still describes the check as “owner or administrator permission”, but the implementation now also authorizes bot-owner (and may derive admin/moderator via the bot API access endpoint). Updating the wording (and the 403 bullet) would keep the docs aligned with the new access model.
| if (attempt === maxRetries || waitMs > maxRetryDelayMs || waitMs > remainingBudgetMs) { | ||
| logger.warn( | ||
| `[discord] Rate limited on ${url}, not retrying after ${waitMs}ms ` + | ||
| `(attempt ${attempt + 1}/${maxRetries}, remaining budget ${Math.max(remainingBudgetMs, 0)}ms)`, | ||
| ); |
There was a problem hiding this comment.
maxRetries is treated as the number of retries (loop runs attempt <= maxRetries), but the log prints (attempt ${attempt + 1}/${maxRetries}), which will produce confusing output like 4/3 on the final attempt when maxRetries=3. Consider logging either (attempt ${attempt + 1}/${maxRetries + 1}) (total attempts) or retry ${attempt}/${maxRetries} (retry count) to match the control flow.
web/src/app/api/guilds/route.ts
Outdated
| try { | ||
| const url = new URL(`${botApiBaseUrl}/guilds/access`); | ||
| url.searchParams.set('userId', userId); | ||
| url.searchParams.set( | ||
| 'guildIds', | ||
| guilds | ||
| .filter((guild) => guild.botPresent) | ||
| .map((guild) => guild.id) | ||
| .join(','), | ||
| ); |
There was a problem hiding this comment.
applyAccessLevels() will still call the bot API even when no guilds have botPresent (because guilds.length !== 0), sending guildIds= as an empty string. Consider computing the botPresent IDs first and returning early when that list is empty to avoid an unnecessary internal request.
| try { | |
| const url = new URL(`${botApiBaseUrl}/guilds/access`); | |
| url.searchParams.set('userId', userId); | |
| url.searchParams.set( | |
| 'guildIds', | |
| guilds | |
| .filter((guild) => guild.botPresent) | |
| .map((guild) => guild.id) | |
| .join(','), | |
| ); | |
| const botGuildIds = guilds | |
| .filter((guild) => guild.botPresent) | |
| .map((guild) => guild.id); | |
| if (botGuildIds.length === 0) { | |
| return guilds; | |
| } | |
| try { | |
| const url = new URL(`${botApiBaseUrl}/guilds/access`); | |
| url.searchParams.set('userId', userId); | |
| url.searchParams.set('guildIds', botGuildIds.join(',')); |
src/api/routes/guilds.js
Outdated
| acc.push({ | ||
| if (!botGuild) continue; | ||
|
|
||
| const access = await getGuildAccessLevel(botGuild, req.user.userId); |
There was a problem hiding this comment.
In GET /guilds, getGuildAccessLevel() may call guild.members.fetch(userId) when the member isn’t cached. Doing this inside the loop over userGuilds can trigger up to O(N) Discord REST calls per request (potentially ~200), which risks rate-limits and slow responses. Since fetchUserGuilds() already returns ug.permissions (and ug.owner), consider short-circuiting: derive access from those bits first (Admin/ManageGuild), and only call getGuildAccessLevel() for guilds where OAuth permissions don’t grant mod/admin but configured roles might.
| const access = await getGuildAccessLevel(botGuild, req.user.userId); | |
| // Derive access from OAuth guild permissions first to avoid extra Discord REST calls. | |
| // If OAuth grants ADMINISTRATOR or MANAGE_GUILD (or the user is guild owner), | |
| // we can short-circuit without calling getGuildAccessLevel(), which may fetch members. | |
| let access; | |
| const isOwner = Boolean(ug.owner); | |
| let permsBigInt = 0n; | |
| if (ug.permissions != null) { | |
| try { | |
| // Discord returns permissions as a stringified integer. | |
| permsBigInt = BigInt(ug.permissions); | |
| } catch { | |
| permsBigInt = 0n; | |
| } | |
| } | |
| const ADMINISTRATOR = 0x8n; | |
| const MANAGE_GUILD = 0x20n; | |
| const hasAdmin = isOwner || (permsBigInt & ADMINISTRATOR) === ADMINISTRATOR; | |
| const hasManageGuild = | |
| !hasAdmin && (permsBigInt & MANAGE_GUILD) === MANAGE_GUILD; | |
| if (hasAdmin) { | |
| access = 'admin'; | |
| } else if (hasManageGuild) { | |
| access = 'moderator'; | |
| } else { | |
| access = await getGuildAccessLevel(botGuild, req.user.userId); | |
| } |
src/api/routes/guilds.js
Outdated
| const guildIds = [...new Set(guildIdsRaw.split(',').map((id) => id.trim()).filter(Boolean))]; | ||
| if (guildIds.length === 0) { | ||
| return res.json([]); | ||
| } | ||
|
|
||
| const { client } = req.app.locals; | ||
|
|
||
| const accessEntries = await Promise.all( | ||
| guildIds.map(async (guildId) => { | ||
| const guild = client.guilds.cache.get(guildId); | ||
| if (!guild) return null; | ||
|
|
||
| const access = await getGuildAccessLevel(guild, userId); | ||
| return { id: guildId, access }; | ||
| }), | ||
| ); |
There was a problem hiding this comment.
GET /guilds/access accepts an arbitrary comma-separated guildIds list and then runs getGuildAccessLevel() for each ID concurrently via Promise.all. With a large list, this can fan out into many guild.members.fetch() calls at once, amplifying Discord rate-limit pressure and memory usage. Consider enforcing a reasonable max number of guildIds per request and/or adding concurrency limiting + short TTL caching for {guildId,userId} access results.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
web/src/app/api/guilds/[guildId]/members/route.ts (1)
15-31:⚠️ Potential issue | 🟡 MinorUpdate the authorization wording in the function docs.
Line 17 says “requester is a guild admin,” but Line 30 now checks moderator authorization.
Proposed fix
- * Validates presence of `guildId` and that the requester is a guild admin, forwards `limit`, `after`, `search`, `sort`, and `order` query parameters to the upstream `/guilds/{guildId}/members` path, and proxies the response from the bot API. + * Validates presence of `guildId` and that the requester is a guild moderator, forwards `limit`, `after`, `search`, `sort`, and `order` query parameters to the upstream `/guilds/{guildId}/members` path, and proxies the response from the bot API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/guilds/`[guildId]/members/route.ts around lines 15 - 31, The function comment incorrectly states "requester is a guild admin" while the actual authorization call is authorizeGuildModerator in the GET handler; update the docblock to reflect that the requester must be a guild moderator (or use consistent terminology "moderator" across the doc) and mention authorizeGuildModerator/LOG_PREFIX as the enforcing mechanism so the description matches the implemented check.web/src/app/api/guilds/[guildId]/members/export/route.ts (1)
12-28:⚠️ Potential issue | 🟡 MinorJSDoc still documents admin-only access.
Line 14 says “guild-admin authorization,” but Line 27 now authorizes moderators as well.
Proposed fix
- * Validates the route parameter, enforces guild-admin authorization, forwards the upstream export request with a timeout, and returns the upstream CSV body with appropriate `Content-Type` and `Content-Disposition`. On failure returns a JSON error response with an appropriate HTTP status (e.g., 400 for missing guildId, the upstream status for upstream errors, 504 for timeouts, or 500 for internal failures). + * Validates the route parameter, enforces guild-moderator authorization, forwards the upstream export request with a timeout, and returns the upstream CSV body with appropriate `Content-Type` and `Content-Disposition`. On failure returns a JSON error response with an appropriate HTTP status (e.g., 400 for missing guildId, the upstream status for upstream errors, 504 for timeouts, or 500 for internal failures).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/guilds/`[guildId]/members/export/route.ts around lines 12 - 28, The JSDoc incorrectly states "guild-admin authorization" but the code uses authorizeGuildModerator (and LOG_PREFIX) to allow moderators; update the function comment above export GET to accurately describe that the route enforces guild-moderator (not admin-only) authorization, and adjust any wording about who can access the export and required permissions to match the authorizeGuildModerator behavior used in the GET handler.web/src/app/api/guilds/[guildId]/members/[userId]/cases/route.ts (1)
15-31:⚠️ Potential issue | 🟡 MinorDoc comment is outdated after the moderator auth change.
Line 17 still documents guild-admin authorization, but Line 30 now enforces moderator authorization.
Proposed fix
- * Validates route parameters, enforces guild-admin authorization, forwards the original query parameters, and returns the upstream bot API response. + * Validates route parameters, enforces guild-moderator authorization, forwards the original query parameters, and returns the upstream bot API response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/guilds/`[guildId]/members/[userId]/cases/route.ts around lines 15 - 31, The top doc comment for the GET handler is outdated: it says "enforces guild-admin authorization" but the code calls authorizeGuildModerator (with LOG_PREFIX) and enforces moderator authorization; update the comment block above the GET function to describe that the route validates params, enforces guild-moderator authorization via authorizeGuildModerator, forwards query params to the bot API, and returns the upstream NextResponse or error (e.g., 400 for missing guildId/userId or authorization error) so the documentation matches the actual behavior.web/src/app/api/moderation/cases/route.ts (1)
15-27:⚠️ Potential issue | 🟡 MinorUpdate JSDoc to match the new authorization level.
Line 18 still states “admin authorization,” but Line 26 now enforces moderator authorization.
Proposed fix
/** * GET /api/moderation/cases * Proxies to bot API GET /api/v1/moderation/cases - * Requires guildId query param and admin authorization. + * Requires guildId query param and moderator authorization. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/moderation/cases/route.ts` around lines 15 - 27, The JSDoc above the exported GET function is out of date—update the comment to reflect moderator (not admin) authorization: change the description line that currently says “Requires guildId query param and admin authorization.” to reference moderator authorization, and mention that authorizeGuildModerator is used (see GET and authorizeGuildModerator with LOG_PREFIX) so the docstring matches the enforced behavior.web/src/app/api/moderation/stats/route.ts (1)
14-25:⚠️ Potential issue | 🟡 MinorStale JSDoc comment.
The comment on line 17 states "Requires guildId query param and admin authorization" but the implementation now uses
authorizeGuildModerator. Update the documentation to reflect the actual authorization requirement.📝 Proposed fix
/** * GET /api/moderation/stats * Proxies to bot API GET /api/v1/moderation/stats - * Requires guildId query param and admin authorization. + * Requires guildId query param and moderator authorization. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/moderation/stats/route.ts` around lines 14 - 25, Update the stale JSDoc above the GET handler to reflect the current authorization check: replace "admin authorization" with the accurate requirement (e.g., "guild moderator authorization via authorizeGuildModerator"). Ensure the comment references the guildId query param and the use of authorizeGuildModerator and LOG_PREFIX so readers understand the actual authorization logic used by the GET(request: NextRequest) function.web/src/app/api/guilds/[guildId]/members/[userId]/route.ts (1)
14-34:⚠️ Potential issue | 🟡 MinorStale JSDoc comment.
The comment on line 17 states "enforces guild admin authorization" but the implementation now uses
authorizeGuildModerator. Update the documentation to reflect the actual authorization level.📝 Proposed fix
/** * Proxy a GET request for a guild member's details to the bot API. * - * Validates required path parameters, enforces guild admin authorization, builds the upstream URL, + * Validates required path parameters, enforces guild moderator authorization, builds the upstream URL, * and forwards the request to the bot API. Returns a 400 response if `guildId` or `userId` is missing,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/guilds/`[guildId]/members/[userId]/route.ts around lines 14 - 34, Update the stale JSDoc on the GET handler to reflect the actual authorization check: replace "enforces guild admin authorization" with "enforces guild moderator authorization" (or otherwise mention use of authorizeGuildModerator) and ensure the summary and param/return descriptions remain accurate for the GET function that calls authorizeGuildModerator (referencing authorizeGuildModerator, GET, and LOG_PREFIX to locate the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/014_audit_logs_user_tag_backfill.cjs`:
- Around line 30-36: Add a brief inline comment inside the exports.down function
explaining why the index created in the corresponding up migration
(idx_audit_logs_guild_user) is intentionally not dropped here; reference the
existing migration that also creates that index (013_audit_log.cjs) and state
that dropping the index here would cause inconsistency with that migration, so
only the user_tag column is removed in exports.down.
In `@src/api/routes/guilds.js`:
- Around line 259-264: The current blanket catch around
guild.members.fetch(userId) hides transient Discord failures; change the catch
to only swallow the "unknown member" error (Discord API error code 10007 or
error.message includes "Unknown Member") by setting member = null for that case,
and rethrow any other errors so they bubble up and result in a 5xx from /access;
apply the same change to the other fetch sites mentioned (the GET /guilds
filter, the GET /access fan-out handling, and the permission middleware) so only
the explicit "unknown member" case is treated as missing and all other Discord
errors are propagated.
In `@web/src/app/api/guilds/route.ts`:
- Around line 57-72: The filter currently only checks that entry.access is a
string, which allows unknown values through; update the runtime validation in
the accessEntries filter used to build accessMap to ensure entry.access is one
of the allowed literals ('admin','moderator','viewer','bot-owner') (e.g., check
against a Set of VALID_ACCESS values) before mapping to [entry.id,
entry.access], so the resulting accessMap.get(guild.id) can never return an
unexpected string and MutualGuild['access'] remains one of the known enum
values.
In `@web/src/components/layout/server-selector.tsx`:
- Line 272: Update the empty-state copy to match the actual access rules
enforced by isGuildManageable(): replace the current admin-only message with one
that mentions moderator/admin access (e.g., "You need moderator or admin
permissions to manage a server.") so the UI text aligns with the behavior of
isGuildManageable() in server-selector.tsx.
In `@web/src/components/layout/sidebar.tsx`:
- Around line 108-113: The current logic uses activeGuildAccess (derived from
guilds) and defaults to showing primaryNav when access is null, so moderators
see the full nav on first paint or on fetch errors; change the fallback to a
restrictive default until access is known: update the expressions that set
visiblePrimaryNav and visibleSecondaryNav so they only show primaryNav when
activeGuildAccess === 'moderator' and otherwise hide primary (e.g.,
visiblePrimaryNav = activeGuildAccess === 'moderator' ? moderatorPrimaryNav :
[]) while keeping visibleSecondaryNav = activeGuildAccess === 'moderator' ? [] :
secondaryNav, and ensure references to guilds and activeGuildAccess remain
unchanged elsewhere.
In `@web/src/hooks/use-guild-role.ts`:
- Around line 29-33: The current role resolution checks explicit access levels
before honoring guild.owner, causing { owner: true, access: 'viewer' } to return
'viewer'; update the logic in use-guild-role to first short-circuit if
guild.owner is truthy (return 'owner') and also handle access === 'owner' before
falling back to the 'viewer' branch, then keep the remaining checks for
'bot-owner', 'admin', and 'moderator' as-is so ownership always takes precedence
over augmented access.
In `@web/src/lib/bot-api-proxy.ts`:
- Around line 102-110: The current entries.find predicate accepts any string for
entry.access so typos/new values override fallbackAccess and can cause hard
denies in accessSatisfiesRequirement; update the predicate and/or the return to
whitelist supported GuildAccessLevel values (use the GuildAccessLevel type/enum
or a Set of allowed values) and only accept entry.access when it matches one of
those supported values, otherwise fall back to fallbackAccess; e.g., locate the
entries.find predicate and the return that uses entry?.access and replace the
unconditional use of entry.access with a validated value
(supported.includes(entry.access) ? entry.access : fallbackAccess) so unknown
strings never override the fallback.
- Around line 58-63: resolveGuildAccess creates independent timeouts for
getMutualGuilds and the subsequent /guilds/access call, allowing the overall
auth path to exceed the intended REQUEST_TIMEOUT_MS; fix by creating a single
AbortController/AbortSignal with a single timeout budget (use AbortController +
setTimeout or compute a shared deadline) at the start of resolveGuildAccess and
pass that same signal into getMutualGuilds and the downstream request (the call
that hits "/guilds/access" / proxyToBotApi), instead of calling
AbortSignal.timeout(...) twice; update calls to
getMutualGuilds(token.accessToken, sharedSignal) and the proxyToBotApi/fetch for
"/guilds/access" to accept and use the same sharedSignal so the whole auth check
shares one deadline.
In `@web/src/lib/discord.server.ts`:
- Around line 29-35: In parseRetryAfterMs, don’t prefer the raw 'retry-after'
header before validating it; instead attempt to parse 'retry-after' and if that
yields an invalid value then try parsing 'x-ratelimit-reset-after' and only fall
back to 1000ms if both parse to non-positive/NaN; update parseRetryAfterMs to
read both headers (response.headers.get('retry-after') and
response.headers.get('x-ratelimit-reset-after')), parse each into a finite
positive number (multiply by 1000 for ms) and return the first valid one,
otherwise return 1000.
In `@web/tests/api/guilds.test.ts`:
- Around line 39-45: The tests currently clear mocks and set NEXTAUTH_SECRET in
beforeEach but delete BOT_API_URL/BOT_API_SECRET and override globalThis.fetch
(and call mockGetBotApiBaseUrl.mockReturnValue(null)) without restoring them,
which can leak into other suites; add an afterEach teardown that restores
process.env.BOT_API_URL and process.env.BOT_API_SECRET to their original values
(capture originals at the top of the suite), restores globalThis.fetch to its
original value, and resets mockGetBotApiBaseUrl (and any other mocks) so that
functions like beforeEach, mockGetBotApiBaseUrl, and globalThis.fetch are
returned to their pre-test state.
In `@web/tests/lib/bot-api-proxy-branches.test.ts`:
- Around line 145-157: The test only stubs the global fetch response once via
mockResolvedValueOnce so authorizeGuildModerator(...) gets the mocked
[{id:'guild-1', access:'moderator'}] but authorizeGuildAdmin(...) falls back to
error behavior; update the test to mock the /guilds/access response for both
authorization calls—either add a second (globalThis.fetch as ReturnType<typeof
vi.fn>).mockResolvedValueOnce(...) with the same moderator access payload or
replace mockResolvedValueOnce with mockResolvedValue so both
authorizeGuildModerator and authorizeGuildAdmin receive the intended [{ id:
'guild-1', access: 'moderator' }] response while keeping mockGetToken and
mockGetMutualGuilds as-is.
In `@web/tests/lib/discord.test.ts`:
- Around line 473-493: The test "fails fast when bot API retry-after is too
large" uses a retry-after of "728"s which exceeds the generic fetchWithRateLimit
cap and doesn't validate the bot-specific fail-fast; update the mocked response
headers in this test (the fetchSpy mock used by fetchBotGuilds) to return a
sub-5s retry-after (e.g., "1") so the bot-specific cap is exercised and this
becomes a true regression test for fetchBotGuilds' bot API path.
---
Outside diff comments:
In `@web/src/app/api/guilds/`[guildId]/members/[userId]/cases/route.ts:
- Around line 15-31: The top doc comment for the GET handler is outdated: it
says "enforces guild-admin authorization" but the code calls
authorizeGuildModerator (with LOG_PREFIX) and enforces moderator authorization;
update the comment block above the GET function to describe that the route
validates params, enforces guild-moderator authorization via
authorizeGuildModerator, forwards query params to the bot API, and returns the
upstream NextResponse or error (e.g., 400 for missing guildId/userId or
authorization error) so the documentation matches the actual behavior.
In `@web/src/app/api/guilds/`[guildId]/members/[userId]/route.ts:
- Around line 14-34: Update the stale JSDoc on the GET handler to reflect the
actual authorization check: replace "enforces guild admin authorization" with
"enforces guild moderator authorization" (or otherwise mention use of
authorizeGuildModerator) and ensure the summary and param/return descriptions
remain accurate for the GET function that calls authorizeGuildModerator
(referencing authorizeGuildModerator, GET, and LOG_PREFIX to locate the code).
In `@web/src/app/api/guilds/`[guildId]/members/export/route.ts:
- Around line 12-28: The JSDoc incorrectly states "guild-admin authorization"
but the code uses authorizeGuildModerator (and LOG_PREFIX) to allow moderators;
update the function comment above export GET to accurately describe that the
route enforces guild-moderator (not admin-only) authorization, and adjust any
wording about who can access the export and required permissions to match the
authorizeGuildModerator behavior used in the GET handler.
In `@web/src/app/api/guilds/`[guildId]/members/route.ts:
- Around line 15-31: The function comment incorrectly states "requester is a
guild admin" while the actual authorization call is authorizeGuildModerator in
the GET handler; update the docblock to reflect that the requester must be a
guild moderator (or use consistent terminology "moderator" across the doc) and
mention authorizeGuildModerator/LOG_PREFIX as the enforcing mechanism so the
description matches the implemented check.
In `@web/src/app/api/moderation/cases/route.ts`:
- Around line 15-27: The JSDoc above the exported GET function is out of
date—update the comment to reflect moderator (not admin) authorization: change
the description line that currently says “Requires guildId query param and admin
authorization.” to reference moderator authorization, and mention that
authorizeGuildModerator is used (see GET and authorizeGuildModerator with
LOG_PREFIX) so the docstring matches the enforced behavior.
In `@web/src/app/api/moderation/stats/route.ts`:
- Around line 14-25: Update the stale JSDoc above the GET handler to reflect the
current authorization check: replace "admin authorization" with the accurate
requirement (e.g., "guild moderator authorization via authorizeGuildModerator").
Ensure the comment references the guildId query param and the use of
authorizeGuildModerator and LOG_PREFIX so readers understand the actual
authorization logic used by the GET(request: NextRequest) function.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ff78055-4f9f-4a85-82bf-46657b944fc8
📒 Files selected for processing (38)
migrations/014_audit_logs_user_tag_backfill.cjsmigrations/015_perf_indexes.cjssrc/api/middleware/rateLimit.jssrc/api/middleware/redisRateLimit.jssrc/api/middleware/trustedInternalRequest.jssrc/api/routes/guilds.jssrc/api/routes/members.jssrc/api/routes/tickets.jssrc/utils/loadCommands.jstests/api/middleware/rateLimit.test.jstests/api/middleware/redisRateLimit.test.jstests/api/routes/guilds.coverage.test.jstests/commands.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/moderation/cases/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/src/components/layout/server-selector.tsxweb/src/components/layout/sidebar.tsxweb/src/hooks/use-guild-role.tsweb/src/lib/bot-api-proxy.tsweb/src/lib/discord.server.tsweb/src/types/discord.tsweb/tests/api/guilds.test.tsweb/tests/components/layout/server-selector.test.tsxweb/tests/components/layout/sidebar.test.tsxweb/tests/hooks/use-guild-role.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.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: Agent
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (13)
migrations/*.cjs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must use
.cjsextension (CommonJS) because node-pg-migrate does not support ESM; use sequential numbering (001_, 002_, etc.) and always use parameterized queries—never string interpolation
Files:
migrations/015_perf_indexes.cjsmigrations/014_audit_logs_user_tag_backfill.cjs
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
web/src/app/api/moderation/cases/route.tstests/commands.test.jsweb/src/components/layout/server-selector.tsxweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tssrc/utils/loadCommands.jsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tssrc/api/middleware/redisRateLimit.jsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tssrc/api/middleware/trustedInternalRequest.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jsweb/tests/components/layout/server-selector.test.tsxtests/api/middleware/rateLimit.test.jssrc/api/routes/members.jstests/api/middleware/redisRateLimit.test.jssrc/api/middleware/rateLimit.jsweb/tests/lib/bot-api-proxy-branches.test.tstests/api/routes/guilds.coverage.test.jsweb/tests/components/layout/sidebar.test.tsxweb/tests/lib/discord.test.tsweb/src/components/layout/sidebar.tsxweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/app/api/moderation/cases/route.tsweb/src/components/layout/server-selector.tsxweb/src/app/api/moderation/user/[userId]/history/route.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tsweb/src/components/layout/sidebar.tsxweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.ts
**/*.{js,mjs,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only
Files:
web/src/app/api/moderation/cases/route.tstests/commands.test.jsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tssrc/utils/loadCommands.jsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tssrc/api/middleware/redisRateLimit.jsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tssrc/api/middleware/trustedInternalRequest.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jstests/api/middleware/rateLimit.test.jssrc/api/routes/members.jstests/api/middleware/redisRateLimit.test.jssrc/api/middleware/rateLimit.jsweb/tests/lib/bot-api-proxy-branches.test.tstests/api/routes/guilds.coverage.test.jsweb/tests/lib/discord.test.tsweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts}: Use src/logger.js; do not use console.*
Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Use parameterized SQL only
Community features should be gated behind config..enabled. Moderation commands are the exception
Files:
web/src/app/api/moderation/cases/route.tstests/commands.test.jsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tssrc/utils/loadCommands.jsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tssrc/api/middleware/redisRateLimit.jsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tssrc/api/middleware/trustedInternalRequest.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jstests/api/middleware/rateLimit.test.jssrc/api/routes/members.jstests/api/middleware/redisRateLimit.test.jssrc/api/middleware/rateLimit.jsweb/tests/lib/bot-api-proxy-branches.test.tstests/api/routes/guilds.coverage.test.jsweb/tests/lib/discord.test.tsweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
web/**/*.{css,scss,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
web/**/*.{css,scss,ts,tsx}: Check both light and dark themes if colors or theming changed
Check responsive behavior if layout changed. Verify on mobile, tablet, and desktop
Files:
web/src/app/api/moderation/cases/route.tsweb/src/components/layout/server-selector.tsxweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tsweb/tests/components/layout/server-selector.test.tsxweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/components/layout/sidebar.test.tsxweb/tests/lib/discord.test.tsweb/src/components/layout/sidebar.tsxweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.ts
web/**/*.{ts,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the design system when making changes to the UI/UX. If you don't follow the design system, your changes will be rejected
Files:
web/src/app/api/moderation/cases/route.tsweb/src/components/layout/server-selector.tsxweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tsweb/tests/components/layout/server-selector.test.tsxweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/components/layout/sidebar.test.tsxweb/tests/lib/discord.test.tsweb/src/components/layout/sidebar.tsxweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.ts
{web/**/*.{ts,tsx},**/DESIGN.md}
📄 CodeRabbit inference engine (AGENTS.md)
Always update the design system when making changes to the UI/UX
Files:
web/src/app/api/moderation/cases/route.tsweb/src/components/layout/server-selector.tsxweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tsweb/tests/components/layout/server-selector.test.tsxweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/components/layout/sidebar.test.tsxweb/tests/lib/discord.test.tsweb/src/components/layout/sidebar.tsxweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.ts
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
tests/commands.test.jssrc/utils/loadCommands.jssrc/api/middleware/redisRateLimit.jssrc/api/middleware/trustedInternalRequest.jssrc/api/routes/tickets.jstests/api/middleware/rateLimit.test.jssrc/api/routes/members.jstests/api/middleware/redisRateLimit.test.jssrc/api/middleware/rateLimit.jstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/commands.test.jstests/api/middleware/rateLimit.test.jstests/api/middleware/redisRateLimit.test.jstests/api/routes/guilds.coverage.test.js
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/hooks/use-guild-role.test.tsweb/tests/components/layout/server-selector.test.tsxweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/components/layout/sidebar.test.tsxweb/tests/lib/discord.test.tsweb/tests/api/guilds.test.ts
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/utils/loadCommands.jssrc/api/middleware/redisRateLimit.jssrc/api/middleware/trustedInternalRequest.jssrc/api/routes/tickets.jssrc/api/routes/members.jssrc/api/middleware/rateLimit.jssrc/api/routes/guilds.js
src/api/routes/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create API route handlers in
src/api/routes/and mount them insrc/api/server.js; add auth middleware if the endpoint requires authentication
Files:
src/api/routes/tickets.jssrc/api/routes/members.jssrc/api/routes/guilds.js
🧠 Learnings (20)
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to migrations/*.cjs : Rename database migration files to maintain sequential ordering and avoid conflicts with production migrations that have already been executed
Applied to files:
migrations/015_perf_indexes.cjsmigrations/014_audit_logs_user_tag_backfill.cjs
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to migrations/*.cjs : Database migrations must use `.cjs` extension (CommonJS) because node-pg-migrate does not support ESM; use sequential numbering (001_, 002_, etc.) and always use parameterized queries—never string interpolation
Applied to files:
migrations/015_perf_indexes.cjsmigrations/014_audit_logs_user_tag_backfill.cjs
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
web/src/app/api/moderation/cases/route.tsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jsweb/tests/components/layout/server-selector.test.tsxsrc/api/routes/members.jsweb/tests/lib/bot-api-proxy-branches.test.tstests/api/routes/guilds.coverage.test.jsweb/tests/components/layout/sidebar.test.tsxweb/src/app/api/guilds/route.tssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
web/src/app/api/moderation/cases/route.tsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jsweb/tests/components/layout/server-selector.test.tsxsrc/api/routes/members.jstests/api/routes/guilds.coverage.test.jsweb/src/app/api/guilds/route.tssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/app/api/**/route.ts : Include guildId in signed WebSocket ticket payload when issuing tickets from dashboard endpoints
Applied to files:
web/src/app/api/moderation/cases/route.tsweb/src/app/api/moderation/user/[userId]/history/route.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/moderation/stats/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/moderation/cases/[id]/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/hooks/use-guild-role.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jssrc/api/routes/members.jsweb/tests/api/guilds.test.tsweb/src/app/api/guilds/route.tssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
tests/commands.test.jstests/api/middleware/rateLimit.test.jstests/api/middleware/redisRateLimit.test.jstests/api/routes/guilds.coverage.test.jsweb/tests/components/layout/sidebar.test.tsxweb/tests/api/guilds.test.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
tests/commands.test.jstests/api/middleware/redisRateLimit.test.jsweb/tests/components/layout/sidebar.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/commands/**/*.js : Create slash command definitions in `src/commands/`, exporting a slash command builder and an `execute` function
Applied to files:
tests/commands.test.jssrc/utils/loadCommands.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
web/tests/hooks/use-guild-role.test.tsweb/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/members/export/route.tsweb/src/app/api/guilds/[guildId]/members/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/cases/route.tsweb/src/app/api/guilds/[guildId]/members/[userId]/route.tsweb/src/types/discord.tsweb/src/hooks/use-guild-role.tssrc/api/routes/tickets.jsweb/tests/components/layout/server-selector.test.tsxsrc/api/routes/members.jstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.jsweb/src/lib/bot-api-proxy.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Applied to files:
web/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.tsweb/src/app/api/guilds/[guildId]/tickets/stats/route.tsweb/src/app/api/guilds/[guildId]/tickets/route.tssrc/api/routes/tickets.jsweb/tests/api/guilds.test.tsweb/src/lib/bot-api-proxy.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/ws/**/*.js : Enforce tenant scoping on WebSocket handlers: reject mismatched guildId in filters and require entry.guild_id === ws.guildId for broadcast matching
Applied to files:
web/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.ts
📚 Learning: 2026-03-22T01:55:35.217Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-22T01:55:35.217Z
Learning: Applies to **/*.{js,ts} : Use src/logger.js; do not use console.*
Applied to files:
src/utils/loadCommands.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/utils/cache.js : Use `src/utils/cache.js` for generic caching with Redis primary + in-memory fallback; use `src/utils/discordCache.js` for Discord API response caching (channels, roles, members); use `src/utils/reputationCache.js` for leaderboard and user reputation data; all caches auto-invalidate when config changes
Applied to files:
src/api/middleware/redisRateLimit.jssrc/api/routes/members.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/api/routes/**/*.js : Create API route handlers in `src/api/routes/` and mount them in `src/api/server.js`; add auth middleware if the endpoint requires authentication
Applied to files:
src/api/routes/tickets.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Applied to files:
web/tests/lib/bot-api-proxy-branches.test.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Applied to files:
tests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-22T01:55:35.217Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-22T01:55:35.217Z
Learning: Applies to **/*.{js,ts} : Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Applied to files:
tests/api/routes/guilds.coverage.test.jsweb/src/lib/discord.server.tssrc/api/routes/guilds.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{ts,tsx} : Web dashboard config editor should use category workspace navigation with reusable SettingsFeatureCard pattern (header + master toggle + Basic/Advanced blocks)
Applied to files:
web/src/components/layout/sidebar.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Always use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Never use `console.*` methods; use the Winston logger instead via `import logger from '../logger.js'` (adjust path as needed), then call `logger.info()`, `logger.warn()`, `logger.error()`, or `logger.debug()`
Applied to files:
src/api/routes/guilds.js
🪛 GitHub Check: CodeQL
src/api/routes/tickets.js
[failure] 77-77: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 205-205: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 337-337: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
src/api/routes/members.js
[failure] 89-89: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 299-299: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 299-475: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 608-608: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 806-806: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 937-937: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
🪛 GitHub Check: SonarCloud Code Analysis
web/tests/components/layout/sidebar.test.tsx
[warning] 19-19: Prefer globalThis over global.
web/src/app/api/guilds/route.ts
[warning] 97-99: Extract this nested ternary operation into an independent statement.
web/src/lib/bot-api-proxy.ts
[warning] 72-72: Extract this nested ternary operation into an independent statement.
🔇 Additional comments (15)
src/utils/loadCommands.js (1)
3-3: Cross-platform ESM module loading fix looks good.Using
pathToFileURL(...).hreffor dynamic import is the right fix for Windows path compatibility while
keeping ESM semantics intact.Also applies to: 35-35
tests/commands.test.js (1)
3-3: Test loader update is aligned with runtime loader behavior.Switching test imports to
file://URLs keeps command loading consistent across platforms and avoids
path-resolution issues on Windows.Also applies to: 24-24
migrations/015_perf_indexes.cjs (1)
1-2: LGTM!Comment correctly updated to reflect the new migration slot (015). The renumbering aligns with preserving the historical
014slot for the audit_logs backfill migration.migrations/014_audit_logs_user_tag_backfill.cjs (1)
1-28: LGTM! Well-designed repair migration with proper idempotency guards.The use of
IF EXISTS/IF NOT EXISTSguards ensures safe execution regardless of the database's prior migration state. The documentation clearly explains the schema drift scenario being addressed.web/src/app/api/moderation/user/[userId]/history/route.ts (1)
4-29: Moderator authorization swap is correct and clean.The import and auth guard update are consistent, and the control flow remains correct.
web/src/app/api/moderation/cases/[id]/route.ts (1)
4-29: Authorization change looks good here.Switching this read endpoint to
authorizeGuildModeratoris implemented correctly.web/src/app/api/guilds/[guildId]/tickets/[ticketId]/route.ts (1)
4-24: This moderator-gated ticket read path is implemented correctly.The updated guard is clean and preserves early-return auth handling.
web/src/app/api/guilds/[guildId]/tickets/stats/route.ts (1)
4-24: Permission guard update is correct.
authorizeGuildModeratoris applied consistently with the existing early-return auth pattern.src/api/middleware/redisRateLimit.js (1)
11-11: LGTM!The trusted internal request bypass is correctly placed before any Redis operations, making it efficient. This aligns with the in-memory rate limiter implementation and ensures dashboard-to-bot API calls aren't unnecessarily rate-limited.
Also applies to: 28-30
web/src/app/api/guilds/[guildId]/tickets/route.ts (1)
4-4: LGTM!The authorization downgrade from admin to moderator for the tickets endpoint aligns with the PR objective to grant moderators access to the Tickets section. The authorization flow remains correct.
Also applies to: 23-24
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts (1)
4-4: LGTM!The authorization change to moderator level for XP adjustments is appropriate since this is a moderation action. The thorough input validation and payload sanitization before proxying upstream are well-implemented.
Also applies to: 38-39
src/api/middleware/rateLimit.js (1)
6-7: LGTM!The trusted internal request bypass is consistent with the Redis rate limiter implementation. Placing it before any rate limit accounting is efficient and ensures both rate limiters behave uniformly for internal requests.
Also applies to: 61-64
src/api/routes/members.js (2)
17-17: LGTM! Static analysis false positive.The authorization change to
requireGuildModeratoraligns with the PR objectives. The CodeQL "Missing rate limiting" warnings are false positives —membersRateLimitmiddleware is correctly applied before authorization in the middleware chain on all routes (e.g., line 88 shows the order:membersRateLimit, thenrequireGuildModerator, thenvalidateGuild).Also applies to: 86-90
299-299: LGTM!The remaining authorization changes from
requireGuildAdmintorequireGuildModeratorare consistent and appropriate for the member management routes. All routes maintain the correct middleware order with rate limiting applied first.Also applies to: 605-609, 803-807, 934-938
src/api/routes/tickets.js (1)
12-12: LGTM! Static analysis false positive.The authorization changes to
requireGuildModeratorfor all ticket routes align with the PR objective to grant moderators access to the Tickets section. The CodeQL "Missing rate limiting" warnings are false positives —ticketRateLimitmiddleware is correctly applied before authorization in the middleware chain (e.g., lines 76, 204, 337 all show rate limiting applied first).Also applies to: 74-78, 202-206, 337-337
| const [guilds, setGuilds] = useState<MutualGuild[]>([]); | ||
| const isNavItemActive = (href: string) => | ||
| pathname === href || (href !== '/dashboard' && pathname.startsWith(`${href}/`)); | ||
| const hasActiveSecondaryItem = secondaryNav.some((item) => isNavItemActive(item.href)); | ||
| const activeSecondaryHref = secondaryNav.find((item) => isNavItemActive(item.href))?.href ?? null; | ||
| const activeGuildAccess = guilds.find((guild) => guild.id === guildId)?.access ?? null; | ||
| const visiblePrimaryNav = activeGuildAccess === 'moderator' ? moderatorPrimaryNav : primaryNav; | ||
| const visibleSecondaryNav = activeGuildAccess === 'moderator' ? [] : secondaryNav; |
There was a problem hiding this comment.
Don't default moderators to the full nav while access is unresolved.
guilds starts empty and the error path leaves it empty, so activeGuildAccess stays null until /api/guilds succeeds. That makes Lines 112-113 render the full nav on first paint and on any 401/timeout/parse failure, which undercuts the moderator-only dashboard restriction this change is trying to add.
Also applies to: 125-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/layout/sidebar.tsx` around lines 108 - 113, The current
logic uses activeGuildAccess (derived from guilds) and defaults to showing
primaryNav when access is null, so moderators see the full nav on first paint or
on fetch errors; change the fallback to a restrictive default until access is
known: update the expressions that set visiblePrimaryNav and visibleSecondaryNav
so they only show primaryNav when activeGuildAccess === 'moderator' and
otherwise hide primary (e.g., visiblePrimaryNav = activeGuildAccess ===
'moderator' ? moderatorPrimaryNav : []) while keeping visibleSecondaryNav =
activeGuildAccess === 'moderator' ? [] : secondaryNav, and ensure references to
guilds and activeGuildAccess remain unchanged elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/routes/guilds.js`:
- Around line 532-582: In the router.get('/access') handler, change the response
for missing API secret authentication by replacing the status code returned when
req.authMethod !== 'api-secret' from 403 to 401 (i.e., in the early return that
currently sends res.status(403).json(...)); keep the error message but use 401
Unauthorized to match the test expectation for missing authentication in this
endpoint.
In `@web/src/app/api/guilds/route.ts`:
- Around line 97-99: The nested ternary used to derive userId (using token.id
and token.sub) is hard to read; replace it with a small, explicit conditional or
helper to compute userId (e.g., a getUserId function or an if/else block) so the
logic is clearer, then pass that userId into applyAccessLevels(guilds, userId,
signal) as before; update the code around the userId declaration and the call to
applyAccessLevels to use the new clearer variable assignment.
In `@web/src/lib/bot-api-proxy.ts`:
- Around line 86-87: The nested ternary assigning userId from token is hard to
read; replace it by extracting the logic into a small clear block or helper:
compute userId by first checking typeof token.id === 'string' then falling back
to typeof token.sub === 'string' else ''. Update the assignment in
web/src/lib/bot-api-proxy.ts (variable userId) to use an if/else or a helper
function (e.g., getUserIdFromToken) to make the intent explicit and remove the
nested ternary.
In `@web/tests/hooks/use-guild-role.test.ts`:
- Around line 31-33: The test description for the case using
getGuildDashboardRole(createGuild({ permissions: '32' })) is inconsistent with
its assertion; either update the test title or the expected value. Change the
it(...) string from "returns admin for manage guild permissions" to "returns
moderator for manage guild permissions" to match the
expect(...).toBe('moderator') assertion (or, if the intended behavior is to
return 'admin', change the expected value to 'admin' instead); reference
getGuildDashboardRole and createGuild in the test when making the fix.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01acf502-1a2f-4c10-8088-538f7f03755c
📒 Files selected for processing (10)
src/api/routes/guilds.jstests/api/routes/guilds.coverage.test.jsweb/src/app/api/guilds/route.tsweb/src/hooks/use-guild-role.tsweb/src/lib/bot-api-proxy.tsweb/src/lib/discord.server.tsweb/tests/api/guilds.test.tsweb/tests/hooks/use-guild-role.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.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: Greptile Review
- GitHub Check: Docker Build Validation
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.tssrc/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/hooks/use-guild-role.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.ts
**/*.{js,mjs,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only
Files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.tssrc/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts}: Use src/logger.js; do not use console.*
Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Use parameterized SQL only
Community features should be gated behind config..enabled. Moderation commands are the exception
Files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.tssrc/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
web/**/*.{css,scss,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
web/**/*.{css,scss,ts,tsx}: Check both light and dark themes if colors or theming changed
Check responsive behavior if layout changed. Verify on mobile, tablet, and desktop
Files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.ts
web/**/*.{ts,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the design system when making changes to the UI/UX. If you don't follow the design system, your changes will be rejected
Files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.ts
{web/**/*.{ts,tsx},**/DESIGN.md}
📄 CodeRabbit inference engine (AGENTS.md)
Always update the design system when making changes to the UI/UX
Files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/discord.server.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.ts
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/tests/lib/discord.test.tsweb/tests/hooks/use-guild-role.test.ts
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
src/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/api/routes/guilds.js
src/api/routes/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create API route handlers in
src/api/routes/and mount them insrc/api/server.js; add auth middleware if the endpoint requires authentication
Files:
src/api/routes/guilds.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/api/routes/guilds.coverage.test.js
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
web/src/hooks/use-guild-role.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.tssrc/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
web/src/hooks/use-guild-role.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.tssrc/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/reactionrole.js : Enforce invoker role hierarchy check in /reactionrole add command to prevent non-owner users from configuring roles at or above their highest role
Applied to files:
web/src/hooks/use-guild-role.tsweb/tests/hooks/use-guild-role.test.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/bot-api-proxy.tsweb/tests/hooks/use-guild-role.test.tssrc/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/app/api/**/route.ts : Include guildId in signed WebSocket ticket payload when issuing tickets from dashboard endpoints
Applied to files:
web/src/hooks/use-guild-role.tsweb/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/bot-api-proxy.tssrc/api/routes/guilds.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Applied to files:
web/tests/api/guilds.test.tsweb/tests/lib/bot-api-proxy-branches.test.tsweb/src/app/api/guilds/route.tsweb/src/lib/bot-api-proxy.tssrc/api/routes/guilds.js
📚 Learning: 2026-03-11T17:18:17.626Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T17:18:17.626Z
Learning: Applies to src/**/{startup,command-register,reload}*.{js,ts} : Remove process.env.GUILD_ID runtime reads from bot startup and reload command registration
Applied to files:
web/tests/api/guilds.test.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
web/tests/api/guilds.test.tstests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/ws/**/*.js : Enforce tenant scoping on WebSocket handlers: reject mismatched guildId in filters and require entry.guild_id === ws.guildId for broadcast matching
Applied to files:
web/tests/lib/bot-api-proxy-branches.test.tsweb/src/app/api/guilds/route.tssrc/api/routes/guilds.js
📚 Learning: 2026-03-22T01:55:35.229Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-22T01:55:35.229Z
Learning: Applies to **/*.{js,ts} : Community features should be gated behind config.<feature>.enabled. Moderation commands are the exception
Applied to files:
web/src/app/api/guilds/route.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Always use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-22T01:55:35.229Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-22T01:55:35.229Z
Learning: Applies to **/*.{js,ts} : Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Applied to files:
src/api/routes/guilds.jstests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Never use `console.*` methods; use the Winston logger instead via `import logger from '../logger.js'` (adjust path as needed), then call `logger.info()`, `logger.warn()`, `logger.error()`, or `logger.debug()`
Applied to files:
src/api/routes/guilds.js
🪛 GitHub Check: SonarCloud Code Analysis
web/src/app/api/guilds/route.ts
[warning] 98-98: Extract this nested ternary operation into an independent statement.
web/src/lib/bot-api-proxy.ts
[warning] 87-87: Extract this nested ternary operation into an independent statement.
🔇 Additional comments (30)
src/api/routes/guilds.js (5)
239-243:accessSatisfiesRequirementlogic is correct but could be clearer.The function correctly implements the access hierarchy where
bot-ownersatisfies all requirements. The implementation is accurate for the moderator/admin access model.
245-251: BigInt permission check looks correct.The
hasPermissionFlagfunction properly handles BigInt conversion with a try-catch for invalid inputs, returningfalseon failure. This is a safe approach.
264-278: Potential race condition inmapWithConcurrencyworker index increment.The
index++operation is not atomic in JavaScript's single-threaded event loop, but since workers are async functions that await between iterations, there's no actual race condition here—each worker grabs an index before yielding. The implementation is correct for Node.js's cooperative concurrency model.
287-320:getGuildAccessLevelimplementation is solid.The function correctly:
- Checks bot owner status first
- Tries cache before fetching
- Only swallows "Unknown Member" errors (per previous review feedback)
- Propagates other errors for proper 5xx handling
- Falls back to
viewerfor non-members
488-505: GET /guilds filtering correctly uses the new access model.The code properly derives access level from OAuth data first, then falls back to
getGuildAccessLevelfor bot-based role evaluation. Guilds withvieweraccess are correctly filtered out.web/tests/hooks/use-guild-role.test.ts (1)
35-39: New test cases cover important edge cases.The tests correctly verify:
- Explicit
accessfield from API takes precedence over permission-derived access- Guild ownership (
owner: true) overrides a downgradedaccessvalueThis aligns with the past review feedback that was addressed.
Also applies to: 74-76
web/src/hooks/use-guild-role.ts (3)
29-33: Ownership precedence logic is now correct.The past review feedback was addressed:
guild.owneris now checked alongsideguild.access === 'owner'at the top of the function, ensuring a real server owner is never locked out by a staleaccess: 'viewer'value.
42-48: MANAGE_GUILD now correctly maps to 'moderator'.This change aligns with the PR objective to distinguish moderator access (MANAGE_GUILD) from admin access (ADMINISTRATOR), allowing the dashboard to limit moderator users to specific sections.
66-68:isGuildManageableuses explicit role comparison.The explicit equality checks (
role === 'owner' || role === 'admin' || role === 'moderator') are clearer than a negative check against'viewer', and correctly handle any future role additions.web/tests/lib/bot-api-proxy-branches.test.ts (3)
74-80:hasModeratorPermissiontests correctly cover permission bitfields.Tests verify:
'32'(MANAGE_GUILD) → true'2'(KICK_MEMBERS) → true'4'(BAN_MEMBERS) → true'0'(no permissions) → false'garbage'(invalid input) → false
145-162: Moderator authorization test now correctly mocks both fetch calls.The past review feedback was addressed: the test now chains two
mockResolvedValueOncecalls so bothauthorizeGuildModeratorandauthorizeGuildAdminreceive the mocked moderator access response, ensuring the admin assertion actually tests the 403 path rather than an error fallback.
164-173: Unknown access fallback test is well-designed.This test verifies that when the bot API returns an unrecognized access string (
'super-admin'), the authorization falls back to OAuth-derived permissions (permissions: '8'= ADMINISTRATOR), correctly allowing admin access.web/tests/lib/discord.test.ts (4)
128-138: Test correctly verifies no retry when delay cap exceeded.The test uses
retry-after: '728'(which exceeds the default max delay cap) and asserts only 1 fetch call is made, confirming the rate limiter correctly gives up early.
140-166: Budget exhaustion test has proper rateLimit configuration.The test configures a 2-second total budget and a 1.5-second retry-after, correctly expecting only 2 fetch calls (initial + one retry) before the budget is exhausted.
267-290: Fallback header test covers important edge case.When
retry-afteris malformed ('nope'), the test verifies the code falls back tox-ratelimit-reset-afterand successfully retries.
497-517: Bot API fail-fast test now uses appropriate retry-after value.The past review feedback was addressed: the test now uses
retry-after: '1'which is within the generic cap but exercises the bot-specific fail-fast path (maxRetryDelayMs: 250, totalRetryBudgetMs: 500).tests/api/routes/guilds.coverage.test.js (2)
121-146: Test properly restores mock state after member cache manipulation.The test correctly saves
originalCacheandoriginalFetch, manipulates them for the test scenarios, and restores them in Lines 144-145. This prevents test pollution.
107-153:⚠️ Potential issue | 🟡 MinorFix test assertion: expect 403, not 401 for missing API secret auth.
The test at lines 148-152 expects HTTP 401, but the
/accessendpoint implementation returns HTTP 403 whenauthMethod !== 'api-secret'. Update the assertion fromexpect(res.status).toBe(401)toexpect(res.status).toBe(403).⛔ Skipped due to learnings
Learnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-10T23:29:51.063Z Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resourcesLearnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-11T06:42:38.728Z Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectationsLearnt from: CR Repo: VolvoxLLC/volvox-bot PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-10T23:21:49.730Z Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record idweb/tests/api/guilds.test.ts (3)
50-66: Proper teardown now restores all globals.The past review feedback was addressed:
afterEachnow correctly restoresglobalThis.fetch,BOT_API_URL,BOT_API_SECRET, andNEXTAUTH_SECRETto their original values, preventing test pollution.
149-193: Access augmentation test verifies end-to-end behavior.The test correctly:
- Sets up bot API credentials
- Mocks fetch to return access entries
- Verifies the response includes the augmented
accessfield- Asserts the correct endpoint URL and headers were used
195-234: Unknown access fallback test ensures resilience.The test verifies that when the bot API returns an invalid access value (
'super-admin'), the original guildaccessvalue ('viewer') is preserved rather than being overwritten with the unknown value.web/src/app/api/guilds/route.ts (2)
57-70: Access validation correctly restricts to known enum values.The past review feedback was addressed: Line 67 now validates that
entry.accessis inVALID_ACCESS_LEVELSbefore including it in the map, preventing schema drift from silently breaking the moderator nav restriction.
14-80:applyAccessLevelsimplementation is robust.The function correctly:
- Short-circuits when bot API is not configured or no guilds exist
- Only queries for guilds where
botPresentis true- Validates response shape (array check + typed filter)
- Falls back gracefully on any error
- Uses the request signal for timeout propagation
web/src/lib/discord.server.ts (2)
29-42: LGTM! The retry-after parsing issue has been addressed.The
parseSecondshelper correctly validates each header independently before falling through to the next, ensuring a malformedretry-afterwon't mask a validx-ratelimit-reset-after.
49-98: LGTM! Well-structured retry budget tracking.The configurable rate limit options with per-request overrides allow fine-grained control over retry behavior. The early-exit conditions at lines 69-74 properly prevent excessive waits when delay cap or budget would be exceeded.
web/src/lib/bot-api-proxy.ts (5)
156-167: LGTM! The shared timeout budget has been implemented correctly.A single
AbortControllernow governs the entire authorization path, ensuring the total auth check respectsREQUEST_TIMEOUT_MS. Thefinallyblock properly cleans up the timeout.
117-126: LGTM! Access level validation now whitelists supported values.The predicate at line 123 properly validates against
GUILD_ACCESS_LEVELS, preventing unknown or typo'd access values from overriding the fallback.
43-55: LGTM! Moderator permission detection looks correct.The permission bitfield checks align with Discord's permission model. The OR-based check appropriately grants moderator access to users with any of MANAGE_GUILD, KICK_MEMBERS, BAN_MEMBERS, or MODERATE_MEMBERS.
57-70: LGTM! Access hierarchy is well-structured.The access satisfaction logic correctly places
bot-ownerandadminabovemoderator. Guild owners are appropriately granted admin-level access in the fallback path, aligning with Discord's permission model expectations. Based on learnings: "Guild owners should be explicitly exempted from role hierarchy restrictions."
188-202: LGTM! Clean authorization API for route handlers.The exported functions provide clear semantics for admin vs moderator authorization. The delegation pattern keeps the implementation DRY while exposing an intuitive interface.
| const userId = | ||
| typeof token.id === 'string' ? token.id : typeof token.sub === 'string' ? token.sub : ''; | ||
| const guildsWithAccess = userId ? await applyAccessLevels(guilds, userId, signal) : guilds; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract nested ternary for readability.
The nested ternary on Line 98 was flagged by SonarCloud. While functionally correct, it's harder to read at a glance.
♻️ Suggested refactor
- const userId =
- typeof token.id === 'string' ? token.id : typeof token.sub === 'string' ? token.sub : '';
+ let userId = '';
+ if (typeof token.id === 'string') {
+ userId = token.id;
+ } else if (typeof token.sub === 'string') {
+ userId = token.sub;
+ }
const guildsWithAccess = userId ? await applyAccessLevels(guilds, userId, signal) : guilds;📝 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 userId = | |
| typeof token.id === 'string' ? token.id : typeof token.sub === 'string' ? token.sub : ''; | |
| const guildsWithAccess = userId ? await applyAccessLevels(guilds, userId, signal) : guilds; | |
| let userId = ''; | |
| if (typeof token.id === 'string') { | |
| userId = token.id; | |
| } else if (typeof token.sub === 'string') { | |
| userId = token.sub; | |
| } | |
| const guildsWithAccess = userId ? await applyAccessLevels(guilds, userId, signal) : guilds; |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 98-98: Extract this nested ternary operation into an independent statement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/app/api/guilds/route.ts` around lines 97 - 99, The nested ternary
used to derive userId (using token.id and token.sub) is hard to read; replace it
with a small, explicit conditional or helper to compute userId (e.g., a
getUserId function or an if/else block) so the logic is clearer, then pass that
userId into applyAccessLevels(guilds, userId, signal) as before; update the code
around the userId declaration and the call to applyAccessLevels to use the new
clearer variable assignment.
# Conflicts: # migrations/015_perf_indexes.cjs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/api/routes/guilds.js:357
- When
guildis present in the client cache, the middleware always callsgetGuildAccessLevel()(which may doguild.members.fetch) and never uses the OAuth permission-bit path (permissionCheck, which is backed by the cached/users/@me/guildscall). This can increase Discord API load on hot endpoints. Consider checkingpermissionCheckfirst (fast path) and only falling back togetGuildAccessLevel()when permission bits aren’t sufficient but configured roles might still grant access.
try {
const guild = req.app.locals.client?.guilds?.cache?.get(req.params.id);
if (guild) {
const access = await getGuildAccessLevel(guild, req.user.userId);
if (!accessSatisfiesRequirement(access, requiredAccess)) {
return res.status(403).json({ error: errorMessage });
}
return next();
}
if (!(await permissionCheck(req.user, req.params.id))) {
return res.status(403).json({ error: errorMessage });
}
return next();
} catch (err) {
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/routes/members.js`:
- Around line 928-932: Update the comment that currently reads "Admin XP
adjustment" above the XP route to match the actual authorization used by the
route handler: change it to "Moderator XP adjustment" so it aligns with the
middleware and proxy (the router.post route for '/:id/members/:userId/xp' which
uses requireGuildModerator and the corresponding authorizeGuildModerator in the
web proxy). Ensure the comment text is exactly updated and no other code is
changed.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 28d1c7c6-8c93-46c3-b06c-04cb9df32bcb
📒 Files selected for processing (2)
src/api/routes/guilds.jssrc/api/routes/members.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Greptile Review
- GitHub Check: E2E Tests (1/2)
- GitHub Check: E2E Tests (2/2)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
src/api/routes/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create API route handlers in
src/api/routes/and mount them insrc/api/server.js; add auth middleware if the endpoint requires authentication
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts}: Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Community features should be gated behind config..enabled. Moderation commands are the exception.
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
src/api/routes/members.jssrc/api/routes/guilds.js
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
src/api/routes/members.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
src/api/routes/members.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/app/api/**/route.ts : Include guildId in signed WebSocket ticket payload when issuing tickets from dashboard endpoints
Applied to files:
src/api/routes/members.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/api/routes/**/*.js : Create API route handlers in `src/api/routes/` and mount them in `src/api/server.js`; add auth middleware if the endpoint requires authentication
Applied to files:
src/api/routes/members.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
src/api/routes/members.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/utils/cache.js : Use `src/utils/cache.js` for generic caching with Redis primary + in-memory fallback; use `src/utils/discordCache.js` for Discord API response caching (channels, roles, members); use `src/utils/reputationCache.js` for leaderboard and user reputation data; all caches auto-invalidate when config changes
Applied to files:
src/api/routes/members.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/ws/**/*.js : Enforce tenant scoping on WebSocket handlers: reject mismatched guildId in filters and require entry.guild_id === ws.guildId for broadcast matching
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Always use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-26T00:04:14.673Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.673Z
Learning: Applies to src/**/*.{js,ts} : Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Never use `console.*` methods; use the Winston logger instead via `import logger from '../logger.js'` (adjust path as needed), then call `logger.info()`, `logger.warn()`, `logger.error()`, or `logger.debug()`
Applied to files:
src/api/routes/guilds.js
🪛 GitHub Check: CodeQL
src/api/routes/members.js
[failure] 77-77: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 289-289: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 289-468: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 601-601: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 800-800: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
[failure] 931-931: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
🔇 Additional comments (12)
src/api/routes/guilds.js (7)
533-537: Return 401 instead of 403 for missing API secret authentication.The endpoint returns 403 when
req.authMethod !== 'api-secret', but this should be 401 (Unauthorized) since the request lacks the required authentication method. Reserve 403 for cases where authentication succeeded but authorization failed. The test attests/api/routes/guilds.coverage.test.jsalso expects 401.🔧 Proposed fix
if (req.authMethod !== 'api-secret') { return res - .status(403) + .status(401) .json({ error: 'Guild access endpoint requires API secret authentication' }); }
239-258: LGTM! Well-structured access level helpers.The helper functions are clean and correctly implement the permission hierarchy:
accessSatisfiesRequirementproperly treatsbot-owneras having all permissionshasPermissionFlagsafely handles BigInt conversion with a try/catchgetOAuthDerivedAccessLevelderives access from OAuth permissions with appropriate fallback
260-278: LGTM! Correct concurrent mapping implementation.The
mapWithConcurrencyhelper correctly implements a bounded-concurrency worker pattern, andisUnknownMemberErrorproperly identifies Discord's "Unknown Member" error (code 10007) for the member fetch fallback logic.
287-320: LGTM! Robust access level resolution.The function correctly implements the permission hierarchy and properly handles Discord member fetch errors by only treating "Unknown Member" as a non-member (returning
viewer), while re-throwing transient errors to surface them as 502s upstream.
336-387: LGTM! Well-layered permission middleware.The updated middleware properly:
- Bypasses checks for API-secret and bot owners
- Uses
getGuildAccessLevelfor cached guilds with config-aware role evaluation- Falls back to OAuth permission check when guild isn't cached
- Returns 502 (not 403) on transient Discord errors
488-505: LGTM! Proper access-level filtering with fallback.The filtering correctly derives access from OAuth permissions first (
getOAuthDerivedAccessLevel), falling back togetGuildAccessLevelonly when needed. This minimizes Discord API calls while still honoring configured moderator/admin roles.
560-582: LGTM! Bounded-concurrency access resolution.The
/accessendpoint correctly:
- Normalizes and deduplicates guild IDs
- Uses
mapWithConcurrencyto limit parallel Discord lookups- Filters out null entries (guilds not in cache)
- Returns 502 on transient Discord errors
src/api/routes/members.js (5)
15-15: LGTM! Authorization import aligns with PR objectives.The switch from
requireGuildAdmintorequireGuildModeratorcorrectly enables moderator-level access to the Members dashboard section, consistent with the web proxy layer changes.
74-78: LGTM! Rate limiting is present.The static analysis hint about "Missing rate limiting" is a false positive—
membersRateLimitis correctly applied at line 76 beforerequireGuildModeratorat line 77.
289-289: LGTM! Authorization and rate limiting are correctly chained.The static analysis warnings are false positives. The middleware chain correctly applies:
membersRateLimit— rate limitingrequireGuildModerator— authorizationvalidateGuild— guild validation
598-602: LGTM! Member detail endpoint correctly gated.Rate limiting (
membersRateLimit) and authorization (requireGuildModerator) are both applied, matching the PR's intent to allow moderator access to member details.
797-801: LGTM! Case history endpoint correctly gated.Moderators can now view member moderation history, which aligns with their role in handling moderation tasks. Rate limiting is properly applied.
BillChirico
added
scope: api
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
web/src/components/layout/guild-directory-context.tsx (2)
51-54: 🧹 Nitpick | 🔵 TrivialUse
TypeErrorfor invalid data shape.When the response data shape doesn't match expectations,
TypeErroris more semantically appropriate than genericError.✏️ Suggested fix
const data: unknown = await response.json(); if (!Array.isArray(data)) { - throw new Error('Invalid guild response'); + throw new TypeError('Invalid guild response'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/guild-directory-context.tsx` around lines 51 - 54, Replace the generic Error thrown when the fetched response shape is wrong with a TypeError to reflect an invalid data type; in the block where you parse response.json() into the local variable data and check Array.isArray(data), throw new TypeError('Invalid guild response') (or equivalent) instead of Error('Invalid guild response') so the error type correctly represents a type/shape violation.
27-27: 🧹 Nitpick | 🔵 TrivialMark the props of the component as read-only.
Per React best practices and static analysis hints, the props should be marked as read-only.
✏️ Suggested fix
-export function GuildDirectoryProvider({ children }: { children: React.ReactNode }) { +export function GuildDirectoryProvider({ children }: Readonly<{ children: React.ReactNode }>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/guild-directory-context.tsx` at line 27, The GuildDirectoryProvider component's props should be marked read-only; update the function signature for GuildDirectoryProvider to use a readonly prop type (e.g., Readonly<{ children: React.ReactNode }> or { readonly children: React.ReactNode }) so the incoming props cannot be mutated; locate the GuildDirectoryProvider declaration and change its parameter type accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/api/middleware/auth.test.js`:
- Around line 139-150: Add two unit tests for requireAuth middleware covering
edge cases when x-api-secret is valid but x-discord-user-id is missing or blank:
(1) stub BOT_API_SECRET and set req.headers['x-api-secret'] to the secret but do
NOT set req.headers['x-discord-user-id'], then call middleware and assert next()
was called, req.authMethod === 'api-secret' and req.user is undefined; (2) same
setup but set req.headers['x-discord-user-id'] = ' ' (whitespace), call
middleware and assert next() was called, req.authMethod === 'api-secret' and
req.user is undefined to ensure trimming/empty-check logic in requireAuth is
exercised.
In `@tests/api/routes/guilds.coverage.test.js`:
- Line 258: The assertion in the test that checks the POST /:id/actions response
currently expects 'admin access' but the endpoint actually returns 'API secret';
update the assertion in tests/api/routes/guilds.coverage.test.js (the
expect(res.body.error).toContain(...) call) to look for 'API secret' instead so
it matches the error text produced by the route handler (see the error string in
the guilds route) and aligns with the other test that expects 'API secret'.
In `@web/src/app/api/guilds/`[guildId]/members/[userId]/xp/route.ts:
- Around line 106-119: The code currently falls back from token.id to token.sub
when building the x-discord-user-id header; remove that fallback and require
token.id only: use getToken(...) and if token?.id is not a string return a 401
response instead of calling proxyToBotApi, and when present send only token.id
in the 'x-discord-user-id' header; update the route handler around requesterId
(and its usage in the headers) to enforce this, and add a regression test that
POST to this route returns 401 when getToken() yields no id (mock getToken to
return undefined/id missing) to ensure we never proxy with token.sub.
---
Duplicate comments:
In `@web/src/components/layout/guild-directory-context.tsx`:
- Around line 51-54: Replace the generic Error thrown when the fetched response
shape is wrong with a TypeError to reflect an invalid data type; in the block
where you parse response.json() into the local variable data and check
Array.isArray(data), throw new TypeError('Invalid guild response') (or
equivalent) instead of Error('Invalid guild response') so the error type
correctly represents a type/shape violation.
- Line 27: The GuildDirectoryProvider component's props should be marked
read-only; update the function signature for GuildDirectoryProvider to use a
readonly prop type (e.g., Readonly<{ children: React.ReactNode }> or { readonly
children: React.ReactNode }) so the incoming props cannot be mutated; locate the
GuildDirectoryProvider declaration and change its parameter type accordingly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3eef2bbf-fd88-4850-8cfc-c91301ccf179
📒 Files selected for processing (8)
src/api/middleware/auth.jssrc/api/routes/guilds.jstests/api/middleware/auth.test.jstests/api/routes/guilds.coverage.test.jstests/api/routes/members.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/layout/guild-directory-context.tsxweb/tests/api/xp-route.test.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). (4)
- GitHub Check: Agent
- GitHub Check: Greptile Review
- GitHub Check: E2E Tests (1/2)
- GitHub Check: E2E Tests (2/2)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
tests/api/middleware/auth.test.jssrc/api/middleware/auth.jstests/api/routes/members.test.jstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
tests/api/middleware/auth.test.jssrc/api/middleware/auth.jstests/api/routes/members.test.jsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.coverage.test.jsweb/src/components/layout/guild-directory-context.tsxsrc/api/routes/guilds.js
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/api/middleware/auth.test.jstests/api/routes/members.test.jstests/api/routes/guilds.coverage.test.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
tests/api/middleware/auth.test.jssrc/api/middleware/auth.jstests/api/routes/members.test.jsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.coverage.test.jsweb/src/components/layout/guild-directory-context.tsxsrc/api/routes/guilds.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
tests/api/middleware/auth.test.jssrc/api/middleware/auth.jstests/api/routes/members.test.jsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/api/middleware/auth.jssrc/api/routes/guilds.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts}: Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Community features should be gated behind config..enabled. Moderation commands are the exception.
Files:
src/api/middleware/auth.jssrc/api/routes/guilds.js
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/api/xp-route.test.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/layout/guild-directory-context.tsx
src/api/routes/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create API route handlers in
src/api/routes/and mount them insrc/api/server.js; add auth middleware if the endpoint requires authentication
Files:
src/api/routes/guilds.js
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Applied to files:
src/api/middleware/auth.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
web/tests/api/xp-route.test.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/app/api/**/route.ts : Include guildId in signed WebSocket ticket payload when issuing tickets from dashboard endpoints
Applied to files:
web/tests/api/xp-route.test.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tssrc/api/routes/guilds.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
web/tests/api/xp-route.test.tsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-26T00:04:14.673Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.673Z
Learning: Applies to src/**/*.{js,ts} : Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Applied to files:
tests/api/routes/guilds.coverage.test.jssrc/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
tests/api/routes/guilds.coverage.test.js
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/ws/**/*.js : Enforce tenant scoping on WebSocket handlers: reject mismatched guildId in filters and require entry.guild_id === ws.guildId for broadcast matching
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Always use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Applied to files:
src/api/routes/guilds.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Never use `console.*` methods; use the Winston logger instead via `import logger from '../logger.js'` (adjust path as needed), then call `logger.info()`, `logger.warn()`, `logger.error()`, or `logger.debug()`
Applied to files:
src/api/routes/guilds.js
🪛 GitHub Check: SonarCloud Code Analysis
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts
[warning] 110-112: Extract this nested ternary operation into an independent statement.
web/src/components/layout/guild-directory-context.tsx
[warning] 53-53: new Error() is too unspecific for a type check. Use new TypeError() instead.
[warning] 44-44: Prefer globalThis over window.
[warning] 27-27: Mark the props of the component as read-only.
🔇 Additional comments (10)
src/api/middleware/auth.js (1)
51-57: LGTM!The trusted user identity propagation is correctly implemented:
- Header is only read after API-secret validation succeeds
- Value is properly trimmed and only assigned if non-empty
- The
req.usershape{ userId }aligns with OAuth JWT auth pathtests/api/routes/members.test.js (1)
511-535: LGTM!The test properly validates moderator attribution logging for XP adjustments. The use of
expect.objectContainingprovides appropriate flexibility while ensuring the key assertion.tests/api/routes/guilds.coverage.test.js (1)
107-164: LGTM!The new
/accessendpoint tests provide good coverage:
- Moderator access resolution via mocked
isModerator- Graceful handling of
Unknown Membererrors (→viewer) vs transient Discord failures (→502)- Auth enforcement (401 for non-api-secret)
- Batch size cap validation
The save/restore pattern for
mockGuild.members.cacheandmockGuild.members.fetchin lines 122-145 is clean.web/src/components/layout/guild-directory-context.tsx (1)
33-67: LGTM on the fetch/abort logic.The
refreshGuildsimplementation correctly:
- Aborts previous in-flight requests before starting a new one
- Uses
AbortControllersignal for cancellation- Handles abort errors gracefully (returns early, doesn't set error state)
- Only clears loading state if this request is still the current one (line 63 check)
src/api/routes/guilds.js (5)
240-244: LGTM!The access hierarchy logic is correct:
bot-ownersupersedes all,adminis required for admin-gated routes, and bothadminandmoderatorsatisfy moderator requirements.
265-279: LGTM!The
mapWithConcurrencyimplementation is correct. The sharedindexvariable is safe in Node.js's single-threaded event loop since JS-level increments are atomic between await points.
288-321: LGTM!The
getGuildAccessLevelfunction correctly:
- Prioritizes bot owner check
- Uses cache-first with API fallback for member lookup
- Only swallows "Unknown Member" errors, re-throws transient failures (addresses past review)
- Falls through to role-based checks via
isAdmin/isModerator
340-377: LGTM!The updated
requireGuildPermissionmiddleware correctly gates OAuth requests using the newrequiredAccessparameter andaccessSatisfiesRequirementcheck, while preserving the API-secret and bot-owner bypass paths.
539-594: LGTM!The
/accessendpoint correctly:
- Enforces API-secret authentication with 401 (addresses past review)
- Validates and deduplicates
guildIds- Caps batch size at 100 (addresses past review)
- Uses concurrency-limited resolution
- Returns 502 for Discord API failures
web/tests/api/xp-route.test.ts (1)
180-188: Good regression coverage for requester attribution.This locks in the
x-discord-user-idforwarding contract the bot API now depends on for moderator-side re-authorization.
|
|
||
| expect(res.status).toBe(403); | ||
| expect(res.body.error).toContain('API secret'); | ||
| expect(res.body.error).toContain('admin access'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual error message returned by POST /:id/actions for OAuth (non-api-secret) auth
rg -n "Actions endpoint requires" src/api/routes/guilds.js -A 1 -B 1
# Also check both test files for the 403 assertion on POST /:id/actions
echo "--- guilds.coverage.test.js assertion ---"
rg -n "expect.*error.*toContain" tests/api/routes/guilds.coverage.test.js | grep -i "403\|actions\|admin\|api.secret" | head -5
echo "--- guilds.test.js assertion ---"
rg -n "expect.*error.*toContain.*API secret" tests/api/routes/guilds.test.js | head -5Repository: VolvoxLLC/volvox-bot
Length of output: 416
Fix inconsistent error message assertion in guilds.coverage.test.js.
The test at line 258 expects 'admin access' but should expect 'API secret' to match the actual error message returned by the endpoint (line 1832 in src/api/routes/guilds.js). This test covers the same POST /:id/actions scenario as tests/api/routes/guilds.test.js at line 1340, which correctly expects 'API secret'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/api/routes/guilds.coverage.test.js` at line 258, The assertion in the
test that checks the POST /:id/actions response currently expects 'admin access'
but the endpoint actually returns 'API secret'; update the assertion in
tests/api/routes/guilds.coverage.test.js (the
expect(res.body.error).toContain(...) call) to look for 'API secret' instead so
it matches the error text produced by the route handler (see the error string in
the guilds route) and aligns with the other test that expects 'API secret'.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/tests/components/dashboard/config-editor-autosave.test.tsx (1)
141-166:⚠️ Potential issue | 🟡 MinorAdd the inverse assertion so these tests actually prove route gating.
ConfigEditormounts both<AiAutomationCategory />and<OnboardingGrowthCategory />. With only positive assertions here, both tests still pass if both sections render on every route. Please also assert that the non-selected category is hidden on each pathname so the pathname-based filtering is actually covered.Also applies to: 168-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/tests/components/dashboard/config-editor-autosave.test.tsx` around lines 141 - 166, The test only asserts presence of the selected category but not absence of the other, so add inverse assertions to verify route gating: in the 'renders AI settings on the AI category route' test (using ConfigEditor) assert that the non-selected category component (e.g., text unique to OnboardingGrowthCategory like "Onboarding" or whatever its visible label is) is not present (use queryByText and expect(...).toBeNull() or expect(...).not.toBeInTheDocument()); and make the analogous change in the other test (the one covering the Onboarding/Growth route) to assert AiAutomationCategory-specific text (e.g., "AI Chat") is not present. Reference the ConfigEditor render and the AiAutomationCategory / OnboardingGrowthCategory visible labels when adding these negative assertions so the tests actually verify pathname-based filtering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/api/routes/guilds.test.js`:
- Around line 369-384: The test mutates shared mocks mockGuild.members.cache and
mockGuild.members.fetch and restores them only after assertions, which can leak
state if an assertion fails; wrap the mutation and assertions in a try/finally
block (capture originalCache and originalFetch, assign the new values, run the
request/assertions inside try, and restore originalCache/originalFetch in the
finally) for this test and the other similar test that mutates the same mocks to
ensure cleanup always runs.
In `@tests/modules/cli-process.test.js`:
- Around line 650-657: The test currently converts sendP into a resolved error
value which weakens the assertion; instead attach a noop catch to
cli.send('slow') to suppress an early unhandled-rejection, keep sendP as the
original Promise, advance timers as before, and then assert on sendP itself that
it rejects with an instance of CLIProcessError and that the rejection message
contains 'timed out after 3000ms' (use the existing sendP, cli.send, and
CLIProcessError identifiers to locate and update the assertions).
In `@web/src/components/layout/guild-directory-context.tsx`:
- Around line 71-79: The abort cleanup leaves abortControllerRef.current
pointing at the aborted controller so refreshGuilds()'s finally can still call
setLoading(false) after unmount; update the useEffect cleanup to both abort and
clear the ref (set abortControllerRef.current = undefined/null) so the equality
check in refreshGuilds (abortControllerRef.current === controller) will fail
after unmount; ensure the change is applied around the useEffect that calls
refreshGuilds and references abortControllerRef, so no other logic needs
changing.
- Around line 23-32: The runtime guard is missing validation for the optional
MutualGuild.access field, so invalid values like { access: 'bogus' } pass;
update isMutualGuild to, when an access property exists, ensure typeof access
=== 'string' and that its value matches the exact closed set of literals defined
for MutualGuild.access (use the same union literal values used in the
MutualGuild type/enum), returning false for any other types or unexpected
strings; keep the rest of the existing checks (id, name, permissions, owner,
botPresent) intact.
In `@web/tests/api/xp-route.test.ts`:
- Around line 56-60: The test uses mockGetToken.mockResolvedValue({ id:
'moderator-1' }) which is not a valid Discord snowflake and fails the new auth
check; change the mocked token id(s) in xp-route.test.ts (both occurrences where
mockGetToken.mockResolvedValue is called) to a numeric string matching
/^\d{17,20}$/ (e.g., a 17–20 digit snowflake) so the header x-discord-user-id
will pass src/api/middleware/auth.js validation and exercise the real
success-path behavior.
---
Outside diff comments:
In `@web/tests/components/dashboard/config-editor-autosave.test.tsx`:
- Around line 141-166: The test only asserts presence of the selected category
but not absence of the other, so add inverse assertions to verify route gating:
in the 'renders AI settings on the AI category route' test (using ConfigEditor)
assert that the non-selected category component (e.g., text unique to
OnboardingGrowthCategory like "Onboarding" or whatever its visible label is) is
not present (use queryByText and expect(...).toBeNull() or
expect(...).not.toBeInTheDocument()); and make the analogous change in the other
test (the one covering the Onboarding/Growth route) to assert
AiAutomationCategory-specific text (e.g., "AI Chat") is not present. Reference
the ConfigEditor render and the AiAutomationCategory / OnboardingGrowthCategory
visible labels when adding these negative assertions so the tests actually
verify pathname-based filtering.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ffde336b-aed7-4363-aa56-24994ba6005a
📒 Files selected for processing (14)
src/api/middleware/auth.jstests/api/middleware/auth.test.jstests/api/routes/guilds.test.jstests/api/routes/members.test.jstests/modules/ai.test.jstests/modules/cli-process.test.jsvitest.config.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/dashboard/config-editor.tsxweb/src/components/layout/guild-directory-context.tsxweb/tests/api/xp-route.test.tsweb/tests/components/dashboard/analytics-dashboard.test.tsxweb/tests/components/dashboard/config-diff-modal.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
vitest.config.jstests/modules/ai.test.jstests/modules/cli-process.test.jssrc/api/middleware/auth.jstests/api/middleware/auth.test.jstests/api/routes/members.test.jstests/api/routes/guilds.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
vitest.config.jsweb/tests/components/dashboard/config-diff-modal.test.tsxweb/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/analytics-dashboard.test.tsxtests/modules/ai.test.jstests/modules/cli-process.test.jssrc/api/middleware/auth.jstests/api/middleware/auth.test.jstests/api/routes/members.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/components/dashboard/config-editor-autosave.test.tsxtests/api/routes/guilds.test.jsweb/src/components/layout/guild-directory-context.tsxweb/tests/api/xp-route.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
vitest.config.jsweb/tests/components/dashboard/config-diff-modal.test.tsxweb/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/analytics-dashboard.test.tsxtests/modules/ai.test.jstests/modules/cli-process.test.jssrc/api/middleware/auth.jstests/api/middleware/auth.test.jstests/api/routes/members.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/components/dashboard/config-editor-autosave.test.tsxtests/api/routes/guilds.test.jsweb/src/components/layout/guild-directory-context.tsxweb/tests/api/xp-route.test.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
vitest.config.jstests/modules/ai.test.jstests/modules/cli-process.test.jssrc/api/middleware/auth.jstests/api/middleware/auth.test.jstests/api/routes/members.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.test.jsweb/tests/api/xp-route.test.ts
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/components/dashboard/config-diff-modal.test.tsxweb/tests/components/dashboard/analytics-dashboard.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/xp-route.test.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/components/dashboard/config-editor.tsxweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/layout/guild-directory-context.tsx
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/modules/ai.test.jstests/modules/cli-process.test.jstests/api/middleware/auth.test.jstests/api/routes/members.test.jstests/api/routes/guilds.test.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/api/middleware/auth.js
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts}: Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Community features should be gated behind config..enabled. Moderation commands are the exception.
Files:
src/api/middleware/auth.js
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Applied to files:
vitest.config.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Applied to files:
web/tests/components/dashboard/config-diff-modal.test.tsxweb/tests/components/dashboard/analytics-dashboard.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{ts,tsx} : Web dashboard config editor should use category workspace navigation with reusable SettingsFeatureCard pattern (header + master toggle + Basic/Advanced blocks)
Applied to files:
web/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/layout/dashboard-shell.tsx : Dashboard page titles should sync with route changes using DashboardTitleSync component mounted in dashboard-shell.tsx and canonical title string 'Volvox.Bot - AI Powered Discord Bot'
Applied to files:
web/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-26T00:04:14.693Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to {web/src/lib/page-titles.ts,web/src/**/+page.ts,web/src/**/+page.svelte} : New dashboard routes need title wiring in web/src/lib/page-titles.ts: use createPageMetadata() for SSR and keep DashboardTitleSync aligned for client navigation.
Applied to files:
web/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/pages/dashboard/**/*.{ts,tsx} : Use shared title helpers from web/src/lib/page-titles.ts for setting browser titles in dashboard pages
Applied to files:
web/src/components/dashboard/config-editor.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
web/tests/components/dashboard/analytics-dashboard.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Applied to files:
src/api/middleware/auth.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
tests/api/routes/members.test.jsweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.test.jsweb/tests/api/xp-route.test.ts
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.test.jsweb/tests/api/xp-route.test.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/app/api/**/route.ts : Include guildId in signed WebSocket ticket payload when issuing tickets from dashboard endpoints
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/api/xp-route.test.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tstests/api/routes/guilds.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/src/app/dashboard/**/*.tsx : For dashboard routes, add a matcher entry to `dashboardTitleMatchers` in `web/src/lib/page-titles.ts`: use exact equality for leaf routes (`pathname === '/dashboard/my-route'`) and subtree checks (`pathname.startsWith('/dashboard/my-route/')`); export `metadata` using `createPageMetadata(title)` for SSR entry points
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsxtests/api/routes/guilds.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/src/**/*.{ts,tsx} : Never use `console.*` methods in web dashboard code; use appropriate logging mechanisms for React applications
Applied to files:
web/src/components/layout/guild-directory-context.tsx
🪛 GitHub Check: SonarCloud Code Analysis
tests/api/routes/guilds.test.js
[warning] 167-167: Move function 'createOAuthToken' to the outer scope.
🔇 Additional comments (8)
tests/modules/ai.test.js (2)
52-54: LGTM!The assertion correctly uses
expect.objectContainingto verify thetimestampfield added byaddToHistory. This aligns with the implementation atsrc/modules/ai.js:304whereDate.now()is attached to in-memory messages.
88-96: LGTM!The assertion correctly distinguishes between DB-hydrated messages (lines 89-90, no
timestamp) and the in-memory concurrent message (lines 91-95, withtimestamp). This matches the implementation behavior where:
hydrateHistorymaps DB rows to{ role, content }onlyaddToHistoryincludestimestamp: Date.now()tests/api/routes/guilds.test.js (2)
103-113: Good permission/access mocking foundation for the new auth modelThe
permissionState-drivenpermissions.has()stub and Unknown Member (code: 10007) fetch behavior are well aligned with the route’s access-resolution branches, and the helper keeps test setup explicit.Also applies to: 129-134, 188-191
260-342: Strong coverage for admin vs moderator authorization pathsThese additions clearly exercise admin-only vs moderator-allowed endpoints and mixed-guild access outcomes, which materially reduces regression risk for the permission changes in this PR.
Also applies to: 429-457, 1235-1276
src/api/middleware/auth.js (1)
10-10: Good validation boundary for forwarded actor identity.
req.useris only populated after API-secret auth succeeds, and malformed IDs are ignored instead of leaking into downstream authorization.Also applies to: 53-59
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts (1)
106-117: Failing closed when the Discord actor is unavailable is the right contract.Returning 401 before proxying prevents unauthenticated XP writes from reaching the bot API and keeps the forwarded
x-discord-user-idheader explicit.tests/api/middleware/auth.test.js (1)
139-192: Good edge-case coverage for trusted actor parsing.The valid, blank, and invalid header cases lock in the new
trim()and snowflake-validation behavior well.web/src/components/dashboard/config-editor.tsx (1)
11-16: ConfigEditor is correctly scoped to test-only usage—no action needed.The component is documented as a backward-compatible entry point for older tests and is only imported by
web/tests/components/dashboard/config-editor-autosave.test.tsx. It is not re-exported, not imported by any runtime routes or pages, and not accessible from the main app. No changes required.
Signed-off-by: Bill Chirico <bill@chirico.dev>
…inline comment guidelines - Clarified the project context in the review prompt for the Discord bot Volvox.Bot. - Added detailed instructions for posting inline comments, including formatting requirements and examples for AI fix prompts. - Ensured that the review process emphasizes only reporting issues without providing praise or compliments.
* feat(dashboard): add Discord markdown parser and WYSIWYG editor component - Add discord-markdown.ts utility with parser, wrapSelection, insertAtCursor, wrapLine - Add DiscordMarkdownEditor component with: - Toolbar (Bold, Italic, Underline, Strikethrough, Code, Code Block, Spoiler, Quote, H1-H3, Lists) - Variable inserter dropdown for template variables - Split view: raw editor + live Discord-style preview - Character counter with configurable limit - Keyboard shortcuts (Ctrl+B, Ctrl+I, Ctrl+U) - Follows existing Radix UI + Tailwind + Lucide patterns * test: add DiscordMarkdownEditor and discord-markdown parser tests - 29 tests covering parser, utility functions, and component behavior - Tests: toolbar rendering, variable insertion, character counter, markdown preview, disabled state, XSS prevention * fix: address markdown editor review feedback * fix: restore dashboard config test compatibility * fix: address discord markdown review feedback * fix: resolve markdown parser edge cases * fix(ci): stabilize markdown editor branch checks * fix(sonar): remove regex hotspot parsing * fix(sonar): remove remaining regex markdown hotspots * fix: address remaining review feedback on markdown editor - Use stable index-based keys for badge rows to prevent remount on edit - Add timestamp to initConversationHistory startup hydration entries - Replace placeholder text in Welcome Messages section with proper UI copy - Fix block quote parser to require space after > (matching Discord behavior) - Render plain newlines as line breaks instead of paragraph blocks - Update ReputationSection JSDoc to remove stale 'announcements' reference - Add fallback to reputation.levelThresholds for backward compatibility * fix: address final review feedback on markdown editor - Fix hydration mismatch in preview rendering (isMounted gate) - Remove * as unordered list marker (Discord only supports -) - Remove unused act import from autosave tests - Add TODO for Welcome Messages section placeholder - Unify ReputationSection to use updateDraftConfig consistently - Remove stale reputation.levelThresholds fallback - Add sweepDmLimits test for stale-entry eviction - Strengthen ai.test.js hydration timestamp assertions with exact values # Conflicts: # tests/modules/ai.test.js # web/src/components/dashboard/config-editor.tsx # web/tests/components/dashboard/config-editor-autosave.test.tsx
* feat(dashboard): add visual Discord embed builder component (Closes #371) - Color picker with Discord preset colors + hex input - Title, description fields with character limits (256, 4096) - Thumbnail selector: none / user avatar / server icon / custom URL - Field management: add/remove/reorder with inline toggle - Footer text (2048) + icon URL, full-width image URL - Timestamp toggle, format selector (text/embed/text+embed) - Live Discord-style embed preview with accent color bar - Variable template support with {{var}} badge rendering - Lightweight Discord markdown preview (bold, italic, code) - Character count indicators with color-coded warnings - Aria-labels on switches for accessibility - 29 unit tests covering all features * fix: address review comments on embed-builder * fix: address embed builder review feedback * fix: remove regex hotspots from embed preview * fix: address final embed builder review feedback * test: cover auth route fallback branches * test: cover auth route fallback cache recovery # Conflicts: # tests/modules/ai.test.js # web/src/components/dashboard/config-editor.tsx # web/tests/components/dashboard/config-editor-autosave.test.tsx
| const botGuildIds = guilds.filter((guild) => guild.botPresent).map((guild) => guild.id); | ||
| if (botGuildIds.length === 0) { | ||
| return guilds; | ||
| } | ||
|
|
||
| try { | ||
| const url = new URL(`${botApiBaseUrl}/guilds/access`); | ||
| url.searchParams.set('userId', userId); | ||
| url.searchParams.set('guildIds', botGuildIds.join(',')); | ||
|
|
There was a problem hiding this comment.
applyAccessLevels() sends all bot-present guild IDs in a single guildIds query param, but the bot API /guilds/access endpoint caps lookups at 100 guilds (returns 400 when exceeded). If a user shares >100 bot-present guilds, access augmentation will fail and the dashboard will fall back to OAuth-only permissions.
Consider batching botGuildIds into chunks of 100 (or truncating with a log) and merging the results before applying them to the guild list.
| export function Sidebar({ className, onNavClick }: SidebarProps) { | ||
| const pathname = usePathname(); | ||
| const guildId = useGuildSelection(); | ||
| const { guilds } = useGuildDirectory(); | ||
| const isNavItemActive = (href: string) => | ||
| pathname === href || (href !== '/dashboard' && pathname.startsWith(`${href}/`)); | ||
| const hasActiveSecondaryItem = secondaryNav.some((item) => isNavItemActive(item.href)); | ||
| const activeSecondaryHref = secondaryNav.find((item) => isNavItemActive(item.href))?.href ?? null; | ||
| const activeGuildAccess = guilds.find((guild) => guild.id === guildId)?.access ?? null; | ||
| const visiblePrimaryNav = activeGuildAccess === 'moderator' ? moderatorPrimaryNav : primaryNav; | ||
| const visibleSecondaryNav = activeGuildAccess === 'moderator' ? [] : secondaryNav; | ||
| const hasActiveSecondaryItem = visibleSecondaryNav.some((item) => isNavItemActive(item.href)); |
There was a problem hiding this comment.
Sidebar computes activeGuildAccess from guilds but doesn’t account for the directory still loading/empty. On first render guilds is empty, so activeGuildAccess is null and the full navigation (including admin-only pages) is rendered even for moderator-only users until the fetch completes.
To fully enforce the intended moderator navigation restrictions, gate visiblePrimaryNav/visibleSecondaryNav on the directory loading state (e.g., render a skeleton/placeholder, or default to the most-restrictive nav until access is known).
| const token = await getToken({ req: request }); | ||
| const requesterId = typeof token?.id === 'string' ? token.id : null; | ||
| if (!requesterId) { | ||
| return NextResponse.json({ error: 'Unable to determine Discord user id' }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
This route requires token.id to be present to forward x-discord-user-id, but other routes in the codebase fall back to token.sub when id is unavailable. If an older/partial NextAuth JWT is missing id, this will return 401 even though the request is otherwise authenticated.
Use the same id ?? sub extraction pattern as web/src/app/api/guilds/route.ts (and only 401 when neither value is available).
| 'use client'; | ||
|
|
||
| import { | ||
| AlignLeft, | ||
| ChevronDown, | ||
| ChevronUp, | ||
| Clock, | ||
| Columns, | ||
| Eye, | ||
| ImageIcon, | ||
| Plus, | ||
| Settings, | ||
| Trash2, | ||
| Type, | ||
| } from 'lucide-react'; | ||
| import * as React from 'react'; | ||
| import { generateId } from '@/components/dashboard/config-editor-utils'; | ||
| import { Button } from '@/components/ui/button'; |
There was a problem hiding this comment.
The PR title/description focus on dashboard permission access controls, migration ordering, command loading, and rate-limit handling, but this PR also introduces a large new Embed Builder UI (and associated preview/utility exports). If this is intentional, the PR description should call it out (or consider splitting into a separate PR) so reviewers can assess scope and risk appropriately.
| globals: false, | ||
| environment: 'node', | ||
| include: ['tests/**/*.test.js'], | ||
| testTimeout: 10000, | ||
| testTimeout: 30000, | ||
| hookTimeout: 30000, |
There was a problem hiding this comment.
Global Vitest timeouts were increased from 10s to 30s for all tests. This can hide hangs/slow tests and makes timeouts less actionable.
If only a few suites need longer (many already pass explicit 30_000 timeouts), prefer keeping the global defaults tight and setting per-test/per-suite timeouts where required (or using vi.setConfig({ testTimeout }) locally).
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/modules/ai.js (1)
354-377:⚠️ Potential issue | 🟠 MajorSelect
created_atininitConversationHistoryouter query.
timestampis derived fromrow.created_at, but the outerSELECTdoes not returncreated_at.
Result: startup hydration always falls back toDate.now()and loses persisted chronology.💡 Proposed fix
- `SELECT channel_id, role, content + `SELECT channel_id, role, content, created_at FROM ( SELECT channel_id, role, content, created_at, ROW_NUMBER() OVER (PARTITION BY channel_id ORDER BY created_at DESC) AS rn🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/ai.js` around lines 354 - 377, The outer SQL in initConversationHistory omits created_at so rows lack created_at and timestamp falls back to Date.now(), losing persisted chronology; update the outer SELECT to include created_at (i.e., SELECT channel_id, role, content, created_at FROM (... ) sub) so the row.created_at value is returned and hydratedByChannel entries get accurate timestamp values when constructing the objects in the for loop that sets timestamp from row.created_at..github/workflows/claude-review.yml (1)
49-49: 🧹 Nitpick | 🔵 TrivialSimplify the PR_NUMBER expression.
The
X && Xpattern is redundant. Since GitHub expressions use JavaScript-style short-circuit evaluation, the first operand alone suffices for truthiness.♻️ Simplified expression
- PR_NUMBER: ${{ github.event.pull_request.number && github.event.pull_request.number || github.event.issue.number }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/claude-review.yml at line 49, The PR_NUMBER assignment uses a redundant `X && X` pattern; replace `github.event.pull_request.number && github.event.pull_request.number || github.event.issue.number` with the simplified short-circuit expression `github.event.pull_request.number || github.event.issue.number` so the first truthy value is used; update the PR_NUMBER environment variable assignment accordingly to remove the duplicate check.
🤖 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/claude-review.yml:
- Around line 92-98: The gh API call for creating a PR review uses the wrong
flags: replace the output-format flag `--json comments` and the lowercase `-f`
form for the comments payload with the form-JSON `-F` flags so the array is
parsed as JSON; specifically update the command that posts to `gh api repos/${{
github.repository }}/pulls/${{ env.PR_NUMBER }}/reviews` to use `-F
event="REQUEST_CHANGES"`, `-F body="..."`, and `-F comments='[...]'` instead of
`-f` and `--json comments`.
In `@tests/modules/actions/sendDm.test.js`:
- Around line 179-205: The test uses vi.useFakeTimers() but restores timers
inline; to avoid leaking fake timers if the test errors, add an afterEach hook
that calls vi.useRealTimers() instead of relying on the final line in the test.
Update the describe('sweepDmLimits', ...) block to include afterEach(() =>
vi.useRealTimers()); so any test using vi.useFakeTimers() (the current test that
calls vi.useFakeTimers(), recordDmSend, sweepDmLimits, checkDmRateLimit, etc.)
will always restore real timers even on failure.
In `@tests/modules/ai.test.js`:
- Around line 89-103: The timestamp assertions are too loose—replace
expect.any(Number) with exact millisecond values by mocking or seeding the DB
rows' created_at timestamps used for the messages ('db message', 'db reply',
'concurrent message') and asserting those exact numeric values; locate the
assertions in tests/modules/ai.test.js (the expect.objectContaining blocks for
the three messages) and change the test setup to provide deterministic
created_at fields (or stub Date.now() only if the code uses it) and assert the
exact milliseconds for each message instead of using expect.any(Number).
In `@web/src/app/api/auth/`[...nextauth]/route.ts:
- Around line 44-50: The current catch wraps both getHandler() and
handler(request, context), causing runtime request errors to be treated as
handler construction failures; change this by isolating handler construction:
call getHandler() inside its own try/catch, and in that catch reset
cachedHandler = undefined and return fallbackAuthResponse(request, error); after
a successful getHandler() return await handler(request, context) with no
surrounding catch so request-time errors from handler(request, context)
propagate as normal 5xxs.
In `@web/src/components/ui/discord-markdown-editor.tsx`:
- Around line 207-229: The textarea's maxLength is not enforced: add the
maxLength prop to the textarea element and ensure any programmatic edits in
applyAction (function applyAction) and insertVariable (the other handler
referenced around 232-253 and 359-368) truncate the resulting text to the
provided maxLength before calling onChange; when trimming, also adjust
result.selectionStart/result.selectionEnd/result.cursorPos to be within the
truncated length so setSelectionRange uses valid indices. Ensure you reference
textareaRef, the result object returned by action/value transforms, and the
maxLength prop when implementing the truncation and selection adjustment.
In `@web/src/components/ui/embed-builder.tsx`:
- Around line 545-559: The builder currently emits configs that can exceed
Discord's 6000-char embed cap because update() and updateField() blindly call
onChange; fix both functions to enforce CHAR_LIMITS.total by computing the
current total length (summing value.title, value.description, value.footer, and
every field's name+value) and, before calling onChange, either trim the incoming
patch strings to fit the remaining allowed characters or reject the change —
e.g., for update(), compute allowedRemaining = CHAR_LIMITS.total - currentTotal
+ lengthOfAnyReplacedParts and clamp patch.title/description/footer/fields
accordingly; for updateField(), compute total excluding the target field,
determine available space for that field's name/value, trim the patch for that
field to fit, then call onChange with the adjusted fields so no emitted config
exceeds CHAR_LIMITS.total.
- Around line 168-323: This file duplicates Discord markdown parsing logic
(tokenizeMarkdownSegments, renderMarkdownSegment, renderDiscordMarkdown) which
causes drift; replace those local implementations by importing and using the
shared parser/renderer from web/src/lib/discord-markdown.ts (import the exported
function(s) there, e.g., the parser that returns tokenized segments or React
nodes) and remove the local tokenizeMarkdownSegments, renderMarkdownSegment and
renderDiscordMarkdown implementations; keep or adapt
tokenizeVariableSegments/renderVariablePreview to use the token stream or React
nodes produced by the shared parser so variables render as the same badges
(update calls that used renderDiscordMarkdown to call the shared export
instead).
In `@web/tests/api/auth-route.test.ts`:
- Around line 24-26: The helper createRequest currently constructs NextRequest
without an init object so it defaults to GET; update createRequest to accept an
optional method parameter (or an options/init param) and pass it through to new
NextRequest(url, init) so tests can explicitly set method: "POST", then update
the POST test calls (the ones around the createRequest usages noted) to call
createRequest(pathname, "POST") (or provide { method: "POST" }) so the tests
send real POST requests instead of defaulting to GET.
In `@web/tests/components/dashboard/config-editor-autosave.test.tsx`:
- Around line 203-207: Remove the duplicated waitFor assertion and fix the extra
indentation in the test block: eliminate the redundant
expect(screen.getAllByText('Welcome Messages').length).toBeGreaterThan(0) that
repeats the earlier assertion, and unindent the remaining lines so the expect
calls inside the waitFor callback align properly; locate the waitFor usage and
the expect(...) assertions in the test function in
config-editor-autosave.test.tsx to make these edits.
- Around line 174-179: The test contains a redundant assertion and wrong
indentation: remove the duplicated expect(screen.getAllByText('AI
Chat').length).toBeGreaterThan(0) that repeats an earlier check inside the same
waitFor, and fix the indentation of the remaining lines inside the waitFor block
to use 2 spaces (not 6), keeping the
expect(screen.getByText('Settings')).toBeInTheDocument() and
expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0) assertions
unchanged; locate the assertions within the waitFor callback in
web/tests/components/dashboard/config-editor-autosave.test.tsx to apply these
edits.
---
Outside diff comments:
In @.github/workflows/claude-review.yml:
- Line 49: The PR_NUMBER assignment uses a redundant `X && X` pattern; replace
`github.event.pull_request.number && github.event.pull_request.number ||
github.event.issue.number` with the simplified short-circuit expression
`github.event.pull_request.number || github.event.issue.number` so the first
truthy value is used; update the PR_NUMBER environment variable assignment
accordingly to remove the duplicate check.
In `@src/modules/ai.js`:
- Around line 354-377: The outer SQL in initConversationHistory omits created_at
so rows lack created_at and timestamp falls back to Date.now(), losing persisted
chronology; update the outer SELECT to include created_at (i.e., SELECT
channel_id, role, content, created_at FROM (... ) sub) so the row.created_at
value is returned and hydratedByChannel entries get accurate timestamp values
when constructing the objects in the for loop that sets timestamp from
row.created_at.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d32f881a-41b9-4e9a-b9e7-03ab9bd67658
📒 Files selected for processing (30)
.github/workflows/ci.yml.github/workflows/claude-review.ymlbiome.jsonsrc/modules/actions/buildPayload.jssrc/modules/actions/xpBonus.jssrc/modules/ai.jssrc/modules/levelUpActions.jssrc/modules/triage-parse.jssrc/modules/triage.jssrc/modules/welcomeOnboarding.jstests/api/utils/configValidation.test.jstests/modules/actions/addReaction.test.jstests/modules/actions/announce.test.jstests/modules/actions/nickPrefix.test.jstests/modules/actions/sendDm.test.jstests/modules/actions/webhook.test.jstests/modules/actions/xpBonus.test.jstests/modules/ai.test.jsweb/src/app/api/auth/[...nextauth]/route.tsweb/src/components/dashboard/analytics-dashboard-sections.tsxweb/src/components/dashboard/analytics-dashboard.tsxweb/src/components/dashboard/config-sections/ReputationSection.tsxweb/src/components/dashboard/config-sections/index.tsweb/src/components/ui/discord-markdown-editor.tsxweb/src/components/ui/embed-builder.tsxweb/src/lib/discord-markdown.tsweb/tests/api/auth-route.test.tsweb/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/components/ui/discord-markdown-editor.test.tsxweb/tests/components/ui/embed-builder.test.tsx
💤 Files with no reviewable changes (3)
- web/src/components/dashboard/config-sections/index.ts
- web/src/components/dashboard/analytics-dashboard.tsx
- web/src/components/dashboard/config-sections/ReputationSection.tsx
📜 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: Agent
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (11)
**/*.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use double quotes in JSON files (standard JSON format); this exception applies only to JSON files, not JavaScript/TypeScript
Files:
biome.json
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESM-only syntax:
import/export, neverrequire()/module.exports
Files:
src/modules/actions/buildPayload.jstests/modules/actions/xpBonus.test.jssrc/modules/welcomeOnboarding.jstests/modules/actions/webhook.test.jssrc/modules/triage.jstests/api/utils/configValidation.test.jstests/modules/actions/announce.test.jstests/modules/actions/addReaction.test.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jstests/modules/actions/nickPrefix.test.jssrc/modules/levelUpActions.jssrc/modules/ai.jstests/modules/ai.test.jstests/modules/actions/sendDm.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
src/modules/actions/buildPayload.jstests/modules/actions/xpBonus.test.jssrc/modules/welcomeOnboarding.jstests/modules/actions/webhook.test.jssrc/modules/triage.jstests/api/utils/configValidation.test.jstests/modules/actions/announce.test.jstests/modules/actions/addReaction.test.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jstests/modules/actions/nickPrefix.test.jssrc/modules/levelUpActions.jssrc/modules/ai.jstests/modules/ai.test.jsweb/src/components/dashboard/analytics-dashboard-sections.tsxtests/modules/actions/sendDm.test.jsweb/tests/components/ui/discord-markdown-editor.test.tsxweb/src/app/api/auth/[...nextauth]/route.tsweb/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/auth-route.test.tsweb/src/components/ui/discord-markdown-editor.tsxweb/src/lib/discord-markdown.tsweb/tests/components/ui/embed-builder.test.tsxweb/src/components/ui/embed-builder.tsx
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Never useconsole.*methods; use the Winston logger instead viaimport logger from '../logger.js'(adjust path as needed), then calllogger.info(),logger.warn(),logger.error(), orlogger.debug()
Always usesafeReply(),safeSend(), orsafeEditReply()instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Files:
src/modules/actions/buildPayload.jssrc/modules/welcomeOnboarding.jssrc/modules/triage.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jssrc/modules/levelUpActions.jssrc/modules/ai.js
src/modules/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create feature modules in
src/modules/and add corresponding config sections toconfig.json
Files:
src/modules/actions/buildPayload.jssrc/modules/welcomeOnboarding.jssrc/modules/triage.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jssrc/modules/levelUpActions.jssrc/modules/ai.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
src/modules/actions/buildPayload.jstests/modules/actions/xpBonus.test.jssrc/modules/welcomeOnboarding.jstests/modules/actions/webhook.test.jssrc/modules/triage.jstests/api/utils/configValidation.test.jstests/modules/actions/announce.test.jstests/modules/actions/addReaction.test.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jstests/modules/actions/nickPrefix.test.jssrc/modules/levelUpActions.jssrc/modules/ai.jstests/modules/ai.test.jsweb/src/components/dashboard/analytics-dashboard-sections.tsxtests/modules/actions/sendDm.test.jsweb/tests/components/ui/discord-markdown-editor.test.tsxweb/src/app/api/auth/[...nextauth]/route.tsweb/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/auth-route.test.tsweb/src/components/ui/discord-markdown-editor.tsxweb/src/lib/discord-markdown.tsweb/tests/components/ui/embed-builder.test.tsxweb/src/components/ui/embed-builder.tsx
src/**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{js,ts}: Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Community features should be gated behind config..enabled. Moderation commands are the exception.
Files:
src/modules/actions/buildPayload.jssrc/modules/welcomeOnboarding.jssrc/modules/triage.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jssrc/modules/levelUpActions.jssrc/modules/ai.js
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
src/modules/actions/buildPayload.jstests/modules/actions/xpBonus.test.jssrc/modules/welcomeOnboarding.jstests/modules/actions/webhook.test.jssrc/modules/triage.jstests/api/utils/configValidation.test.jstests/modules/actions/announce.test.jstests/modules/actions/addReaction.test.jssrc/modules/actions/xpBonus.jssrc/modules/triage-parse.jstests/modules/actions/nickPrefix.test.jssrc/modules/levelUpActions.jssrc/modules/ai.jstests/modules/ai.test.jstests/modules/actions/sendDm.test.jsweb/src/app/api/auth/[...nextauth]/route.tsweb/tests/api/auth-route.test.tsweb/src/lib/discord-markdown.ts
tests/**/*.test.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.test.js: Write bot tests using Vitest 4 with thenodeenvironment, matching thesrc/structure in thetests/directory
Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Files:
tests/modules/actions/xpBonus.test.jstests/modules/actions/webhook.test.jstests/api/utils/configValidation.test.jstests/modules/actions/announce.test.jstests/modules/actions/addReaction.test.jstests/modules/actions/nickPrefix.test.jstests/modules/ai.test.jstests/modules/actions/sendDm.test.js
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/components/dashboard/analytics-dashboard-sections.tsxweb/src/app/api/auth/[...nextauth]/route.tsweb/src/components/ui/discord-markdown-editor.tsxweb/src/lib/discord-markdown.tsweb/src/components/ui/embed-builder.tsx
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/components/ui/discord-markdown-editor.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/auth-route.test.tsweb/tests/components/ui/embed-builder.test.tsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to **/*.js : Use ESM-only syntax: `import`/`export`, never `require()`/`module.exports`
Applied to files:
biome.json
📚 Learning: 2026-03-26T00:04:14.693Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : ESM only - do not use CommonJS modules
Applied to files:
biome.json
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/reactionrole.js : Enforce invoker role hierarchy check in /reactionrole add command to prevent non-owner users from configuring roles at or above their highest role
Applied to files:
src/modules/welcomeOnboarding.js
📚 Learning: 2026-03-26T00:04:14.693Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to src/**/*.{js,ts} : Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Applied to files:
tests/modules/actions/announce.test.jssrc/modules/levelUpActions.jsweb/src/components/ui/discord-markdown-editor.tsxweb/src/lib/discord-markdown.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/**/*.js : Always use `safeReply()`, `safeSend()`, or `safeEditReply()` instead of raw Discord.js methods for safe Discord messaging that handles errors gracefully
Applied to files:
tests/modules/actions/announce.test.js
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Applied to files:
web/tests/components/ui/discord-markdown-editor.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/tests/**/*.test.{ts,tsx} : Write web dashboard tests using Vitest 4 with the `jsdom` environment and React Testing Library, matching the `web/src/` structure
Applied to files:
web/tests/components/ui/discord-markdown-editor.test.tsxweb/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/auth-route.test.tsweb/tests/components/ui/embed-builder.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Maintain test coverage thresholds: statements 85%, branches 82%, functions 85%, lines 85%; never lower thresholds—add tests to cover new code instead
Applied to files:
web/tests/components/ui/discord-markdown-editor.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to src/api/routes/**/*.js : Create API route handlers in `src/api/routes/` and mount them in `src/api/server.js`; add auth middleware if the endpoint requires authentication
Applied to files:
web/src/app/api/auth/[...nextauth]/route.tsweb/tests/api/auth-route.test.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/layout/dashboard-shell.tsx : Dashboard page titles should sync with route changes using DashboardTitleSync component mounted in dashboard-shell.tsx and canonical title string 'Volvox.Bot - AI Powered Discord Bot'
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{ts,tsx} : Web dashboard config editor should use category workspace navigation with reusable SettingsFeatureCard pattern (header + master toggle + Basic/Advanced blocks)
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to web/src/app/dashboard/**/*.tsx : For dashboard routes, add a matcher entry to `dashboardTitleMatchers` in `web/src/lib/page-titles.ts`: use exact equality for leaf routes (`pathname === '/dashboard/my-route'`) and subtree checks (`pathname.startsWith('/dashboard/my-route/')`); export `metadata` using `createPageMetadata(title)` for SSR entry points
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/auth-route.test.ts
📚 Learning: 2026-03-26T00:04:14.693Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to {web/src/lib/page-titles.ts,web/src/**/+page.ts,web/src/**/+page.svelte} : New dashboard routes need title wiring in web/src/lib/page-titles.ts: use createPageMetadata() for SSR and keep DashboardTitleSync aligned for client navigation.
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Config editor save contract: maintain global save/discard, diff-modal confirmation, per-section PATCH batching, and partial-failure behavior
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsx
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to tests/**/*.test.js : Write bot tests using Vitest 4 with the `node` environment, matching the `src/` structure in the `tests/` directory
Applied to files:
web/tests/components/dashboard/config-editor-autosave.test.tsxweb/tests/api/auth-route.test.tsweb/tests/components/ui/embed-builder.test.tsx
🔇 Additional comments (30)
biome.json (1)
2-2: Schema version update looks good.Scoped
$schemabump only, with valid JSON formatting and no adverse config changes detected.src/modules/welcomeOnboarding.js (1)
234-234: Optional-chaining guard is a solid simplification.This preserves behavior while making the role eligibility check more concise and safe for cache/fetch misses.
tests/api/utils/configValidation.test.js (2)
414-417: Formatting-only refactor looks good.This keeps the same assertion behavior while improving readability.
425-427: Formatting-only refactor looks good.No behavior change; assertion intent remains clear.
web/src/components/dashboard/analytics-dashboard-sections.tsx (1)
120-120: Unused-prop aliasing is clean and safe.Keeping
compareModein the public prop contract while aliasing to_compareModeavoids lint noise without behavior changes..github/workflows/ci.yml (1)
140-140: Good cache-key correction for monorepo lockfile.Using
hashFiles('pnpm-lock.yaml')is the right fix here; it properly invalidates Playwright cache when workspace dependencies change.src/modules/triage-parse.js (1)
77-77: LGTM: Safe optional chaining refactor.The use of optional chaining (
parsed?.classification) safely handles bothnull/undefinedparsedvalues and missingclassificationproperties in a single, concise expression. This is cleaner than the previous two-step check and aligns with modern JavaScript best practices.src/modules/triage.js (2)
328-328: LGTM: Defensive optional chaining.The optional chaining (
parsed?.responses?.length) safely guards againstnull/undefinedparsedobjects and missingresponsesarrays before checking length. This is more defensive than the previous approach and prevents potential runtime errors.
541-549: LGTM: Clean formatting with trailing commas.Reformatting the
sendResponsesinvocation across multiple lines with trailing commas improves readability and produces cleaner git diffs when arguments are added or removed. This follows the coding guideline for multi-line function calls.src/modules/actions/buildPayload.js (1)
32-33: LGTM! Formatting improvement enforces line-width guideline.The multi-line format keeps the line under the 100-character maximum and improves consistency with the
descriptionandfooterblocks above. As per coding guidelines, the 100-character line width is maintained.tests/modules/actions/addReaction.test.js (1)
9-10: LGTM!The import reordering correctly places the
handleAddReactionimport after the logger mock setup. This follows the standard Vitest ESM mocking pattern wherevi.mock()declarations must precede actual imports for proper hoisting.tests/modules/actions/announce.test.js (1)
17-20: LGTM!Import ordering correctly places module imports after all
vi.mock()declarations, ensuring proper mock hoisting for ESM modules.tests/modules/actions/webhook.test.js (2)
9-10: LGTM!Import reordering follows the correct Vitest ESM mock hoisting pattern.
92-92: LGTM!The single-line reformatting of assertion calls improves readability while staying within the 100-character line limit.
src/modules/actions/xpBonus.js (1)
68-72: LGTM!The query formatting change is cosmetic. The parameterized SQL properly prevents injection, and the
try/finallyblock ensures the recursion guard is cleared even on errors.tests/modules/actions/xpBonus.test.js (1)
14-15: LGTM!Import reordering follows the correct Vitest ESM mock hoisting pattern, placing module imports after
vi.mock()declarations.src/modules/levelUpActions.js (1)
16-18: LGTM!Import reordering improves consistency by grouping action imports alphabetically. The action registry registrations remain correctly mapped.
tests/modules/actions/nickPrefix.test.js (1)
9-12: LGTM!Import reordering follows proper Vitest mock hoisting patterns, and the single-line parameter formatting improves readability while staying within line length limits.
tests/modules/actions/sendDm.test.js (1)
14-22: LGTM!Import reordering correctly places logger imports before action module imports, following Vitest's ESM mock hoisting pattern.
src/modules/ai.js (1)
224-235: Good hydration update for DB-backed timestamps.Selecting
created_atand mapping it totimestampinhydrateHistoryis a solid fix for preserving message chronology during lazy loads.tests/modules/ai.test.js (1)
124-126: Nice query assertion update.Including
created_atin the SQL expectation adds a useful guard for hydration query regressions..github/workflows/claude-review.yml (3)
17-30: LGTM!The job condition correctly triggers on
@claudementions across all three event types, with proper null-safety using|| ''fallback.
56-164: Prompt structure is comprehensive and well-organized.The categorization of rules (HARD RULES, DISCORD-SPECIFIC, DOCS, OUTPUT), the inline comment format with AI Fix Prompts, and the concrete examples provide clear guidance for the review agent.
41-44: No issues found.actions/checkout@v6is the current stable version.web/tests/components/dashboard/config-editor-autosave.test.tsx (5)
13-18: LGTM!The module-level
mockPathnamepattern with thenext/navigationmock is a clean approach for testing route-dependent behavior. The mock properly stubsuseRoutermethods.
69-84: LGTM!The conditional rendering based on
openand the exposedonConfirmhandler provide adequate test coverage for the confirmation flow.
232-294: LGTM!The test correctly verifies the PATCH request body format
{ path: 'ai.systemPrompt', value: 'Updated prompt' }, which matches the API contract invalidateConfigPatchBodyand the component'supdateSystemPromptimplementation. The three-response mock chain properly simulates the initial load, PATCH response, and subsequent config refresh.
296-323: LGTM!Clean test for the discard functionality — properly sets up the saved state, simulates user edits, triggers discard, and verifies restoration to the last saved value.
326-408: LGTM!The unit tests for normalization and config update utilities are well-structured with good coverage of edge cases (clamping, empty strings, nested updates). The immutability checks confirming source objects are not mutated are valuable for catching accidental mutations.
web/src/app/api/auth/[...nextauth]/route.ts (1)
14-16: The current implementation is correct for next-auth v4.24.x.The
/sessionendpoint should return{}(empty object) when there is no active session, which matches the standard next-auth v4.24.x behavior. Clients built to work with next-auth expect this response format. Changing it tonullwould break compatibility with the next-auth contract.> Likely an incorrect or invalid review comment.
| ```bash | ||
| gh api repos/${{ github.repository }}/pulls/${{ env.PR_NUMBER }}/reviews \ | ||
| -X POST \ | ||
| -f event="REQUEST_CHANGES" \ | ||
| -f body="Code review complete. See inline comments for issues and AI fix prompts." \ | ||
| --json comments -f comments='[...array of comment objects...]' | ||
| ``` |
There was a problem hiding this comment.
Incorrect gh api syntax: --json is for output formatting, not input.
The --json comments flag tells gh which fields to include in the JSON output. To pass a JSON array as input, use -F (capital F) which parses the value as JSON. The current syntax will fail or behave unexpectedly, causing Claude to generate broken commands.
🐛 Proposed fix
```bash
gh api repos/${{ github.repository }}/pulls/${{ env.PR_NUMBER }}/reviews \
-X POST \
- -f event="REQUEST_CHANGES" \
- -f body="Code review complete. See inline comments for issues and AI fix prompts." \
- --json comments -f comments='[...array of comment objects...]'
+ -F event="REQUEST_CHANGES" \
+ -F body="Code review complete. See inline comments for issues and AI fix prompts." \
+ -F comments='[...array of comment objects...]'
```Using -F (instead of -f) enables JSON type inference, so arrays like [{"path":"...","line":10,"body":"..."}] are correctly parsed as JSON rather than strings.
📝 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.
| ```bash | |
| gh api repos/${{ github.repository }}/pulls/${{ env.PR_NUMBER }}/reviews \ | |
| -X POST \ | |
| -f event="REQUEST_CHANGES" \ | |
| -f body="Code review complete. See inline comments for issues and AI fix prompts." \ | |
| --json comments -f comments='[...array of comment objects...]' | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude-review.yml around lines 92 - 98, The gh API call
for creating a PR review uses the wrong flags: replace the output-format flag
`--json comments` and the lowercase `-f` form for the comments payload with the
form-JSON `-F` flags so the array is parsed as JSON; specifically update the
command that posts to `gh api repos/${{ github.repository }}/pulls/${{
env.PR_NUMBER }}/reviews` to use `-F event="REQUEST_CHANGES"`, `-F body="..."`,
and `-F comments='[...]'` instead of `-f` and `--json comments`.
| describe('sweepDmLimits', () => { | ||
| beforeEach(() => { | ||
| resetDmLimits(); | ||
| }); | ||
|
|
||
| it('should evict stale entries and keep recent ones', () => { | ||
| vi.useFakeTimers(); | ||
|
|
||
| // Record an entry that will become stale | ||
| recordDmSend('g1', 'u-stale'); | ||
|
|
||
| // Advance past the rate window (60s) | ||
| vi.advanceTimersByTime(60_001); | ||
|
|
||
| // Record a recent entry | ||
| recordDmSend('g1', 'u-recent'); | ||
|
|
||
| // Sweep should evict the stale entry | ||
| sweepDmLimits(); | ||
|
|
||
| // Stale entry evicted — should be allowed again | ||
| expect(checkDmRateLimit('g1', 'u-stale')).toBe(true); | ||
| // Recent entry still rate-limited | ||
| expect(checkDmRateLimit('g1', 'u-recent')).toBe(false); | ||
|
|
||
| vi.useRealTimers(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add afterEach to restore real timers for test isolation.
If the test fails before reaching vi.useRealTimers() on line 204, fake timers will leak into subsequent tests, potentially causing flaky failures. Move timer restoration to an afterEach hook for robustness.
♻️ Proposed fix
describe('sweepDmLimits', () => {
beforeEach(() => {
resetDmLimits();
});
+
+ afterEach(() => {
+ vi.useRealTimers();
+ });
it('should evict stale entries and keep recent ones', () => {
vi.useFakeTimers();
// Record an entry that will become stale
recordDmSend('g1', 'u-stale');
// Advance past the rate window (60s)
vi.advanceTimersByTime(60_001);
// Record a recent entry
recordDmSend('g1', 'u-recent');
// Sweep should evict the stale entry
sweepDmLimits();
// Stale entry evicted — should be allowed again
expect(checkDmRateLimit('g1', 'u-stale')).toBe(true);
// Recent entry still rate-limited
expect(checkDmRateLimit('g1', 'u-recent')).toBe(false);
-
- vi.useRealTimers();
});
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/modules/actions/sendDm.test.js` around lines 179 - 205, The test uses
vi.useFakeTimers() but restores timers inline; to avoid leaking fake timers if
the test errors, add an afterEach hook that calls vi.useRealTimers() instead of
relying on the final line in the test. Update the describe('sweepDmLimits', ...)
block to include afterEach(() => vi.useRealTimers()); so any test using
vi.useFakeTimers() (the current test that calls vi.useFakeTimers(),
recordDmSend, sweepDmLimits, checkDmRateLimit, etc.) will always restore real
timers even on failure.
| expect.objectContaining({ | ||
| role: 'user', | ||
| content: 'db message', | ||
| timestamp: expect.any(Number), | ||
| }), | ||
| expect.objectContaining({ | ||
| role: 'assistant', | ||
| content: 'db reply', | ||
| timestamp: expect.any(Number), | ||
| }), | ||
| expect.objectContaining({ | ||
| role: 'user', | ||
| content: 'concurrent message', | ||
| timestamp: expect.any(Number), | ||
| }), |
There was a problem hiding this comment.
Timestamp assertions are too permissive for this behavior change.
expect.any(Number) passes even when timestamps come from Date.now() fallback instead of DB created_at.
Please make this deterministic by mocking created_at values and asserting exact milliseconds.
💡 Suggested test hardening
resolveHydration({
rows: [
- { role: 'assistant', content: 'db reply' },
- { role: 'user', content: 'db message' },
+ {
+ role: 'assistant',
+ content: 'db reply',
+ created_at: '2026-03-01T10:01:00.000Z',
+ },
+ {
+ role: 'user',
+ content: 'db message',
+ created_at: '2026-03-01T10:00:00.000Z',
+ },
],
});
@@
expect(historyRef).toEqual([
expect.objectContaining({
role: 'user',
content: 'db message',
- timestamp: expect.any(Number),
+ timestamp: new Date('2026-03-01T10:00:00.000Z').getTime(),
}),
expect.objectContaining({
role: 'assistant',
content: 'db reply',
- timestamp: expect.any(Number),
+ timestamp: new Date('2026-03-01T10:01:00.000Z').getTime(),
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/modules/ai.test.js` around lines 89 - 103, The timestamp assertions are
too loose—replace expect.any(Number) with exact millisecond values by mocking or
seeding the DB rows' created_at timestamps used for the messages ('db message',
'db reply', 'concurrent message') and asserting those exact numeric values;
locate the assertions in tests/modules/ai.test.js (the expect.objectContaining
blocks for the three messages) and change the test setup to provide
deterministic created_at fields (or stub Date.now() only if the code uses it)
and assert the exact milliseconds for each message instead of using
expect.any(Number).
| function tokenizeVariableSegments(text: string): string[] { | ||
| const segments: string[] = []; | ||
| let cursor = 0; | ||
|
|
||
| while (cursor < text.length) { | ||
| const variableStart = text.indexOf('{{', cursor); | ||
| if (variableStart === -1) { | ||
| segments.push(text.slice(cursor)); | ||
| break; | ||
| } | ||
|
|
||
| if (variableStart > cursor) { | ||
| segments.push(text.slice(cursor, variableStart)); | ||
| } | ||
|
|
||
| const variableEnd = text.indexOf('}}', variableStart + 2); | ||
| if (variableEnd === -1) { | ||
| segments.push(text.slice(variableStart)); | ||
| break; | ||
| } | ||
|
|
||
| segments.push(text.slice(variableStart, variableEnd + 2)); | ||
| cursor = variableEnd + 2; | ||
| } | ||
|
|
||
| return segments; | ||
| } | ||
|
|
||
| function tokenizeMarkdownSegments(line: string): string[] { | ||
| const segments: string[] = []; | ||
| let cursor = 0; | ||
|
|
||
| while (cursor < line.length) { | ||
| if (line.startsWith('{{', cursor)) { | ||
| const variableEnd = line.indexOf('}}', cursor + 2); | ||
| if (variableEnd !== -1) { | ||
| segments.push(line.slice(cursor, variableEnd + 2)); | ||
| cursor = variableEnd + 2; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (line.startsWith('**', cursor)) { | ||
| const boldEnd = line.indexOf('**', cursor + 2); | ||
| if (boldEnd !== -1) { | ||
| segments.push(line.slice(cursor, boldEnd + 2)); | ||
| cursor = boldEnd + 2; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (line.startsWith('*', cursor)) { | ||
| const italicEnd = line.indexOf('*', cursor + 1); | ||
| if (italicEnd !== -1) { | ||
| segments.push(line.slice(cursor, italicEnd + 1)); | ||
| cursor = italicEnd + 1; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (line.startsWith('`', cursor)) { | ||
| const codeEnd = line.indexOf('`', cursor + 1); | ||
| if (codeEnd !== -1) { | ||
| segments.push(line.slice(cursor, codeEnd + 1)); | ||
| cursor = codeEnd + 1; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| const nextTokenStarts = [ | ||
| line.indexOf('{{', cursor + 1), | ||
| line.indexOf('**', cursor + 1), | ||
| line.indexOf('*', cursor + 1), | ||
| line.indexOf('`', cursor + 1), | ||
| ].filter((index) => index !== -1); | ||
| const nextTokenStart = nextTokenStarts.length > 0 ? Math.min(...nextTokenStarts) : line.length; | ||
|
|
||
| segments.push(line.slice(cursor, nextTokenStart)); | ||
| cursor = nextTokenStart; | ||
| } | ||
|
|
||
| return segments; | ||
| } | ||
|
|
||
| /** Render template variables as styled badges in a string for preview */ | ||
| function renderVariablePreview(text: string): React.ReactNode[] { | ||
| if (!text) return []; | ||
|
|
||
| return tokenizeVariableSegments(text).map((part, index) => { | ||
| if (part.startsWith('{{') && part.endsWith('}}')) { | ||
| const varName = part.slice(2, -2); | ||
| return ( | ||
| <span | ||
| key={`${varName}-${index}`} | ||
| className="inline-flex items-center rounded bg-primary/20 px-1.5 py-0.5 text-xs font-medium text-primary" | ||
| > | ||
| {varName} | ||
| </span> | ||
| ); | ||
| } | ||
| return <span key={`text-${index}`}>{part}</span>; | ||
| }); | ||
| } | ||
|
|
||
| /** Very lightweight Discord markdown → HTML (bold, italic, inline code) */ | ||
| function renderMarkdownSegment(segment: string, lineIndex: number, segmentIndex: number) { | ||
| if (segment.startsWith('{{') && segment.endsWith('}}')) { | ||
| const varName = segment.slice(2, -2); | ||
| return ( | ||
| <span | ||
| key={`var-${lineIndex}-${segmentIndex}`} | ||
| className="inline-flex items-center rounded bg-primary/20 px-1.5 py-0.5 text-xs font-medium text-primary" | ||
| > | ||
| {varName} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| if (segment.startsWith('**') && segment.endsWith('**')) { | ||
| return <strong key={`b-${lineIndex}-${segmentIndex}`}>{segment.slice(2, -2)}</strong>; | ||
| } | ||
|
|
||
| if (segment.startsWith('*') && segment.endsWith('*')) { | ||
| return <em key={`i-${lineIndex}-${segmentIndex}`}>{segment.slice(1, -1)}</em>; | ||
| } | ||
|
|
||
| if (segment.startsWith('`') && segment.endsWith('`')) { | ||
| return ( | ||
| <code | ||
| key={`c-${lineIndex}-${segmentIndex}`} | ||
| className="rounded bg-muted px-1 py-0.5 text-xs font-mono" | ||
| > | ||
| {segment.slice(1, -1)} | ||
| </code> | ||
| ); | ||
| } | ||
|
|
||
| return <span key={`t-${lineIndex}-${segmentIndex}`}>{segment}</span>; | ||
| } | ||
|
|
||
| function renderDiscordMarkdown(text: string): React.ReactNode[] { | ||
| const lines = text.split('\n'); | ||
| const result: React.ReactNode[] = []; | ||
|
|
||
| for (let li = 0; li < lines.length; li++) { | ||
| if (li > 0) result.push(<br key={`br-${li}`} />); | ||
| const line = lines[li]; | ||
| const segments = tokenizeMarkdownSegments(line); | ||
| for (let si = 0; si < segments.length; si++) { | ||
| const seg = segments[si]; | ||
| if (!seg) continue; | ||
| result.push(renderMarkdownSegment(seg, li, si)); | ||
| } | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reuse the shared Discord markdown parser instead of maintaining a second one here.
This file now has its own markdown tokenizer/renderer, but it already supports a smaller syntax subset than web/src/lib/discord-markdown.ts. That means the embed preview will drift from the rest of the dashboard as soon as one side changes. Please extract a shared preview-rendering helper and feed both UIs through the same parser.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/embed-builder.tsx` around lines 168 - 323, This file
duplicates Discord markdown parsing logic (tokenizeMarkdownSegments,
renderMarkdownSegment, renderDiscordMarkdown) which causes drift; replace those
local implementations by importing and using the shared parser/renderer from
web/src/lib/discord-markdown.ts (import the exported function(s) there, e.g.,
the parser that returns tokenized segments or React nodes) and remove the local
tokenizeMarkdownSegments, renderMarkdownSegment and renderDiscordMarkdown
implementations; keep or adapt tokenizeVariableSegments/renderVariablePreview to
use the token stream or React nodes produced by the shared parser so variables
render as the same badges (update calls that used renderDiscordMarkdown to call
the shared export instead).
| function createRequest(pathname: string) { | ||
| return new NextRequest(new URL(`http://localhost:3000${pathname}`)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Next.js 16, what method does new NextRequest(url)use when nomethod is provided in the init object?
💡 Result:
In Next.js 16, new NextRequest(url) uses the standard Web API Request constructor behavior, defaulting to the "GET" method when no method is provided in the init object.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/RequestInit
- 2: https://nextjs.org/docs/app/api-reference/functions/next-request
- 3: https://nextjs.org/docs/13/pages/api-reference/functions/next-server
- 4: Bug:
expected non-null body sourcewhen passing aRequestobject to fetch AND receiving an error response (Node >= 24.14.0) vercel/next.js#90826
🏁 Script executed:
# First, check the test file structure and the createRequest function
head -30 web/tests/api/auth-route.test.ts | cat -nRepository: VolvoxLLC/volvox-bot
Length of output: 985
🏁 Script executed:
# Get the specific lines mentioned in the review (24-26, 149-153, 196-203)
sed -n '24,26p; 149,153p; 196,203p' web/tests/api/auth-route.test.ts | cat -nRepository: VolvoxLLC/volvox-bot
Length of output: 848
🏁 Script executed:
# Check the route file to see if POST export validates HTTP method
find . -path '*/app/api/auth/*/route.ts' -type fRepository: VolvoxLLC/volvox-bot
Length of output: 109
🏁 Script executed:
# Check the POST export in the auth route to see if it validates method
cat -n web/src/app/api/auth/\[...nextauth\]/route.tsRepository: VolvoxLLC/volvox-bot
Length of output: 2434
🏁 Script executed:
# Also verify the test file structure around lines 149-153 and 196-203 with more context
sed -n '145,210p' web/tests/api/auth-route.test.ts | cat -nRepository: VolvoxLLC/volvox-bot
Length of output: 2997
Specify HTTP method explicitly in POST test cases.
The createRequest() helper defaults to GET because NextRequest defaults to GET when no init object is provided. While the route handler doesn't currently validate the method, POST tests should pass actual POST requests to avoid silently passing if auth logic becomes method-sensitive.
Suggested fix
-function createRequest(pathname: string) {
- return new NextRequest(new URL(`http://localhost:3000${pathname}`));
+function createRequest(pathname: string, method: string = 'GET') {
+ return new NextRequest(new URL(`http://localhost:3000${pathname}`), { method });
}
@@
- const response = await POST(createRequest('/api/auth/csrf'), createContext(['csrf']));
+ const response = await POST(
+ createRequest('/api/auth/csrf', 'POST'),
+ createContext(['csrf']),
+ );
@@
const response = await POST(
- createRequest('/api/auth/callback/discord'),
+ createRequest('/api/auth/callback/discord', 'POST'),
createContext(['callback', 'discord']),
);Also applies to: 149-153, 196-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/api/auth-route.test.ts` around lines 24 - 26, The helper
createRequest currently constructs NextRequest without an init object so it
defaults to GET; update createRequest to accept an optional method parameter (or
an options/init param) and pass it through to new NextRequest(url, init) so
tests can explicitly set method: "POST", then update the POST test calls (the
ones around the createRequest usages noted) to call createRequest(pathname,
"POST") (or provide { method: "POST" }) so the tests send real POST requests
instead of defaulting to GET.
| expect(screen.getAllByText('AI Chat').length).toBeGreaterThan(0); | ||
| expect(screen.getByText('Settings')).toBeInTheDocument(); | ||
| expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0); | ||
| }, | ||
| 30_000, | ||
| ); |
There was a problem hiding this comment.
Fix indentation and remove redundant assertion.
Lines 175-176 have inconsistent indentation (6 spaces instead of 4), violating the 2-space indentation guideline. Additionally, line 174 duplicates the assertion from line 169 inside waitFor — once waitFor passes, the element is guaranteed to exist.
🛠️ Proposed fix
await waitFor(
() => {
expect(screen.getAllByText('AI Chat').length).toBeGreaterThan(0);
},
{ timeout: 3000 },
);
- expect(screen.getAllByText('AI Chat').length).toBeGreaterThan(0);
- expect(screen.getByText('Settings')).toBeInTheDocument();
- expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0);
+ expect(screen.getByText('Settings')).toBeInTheDocument();
+ expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0);
},
30_000,
);As per coding guidelines: "Use 2-space indentation (spaces, not tabs)".
📝 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.
| expect(screen.getAllByText('AI Chat').length).toBeGreaterThan(0); | |
| expect(screen.getByText('Settings')).toBeInTheDocument(); | |
| expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0); | |
| }, | |
| 30_000, | |
| ); | |
| expect(screen.getByText('Settings')).toBeInTheDocument(); | |
| expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0); | |
| }, | |
| 30_000, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/components/dashboard/config-editor-autosave.test.tsx` around lines
174 - 179, The test contains a redundant assertion and wrong indentation: remove
the duplicated expect(screen.getAllByText('AI Chat').length).toBeGreaterThan(0)
that repeats an earlier check inside the same waitFor, and fix the indentation
of the remaining lines inside the waitFor block to use 2 spaces (not 6), keeping
the expect(screen.getByText('Settings')).toBeInTheDocument() and
expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0) assertions
unchanged; locate the assertions within the waitFor callback in
web/tests/components/dashboard/config-editor-autosave.test.tsx to apply these
edits.
| expect(screen.getAllByText('Welcome Messages').length).toBeGreaterThan(0); | ||
| expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0); | ||
| }, | ||
| 30_000, | ||
| ); |
There was a problem hiding this comment.
Same indentation and redundancy issues as above.
Lines 203-204 have extra indentation, and line 203 duplicates the waitFor assertion from line 198.
🛠️ Proposed fix
await waitFor(
() => {
expect(screen.getAllByText('Welcome Messages').length).toBeGreaterThan(0);
},
{ timeout: 3000 },
);
- expect(screen.getAllByText('Welcome Messages').length).toBeGreaterThan(0);
- expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0);
+ expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0);
},
30_000,
);📝 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.
| expect(screen.getAllByText('Welcome Messages').length).toBeGreaterThan(0); | |
| expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0); | |
| }, | |
| 30_000, | |
| ); | |
| expect(screen.getAllByText('Save Changes').length).toBeGreaterThan(0); | |
| }, | |
| 30_000, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/components/dashboard/config-editor-autosave.test.tsx` around lines
203 - 207, Remove the duplicated waitFor assertion and fix the extra indentation
in the test block: eliminate the redundant expect(screen.getAllByText('Welcome
Messages').length).toBeGreaterThan(0) that repeats the earlier assertion, and
unindent the remaining lines so the expect calls inside the waitFor callback
align properly; locate the waitFor usage and the expect(...) assertions in the
test function in config-editor-autosave.test.tsx to make these edits.
# Conflicts: # tests/modules/ai.test.js # web/src/components/dashboard/config-editor.tsx # web/tests/components/dashboard/config-editor-autosave.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
web/tests/api/xp-route.test.ts (1)
59-59:⚠️ Potential issue | 🟡 MinorUse a valid Discord snowflake in the token mock to exercise real behavior.
'moderator-1'does not match the Discord snowflake pattern (^\d{17,20}$) that the bot API middleware validates. This test passes locally but doesn't verify the actual cross-service contract—the bot API would reject or ignore this value.Suggested fix
- mockGetToken.mockResolvedValue({ id: 'moderator-1' }); + mockGetToken.mockResolvedValue({ id: '123456789012345678' });And update the assertion at lines 186-187:
expect(mockProxyToBotApi.mock.calls[0][4].headers).toMatchObject({ 'Content-Type': 'application/json', - 'x-discord-user-id': 'moderator-1', + 'x-discord-user-id': '123456789012345678', });Also applies to: 180-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/tests/api/xp-route.test.ts` at line 59, Replace the invalid token mock value 'moderator-1' used in mockGetToken.mockResolvedValue with a valid Discord snowflake string (e.g., a 17–20 digit numeric string like '12345678901234567') so the bot API middleware's regex validation is exercised; update any related assertions that expect behaviors tied to the mock id (the assertions around the test that reference the mocked moderator id—previously at the assertion block for lines ~186-187 and the similar block at ~180-188) to use the new numeric snowflake value (referencing mockGetToken and the test's expectation/assert calls) so the test verifies the real cross-service contract.web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts (1)
106-111:⚠️ Potential issue | 🟠 MajorRemove the
token.subfallback; it is not a Discord snowflake and cannot be validated by the bot API.The fallback to
token.subis intentionally tested (line 190 ofweb/tests/api/xp-route.test.ts), but the test mocks the bot API and does not validate production behavior. In reality,token.sub(a NextAuth internal identifier like'nextauth-sub-only') does not match theDISCORD_SNOWFLAKE_PATTERN(/^\d{17,20}$/) enforced by the bot API. When sent as thex-discord-user-idheader, validation fails silently at the bot API level:req.useris not set, and the XP adjustment request proceeds without proper user context.Additionally, refactor the nested ternary for clarity:
Suggested fix: fail closed when token.id is unavailable
const token = await getToken({ req: request }); - const requesterId = - typeof token?.id === 'string' ? token.id : typeof token?.sub === 'string' ? token.sub : null; + const requesterId = typeof token?.id === 'string' ? token.id : null; if (!requesterId) { return NextResponse.json({ error: 'Unable to determine Discord user id' }, { status: 401 }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/guilds/`[guildId]/members/[userId]/xp/route.ts around lines 106 - 111, The code currently falls back to token.sub when determining requesterId; remove that fallback and require token.id only (use getToken result), so if token?.id is not a string the handler returns the 401 via NextResponse.json immediately; replace the nested ternary that sets requesterId with a clear if-check that assigns requesterId = token?.id when it's a string and otherwise returns the 401 error response (update the block around getToken, requesterId and the existing NextResponse.json call accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/ui/embed-builder.tsx`:
- Around line 566-577: The conditional expressions inside baseConfig's fields
mapping use negated/indirect checks (patch.name !== undefined and patch.value
!== undefined); replace them with positive property-existence checks (for
example use 'name' in patch and 'value' in patch) so the fields.map callback
explicitly applies { name: '' } when the patch contains name and { value: '' }
when it contains value; update the mapping that constructs baseConfig (the
fields.map block referencing patch, index, and value) to use those positive
conditions for clarity.
In `@web/tests/api/guilds.test.ts`:
- Around line 149-277: Tests ("augments guilds with bot-evaluated access levels
when bot api is configured", "ignores unknown access values from the bot api",
"batches guild access lookups to avoid exceeding the 100-guild API cap") use
double quotes; update string literals in these test cases (and optionally the
whole file) to use single quotes to match project style, e.g. change
process.env.BOT_API_SECRET, URLs passed to mockGetBotApiBaseUrl, the mocked
fetch responses, object property string values, and any string constants passed
to GET(createMockRequest()) or used in expect calls; run the repo formatter or
ESLint auto-fix (prettier/eslint --fix) after editing to ensure consistent
single-quote usage across the file.
In `@web/tests/api/xp-route.test.ts`:
- Around line 190-200: The test asserts that when mockGetToken returns only {
sub } the route should succeed and proxy to bot API, but per the route fix it
should reject such requests; update the test in xp-route.test.ts (the 'falls
back to token.sub when token.id is unavailable' case) to expect a 401 response
instead of 200, assert that mockProxyToBotApi is not called (or not called with
a proxied header), and remove or change the header assertion against
mockProxyToBotApi.mock.calls[0][4].headers; keep the use of mockGetToken, POST,
makeRequest, makeParams and mockProxyToBotApi to locate and modify this test.
---
Duplicate comments:
In `@web/src/app/api/guilds/`[guildId]/members/[userId]/xp/route.ts:
- Around line 106-111: The code currently falls back to token.sub when
determining requesterId; remove that fallback and require token.id only (use
getToken result), so if token?.id is not a string the handler returns the 401
via NextResponse.json immediately; replace the nested ternary that sets
requesterId with a clear if-check that assigns requesterId = token?.id when it's
a string and otherwise returns the 401 error response (update the block around
getToken, requesterId and the existing NextResponse.json call accordingly).
In `@web/tests/api/xp-route.test.ts`:
- Line 59: Replace the invalid token mock value 'moderator-1' used in
mockGetToken.mockResolvedValue with a valid Discord snowflake string (e.g., a
17–20 digit numeric string like '12345678901234567') so the bot API middleware's
regex validation is exercised; update any related assertions that expect
behaviors tied to the mock id (the assertions around the test that reference the
mocked moderator id—previously at the assertion block for lines ~186-187 and the
similar block at ~180-188) to use the new numeric snowflake value (referencing
mockGetToken and the test's expectation/assert calls) so the test verifies the
real cross-service 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa904439-416d-46fa-b0f0-57a56f9aed6e
📒 Files selected for processing (8)
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/route.tsweb/src/components/ui/discord-markdown-editor.tsxweb/src/components/ui/embed-builder.tsxweb/tests/api/guilds.test.tsweb/tests/api/xp-route.test.tsweb/tests/components/ui/discord-markdown-editor.test.tsxweb/tests/components/ui/embed-builder.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx}: Use single quotes for strings (except in JSON files); no double quotes
Always include semicolons at the end of statements
Use 2-space indentation (spaces, not tabs)
Always include trailing commas in multi-line arrays, objects, and function parameters
Maintain a maximum line width of 100 characters
Files:
web/tests/components/ui/discord-markdown-editor.test.tsxweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/ui/discord-markdown-editor.tsxweb/tests/components/ui/embed-builder.test.tsxweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.tsweb/tests/api/guilds.test.tsweb/src/components/ui/embed-builder.tsx
web/tests/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/tests/**/*.test.{ts,tsx}: Write web dashboard tests using Vitest 4 with thejsdomenvironment and React Testing Library, matching theweb/src/structure
Maintain test coverage thresholds of 85% across all metrics (statements, branches, functions, lines) for web dashboard tests
Files:
web/tests/components/ui/discord-markdown-editor.test.tsxweb/tests/components/ui/embed-builder.test.tsxweb/tests/api/xp-route.test.tsweb/tests/api/guilds.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: ESM only - do not use CommonJS modules
Use src/logger.js; do not use console.*
Files:
web/tests/components/ui/discord-markdown-editor.test.tsxweb/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/ui/discord-markdown-editor.tsxweb/tests/components/ui/embed-builder.test.tsxweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.tsweb/tests/api/guilds.test.tsweb/src/components/ui/embed-builder.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use
console.*methods in web dashboard code; use appropriate logging mechanisms for React applications
Files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/components/ui/discord-markdown-editor.tsxweb/src/app/api/guilds/route.tsweb/src/components/ui/embed-builder.tsx
**/*.{js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use parameterized SQL only; do not use string concatenation for SQL queries
Files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.tsweb/tests/api/guilds.test.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to src/**/*.{js,ts} : Community features should be gated behind config.<feature>.enabled. Moderation commands are the exception.
📚 Learning: 2026-03-10T23:29:51.063Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:29:51.063Z
Learning: Applies to src/api/routes/*.js : Apply requireGuildModerator authorization check to DELETE endpoints that modify guild-scoped resources
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/routes/*.js : Add adaptDeleteGuildIdParam in API route handlers to enforce guild moderation checks on DELETE requests without losing record id
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/app/api/**/route.ts : Include guildId in signed WebSocket ticket payload when issuing tickets from dashboard endpoints
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/commands/**/*.js : Guild owners should be explicitly exempted from role hierarchy restrictions in command handlers to match Discord permission model expectations
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to src/api/ws/*.js : Bind audit-log WebSocket auth tickets to guild context by including nonce.expiry.guildId.hmac in ticket validation
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/tests/api/xp-route.test.tsweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-10T23:21:49.730Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T23:21:49.730Z
Learning: Applies to src/api/ws/**/*.js : Enforce tenant scoping on WebSocket handlers: reject mismatched guildId in filters and require entry.guild_id === ws.guildId for broadcast matching
Applied to files:
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.tsweb/src/app/api/guilds/route.ts
📚 Learning: 2026-03-12T02:03:36.493Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-12T02:03:36.493Z
Learning: Applies to **/*.{js,ts,tsx} : Maintain a maximum line width of 100 characters
Applied to files:
web/src/components/ui/discord-markdown-editor.tsxweb/src/components/ui/embed-builder.tsx
📚 Learning: 2026-03-26T00:04:14.693Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to src/**/*.{js,ts} : Community features should be gated behind config.<feature>.enabled. Moderation commands are the exception.
Applied to files:
web/src/app/api/guilds/route.ts
📚 Learning: 2026-03-11T17:18:17.626Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T17:18:17.626Z
Learning: Applies to src/**/{startup,command-register,reload}*.{js,ts} : Remove process.env.GUILD_ID runtime reads from bot startup and reload command registration
Applied to files:
web/tests/api/guilds.test.ts
📚 Learning: 2026-03-11T06:42:38.728Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-11T06:42:38.728Z
Learning: Applies to web/src/pages/dashboard/**/*.{ts,tsx} : Use shared title helpers from web/src/lib/page-titles.ts for setting browser titles in dashboard pages
Applied to files:
web/src/components/ui/embed-builder.tsx
📚 Learning: 2026-03-26T00:04:14.693Z
Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T00:04:14.693Z
Learning: Applies to src/**/*.{js,ts} : Use the safe Discord messaging helpers in src/utils/safeSend.js instead of raw reply/send/edit calls
Applied to files:
web/src/components/ui/embed-builder.tsx
🪛 GitHub Check: SonarCloud Code Analysis
web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts
[warning] 108-108: Extract this nested ternary operation into an independent statement.
web/src/app/api/guilds/route.ts
[warning] 105-105: Extract this nested ternary operation into an independent statement.
web/src/components/ui/embed-builder.tsx
[warning] 573-573: Unexpected negated condition.
[warning] 572-572: Unexpected negated condition.
🔇 Additional comments (22)
web/src/components/ui/embed-builder.tsx (4)
344-346: LGTM!Clean helper that correctly handles the edge case where
available <= 0by returning an empty string.
549-556: LGTM! This addresses the prior review concern about emitting configs that exceed Discord's 6,000-character embed cap.The gate correctly prevents
onChangefrom receiving over-limit configs. Users still get visual feedback via theCharCountindicators.
602-614: LGTM!Clean delegation to the new
commitandcommitFieldPatchfunctions, with correct dependency arrays.
616-642: LGTM!Field array operations correctly routed through
commit()for consistency, even thoughremoveFieldandmoveFieldcan never increase the total character count.web/tests/components/ui/embed-builder.test.tsx (2)
24-47: LGTM!Solid test helper that wraps
EmbedBuilderin a controlled component, enabling assertions on both theonChangemock and the actual emitted state. This pattern correctly handles React's state updates during user interactions.
367-399: Good regression coverage for character clamping.The test cases correctly verify the boundary conditions:
- Line 371-372:
4096 + 1900 = 5996, leaving exactly 4 characters for the title →'abcd'- Line 387-389:
256 + 4096 + 1647 = 5999, leaving 1 character for the field name →'a'web/src/components/ui/discord-markdown-editor.tsx (3)
65-83: LGTM!Clean implementation that truncates text and clamps all selection/cursor indices to stay within bounds. The early return for compliant results avoids unnecessary object creation.
227-272: LGTM! This addresses the prior review concern about enforcingmaxLengthon toolbar and variable insertion edits.Both
applyActionandinsertVariablenow route results throughclampEditResultbefore callingonChange, ensuring programmatic edits respect the character limit. The dependency arrays are correctly updated to includemaxLength.
379-389: LGTM!The native
maxLengthattribute is now set on the textarea, providing browser-level enforcement for direct typing alongside the programmatic clamping for toolbar actions.web/tests/components/ui/discord-markdown-editor.test.tsx (1)
226-265: LGTM! Good coverage for the newmaxLengthenforcement.The tests correctly verify:
- Native
maxlengthattribute propagation- Toolbar edits clamped mid-syntax (
"**hell"formaxLength={6})- Variable insertion clamped mid-token (
"hello{{u"formaxLength={8})Note: The truncation can produce syntactically invalid markdown (unclosed markers), but this is acceptable given the hard limit enforcement takes priority over syntax validity.
web/src/app/api/guilds/route.ts (3)
104-106: Extract the nested ternary for readability.The nested ternary for
userIdderivation was previously flagged by SonarCloud and in prior review. Consider extracting to an explicit conditional block.♻️ Suggested refactor
- const userId = - typeof token.id === 'string' ? token.id : typeof token.sub === 'string' ? token.sub : ''; + let userId = ''; + if (typeof token.id === 'string') { + userId = token.id; + } else if (typeof token.sub === 'string') { + userId = token.sub; + } const guildsWithAccess = userId ? await applyAccessLevels(guilds, userId, signal) : guilds;
12-13: LGTM! Access level validation constants are well-defined.The
VALID_ACCESS_LEVELSSet properly constrains accepted values, addressing the schema drift concern. The batch size constant is appropriately scoped.
15-87: LGTM! Well-structured access level augmentation with proper validation and error handling.The implementation correctly:
- Returns early when config is missing or no bot-present guilds exist
- Batches requests to respect the 100-guild API cap
- Validates each entry against
VALID_ACCESS_LEVELSbefore mapping- Falls back gracefully to original guilds on any error
web/tests/api/guilds.test.ts (5)
25-28: LGTM! Mock setup follows established pattern.The
mockGetBotApiBaseUrlwrapper pattern correctly allows per-test return value configuration, consistent with other mocks in this file.
38-67: LGTM! Comprehensive setup and teardown prevents test pollution.The test now properly saves and restores
BOT_API_URL,BOT_API_SECRET,globalThis.fetch, andNEXTAUTH_SECRET, addressing the prior concern about globals leaking between tests.
149-193: LGTM! Thorough test for access level augmentation.The test correctly verifies:
- Bot API is called when
BOT_API_SECRETand base URL are configured- Response includes the augmented
accessvalue from the bot API- Request includes the
x-api-secretheader
195-234: LGTM! Good edge case coverage for invalid access values.This test validates that unknown
accessvalues from the bot API are filtered out and the original guildaccessis preserved, ensuring theVALID_ACCESS_LEVELScheck works correctly.
236-277: LGTM! Effective batching verification.The test creates 205 guilds and correctly asserts that:
fetchis called exactly 3 times- The
guildIdsquery parameter sizes are[100, 100, 5]This validates the
MAX_ACCESS_LOOKUP_GUILDSchunking logic inapplyAccessLevels.web/src/app/api/guilds/[guildId]/members/[userId]/xp/route.ts (2)
1-9: LGTM: Authorization downgrade to moderator level is appropriate.The switch from
authorizeGuildAdmintoauthorizeGuildModeratorcorrectly aligns with the PR objective to allow moderators (not just admins) to adjust member XP. The relevant context snippet confirmsauthorizeGuildModeratorpasses 'moderator' as the required access level, which admins and bot-owners also satisfy via superset logic.Also applies to: 39-40
113-120: LGTM: Proxy call correctly forwards the user ID header.The
proxyToBotApifunction (per context snippet 3) correctly spreads caller-supplied headers before adding the auth secret, sox-discord-user-idwill be forwarded to the bot API.web/tests/api/xp-route.test.ts (2)
6-18: LGTM: Mock setup correctly mirrors the route changes.The hoisted mocks are properly updated to reflect the authorization change from admin to moderator, and the
getTokenmock is correctly wired.Also applies to: 20-29
202-209: LGTM: Correctly verifies 401 when no user ID can be determined.This test properly validates the fail-closed behavior when authentication data is missing.
Signed-off-by: MohsinCoding <91380732+MohsinCoding@users.noreply.github.com>
|
| @@ -244,7 +264,7 @@ export function DiscordMarkdownEditor({ | |||
| if (!textarea) return; | |||
|
|
|||
| const cursor = textarea.selectionStart; | |||
| const result = insertAtCursor(value, cursor, `${varOpen}${varName}${varClose}`); | |||
| const result = clampEditResult(insertAtCursor(value, cursor, `{{${varName}}}`), maxLength); | |||
|
|
|||
There was a problem hiding this comment.
insertVariable ignores the variableDelimiters prop and always inserts {{...}} (hard-coded), which breaks custom delimiter configurations and makes the UI inconsistent with the prop contract. Use the [varOpen, varClose] values when building the inserted variable string so toolbar insertion matches the configured delimiters.
| /** | ||
| * Repair migration for audit_logs schema drift. | ||
| * | ||
| * Background: | ||
| * `013_audit_log.cjs` now creates `audit_logs.user_tag`, but some databases | ||
| * already had an older `audit_logs` table created before the `user_tag` | ||
| * column existed. Because `013_audit_log.cjs` uses `ifNotExists`, those | ||
| * existing tables do not receive the new column automatically. | ||
| * | ||
| * Purpose: | ||
| * Preserve the historical `014_*` slot already recorded in some databases | ||
| * and backfill the missing column/index when needed. |
There was a problem hiding this comment.
This migration’s header comment references 013_audit_log.cjs, but the repository’s migrations directory does not contain that migration and the base schema migration already defines audit_logs.user_tag and idx_audit_logs_guild_user (migrations/001_initial-schema.cjs). Please update the comment to reflect the actual migration history in this repo so future debugging doesn’t chase a non-existent migration.
Summary
This PR tightens and clarifies dashboard access behavior across the bot API and web app.
Changes
014migration and moving perf indexes to015file://URLs